[dpdk-dev] [PATCH v5 0/3] Extend rte_ipv6_frag_get_ipv6_fragment_header()

[dpdk-dev] [PATCH v5 0/3] Extend rte_ipv6_frag_get_ipv6_fragment_header()

[Cody Doucette]

Extend rte_ipv6_frag_get_ipv6_fragment_header() to skip over any
other IPv6 extension headers when finding the fragment header.
---
v5:
* Removed duplicate patch from submission.

v4:
* Separated into multiple patches and added cover letter.

v3:
* Removed compilation flag D_XOPEN_SOURCE=700 from the
  failsafe driver to allow compilation on freebsd.

v2:
* Moved IPv6 extension header definitions to lib_net.

Cody Doucette (3):
  net/failsafe: remove D_XOPEN_SOURCE flag
  net: add IPv6 extension header definitions
  ip_frag: extend IPv6 fragment header retrieval

 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 ++--
 9 files changed, 86 insertions(+), 21 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH v5 1/3] net/failsafe: remove D_XOPEN_SOURCE flag

[Cody Doucette]

Compilation flag D_XOPEN_SOURCE is causing failsafe to fail to
build when changing some DPDK libraries. The flag is not needed
and therefore is removed.

Signed-off-by: Cody Doucette <doucette@bu.edu>
---
 drivers/net/failsafe/Makefile    | 1 -
 drivers/net/failsafe/meson.build | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/net/failsafe/Makefile b/drivers/net/failsafe/Makefile
index 81802d092..eb9292425 100644
--- a/drivers/net/failsafe/Makefile
+++ b/drivers/net/failsafe/Makefile
@@ -34,7 +34,6 @@ CFLAGS += -std=gnu99 -Wextra
 CFLAGS += -O3
 CFLAGS += -I.
 CFLAGS += -D_DEFAULT_SOURCE
-CFLAGS += -D_XOPEN_SOURCE=700
 CFLAGS += $(WERROR_FLAGS)
 CFLAGS += -Wno-strict-prototypes
 CFLAGS += -pedantic
diff --git a/drivers/net/failsafe/meson.build b/drivers/net/failsafe/meson.build
index a249ff4af..d47a52acd 100644
--- a/drivers/net/failsafe/meson.build
+++ b/drivers/net/failsafe/meson.build
@@ -3,7 +3,6 @@
 
 cflags += '-std=gnu99'
 cflags += '-D_DEFAULT_SOURCE'
-cflags += '-D_XOPEN_SOURCE=700'
 cflags += '-pedantic'
 if host_machine.system() == 'linux'
 	cflags += '-DLINUX'
-- 
2.17.1

Re: [dpdk-dev] [PATCH v5 1/3] net/failsafe: remove D_XOPEN_SOURCE flag

[Thomas Monjalon]

31/10/2018 03:17, Cody Doucette:
> Compilation flag D_XOPEN_SOURCE is causing failsafe to fail to
> build when changing some DPDK libraries. The flag is not needed
> and therefore is removed.
> 
> Signed-off-by: Cody Doucette <doucette@bu.edu>

Gaetan, please approve that the flag is not needed.

Re: [dpdk-dev] [PATCH v5 1/3] net/failsafe: remove D_XOPEN_SOURCE flag

[Gaëtan Rivet]

On Wed, Oct 31, 2018 at 06:02:29PM +0100, Thomas Monjalon wrote:
> 31/10/2018 03:17, Cody Doucette:
> > Compilation flag D_XOPEN_SOURCE is causing failsafe to fail to
> > build when changing some DPDK libraries. The flag is not needed
> > and therefore is removed.
> > 
> > Signed-off-by: Cody Doucette <doucette@bu.edu>
> 
> Gaetan, please approve that the flag is not needed.
> 

I suspected popen or fgets from failsafe_args.c, but I wasn't able
to find a reason for the define. I can't remember the initial reason for
it, sorry.

I have tested the build on linux and freebsd, without issue, so at this
point I think this is ok.

-- 
Gaëtan Rivet
6WIND

[dpdk-dev] [PATCH v5 2/3] net: add IPv6 extension header definitions

[Cody Doucette]

Add a common IPv6 extension header and an inline function
for determining whether a next header field represents
an IPv6 extension header.

Signed-off-by: Cody Doucette <doucette@bu.edu>
---
 lib/librte_net/rte_ip.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index f2a8904a2..43dbb15e4 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -350,6 +350,14 @@ struct ipv6_hdr {
 #define IPV6_HDR_FL_MASK ((1u << IPV6_HDR_TC_SHIFT) - 1)
 #define IPV6_HDR_TC_MASK (0xf << IPV6_HDR_TC_SHIFT)
 
+/**
+ * IPv6 extension header
+ */
+struct ipv6_opt_hdr {
+	uint8_t nexthdr; /**< Next header. */
+	uint8_t hdrlen;	 /**< Length; presence & meaning varies by ext type. */
+} __attribute__((packed));
+
 /**
  * Process the pseudo-header checksum of an IPv6 header.
  *
@@ -421,6 +429,25 @@ rte_ipv6_udptcp_cksum(const struct ipv6_hdr *ipv6_hdr, const void *l4_hdr)
 	return (uint16_t)cksum;
 }
 
+/**
+ * Check whether an IPv6 nexthdr value is for an IPv6 extension header.
+ *
+ * @param nexthdr
+ *   The protocol/next header field from an IPv6 (extension) header.
+ * @return
+ *   Whether the nexthdr field is for an IPv6 extension header.
+ */
+static inline int
+ipv6_ext_hdr(uint8_t nexthdr)
+{
+	return (nexthdr == IPPROTO_HOPOPTS) ||
+		(nexthdr == IPPROTO_ROUTING) ||
+		(nexthdr == IPPROTO_FRAGMENT) ||
+		(nexthdr == IPPROTO_AH) ||
+		(nexthdr == IPPROTO_NONE) ||
+		(nexthdr == IPPROTO_DSTOPTS);
+}
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.17.1

Re: [dpdk-dev] [PATCH v5 2/3] net: add IPv6 extension header definitions

[Thomas Monjalon]

31/10/2018 03:17, Cody Doucette:
> Add a common IPv6 extension header and an inline function
> for determining whether a next header field represents
> an IPv6 extension header.
> 
> Signed-off-by: Cody Doucette <doucette@bu.edu>

Olivier, is this patch OK?

Re: [dpdk-dev] [PATCH v5 2/3] net: add IPv6 extension header definitions

[Olivier Matz]

On Wed, Oct 31, 2018 at 06:03:37PM +0100, Thomas Monjalon wrote:
> 31/10/2018 03:17, Cody Doucette:
> > Add a common IPv6 extension header and an inline function
> > for determining whether a next header field represents
> > an IPv6 extension header.
> > 
> > Signed-off-by: Cody Doucette <doucette@bu.edu>
> 
> Olivier, is this patch OK?

Yes, thanks

Acked-by: Olivier Matz <olivier.matz@6wind.com>

[dpdk-dev] [PATCH v5 3/3] ip_frag: extend IPv6 fragment header retrieval

[Cody Doucette]

Add the ability to parse IPv6 extenders to find the
IPv6 fragment header, and update callers.

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>
---
 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_port/rte_port_ras.c              |  6 ++--
 6 files changed, 59 insertions(+), 19 deletions(-)

diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
index 17b55d4c7..3a827bd6c 100644
--- a/examples/ip_reassembly/main.c
+++ b/examples/ip_reassembly/main.c
@@ -365,12 +365,14 @@ reassemble(struct rte_mbuf *m, uint16_t portid, uint32_t queue,
 		eth_hdr->ether_type = rte_be_to_cpu_16(ETHER_TYPE_IPv4);
 	} else if (RTE_ETH_IS_IPV6_HDR(m->packet_type)) {
 		/* if packet is IPv6 */
-		struct ipv6_extension_fragment *frag_hdr;
+		const struct ipv6_extension_fragment *frag_hdr;
+		struct ipv6_extension_fragment frag_hdr_buf;
 		struct ipv6_hdr *ip_hdr;
 
 		ip_hdr = (struct ipv6_hdr *)(eth_hdr + 1);
 
-		frag_hdr = rte_ipv6_frag_get_ipv6_fragment_header(ip_hdr);
+		frag_hdr = rte_ipv6_frag_get_ipv6_fragment_header(m,
+			ip_hdr, &frag_hdr_buf);
 
 		if (frag_hdr != NULL) {
 			struct rte_mbuf *mo;
diff --git a/lib/librte_ip_frag/rte_ip_frag.h b/lib/librte_ip_frag/rte_ip_frag.h
index 7f425f610..6fc8106bc 100644
--- a/lib/librte_ip_frag/rte_ip_frag.h
+++ b/lib/librte_ip_frag/rte_ip_frag.h
@@ -211,28 +211,25 @@ rte_ipv6_fragment_packet(struct rte_mbuf *pkt_in,
 struct rte_mbuf *rte_ipv6_frag_reassemble_packet(struct rte_ip_frag_tbl *tbl,
 		struct rte_ip_frag_death_row *dr,
 		struct rte_mbuf *mb, uint64_t tms, struct ipv6_hdr *ip_hdr,
-		struct ipv6_extension_fragment *frag_hdr);
+		const struct ipv6_extension_fragment *frag_hdr);
 
 /**
  * Return a pointer to the packet's fragment header, if found.
- * It only looks at the extension header that's right after the fixed IPv6
- * header, and doesn't follow the whole chain of extension headers.
  *
- * @param hdr
+ * @param pkt
+ *   Pointer to the mbuf of the packet.
+ * @param ip_hdr
  *   Pointer to the IPv6 header.
+ * @param frag_hdr
+ *   A pointer to the buffer where the fragment header
+ *   will be copied if it is not contiguous in mbuf data.
  * @return
  *   Pointer to the IPv6 fragment extension header, or NULL if it's not
  *   present.
  */
-static inline struct ipv6_extension_fragment *
-rte_ipv6_frag_get_ipv6_fragment_header(struct ipv6_hdr *hdr)
-{
-	if (hdr->proto == IPPROTO_FRAGMENT) {
-		return (struct ipv6_extension_fragment *) ++hdr;
-	}
-	else
-		return NULL;
-}
+const struct ipv6_extension_fragment *rte_ipv6_frag_get_ipv6_fragment_header(
+		struct rte_mbuf *pkt, const struct ipv6_hdr *ip_hdr,
+		struct ipv6_extension_fragment *frag_hdr);
 
 /**
  * IPv4 fragmentation.
diff --git a/lib/librte_ip_frag/rte_ip_frag_version.map b/lib/librte_ip_frag/rte_ip_frag_version.map
index d40d5515f..8b4c82d08 100644
--- a/lib/librte_ip_frag/rte_ip_frag_version.map
+++ b/lib/librte_ip_frag/rte_ip_frag_version.map
@@ -8,6 +8,7 @@ DPDK_2.0 {
 	rte_ipv4_fragment_packet;
 	rte_ipv6_frag_reassemble_packet;
 	rte_ipv6_fragment_packet;
+	rte_ipv6_frag_get_ipv6_fragment_header;
 
 	local: *;
 };
diff --git a/lib/librte_ip_frag/rte_ipv6_fragmentation.c b/lib/librte_ip_frag/rte_ipv6_fragmentation.c
index 62a7e4e83..bd847dd3d 100644
--- a/lib/librte_ip_frag/rte_ipv6_fragmentation.c
+++ b/lib/librte_ip_frag/rte_ipv6_fragmentation.c
@@ -176,3 +176,41 @@ rte_ipv6_fragment_packet(struct rte_mbuf *pkt_in,
 
 	return out_pkt_pos;
 }
+
+const struct ipv6_extension_fragment *
+rte_ipv6_frag_get_ipv6_fragment_header(struct rte_mbuf *pkt,
+	const struct ipv6_hdr *ip_hdr,
+	struct ipv6_extension_fragment *frag_hdr)
+{
+	size_t offset = sizeof(struct ipv6_hdr);
+	uint8_t nexthdr = ip_hdr->proto;
+
+	while (ipv6_ext_hdr(nexthdr)) {
+		struct ipv6_opt_hdr opt;
+		const struct ipv6_opt_hdr *popt = rte_pktmbuf_read(pkt,
+			offset, sizeof(opt), &opt);
+		if (popt == NULL)
+			return NULL;
+
+		switch (nexthdr) {
+		case IPPROTO_NONE:
+			return NULL;
+
+		case IPPROTO_FRAGMENT:
+			return rte_pktmbuf_read(pkt, offset,
+				sizeof(*frag_hdr), frag_hdr);
+
+		case IPPROTO_AH:
+			offset += (popt->hdrlen + 2) << 2;
+			break;
+
+		default:
+			offset += (popt->hdrlen + 1) << 3;
+			break;
+		}
+
+		nexthdr = popt->nexthdr;
+	}
+
+	return NULL;
+}
diff --git a/lib/librte_ip_frag/rte_ipv6_reassembly.c b/lib/librte_ip_frag/rte_ipv6_reassembly.c
index db249fe60..b2d67a3f0 100644
--- a/lib/librte_ip_frag/rte_ipv6_reassembly.c
+++ b/lib/librte_ip_frag/rte_ipv6_reassembly.c
@@ -135,8 +135,8 @@ ipv6_frag_reassemble(struct ip_frag_pkt *fp)
 #define FRAG_OFFSET(x) (rte_cpu_to_be_16(x) >> 3)
 struct rte_mbuf *
 rte_ipv6_frag_reassemble_packet(struct rte_ip_frag_tbl *tbl,
-		struct rte_ip_frag_death_row *dr, struct rte_mbuf *mb, uint64_t tms,
-		struct ipv6_hdr *ip_hdr, struct ipv6_extension_fragment *frag_hdr)
+	struct rte_ip_frag_death_row *dr, struct rte_mbuf *mb, uint64_t tms,
+	struct ipv6_hdr *ip_hdr, const struct ipv6_extension_fragment *frag_hdr)
 {
 	struct ip_frag_pkt *fp;
 	struct ip_frag_key key;
diff --git a/lib/librte_port/rte_port_ras.c b/lib/librte_port/rte_port_ras.c
index c8b2e19bf..28764f744 100644
--- a/lib/librte_port/rte_port_ras.c
+++ b/lib/librte_port/rte_port_ras.c
@@ -184,9 +184,11 @@ process_ipv6(struct rte_port_ring_writer_ras *p, struct rte_mbuf *pkt)
 	/* Assume there is no ethernet header */
 	struct ipv6_hdr *pkt_hdr = rte_pktmbuf_mtod(pkt, struct ipv6_hdr *);
 
-	struct ipv6_extension_fragment *frag_hdr;
+	const struct ipv6_extension_fragment *frag_hdr;
+	struct ipv6_extension_fragment frag_hdr_buf;
 	uint16_t frag_data = 0;
-	frag_hdr = rte_ipv6_frag_get_ipv6_fragment_header(pkt_hdr);
+	frag_hdr = rte_ipv6_frag_get_ipv6_fragment_header(pkt, pkt_hdr,
+		&frag_hdr_buf);
 	if (frag_hdr != NULL)
 		frag_data = rte_be_to_cpu_16(frag_hdr->frag_data);
 
-- 
2.17.1

Re: [dpdk-dev] [PATCH v5 3/3] ip_frag: extend IPv6 fragment header retrieval

[Thomas Monjalon]

31/10/2018 03:17, Cody Doucette:
> Add the ability to parse IPv6 extenders to find the
> IPv6 fragment header, and update callers.
> 
> 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>

Konstantin, did you already ack this patch?

Re: [dpdk-dev] [PATCH v5 3/3] ip_frag: extend IPv6 fragment header retrieval

[Ananyev, Konstantin]

Hi Cody,

> 
> Add the ability to parse IPv6 extenders to find the
> IPv6 fragment header, and update callers.
> 
> 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>
> ---
>  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_port/rte_port_ras.c              |  6 ++--
>  6 files changed, 59 insertions(+), 19 deletions(-)
> 
> diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
> index 17b55d4c7..3a827bd6c 100644
> --- a/examples/ip_reassembly/main.c
> +++ b/examples/ip_reassembly/main.c
> @@ -365,12 +365,14 @@ reassemble(struct rte_mbuf *m, uint16_t portid, uint32_t queue,
>  		eth_hdr->ether_type = rte_be_to_cpu_16(ETHER_TYPE_IPv4);
>  	} else if (RTE_ETH_IS_IPV6_HDR(m->packet_type)) {
>  		/* if packet is IPv6 */
> -		struct ipv6_extension_fragment *frag_hdr;
> +		const struct ipv6_extension_fragment *frag_hdr;
> +		struct ipv6_extension_fragment frag_hdr_buf;
>  		struct ipv6_hdr *ip_hdr;
> 
>  		ip_hdr = (struct ipv6_hdr *)(eth_hdr + 1);
> 
> -		frag_hdr = rte_ipv6_frag_get_ipv6_fragment_header(ip_hdr);
> +		frag_hdr = rte_ipv6_frag_get_ipv6_fragment_header(m,
> +			ip_hdr, &frag_hdr_buf);

I looked at the patch once again, and it seems incomplete to me.
Sorry for late comments.
Yes, right now te_ipv6_frag_get_ipv6_fragment_header can properly
retrieve ipv6 fragment info, but it is not enough to make things work
for situation when we have packet with frag header not the first and only
extension header.
In the same function, few lines below, we setup l3_len based on that assumption:

m->l3_len = sizeof(*ip_hdr) + sizeof(*frag_hdr);

mo = rte_ipv6_frag_reassemble_packet(tbl, dr, m, tms, ip_hdr, frag_hdr);

And inside rte_ipv6_frag_reassemble_packet() we still assume the same:
...
frag_hdr = (struct ipv6_extension_fragment *) (ip_hdr + 1);
ip_hdr->proto = frag_hdr->next_header;

I think we need a function that would allow us to get offset of frag_hdr. 
Actually probably we can have a generic one here, that can return offset for
any requested ext header or total length of ipv6 header.
Something like that:

struct rte_ipv6_get_xhdr_ofs {
   uint16_t find_proto;    /* header proto to find */
   uint16_t  next_proto;  /* next header proto */
   uint32_t next_ofs;       /* offset to start search */
};

struct int 
rte_ipv6_get_xhdr_ofs(struct rte_mbuf *pkt, rte_ipv6_get_xhdr_ofs *find); 

that would go through ipv6 ext headers till either requested proto is found, or end of IPv6 header. 
Then user can do something like that:

/* find fragment extention */
ipv6_get_xhdr_ofs ofs = {
	.find_proto = IPPROTO_FRAGMENT,
	.next_proto = ipv6_hdr->proto,
	.ofs = sizeof(struct ipv6_hdr),
};
rc = rte_ipv6_get_xhdr_ofs(pkt, &ofs);
if(rc == 0) 
     frag_hdr = rte_pktmbuf_mtod_offset(m, .., ofs.ofs);
...     

/* get size of IPv6 header plus all known extensions */
ipv6_get_xhdr_ofs ofs = {
	.find_proto = IPPROTO_MAX,
	.next_proto = ipv6_hdr->proto,
	.ofs = sizeof(struct ipv6_hdr),
};
rc = rte_ipv6_get_xhdr_ofs(pkt, &ofs);     


> 
>  		if (frag_hdr != NULL) {
>  			struct rte_mbuf *mo;
> diff --git a/lib/librte_ip_frag/rte_ip_frag.h b/lib/librte_ip_frag/rte_ip_frag.h
> index 7f425f610..6fc8106bc 100644
> --- a/lib/librte_ip_frag/rte_ip_frag.h
> +++ b/lib/librte_ip_frag/rte_ip_frag.h
> @@ -211,28 +211,25 @@ rte_ipv6_fragment_packet(struct rte_mbuf *pkt_in,
>  struct rte_mbuf *rte_ipv6_frag_reassemble_packet(struct rte_ip_frag_tbl *tbl,
>  		struct rte_ip_frag_death_row *dr,
>  		struct rte_mbuf *mb, uint64_t tms, struct ipv6_hdr *ip_hdr,
> -		struct ipv6_extension_fragment *frag_hdr);
> +		const struct ipv6_extension_fragment *frag_hdr);
> 
>  /**
>   * Return a pointer to the packet's fragment header, if found.
> - * It only looks at the extension header that's right after the fixed IPv6
> - * header, and doesn't follow the whole chain of extension headers.
>   *
> - * @param hdr
> + * @param pkt
> + *   Pointer to the mbuf of the packet.
> + * @param ip_hdr
>   *   Pointer to the IPv6 header.
> + * @param frag_hdr
> + *   A pointer to the buffer where the fragment header
> + *   will be copied if it is not contiguous in mbuf data.
>   * @return
>   *   Pointer to the IPv6 fragment extension header, or NULL if it's not
>   *   present.
>   */
> -static inline struct ipv6_extension_fragment *
> -rte_ipv6_frag_get_ipv6_fragment_header(struct ipv6_hdr *hdr)
> -{
> -	if (hdr->proto == IPPROTO_FRAGMENT) {
> -		return (struct ipv6_extension_fragment *) ++hdr;
> -	}
> -	else
> -		return NULL;
> -}
> +const struct ipv6_extension_fragment *rte_ipv6_frag_get_ipv6_fragment_header(
> +		struct rte_mbuf *pkt, const struct ipv6_hdr *ip_hdr,
> +		struct ipv6_extension_fragment *frag_hdr);

Another thing - wouldn't it be ab API/ABI breakage?
One more question - making it non-inline - how much it would  affect performance?
My guess - no big difference, but did you check?
Konstantin

> 
>  /**
>   * IPv4 fragmentation.
> diff --git a/lib/librte_ip_frag/rte_ip_frag_version.map b/lib/librte_ip_frag/rte_ip_frag_version.map
> index d40d5515f..8b4c82d08 100644
> --- a/lib/librte_ip_frag/rte_ip_frag_version.map
> +++ b/lib/librte_ip_frag/rte_ip_frag_version.map
> @@ -8,6 +8,7 @@ DPDK_2.0 {
>  	rte_ipv4_fragment_packet;
>  	rte_ipv6_frag_reassemble_packet;
>  	rte_ipv6_fragment_packet;
> +	rte_ipv6_frag_get_ipv6_fragment_header;
> 
>  	local: *;
>  };
> diff --git a/lib/librte_ip_frag/rte_ipv6_fragmentation.c b/lib/librte_ip_frag/rte_ipv6_fragmentation.c
> index 62a7e4e83..bd847dd3d 100644
> --- a/lib/librte_ip_frag/rte_ipv6_fragmentation.c
> +++ b/lib/librte_ip_frag/rte_ipv6_fragmentation.c
> @@ -176,3 +176,41 @@ rte_ipv6_fragment_packet(struct rte_mbuf *pkt_in,
> 
>  	return out_pkt_pos;
>  }
> +
> +const struct ipv6_extension_fragment *
> +rte_ipv6_frag_get_ipv6_fragment_header(struct rte_mbuf *pkt,
> +	const struct ipv6_hdr *ip_hdr,
> +	struct ipv6_extension_fragment *frag_hdr)
> +{
> +	size_t offset = sizeof(struct ipv6_hdr);
> +	uint8_t nexthdr = ip_hdr->proto;
> +
> +	while (ipv6_ext_hdr(nexthdr)) {
> +		struct ipv6_opt_hdr opt;
> +		const struct ipv6_opt_hdr *popt = rte_pktmbuf_read(pkt,
> +			offset, sizeof(opt), &opt);
> +		if (popt == NULL)
> +			return NULL;
> +
> +		switch (nexthdr) {
> +		case IPPROTO_NONE:
> +			return NULL;
> +
> +		case IPPROTO_FRAGMENT:
> +			return rte_pktmbuf_read(pkt, offset,
> +				sizeof(*frag_hdr), frag_hdr);
> +
> +		case IPPROTO_AH:
> +			offset += (popt->hdrlen + 2) << 2;
> +			break;
> +
> +		default:
> +			offset += (popt->hdrlen + 1) << 3;
> +			break;
> +		}
> +
> +		nexthdr = popt->nexthdr;
> +	}
> +
> +	return NULL;
> +}
> diff --git a/lib/librte_ip_frag/rte_ipv6_reassembly.c b/lib/librte_ip_frag/rte_ipv6_reassembly.c
> index db249fe60..b2d67a3f0 100644
> --- a/lib/librte_ip_frag/rte_ipv6_reassembly.c
> +++ b/lib/librte_ip_frag/rte_ipv6_reassembly.c
> @@ -135,8 +135,8 @@ ipv6_frag_reassemble(struct ip_frag_pkt *fp)
>  #define FRAG_OFFSET(x) (rte_cpu_to_be_16(x) >> 3)
>  struct rte_mbuf *
>  rte_ipv6_frag_reassemble_packet(struct rte_ip_frag_tbl *tbl,
> -		struct rte_ip_frag_death_row *dr, struct rte_mbuf *mb, uint64_t tms,
> -		struct ipv6_hdr *ip_hdr, struct ipv6_extension_fragment *frag_hdr)
> +	struct rte_ip_frag_death_row *dr, struct rte_mbuf *mb, uint64_t tms,
> +	struct ipv6_hdr *ip_hdr, const struct ipv6_extension_fragment *frag_hdr)
>  {
>  	struct ip_frag_pkt *fp;
>  	struct ip_frag_key key;
> diff --git a/lib/librte_port/rte_port_ras.c b/lib/librte_port/rte_port_ras.c
> index c8b2e19bf..28764f744 100644
> --- a/lib/librte_port/rte_port_ras.c
> +++ b/lib/librte_port/rte_port_ras.c
> @@ -184,9 +184,11 @@ process_ipv6(struct rte_port_ring_writer_ras *p, struct rte_mbuf *pkt)
>  	/* Assume there is no ethernet header */
>  	struct ipv6_hdr *pkt_hdr = rte_pktmbuf_mtod(pkt, struct ipv6_hdr *);
> 
> -	struct ipv6_extension_fragment *frag_hdr;
> +	const struct ipv6_extension_fragment *frag_hdr;
> +	struct ipv6_extension_fragment frag_hdr_buf;
>  	uint16_t frag_data = 0;
> -	frag_hdr = rte_ipv6_frag_get_ipv6_fragment_header(pkt_hdr);
> +	frag_hdr = rte_ipv6_frag_get_ipv6_fragment_header(pkt, pkt_hdr,
> +		&frag_hdr_buf);
>  	if (frag_hdr != NULL)
>  		frag_data = rte_be_to_cpu_16(frag_hdr->frag_data);
> 
> --
> 2.17.1

Re: [dpdk-dev] [PATCH v5 3/3] ip_frag: extend IPv6 fragment header retrieval

[Cody Doucette]

Hey Konstantin,

Thanks for reviewing -- I see your point about this patch only removing one
of the places where the code makes assumptions about the position of the
fragmentation header.

Unfortunately at the moment I don't have the resources to dedicate to
writing the complete solution and doing a performance check, so I think I
should withdraw the patch. I hope it at least serves as a blueprint in case
someone comes back to it.

I might be able to come back to it eventually, but likely not soon.

Thanks,
Cody

On Wed, Oct 31, 2018 at 3:57 PM Ananyev, Konstantin <
konstantin.ananyev@intel.com> wrote:

> Hi Cody,
>
> >
> > Add the ability to parse IPv6 extenders to find the
> > IPv6 fragment header, and update callers.
> >
> > 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>
> > ---
> >  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_port/rte_port_ras.c              |  6 ++--
> >  6 files changed, 59 insertions(+), 19 deletions(-)
> >
> > diff --git a/examples/ip_reassembly/main.c
> b/examples/ip_reassembly/main.c
> > index 17b55d4c7..3a827bd6c 100644
> > --- a/examples/ip_reassembly/main.c
> > +++ b/examples/ip_reassembly/main.c
> > @@ -365,12 +365,14 @@ reassemble(struct rte_mbuf *m, uint16_t portid,
> uint32_t queue,
> >               eth_hdr->ether_type = rte_be_to_cpu_16(ETHER_TYPE_IPv4);
> >       } else if (RTE_ETH_IS_IPV6_HDR(m->packet_type)) {
> >               /* if packet is IPv6 */
> > -             struct ipv6_extension_fragment *frag_hdr;
> > +             const struct ipv6_extension_fragment *frag_hdr;
> > +             struct ipv6_extension_fragment frag_hdr_buf;
> >               struct ipv6_hdr *ip_hdr;
> >
> >               ip_hdr = (struct ipv6_hdr *)(eth_hdr + 1);
> >
> > -             frag_hdr = rte_ipv6_frag_get_ipv6_fragment_header(ip_hdr);
> > +             frag_hdr = rte_ipv6_frag_get_ipv6_fragment_header(m,
> > +                     ip_hdr, &frag_hdr_buf);
>
> I looked at the patch once again, and it seems incomplete to me.
> Sorry for late comments.
> Yes, right now te_ipv6_frag_get_ipv6_fragment_header can properly
> retrieve ipv6 fragment info, but it is not enough to make things work
> for situation when we have packet with frag header not the first and only
> extension header.
> In the same function, few lines below, we setup l3_len based on that
> assumption:
>
> m->l3_len = sizeof(*ip_hdr) + sizeof(*frag_hdr);
>
> mo = rte_ipv6_frag_reassemble_packet(tbl, dr, m, tms, ip_hdr, frag_hdr);
>
> And inside rte_ipv6_frag_reassemble_packet() we still assume the same:
> ...
> frag_hdr = (struct ipv6_extension_fragment *) (ip_hdr + 1);
> ip_hdr->proto = frag_hdr->next_header;
>
> I think we need a function that would allow us to get offset of frag_hdr.
> Actually probably we can have a generic one here, that can return offset
> for
> any requested ext header or total length of ipv6 header.
> Something like that:
>
> struct rte_ipv6_get_xhdr_ofs {
>    uint16_t find_proto;    /* header proto to find */
>    uint16_t  next_proto;  /* next header proto */
>    uint32_t next_ofs;       /* offset to start search */
> };
>
> struct int
> rte_ipv6_get_xhdr_ofs(struct rte_mbuf *pkt, rte_ipv6_get_xhdr_ofs *find);
>
> that would go through ipv6 ext headers till either requested proto is
> found, or end of IPv6 header.
> Then user can do something like that:
>
> /* find fragment extention */
> ipv6_get_xhdr_ofs ofs = {
>         .find_proto = IPPROTO_FRAGMENT,
>         .next_proto = ipv6_hdr->proto,
>         .ofs = sizeof(struct ipv6_hdr),
> };
> rc = rte_ipv6_get_xhdr_ofs(pkt, &ofs);
> if(rc == 0)
>      frag_hdr = rte_pktmbuf_mtod_offset(m, .., ofs.ofs);
> ...
>
> /* get size of IPv6 header plus all known extensions */
> ipv6_get_xhdr_ofs ofs = {
>         .find_proto = IPPROTO_MAX,
>         .next_proto = ipv6_hdr->proto,
>         .ofs = sizeof(struct ipv6_hdr),
> };
> rc = rte_ipv6_get_xhdr_ofs(pkt, &ofs);
>
>
> >
> >               if (frag_hdr != NULL) {
> >                       struct rte_mbuf *mo;
> > diff --git a/lib/librte_ip_frag/rte_ip_frag.h
> b/lib/librte_ip_frag/rte_ip_frag.h
> > index 7f425f610..6fc8106bc 100644
> > --- a/lib/librte_ip_frag/rte_ip_frag.h
> > +++ b/lib/librte_ip_frag/rte_ip_frag.h
> > @@ -211,28 +211,25 @@ rte_ipv6_fragment_packet(struct rte_mbuf *pkt_in,
> >  struct rte_mbuf *rte_ipv6_frag_reassemble_packet(struct rte_ip_frag_tbl
> *tbl,
> >               struct rte_ip_frag_death_row *dr,
> >               struct rte_mbuf *mb, uint64_t tms, struct ipv6_hdr *ip_hdr,
> > -             struct ipv6_extension_fragment *frag_hdr);
> > +             const struct ipv6_extension_fragment *frag_hdr);
> >
> >  /**
> >   * Return a pointer to the packet's fragment header, if found.
> > - * It only looks at the extension header that's right after the fixed
> IPv6
> > - * header, and doesn't follow the whole chain of extension headers.
> >   *
> > - * @param hdr
> > + * @param pkt
> > + *   Pointer to the mbuf of the packet.
> > + * @param ip_hdr
> >   *   Pointer to the IPv6 header.
> > + * @param frag_hdr
> > + *   A pointer to the buffer where the fragment header
> > + *   will be copied if it is not contiguous in mbuf data.
> >   * @return
> >   *   Pointer to the IPv6 fragment extension header, or NULL if it's not
> >   *   present.
> >   */
> > -static inline struct ipv6_extension_fragment *
> > -rte_ipv6_frag_get_ipv6_fragment_header(struct ipv6_hdr *hdr)
> > -{
> > -     if (hdr->proto == IPPROTO_FRAGMENT) {
> > -             return (struct ipv6_extension_fragment *) ++hdr;
> > -     }
> > -     else
> > -             return NULL;
> > -}
> > +const struct ipv6_extension_fragment
> *rte_ipv6_frag_get_ipv6_fragment_header(
> > +             struct rte_mbuf *pkt, const struct ipv6_hdr *ip_hdr,
> > +             struct ipv6_extension_fragment *frag_hdr);
>
> Another thing - wouldn't it be ab API/ABI breakage?
> One more question - making it non-inline - how much it would  affect
> performance?
> My guess - no big difference, but did you check?
> Konstantin
>
> >
> >  /**
> >   * IPv4 fragmentation.
> > diff --git a/lib/librte_ip_frag/rte_ip_frag_version.map
> b/lib/librte_ip_frag/rte_ip_frag_version.map
> > index d40d5515f..8b4c82d08 100644
> > --- a/lib/librte_ip_frag/rte_ip_frag_version.map
> > +++ b/lib/librte_ip_frag/rte_ip_frag_version.map
> > @@ -8,6 +8,7 @@ DPDK_2.0 {
> >       rte_ipv4_fragment_packet;
> >       rte_ipv6_frag_reassemble_packet;
> >       rte_ipv6_fragment_packet;
> > +     rte_ipv6_frag_get_ipv6_fragment_header;
> >
> >       local: *;
> >  };
> > diff --git a/lib/librte_ip_frag/rte_ipv6_fragmentation.c
> b/lib/librte_ip_frag/rte_ipv6_fragmentation.c
> > index 62a7e4e83..bd847dd3d 100644
> > --- a/lib/librte_ip_frag/rte_ipv6_fragmentation.c
> > +++ b/lib/librte_ip_frag/rte_ipv6_fragmentation.c
> > @@ -176,3 +176,41 @@ rte_ipv6_fragment_packet(struct rte_mbuf *pkt_in,
> >
> >       return out_pkt_pos;
> >  }
> > +
> > +const struct ipv6_extension_fragment *
> > +rte_ipv6_frag_get_ipv6_fragment_header(struct rte_mbuf *pkt,
> > +     const struct ipv6_hdr *ip_hdr,
> > +     struct ipv6_extension_fragment *frag_hdr)
> > +{
> > +     size_t offset = sizeof(struct ipv6_hdr);
> > +     uint8_t nexthdr = ip_hdr->proto;
> > +
> > +     while (ipv6_ext_hdr(nexthdr)) {
> > +             struct ipv6_opt_hdr opt;
> > +             const struct ipv6_opt_hdr *popt = rte_pktmbuf_read(pkt,
> > +                     offset, sizeof(opt), &opt);
> > +             if (popt == NULL)
> > +                     return NULL;
> > +
> > +             switch (nexthdr) {
> > +             case IPPROTO_NONE:
> > +                     return NULL;
> > +
> > +             case IPPROTO_FRAGMENT:
> > +                     return rte_pktmbuf_read(pkt, offset,
> > +                             sizeof(*frag_hdr), frag_hdr);
> > +
> > +             case IPPROTO_AH:
> > +                     offset += (popt->hdrlen + 2) << 2;
> > +                     break;
> > +
> > +             default:
> > +                     offset += (popt->hdrlen + 1) << 3;
> > +                     break;
> > +             }
> > +
> > +             nexthdr = popt->nexthdr;
> > +     }
> > +
> > +     return NULL;
> > +}
> > diff --git a/lib/librte_ip_frag/rte_ipv6_reassembly.c
> b/lib/librte_ip_frag/rte_ipv6_reassembly.c
> > index db249fe60..b2d67a3f0 100644
> > --- a/lib/librte_ip_frag/rte_ipv6_reassembly.c
> > +++ b/lib/librte_ip_frag/rte_ipv6_reassembly.c
> > @@ -135,8 +135,8 @@ ipv6_frag_reassemble(struct ip_frag_pkt *fp)
> >  #define FRAG_OFFSET(x) (rte_cpu_to_be_16(x) >> 3)
> >  struct rte_mbuf *
> >  rte_ipv6_frag_reassemble_packet(struct rte_ip_frag_tbl *tbl,
> > -             struct rte_ip_frag_death_row *dr, struct rte_mbuf *mb,
> uint64_t tms,
> > -             struct ipv6_hdr *ip_hdr, struct ipv6_extension_fragment
> *frag_hdr)
> > +     struct rte_ip_frag_death_row *dr, struct rte_mbuf *mb, uint64_t
> tms,
> > +     struct ipv6_hdr *ip_hdr, const struct ipv6_extension_fragment
> *frag_hdr)
> >  {
> >       struct ip_frag_pkt *fp;
> >       struct ip_frag_key key;
> > diff --git a/lib/librte_port/rte_port_ras.c
> b/lib/librte_port/rte_port_ras.c
> > index c8b2e19bf..28764f744 100644
> > --- a/lib/librte_port/rte_port_ras.c
> > +++ b/lib/librte_port/rte_port_ras.c
> > @@ -184,9 +184,11 @@ process_ipv6(struct rte_port_ring_writer_ras *p,
> struct rte_mbuf *pkt)
> >       /* Assume there is no ethernet header */
> >       struct ipv6_hdr *pkt_hdr = rte_pktmbuf_mtod(pkt, struct ipv6_hdr
> *);
> >
> > -     struct ipv6_extension_fragment *frag_hdr;
> > +     const struct ipv6_extension_fragment *frag_hdr;
> > +     struct ipv6_extension_fragment frag_hdr_buf;
> >       uint16_t frag_data = 0;
> > -     frag_hdr = rte_ipv6_frag_get_ipv6_fragment_header(pkt_hdr);
> > +     frag_hdr = rte_ipv6_frag_get_ipv6_fragment_header(pkt, pkt_hdr,
> > +             &frag_hdr_buf);
> >       if (frag_hdr != NULL)
> >               frag_data = rte_be_to_cpu_16(frag_hdr->frag_data);
> >
> > --
> > 2.17.1
>
>

Re: [dpdk-dev] [PATCH v5 3/3] ip_frag: extend IPv6 fragment header retrieval

[Ananyev, Konstantin]

Hi Cody,

>Hey Konstantin,
>Thanks for reviewing -- I see your point about this patch only removing one of the places where the code makes assumptions about the position of the fragmentation header.

>Unfortunately at the moment I don't have the resources to dedicate to writing the complete solution and doing a performance check, so I think I should withdraw the patch.

Ok, NP that's understandable.
Again I provided my comments quite late.

> I hope it at least serves as a blueprint in case someone comes back to it.

Yes, I think it definitely will be useful.

>I might be able to come back to it eventually, but likely not soon.

Hopefully you'll will :)
Thanks
Konstantin

> 
> Add the ability to parse IPv6 extenders to find the
> IPv6 fragment header, and update callers.
> 
> 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>
> ---
>  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_port/rte_port_ras.c              |  6 ++--
>  6 files changed, 59 insertions(+), 19 deletions(-)
> 
> diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
> index 17b55d4c7..3a827bd6c 100644
> --- a/examples/ip_reassembly/main.c
> +++ b/examples/ip_reassembly/main.c
> @@ -365,12 +365,14 @@ reassemble(struct rte_mbuf *m, uint16_t portid, uint32_t queue,
>               eth_hdr->ether_type = rte_be_to_cpu_16(ETHER_TYPE_IPv4);
>       } else if (RTE_ETH_IS_IPV6_HDR(m->packet_type)) {
>               /* if packet is IPv6 */
> -             struct ipv6_extension_fragment *frag_hdr;
> +             const struct ipv6_extension_fragment *frag_hdr;
> +             struct ipv6_extension_fragment frag_hdr_buf;
>               struct ipv6_hdr *ip_hdr;
> 
>               ip_hdr = (struct ipv6_hdr *)(eth_hdr + 1);
> 
> -             frag_hdr = rte_ipv6_frag_get_ipv6_fragment_header(ip_hdr);
> +             frag_hdr = rte_ipv6_frag_get_ipv6_fragment_header(m,
> +                     ip_hdr, &frag_hdr_buf);

I looked at the patch once again, and it seems incomplete to me.
Sorry for late comments.
Yes, right now te_ipv6_frag_get_ipv6_fragment_header can properly
retrieve ipv6 fragment info, but it is not enough to make things work
for situation when we have packet with frag header not the first and only
extension header.
In the same function, few lines below, we setup l3_len based on that assumption:

m->l3_len = sizeof(*ip_hdr) + sizeof(*frag_hdr);

mo = rte_ipv6_frag_reassemble_packet(tbl, dr, m, tms, ip_hdr, frag_hdr);

And inside rte_ipv6_frag_reassemble_packet() we still assume the same:
...
frag_hdr = (struct ipv6_extension_fragment *) (ip_hdr + 1);
ip_hdr->proto = frag_hdr->next_header;

I think we need a function that would allow us to get offset of frag_hdr. 
Actually probably we can have a generic one here, that can return offset for
any requested ext header or total length of ipv6 header.
Something like that:

struct rte_ipv6_get_xhdr_ofs {
   uint16_t find_proto;    /* header proto to find */
   uint16_t  next_proto;  /* next header proto */
   uint32_t next_ofs;       /* offset to start search */
};

struct int 
rte_ipv6_get_xhdr_ofs(struct rte_mbuf *pkt, rte_ipv6_get_xhdr_ofs *find); 

that would go through ipv6 ext headers till either requested proto is found, or end of IPv6 header. 
Then user can do something like that:

/* find fragment extention */
ipv6_get_xhdr_ofs ofs = {
        .find_proto = IPPROTO_FRAGMENT,
        .next_proto = ipv6_hdr->proto,
        .ofs = sizeof(struct ipv6_hdr),
};
rc = rte_ipv6_get_xhdr_ofs(pkt, &ofs);
if(rc == 0) 
     frag_hdr = rte_pktmbuf_mtod_offset(m, .., ofs.ofs);
...     

/* get size of IPv6 header plus all known extensions */
ipv6_get_xhdr_ofs ofs = {
        .find_proto = IPPROTO_MAX,
        .next_proto = ipv6_hdr->proto,
        .ofs = sizeof(struct ipv6_hdr),
};
rc = rte_ipv6_get_xhdr_ofs(pkt, &ofs);     


> 
>               if (frag_hdr != NULL) {
>                       struct rte_mbuf *mo;
> diff --git a/lib/librte_ip_frag/rte_ip_frag.h b/lib/librte_ip_frag/rte_ip_frag.h
> index 7f425f610..6fc8106bc 100644
> --- a/lib/librte_ip_frag/rte_ip_frag.h
> +++ b/lib/librte_ip_frag/rte_ip_frag.h
> @@ -211,28 +211,25 @@ rte_ipv6_fragment_packet(struct rte_mbuf *pkt_in,
>  struct rte_mbuf *rte_ipv6_frag_reassemble_packet(struct rte_ip_frag_tbl *tbl,
>               struct rte_ip_frag_death_row *dr,
>               struct rte_mbuf *mb, uint64_t tms, struct ipv6_hdr *ip_hdr,
> -             struct ipv6_extension_fragment *frag_hdr);
> +             const struct ipv6_extension_fragment *frag_hdr);
> 
>  /**
>   * Return a pointer to the packet's fragment header, if found.
> - * It only looks at the extension header that's right after the fixed IPv6
> - * header, and doesn't follow the whole chain of extension headers.
>   *
> - * @param hdr
> + * @param pkt
> + *   Pointer to the mbuf of the packet.
> + * @param ip_hdr
>   *   Pointer to the IPv6 header.
> + * @param frag_hdr
> + *   A pointer to the buffer where the fragment header
> + *   will be copied if it is not contiguous in mbuf data.
>   * @return
>   *   Pointer to the IPv6 fragment extension header, or NULL if it's not
>   *   present.
>   */
> -static inline struct ipv6_extension_fragment *
> -rte_ipv6_frag_get_ipv6_fragment_header(struct ipv6_hdr *hdr)
> -{
> -     if (hdr->proto == IPPROTO_FRAGMENT) {
> -             return (struct ipv6_extension_fragment *) ++hdr;
> -     }
> -     else
> -             return NULL;
> -}
> +const struct ipv6_extension_fragment *rte_ipv6_frag_get_ipv6_fragment_header(
> +             struct rte_mbuf *pkt, const struct ipv6_hdr *ip_hdr,
> +             struct ipv6_extension_fragment *frag_hdr);

Another thing - wouldn't it be ab API/ABI breakage?
One more question - making it non-inline - how much it would  affect performance?
My guess - no big difference, but did you check?
Konstantin

> 
>  /**
>   * IPv4 fragmentation.
> diff --git a/lib/librte_ip_frag/rte_ip_frag_version.map b/lib/librte_ip_frag/rte_ip_frag_version.map
> index d40d5515f..8b4c82d08 100644
> --- a/lib/librte_ip_frag/rte_ip_frag_version.map
> +++ b/lib/librte_ip_frag/rte_ip_frag_version.map
> @@ -8,6 +8,7 @@ DPDK_2.0 {
>       rte_ipv4_fragment_packet;
>       rte_ipv6_frag_reassemble_packet;
>       rte_ipv6_fragment_packet;
> +     rte_ipv6_frag_get_ipv6_fragment_header;
> 
>       local: *;
>  };
> diff --git a/lib/librte_ip_frag/rte_ipv6_fragmentation.c b/lib/librte_ip_frag/rte_ipv6_fragmentation.c
> index 62a7e4e83..bd847dd3d 100644
> --- a/lib/librte_ip_frag/rte_ipv6_fragmentation.c
> +++ b/lib/librte_ip_frag/rte_ipv6_fragmentation.c
> @@ -176,3 +176,41 @@ rte_ipv6_fragment_packet(struct rte_mbuf *pkt_in,
> 
>       return out_pkt_pos;
>  }
> +
> +const struct ipv6_extension_fragment *
> +rte_ipv6_frag_get_ipv6_fragment_header(struct rte_mbuf *pkt,
> +     const struct ipv6_hdr *ip_hdr,
> +     struct ipv6_extension_fragment *frag_hdr)
> +{
> +     size_t offset = sizeof(struct ipv6_hdr);
> +     uint8_t nexthdr = ip_hdr->proto;
> +
> +     while (ipv6_ext_hdr(nexthdr)) {
> +             struct ipv6_opt_hdr opt;
> +             const struct ipv6_opt_hdr *popt = rte_pktmbuf_read(pkt,
> +                     offset, sizeof(opt), &opt);
> +             if (popt == NULL)
> +                     return NULL;
> +
> +             switch (nexthdr) {
> +             case IPPROTO_NONE:
> +                     return NULL;
> +
> +             case IPPROTO_FRAGMENT:
> +                     return rte_pktmbuf_read(pkt, offset,
> +                             sizeof(*frag_hdr), frag_hdr);
> +
> +             case IPPROTO_AH:
> +                     offset += (popt->hdrlen + 2) << 2;
> +                     break;
> +
> +             default:
> +                     offset += (popt->hdrlen + 1) << 3;
> +                     break;
> +             }
> +
> +             nexthdr = popt->nexthdr;
> +     }
> +
> +     return NULL;
> +}
> diff --git a/lib/librte_ip_frag/rte_ipv6_reassembly.c b/lib/librte_ip_frag/rte_ipv6_reassembly.c
> index db249fe60..b2d67a3f0 100644
> --- a/lib/librte_ip_frag/rte_ipv6_reassembly.c
> +++ b/lib/librte_ip_frag/rte_ipv6_reassembly.c
> @@ -135,8 +135,8 @@ ipv6_frag_reassemble(struct ip_frag_pkt *fp)
>  #define FRAG_OFFSET(x) (rte_cpu_to_be_16(x) >> 3)
>  struct rte_mbuf *
>  rte_ipv6_frag_reassemble_packet(struct rte_ip_frag_tbl *tbl,
> -             struct rte_ip_frag_death_row *dr, struct rte_mbuf *mb, uint64_t tms,
> -             struct ipv6_hdr *ip_hdr, struct ipv6_extension_fragment *frag_hdr)
> +     struct rte_ip_frag_death_row *dr, struct rte_mbuf *mb, uint64_t tms,
> +     struct ipv6_hdr *ip_hdr, const struct ipv6_extension_fragment *frag_hdr)
>  {
>       struct ip_frag_pkt *fp;
>       struct ip_frag_key key;
> diff --git a/lib/librte_port/rte_port_ras.c b/lib/librte_port/rte_port_ras.c
> index c8b2e19bf..28764f744 100644
> --- a/lib/librte_port/rte_port_ras.c
> +++ b/lib/librte_port/rte_port_ras.c
> @@ -184,9 +184,11 @@ process_ipv6(struct rte_port_ring_writer_ras *p, struct rte_mbuf *pkt)
>       /* Assume there is no ethernet header */
>       struct ipv6_hdr *pkt_hdr = rte_pktmbuf_mtod(pkt, struct ipv6_hdr *);
> 
> -     struct ipv6_extension_fragment *frag_hdr;
> +     const struct ipv6_extension_fragment *frag_hdr;
> +     struct ipv6_extension_fragment frag_hdr_buf;
>       uint16_t frag_data = 0;
> -     frag_hdr = rte_ipv6_frag_get_ipv6_fragment_header(pkt_hdr);
> +     frag_hdr = rte_ipv6_frag_get_ipv6_fragment_header(pkt, pkt_hdr,
> +             &frag_hdr_buf);
>       if (frag_hdr != NULL)
>               frag_data = rte_be_to_cpu_16(frag_hdr->frag_data);
> 
> --
> 2.17.1

Re: [dpdk-dev] [PATCH v5 3/3] ip_frag: extend IPv6 fragment header retrieval

[Stephen Hemminger]

On Wed, 7 Nov 2018 23:00:19 +0000
"Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:

> >Thanks for reviewing -- I see your point about this patch only removing one of the places where the code makes assumptions about the position of the fragmentation header.  
> 
> >Unfortunately at the moment I don't have the resources to dedicate to writing the complete solution and doing a performance check, so I think I should withdraw the patch.  
> 
> Ok, NP that's understandable.
> Again I provided my comments quite late.

Marking this patchset as closed with changes requested.
If someone wants to revive it they can rebase and resubmit.

[dpdk-dev] [PATCH v4 0/3] Extend rte_ipv6_frag_get_ipv6_fragment_header()

[Cody Doucette]

Extend rte_ipv6_frag_get_ipv6_fragment_header() to skip over any
other IPv6 extension headers when finding the fragment header.
---
v5:
* Removed duplicate patch from submission.

v4:
* Separated into multiple patches and added cover letter.

v3:
* Removed compilation flag D_XOPEN_SOURCE=700 from the
  failsafe driver to allow compilation on freebsd.

v2:
* Moved IPv6 extension header definitions to lib_net.

Cody Doucette (3):
  net/failsafe: remove D_XOPEN_SOURCE flag
  net: add IPv6 extension header definitions
  ip_frag: extend IPv6 fragment header retrieval

 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 ++--
 9 files changed, 86 insertions(+), 21 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH v5 2/3] net: add IPv6 extension header definitions

[Cody Doucette]

Add a common IPv6 extension header and an inline function
for determining whether a next header field represents
an IPv6 extension header.

Signed-off-by: Cody Doucette <doucette@bu.edu>
---
 lib/librte_net/rte_ip.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index f2a8904a2..43dbb15e4 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -350,6 +350,14 @@ struct ipv6_hdr {
 #define IPV6_HDR_FL_MASK ((1u << IPV6_HDR_TC_SHIFT) - 1)
 #define IPV6_HDR_TC_MASK (0xf << IPV6_HDR_TC_SHIFT)
 
+/**
+ * IPv6 extension header
+ */
+struct ipv6_opt_hdr {
+	uint8_t nexthdr; /**< Next header. */
+	uint8_t hdrlen;	 /**< Length; presence & meaning varies by ext type. */
+} __attribute__((packed));
+
 /**
  * Process the pseudo-header checksum of an IPv6 header.
  *
@@ -421,6 +429,25 @@ rte_ipv6_udptcp_cksum(const struct ipv6_hdr *ipv6_hdr, const void *l4_hdr)
 	return (uint16_t)cksum;
 }
 
+/**
+ * Check whether an IPv6 nexthdr value is for an IPv6 extension header.
+ *
+ * @param nexthdr
+ *   The protocol/next header field from an IPv6 (extension) header.
+ * @return
+ *   Whether the nexthdr field is for an IPv6 extension header.
+ */
+static inline int
+ipv6_ext_hdr(uint8_t nexthdr)
+{
+	return (nexthdr == IPPROTO_HOPOPTS) ||
+		(nexthdr == IPPROTO_ROUTING) ||
+		(nexthdr == IPPROTO_FRAGMENT) ||
+		(nexthdr == IPPROTO_AH) ||
+		(nexthdr == IPPROTO_NONE) ||
+		(nexthdr == IPPROTO_DSTOPTS);
+}
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.17.1