[dpdk-dev] [PATCH v4 3/5] gso: add VxLAN GSO support

[Hu, Jiayu]

Hi Jianfeng,

> -----Original Message-----
> From: Tan, Jianfeng
> Sent: Wednesday, September 20, 2017 11:11 AM
> To: Hu, Jiayu <jiayu.hu at intel.com>; dev at dpdk.org
> Cc: Ananyev, Konstantin <konstantin.ananyev at intel.com>; Kavanagh, Mark
> B <mark.b.kavanagh at intel.com>; Yigit, Ferruh <ferruh.yigit at intel.com>;
> thomas at monjalon.net
> Subject: Re: [PATCH v4 3/5] gso: add VxLAN GSO support
> 
> 
> On 9/19/2017 3:32 PM, Jiayu Hu wrote:
> > From: Mark Kavanagh <mark.b.kavanagh at intel.com>
> >
> > This patch adds GSO support for VxLAN-encapsulated packets. Supported
> > VxLAN packets must have an outer IPv4 header (prepended by an optional
> > VLAN tag), and contain an inner TCP/IPv4 packet (with an optional inner
> > VLAN tag).
> 
> This patch not only adds support for VxLAN, but also support for tunnel
> framework. Better to mention it in the first place.
> 
> > VxLAN GSO doesn't check if all input packets have correct checksums and
> > doesn't update checksums for output packets. Additionally, it doesn't
> > process IP fragmented packets.
> >
> > As with TCP/IPv4 GSO, VxLAN GSO uses a two-segment MBUF to organize
> each
> > output packet, which mandates support for multi-segment mbufs in the TX
> > functions of the NIC driver. Also, if a packet is GSOed, VxLAN GSO
> > reduces its MBUF refcnt by 1. As a result, when all of its GSOed segments
> > are freed, the packet is freed automatically.
> >
> > VxLAN GSO clears the PKT_TX_TCP_SEG flag for the input packet and GSO
> > segments on the event of success.
> 
> This flag is not cleared here, it's cleared in the gso interface. So
> remove above sentence?

Make sense. I will remove the above sentence.

> 
> >
> > Signed-off-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
> > Signed-off-by: Jiayu Hu <jiayu.hu at intel.com>
> > ---
> >   doc/guides/rel_notes/release_17_11.rst |  3 ++
> >   lib/librte_gso/Makefile                |  1 +
> >   lib/librte_gso/gso_common.c            | 58 +++++++++++++++++++++++
> >   lib/librte_gso/gso_common.h            | 25 ++++++++++
> >   lib/librte_gso/gso_tunnel_tcp4.c       | 87
> ++++++++++++++++++++++++++++++++++
> >   lib/librte_gso/gso_tunnel_tcp4.h       | 76
> +++++++++++++++++++++++++++++
> >   lib/librte_gso/rte_gso.c               | 13 +++--
> >   7 files changed, 260 insertions(+), 3 deletions(-)
> >   create mode 100644 lib/librte_gso/gso_tunnel_tcp4.c
> >   create mode 100644 lib/librte_gso/gso_tunnel_tcp4.h
> >
> > diff --git a/doc/guides/rel_notes/release_17_11.rst
> b/doc/guides/rel_notes/release_17_11.rst
> > index 7453bb0..2dc6b89 100644
> > --- a/doc/guides/rel_notes/release_17_11.rst
> > +++ b/doc/guides/rel_notes/release_17_11.rst
> > @@ -48,6 +48,9 @@ New Features
> >     ones (e.g. MTU is 1500B). Supported packet types are:
> >
> >     * TCP/IPv4 packets, which may include a single VLAN tag.
> > +  * VxLAN packets, which must have an outer IPv4 header (prepended by
> > +    an optional VLAN tag), and contain an inner TCP/IPv4 packet (with
> > +    an optional VLAN tag).
> >
> >     The GSO library doesn't check if the input packets have correct
> >     checksums, and doesn't update checksums for output packets.
> > diff --git a/lib/librte_gso/Makefile b/lib/librte_gso/Makefile
> > index 2be64d1..e6d41df 100644
> > --- a/lib/librte_gso/Makefile
> > +++ b/lib/librte_gso/Makefile
> > @@ -44,6 +44,7 @@ LIBABIVER := 1
> >   SRCS-$(CONFIG_RTE_LIBRTE_GSO) += rte_gso.c
> >   SRCS-$(CONFIG_RTE_LIBRTE_GSO) += gso_common.c
> >   SRCS-$(CONFIG_RTE_LIBRTE_GSO) += gso_tcp4.c
> > +SRCS-$(CONFIG_RTE_LIBRTE_GSO) += gso_tunnel_tcp4.c
> >
> >   # install this header file
> >   SYMLINK-$(CONFIG_RTE_LIBRTE_GSO)-include += rte_gso.h
> > diff --git a/lib/librte_gso/gso_common.c b/lib/librte_gso/gso_common.c
> > index b2c84f6..90fcb2a 100644
> > --- a/lib/librte_gso/gso_common.c
> > +++ b/lib/librte_gso/gso_common.c
> > @@ -39,6 +39,7 @@
> >   #include <rte_ether.h>
> >   #include <rte_ip.h>
> >   #include <rte_tcp.h>
> > +#include <rte_udp.h>
> >
> >   #include "gso_common.h"
> >
> > @@ -200,3 +201,60 @@ update_tcp4_header(struct rte_mbuf *pkt,
> uint8_t ipid_delta,
> >   		sent_seq += (segs[i]->pkt_len - segs[i]->data_len);
> >   	}
> >   }
> > +
> > +static inline void
> > +__update_outer_ipv4_header(struct rte_mbuf *pkt, uint16_t id)
> > +{
> > +	struct ipv4_hdr *ipv4_hdr;
> > +
> > +	ipv4_hdr = (struct ipv4_hdr *)(rte_pktmbuf_mtod(pkt, char *) +
> > +			pkt->outer_l2_len);
> > +	ipv4_hdr->total_length = rte_cpu_to_be_16(pkt->pkt_len -
> > +			pkt->outer_l2_len);
> > +	ipv4_hdr->packet_id = rte_cpu_to_be_16(id);
> > +}
> > +
> > +static inline void
> > +__update_outer_udp_header(struct rte_mbuf *pkt)
> > +{
> > +	struct udp_hdr *udp_hdr;
> > +	uint16_t length;
> > +
> > +	length = pkt->outer_l2_len + pkt->outer_l3_len;
> > +	udp_hdr = (struct udp_hdr *)(rte_pktmbuf_mtod(pkt, char *) +
> > +			length);
> > +	udp_hdr->dgram_len = rte_cpu_to_be_16(pkt->pkt_len - length);
> > +}
> > +
> > +void
> > +update_ipv4_vxlan_tcp4_header(struct rte_mbuf *pkt, uint8_t ipid_delta,
> > +		struct rte_mbuf **segs, uint16_t nb_segs)
> 
> This function is specific to tunnel, better move to gso_tunnel_tcp4.c

Make sense. I will remove GRE header update function too.

> 
> > +{
> > +	struct ipv4_hdr *ipv4_hdr;
> > +	struct tcp_hdr *tcp_hdr;
> > +	uint32_t sent_seq;
> > +	uint16_t l2_len, outer_id, inner_id, tail_idx, i;
> > +
> > +	ipv4_hdr = (struct ipv4_hdr *)(rte_pktmbuf_mtod(pkt, char *) +
> > +			pkt->outer_l2_len);
> > +	outer_id = rte_be_to_cpu_16(ipv4_hdr->packet_id);
> > +
> > +	l2_len = pkt->outer_l2_len + pkt->outer_l3_len + pkt->l2_len;
> > +	ipv4_hdr = (struct ipv4_hdr *)(rte_pktmbuf_mtod(pkt, char *) +
> > +			l2_len);
> > +	inner_id = rte_be_to_cpu_16(ipv4_hdr->packet_id);
> > +	tcp_hdr = (struct tcp_hdr *)((char *)ipv4_hdr + pkt->l3_len);
> > +	sent_seq = rte_be_to_cpu_32(tcp_hdr->sent_seq);
> > +	tail_idx = nb_segs - 1;
> > +
> > +	for (i = 0; i < nb_segs; i++) {
> > +		__update_outer_ipv4_header(segs[i], outer_id);
> > +		outer_id += ipid_delta;
> > +		__update_outer_udp_header(segs[i]);
> > +
> > +		__update_ipv4_tcp_header(segs[i], l2_len, inner_id,
> sent_seq,
> > +				i < tail_idx);
> > +		inner_id += ipid_delta;
> > +		sent_seq += (segs[i]->pkt_len - segs[i]->data_len);
> > +	}
> > +}
> > diff --git a/lib/librte_gso/gso_common.h b/lib/librte_gso/gso_common.h
> > index 2a01cd0..0b0d8ed 100644
> > --- a/lib/librte_gso/gso_common.h
> > +++ b/lib/librte_gso/gso_common.h
> > @@ -48,6 +48,11 @@
> >   #define IS_IPV4_TCP(flag) (((flag) & (PKT_TX_TCP_SEG | PKT_TX_IPV4)) ==
> \
> >   		(PKT_TX_TCP_SEG | PKT_TX_IPV4))
> >
> > +#define IS_IPV4_VXLAN_TCP4(flag) (((flag) & (PKT_TX_TCP_SEG |
> PKT_TX_IPV4 | \
> > +				PKT_TX_OUTER_IPV4 |
> PKT_TX_TUNNEL_VXLAN)) == \
> > +		(PKT_TX_TCP_SEG | PKT_TX_IPV4 | PKT_TX_OUTER_IPV4 | \
> > +		 PKT_TX_TUNNEL_VXLAN))
> > +
> >   /**
> >    * Internal function which updates relevant packet headers for TCP/IPv4
> >    * packets, following segmentation. This is required to update, for
> > @@ -69,6 +74,26 @@ void update_tcp4_header(struct rte_mbuf *pkt,
> >   		uint16_t nb_segs);
> >
> >   /**
> > + * Internal function which updates relevant packet headers for VxLAN
> > + * packets, following segmentation. This is required to update, for
> > + * example, the IPv4 'total_length' field, to reflect the reduced length
> > + * of the now-segmented packet.
> > + *
> > + * @param pkt
> > + *  The original packet.
> > + * @param ipid_delta
> > + *  The increasing uint of IP ids.
> > + * @param segs
> > + *  Pointer array used for storing mbuf addresses for GSO segments.
> > + * @param nb_segs
> > + *  The number of GSO segments placed in segs.
> > + */
> > +void update_ipv4_vxlan_tcp4_header(struct rte_mbuf *pkt,
> > +		uint8_t ipid_delta,
> > +		struct rte_mbuf **segs,
> > +		uint16_t nb_segs);
> > +
> > +/**
> >    * Internal function which divides the input packet into small segments.
> >    * Each of the newly-created segments is organized as a two-segment
> MBUF,
> >    * where the first segment is a standard mbuf, which stores a copy of
> > diff --git a/lib/librte_gso/gso_tunnel_tcp4.c
> b/lib/librte_gso/gso_tunnel_tcp4.c
> > new file mode 100644
> > index 0000000..cc017bd
> > --- /dev/null
> > +++ b/lib/librte_gso/gso_tunnel_tcp4.c
> > @@ -0,0 +1,87 @@
> > +/*-
> > + *   BSD LICENSE
> > + *
> > + *   Copyright(c) 2017 Intel Corporation. All rights reserved.
> > + *   All rights reserved.
> > + *
> > + *   Redistribution and use in source and binary forms, with or without
> > + *   modification, are permitted provided that the following conditions
> > + *   are met:
> > + *
> > + *     * Redistributions of source code must retain the above copyright
> > + *       notice, this list of conditions and the following disclaimer.
> > + *     * Redistributions in binary form must reproduce the above copyright
> > + *       notice, this list of conditions and the following disclaimer in
> > + *       the documentation and/or other materials provided with the
> > + *       distribution.
> > + *     * Neither the name of Intel Corporation nor the names of its
> > + *       contributors may be used to endorse or promote products derived
> > + *       from this software without specific prior written permission.
> > + *
> > + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> > + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
> NOT
> > + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS FOR
> > + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> > + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> > + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
> BUT NOT
> > + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> LOSS OF USE,
> > + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> AND ON ANY
> > + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> TORT
> > + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
> OF THE USE
> > + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> > + */
> > +
> > +#include <rte_ether.h>
> > +#include <rte_ip.h>
> > +
> > +#include "gso_common.h"
> > +#include "gso_tunnel_tcp4.h"
> > +
> > +int
> > +gso_tunnel_tcp4_segment(struct rte_mbuf *pkt,
> > +		uint16_t gso_size,
> > +		uint8_t ipid_delta,
> > +		struct rte_mempool *direct_pool,
> > +		struct rte_mempool *indirect_pool,
> > +		struct rte_mbuf **pkts_out,
> > +		uint16_t nb_pkts_out)
> > +{
> > +	struct ipv4_hdr *inner_ipv4_hdr;
> > +	uint16_t pyld_unit_size, hdr_offset;
> > +	uint16_t tcp_dl, frag_off;
> > +	int ret = 1;
> > +
> > +	hdr_offset = pkt->outer_l2_len + pkt->outer_l3_len + pkt->l2_len;
> > +	inner_ipv4_hdr = (struct ipv4_hdr *)(rte_pktmbuf_mtod(pkt, char *) +
> > +			hdr_offset);
> > +	/*
> > +	 * Don't process the packet whose MF bit and offset in the inner
> > +	 * IPv4 header are non-zero.
> > +	 */
> > +	frag_off = rte_be_to_cpu_16(inner_ipv4_hdr->fragment_offset);
> > +	if (unlikely(IS_FRAGMENTED(frag_off))) {
> > +		pkts_out[0] = pkt;
> > +		return ret;
> 
> Please use "return 1;" for readability.

OK.

> 
> > +	}
> > +
> > +	/* Don't process the packet without data */
> > +	tcp_dl = pkt->pkt_len - pkt->l2_len - pkt->l3_len - pkt->l4_len;
> > +	if (unlikely(tcp_dl == 0)) {
> > +		pkts_out[0] = pkt;
> > +		return ret;
> 
> Ditto.
> 
> > +	}
> > +
> > +	hdr_offset += pkt->l3_len + pkt->l4_len;
> > +	pyld_unit_size = gso_size - hdr_offset;
> > +
> > +	/* Segment the payload */
> > +	ret = gso_do_segment(pkt, hdr_offset, pyld_unit_size, direct_pool,
> > +			indirect_pool, pkts_out, nb_pkts_out);
> > +	if (ret <= 1)
> > +		return ret;
> > +
> > +	if (pkt->ol_flags & PKT_TX_TUNNEL_VXLAN)
> > +		update_ipv4_vxlan_tcp4_header(pkt, ipid_delta, pkts_out,
> ret);
> > +
> > +	return ret;
> > +}
> > diff --git a/lib/librte_gso/gso_tunnel_tcp4.h
> b/lib/librte_gso/gso_tunnel_tcp4.h
> > new file mode 100644
> > index 0000000..a848a2e
> > --- /dev/null
> > +++ b/lib/librte_gso/gso_tunnel_tcp4.h
> > @@ -0,0 +1,76 @@
> > +/*-
> > + *   BSD LICENSE
> > + *
> > + *   Copyright(c) 2017 Intel Corporation. All rights reserved.
> > + *   All rights reserved.
> > + *
> > + *   Redistribution and use in source and binary forms, with or without
> > + *   modification, are permitted provided that the following conditions
> > + *   are met:
> > + *
> > + *     * Redistributions of source code must retain the above copyright
> > + *       notice, this list of conditions and the following disclaimer.
> > + *     * Redistributions in binary form must reproduce the above copyright
> > + *       notice, this list of conditions and the following disclaimer in
> > + *       the documentation and/or other materials provided with the
> > + *       distribution.
> > + *     * Neither the name of Intel Corporation nor the names of its
> > + *       contributors may be used to endorse or promote products derived
> > + *       from this software without specific prior written permission.
> > + *
> > + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> > + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
> NOT
> > + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS FOR
> > + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> > + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> > + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
> BUT NOT
> > + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> LOSS OF USE,
> > + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> AND ON ANY
> > + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> TORT
> > + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
> OF THE USE
> > + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> > + */
> > +
> > +#ifndef _GSO_TUNNEL_TCP4_H_
> > +#define _GSO_TUNNEL_TCP4_H_
> > +
> > +#include <stdint.h>
> > +#include <rte_mbuf.h>
> > +
> > +/**
> > + * Segment an tunneling packet with inner TCP/IPv4 headers. This function
> > + * doesn't check if the input packet has correct checksums, and doesn't
> > + * update checksums for output GSO segments. Furthermore, it doesn't
> > + * process IP fragment packets.
> > + *
> > + * @param pkt
> > + *  The packet mbuf to segment.
> > + * @param gso_size
> > + *  The max length of a GSO segment, measured in bytes.
> > + * @param ipid_delta
> > + *  The increasing uint of IP ids.
> > + * @param direct_pool
> > + *  MBUF pool used for allocating direct buffers for output segments.
> > + * @param indirect_pool
> > + *  MBUF pool used for allocating indirect buffers for output segments.
> > + * @param pkts_out
> > + *  Pointer array used to store the MBUF addresses of output GSO
> > + *  segments, when gso_tunnel_tcp4_segment() successes. If the memory
> > + *  space in pkts_out is insufficient, gso_tcp4_segment() fails and
> 
> "gso_tcp4_segment()" -> "it".

Yes, a typo here. Thanks.

Thanks,
Jiayu

> 
> Thanks,
> Jianfeng