[dpdk-dev,3/4] kni: replace strict_strtoul with kstrtoul

Message ID 1418182383-5744-4-git-send-email-jmiao@redhat.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Jincheng Miao Dec. 10, 2014, 3:33 a.m. UTC
From upstream kernel commit 3db2e9cd, strict_strto* serial functions
are removed. So that we should directly used kstrtoul instead.

And add kni/compat.h for be compatible with older kernel.

Signed-off-by: Jincheng Miao <jmiao@redhat.com>
---
 lib/librte_eal/linuxapp/kni/compat.h    |   16 ++++++++++++++++
 lib/librte_eal/linuxapp/kni/kni_vhost.c |    2 +-
 2 files changed, 17 insertions(+), 1 deletions(-)
 create mode 100644 lib/librte_eal/linuxapp/kni/compat.h
  

Comments

Thomas Monjalon Dec. 10, 2014, 8:39 a.m. UTC | #1
Hi Jincheng,

2014-12-10 11:33, Jincheng Miao:
> From upstream kernel commit 3db2e9cd, strict_strto* serial functions
> are removed. So that we should directly used kstrtoul instead.
> 
> And add kni/compat.h for be compatible with older kernel.
> 
> Signed-off-by: Jincheng Miao <jmiao@redhat.com>
[...]
> new file mode 100644
> index 0000000..c8c662c
> --- /dev/null
> +++ b/lib/librte_eal/linuxapp/kni/compat.h
> @@ -0,0 +1,16 @@
> +/*
> + * Minimal wrappers to allow compiling kni on older kernels.
> + */
> +
> +#ifndef RHEL_RELEASE_VERSION
> +#define RHEL_RELEASE_VERSION(a, b) (((a) << 8) + (b))
> +#endif
> +
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 39) && \
> +        (!(defined(RHEL_RELEASE_CODE) && \
> +           RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(6, 4)))

The first indent character must be a tab (highlighted by checkpatch).

[...]
> --- a/lib/librte_eal/linuxapp/kni/kni_vhost.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_vhost.c
> @@ -740,7 +740,7 @@ set_sock_en(struct device *dev, struct device_attribute *attr,
>  	unsigned long en;
>  	int err = 0;
>  
> -	if (0 != strict_strtoul(buf, 0, &en))
> +	if (0 != kstrtoul(buf, 0, &en))
>  		return -EINVAL;
>  
>  	if (en)

It seems you forgot to include the new compat.h.

Did you do some tests with different Fedora/RHEL versions?

Thanks
  
Jincheng Miao Dec. 10, 2014, 8:48 a.m. UTC | #2
----- Original Message -----
> Hi Jincheng,
> 
> 2014-12-10 11:33, Jincheng Miao:
> > From upstream kernel commit 3db2e9cd, strict_strto* serial functions
> > are removed. So that we should directly used kstrtoul instead.
> > 
> > And add kni/compat.h for be compatible with older kernel.
> > 
> > Signed-off-by: Jincheng Miao <jmiao@redhat.com>
> [...]
> > new file mode 100644
> > index 0000000..c8c662c
> > --- /dev/null
> > +++ b/lib/librte_eal/linuxapp/kni/compat.h
> > @@ -0,0 +1,16 @@
> > +/*
> > + * Minimal wrappers to allow compiling kni on older kernels.
> > + */
> > +
> > +#ifndef RHEL_RELEASE_VERSION
> > +#define RHEL_RELEASE_VERSION(a, b) (((a) << 8) + (b))
> > +#endif
> > +
> > +#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 39) && \
> > +        (!(defined(RHEL_RELEASE_CODE) && \
> > +           RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(6, 4)))
> 
> The first indent character must be a tab (highlighted by checkpatch).

Yes, I think the TAB is replaced by my vim :(

> 
> [...]
> > --- a/lib/librte_eal/linuxapp/kni/kni_vhost.c
> > +++ b/lib/librte_eal/linuxapp/kni/kni_vhost.c
> > @@ -740,7 +740,7 @@ set_sock_en(struct device *dev, struct device_attribute
> > *attr,
> >  	unsigned long en;
> >  	int err = 0;
> >  
> > -	if (0 != strict_strtoul(buf, 0, &en))
> > +	if (0 != kstrtoul(buf, 0, &en))
> >  		return -EINVAL;
> >  
> >  	if (en)
> 
> It seems you forgot to include the new compat.h.
> 
> Did you do some tests with different Fedora/RHEL versions?

Yes, missing compat.h in kni_vhost.c.


And, I want to get your opinion about adding compat.h to kni and xen_dom0.
The pros: easy to implement and minimal wrapper for older kernel.
The cons: there is so many compat.h, and the file kcompat.h also makes user confuse.


> 
> Thanks
> --
> Thomas
>
  
Thomas Monjalon Dec. 10, 2014, 9:18 a.m. UTC | #3
2014-12-10 03:48, Jincheng Miao:
> > It seems you forgot to include the new compat.h.
> > 
> > Did you do some tests with different Fedora/RHEL versions?
> 
> Yes, missing compat.h in kni_vhost.c.
> 
> 
> And, I want to get your opinion about adding compat.h to kni and xen_dom0.
> The pros: easy to implement and minimal wrapper for older kernel.

Yes I think it's the good approach.

> The cons: there is so many compat.h, and the file kcompat.h also makes user confuse.

Why kcompat makes user confuse?

Do you think you could send a new version quickly to integrate it in the
next RC (probably today)?

Please test it with RHEL.

Thanks
  
Jincheng Miao Dec. 10, 2014, 9:39 a.m. UTC | #4
----- Original Message -----
> 2014-12-10 03:48, Jincheng Miao:
> > > It seems you forgot to include the new compat.h.
> > > 
> > > Did you do some tests with different Fedora/RHEL versions?
> > 
> > Yes, missing compat.h in kni_vhost.c.
> > 
> > 
> > And, I want to get your opinion about adding compat.h to kni and xen_dom0.
> > The pros: easy to implement and minimal wrapper for older kernel.
> 
> Yes I think it's the good approach.
> 
> > The cons: there is so many compat.h, and the file kcompat.h also makes user
> > confuse.
> 
> Why kcompat makes user confuse?

Because a lot of compat fallback in kcompat too.
For example, for kni, there is compat.h and kcompat.h.
I just confuse about it before.
But if you agree it, I will also agree it.

> 
> Do you think you could send a new version quickly to integrate it in the
> next RC (probably today)?
> 
> Please test it with RHEL.

Yes, I am working on it.

I will report my test result latter.

Jincheng Miao

> 
> Thanks
> --
> Thomas
>
  

Patch

diff --git a/lib/librte_eal/linuxapp/kni/compat.h b/lib/librte_eal/linuxapp/kni/compat.h
new file mode 100644
index 0000000..c8c662c
--- /dev/null
+++ b/lib/librte_eal/linuxapp/kni/compat.h
@@ -0,0 +1,16 @@ 
+/*
+ * Minimal wrappers to allow compiling kni on older kernels.
+ */
+
+#ifndef RHEL_RELEASE_VERSION
+#define RHEL_RELEASE_VERSION(a, b) (((a) << 8) + (b))
+#endif
+
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 39) && \
+        (!(defined(RHEL_RELEASE_CODE) && \
+           RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(6, 4)))
+
+#define kstrtoul strict_strtoul
+
+#endif /* < 2.6.39 */
+
diff --git a/lib/librte_eal/linuxapp/kni/kni_vhost.c b/lib/librte_eal/linuxapp/kni/kni_vhost.c
index 7bcc985..c05c868 100644
--- a/lib/librte_eal/linuxapp/kni/kni_vhost.c
+++ b/lib/librte_eal/linuxapp/kni/kni_vhost.c
@@ -740,7 +740,7 @@  set_sock_en(struct device *dev, struct device_attribute *attr,
 	unsigned long en;
 	int err = 0;
 
-	if (0 != strict_strtoul(buf, 0, &en))
+	if (0 != kstrtoul(buf, 0, &en))
 		return -EINVAL;
 
 	if (en)