From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Kavanagh, Mark B" Subject: Re: [PATCH v7 0/6] Support TCP/IPv4, VxLAN, and GRE GSO in DPDK Date: Thu, 5 Oct 2017 14:39:03 +0000 Message-ID: References: <1506962749-106779-1-git-send-email-mark.b.kavanagh@intel.com> <1507201331-228465-1-git-send-email-mark.b.kavanagh@intel.com> <2601191342CEEE43887BDE71AB9772585FAA4B6F@IRSMSX103.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "Hu, Jiayu" , "Tan, Jianfeng" , "Yigit, Ferruh" , "thomas@monjalon.net" To: "Ananyev, Konstantin" , "dev@dpdk.org" Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 874E51B203 for ; Thu, 5 Oct 2017 16:39:09 +0200 (CEST) In-Reply-To: <2601191342CEEE43887BDE71AB9772585FAA4B6F@IRSMSX103.ger.corp.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" >From: Ananyev, Konstantin >Sent: Thursday, October 5, 2017 2:23 PM >To: Kavanagh, Mark B ; dev@dpdk.org >Cc: Hu, Jiayu ; Tan, Jianfeng = ; >Yigit, Ferruh ; thomas@monjalon.net >Subject: RE: [PATCH v7 0/6] Support TCP/IPv4, VxLAN, and GRE GSO in DPDK > >Hi Mark, > >> >> Generic Segmentation Offload (GSO) is a SW technique to split large >> packets into small ones. Akin to TSO, GSO enables applications to >> operate on large packets, thus reducing per-packet processing overhead. >> >> To enable more flexibility to applications, DPDK GSO is implemented >> as a standalone library. Applications explicitly use the GSO library >> to segment packets. This patch adds GSO support to DPDK for specific >> packet types: specifically, TCP/IPv4, VxLAN, and GRE. >> >> The first patch introduces the GSO API framework. The second patch >> adds GSO support for TCP/IPv4 packets (containing an optional VLAN >> tag). The third patch adds GSO support for VxLAN packets that contain >> outer IPv4, and inner TCP/IPv4 headers (plus optional inner and/or >> outer VLAN tags). The fourth patch adds GSO support for GRE packets >> that contain outer IPv4, and inner TCP/IPv4 headers (with optional >> outer VLAN tag). The fifth patch in the series enables TCP/IPv4, VxLAN, >> and GRE GSO in testpmd's checksum forwarding engine. The final patch >> in the series adds GSO documentation to the programmer's guide. >> >> Performance Testing >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> The performance of TCP/IPv4 GSO on a 10Gbps link is demonstrated using >> iperf. Setup for the test is described as follows: >> >> a. Connect 2 x 10Gbps physical ports (P0, P1), which are in the same >> machine, together physically. >> b. Launch testpmd with P0 and a vhost-user port, and use csum >> forwarding engine with "retry". >> c. Select IP and TCP HW checksum calculation for P0; select TCP HW >> checksum calculation for vhost-user port. >> d. Launch a VM with csum and tso offloading enabled. >> e. Run iperf-client on virtio-net port in the VM to send TCP packets. >> With enabling csum and tso, the VM can send large TCP/IPv4 packets >> (mss is up to 64KB). >> f. P1 is assigned to linux kernel and enabled kernel GRO. Run >> iperf-server on P1. >> >> We conduct three iperf tests: >> >> test-1: enable GSO for P0 in testpmd, and set max GSO segment length >> to 1518B. Run two iperf-client in the VM. >> test-2: enable TSO for P0 in testpmd, and set TSO segsz to 1518B. Run >> two iperf-client in the VM. >> test-3: disable GSO and TSO in testpmd. Run two iperf-client in the VM. >> >> Throughput of the above three tests: >> >> test-1: 9.4Gbps >> test-2: 9.5Gbps >> test-3: 3Mbps >> >> Functional Testing >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> Unlike TCP packets, VMs can't send large VxLAN or GRE packets. The max >> length of tunneled packets from VMs is 1514B. So current experiment >> method can't be used to measure VxLAN and GRE GSO performance, but simpl= y >> test the functionality via setting small GSO segment length (e.g. 500B). >> >> VxLAN >> ----- >> To test VxLAN GSO functionality, we use the following setup: >> >> a. Connect 2 x 10Gbps physical ports (P0, P1), which are in the same >> machine, together physically. >> b. Launch testpmd with P0 and a vhost-user port, and use csum forwarding >> engine with "retry". >> c. Testpmd commands: >> - csum parse_tunnel on "P0" >> - csum parse_tunnel on "vhost-user port" >> - csum set outer-ip hw "P0" >> - csum set ip hw "P0" >> - csum set tcp hw "P0" >> - csum set tcp hw "vhost-user port" >> - set port "P0" gso on >> - set gso segsz 500 >> d. Launch a VM with csum and tso offloading enabled. >> e. Create a vxlan port for the virtio-net port in the VM. Run iperf-clie= nt >> on the VxLAN port, so TCP packets are VxLAN encapsulated. However, th= e >> max packet length is 1514B. >> f. P1 is assigned to linux kernel and kernel GRO is disabled. Similarly, >> create a VxLAN port for P1, and run iperf-server on the VxLAN port. >> >> In testpmd, we can see the length of all packets sent from P0 is smaller >> than or equal to 500B. Additionally, the packets arriving in P1 is >> encapsulated and is smaller than or equal to 500B. >> >> GRE >> --- >> The same process may be used to test GRE functionality, with the excepti= on >that >> the tunnel type created for both the guest's virtio-net, and the host's >kernel >> interfaces is GRE: >> `ip tunnel add mode gre remote local ` >> >> As in the VxLAN testcase, the length of packets sent from P0, and receiv= ed >on >> P1, is less than 500B. >> >> Change log >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> v7: >> - add RTE_GSO_SEG_SIZE_MIN macro; use this to validate gso_ctx.gso_segsz= . >> - rename 'ipid_flag' member of gso_ctx to 'flag'. >> - remove mention of VLAN tags in supported packet types. >> - don't clear PKT_TX_TCP_SEG flag if GSO fails. >> - take all packet overhead into account when checking for empty packet. >> - ensure that only enabled GSO types are enacted upon (i.e. no fall-thro= ugh >to >> TCP/IPv4 case from tunneled case). >> - validate user-supplied gso segsz arg against RTE_GSO_SEG_SIZE_MIN in >testpmd. >> - simplify error-checking/handling for GSO failure case in testpmd csum >engine. >> - use 0 instead of !RTE_GSO_IPID_FIXED in testpmd. > >Looks o in general, just few nits below. >Konstantin Thanks Konstantin - I'll address those issues and spin up a new patchset ri= ght now. Responses to comments are inline, as usual. Thanks again, Mark > >1. there are few checkpatch errors regarding indentation. I saw the issues regarding indentation - apologies :( The issue reported with the MAINTAINERS file in patch 6/6 is a bit of a mys= tery to me though - any ideas? >2. [dpdk-dev,v7,2/6] gso: add TCP/IPv4 GSO support > >int > rte_gso_segment(struct rte_mbuf *pkt, >@@ -41,12 +46,53 @@ > struct rte_mbuf **pkts_out, > uint16_t nb_pkts_out) > { >+ struct rte_mempool *direct_pool, *indirect_pool; >+ struct rte_mbuf *pkt_seg; >+ uint64_t ol_flags; >+ uint16_t gso_size; >+ uint8_t ipid_delta; >+ int ret =3D 1; >+ > if (pkt =3D=3D NULL || pkts_out =3D=3D NULL || gso_ctx =3D=3D NULL || > nb_pkts_out < 1) > return -EINVAL; > >- pkt->ol_flags &=3D (~PKT_TX_TCP_SEG); >- pkts_out[0] =3D pkt; >+ if ((gso_ctx->gso_size < RTE_GSO_SEG_SIZE_MIN) || >+ (gso_ctx->gso_size >=3D pkt->pkt_len) || >+ (gso_ctx->gso_types & DEV_TX_OFFLOAD_TCP_TSO) !=3D >+ gso_ctx->gso_types) { >+ pkt->ol_flags &=3D (~PKT_TX_TCP_SEG); >+ pkts_out[0] =3D pkt; >+ return 1; >+ } >First and third are just checks for gso_ctx misconfiguration. >I think we don't need to reset PKT_TX_TCP_SEG bit in ol_flags nt case. Yes, you're right - that error appears to have been introduced while manual= ly resolving a merge conflict while rebasing. >I'd suggest either remove them at all or merge with the invalid parameters >check above: > >if (pkt =3D=3D NULL || pkts_out =3D=3D NULL || gso_ctx =3D=3D NULL || > nb_pkts_out < 1 || > gso_ctx->gso_size < RTE_GSO_SEG_SIZE_MIN || > gso_ctx->gso_types !=3D DEV_TX_OFFLOAD_TCP_TSO) > return -EINVAL; > >if ((gso_ctx->gso_size >=3D pkt->pkt_len) { > pkt->ol_flags &=3D (~PKT_TX_TCP_SEG); > pkts_out[0] =3D pkt; > return 1; >} >.... Agreed. > >3. [dpdk-dev,v7,3/6] gso: add VxLAN GSO support > > >lib/librte_gso/rte_gso.c > >int > rte_gso_segment(struct rte_mbuf *pkt, >@@ -59,7 +60,8 @@ > > ... >+ (gso_ctx->gso_types & (DEV_TX_OFFLOAD_TCP_TSO | >+ DEV_TX_OFFLOAD_VXLAN_TNL_TSO)) !=3D > gso_ctx->gso_types) { > >I think the check should be just: >(gso_ctx->gso_types & (DEV_TX_OFFLOAD_TCP_TSO | DEV_TX_OFFLOAD_VXLAN_TNL_T= SO)) >!=3D 0) > >As we do want to allow only VXLAN segmentation for ctx. Ah yes, good catch. In that case though, wouldn't the statement be: if (pkt =3D=3D NULL || pkts_out =3D=3D NULL || gso_ctx =3D=3D NULL = || nb_pkts_out < 1 || gso_ctx->gso_size < RTE_GSO_SEG_SIZE_MIN || (gso_ctx->gso_types & (DEV_TX_OFFLOAD_TCP_TSO | DEV_TX_OFFLOAD_VXLAN_TNL_TSO) =3D=3D 0)) return -EINVAL; > >4. [dpdk-dev,v7,4/6] gso: add GRE GSO support >Same comment as above. > >> >> v6: >> - rebase to HEAD of master (i5dce9fcA) >> - remove 'l3_offset' parameter from 'update_ipv4_tcp_headers' >> >> v5: >> - add GSO section to the programmer's guide. >> - use MF or (previously 'and') offset to check if a packet is IP >> fragmented. >> - move 'update_header' helper functions to gso_common.h. >> - move txp/ipv4 'update_header' function to gso_tcp4.c. >> - move tunnel 'update_header' function to gso_tunnel_tcp4.c. >> - add offset parameter to 'update_header' functions. >> - combine GRE and VxLAN tunnel header update functions into a single >> function. >> - correct typos and errors in comments/commit messages. >> >> v4: >> - use ol_flags instead of packet_type to decide which segmentation >> function to use. >> - use MF and offset to check if a packet is IP fragmented, instead of >> using DF. >> - remove ETHER_CRC_LEN from gso segment payload length calculation. >> - refactor internal header update and other functions. >> - remove RTE_GSO_IPID_INCREASE. >> - add some of GSO documents. >> - set the default GSO length to 1514 and fill PKT_TX_TCP_SEG for the >> packets sent from GSO-enabled ports in testpmd. >> v3: >> - support all IPv4 header flags, including RTE_PTYPE_(INNER_)L3_IPV4, >> RTE_PTYPE_(INNER_)L3_IPV4_EXT and RTE_PTYPE_(INNER_)L3_IPV4_EXT_ >> UNKNOWN. >> - fill mbuf->packet_type instead of using rte_net_get_ptype() in >> csumonly.c, since rte_net_get_ptype() doesn't support vxlan. >> - store the input packet into pkts_out inside gso_tcp4_segment() and >> gso_tunnel_tcp4_segment() instead of rte_gso_segment(), when no GSO >> is performed. >> - add missing incldues. >> - optimize file names, function names and function description. >> - fix one bug in testpmd. >> v2: >> - merge data segments whose data_len is less than mss into a large data >> segment in gso_do_segment(). >> - use mbuf->packet_type/l2_len/l3_len etc. instead of parsing the packet >> header in rte_gso_segment(). >> - provide IP id macros for applications to select fixed or incremental I= P >> ids. >> >> Jiayu Hu (3): >> gso: add Generic Segmentation Offload API framework >> gso: add TCP/IPv4 GSO support >> app/testpmd: enable TCP/IPv4, VxLAN and GRE GSO >> >> Mark Kavanagh (3): >> gso: add VxLAN GSO support >> gso: add GRE GSO support >> doc: add GSO programmer's guide >> >> MAINTAINERS | 6 + >> app/test-pmd/cmdline.c | 179 ++++++++ >> app/test-pmd/config.c | 24 ++ >> app/test-pmd/csumonly.c | 43 +- >> app/test-pmd/testpmd.c | 13 + >> app/test-pmd/testpmd.h | 10 + >> config/common_base | 5 + >> doc/api/doxy-api-index.md | 1 + >> doc/api/doxy-api.conf | 1 + >> .../generic_segmentation_offload_lib.rst | 256 +++++++++++ >> .../prog_guide/img/gso-output-segment-format.svg | 313 ++++++++++++++ >> doc/guides/prog_guide/img/gso-three-seg-mbuf.svg | 477 >+++++++++++++++++++++ >> doc/guides/prog_guide/index.rst | 1 + >> doc/guides/rel_notes/release_17_11.rst | 17 + >> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 46 ++ >> lib/Makefile | 2 + >> lib/librte_eal/common/include/rte_log.h | 1 + >> lib/librte_gso/Makefile | 52 +++ >> lib/librte_gso/gso_common.c | 153 +++++++ >> lib/librte_gso/gso_common.h | 171 ++++++++ >> lib/librte_gso/gso_tcp4.c | 104 +++++ >> lib/librte_gso/gso_tcp4.h | 74 ++++ >> lib/librte_gso/gso_tunnel_tcp4.c | 126 ++++++ >> lib/librte_gso/gso_tunnel_tcp4.h | 75 ++++ >> lib/librte_gso/rte_gso.c | 111 +++++ >> lib/librte_gso/rte_gso.h | 148 +++++++ >> lib/librte_gso/rte_gso_version.map | 7 + >> mk/rte.app.mk | 1 + >> 28 files changed, 2413 insertions(+), 4 deletions(-) >> create mode 100644 >doc/guides/prog_guide/generic_segmentation_offload_lib.rst >> create mode 100644 doc/guides/prog_guide/img/gso-output-segment-format.= svg >> create mode 100644 doc/guides/prog_guide/img/gso-three-seg-mbuf.svg >> create mode 100644 lib/librte_gso/Makefile >> create mode 100644 lib/librte_gso/gso_common.c >> create mode 100644 lib/librte_gso/gso_common.h >> create mode 100644 lib/librte_gso/gso_tcp4.c >> create mode 100644 lib/librte_gso/gso_tcp4.h >> create mode 100644 lib/librte_gso/gso_tunnel_tcp4.c >> create mode 100644 lib/librte_gso/gso_tunnel_tcp4.h >> create mode 100644 lib/librte_gso/rte_gso.c >> create mode 100644 lib/librte_gso/rte_gso.h >> create mode 100644 lib/librte_gso/rte_gso_version.map >> >> -- >> 1.9.3