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

[Cody Doucette <doucette@bu.edu>]

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?

Cody

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