patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Olivier Matz <olivier.matz@6wind.com>
To: Eli Britstein <elibr@nvidia.com>
Cc: dev@dpdk.org, Ilya Maximets <i.maximets@ovn.org>,
	Gaetan Rivet <gaetanr@nvidia.com>, Majd Dibbiny <majd@nvidia.com>,
	Asaf Penso <asafp@nvidia.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	Harry Van Haaren <harry.van.haaren@intel.com>,
	stable@dpdk.org, Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH 2/3] mbuf: avoid cast-align warning in pktmbuf mtod offset macro
Date: Fri, 30 Jul 2021 13:10:15 +0200	[thread overview]
Message-ID: <YQPeFzx3e3VD0TNJ@platinum> (raw)
In-Reply-To: <045f7d4b-c9dc-0a1e-25c8-359f14dfcc66@nvidia.com>

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
> > > 

  reply	other threads:[~2021-07-30 11:10 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20210713064910.12793-1-elibr@nvidia.com>
2021-07-13  6:49 ` [dpdk-stable] [PATCH 1/3] net: avoid cast-align warning in VLAN insert function Eli Britstein
2021-07-30 10:57   ` [dpdk-stable] [dpdk-dev] " Olivier Matz
2021-07-13  6:49 ` [dpdk-stable] [PATCH 2/3] mbuf: avoid cast-align warning in pktmbuf mtod offset macro Eli Britstein
2021-07-13  7:43   ` Thomas Monjalon
2021-07-28 15:28   ` [dpdk-stable] [dpdk-dev] " Olivier Matz
2021-07-29  7:13     ` Eli Britstein
2021-07-30 11:10       ` Olivier Matz [this message]
2021-08-01  8:06         ` Eli Britstein
2021-10-19  6:41           ` Eli Britstein
2021-10-19  9:47             ` Olivier Matz
2021-07-13  6:49 ` [dpdk-stable] [PATCH 3/3] eal/x86: avoid cast-align warning in x86 memcpy functions Eli Britstein
2021-10-21  8:51 ` [dpdk-stable] [PATCH V2 1/3] net: avoid cast-align warning in VLAN insert function Eli Britstein
2021-10-21  8:51   ` [dpdk-stable] [PATCH V2 2/3] mbuf: avoid cast-align warning in pktmbuf mtod offset macro Eli Britstein
2021-10-21  8:51   ` [dpdk-stable] [PATCH V2 3/3] eal/x86: avoid cast-align warning in x86 memcpy functions Eli Britstein
2021-10-25 15:29     ` Thomas Monjalon
2021-10-21 15:48   ` [dpdk-stable] [dpdk-dev] [PATCH V2 1/3] net: avoid cast-align warning in VLAN insert function Stephen Hemminger
2021-10-21 16:16     ` Eli Britstein
2021-10-21 16:22       ` Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YQPeFzx3e3VD0TNJ@platinum \
    --to=olivier.matz@6wind.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=asafp@nvidia.com \
    --cc=dev@dpdk.org \
    --cc=elibr@nvidia.com \
    --cc=gaetanr@nvidia.com \
    --cc=harry.van.haaren@intel.com \
    --cc=i.maximets@ovn.org \
    --cc=majd@nvidia.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).