DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: 'Cody Doucette' <doucette@bu.edu>,
	Olivier Matz <olivier.matz@6wind.com>,
	 "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"michel@digirati.com.br" <michel@digirati.com.br>,
	Qiaobin Fu <qiaobinf@bu.edu>,
	"gaetan.rivet@6wind.com" <gaetan.rivet@6wind.com>
Subject: Re: [dpdk-dev] [PATCH v2] ip_frag: extend rte_ipv6_frag_get_ipv6_fragment_header()
Date: Wed, 25 Jul 2018 09:11:30 +0000	[thread overview]
Message-ID: <2601191342CEEE43887BDE71AB977258DF51D4C3@irsmsx105.ger.corp.intel.com> (raw)

Hi Cody,

> -----Original Message-----
> From: Cody Doucette [mailto:doucette@bu.edu]
> Sent: Thursday, July 19, 2018 4:58 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Olivier Matz <olivier.matz@6wind.com>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>
> Cc: dev@dpdk.org; michel@digirati.com.br; Cody Doucette <doucette@bu.edu>; Qiaobin Fu <qiaobinf@bu.edu>
> Subject: [PATCH v2] ip_frag: extend rte_ipv6_frag_get_ipv6_fragment_header()
> 
> 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>
> ---
> v2:
> * Moved IPv6 extension header definitions to lib_net.
> 

Looks ok to me, though now drivers/net/failsafe fails to build on freebsd:
http://mails.dpdk.org/archives/test-report/2018-July/060532.html

I fixed that locally by commenting out:
CFLAGS += -D_XOPEN_SOURCE=700
at drivers/net/failsafe/Makefile

Not sure what is the root cause here.
Gaetan, do you remember why we need it for failsafe driver?
Konstantin



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

             reply	other threads:[~2018-07-25  9:11 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-25  9:11 Ananyev, Konstantin [this message]
2018-07-25 13:22 ` Gaëtan Rivet
2018-07-26  9:40   ` Ananyev, Konstantin
  -- strict thread matches above, loose matches on Subject: below --
2018-07-19 15:58 Cody Doucette

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2601191342CEEE43887BDE71AB977258DF51D4C3@irsmsx105.ger.corp.intel.com \
    --to=konstantin.ananyev@intel.com \
    --cc=cristian.dumitrescu@intel.com \
    --cc=dev@dpdk.org \
    --cc=doucette@bu.edu \
    --cc=gaetan.rivet@6wind.com \
    --cc=michel@digirati.com.br \
    --cc=olivier.matz@6wind.com \
    --cc=qiaobinf@bu.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).