Re: [dpdk-dev] [PATCH v3] ip_frag: extend rte_ipv6_frag_get_ipv6_fragment_header()

[Neil Horman <nhorman@tuxdriver.com>]

On Wed, Oct 31, 2018 at 12:12:27AM +0100, Thomas Monjalon wrote:
> 30/10/2018 19:09, Cody Doucette:
> > OK, I will send three separate patches plus a cover letter.
> > 
> > I seem to be having trouble with checkpatch complaining that new symbols
> > are not inserted into the EXPERIMENTAL section of the .map file:
> > 
> > ERROR: symbol break is added in a section other than the EXPERIMENTAL
> > section of the version map
> > ERROR: symbol const is added in a section other than the EXPERIMENTAL
> > section of the version map
> > ERROR: symbol &frag_hdr_buf) is added in a section other than the
> > EXPERIMENTAL section of the version map
> > INFO: symbol frag_hdr is being removed, ensure that it has gone
> > through the deprecation process
> > INFO: symbol  is added but patch has insuficient context to determine
> > the section name please ensure the version is EXPERIMENTAL
> > ERROR: symbol offset, is added in a section other than the
> > EXPERIMENTAL section of the version map
> > ERROR: symbol offset is added in a section other than the EXPERIMENTAL
> > section of the version map
> > ERROR: symbol return is added in a section other than the EXPERIMENTAL
> > section of the version map
> > ERROR: symbol return is added in a section other than the EXPERIMENTAL
> > section of the version map
> > INFO: symbol  is added but patch has insuficient context to determine
> > the section name please ensure the version is EXPERIMENTAL
> > ERROR: symbol sizeof(*frag_hdr), is added in a section other than the
> > EXPERIMENTAL section of the version map
> > ERROR: symbol size_t is added in a section other than the EXPERIMENTAL
> > section of the version map
> > ERROR: symbol struct is added in a section other than the EXPERIMENTAL
> > section of the version map
> > INFO: symbol struct is being removed, ensure that it has gone through
> > the deprecation process
> > ERROR: symbol struct is added in a section other than the EXPERIMENTAL
> > section of the version map
> > ERROR: symbol uint8_t is added in a section other than the
> > EXPERIMENTAL section of the version map
> > 
> > Even when moving the new symbol into the EXPERIMENTAL version and
> > recreating the patch, checkpatch still issues the same errors.
> > 
> > Can I leave the .map file as it is in v3? If not, any suggestions on what
> > checkpatch is looking for me to do here?
> 
> Don't worry, it is a bug in the script.
> +Cc Neil who already looked at this issue.
> 
I need to look at the submitted patch to confirm, which I don't have time to do
at this moment, but my first though is that yes, this is fixed by recent commit 
49bcce138374458d1edd1c50d8e5726959108ef4.  Can you try applying and building to
the current head and see if the issue is resolved?

Neil

> 
> > On Tue, Oct 30, 2018 at 10:36 AM Thomas Monjalon <thomas@monjalon.net>
> > wrote:
> > 
> > > 30/10/2018 10:46, Ananyev, Konstantin:
> > > > Hi Thomas,
> > > >
> > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > 28/10/2018 21:54, Cody Doucette:
> > > > > > On Sun, Oct 28, 2018 at 6:22 AM Thomas Monjalon <thomas@monjalon.net>
> > > wrote:
> > > > > > > 27/07/2018 15:52, Cody Doucette:
> > > > > > > > Extend rte_ipv6_frag_get_ipv6_fragment_header() to skip over any
> > > > > > > > other IPv6 extension headers when finding the fragment header.
> > > > > > > >
> > > > > > > > According to RFC 8200, there is no guarantee that the IPv6
> > > > > > > > Fragment extension header will come before any other extension
> > > > > > > > header, even though it is recommended.
> > > > > > > >
> > > > > > > > Signed-off-by: Cody Doucette <doucette@bu.edu>
> > > > > > > > Signed-off-by: Qiaobin Fu <qiaobinf@bu.edu>
> > > > > > > > Reviewed-by: Michel Machado <michel@digirati.com.br>
> > > > > > > > ---
> > > > > > > > v3:
> > > > > > > > * Removed compilation flag D_XOPEN_SOURCE=700 from the
> > > > > > > >   failsafe driver to allow compilation on freebsd.
> > > > > > >
> > > > > > > How failsafe is related to ip_frag?
> > > > > > >
> > > > > > >
> > > > > > > > v2:
> > > > > > > > * Moved IPv6 extension header definitions to lib_net.
> > > > > > > >
> > > > > > > >  drivers/net/failsafe/Makefile               |  1 -
> > > > > > > >  drivers/net/failsafe/meson.build            |  1 -
> > > > > > > >  examples/ip_reassembly/main.c               |  6 ++--
> > > > > > > >  lib/librte_ip_frag/rte_ip_frag.h            | 23 ++++++-------
> > > > > > > >  lib/librte_ip_frag/rte_ip_frag_version.map  |  1 +
> > > > > > > >  lib/librte_ip_frag/rte_ipv6_fragmentation.c | 38
> > > +++++++++++++++++++++
> > > > > > > >  lib/librte_ip_frag/rte_ipv6_reassembly.c    |  4 +--
> > > > > > > >  lib/librte_net/rte_ip.h                     | 27 +++++++++++++++
> > > > > > > >  lib/librte_port/rte_port_ras.c              |  6 ++--
> > > > > > >
> > > > > > > Changes in failsafe, rte_net and rte_port look like garbage.
> > > > > > >
> > > > > > > Anyway, the ip_frag part requires some review.
> > > > > > > +Cc Konstantin, the maintainer.
> > > > > >
> > > > > > Garbage in what sense? I would be happy to amend with a little more
> > > > > > information.
> > > > > >
> > > > > > The changes to failsafe and rte_net were from previous reviews from
> > > > > > Konstantin:
> > > > > >
> > > > > > https://mails.dpdk.org/archives/dev/2018-June/106023.html
> > > > > >
> > > > > > https://mails.dpdk.org/archives/dev/2018-July/108701.html
> > > > >
> > > > > After a better look, the change in rte_port is fine.
> > > > >
> > > > > But the changes in failsafe and rte_net would be better in their own
> > > patch.
> > > > > You can have 3 patches in a patchset (with a cover letter to explain
> > > the
> > > > > global idea).
> > > > > Then, failsafe and rte_net changes must be reviewed by their
> > > maintainers.
> > > > >
> > > >
> > > > The patch looks good to me.
> > > > About failsafe changes - the reason for that was that failsafe driver
> > > didn't build
> > > > properly with the proposed changes.
> > > > Gaetan was ok to remove that extra compiler flag:
> > > > https://mails.dpdk.org/archives/dev/2018-July/108826.html
> > >
> > > OK. Please send the failsafe patch as the first of the series.
> > > Thanks
> > >
> > >
> > >
> > 
> 
> 
> 
> 
> 
>