From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 7BDE337B2 for ; Mon, 19 Sep 2016 14:09:53 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga104.jf.intel.com with ESMTP; 19 Sep 2016 05:09:52 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,361,1470726000"; d="scan'208";a="1033067436" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by orsmga001.jf.intel.com with ESMTP; 19 Sep 2016 05:09:51 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.196]) by irsmsx110.ger.corp.intel.com ([163.33.3.25]) with mapi id 14.03.0248.002; Mon, 19 Sep 2016 13:09:41 +0100 From: "Ananyev, Konstantin" To: "Tan, Jianfeng" , "dev@dpdk.org" CC: "thomas.monjalon@6wind.com" , "De Lara Guarch, Pablo" , "Wu, Jingjing" , "Zhang, Helin" , "Tao, Zhe" Thread-Topic: [PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling packet Thread-Index: AQHR66jQc3Dgxo+vkkK4jQhFkSeQJ6CBAhuQ Date: Mon, 19 Sep 2016 12:09:41 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772583F0B57BD@irsmsx105.ger.corp.intel.com> References: <1467752375-25984-1-git-send-email-zhe.tao@intel.com> <1470023815-23108-1-git-send-email-jianfeng.tan@intel.com> <1470023815-23108-4-git-send-email-jianfeng.tan@intel.com> In-Reply-To: <1470023815-23108-4-git-send-email-jianfeng.tan@intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling packet X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Sep 2016 12:09:54 -0000 Hi Jainfeng, > -----Original Message----- > From: Tan, Jianfeng > Sent: Monday, August 1, 2016 4:57 AM > To: dev@dpdk.org > Cc: thomas.monjalon@6wind.com; De Lara Guarch, Pablo ; Ananyev, Konstantin > ; Wu, Jingjing ; Zha= ng, Helin ; Tan, Jianfeng > ; Tao, Zhe > Subject: [PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling packet >=20 > Tx offload on tunneling packet now requires applications to correctly set= tunneling type. Without setting it, i40e driver does not parse > tunneling parameters. Besides that, add a check to see if NIC supports TS= O on tunneling packet when executing "csum parse_tunnel on > _port" > after "tso set _size _port" or the other way around. >=20 > Fixes: b51c47536a9e ("app/testpmd: support TSO in checksum forward engine= ") >=20 > Signed-off-by: Zhe Tao > Signed-off-by: Jianfeng Tan > --- > app/test-pmd/cmdline.c | 42 ++++++++++++++++++++++++++++++++++++------ > app/test-pmd/csumonly.c | 37 +++++++++++++++++++++++++++++-------- > 2 files changed, 65 insertions(+), 14 deletions(-) >=20 > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index f90bef= c..561839f 100644 > --- a/app/test-pmd/cmdline.c > +++ b/app/test-pmd/cmdline.c > @@ -3426,6 +3426,26 @@ struct cmd_csum_tunnel_result { }; >=20 > static void > +check_tunnel_tso_support(uint8_t port_id) { > + struct rte_eth_dev_info dev_info; > + > + rte_eth_dev_info_get(port_id, &dev_info); > + if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_VXLAN_TNL_TSO)) > + printf("Warning: TSO enabled but VXLAN TUNNEL TSO not " > + "supported by port %d\n", port_id); > + if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_GRE_TNL_TSO)) > + printf("Warning: TSO enabled but GRE TUNNEL TSO not " > + "supported by port %d\n", port_id); > + if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_IPIP_TNL_TSO)) > + printf("Warning: TSO enabled but IPIP TUNNEL TSO not " > + "supported by port %d\n", port_id); > + if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_GENEVE_TNL_TSO)) > + printf("Warning: TSO enabled but GENEVE TUNNEL TSO not " > + "supported by port %d\n", port_id); } > + > +static void > cmd_csum_tunnel_parsed(void *parsed_result, > __attribute__((unused)) struct cmdline *cl, > __attribute__((unused)) void *data) @@ -3435,10 +3455,13 @@ cmd= _csum_tunnel_parsed(void *parsed_result, > if (port_id_is_invalid(res->port_id, ENABLED_WARN)) > return; >=20 > - if (!strcmp(res->onoff, "on")) > + if (!strcmp(res->onoff, "on")) { > ports[res->port_id].tx_ol_flags |=3D > TESTPMD_TX_OFFLOAD_PARSE_TUNNEL; > - else > + > + if (ports[res->port_id].tso_segsz !=3D 0) > + check_tunnel_tso_support(res->port_id); > + } else > ports[res->port_id].tx_ol_flags &=3D > (~TESTPMD_TX_OFFLOAD_PARSE_TUNNEL); >=20 > @@ -3502,10 +3525,17 @@ cmd_tso_set_parsed(void *parsed_result, >=20 > /* display warnings if configuration is not supported by the NIC */ > rte_eth_dev_info_get(res->port_id, &dev_info); > - if ((ports[res->port_id].tso_segsz !=3D 0) && > - (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_TCP_TSO) =3D=3D 0) { > - printf("Warning: TSO enabled but not " > - "supported by port %d\n", res->port_id); > + if (ports[res->port_id].tso_segsz !=3D 0) { > + if (ports[res->port_id].tx_ol_flags & > + TESTPMD_TX_OFFLOAD_PARSE_TUNNEL) > + check_tunnel_tso_support(res->port_id); > + /* For packets, > + * (1) when tnl parse is disabled; > + * (2) when tnl parse is enabled but not deemed as tnl pkts > + */ > + if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_TCP_TSO)) > + printf("Warning: TSO enabled but not " > + "supported by port %d\n", res->port_id); > } > } >=20 > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index ac4b= d8f..0a1f95d 100644 > --- a/app/test-pmd/csumonly.c > +++ b/app/test-pmd/csumonly.c > @@ -412,12 +412,10 @@ process_inner_cksums(void *l3_hdr, const struct tes= tpmd_offload_info *info, > return ol_flags; > } >=20 > -/* Calculate the checksum of outer header (only vxlan is supported, > - * meaning IP + UDP). The caller already checked that it's a vxlan > - * packet */ > +/* Calculate the checksum of outer header */ > static uint64_t > process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *in= fo, > - uint16_t testpmd_ol_flags) > + uint16_t testpmd_ol_flags, int tso_enabled) > { > struct ipv4_hdr *ipv4_hdr =3D outer_l3_hdr; > struct ipv6_hdr *ipv6_hdr =3D outer_l3_hdr; @@ -438,10 +436,20 @@ proce= ss_outer_cksums(void *outer_l3_hdr, struct > testpmd_offload_info *info, > if (info->outer_l4_proto !=3D IPPROTO_UDP) > return ol_flags; >=20 > - /* outer UDP checksum is always done in software as we have no > - * hardware supporting it today, and no API for it. */ > - > udp_hdr =3D (struct udp_hdr *)((char *)outer_l3_hdr + info->outer_l3_le= n); > + > + /* outer UDP checksum is done in software as we have no hardware > + * supporting it today, and no API for it. In the other side, for > + * UDP tunneling, like VXLAN or Geneve, outer UDP checksum can be > + * set to zero. > + * > + * If a packet will be TSOed into small packets by NIC, we cannot > + * set/calculate a non-zero checksum, because it will be a wrong > + * value after the packet be split into several small packets. > + */ > + if (tso_enabled) > + udp_hdr->dgram_cksum =3D 0; > + > /* do not recalculate udp cksum if it was 0 */ > if (udp_hdr->dgram_cksum !=3D 0) { > udp_hdr->dgram_cksum =3D 0; > @@ -704,18 +712,27 @@ pkt_burst_checksum_forward(struct fwd_stream *fs) > if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_PARSE_TUNNEL) { > if (info.l4_proto =3D=3D IPPROTO_UDP) { > struct udp_hdr *udp_hdr; > + > udp_hdr =3D (struct udp_hdr *)((char *)l3_hdr + > info.l3_len); > parse_vxlan(udp_hdr, &info, m->packet_type); > + if (info.is_tunnel) > + ol_flags |=3D PKT_TX_TUNNEL_VXLAN; > } else if (info.l4_proto =3D=3D IPPROTO_GRE) { > struct simple_gre_hdr *gre_hdr; > + > gre_hdr =3D (struct simple_gre_hdr *) > ((char *)l3_hdr + info.l3_len); > parse_gre(gre_hdr, &info); > + if (info.is_tunnel) > + ol_flags |=3D PKT_TX_TUNNEL_GRE; > } else if (info.l4_proto =3D=3D IPPROTO_IPIP) { > void *encap_ip_hdr; > + > encap_ip_hdr =3D (char *)l3_hdr + info.l3_len; > parse_encap_ip(encap_ip_hdr, &info); > + if (info.is_tunnel) > + ol_flags |=3D PKT_TX_TUNNEL_IPIP; > } > } >=20 > @@ -745,7 +762,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs) > * processed in hardware. */ > if (info.is_tunnel =3D=3D 1) { > ol_flags |=3D process_outer_cksums(outer_l3_hdr, &info, > - testpmd_ol_flags); > + testpmd_ol_flags, ol_flags & PKT_TX_TCP_SEG); > } >=20 > /* step 4: fill the mbuf meta data (flags and header lengths) */ @@ -8= 06,6 +823,10 @@ It was a while since I looked a t it closely, but shouldn't you also update= step 4 below: if (info.is_tunnel =3D=3D 1) { if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_= CKSUM) { m->outer_l2_len =3D info.outer_l2_len; m->outer_l3_len =3D info.outer_l3_len; m->l2_len =3D info.l2_len; m->l3_len =3D info.l3_len; m->l4_len =3D info.l4_len; } else { /* if there is a outer UDP cksum processed in sw and the inner in hw, the outer checksum will be wrong as the payload will be modified by the hardware */ m->l2_len =3D info.outer_l2_len + info.outer_l3_len + info.l2_len; m->l3_len =3D info.l3_len; m->l4_len =3D info.l4_len; } ? In particular shouldn't it be something like: if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) !=3D 0 || ((testmpd_ol_flags & TESTPMD_TX_OFFLOAD_PARSE_TUNNEL) !=3D 0 && info.= tso_segsz !=3D 0)) { .... ? Another thought, might be it is worth to introduce new flag: TESTPMD_TX_OFF= LOAD_TSO_TUNNEL, and new command in cmdline.c, that would set/clear that flag. Instead of trying to make assumptions does=20 user wants tso for tunneled packets based on 2 different things: - enable/disable tso - enable/disable tunneled packets parsing ? Konstantin > pkt_burst_checksum_forward(struct fwd_stream *fs) > { PKT_TX_OUTER_IPV4, PKT_TX_OUTER_IPV4 }, > { PKT_TX_OUTER_IPV6, PKT_TX_OUTER_IPV6 }, > { PKT_TX_TCP_SEG, PKT_TX_TCP_SEG }, > + { PKT_TX_TUNNEL_VXLAN, PKT_TX_TUNNEL_MASK }, > + { PKT_TX_TUNNEL_GRE, PKT_TX_TUNNEL_MASK }, > + { PKT_TX_TUNNEL_IPIP, PKT_TX_TUNNEL_MASK }, > + { PKT_TX_TUNNEL_GENEVE, PKT_TX_TUNNEL_MASK }, > }; > unsigned j; > const char *name; > -- > 2.7.4