[dpdk-dev,v8,01/25] eal: define macro container_of

  Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 lib/librte_eal/common/include/rte_common.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
  

On 8/26/2016 2:56 PM, Shreyansh Jain wrote:
> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> ---
>  lib/librte_eal/common/include/rte_common.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
> index 332f2a4..a9b6792 100644
> --- a/lib/librte_eal/common/include/rte_common.h
> +++ b/lib/librte_eal/common/include/rte_common.h
> @@ -322,6 +322,22 @@ rte_bsf32(uint32_t v)
>  #define offsetof(TYPE, MEMBER)  __builtin_offsetof (TYPE, MEMBER)
>  #endif
>  
> +/**
> + * Return pointer to the wrapping struct instance.
> + * Example:
> + *
> + *  struct wrapper {
> + *      ...
> + *      struct child c;
> + *      ...
> + *  };
> + *
> + *  struct child *x = obtain(...);
> + *  struct wrapper *w = container_of(x, struct wrapper, c);
> + */
> +#define container_of(p, type, member) \
> +	((type *) (((char *) (p)) - offsetof(type, member)))
> +
>  #define _RTE_STR(x) #x
>  /** Take a macro value and get a string version of it */
>  #define RTE_STR(x) _RTE_STR(x)
> 

This gives compilation error for mlx5, because the libraries mlx depends
defines same macro:
.../rte_common.h:338:9: error: 'container_of' macro redefined
/usr/include/infiniband/verbs.h:77:9: note: previous definition is here

Does it make sense to protect macro with
#ifndef container_of
...
#endif

OR

add a dpdk prefix?


Regards,
ferruh
  

Hi Ferruh,

On Monday 29 August 2016 10:13 PM, Ferruh Yigit wrote:
> On 8/26/2016 2:56 PM, Shreyansh Jain wrote:
>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>> ---
>>  lib/librte_eal/common/include/rte_common.h | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
>> index 332f2a4..a9b6792 100644
>> --- a/lib/librte_eal/common/include/rte_common.h
>> +++ b/lib/librte_eal/common/include/rte_common.h
>> @@ -322,6 +322,22 @@ rte_bsf32(uint32_t v)
>>  #define offsetof(TYPE, MEMBER)  __builtin_offsetof (TYPE, MEMBER)
>>  #endif
>>
>> +/**
>> + * Return pointer to the wrapping struct instance.
>> + * Example:
>> + *
>> + *  struct wrapper {
>> + *      ...
>> + *      struct child c;
>> + *      ...
>> + *  };
>> + *
>> + *  struct child *x = obtain(...);
>> + *  struct wrapper *w = container_of(x, struct wrapper, c);
>> + */
>> +#define container_of(p, type, member) \
>> +	((type *) (((char *) (p)) - offsetof(type, member)))
>> +
>>  #define _RTE_STR(x) #x
>>  /** Take a macro value and get a string version of it */
>>  #define RTE_STR(x) _RTE_STR(x)
>>
>
> This gives compilation error for mlx5, because the libraries mlx depends
> defines same macro:
> ..../rte_common.h:338:9: error: 'container_of' macro redefined
> /usr/include/infiniband/verbs.h:77:9: note: previous definition is here

I thought testing with scripts/test-build.sh and default configuration 
would compile all drivers - I was wrong. I will retest the patches and 
release again.

Is there a better way to test that no driver breaks? Any particular 
parameters I should use for test-build.sh?

I used 'x86_64-native-linuxapp-gcc+default+debug+shared' for all patches.

>
> Does it make sense to protect macro with
> #ifndef container_of
> ....
> #endif
>
> OR
>
> add a dpdk prefix?
>
>
> Regards,
> ferruh
>

-
Shreyansh
  

Hi Ferruh,

Forgot to add a comment in previous reply to this email:

On Monday 29 August 2016 10:13 PM, Ferruh Yigit wrote:
> On 8/26/2016 2:56 PM, Shreyansh Jain wrote:
>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>> ---
>>  lib/librte_eal/common/include/rte_common.h | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
>> index 332f2a4..a9b6792 100644
>> --- a/lib/librte_eal/common/include/rte_common.h
>> +++ b/lib/librte_eal/common/include/rte_common.h
>> @@ -322,6 +322,22 @@ rte_bsf32(uint32_t v)
>>  #define offsetof(TYPE, MEMBER)  __builtin_offsetof (TYPE, MEMBER)
>>  #endif
>>
>> +/**
>> + * Return pointer to the wrapping struct instance.
>> + * Example:
>> + *
>> + *  struct wrapper {
>> + *      ...
>> + *      struct child c;
>> + *      ...
>> + *  };
>> + *
>> + *  struct child *x = obtain(...);
>> + *  struct wrapper *w = container_of(x, struct wrapper, c);
>> + */
>> +#define container_of(p, type, member) \
>> +	((type *) (((char *) (p)) - offsetof(type, member)))
>> +
>>  #define _RTE_STR(x) #x
>>  /** Take a macro value and get a string version of it */
>>  #define RTE_STR(x) _RTE_STR(x)
>>
>
> This gives compilation error for mlx5, because the libraries mlx depends
> defines same macro:
> ..../rte_common.h:338:9: error: 'container_of' macro redefined
> /usr/include/infiniband/verbs.h:77:9: note: previous definition is here
>
> Does it make sense to protect macro with
> #ifndef container_of
> ....
> #endif

Sounds good - probably would prevent double definition in future if 
someone includes any linux header having similar definition.

Generally the container_of definitions are consistent - that is, they 
would invariably use the offsetof from member. In which case, creating a 
new dpdk_* would only duplicate. Thus, I prefer the #ifndef.

>
> OR
>
> add a dpdk prefix?
>
>
> Regards,
> ferruh
>

-
Shreyansh
  

2016-08-30 09:57, Shreyansh Jain:
> On Monday 29 August 2016 10:13 PM, Ferruh Yigit wrote:
> > This gives compilation error for mlx5, because the libraries mlx depends
> > defines same macro:
> > ..../rte_common.h:338:9: error: 'container_of' macro redefined
> > /usr/include/infiniband/verbs.h:77:9: note: previous definition is here
> 
> I thought testing with scripts/test-build.sh and default configuration 
> would compile all drivers - I was wrong. I will retest the patches and 
> release again.
> 
> Is there a better way to test that no driver breaks? Any particular 
> parameters I should use for test-build.sh?

Yes I suggest to create a file ~/.config/dpdk/devel.config to adapt the
configuration to your system.
Once you have installed the required dependencies, you can make this kind
of configuration:

mlxdep=$root/mlx/mofed-3.3-1.0.0.0
szedep=$root/sze/usr-1.1.4
if echo $DPDK_TARGET | grep -q '^x86_64' ; then
    export DPDK_DEP_ARCHIVE=y
    export DPDK_DEP_ZLIB=y
    export DPDK_DEP_PCAP=y
    export DPDK_DEP_SSL=y
    export DPDK_DEP_MOFED=y
    export DPDK_DEP_SZE=y
    export DPDK_DEP_CFLAGS="-I$mlxdep/include -I$szedep/include"
    export DPDK_DEP_LDFLAGS="-L$mlxdep/lib -L$szedep/lib64 -rpath=$szedep/lib64"
    export AESNI_MULTI_BUFFER_LIB_PATH=$root/aesni/ipsec-043
    export LIBSSO_SNOW3G_PATH=$root/libsso/libsso-snow3g-0.3.1
    export LIBSSO_KASUMI_PATH=$root/libsso/libsso-kasumi-0.3.1
fi

> I used 'x86_64-native-linuxapp-gcc+default+debug+shared' for all patches.

It is a good idea to test also with clang (x86_64-native-linuxapp-clang)
and another arch (e.g. arm64-thunderx-linuxapp-gcc).
  

Hi Thomas,

On Tuesday 30 August 2016 04:00 PM, Thomas Monjalon wrote:
> 2016-08-30 09:57, Shreyansh Jain:
>> On Monday 29 August 2016 10:13 PM, Ferruh Yigit wrote:
>>> This gives compilation error for mlx5, because the libraries mlx depends
>>> defines same macro:
>>> ..../rte_common.h:338:9: error: 'container_of' macro redefined
>>> /usr/include/infiniband/verbs.h:77:9: note: previous definition is here
>>
>> I thought testing with scripts/test-build.sh and default configuration
>> would compile all drivers - I was wrong. I will retest the patches and
>> release again.
>>
>> Is there a better way to test that no driver breaks? Any particular
>> parameters I should use for test-build.sh?
>
> Yes I suggest to create a file ~/.config/dpdk/devel.config to adapt the
> configuration to your system.
> Once you have installed the required dependencies, you can make this kind
> of configuration:

Ok.

>
> mlxdep=$root/mlx/mofed-3.3-1.0.0.0
> szedep=$root/sze/usr-1.1.4

What does '$root' here refer to?
I am assuming 'mofed-3.3-1.0.0.0' and 'usr-1.1.4' are part of some 
dependencies that I should be revolving. Is that so?
As of now I don't have much idea about this - I will have a look and 
ping back in case I am stuck.

> if echo $DPDK_TARGET | grep -q '^x86_64' ; then
>     export DPDK_DEP_ARCHIVE=y
>     export DPDK_DEP_ZLIB=y
>     export DPDK_DEP_PCAP=y
>     export DPDK_DEP_SSL=y
>     export DPDK_DEP_MOFED=y
>     export DPDK_DEP_SZE=y
>     export DPDK_DEP_CFLAGS="-I$mlxdep/include -I$szedep/include"
>     export DPDK_DEP_LDFLAGS="-L$mlxdep/lib -L$szedep/lib64 -rpath=$szedep/lib64"
>     export AESNI_MULTI_BUFFER_LIB_PATH=$root/aesni/ipsec-043
>     export LIBSSO_SNOW3G_PATH=$root/libsso/libsso-snow3g-0.3.1
>     export LIBSSO_KASUMI_PATH=$root/libsso/libsso-kasumi-0.3.1
> fi

Thanks. I will try the above.

>
>> I used 'x86_64-native-linuxapp-gcc+default+debug+shared' for all patches.
>
> It is a good idea to test also with clang (x86_64-native-linuxapp-clang)
> and another arch (e.g. arm64-thunderx-linuxapp-gcc).

Before releasing v9, I will do these steps.
Thank you for suggestions.

-
Shreyansh
  

2016-08-30 17:29, Shreyansh Jain:
> On Tuesday 30 August 2016 04:00 PM, Thomas Monjalon wrote:
> > 2016-08-30 09:57, Shreyansh Jain:
> >> Is there a better way to test that no driver breaks? Any particular
> >> parameters I should use for test-build.sh?
> >
> > Yes I suggest to create a file ~/.config/dpdk/devel.config to adapt the
> > configuration to your system.
> > Once you have installed the required dependencies, you can make this kind
> > of configuration:
> 
> Ok.
> 
> > mlxdep=$root/mlx/mofed-3.3-1.0.0.0
> > szedep=$root/sze/usr-1.1.4
> 
> What does '$root' here refer to?

It is the directory where I compile the DPDK dependencies.

> I am assuming 'mofed-3.3-1.0.0.0' and 'usr-1.1.4' are part of some 
> dependencies that I should be revolving. Is that so?

Yes, download and compile them as explain in the respective guides.

> As of now I don't have much idea about this - I will have a look and 
> ping back in case I am stuck.
> 
> > if echo $DPDK_TARGET | grep -q '^x86_64' ; then
> >     export DPDK_DEP_ARCHIVE=y
> >     export DPDK_DEP_ZLIB=y
> >     export DPDK_DEP_PCAP=y
> >     export DPDK_DEP_SSL=y

You need to install the packages to resolve these dependencies
(libarchive-dev, libzip-dev, libpcap-dev, libcrypto-dev).

> >     export DPDK_DEP_MOFED=y
> >     export DPDK_DEP_SZE=y
> >     export DPDK_DEP_CFLAGS="-I$mlxdep/include -I$szedep/include"
> >     export DPDK_DEP_LDFLAGS="-L$mlxdep/lib -L$szedep/lib64 -rpath=$szedep/lib64"
> >     export AESNI_MULTI_BUFFER_LIB_PATH=$root/aesni/ipsec-043
> >     export LIBSSO_SNOW3G_PATH=$root/libsso/libsso-snow3g-0.3.1
> >     export LIBSSO_KASUMI_PATH=$root/libsso/libsso-kasumi-0.3.1
> > fi
> 
> Thanks. I will try the above.
> 
> >
> >> I used 'x86_64-native-linuxapp-gcc+default+debug+shared' for all patches.
> >
> > It is a good idea to test also with clang (x86_64-native-linuxapp-clang)
> > and another arch (e.g. arm64-thunderx-linuxapp-gcc).
> 
> Before releasing v9, I will do these steps.
> Thank you for suggestions.
  

Hi Thomas,

On Tuesday 30 August 2016 07:12 PM, Thomas Monjalon wrote:
> 2016-08-30 17:29, Shreyansh Jain:
>> On Tuesday 30 August 2016 04:00 PM, Thomas Monjalon wrote:
>>> 2016-08-30 09:57, Shreyansh Jain:
>>>> Is there a better way to test that no driver breaks? Any particular
>>>> parameters I should use for test-build.sh?
>>>
>>> Yes I suggest to create a file ~/.config/dpdk/devel.config to adapt the
>>> configuration to your system.
>>> Once you have installed the required dependencies, you can make this kind
>>> of configuration:
>>
>> Ok.
>>
>>> mlxdep=$root/mlx/mofed-3.3-1.0.0.0
>>> szedep=$root/sze/usr-1.1.4
>>
>> What does '$root' here refer to?
>
> It is the directory where I compile the DPDK dependencies.

Ok - understood. I guessed the same

>
>> I am assuming 'mofed-3.3-1.0.0.0' and 'usr-1.1.4' are part of some
>> dependencies that I should be revolving. Is that so?
>
> Yes, download and compile them as explain in the respective guides.

Ok.

>
>> As of now I don't have much idea about this - I will have a look and
>> ping back in case I am stuck.
>>
>>> if echo $DPDK_TARGET | grep -q '^x86_64' ; then
>>>     export DPDK_DEP_ARCHIVE=y
>>>     export DPDK_DEP_ZLIB=y
>>>     export DPDK_DEP_PCAP=y
>>>     export DPDK_DEP_SSL=y
>
> You need to install the packages to resolve these dependencies
> (libarchive-dev, libzip-dev, libpcap-dev, libcrypto-dev).

Thanks for the info.

>
>>>     export DPDK_DEP_MOFED=y
>>>     export DPDK_DEP_SZE=y
>>>     export DPDK_DEP_CFLAGS="-I$mlxdep/include -I$szedep/include"
>>>     export DPDK_DEP_LDFLAGS="-L$mlxdep/lib -L$szedep/lib64 -rpath=$szedep/lib64"
>>>     export AESNI_MULTI_BUFFER_LIB_PATH=$root/aesni/ipsec-043
>>>     export LIBSSO_SNOW3G_PATH=$root/libsso/libsso-snow3g-0.3.1
>>>     export LIBSSO_KASUMI_PATH=$root/libsso/libsso-kasumi-0.3.1
>>> fi
>>
>> Thanks. I will try the above.
>>
>>>
>>>> I used 'x86_64-native-linuxapp-gcc+default+debug+shared' for all patches.
>>>
>>> It is a good idea to test also with clang (x86_64-native-linuxapp-clang)
>>> and another arch (e.g. arm64-thunderx-linuxapp-gcc).
>>
>> Before releasing v9, I will do these steps.
>> Thank you for suggestions.
>
>

-
Shreyansh
  

Hi Ferruh,

On Monday 29 August 2016 10:13 PM, Ferruh Yigit wrote:
> On 8/26/2016 2:56 PM, Shreyansh Jain wrote:
>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>> ---
>>  lib/librte_eal/common/include/rte_common.h | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
>> index 332f2a4..a9b6792 100644
>> --- a/lib/librte_eal/common/include/rte_common.h
>> +++ b/lib/librte_eal/common/include/rte_common.h
>> @@ -322,6 +322,22 @@ rte_bsf32(uint32_t v)
>>  #define offsetof(TYPE, MEMBER)  __builtin_offsetof (TYPE, MEMBER)
>>  #endif
>>
>> +/**
>> + * Return pointer to the wrapping struct instance.
>> + * Example:
>> + *
>> + *  struct wrapper {
>> + *      ...
>> + *      struct child c;
>> + *      ...
>> + *  };
>> + *
>> + *  struct child *x = obtain(...);
>> + *  struct wrapper *w = container_of(x, struct wrapper, c);
>> + */
>> +#define container_of(p, type, member) \
>> +	((type *) (((char *) (p)) - offsetof(type, member)))
>> +
>>  #define _RTE_STR(x) #x
>>  /** Take a macro value and get a string version of it */
>>  #define RTE_STR(x) _RTE_STR(x)
>>
>
> This gives compilation error for mlx5, because the libraries mlx depends
> defines same macro:
> ..../rte_common.h:338:9: error: 'container_of' macro redefined
> /usr/include/infiniband/verbs.h:77:9: note: previous definition is here

I have converted into a conditional macro in v9.
I still couldn't compile and test mlx* - my environment is still not 
prepared. Though, I can see that this error is not being reported anymore.

>
> Does it make sense to protect macro with
> #ifndef container_of
> ....
> #endif
>
> OR
>
> add a dpdk prefix?
>
>
> Regards,
> ferruh
>

-
Shreyansh