From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Hu, Jiayu" Subject: Re: [PATCH v4 4/5] gso: add GRE GSO support Date: Wed, 20 Sep 2017 06:01:25 +0000 Message-ID: References: <1505184211-36728-1-git-send-email-jiayu.hu@intel.com> <1505806379-71355-1-git-send-email-jiayu.hu@intel.com> <1505806379-71355-5-git-send-email-jiayu.hu@intel.com> <8abc1a5e-9f74-6b83-2cca-99623ad9e95c@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "Ananyev, Konstantin" , "Kavanagh, Mark B" , "Yigit, Ferruh" , "thomas@monjalon.net" To: "Tan, Jianfeng" , "dev@dpdk.org" Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 311CD9FE for ; Wed, 20 Sep 2017 08:01:30 +0200 (CEST) In-Reply-To: <8abc1a5e-9f74-6b83-2cca-99623ad9e95c@intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Jianfeng, > -----Original Message----- > From: Tan, Jianfeng > Sent: Wednesday, September 20, 2017 10:54 AM > To: Hu, Jiayu ; dev@dpdk.org > Cc: Ananyev, Konstantin ; Kavanagh, Mark > B ; Yigit, Ferruh ; > thomas@monjalon.net > Subject: Re: [PATCH v4 4/5] gso: add GRE GSO support >=20 > Hi, >=20 >=20 > On 9/19/2017 3:32 PM, Jiayu Hu wrote: > > From: Mark Kavanagh > > > > This patch adds GSO support for GRE-tunneled packets. Supported GRE > > packets must contain an outer IPv4 header, and inner TCP/IPv4 headers. > > They may also contain a single VLAN tag. GRE 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 VxLAN GSO, GRE GSO uses a two-segment MBUF to organize each > > output packet, which requires multi-segment mbuf support in the TX > > functions of the NIC driver. Also, if a packet is GSOed, GRE GSO reduce= s > > its MBUF refcnt by 1. As a result, when all of its GSOed segments are > > freed, the packet is freed automatically. > > > > GRE GSO clears the PKT_TX_TCP_SEG flag for the input packet and GSO > > segments on the event of success. > > > > Signed-off-by: Mark Kavanagh > > Signed-off-by: Jiayu Hu > > --- > > doc/guides/rel_notes/release_17_11.rst | 3 +++ > > lib/librte_gso/gso_common.c | 31 > +++++++++++++++++++++++++++++++ > > lib/librte_gso/gso_common.h | 25 +++++++++++++++++++++++++ > > lib/librte_gso/gso_tunnel_tcp4.c | 2 ++ > > lib/librte_gso/rte_gso.c | 8 +++++--- > > 5 files changed, 66 insertions(+), 3 deletions(-) > > > > diff --git a/doc/guides/rel_notes/release_17_11.rst > b/doc/guides/rel_notes/release_17_11.rst > > index 2dc6b89..119f662 100644 > > --- a/doc/guides/rel_notes/release_17_11.rst > > +++ b/doc/guides/rel_notes/release_17_11.rst > > @@ -51,6 +51,9 @@ New Features > > * 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). > > + * GRE packets, which must contain an outer IPv4 header (prepended by > > + an optional VLAN tag), and inner TCP/IPv4 headers (with an optiona= l > > + 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/gso_common.c b/lib/librte_gso/gso_common.c > > index 90fcb2a..3629625 100644 > > --- a/lib/librte_gso/gso_common.c > > +++ b/lib/librte_gso/gso_common.c > > @@ -37,6 +37,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -258,3 +259,33 @@ update_ipv4_vxlan_tcp4_header(struct rte_mbuf > *pkt, uint8_t ipid_delta, > > sent_seq +=3D (segs[i]->pkt_len - segs[i]->data_len); > > } > > } > > + > > +void > > +update_ipv4_gre_tcp4_header(struct rte_mbuf *pkt, uint8_t ipid_delta, > > + struct rte_mbuf **segs, uint16_t nb_segs) > > +{ >=20 > This function seems to have too many duplicated code with above > function, can we merge? Yes, they can be merged into one. >=20 > > + 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 =3D (struct ipv4_hdr *)(rte_pktmbuf_mtod(pkt, char *) + > > + pkt->outer_l2_len); > > + outer_id =3D rte_be_to_cpu_16(ipv4_hdr->packet_id); > > + > > + l2_len =3D pkt->outer_l2_len + pkt->outer_l3_len + pkt->l2_len; > > + ipv4_hdr =3D (struct ipv4_hdr *)(rte_pktmbuf_mtod(pkt, char *) + > l2_len); > > + inner_id =3D rte_be_to_cpu_16(ipv4_hdr->packet_id); > > + tcp_hdr =3D (struct tcp_hdr *)((char *)ipv4_hdr + pkt->l3_len); > > + sent_seq =3D rte_be_to_cpu_32(tcp_hdr->sent_seq); > > + tail_idx =3D nb_segs - 1; > > + for (i =3D 0; i < nb_segs; i++) { > > + __update_outer_ipv4_header(segs[i], outer_id); > > + outer_id +=3D ipid_delta; >=20 > We should update both outer and inner IPID? Could you add some spec > reference here? I check the VxLAN RFC, but it doesn't have strict limits on the value of ou= ter IP ID. In the implementation of Linux GSO, the outer IP ID is always incremental, = where SKB_GSO_TCP_FIXEDID is only meaningful to inner IP ID. If no objections, I will take the same design as Linux. Wonder your opinion= s, @Jianfeng @Konstantin @Mark. Thanks, Jiayu >=20 > > + > > + __update_ipv4_tcp_header(segs[i], l2_len, inner_id, > sent_seq, > > + i < tail_idx); > > + inner_id +=3D ipid_delta; > > + sent_seq +=3D (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 0b0d8ed..433e952 100644 > > --- a/lib/librte_gso/gso_common.h > > +++ b/lib/librte_gso/gso_common.h > > @@ -53,6 +53,11 @@ > > (PKT_TX_TCP_SEG | PKT_TX_IPV4 | PKT_TX_OUTER_IPV4 | \ > > PKT_TX_TUNNEL_VXLAN)) > > > > +#define IS_IPV4_GRE_TCP4(flag) (((flag) & (PKT_TX_TCP_SEG | > PKT_TX_IPV4 | \ > > + PKT_TX_OUTER_IPV4 | > PKT_TX_TUNNEL_GRE)) =3D=3D \ > > + (PKT_TX_TCP_SEG | PKT_TX_IPV4 | PKT_TX_OUTER_IPV4 | \ > > + PKT_TX_TUNNEL_GRE)) > > + > > /** > > * Internal function which updates relevant packet headers for TCP/IP= v4 > > * packets, following segmentation. This is required to update, for > > @@ -94,6 +99,26 @@ void update_ipv4_vxlan_tcp4_header(struct > rte_mbuf *pkt, > > uint16_t nb_segs); > > > > /** > > + * Internal function which updates relevant packet headers for GRE > > + * 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. >=20 > uint -> unit? >=20 > > + * @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_gre_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 segmen= ts. > > * 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 > > index cc017bd..5d5930a 100644 > > --- a/lib/librte_gso/gso_tunnel_tcp4.c > > +++ b/lib/librte_gso/gso_tunnel_tcp4.c > > @@ -82,6 +82,8 @@ gso_tunnel_tcp4_segment(struct rte_mbuf *pkt, > > > > if (pkt->ol_flags & PKT_TX_TUNNEL_VXLAN) > > update_ipv4_vxlan_tcp4_header(pkt, ipid_delta, pkts_out, > ret); > > + else if (pkt->ol_flags & PKT_TX_TUNNEL_GRE) > > + update_ipv4_gre_tcp4_header(pkt, ipid_delta, pkts_out, ret); > > > > return ret; > > } > > diff --git a/lib/librte_gso/rte_gso.c b/lib/librte_gso/rte_gso.c > > index d1a723b..5464831 100644 > > --- a/lib/librte_gso/rte_gso.c > > +++ b/lib/librte_gso/rte_gso.c > > @@ -60,8 +60,9 @@ rte_gso_segment(struct rte_mbuf *pkt, > > > > if ((gso_ctx->gso_size >=3D pkt->pkt_len) || (gso_ctx->gso_types & > > (DEV_TX_OFFLOAD_TCP_TSO | > > - DEV_TX_OFFLOAD_VXLAN_TNL_TSO)) !=3D > > - gso_ctx->gso_types) { > > + DEV_TX_OFFLOAD_VXLAN_TNL_TSO | > > + DEV_TX_OFFLOAD_GRE_TNL_TSO)) !=3D > > + gso_ctx->gso_types) { > > pkt->ol_flags &=3D (~PKT_TX_TCP_SEG); > > pkts_out[0] =3D pkt; > > return ret; > > @@ -73,7 +74,8 @@ rte_gso_segment(struct rte_mbuf *pkt, > > ipid_delta =3D (gso_ctx->ipid_flag !=3D RTE_GSO_IPID_FIXED); > > ol_flags =3D pkt->ol_flags; > > > > - if (IS_IPV4_VXLAN_TCP4(pkt->ol_flags)) { > > + if (IS_IPV4_VXLAN_TCP4(pkt->ol_flags) || > > + IS_IPV4_GRE_TCP4(pkt->ol_flags)) { > > pkt->ol_flags &=3D (~PKT_TX_TCP_SEG); > > ret =3D gso_tunnel_tcp4_segment(pkt, gso_size, ipid_delta, > > direct_pool, indirect_pool,