[dpdk-dev] [PATCH 0/3] Avoid cast-align warnings

[dpdk-dev] [PATCH 0/3] Avoid cast-align warnings

[Eli Britstein]

Various functions/macros assume valid alignment for casting. Compiling
an application against it with -Werror and -Wcast-align may trigger
cast-align warnings.
To avoid those, add first (void *) castings.

Note: for eal/arm64 similar changes could be applied to
lib/eal/arm/include/rte_memcpy_64.h. I do not have a system in which I
encounter such warnings, so currently I do not post any change for it.

Eli Britstein (3):
  net: avoid cast-align warning in VLAN insert function
  mbuf: avoid cast-align warning in pktmbuf mtod offset macro
  eal/x86: avoid cast-align warning in x86 memcpy functions

 lib/eal/x86/include/rte_memcpy.h | 80 ++++++++++++++++++--------------
 lib/mbuf/rte_mbuf_core.h         |  2 +-
 lib/net/rte_ether.h              |  2 +-
 3 files changed, 46 insertions(+), 38 deletions(-)

-- 
2.28.0.2311.g225365fb51

[dpdk-dev] [PATCH 1/3] net: avoid cast-align warning in VLAN insert function

[Eli Britstein]

In rte_vlan_insert there is a casting of rte_pktmbuf_prepend returned
value to (struct rte_ether_hdr *), which causes cast-align warning when
using gcc flags '-Werror -Wcast-align':

In file included from .../include/rte_ethdev.h:165,
                 from lib/netdev-dpdk.c:33:
.../include/rte_ether.h: In function 'rte_vlan_insert':
.../include/rte_ether.h:375:7: error: cast increases required alignment
    of target type [-Werror=cast-align]
  375 |  nh = (struct rte_ether_hdr *)
      |       ^

As the code assumes correct alignment, add first a (void *) casting, to
avoid the warning.

Fixes: c974021a5949 ("ether: add soft vlan encap/decap")
Cc: stable@dpdk.org

Signed-off-by: Eli Britstein <elibr@nvidia.com>
---
 lib/net/rte_ether.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/net/rte_ether.h b/lib/net/rte_ether.h
index 7ee5e9a292..6e21155161 100644
--- a/lib/net/rte_ether.h
+++ b/lib/net/rte_ether.h
@@ -372,7 +372,7 @@ static inline int rte_vlan_insert(struct rte_mbuf **m)
 		return -EINVAL;
 
 	oh = rte_pktmbuf_mtod(*m, struct rte_ether_hdr *);
-	nh = (struct rte_ether_hdr *)
+	nh = (struct rte_ether_hdr *)(void *)
 		rte_pktmbuf_prepend(*m, sizeof(struct rte_vlan_hdr));
 	if (nh == NULL)
 		return -ENOSPC;
-- 
2.28.0.2311.g225365fb51

Re: [dpdk-dev] [PATCH 1/3] net: avoid cast-align warning in VLAN insert function

[Olivier Matz]

On Tue, Jul 13, 2021 at 09:49:08AM +0300, Eli Britstein wrote:
> In rte_vlan_insert there is a casting of rte_pktmbuf_prepend returned
> value to (struct rte_ether_hdr *), which causes cast-align warning when
> using gcc flags '-Werror -Wcast-align':
> 
> In file included from .../include/rte_ethdev.h:165,
>                  from lib/netdev-dpdk.c:33:
> .../include/rte_ether.h: In function 'rte_vlan_insert':
> .../include/rte_ether.h:375:7: error: cast increases required alignment
>     of target type [-Werror=cast-align]
>   375 |  nh = (struct rte_ether_hdr *)
>       |       ^
> 
> As the code assumes correct alignment, add first a (void *) casting, to
> avoid the warning.
> 
> Fixes: c974021a5949 ("ether: add soft vlan encap/decap")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Eli Britstein <elibr@nvidia.com>

Acked-by: Olivier Matz <olivier.matz@6wind.com>

[dpdk-dev] [PATCH 2/3] mbuf: avoid cast-align warning in pktmbuf mtod offset macro

[Eli Britstein]

In rte_pktmbuf_mtod_offset macro, there is a casting from char * to type
't', which may cause cast-align warning when using gcc flags
'-Werror -Wcast-align':

.../include/rte_mbuf_core.h:723:3: error: cast increases required alignment
    of target type [-Werror=cast-align]
  723 |  ((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
      |   ^

As the code assumes correct alignment, add first a (void *) casting, to
avoid the warning.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Eli Britstein <elibr@nvidia.com>
---
 lib/mbuf/rte_mbuf_core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
index bb38d7f581..dabdeee604 100644
--- a/lib/mbuf/rte_mbuf_core.h
+++ b/lib/mbuf/rte_mbuf_core.h
@@ -720,7 +720,7 @@ struct rte_mbuf_ext_shared_info {
  *   The type to cast the result into.
  */
 #define rte_pktmbuf_mtod_offset(m, t, o)	\
-	((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
+	((t)(void *)((char *)(m)->buf_addr + (m)->data_off + (o)))
 
 /**
  * A macro that points to the start of the data in the mbuf.
-- 
2.28.0.2311.g225365fb51

Re: [dpdk-dev] [PATCH 2/3] mbuf: avoid cast-align warning in pktmbuf mtod offset macro

[Thomas Monjalon]

+Cc mbuf maintainers

Please use --cc-cmd devtools/get-maintainer.sh to make it automatic.


13/07/2021 08:49, Eli Britstein:
> In rte_pktmbuf_mtod_offset macro, there is a casting from char * to type
> 't', which may cause cast-align warning when using gcc flags
> '-Werror -Wcast-align':
> 
> .../include/rte_mbuf_core.h:723:3: error: cast increases required alignment
>     of target type [-Werror=cast-align]
>   723 |  ((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
>       |   ^
> 
> As the code assumes correct alignment, add first a (void *) casting, to
> avoid the warning.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Eli Britstein <elibr@nvidia.com>
> ---
>  lib/mbuf/rte_mbuf_core.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
> index bb38d7f581..dabdeee604 100644
> --- a/lib/mbuf/rte_mbuf_core.h
> +++ b/lib/mbuf/rte_mbuf_core.h
> @@ -720,7 +720,7 @@ struct rte_mbuf_ext_shared_info {
>   *   The type to cast the result into.
>   */
>  #define rte_pktmbuf_mtod_offset(m, t, o)	\
> -	((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
> +	((t)(void *)((char *)(m)->buf_addr + (m)->data_off + (o)))
>  
>  /**
>   * A macro that points to the start of the data in the mbuf.
> 

Re: [dpdk-dev] [PATCH 2/3] mbuf: avoid cast-align warning in pktmbuf mtod offset macro

[Olivier Matz]

On Tue, Jul 13, 2021 at 09:49:09AM +0300, Eli Britstein wrote:
> In rte_pktmbuf_mtod_offset macro, there is a casting from char * to type
> 't', which may cause cast-align warning when using gcc flags
> '-Werror -Wcast-align':
> 
> .../include/rte_mbuf_core.h:723:3: error: cast increases required alignment
>     of target type [-Werror=cast-align]
>   723 |  ((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
>       |   ^
> 
> As the code assumes correct alignment, add first a (void *) casting, to
> avoid the warning.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Eli Britstein <elibr@nvidia.com>

My initial thinking was that it's the problem of the application: if
-Werror=cast-align is used, it is up to the application to cast the
return value of rte_pktmbuf_mtod_offset() to (void *) before casting it
to the network type.

But, if I understand correctly, the problem is not about the application
code itself, but about inlined code in the header files of dpdk
(i.e. compiling an empty C file that just includes the dpdk headers with
-Werror=cast-align). Is it correct? If yes I think it should be
highlighted in the commit log.

Out of curiosity, how did you find the errors? I mean, is it possible
that some casts are missing some other headers, or is this patchset
exhaustive?

Thanks,
Olivier


> ---
>  lib/mbuf/rte_mbuf_core.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
> index bb38d7f581..dabdeee604 100644
> --- a/lib/mbuf/rte_mbuf_core.h
> +++ b/lib/mbuf/rte_mbuf_core.h
> @@ -720,7 +720,7 @@ struct rte_mbuf_ext_shared_info {
>   *   The type to cast the result into.
>   */
>  #define rte_pktmbuf_mtod_offset(m, t, o)	\
> -	((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
> +	((t)(void *)((char *)(m)->buf_addr + (m)->data_off + (o)))
>  
>  /**
>   * A macro that points to the start of the data in the mbuf.
> -- 
> 2.28.0.2311.g225365fb51
> 

Re: [dpdk-dev] [PATCH 2/3] mbuf: avoid cast-align warning in pktmbuf mtod offset macro

[Eli Britstein]

On 7/28/2021 6:28 PM, Olivier Matz wrote:
> External email: Use caution opening links or attachments
>
>
> On Tue, Jul 13, 2021 at 09:49:09AM +0300, Eli Britstein wrote:
>> In rte_pktmbuf_mtod_offset macro, there is a casting from char * to type
>> 't', which may cause cast-align warning when using gcc flags
>> '-Werror -Wcast-align':
>>
>> .../include/rte_mbuf_core.h:723:3: error: cast increases required alignment
>>      of target type [-Werror=cast-align]
>>    723 |  ((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
>>        |   ^
>>
>> As the code assumes correct alignment, add first a (void *) casting, to
>> avoid the warning.
>>
>> Fixes: af75078fece3 ("first public release")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Eli Britstein <elibr@nvidia.com>
> My initial thinking was that it's the problem of the application: if
> -Werror=cast-align is used, it is up to the application to cast the
> return value of rte_pktmbuf_mtod_offset() to (void *) before casting it
> to the network type.
>
> But, if I understand correctly, the problem is not about the application
> code itself, but about inlined code in the header files of dpdk
> (i.e. compiling an empty C file that just includes the dpdk headers with
> -Werror=cast-align). Is it correct? If yes I think it should be
> highlighted in the commit log.

I think yes, though in this specific patch it is not even an inline 
function, but a macro.

However, I don't have a synthetic application example to show those 
warnings, thus didn't put such in the commit msg.

>
> Out of curiosity, how did you find the errors? I mean, is it possible
> that some casts are missing some other headers, or is this patchset
> exhaustive?
Currently OVS-DPDK is compiled only with -Wno-cast-align.

Following complaint that a recent commit introduced a degradation in OVS 
[1], I compiled OVS without this warning deprecation.
The fixes in OVS are [2] and [3] (already merged). The fixes in DPDK are 
in this patch-set.

[1] https://mail.openvswitch.org/pipermail/ovs-dev/2021-July/385084.html
[2] https://mail.openvswitch.org/pipermail/ovs-dev/2021-July/386278.html
     e8cccd3a3589 ("netdev-offload-dpdk: Fix IPv6 rewrite cast-align 
warning.")
[3] https://mail.openvswitch.org/pipermail/ovs-dev/2021-July/386279.html
     1f7f557603a5 ("netdev-offload-dpdk: Fix vxlan vni cast-align 
warnings.")
> Thanks,
> Olivier
>
>
>> ---
>>   lib/mbuf/rte_mbuf_core.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
>> index bb38d7f581..dabdeee604 100644
>> --- a/lib/mbuf/rte_mbuf_core.h
>> +++ b/lib/mbuf/rte_mbuf_core.h
>> @@ -720,7 +720,7 @@ struct rte_mbuf_ext_shared_info {
>>    *   The type to cast the result into.
>>    */
>>   #define rte_pktmbuf_mtod_offset(m, t, o)     \
>> -     ((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
>> +     ((t)(void *)((char *)(m)->buf_addr + (m)->data_off + (o)))
>>
>>   /**
>>    * A macro that points to the start of the data in the mbuf.
>> --
>> 2.28.0.2311.g225365fb51
>>

Re: [dpdk-dev] [PATCH 2/3] mbuf: avoid cast-align warning in pktmbuf mtod offset macro

[Olivier Matz]

Hi Eli,

On Thu, Jul 29, 2021 at 10:13:45AM +0300, Eli Britstein wrote:
> 
> On 7/28/2021 6:28 PM, Olivier Matz wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Tue, Jul 13, 2021 at 09:49:09AM +0300, Eli Britstein wrote:
> > > In rte_pktmbuf_mtod_offset macro, there is a casting from char * to type
> > > 't', which may cause cast-align warning when using gcc flags
> > > '-Werror -Wcast-align':
> > > 
> > > .../include/rte_mbuf_core.h:723:3: error: cast increases required alignment
> > >      of target type [-Werror=cast-align]
> > >    723 |  ((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
> > >        |   ^
> > > 
> > > As the code assumes correct alignment, add first a (void *) casting, to
> > > avoid the warning.
> > > 
> > > Fixes: af75078fece3 ("first public release")
> > > Cc: stable@dpdk.org
> > > 
> > > Signed-off-by: Eli Britstein <elibr@nvidia.com>
> > My initial thinking was that it's the problem of the application: if
> > -Werror=cast-align is used, it is up to the application to cast the
> > return value of rte_pktmbuf_mtod_offset() to (void *) before casting it
> > to the network type.
> > 
> > But, if I understand correctly, the problem is not about the application
> > code itself, but about inlined code in the header files of dpdk
> > (i.e. compiling an empty C file that just includes the dpdk headers with
> > -Werror=cast-align). Is it correct? If yes I think it should be
> > highlighted in the commit log.
> 
> I think yes, though in this specific patch it is not even an inline
> function, but a macro.
> 
> However, I don't have a synthetic application example to show those
> warnings, thus didn't put such in the commit msg.

For this patch, I think it would be useful to have a way to reproduce
the issue first, so we can check whether it is the proper place to fix
the problem.

To me, it is assumed in the DPDK project that we can mmap a network
structure on mbuf data (maybe I'm wrong?). If an external application
like OVS wants to use -Werror=cast-align, it has to cast the result of
calls to rte_pktmbuf_mtod() family.

The only corner cases are DPDK header files which have static inline
functions or macro that forces the use of rte_pktmbuf_mtod() family
without a cast (like for your patch 1/3), because it cannot be fixed in
the external project.

I think we have to make our header files compliant to projects that want
to use -Werror=cast-align, like we do to make our header files compliant
to C++.

What you suggest in this patch forces the cast to (void *) for all users
of rte_pktmbuf_mtod() family. This could be a problem for projects that
want to see these warnings.

Would it be possible instead to add a cast in DPDK headers, in inline
functions that make use of these mtod functions?

Regards,
Olivier



> > 
> > Out of curiosity, how did you find the errors? I mean, is it possible
> > that some casts are missing some other headers, or is this patchset
> > exhaustive?
> Currently OVS-DPDK is compiled only with -Wno-cast-align.
> 
> Following complaint that a recent commit introduced a degradation in OVS
> [1], I compiled OVS without this warning deprecation.
> The fixes in OVS are [2] and [3] (already merged). The fixes in DPDK are in
> this patch-set.
> 
> [1] https://mail.openvswitch.org/pipermail/ovs-dev/2021-July/385084.html
> [2] https://mail.openvswitch.org/pipermail/ovs-dev/2021-July/386278.html
>     e8cccd3a3589 ("netdev-offload-dpdk: Fix IPv6 rewrite cast-align
> warning.")
> [3] https://mail.openvswitch.org/pipermail/ovs-dev/2021-July/386279.html
>     1f7f557603a5 ("netdev-offload-dpdk: Fix vxlan vni cast-align warnings.")
> > Thanks,
> > Olivier
> > 
> > 
> > > ---
> > >   lib/mbuf/rte_mbuf_core.h | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
> > > index bb38d7f581..dabdeee604 100644
> > > --- a/lib/mbuf/rte_mbuf_core.h
> > > +++ b/lib/mbuf/rte_mbuf_core.h
> > > @@ -720,7 +720,7 @@ struct rte_mbuf_ext_shared_info {
> > >    *   The type to cast the result into.
> > >    */
> > >   #define rte_pktmbuf_mtod_offset(m, t, o)     \
> > > -     ((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
> > > +     ((t)(void *)((char *)(m)->buf_addr + (m)->data_off + (o)))
> > > 
> > >   /**
> > >    * A macro that points to the start of the data in the mbuf.
> > > --
> > > 2.28.0.2311.g225365fb51
> > > 

Re: [dpdk-dev] [PATCH 2/3] mbuf: avoid cast-align warning in pktmbuf mtod offset macro

[Eli Britstein]

On 7/30/2021 2:10 PM, Olivier Matz wrote:
> External email: Use caution opening links or attachments
>
>
> Hi Eli,
>
> On Thu, Jul 29, 2021 at 10:13:45AM +0300, Eli Britstein wrote:
>> On 7/28/2021 6:28 PM, Olivier Matz wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On Tue, Jul 13, 2021 at 09:49:09AM +0300, Eli Britstein wrote:
>>>> In rte_pktmbuf_mtod_offset macro, there is a casting from char * to type
>>>> 't', which may cause cast-align warning when using gcc flags
>>>> '-Werror -Wcast-align':
>>>>
>>>> .../include/rte_mbuf_core.h:723:3: error: cast increases required alignment
>>>>       of target type [-Werror=cast-align]
>>>>     723 |  ((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
>>>>         |   ^
>>>>
>>>> As the code assumes correct alignment, add first a (void *) casting, to
>>>> avoid the warning.
>>>>
>>>> Fixes: af75078fece3 ("first public release")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Eli Britstein <elibr@nvidia.com>
>>> My initial thinking was that it's the problem of the application: if
>>> -Werror=cast-align is used, it is up to the application to cast the
>>> return value of rte_pktmbuf_mtod_offset() to (void *) before casting it
>>> to the network type.
>>>
>>> But, if I understand correctly, the problem is not about the application
>>> code itself, but about inlined code in the header files of dpdk
>>> (i.e. compiling an empty C file that just includes the dpdk headers with
>>> -Werror=cast-align). Is it correct? If yes I think it should be
>>> highlighted in the commit log.
>> I think yes, though in this specific patch it is not even an inline
>> function, but a macro.
>>
>> However, I don't have a synthetic application example to show those
>> warnings, thus didn't put such in the commit msg.
> For this patch, I think it would be useful to have a way to reproduce
> the issue first, so we can check whether it is the proper place to fix
> the problem.
--- a/examples/l2fwd/Makefile
+++ b/examples/l2fwd/Makefile
@@ -22,6 +22,7 @@ static: build/$(APP)-static
         ln -sf $(APP)-static build/$(APP)

  PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
+CFLAGS += -Wcast-align=strict
  CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)

gcc (Ubuntu 9.3.0-10ubuntu2) 9.3.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

make -C examples/l2fwd clean static

>
> To me, it is assumed in the DPDK project that we can mmap a network
> structure on mbuf data (maybe I'm wrong?). If an external application
> like OVS wants to use -Werror=cast-align, it has to cast the result of
> calls to rte_pktmbuf_mtod() family.
>
> The only corner cases are DPDK header files which have static inline
> functions or macro that forces the use of rte_pktmbuf_mtod() family
> without a cast (like for your patch 1/3), because it cannot be fixed in
> the external project.
>
> I think we have to make our header files compliant to projects that want
> to use -Werror=cast-align, like we do to make our header files compliant
> to C++.
>
> What you suggest in this patch forces the cast to (void *) for all users
> of rte_pktmbuf_mtod() family. This could be a problem for projects that
> want to see these warnings.
>
> Would it be possible instead to add a cast in DPDK headers, in inline
> functions that make use of these mtod functions?
>
> Regards,
> Olivier
>
>
>
>>> Out of curiosity, how did you find the errors? I mean, is it possible
>>> that some casts are missing some other headers, or is this patchset
>>> exhaustive?
>> Currently OVS-DPDK is compiled only with -Wno-cast-align.
>>
>> Following complaint that a recent commit introduced a degradation in OVS
>> [1], I compiled OVS without this warning deprecation.
>> The fixes in OVS are [2] and [3] (already merged). The fixes in DPDK are in
>> this patch-set.
>>
>> [1] https://mail.openvswitch.org/pipermail/ovs-dev/2021-July/385084.html
>> [2] https://mail.openvswitch.org/pipermail/ovs-dev/2021-July/386278.html
>>      e8cccd3a3589 ("netdev-offload-dpdk: Fix IPv6 rewrite cast-align
>> warning.")
>> [3] https://mail.openvswitch.org/pipermail/ovs-dev/2021-July/386279.html
>>      1f7f557603a5 ("netdev-offload-dpdk: Fix vxlan vni cast-align warnings.")
>>> Thanks,
>>> Olivier
>>>
>>>
>>>> ---
>>>>    lib/mbuf/rte_mbuf_core.h | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
>>>> index bb38d7f581..dabdeee604 100644
>>>> --- a/lib/mbuf/rte_mbuf_core.h
>>>> +++ b/lib/mbuf/rte_mbuf_core.h
>>>> @@ -720,7 +720,7 @@ struct rte_mbuf_ext_shared_info {
>>>>     *   The type to cast the result into.
>>>>     */
>>>>    #define rte_pktmbuf_mtod_offset(m, t, o)     \
>>>> -     ((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
>>>> +     ((t)(void *)((char *)(m)->buf_addr + (m)->data_off + (o)))
>>>>
>>>>    /**
>>>>     * A macro that points to the start of the data in the mbuf.
>>>> --
>>>> 2.28.0.2311.g225365fb51
>>>>

Re: [dpdk-dev] [PATCH 2/3] mbuf: avoid cast-align warning in pktmbuf mtod offset macro

[Eli Britstein]

Hi Olivier,

On 8/1/2021 11:06 AM, Eli Britstein wrote:
>
> On 7/30/2021 2:10 PM, Olivier Matz wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> Hi Eli,
>>
>> On Thu, Jul 29, 2021 at 10:13:45AM +0300, Eli Britstein wrote:
>>> On 7/28/2021 6:28 PM, Olivier Matz wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> On Tue, Jul 13, 2021 at 09:49:09AM +0300, Eli Britstein wrote:
>>>>> In rte_pktmbuf_mtod_offset macro, there is a casting from char * 
>>>>> to type
>>>>> 't', which may cause cast-align warning when using gcc flags
>>>>> '-Werror -Wcast-align':
>>>>>
>>>>> .../include/rte_mbuf_core.h:723:3: error: cast increases required 
>>>>> alignment
>>>>>       of target type [-Werror=cast-align]
>>>>>     723 |  ((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
>>>>>         |   ^
>>>>>
>>>>> As the code assumes correct alignment, add first a (void *) 
>>>>> casting, to
>>>>> avoid the warning.
>>>>>
>>>>> Fixes: af75078fece3 ("first public release")
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: Eli Britstein <elibr@nvidia.com>
>>>> My initial thinking was that it's the problem of the application: if
>>>> -Werror=cast-align is used, it is up to the application to cast the
>>>> return value of rte_pktmbuf_mtod_offset() to (void *) before 
>>>> casting it
>>>> to the network type.
>>>>
>>>> But, if I understand correctly, the problem is not about the 
>>>> application
>>>> code itself, but about inlined code in the header files of dpdk
>>>> (i.e. compiling an empty C file that just includes the dpdk headers 
>>>> with
>>>> -Werror=cast-align). Is it correct? If yes I think it should be
>>>> highlighted in the commit log.
>>> I think yes, though in this specific patch it is not even an inline
>>> function, but a macro.
>>>
>>> However, I don't have a synthetic application example to show those
>>> warnings, thus didn't put such in the commit msg.
>> For this patch, I think it would be useful to have a way to reproduce
>> the issue first, so we can check whether it is the proper place to fix
>> the problem.
> --- a/examples/l2fwd/Makefile
> +++ b/examples/l2fwd/Makefile
> @@ -22,6 +22,7 @@ static: build/$(APP)-static
>         ln -sf $(APP)-static build/$(APP)
>
>  PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
> +CFLAGS += -Wcast-align=strict
>  CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
>
> gcc (Ubuntu 9.3.0-10ubuntu2) 9.3.0
> Copyright (C) 2019 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR 
> PURPOSE.
>
> make -C examples/l2fwd clean static

To reproduce locally with DPDK only, no need to change any file. Only run:

CFLAGS="-Wcast-align=strict" make V=1 -C examples/l2fwd clean static

How would you like to proceed?

Thanks,

Eli

>
>>
>> To me, it is assumed in the DPDK project that we can mmap a network
>> structure on mbuf data (maybe I'm wrong?). If an external application
>> like OVS wants to use -Werror=cast-align, it has to cast the result of
>> calls to rte_pktmbuf_mtod() family.
>>
>> The only corner cases are DPDK header files which have static inline
>> functions or macro that forces the use of rte_pktmbuf_mtod() family
>> without a cast (like for your patch 1/3), because it cannot be fixed in
>> the external project.
>>
>> I think we have to make our header files compliant to projects that want
>> to use -Werror=cast-align, like we do to make our header files compliant
>> to C++.
>>
>> What you suggest in this patch forces the cast to (void *) for all users
>> of rte_pktmbuf_mtod() family. This could be a problem for projects that
>> want to see these warnings.
>>
>> Would it be possible instead to add a cast in DPDK headers, in inline
>> functions that make use of these mtod functions?
>>
>> Regards,
>> Olivier
>>
>>
>>
>>>> Out of curiosity, how did you find the errors? I mean, is it possible
>>>> that some casts are missing some other headers, or is this patchset
>>>> exhaustive?
>>> Currently OVS-DPDK is compiled only with -Wno-cast-align.
>>>
>>> Following complaint that a recent commit introduced a degradation in 
>>> OVS
>>> [1], I compiled OVS without this warning deprecation.
>>> The fixes in OVS are [2] and [3] (already merged). The fixes in DPDK 
>>> are in
>>> this patch-set.
>>>
>>> [1] 
>>> https://mail.openvswitch.org/pipermail/ovs-dev/2021-July/385084.html
>>> [2] 
>>> https://mail.openvswitch.org/pipermail/ovs-dev/2021-July/386278.html
>>>      e8cccd3a3589 ("netdev-offload-dpdk: Fix IPv6 rewrite cast-align
>>> warning.")
>>> [3] 
>>> https://mail.openvswitch.org/pipermail/ovs-dev/2021-July/386279.html
>>>      1f7f557603a5 ("netdev-offload-dpdk: Fix vxlan vni cast-align 
>>> warnings.")
>>>> Thanks,
>>>> Olivier
>>>>
>>>>
>>>>> ---
>>>>>    lib/mbuf/rte_mbuf_core.h | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
>>>>> index bb38d7f581..dabdeee604 100644
>>>>> --- a/lib/mbuf/rte_mbuf_core.h
>>>>> +++ b/lib/mbuf/rte_mbuf_core.h
>>>>> @@ -720,7 +720,7 @@ struct rte_mbuf_ext_shared_info {
>>>>>     *   The type to cast the result into.
>>>>>     */
>>>>>    #define rte_pktmbuf_mtod_offset(m, t, o)     \
>>>>> -     ((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
>>>>> +     ((t)(void *)((char *)(m)->buf_addr + (m)->data_off + (o)))
>>>>>
>>>>>    /**
>>>>>     * A macro that points to the start of the data in the mbuf.
>>>>> -- 
>>>>> 2.28.0.2311.g225365fb51
>>>>>

Re: [dpdk-dev] [PATCH 2/3] mbuf: avoid cast-align warning in pktmbuf mtod offset macro

[Olivier Matz]

Hi Eli,

On Tue, Oct 19, 2021 at 09:41:56AM +0300, Eli Britstein wrote:
> Hi Olivier,
> 
> On 8/1/2021 11:06 AM, Eli Britstein wrote:
> > 
> > On 7/30/2021 2:10 PM, Olivier Matz wrote:
> > > External email: Use caution opening links or attachments
> > > 
> > > 
> > > Hi Eli,
> > > 
> > > On Thu, Jul 29, 2021 at 10:13:45AM +0300, Eli Britstein wrote:
> > > > On 7/28/2021 6:28 PM, Olivier Matz wrote:
> > > > > External email: Use caution opening links or attachments
> > > > > 
> > > > > 
> > > > > On Tue, Jul 13, 2021 at 09:49:09AM +0300, Eli Britstein wrote:
> > > > > > In rte_pktmbuf_mtod_offset macro, there is a casting
> > > > > > from char * to type
> > > > > > 't', which may cause cast-align warning when using gcc flags
> > > > > > '-Werror -Wcast-align':
> > > > > > 
> > > > > > .../include/rte_mbuf_core.h:723:3: error: cast increases
> > > > > > required alignment
> > > > > >       of target type [-Werror=cast-align]
> > > > > >     723 |  ((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
> > > > > >         |   ^
> > > > > > 
> > > > > > As the code assumes correct alignment, add first a (void
> > > > > > *) casting, to
> > > > > > avoid the warning.
> > > > > > 
> > > > > > Fixes: af75078fece3 ("first public release")
> > > > > > Cc: stable@dpdk.org
> > > > > > 
> > > > > > Signed-off-by: Eli Britstein <elibr@nvidia.com>
> > > > > My initial thinking was that it's the problem of the application: if
> > > > > -Werror=cast-align is used, it is up to the application to cast the
> > > > > return value of rte_pktmbuf_mtod_offset() to (void *) before
> > > > > casting it
> > > > > to the network type.
> > > > > 
> > > > > But, if I understand correctly, the problem is not about the
> > > > > application
> > > > > code itself, but about inlined code in the header files of dpdk
> > > > > (i.e. compiling an empty C file that just includes the dpdk
> > > > > headers with
> > > > > -Werror=cast-align). Is it correct? If yes I think it should be
> > > > > highlighted in the commit log.
> > > > I think yes, though in this specific patch it is not even an inline
> > > > function, but a macro.
> > > > 
> > > > However, I don't have a synthetic application example to show those
> > > > warnings, thus didn't put such in the commit msg.
> > > For this patch, I think it would be useful to have a way to reproduce
> > > the issue first, so we can check whether it is the proper place to fix
> > > the problem.
> > --- a/examples/l2fwd/Makefile
> > +++ b/examples/l2fwd/Makefile
> > @@ -22,6 +22,7 @@ static: build/$(APP)-static
> >         ln -sf $(APP)-static build/$(APP)
> > 
> >  PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
> > +CFLAGS += -Wcast-align=strict
> >  CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
> > 
> > gcc (Ubuntu 9.3.0-10ubuntu2) 9.3.0
> > Copyright (C) 2019 Free Software Foundation, Inc.
> > This is free software; see the source for copying conditions. There is NO
> > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
> > PURPOSE.
> > 
> > make -C examples/l2fwd clean static
> 
> To reproduce locally with DPDK only, no need to change any file. Only run:
> 
> CFLAGS="-Wcast-align=strict" make V=1 -C examples/l2fwd clean static
> 
> How would you like to proceed?

Sorry, I missed your previous message. I reproduced the issue, with
a slightly modified command:

  # no error, my gcc is 8.3.0-6 (debian)
  CFLAGS="-Wcast-align=strict" make V=1 -C examples/l2fwd clean static


  # bad option name with clang
  CC=clang CFLAGS="-Wcast-align=strict" make V=1 -C examples/l2fwd clean static
  ...
  warning: unknown warning option '-Wcast-align=strict'; did you mean '-Wcast-align'? [-Wunknown-warning-option]


  # problem reproduced with clang
  CC=clang CFLAGS="-Wcast-align" make V=1 -C examples/l2fwd clean static
  main.c:170:8: warning: cast from 'char *' to 'struct rte_ether_hdr *' increases required alignment from 1 to 2 [-Wcast-align]
          eth = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  /usr/local/include/rte_mbuf_core.h:830:32: note: expanded from macro 'rte_pktmbuf_mtod'
  #define rte_pktmbuf_mtod(m, t) rte_pktmbuf_mtod_offset(m, t, 0)
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  /usr/local/include/rte_mbuf_core.h:816:3: note: expanded from macro 'rte_pktmbuf_mtod_offset'
          ((t)((char *)(m)->buf_addr + (m)->data_off + (o)))


I confirm the patch fixes the issue.

Acked-by: Olivier Matz <olivier.matz@6wind.com>

Thanks

[dpdk-dev] [PATCH 3/3] eal/x86: avoid cast-align warning in x86 memcpy functions

[Eli Britstein]

Functions and macros in x86 rte_memcpy.h may cause cast-align warnings,
when using gcc flags '-Werror -Wcast-align':

For example:
.../include/rte_memcpy.h:499:42: error: cast increases required alignment
    of target type [-Werror=cast-align]
  499 |  xmm0 = _mm_loadu_si128((const __m128i *)(const __m128i *)src);
      |                                          ^

As the code assumes correct alignment, add first a (void *) or (const
void *) castings, to avoid the warnings.

Fixes: 9484092baad3 ("eal/x86: optimize memcpy for AVX512 platforms")
Cc: stable@dpdk.org

Signed-off-by: Eli Britstein <elibr@nvidia.com>
---
 lib/eal/x86/include/rte_memcpy.h | 80 ++++++++++++++++++--------------
 1 file changed, 44 insertions(+), 36 deletions(-)

diff --git a/lib/eal/x86/include/rte_memcpy.h b/lib/eal/x86/include/rte_memcpy.h
index 79f381dd9b..1b6c6e585f 100644
--- a/lib/eal/x86/include/rte_memcpy.h
+++ b/lib/eal/x86/include/rte_memcpy.h
@@ -303,8 +303,8 @@ rte_mov16(uint8_t *dst, const uint8_t *src)
 {
 	__m128i xmm0;
 
-	xmm0 = _mm_loadu_si128((const __m128i *)src);
-	_mm_storeu_si128((__m128i *)dst, xmm0);
+	xmm0 = _mm_loadu_si128((const __m128i *)(const void *)src);
+	_mm_storeu_si128((__m128i *)(void *)dst, xmm0);
 }
 
 /**
@@ -316,8 +316,8 @@ rte_mov32(uint8_t *dst, const uint8_t *src)
 {
 	__m256i ymm0;
 
-	ymm0 = _mm256_loadu_si256((const __m256i *)src);
-	_mm256_storeu_si256((__m256i *)dst, ymm0);
+	ymm0 = _mm256_loadu_si256((const __m256i *)(const void *)src);
+	_mm256_storeu_si256((__m256i *)(void *)dst, ymm0);
 }
 
 /**
@@ -354,16 +354,24 @@ rte_mov128blocks(uint8_t *dst, const uint8_t *src, size_t n)
 	__m256i ymm0, ymm1, ymm2, ymm3;
 
 	while (n >= 128) {
-		ymm0 = _mm256_loadu_si256((const __m256i *)((const uint8_t *)src + 0 * 32));
+		ymm0 = _mm256_loadu_si256((const __m256i *)(const void *)
+					  ((const uint8_t *)src + 0 * 32));
 		n -= 128;
-		ymm1 = _mm256_loadu_si256((const __m256i *)((const uint8_t *)src + 1 * 32));
-		ymm2 = _mm256_loadu_si256((const __m256i *)((const uint8_t *)src + 2 * 32));
-		ymm3 = _mm256_loadu_si256((const __m256i *)((const uint8_t *)src + 3 * 32));
+		ymm1 = _mm256_loadu_si256((const __m256i *)(const void *)
+					  ((const uint8_t *)src + 1 * 32));
+		ymm2 = _mm256_loadu_si256((const __m256i *)(const void *)
+					  ((const uint8_t *)src + 2 * 32));
+		ymm3 = _mm256_loadu_si256((const __m256i *)(const void *)
+					  ((const uint8_t *)src + 3 * 32));
 		src = (const uint8_t *)src + 128;
-		_mm256_storeu_si256((__m256i *)((uint8_t *)dst + 0 * 32), ymm0);
-		_mm256_storeu_si256((__m256i *)((uint8_t *)dst + 1 * 32), ymm1);
-		_mm256_storeu_si256((__m256i *)((uint8_t *)dst + 2 * 32), ymm2);
-		_mm256_storeu_si256((__m256i *)((uint8_t *)dst + 3 * 32), ymm3);
+		_mm256_storeu_si256((__m256i *)(void *)
+				    ((uint8_t *)dst + 0 * 32), ymm0);
+		_mm256_storeu_si256((__m256i *)(void *)
+				    ((uint8_t *)dst + 1 * 32), ymm1);
+		_mm256_storeu_si256((__m256i *)(void *)
+				    ((uint8_t *)dst + 2 * 32), ymm2);
+		_mm256_storeu_si256((__m256i *)(void *)
+				    ((uint8_t *)dst + 3 * 32), ymm3);
 		dst = (uint8_t *)dst + 128;
 	}
 }
@@ -496,8 +504,8 @@ rte_mov16(uint8_t *dst, const uint8_t *src)
 {
 	__m128i xmm0;
 
-	xmm0 = _mm_loadu_si128((const __m128i *)(const __m128i *)src);
-	_mm_storeu_si128((__m128i *)dst, xmm0);
+	xmm0 = _mm_loadu_si128((const __m128i *)(const void *)src);
+	_mm_storeu_si128((__m128i *)(void *)dst, xmm0);
 }
 
 /**
@@ -581,25 +589,25 @@ rte_mov256(uint8_t *dst, const uint8_t *src)
 __extension__ ({                                                                                            \
     size_t tmp;                                                                                                \
     while (len >= 128 + 16 - offset) {                                                                      \
-        xmm0 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 0 * 16));                  \
+        xmm0 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 0 * 16));                  \
         len -= 128;                                                                                         \
-        xmm1 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 1 * 16));                  \
-        xmm2 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 2 * 16));                  \
-        xmm3 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 3 * 16));                  \
-        xmm4 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 4 * 16));                  \
-        xmm5 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 5 * 16));                  \
-        xmm6 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 6 * 16));                  \
-        xmm7 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 7 * 16));                  \
-        xmm8 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 8 * 16));                  \
+        xmm1 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 1 * 16));                  \
+        xmm2 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 2 * 16));                  \
+        xmm3 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 3 * 16));                  \
+        xmm4 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 4 * 16));                  \
+        xmm5 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 5 * 16));                  \
+        xmm6 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 6 * 16));                  \
+        xmm7 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 7 * 16));                  \
+        xmm8 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 8 * 16));                  \
         src = (const uint8_t *)src + 128;                                                                   \
-        _mm_storeu_si128((__m128i *)((uint8_t *)dst + 0 * 16), _mm_alignr_epi8(xmm1, xmm0, offset));        \
-        _mm_storeu_si128((__m128i *)((uint8_t *)dst + 1 * 16), _mm_alignr_epi8(xmm2, xmm1, offset));        \
-        _mm_storeu_si128((__m128i *)((uint8_t *)dst + 2 * 16), _mm_alignr_epi8(xmm3, xmm2, offset));        \
-        _mm_storeu_si128((__m128i *)((uint8_t *)dst + 3 * 16), _mm_alignr_epi8(xmm4, xmm3, offset));        \
-        _mm_storeu_si128((__m128i *)((uint8_t *)dst + 4 * 16), _mm_alignr_epi8(xmm5, xmm4, offset));        \
-        _mm_storeu_si128((__m128i *)((uint8_t *)dst + 5 * 16), _mm_alignr_epi8(xmm6, xmm5, offset));        \
-        _mm_storeu_si128((__m128i *)((uint8_t *)dst + 6 * 16), _mm_alignr_epi8(xmm7, xmm6, offset));        \
-        _mm_storeu_si128((__m128i *)((uint8_t *)dst + 7 * 16), _mm_alignr_epi8(xmm8, xmm7, offset));        \
+        _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 0 * 16), _mm_alignr_epi8(xmm1, xmm0, offset));        \
+        _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 1 * 16), _mm_alignr_epi8(xmm2, xmm1, offset));        \
+        _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 2 * 16), _mm_alignr_epi8(xmm3, xmm2, offset));        \
+        _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 3 * 16), _mm_alignr_epi8(xmm4, xmm3, offset));        \
+        _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 4 * 16), _mm_alignr_epi8(xmm5, xmm4, offset));        \
+        _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 5 * 16), _mm_alignr_epi8(xmm6, xmm5, offset));        \
+        _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 6 * 16), _mm_alignr_epi8(xmm7, xmm6, offset));        \
+        _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 7 * 16), _mm_alignr_epi8(xmm8, xmm7, offset));        \
         dst = (uint8_t *)dst + 128;                                                                         \
     }                                                                                                       \
     tmp = len;                                                                                              \
@@ -609,13 +617,13 @@ __extension__ ({
     dst = (uint8_t *)dst + tmp;                                                                             \
     if (len >= 32 + 16 - offset) {                                                                          \
         while (len >= 32 + 16 - offset) {                                                                   \
-            xmm0 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 0 * 16));              \
+            xmm0 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 0 * 16));              \
             len -= 32;                                                                                      \
-            xmm1 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 1 * 16));              \
-            xmm2 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 2 * 16));              \
+            xmm1 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 1 * 16));              \
+            xmm2 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 2 * 16));              \
             src = (const uint8_t *)src + 32;                                                                \
-            _mm_storeu_si128((__m128i *)((uint8_t *)dst + 0 * 16), _mm_alignr_epi8(xmm1, xmm0, offset));    \
-            _mm_storeu_si128((__m128i *)((uint8_t *)dst + 1 * 16), _mm_alignr_epi8(xmm2, xmm1, offset));    \
+            _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 0 * 16), _mm_alignr_epi8(xmm1, xmm0, offset));    \
+            _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 1 * 16), _mm_alignr_epi8(xmm2, xmm1, offset));    \
             dst = (uint8_t *)dst + 32;                                                                      \
         }                                                                                                   \
         tmp = len;                                                                                          \
-- 
2.28.0.2311.g225365fb51

[dpdk-dev] [PATCH V2 1/3] net: avoid cast-align warning in VLAN insert function

[Eli Britstein]

In rte_vlan_insert there is a casting of rte_pktmbuf_prepend returned
value to (struct rte_ether_hdr *), which causes cast-align warning when
using strict cast align flag with supporting gcc:
gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
CFLAGS="-Wcast-align=strict" make V=1 -C examples/l2fwd clean static

In file included from main.c:35:
/dpdk/build/include/rte_ether.h:370:7: warning: cast increases required
alignment of target type [-Wcast-align]
  370 |  nh = (struct rte_ether_hdr *)
      |       ^

As the code assumes correct alignment, add first a (void *) casting, to
avoid the warning.

Fixes: c974021a5949 ("ether: add soft vlan encap/decap")
Cc: stable@dpdk.org

Signed-off-by: Eli Britstein <elibr@nvidia.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/net/rte_ether.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/net/rte_ether.h b/lib/net/rte_ether.h
index b83e0d3fce..9febb60300 100644
--- a/lib/net/rte_ether.h
+++ b/lib/net/rte_ether.h
@@ -367,7 +367,7 @@ static inline int rte_vlan_insert(struct rte_mbuf **m)
 		return -EINVAL;
 
 	oh = rte_pktmbuf_mtod(*m, struct rte_ether_hdr *);
-	nh = (struct rte_ether_hdr *)
+	nh = (struct rte_ether_hdr *)(void *)
 		rte_pktmbuf_prepend(*m, sizeof(struct rte_vlan_hdr));
 	if (nh == NULL)
 		return -ENOSPC;
-- 
2.28.0.2311.g225365fb51

[dpdk-dev] [PATCH V2 2/3] mbuf: avoid cast-align warning in pktmbuf mtod offset macro

[Eli Britstein]

In rte_pktmbuf_mtod_offset macro, there is a casting from char * to type
't', which may cause cast-align warning when using strict cast align
flag with supporting gcc:
gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
CFLAGS="-Wcast-align=strict" make V=1 -C examples/l2fwd clean static

main.c: In function 'l2fwd_mac_updating':
/dpdk/build/include/rte_mbuf_core.h:719:3: warning: cast increases
required alignment of target type [-Wcast-align]
  719 |  ((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
      |   ^
/dpdk/build/include/rte_mbuf_core.h:733:32: note: in expansion of macro
'rte_pktmbuf_mtod_offset'
  733 | #define rte_pktmbuf_mtod(m, t) rte_pktmbuf_mtod_offset(m, t, 0)
      |                                ^~~~~~~~~~~~~~~~~~~~~~~

As the code assumes correct alignment, add first a (void *) casting, to
avoid the warning.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Eli Britstein <elibr@nvidia.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/mbuf/rte_mbuf_core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
index fdaaaf67f2..dd08d42aca 100644
--- a/lib/mbuf/rte_mbuf_core.h
+++ b/lib/mbuf/rte_mbuf_core.h
@@ -716,7 +716,7 @@ struct rte_mbuf_ext_shared_info {
  *   The type to cast the result into.
  */
 #define rte_pktmbuf_mtod_offset(m, t, o)	\
-	((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
+	((t)(void *)((char *)(m)->buf_addr + (m)->data_off + (o)))
 
 /**
  * A macro that points to the start of the data in the mbuf.
-- 
2.28.0.2311.g225365fb51

[dpdk-dev] [PATCH V2 3/3] eal/x86: avoid cast-align warning in x86 memcpy functions

[Eli Britstein]

Functions and macros in x86 rte_memcpy.h may cause cast-align warnings,
when using strict cast align flag with supporting gcc:
gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
CFLAGS="-Wcast-align=strict" make V=1 -C examples/l2fwd clean static

For example:
In file included from main.c:24:
/dpdk/build/include/rte_memcpy.h: In function 'rte_mov16':
/dpdk/build/include/rte_memcpy.h:306:25: warning: cast increases
required alignment of target type [-Wcast-align]
  306 |  xmm0 = _mm_loadu_si128((const __m128i *)src);
      |                         ^

As the code assumes correct alignment, add first a (void *) or (const
void *) castings, to avoid the warnings.

Fixes: 9484092baad3 ("eal/x86: optimize memcpy for AVX512 platforms")
Cc: stable@dpdk.org

Signed-off-by: Eli Britstein <elibr@nvidia.com>
---
 lib/eal/x86/include/rte_memcpy.h | 80 ++++++++++++++++++--------------
 1 file changed, 44 insertions(+), 36 deletions(-)

diff --git a/lib/eal/x86/include/rte_memcpy.h b/lib/eal/x86/include/rte_memcpy.h
index 79f381dd9b..1b6c6e585f 100644
--- a/lib/eal/x86/include/rte_memcpy.h
+++ b/lib/eal/x86/include/rte_memcpy.h
@@ -303,8 +303,8 @@ rte_mov16(uint8_t *dst, const uint8_t *src)
 {
 	__m128i xmm0;
 
-	xmm0 = _mm_loadu_si128((const __m128i *)src);
-	_mm_storeu_si128((__m128i *)dst, xmm0);
+	xmm0 = _mm_loadu_si128((const __m128i *)(const void *)src);
+	_mm_storeu_si128((__m128i *)(void *)dst, xmm0);
 }
 
 /**
@@ -316,8 +316,8 @@ rte_mov32(uint8_t *dst, const uint8_t *src)
 {
 	__m256i ymm0;
 
-	ymm0 = _mm256_loadu_si256((const __m256i *)src);
-	_mm256_storeu_si256((__m256i *)dst, ymm0);
+	ymm0 = _mm256_loadu_si256((const __m256i *)(const void *)src);
+	_mm256_storeu_si256((__m256i *)(void *)dst, ymm0);
 }
 
 /**
@@ -354,16 +354,24 @@ rte_mov128blocks(uint8_t *dst, const uint8_t *src, size_t n)
 	__m256i ymm0, ymm1, ymm2, ymm3;
 
 	while (n >= 128) {
-		ymm0 = _mm256_loadu_si256((const __m256i *)((const uint8_t *)src + 0 * 32));
+		ymm0 = _mm256_loadu_si256((const __m256i *)(const void *)
+					  ((const uint8_t *)src + 0 * 32));
 		n -= 128;
-		ymm1 = _mm256_loadu_si256((const __m256i *)((const uint8_t *)src + 1 * 32));
-		ymm2 = _mm256_loadu_si256((const __m256i *)((const uint8_t *)src + 2 * 32));
-		ymm3 = _mm256_loadu_si256((const __m256i *)((const uint8_t *)src + 3 * 32));
+		ymm1 = _mm256_loadu_si256((const __m256i *)(const void *)
+					  ((const uint8_t *)src + 1 * 32));
+		ymm2 = _mm256_loadu_si256((const __m256i *)(const void *)
+					  ((const uint8_t *)src + 2 * 32));
+		ymm3 = _mm256_loadu_si256((const __m256i *)(const void *)
+					  ((const uint8_t *)src + 3 * 32));
 		src = (const uint8_t *)src + 128;
-		_mm256_storeu_si256((__m256i *)((uint8_t *)dst + 0 * 32), ymm0);
-		_mm256_storeu_si256((__m256i *)((uint8_t *)dst + 1 * 32), ymm1);
-		_mm256_storeu_si256((__m256i *)((uint8_t *)dst + 2 * 32), ymm2);
-		_mm256_storeu_si256((__m256i *)((uint8_t *)dst + 3 * 32), ymm3);
+		_mm256_storeu_si256((__m256i *)(void *)
+				    ((uint8_t *)dst + 0 * 32), ymm0);
+		_mm256_storeu_si256((__m256i *)(void *)
+				    ((uint8_t *)dst + 1 * 32), ymm1);
+		_mm256_storeu_si256((__m256i *)(void *)
+				    ((uint8_t *)dst + 2 * 32), ymm2);
+		_mm256_storeu_si256((__m256i *)(void *)
+				    ((uint8_t *)dst + 3 * 32), ymm3);
 		dst = (uint8_t *)dst + 128;
 	}
 }
@@ -496,8 +504,8 @@ rte_mov16(uint8_t *dst, const uint8_t *src)
 {
 	__m128i xmm0;
 
-	xmm0 = _mm_loadu_si128((const __m128i *)(const __m128i *)src);
-	_mm_storeu_si128((__m128i *)dst, xmm0);
+	xmm0 = _mm_loadu_si128((const __m128i *)(const void *)src);
+	_mm_storeu_si128((__m128i *)(void *)dst, xmm0);
 }
 
 /**
@@ -581,25 +589,25 @@ rte_mov256(uint8_t *dst, const uint8_t *src)
 __extension__ ({                                                                                            \
     size_t tmp;                                                                                                \
     while (len >= 128 + 16 - offset) {                                                                      \
-        xmm0 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 0 * 16));                  \
+        xmm0 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 0 * 16));                  \
         len -= 128;                                                                                         \
-        xmm1 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 1 * 16));                  \
-        xmm2 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 2 * 16));                  \
-        xmm3 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 3 * 16));                  \
-        xmm4 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 4 * 16));                  \
-        xmm5 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 5 * 16));                  \
-        xmm6 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 6 * 16));                  \
-        xmm7 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 7 * 16));                  \
-        xmm8 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 8 * 16));                  \
+        xmm1 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 1 * 16));                  \
+        xmm2 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 2 * 16));                  \
+        xmm3 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 3 * 16));                  \
+        xmm4 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 4 * 16));                  \
+        xmm5 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 5 * 16));                  \
+        xmm6 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 6 * 16));                  \
+        xmm7 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 7 * 16));                  \
+        xmm8 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 8 * 16));                  \
         src = (const uint8_t *)src + 128;                                                                   \
-        _mm_storeu_si128((__m128i *)((uint8_t *)dst + 0 * 16), _mm_alignr_epi8(xmm1, xmm0, offset));        \
-        _mm_storeu_si128((__m128i *)((uint8_t *)dst + 1 * 16), _mm_alignr_epi8(xmm2, xmm1, offset));        \
-        _mm_storeu_si128((__m128i *)((uint8_t *)dst + 2 * 16), _mm_alignr_epi8(xmm3, xmm2, offset));        \
-        _mm_storeu_si128((__m128i *)((uint8_t *)dst + 3 * 16), _mm_alignr_epi8(xmm4, xmm3, offset));        \
-        _mm_storeu_si128((__m128i *)((uint8_t *)dst + 4 * 16), _mm_alignr_epi8(xmm5, xmm4, offset));        \
-        _mm_storeu_si128((__m128i *)((uint8_t *)dst + 5 * 16), _mm_alignr_epi8(xmm6, xmm5, offset));        \
-        _mm_storeu_si128((__m128i *)((uint8_t *)dst + 6 * 16), _mm_alignr_epi8(xmm7, xmm6, offset));        \
-        _mm_storeu_si128((__m128i *)((uint8_t *)dst + 7 * 16), _mm_alignr_epi8(xmm8, xmm7, offset));        \
+        _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 0 * 16), _mm_alignr_epi8(xmm1, xmm0, offset));        \
+        _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 1 * 16), _mm_alignr_epi8(xmm2, xmm1, offset));        \
+        _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 2 * 16), _mm_alignr_epi8(xmm3, xmm2, offset));        \
+        _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 3 * 16), _mm_alignr_epi8(xmm4, xmm3, offset));        \
+        _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 4 * 16), _mm_alignr_epi8(xmm5, xmm4, offset));        \
+        _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 5 * 16), _mm_alignr_epi8(xmm6, xmm5, offset));        \
+        _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 6 * 16), _mm_alignr_epi8(xmm7, xmm6, offset));        \
+        _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 7 * 16), _mm_alignr_epi8(xmm8, xmm7, offset));        \
         dst = (uint8_t *)dst + 128;                                                                         \
     }                                                                                                       \
     tmp = len;                                                                                              \
@@ -609,13 +617,13 @@ __extension__ ({
     dst = (uint8_t *)dst + tmp;                                                                             \
     if (len >= 32 + 16 - offset) {                                                                          \
         while (len >= 32 + 16 - offset) {                                                                   \
-            xmm0 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 0 * 16));              \
+            xmm0 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 0 * 16));              \
             len -= 32;                                                                                      \
-            xmm1 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 1 * 16));              \
-            xmm2 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 2 * 16));              \
+            xmm1 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 1 * 16));              \
+            xmm2 = _mm_loadu_si128((const __m128i *)(const void *)((const uint8_t *)src - offset + 2 * 16));              \
             src = (const uint8_t *)src + 32;                                                                \
-            _mm_storeu_si128((__m128i *)((uint8_t *)dst + 0 * 16), _mm_alignr_epi8(xmm1, xmm0, offset));    \
-            _mm_storeu_si128((__m128i *)((uint8_t *)dst + 1 * 16), _mm_alignr_epi8(xmm2, xmm1, offset));    \
+            _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 0 * 16), _mm_alignr_epi8(xmm1, xmm0, offset));    \
+            _mm_storeu_si128((__m128i *)(void *)((uint8_t *)dst + 1 * 16), _mm_alignr_epi8(xmm2, xmm1, offset));    \
             dst = (uint8_t *)dst + 32;                                                                      \
         }                                                                                                   \
         tmp = len;                                                                                          \
-- 
2.28.0.2311.g225365fb51

Re: [dpdk-dev] [dpdk-stable] [PATCH V2 3/3] eal/x86: avoid cast-align warning in x86 memcpy functions

[Thomas Monjalon]

21/10/2021 10:51, Eli Britstein:
> Functions and macros in x86 rte_memcpy.h may cause cast-align warnings,
> when using strict cast align flag with supporting gcc:
> gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
> CFLAGS="-Wcast-align=strict" make V=1 -C examples/l2fwd clean static
> 
> For example:
> In file included from main.c:24:
> /dpdk/build/include/rte_memcpy.h: In function 'rte_mov16':
> /dpdk/build/include/rte_memcpy.h:306:25: warning: cast increases
> required alignment of target type [-Wcast-align]
>   306 |  xmm0 = _mm_loadu_si128((const __m128i *)src);
>       |                         ^
> 
> As the code assumes correct alignment, add first a (void *) or (const
> void *) castings, to avoid the warnings.
> 
> Fixes: 9484092baad3 ("eal/x86: optimize memcpy for AVX512 platforms")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Eli Britstein <elibr@nvidia.com>

Series applied, thanks.

Re: [dpdk-dev] [PATCH V2 1/3] net: avoid cast-align warning in VLAN insert function

[Stephen Hemminger]

On Thu, 21 Oct 2021 11:51:30 +0300
Eli Britstein <elibr@nvidia.com> wrote:

> In rte_vlan_insert there is a casting of rte_pktmbuf_prepend returned
> value to (struct rte_ether_hdr *), which causes cast-align warning when
> using strict cast align flag with supporting gcc:
> gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
> CFLAGS="-Wcast-align=strict" make V=1 -C examples/l2fwd clean static
> 
> In file included from main.c:35:
> /dpdk/build/include/rte_ether.h:370:7: warning: cast increases required
> alignment of target type [-Wcast-align]
>   370 |  nh = (struct rte_ether_hdr *)
>       |       ^
> 
> As the code assumes correct alignment, add first a (void *) casting, to
> avoid the warning.
> 
> Fixes: c974021a5949 ("ether: add soft vlan encap/decap")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Eli Britstein <elibr@nvidia.com>
> Acked-by: Olivier Matz <olivier.matz@6wind.com>

After cast to void * the second cast is not necessary.

	nh = (void *)rte_pktmbuf_prepend(...)

Ideally rte_pktmbuf_prepend() should return void * but that is
an API change.

Re: [dpdk-dev] [PATCH V2 1/3] net: avoid cast-align warning in VLAN insert function

[Eli Britstein]

On 10/21/2021 6:48 PM, Stephen Hemminger wrote:
> External email: Use caution opening links or attachments
>
>
> On Thu, 21 Oct 2021 11:51:30 +0300
> Eli Britstein <elibr@nvidia.com> wrote:
>
>> In rte_vlan_insert there is a casting of rte_pktmbuf_prepend returned
>> value to (struct rte_ether_hdr *), which causes cast-align warning when
>> using strict cast align flag with supporting gcc:
>> gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
>> CFLAGS="-Wcast-align=strict" make V=1 -C examples/l2fwd clean static
>>
>> In file included from main.c:35:
>> /dpdk/build/include/rte_ether.h:370:7: warning: cast increases required
>> alignment of target type [-Wcast-align]
>>    370 |  nh = (struct rte_ether_hdr *)
>>        |       ^
>>
>> As the code assumes correct alignment, add first a (void *) casting, to
>> avoid the warning.
>>
>> Fixes: c974021a5949 ("ether: add soft vlan encap/decap")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Eli Britstein <elibr@nvidia.com>
>> Acked-by: Olivier Matz <olivier.matz@6wind.com>
> After cast to void * the second cast is not necessary.
>
>          nh = (void *)rte_pktmbuf_prepend(...)
>
> Ideally rte_pktmbuf_prepend() should return void * but that is
> an API change.

Removing the second cast, it is silently done anyway, as 'nh' is of type 
'struct rte_ether_hdr *'.

Going with this approach (I can also do it for patch 3/3), we can change 
rte_pktmbuf_prepend to return (void *), and let the applications using 
it do the silent cast.

What do you think?

Re: [dpdk-dev] [PATCH V2 1/3] net: avoid cast-align warning in VLAN insert function

[Stephen Hemminger]

On Thu, 21 Oct 2021 19:16:19 +0300
Eli Britstein <elibr@nvidia.com> wrote:

> On 10/21/2021 6:48 PM, Stephen Hemminger wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On Thu, 21 Oct 2021 11:51:30 +0300
> > Eli Britstein <elibr@nvidia.com> wrote:
> >  
> >> In rte_vlan_insert there is a casting of rte_pktmbuf_prepend returned
> >> value to (struct rte_ether_hdr *), which causes cast-align warning when
> >> using strict cast align flag with supporting gcc:
> >> gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
> >> CFLAGS="-Wcast-align=strict" make V=1 -C examples/l2fwd clean static
> >>
> >> In file included from main.c:35:
> >> /dpdk/build/include/rte_ether.h:370:7: warning: cast increases required
> >> alignment of target type [-Wcast-align]
> >>    370 |  nh = (struct rte_ether_hdr *)
> >>        |       ^
> >>
> >> As the code assumes correct alignment, add first a (void *) casting, to
> >> avoid the warning.
> >>
> >> Fixes: c974021a5949 ("ether: add soft vlan encap/decap")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Eli Britstein <elibr@nvidia.com>
> >> Acked-by: Olivier Matz <olivier.matz@6wind.com>  
> > After cast to void * the second cast is not necessary.
> >
> >          nh = (void *)rte_pktmbuf_prepend(...)
> >
> > Ideally rte_pktmbuf_prepend() should return void * but that is
> > an API change.  
> 
> Removing the second cast, it is silently done anyway, as 'nh' is of type 
> 'struct rte_ether_hdr *'.
> 
> Going with this approach (I can also do it for patch 3/3), we can change 
> rte_pktmbuf_prepend to return (void *), and let the applications using 
> it do the silent cast.
> 
> What do you think?

Changing return type is an API change so it would need the whole
multistep process.

I overstated a little, it turns out the cast is necessary when header
is included by C++ code. C++ is pickier and doesn't allow void * to be
converted to other type by assignment.

Probably best to stick with what you originally proposed.
Gcc does have a bunch of alignment attribute types that could also fix this
but that gets even messier.