Re: [dpdk-dev] [PATCH 1/3] net: new ipv6 header extension parsing function

["Smoczynski, MarcinX" <marcinx.smoczynski@intel.com>]

Hi Akhil,
I've double checked and there is no problem with linking application against shared libraries, because inline functions are being inlined during compilation and before linking is done. To be more specific: there is not such symbol as rte_ipv6_get_next_ext in ipsec-secgw.o (which uses this function in prepare_one_packet function).

There are a number of inline experimental functions defined today  and they are mostly (43 out of 59) not listed in maps, e.g. rte_ticketlock functions family defined here:
lib/librte_eal/common/include/generic/rte_ticketlock.h

Is there any particular reason we should put inlines in maps? Maps are used by linker to control versions of shared libraries and inlines are not processed by linker.

I am aware this patchset causes build failures on BSDs - this is a project-wide issue which I have addressed here:
http://patchwork.dpdk.org/patch/53406/
I'll put a note in the next version of this patchset's cover that it depends on abovementioned patchset.

Marcin
> 
> Hi Konstantin,
> 
> >
> > Hi Akhil,
> >
> > >
> > > Hi Marcin,
> > >
> > > >
> > > > Introduce new function for IPv6 header extension parsing able to
> > > > determine extension length and next protocol number.
> > > >
> > > > This function is helpful when implementing IPv6 header traversing.
> > > >
> > > > Signed-off-by: Marcin Smoczynski <marcinx.smoczynski@intel.com>
> > > > ---
> > > >  lib/librte_net/rte_ip.h | 49
> > +++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 49 insertions(+)
> > > >
> > > > diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
> > > > index f9b909090..be64da662 100644
> > > > --- a/lib/librte_net/rte_ip.h
> > > > +++ b/lib/librte_net/rte_ip.h
> > > > @@ -425,6 +425,55 @@ rte_ipv6_udptcp_cksum(const struct ipv6_hdr
> > > > *ipv6_hdr, const void *l4_hdr)
> > > >  	return (uint16_t)cksum;
> > > >  }
> > > >
> > > > +/* IPv6 fragmentation header size */ #define
> > > > +RTE_IPV6_FRAG_HDR_SIZE 8
> > > > +
> > > > +/**
> > > > + * Parse next IPv6 header extension
> > > > + *
> > > > + * This function checks if proto number is an IPv6 extensions and
> > > > +parses its
> > > > + * data if so, providing information on next header and extension
> length.
> > > > + *
> > > > + * @param p
> > > > + *   Pointer to an extension raw data.
> > > > + * @param proto
> > > > + *   Protocol number extracted from the "next header" field from
> > > > + *   the IPv6 header or the previous extension.
> > > > + * @param ext_len
> > > > + *   Extension data length.
> > > > + * @return
> > > > + *   next protocol number if proto is an IPv6 extension, -EINVAL
> otherwise
> > > > + */
> > > > +static inline int __rte_experimental
> > >
> > >
> > > Rte_experimental may not be required for inline functions.
> >
> > AFAIK we do need that tag for both inline and non-inline functions,
> > till API will be transferred to 'stable' state:
> >
> > $ find lib -type f | xargs grep __rte_experimental | grep inline | wc
> > -l
> > 57
> 
> My first impression was that this API was not exposed. Inline APIs are
> normally not exposed, So I thought rte_experimental is not required. I
> believe only exposed APIs Need to be tagged with rte_experimental.
> 
> Now since this API is exposed to the application as well, it should have an
> entry in the .map file.
> Otherwise shared build will fail I think for application.
> 
> >
> > >
> > >
> > > > +rte_ipv6_get_next_ext(uint8_t *p, int proto, size_t *ext_len) {
> > > > +	int next_proto;
> > > > +
> > > > +	switch (proto) {
> > > > +	case IPPROTO_AH:
> > > > +		next_proto = *p++;
> > > > +		*ext_len = (*p + 2) * sizeof(uint32_t);
> > > > +		break;
> > > > +
> > > > +	case IPPROTO_HOPOPTS:
> > > > +	case IPPROTO_ROUTING:
> > > > +	case IPPROTO_DSTOPTS:
> > > > +		next_proto = *p++;
> > > > +		*ext_len = (*p + 1) * sizeof(uint64_t);
> > > > +		break;
> > > > +
> > > > +	case IPPROTO_FRAGMENT:
> > > I see that there is some compilation issues with respect to IPPROTO_xxx
> fields.
> > These are reported in patchworks if you need details.
> > >
> > > Could you please fix these and send next rev.
> > >
> > > Thanks,
> > > Akhil