diff mbox series

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

Message ID 20211021085132.12672-1-elibr@nvidia.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [V2,1/3] net: avoid cast-align warning in VLAN insert function |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Eli Britstein Oct. 21, 2021, 8:51 a.m. UTC 
  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(-)

Comments

Stephen Hemminger Oct. 21, 2021, 3:48 p.m. UTC | #1 
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.
Eli Britstein Oct. 21, 2021, 4:16 p.m. UTC | #2 
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?
Stephen Hemminger Oct. 21, 2021, 4:22 p.m. UTC | #3 
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.
diff mbox series

Patch

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;