[dpdk-dev,v2] kni: fix build on SLE12 SP3

Message ID 20170829150613.10597-1-ndas@suse.de (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Nirmoy Das Aug. 29, 2017, 3:06 p.m. UTC
  compilation error:
build/lib/librte_eal/linuxapp/kni/kni_net.c:215:5: error:
‘struct net_device’ has no member named ‘trans_start’
  dev->trans_start = jiffies;

Signed-off-by: Nirmoy Das <ndas@suse.de>
---
 lib/librte_eal/linuxapp/kni/compat.h | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)
  

Comments

Stephen Hemminger Aug. 29, 2017, 3:12 p.m. UTC | #1
On Tue, 29 Aug 2017 17:06:13 +0200
Nirmoy Das <ndas@suse.de> wrote:

> compilation error:
> build/lib/librte_eal/linuxapp/kni/kni_net.c:215:5: error:
> ‘struct net_device’ has no member named ‘trans_start’
>   dev->trans_start = jiffies;
> 
> Signed-off-by: Nirmoy Das <ndas@suse.de>
> ---
>  lib/librte_eal/linuxapp/kni/compat.h | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/linuxapp/kni/compat.h b/lib/librte_eal/linuxapp/kni/compat.h
> index 6a1587b4e..19f8e96ce 100644
> --- a/lib/librte_eal/linuxapp/kni/compat.h
> +++ b/lib/librte_eal/linuxapp/kni/compat.h
> @@ -8,6 +8,34 @@
>  #define RHEL_RELEASE_VERSION(a, b) (((a) << 8) + (b))
>  #endif
>  
> +/* SuSE version macro is the same as Linux kernel version */
> +#ifndef SLE_VERSION
> +#define SLE_VERSION(a, b, c) KERNEL_VERSION(a, b, c)
> +#endif
> +#ifdef CONFIG_SUSE_KERNEL
> +#if (LINUX_VERSION_CODE >= KERNEL_VERSION(4, 4, 57))
> +/* SLES12SP3 is at least 4.4.57+ based */
> +#define SLE_VERSION_CODE SLE_VERSION(12, 3, 0)
> +#elif (LINUX_VERSION_CODE >= KERNEL_VERSION(3, 12, 28))
> +/* SLES12 is at least 3.12.28+ based */
> +#define SLE_VERSION_CODE SLE_VERSION(12, 0, 0)
> +#elif ((LINUX_VERSION_CODE >= KERNEL_VERSION(3, 0, 61)) && \
> +       (LINUX_VERSION_CODE < KERNEL_VERSION(3, 1, 0)))
> +/* SLES11 SP3 is at least 3.0.61+ based */
> +#define SLE_VERSION_CODE SLE_VERSION(11, 3, 0)
> +#elif (LINUX_VERSION_CODE == KERNEL_VERSION(2, 6, 32))
> +/* SLES11 SP1 is 2.6.32 based */
> +#define SLE_VERSION_CODE SLE_VERSION(11, 1, 0)
> +#elif (LINUX_VERSION_CODE == KERNEL_VERSION(2, 6, 27))
> +/* SLES11 GA is 2.6.27 based */
> +#define SLE_VERSION_CODE SLE_VERSION(11, 0, 0)
> +#endif /* LINUX_VERSION_CODE == KERNEL_VERSION(x,y,z) */
> +#endif /* CONFIG_SUSE_KERNEL */
> +#ifndef SLE_VERSION_CODE
> +#define SLE_VERSION_CODE 0
> +#endif /* SLE_VERSION_CODE */
> +
> +
>  #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 39) && \
>  	(!(defined(RHEL_RELEASE_CODE) && \
>  	   RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(6, 4)))
> @@ -55,7 +83,9 @@
>  
>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 7, 0) || \
>  	(defined(RHEL_RELEASE_CODE) && \
> -	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(7, 4))
> +	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(7, 4)) || \
> +	 (defined(SLE_VERSION_CODE) && \
> +	  SLE_VERSION_CODE == SLE_VERSION(12, 3, 0))
>  #define HAVE_TRANS_START_HELPER
>  #endif
>  

Sigh. KNI needs to get upstream, or we need to leave RHEL and SLES
version magic to the distros to manage as their own patches.

This kind of spaghetti of ifdef's is unmanageable, untestable, and 
just plain ugly.
  
Ferruh Yigit Sept. 4, 2017, 9:46 a.m. UTC | #2
On 8/29/2017 4:06 PM, Nirmoy Das wrote:
> compilation error:
> build/lib/librte_eal/linuxapp/kni/kni_net.c:215:5: error:
> ‘struct net_device’ has no member named ‘trans_start’
>   dev->trans_start = jiffies;
> 
> Signed-off-by: Nirmoy Das <ndas@suse.de>
> ---
>  lib/librte_eal/linuxapp/kni/compat.h | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/linuxapp/kni/compat.h b/lib/librte_eal/linuxapp/kni/compat.h
> index 6a1587b4e..19f8e96ce 100644
> --- a/lib/librte_eal/linuxapp/kni/compat.h
> +++ b/lib/librte_eal/linuxapp/kni/compat.h
> @@ -8,6 +8,34 @@
>  #define RHEL_RELEASE_VERSION(a, b) (((a) << 8) + (b))
>  #endif
>  
> +/* SuSE version macro is the same as Linux kernel version */
> +#ifndef SLE_VERSION
> +#define SLE_VERSION(a, b, c) KERNEL_VERSION(a, b, c)
> +#endif
> +#ifdef CONFIG_SUSE_KERNEL
> +#if (LINUX_VERSION_CODE >= KERNEL_VERSION(4, 4, 57))
> +/* SLES12SP3 is at least 4.4.57+ based */
> +#define SLE_VERSION_CODE SLE_VERSION(12, 3, 0)

Just to double check, is there a macro set in SUSE that we can use here,
instead of defining here ourselves, like RHEL_RELEASE_CODE?

> +#elif (LINUX_VERSION_CODE >= KERNEL_VERSION(3, 12, 28))
> +/* SLES12 is at least 3.12.28+ based */
> +#define SLE_VERSION_CODE SLE_VERSION(12, 0, 0)
> +#elif ((LINUX_VERSION_CODE >= KERNEL_VERSION(3, 0, 61)) && \
> +       (LINUX_VERSION_CODE < KERNEL_VERSION(3, 1, 0)))

This line gives following checkpatch warning:
WARNING:LEADING_SPACE: please, no spaces at the start of a line

> +/* SLES11 SP3 is at least 3.0.61+ based */
> +#define SLE_VERSION_CODE SLE_VERSION(11, 3, 0)
> +#elif (LINUX_VERSION_CODE == KERNEL_VERSION(2, 6, 32))
> +/* SLES11 SP1 is 2.6.32 based */
> +#define SLE_VERSION_CODE SLE_VERSION(11, 1, 0)
> +#elif (LINUX_VERSION_CODE == KERNEL_VERSION(2, 6, 27))
> +/* SLES11 GA is 2.6.27 based */
> +#define SLE_VERSION_CODE SLE_VERSION(11, 0, 0)
> +#endif /* LINUX_VERSION_CODE == KERNEL_VERSION(x,y,z) */
> +#endif /* CONFIG_SUSE_KERNEL */
> +#ifndef SLE_VERSION_CODE
> +#define SLE_VERSION_CODE 0

[1] see  below.

> +#endif /* SLE_VERSION_CODE */
> +
> +
>  #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 39) && \
>  	(!(defined(RHEL_RELEASE_CODE) && \
>  	   RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(6, 4)))
> @@ -55,7 +83,9 @@
>  
>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 7, 0) || \
>  	(defined(RHEL_RELEASE_CODE) && \
> -	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(7, 4))
> +	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(7, 4)) || \
> +	 (defined(SLE_VERSION_CODE) && \

defined check is not required, since SLE_VERSION_CODE always defined [1].

It can be either:
a) (SLE_VERSION_CODE && SLE_VERSION_CODE == SLE_VERSION(12, 3, 0))
or directly check:
b) (SLE_VERSION_CODE == SLE_VERSION(12, 3, 0))

> +	  SLE_VERSION_CODE == SLE_VERSION(12, 3, 0))
>  #define HAVE_TRANS_START_HELPER
>  #endif
>  
>
  
Ferruh Yigit Sept. 4, 2017, 9:47 a.m. UTC | #3
On 8/29/2017 4:12 PM, Stephen Hemminger wrote:
> On Tue, 29 Aug 2017 17:06:13 +0200
> Nirmoy Das <ndas@suse.de> wrote:
> 
>> compilation error:
>> build/lib/librte_eal/linuxapp/kni/kni_net.c:215:5: error:
>> ‘struct net_device’ has no member named ‘trans_start’
>>   dev->trans_start = jiffies;
>>
>> Signed-off-by: Nirmoy Das <ndas@suse.de>

<...>

> Sigh. KNI needs to get upstream, or we need to leave RHEL and SLES
> version magic to the distros to manage as their own patches.
> 
> This kind of spaghetti of ifdef's is unmanageable, untestable, and 
> just plain ugly.

I agree with above comment, meanwhile I believe we need to support this
as long as maintained in DPDK.
  
Nirmoy Das Sept. 5, 2017, 9:55 a.m. UTC | #4
On 09/04/2017 11:46 AM, Ferruh Yigit wrote:
> On 8/29/2017 4:06 PM, Nirmoy Das wrote:
>> compilation error:
>> build/lib/librte_eal/linuxapp/kni/kni_net.c:215:5: error:
>> ‘struct net_device’ has no member named ‘trans_start’
>>   dev->trans_start = jiffies;
>>
>> Signed-off-by: Nirmoy Das <ndas@suse.de>
>> ---
>>  lib/librte_eal/linuxapp/kni/compat.h | 32 +++++++++++++++++++++++++++++++-
>>  1 file changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_eal/linuxapp/kni/compat.h b/lib/librte_eal/linuxapp/kni/compat.h
>> index 6a1587b4e..19f8e96ce 100644
>> --- a/lib/librte_eal/linuxapp/kni/compat.h
>> +++ b/lib/librte_eal/linuxapp/kni/compat.h
>> @@ -8,6 +8,34 @@
>>  #define RHEL_RELEASE_VERSION(a, b) (((a) << 8) + (b))
>>  #endif
>>  
>> +/* SuSE version macro is the same as Linux kernel version */
>> +#ifndef SLE_VERSION
>> +#define SLE_VERSION(a, b, c) KERNEL_VERSION(a, b, c)
>> +#endif
>> +#ifdef CONFIG_SUSE_KERNEL
>> +#if (LINUX_VERSION_CODE >= KERNEL_VERSION(4, 4, 57))
>> +/* SLES12SP3 is at least 4.4.57+ based */
>> +#define SLE_VERSION_CODE SLE_VERSION(12, 3, 0)
> Just to double check, is there a macro set in SUSE that we can use here,
> instead of defining here ourselves, like RHEL_RELEASE_CODE?
Unfortunately SUSE doesn't have such logic/macro.
>
>> +#elif (LINUX_VERSION_CODE >= KERNEL_VERSION(3, 12, 28))
>> +/* SLES12 is at least 3.12.28+ based */
>> +#define SLE_VERSION_CODE SLE_VERSION(12, 0, 0)
>> +#elif ((LINUX_VERSION_CODE >= KERNEL_VERSION(3, 0, 61)) && \
>> +       (LINUX_VERSION_CODE < KERNEL_VERSION(3, 1, 0)))
> This line gives following checkpatch warning:
> WARNING:LEADING_SPACE: please, no spaces at the start of a line
>
>> +/* SLES11 SP3 is at least 3.0.61+ based */
>> +#define SLE_VERSION_CODE SLE_VERSION(11, 3, 0)
>> +#elif (LINUX_VERSION_CODE == KERNEL_VERSION(2, 6, 32))
>> +/* SLES11 SP1 is 2.6.32 based */
>> +#define SLE_VERSION_CODE SLE_VERSION(11, 1, 0)
>> +#elif (LINUX_VERSION_CODE == KERNEL_VERSION(2, 6, 27))
>> +/* SLES11 GA is 2.6.27 based */
>> +#define SLE_VERSION_CODE SLE_VERSION(11, 0, 0)
>> +#endif /* LINUX_VERSION_CODE == KERNEL_VERSION(x,y,z) */
>> +#endif /* CONFIG_SUSE_KERNEL */
>> +#ifndef SLE_VERSION_CODE
>> +#define SLE_VERSION_CODE 0
> [1] see  below.
>
>> +#endif /* SLE_VERSION_CODE */
>> +
>> +
>>  #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 39) && \
>>  	(!(defined(RHEL_RELEASE_CODE) && \
>>  	   RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(6, 4)))
>> @@ -55,7 +83,9 @@
>>  
>>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 7, 0) || \
>>  	(defined(RHEL_RELEASE_CODE) && \
>> -	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(7, 4))
>> +	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(7, 4)) || \
>> +	 (defined(SLE_VERSION_CODE) && \
> defined check is not required, since SLE_VERSION_CODE always defined [1].
>
> It can be either:
> a) (SLE_VERSION_CODE && SLE_VERSION_CODE == SLE_VERSION(12, 3, 0))
> or directly check:
> b) (SLE_VERSION_CODE == SLE_VERSION(12, 3, 0))
Thanks for your comments I will modify the patch.
>
>> +	  SLE_VERSION_CODE == SLE_VERSION(12, 3, 0))
>>  #define HAVE_TRANS_START_HELPER
>>  #endif
>>  
>>
Regards,
Nirmoy
  
Ferruh Yigit Oct. 6, 2017, 9:42 p.m. UTC | #5
On 9/5/2017 10:55 AM, Nirmoy Das wrote:
> 
> 
> On 09/04/2017 11:46 AM, Ferruh Yigit wrote:
>> On 8/29/2017 4:06 PM, Nirmoy Das wrote:
>>> compilation error:
>>> build/lib/librte_eal/linuxapp/kni/kni_net.c:215:5: error:
>>> ‘struct net_device’ has no member named ‘trans_start’
>>>   dev->trans_start = jiffies;
>>>
>>> Signed-off-by: Nirmoy Das <ndas@suse.de>

<...>

>>> @@ -55,7 +83,9 @@
>>>  
>>>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 7, 0) || \
>>>  	(defined(RHEL_RELEASE_CODE) && \
>>> -	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(7, 4))
>>> +	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(7, 4)) || \
>>> +	 (defined(SLE_VERSION_CODE) && \
>> defined check is not required, since SLE_VERSION_CODE always defined [1].
>>
>> It can be either:
>> a) (SLE_VERSION_CODE && SLE_VERSION_CODE == SLE_VERSION(12, 3, 0))
>> or directly check:
>> b) (SLE_VERSION_CODE == SLE_VERSION(12, 3, 0))
> Thanks for your comments I will modify the patch.

Hi Nirmoy,

Reminder of this patch, would be able to send a new version?

Thanks,
ferruh

>>
>>> +	  SLE_VERSION_CODE == SLE_VERSION(12, 3, 0))
>>>  #define HAVE_TRANS_START_HELPER
>>>  #endif
>>>  
>>>
> Regards,
> Nirmoy
> 
>
  

Patch

diff --git a/lib/librte_eal/linuxapp/kni/compat.h b/lib/librte_eal/linuxapp/kni/compat.h
index 6a1587b4e..19f8e96ce 100644
--- a/lib/librte_eal/linuxapp/kni/compat.h
+++ b/lib/librte_eal/linuxapp/kni/compat.h
@@ -8,6 +8,34 @@ 
 #define RHEL_RELEASE_VERSION(a, b) (((a) << 8) + (b))
 #endif
 
+/* SuSE version macro is the same as Linux kernel version */
+#ifndef SLE_VERSION
+#define SLE_VERSION(a, b, c) KERNEL_VERSION(a, b, c)
+#endif
+#ifdef CONFIG_SUSE_KERNEL
+#if (LINUX_VERSION_CODE >= KERNEL_VERSION(4, 4, 57))
+/* SLES12SP3 is at least 4.4.57+ based */
+#define SLE_VERSION_CODE SLE_VERSION(12, 3, 0)
+#elif (LINUX_VERSION_CODE >= KERNEL_VERSION(3, 12, 28))
+/* SLES12 is at least 3.12.28+ based */
+#define SLE_VERSION_CODE SLE_VERSION(12, 0, 0)
+#elif ((LINUX_VERSION_CODE >= KERNEL_VERSION(3, 0, 61)) && \
+       (LINUX_VERSION_CODE < KERNEL_VERSION(3, 1, 0)))
+/* SLES11 SP3 is at least 3.0.61+ based */
+#define SLE_VERSION_CODE SLE_VERSION(11, 3, 0)
+#elif (LINUX_VERSION_CODE == KERNEL_VERSION(2, 6, 32))
+/* SLES11 SP1 is 2.6.32 based */
+#define SLE_VERSION_CODE SLE_VERSION(11, 1, 0)
+#elif (LINUX_VERSION_CODE == KERNEL_VERSION(2, 6, 27))
+/* SLES11 GA is 2.6.27 based */
+#define SLE_VERSION_CODE SLE_VERSION(11, 0, 0)
+#endif /* LINUX_VERSION_CODE == KERNEL_VERSION(x,y,z) */
+#endif /* CONFIG_SUSE_KERNEL */
+#ifndef SLE_VERSION_CODE
+#define SLE_VERSION_CODE 0
+#endif /* SLE_VERSION_CODE */
+
+
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 39) && \
 	(!(defined(RHEL_RELEASE_CODE) && \
 	   RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(6, 4)))
@@ -55,7 +83,9 @@ 
 
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 7, 0) || \
 	(defined(RHEL_RELEASE_CODE) && \
-	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(7, 4))
+	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(7, 4)) || \
+	 (defined(SLE_VERSION_CODE) && \
+	  SLE_VERSION_CODE == SLE_VERSION(12, 3, 0))
 #define HAVE_TRANS_START_HELPER
 #endif