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

[dpdk-dev] [PATCH v3] ip_frag: 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.

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.

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 ++--
 9 files changed, 86 insertions(+), 21 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'
diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
index b830f67a5..58c388708 100644
--- a/examples/ip_reassembly/main.c
+++ b/examples/ip_reassembly/main.c
@@ -366,12 +366,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 b3f3f78df..bd6f02a16 100644
--- a/lib/librte_ip_frag/rte_ip_frag.h
+++ b/lib/librte_ip_frag/rte_ip_frag.h
@@ -208,28 +208,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 d1acf07cb..98fe4f2d4 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_net/rte_ip.h b/lib/librte_net/rte_ip.h
index f2a8904a2..edcbdb67a 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;
+	uint8_t hdrlen;
+} __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
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 v3] ip_frag: extend rte_ipv6_frag_get_ipv6_fragment_header()

[Cody Doucette]

Re-upping for review.

Thanks,
Cody

On Fri, Jul 27, 2018 at 9:52 AM, Cody Doucette <doucette@bu.edu> wrote:

> 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.
>
> 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 ++--
>  9 files changed, 86 insertions(+), 21 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'
> diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
> index b830f67a5..58c388708 100644
> --- a/examples/ip_reassembly/main.c
> +++ b/examples/ip_reassembly/main.c
> @@ -366,12 +366,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 b3f3f78df..bd6f02a16 100644
> --- a/lib/librte_ip_frag/rte_ip_frag.h
> +++ b/lib/librte_ip_frag/rte_ip_frag.h
> @@ -208,28 +208,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 d1acf07cb..98fe4f2d4 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_net/rte_ip.h b/lib/librte_net/rte_ip.h
> index f2a8904a2..edcbdb67a 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;
> +       uint8_t hdrlen;
> +} __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
> 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 v3] ip_frag: extend rte_ipv6_frag_get_ipv6_fragment_header()

[Thomas Monjalon]

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.

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

[Cody Doucette]

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

Best,
Cody

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

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

[Thomas Monjalon]

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.

Thanks

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

[Ananyev, Konstantin]

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Sunday, October 28, 2018 9:19 PM
> To: Cody Doucette <doucette@bu.edu>
> Cc: dev@dpdk.org; Gaetan Rivet <gaetan.rivet@6wind.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Olivier Matz
> <olivier.matz@6wind.com>; Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Michel Machado <michel@digirati.com.br>; Fu, Qiaobin
> <qiaobinf@bu.edu>
> Subject: Re: [dpdk-dev] [PATCH v3] ip_frag: extend rte_ipv6_frag_get_ipv6_fragment_header()
> 
> 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
Konstantin
  

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

[Thomas Monjalon]

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

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

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

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

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

[Thomas Monjalon]

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.


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

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

[Neil Horman]

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

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

[Cody Doucette]

Thanks for the suggestion. It looks like
49bcce138374458d1edd1c50d8e5726959108ef4 is already in my tree. I tried
applying and checking again anyway and it seems that the error is still
present.

On Wed, Oct 31, 2018 at 5:28 AM Neil Horman <nhorman@tuxdriver.com> wrote:

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

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

[Neil Horman]

On Wed, Oct 31, 2018 at 10:20:46AM -0400, Cody Doucette wrote:
> Thanks for the suggestion. It looks like
> 49bcce138374458d1edd1c50d8e5726959108ef4 is already in my tree. I tried
> applying and checking again anyway and it seems that the error is still
> present.
> 
Thats not a commit in the upstream tree, I've no idea what patch you are referring to

Neil

> On Wed, Oct 31, 2018 at 5:28 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> > 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
> > > > >
> > > > >
> > > > >
> > > >
> > >
> > >
> > >
> > >
> > >
> > >
> >

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

[Thomas Monjalon]

31/10/2018 16:03, Neil Horman:
> On Wed, Oct 31, 2018 at 10:20:46AM -0400, Cody Doucette wrote:
> > Thanks for the suggestion. It looks like
> > 49bcce138374458d1edd1c50d8e5726959108ef4 is already in my tree. I tried
> > applying and checking again anyway and it seems that the error is still
> > present.
> > 
> Thats not a commit in the upstream tree, I've no idea what patch you are referring to

Yes it is in the tree, in 18.11-rc1.


> > On Wed, Oct 31, 2018 at 5:28 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> > 
> > > 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
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> 

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

[Neil Horman]

On Wed, Oct 31, 2018 at 04:08:18PM +0100, Thomas Monjalon wrote:
> 31/10/2018 16:03, Neil Horman:
> > On Wed, Oct 31, 2018 at 10:20:46AM -0400, Cody Doucette wrote:
> > > Thanks for the suggestion. It looks like
> > > 49bcce138374458d1edd1c50d8e5726959108ef4 is already in my tree. I tried
> > > applying and checking again anyway and it seems that the error is still
> > > present.
> > > 
> > Thats not a commit in the upstream tree, I've no idea what patch you are referring to
> 
> Yes it is in the tree, in 18.11-rc1.
> 
Got it now, thanks.

So this actually looks to be a new bug, and I'm not sure how it got in there.
The awk match to close the in_map section isn't triggering on this patch set,
which is very odd, as Its been working. The match is ^(map) which should be ok
(allbeit perl regex syntax), which I thought awk supported.  That said, it
certainly doesn't seem to be matching now. 

I've got a patch posted to you which replaces this with a traditional character
class match [^map], and thats working with this patch set.

Neil

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

[Thomas Monjalon]

01/11/2018 14:53, Neil Horman:
> On Wed, Oct 31, 2018 at 04:08:18PM +0100, Thomas Monjalon wrote:
> > 31/10/2018 16:03, Neil Horman:
> > > On Wed, Oct 31, 2018 at 10:20:46AM -0400, Cody Doucette wrote:
> > > > Thanks for the suggestion. It looks like
> > > > 49bcce138374458d1edd1c50d8e5726959108ef4 is already in my tree. I tried
> > > > applying and checking again anyway and it seems that the error is still
> > > > present.
> > > > 
> > > Thats not a commit in the upstream tree, I've no idea what patch you are referring to
> > 
> > Yes it is in the tree, in 18.11-rc1.
> > 
> Got it now, thanks.
> 
> So this actually looks to be a new bug, and I'm not sure how it got in there.
> The awk match to close the in_map section isn't triggering on this patch set,
> which is very odd, as Its been working. The match is ^(map) which should be ok
> (allbeit perl regex syntax), which I thought awk supported.  That said, it
> certainly doesn't seem to be matching now. 

All these regex things are painful sometimes :)

> I've got a patch posted to you which replaces this with a traditional character
> class match [^map], and thats working with this patch set.

OK, I'll check it, thanks.