From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 1A9471B684 for ; Wed, 4 Oct 2017 17:08:42 +0200 (CEST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Oct 2017 08:08:41 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,477,1500966000"; d="scan'208";a="158782545" Received: from irsmsx107.ger.corp.intel.com ([163.33.3.99]) by fmsmga005.fm.intel.com with ESMTP; 04 Oct 2017 08:08:40 -0700 Received: from irsmsx103.ger.corp.intel.com ([169.254.3.49]) by IRSMSX107.ger.corp.intel.com ([169.254.10.65]) with mapi id 14.03.0319.002; Wed, 4 Oct 2017 16:08:40 +0100 From: "Ananyev, Konstantin" To: "Kavanagh, Mark B" , "dev@dpdk.org" CC: "Hu, Jiayu" , "Tan, Jianfeng" , "Yigit, Ferruh" , "thomas@monjalon.net" Thread-Topic: [PATCH v6 5/6] app/testpmd: enable TCP/IPv4, VxLAN and GRE GSO Thread-Index: AQHTO536UF59lNhul06JGwlW1nU44qLTyxLw Date: Wed, 4 Oct 2017 15:08:38 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772585FAA3E62@IRSMSX103.ger.corp.intel.com> References: <1506636833-25851-1-git-send-email-mark.b.kavanagh@intel.com> <1506962749-106779-1-git-send-email-mark.b.kavanagh@intel.com> <1506962749-106779-6-git-send-email-mark.b.kavanagh@intel.com> In-Reply-To: <1506962749-106779-6-git-send-email-mark.b.kavanagh@intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMzVhNWZlNmUtZTJkOC00ODIyLTg4ZmUtMjc2MGVlNzE0YmVhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6ImVTNlNXTW9oR1RBZUR0MDBWbnJqOEk3enFQYVRNY3Z0NzVCWkUyVmV6VWc9In0= x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v6 5/6] app/testpmd: enable TCP/IPv4, VxLAN and GRE GSO X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 Oct 2017 15:08:43 -0000 > -----Original Message----- > From: Kavanagh, Mark B > Sent: Monday, October 2, 2017 5:46 PM > To: dev@dpdk.org > Cc: Hu, Jiayu ; Tan, Jianfeng ; Ananyev, Konstantin ; Yigit, > Ferruh ; thomas@monjalon.net; Kavanagh, Mark B > Subject: [PATCH v6 5/6] app/testpmd: enable TCP/IPv4, VxLAN and GRE GSO >=20 > From: Jiayu Hu >=20 > This patch adds GSO support to the csum forwarding engine. Oversized > packets transmitted over a GSO-enabled port will undergo segmentation > (with the exception of packet-types unsupported by the GSO library). > GSO support is disabled by default. >=20 > GSO support may be toggled on a per-port basis, using the command: >=20 > "set port gso on|off" >=20 > The maximum packet length (including the packet header and payload) for > GSO segments may be set with the command: >=20 > "set gso segsz " >=20 > Show GSO configuration for a given port with the command: >=20 > "show port gso" >=20 > Signed-off-by: Jiayu Hu > Signed-off-by: Mark Kavanagh > --- > app/test-pmd/cmdline.c | 178 ++++++++++++++++++++++= ++++++ > app/test-pmd/config.c | 24 ++++ > app/test-pmd/csumonly.c | 69 ++++++++++- > app/test-pmd/testpmd.c | 13 ++ > app/test-pmd/testpmd.h | 10 ++ > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 46 +++++++ > 6 files changed, 335 insertions(+), 5 deletions(-) >=20 > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c > index ccdf239..05b0ce8 100644 > --- a/app/test-pmd/cmdline.c > +++ b/app/test-pmd/cmdline.c > @@ -431,6 +431,17 @@ static void cmd_help_long_parsed(void *parsed_result= , > " Set max flow number and max packet number per-flow" > " for GRO.\n\n" >=20 > + "set port (port_id) gso (on|off)" > + " Enable or disable Generic Segmentation Offload in" > + " csum forwarding engine.\n\n" > + > + "set gso segsz (length)\n" > + " Set max packet length for output GSO segments," > + " including packet header and payload.\n\n" Probably a good future improvement would be to allow user to specify gso_t= ype too. > + > + "show port (port_id) gso\n" > + " Show GSO configuration.\n\n" > + > "set fwd (%s)\n" > " Set packet forwarding mode.\n\n" >=20 > @@ -3967,6 +3978,170 @@ struct cmd_gro_set_result { > }, > }; >=20 > +/* *** ENABLE/DISABLE GSO *** */ > +struct cmd_gso_enable_result { > + cmdline_fixed_string_t cmd_set; > + cmdline_fixed_string_t cmd_port; > + cmdline_fixed_string_t cmd_keyword; > + cmdline_fixed_string_t cmd_mode; > + uint8_t cmd_pid; > +}; > + > +static void > +cmd_gso_enable_parsed(void *parsed_result, > + __attribute__((unused)) struct cmdline *cl, > + __attribute__((unused)) void *data) > +{ > + struct cmd_gso_enable_result *res; > + > + res =3D parsed_result; > + if (!strcmp(res->cmd_keyword, "gso")) > + setup_gso(res->cmd_mode, res->cmd_pid); > +} > + > +cmdline_parse_token_string_t cmd_gso_enable_set =3D > + TOKEN_STRING_INITIALIZER(struct cmd_gso_enable_result, > + cmd_set, "set"); > +cmdline_parse_token_string_t cmd_gso_enable_port =3D > + TOKEN_STRING_INITIALIZER(struct cmd_gso_enable_result, > + cmd_port, "port"); > +cmdline_parse_token_string_t cmd_gso_enable_keyword =3D > + TOKEN_STRING_INITIALIZER(struct cmd_gso_enable_result, > + cmd_keyword, "gso"); > +cmdline_parse_token_string_t cmd_gso_enable_mode =3D > + TOKEN_STRING_INITIALIZER(struct cmd_gso_enable_result, > + cmd_mode, "on#off"); > +cmdline_parse_token_num_t cmd_gso_enable_pid =3D > + TOKEN_NUM_INITIALIZER(struct cmd_gso_enable_result, > + cmd_pid, UINT8); > + > +cmdline_parse_inst_t cmd_gso_enable =3D { > + .f =3D cmd_gso_enable_parsed, > + .data =3D NULL, > + .help_str =3D "set port gso on|off", > + .tokens =3D { > + (void *)&cmd_gso_enable_set, > + (void *)&cmd_gso_enable_port, > + (void *)&cmd_gso_enable_pid, > + (void *)&cmd_gso_enable_keyword, > + (void *)&cmd_gso_enable_mode, > + NULL, > + }, > +}; > + > +/* *** SET MAX PACKET LENGTH FOR GSO SEGMENTS *** */ > +struct cmd_gso_size_result { > + cmdline_fixed_string_t cmd_set; > + cmdline_fixed_string_t cmd_keyword; > + cmdline_fixed_string_t cmd_segsz; > + uint16_t cmd_size; > +}; > + > +static void > +cmd_gso_size_parsed(void *parsed_result, > + __attribute__((unused)) struct cmdline *cl, > + __attribute__((unused)) void *data) > +{ > + struct cmd_gso_size_result *res =3D parsed_result; > + > + if (test_done =3D=3D 0) { > + printf("Before setting GSO segsz, please first stop fowarding\n"); > + return; > + } > + > + if (!strcmp(res->cmd_keyword, "gso") && > + !strcmp(res->cmd_segsz, "segsz")) { > + if (res->cmd_size =3D=3D 0) { As your gso_size includes packet header too, you probably shouldn't allow g= so_size less then some minimal value of l2_len + l3_len + l4_len + ... Another alternative change gso_ctx.gso_size to count only payload size. > + printf("gso_size should be larger than 0." > + " Please input a legal value\n"); > + } else > + gso_max_segment_size =3D res->cmd_size; > + } > +} > + > +cmdline_parse_token_string_t cmd_gso_size_set =3D > + TOKEN_STRING_INITIALIZER(struct cmd_gso_size_result, > + cmd_set, "set"); > +cmdline_parse_token_string_t cmd_gso_size_keyword =3D > + TOKEN_STRING_INITIALIZER(struct cmd_gso_size_result, > + cmd_keyword, "gso"); > +cmdline_parse_token_string_t cmd_gso_size_segsz =3D > + TOKEN_STRING_INITIALIZER(struct cmd_gso_size_result, > + cmd_segsz, "segsz"); > +cmdline_parse_token_num_t cmd_gso_size_size =3D > + TOKEN_NUM_INITIALIZER(struct cmd_gso_size_result, > + cmd_size, UINT16); > + > +cmdline_parse_inst_t cmd_gso_size =3D { > + .f =3D cmd_gso_size_parsed, > + .data =3D NULL, > + .help_str =3D "set gso segsz ", > + .tokens =3D { > + (void *)&cmd_gso_size_set, > + (void *)&cmd_gso_size_keyword, > + (void *)&cmd_gso_size_segsz, > + (void *)&cmd_gso_size_size, > + NULL, > + }, > +}; > + > +/* *** SHOW GSO CONFIGURATION *** */ > +struct cmd_gso_show_result { > + cmdline_fixed_string_t cmd_show; > + cmdline_fixed_string_t cmd_port; > + cmdline_fixed_string_t cmd_keyword; > + uint8_t cmd_pid; > +}; > + > +static void > +cmd_gso_show_parsed(void *parsed_result, > + __attribute__((unused)) struct cmdline *cl, > + __attribute__((unused)) void *data) > +{ > + struct cmd_gso_show_result *res =3D parsed_result; > + > + if (!rte_eth_dev_is_valid_port(res->cmd_pid)) { > + printf("invalid port id %u\n", res->cmd_pid); > + return; > + } > + if (!strcmp(res->cmd_keyword, "gso")) { > + if (gso_ports[res->cmd_pid].enable) { > + printf("Max GSO'd packet size: %uB\n" > + "Supported GSO types: TCP/IPv4, " > + "VxLAN with inner TCP/IPv4 packet, " > + "GRE with inner TCP/IPv4 packet\n", > + gso_max_segment_size); > + } else > + printf("GSO is not enabled on Port %u\n", res->cmd_pid); > + } > +} > + > +cmdline_parse_token_string_t cmd_gso_show_show =3D > +TOKEN_STRING_INITIALIZER(struct cmd_gso_show_result, > + cmd_show, "show"); > +cmdline_parse_token_string_t cmd_gso_show_port =3D > +TOKEN_STRING_INITIALIZER(struct cmd_gso_show_result, > + cmd_port, "port"); > +cmdline_parse_token_string_t cmd_gso_show_keyword =3D > + TOKEN_STRING_INITIALIZER(struct cmd_gso_show_result, > + cmd_keyword, "gso"); > +cmdline_parse_token_num_t cmd_gso_show_pid =3D > + TOKEN_NUM_INITIALIZER(struct cmd_gso_show_result, > + cmd_pid, UINT8); > + > +cmdline_parse_inst_t cmd_gso_show =3D { > + .f =3D cmd_gso_show_parsed, > + .data =3D NULL, > + .help_str =3D "show port gso", > + .tokens =3D { > + (void *)&cmd_gso_show_show, > + (void *)&cmd_gso_show_port, > + (void *)&cmd_gso_show_pid, > + (void *)&cmd_gso_show_keyword, > + NULL, > + }, > +}; > + > /* *** ENABLE/DISABLE FLUSH ON RX STREAMS *** */ > struct cmd_set_flush_rx { > cmdline_fixed_string_t set; > @@ -14255,6 +14430,9 @@ struct cmd_cmdfile_result { > (cmdline_parse_inst_t *)&cmd_tunnel_tso_show, > (cmdline_parse_inst_t *)&cmd_enable_gro, > (cmdline_parse_inst_t *)&cmd_gro_set, > + (cmdline_parse_inst_t *)&cmd_gso_enable, > + (cmdline_parse_inst_t *)&cmd_gso_size, > + (cmdline_parse_inst_t *)&cmd_gso_show, > (cmdline_parse_inst_t *)&cmd_link_flow_control_set, > (cmdline_parse_inst_t *)&cmd_link_flow_control_set_rx, > (cmdline_parse_inst_t *)&cmd_link_flow_control_set_tx, > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c > index 3ae3e1c..88d09d0 100644 > --- a/app/test-pmd/config.c > +++ b/app/test-pmd/config.c > @@ -2454,6 +2454,30 @@ struct igb_ring_desc_16_bytes { > } > } >=20 > +void > +setup_gso(const char *mode, uint8_t port_id) > +{ > + if (!rte_eth_dev_is_valid_port(port_id)) { > + printf("invalid port id %u\n", port_id); > + return; > + } > + if (strcmp(mode, "on") =3D=3D 0) { > + if (test_done =3D=3D 0) { > + printf("before enabling GSO," > + " please stop forwarding first\n"); > + return; > + } > + gso_ports[port_id].enable =3D 1; > + } else if (strcmp(mode, "off") =3D=3D 0) { > + if (test_done =3D=3D 0) { > + printf("before disabling GSO," > + " please stop forwarding first\n"); > + return; > + } > + gso_ports[port_id].enable =3D 0; > + } > +} > + > char* > list_pkt_forwarding_modes(void) > { > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c > index 90c8119..bd1a287 100644 > --- a/app/test-pmd/csumonly.c > +++ b/app/test-pmd/csumonly.c > @@ -70,6 +70,8 @@ > #include > #include > #include > +#include > + > #include "testpmd.h" >=20 > #define IP_DEFTTL 64 /* from RFC 1340. */ > @@ -91,6 +93,7 @@ > /* structure that caches offload info for the current packet */ > struct testpmd_offload_info { > uint16_t ethertype; > + uint8_t gso_enable; > uint16_t l2_len; > uint16_t l3_len; > uint16_t l4_len; > @@ -381,6 +384,8 @@ struct simple_gre_hdr { > get_udptcp_checksum(l3_hdr, tcp_hdr, > info->ethertype); > } > + if (info->gso_enable) > + ol_flags |=3D PKT_TX_TCP_SEG; > } else if (info->l4_proto =3D=3D IPPROTO_SCTP) { > sctp_hdr =3D (struct sctp_hdr *)((char *)l3_hdr + info->l3_len); > sctp_hdr->cksum =3D 0; > @@ -627,6 +632,9 @@ struct simple_gre_hdr { > pkt_burst_checksum_forward(struct fwd_stream *fs) > { > struct rte_mbuf *pkts_burst[MAX_PKT_BURST]; > + struct rte_mbuf *gso_segments[GSO_MAX_PKT_BURST]; > + struct rte_gso_ctx *gso_ctx; > + struct rte_mbuf **tx_pkts_burst; > struct rte_port *txp; > struct rte_mbuf *m, *p; > struct ether_hdr *eth_hdr; > @@ -634,13 +642,15 @@ struct simple_gre_hdr { > uint16_t nb_rx; > uint16_t nb_tx; > uint16_t nb_prep; > - uint16_t i; > + uint16_t i, j; > uint64_t rx_ol_flags, tx_ol_flags; > uint16_t testpmd_ol_flags; > uint32_t retry; > uint32_t rx_bad_ip_csum; > uint32_t rx_bad_l4_csum; > struct testpmd_offload_info info; > + uint16_t nb_segments =3D 0; > + int ret; >=20 > #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES > uint64_t start_tsc; > @@ -674,6 +684,8 @@ struct simple_gre_hdr { > memset(&info, 0, sizeof(info)); > info.tso_segsz =3D txp->tso_segsz; > info.tunnel_tso_segsz =3D txp->tunnel_tso_segsz; > + if (gso_ports[fs->tx_port].enable) > + info.gso_enable =3D 1; >=20 > for (i =3D 0; i < nb_rx; i++) { > if (likely(i < nb_rx - 1)) > @@ -851,13 +863,59 @@ struct simple_gre_hdr { > } > } >=20 > + if (gso_ports[fs->tx_port].enable =3D=3D 0) > + tx_pkts_burst =3D pkts_burst; > + else { > + gso_ctx =3D &(current_fwd_lcore()->gso_ctx); > + gso_ctx->gso_size =3D gso_max_segment_size; > + for (i =3D 0; i < nb_rx; i++) { It seems quite a lot of code to handle an error case, which I suppose will happen pretty rare. Why not just: ret =3D rte_gso_segment(pkts_burst[i], gso_ctx, &gso_segments[nb_segments], RTE_DIM(gso_segments) - nb_segments); If (ret < 0) { RTE_LOG(DEBUG, ....); rte_free(pkts_burst[i]); } else nb_segments +=3D ret; > + if (unlikely(nb_rx - i >=3D GSO_MAX_PKT_BURST - > + nb_segments)) { > + /* > + * insufficient space in gso_segments, > + * stop GSO. > + */ > + for (j =3D i; j < GSO_MAX_PKT_BURST - > + nb_segments; j++) { > + pkts_burst[j]->ol_flags &=3D > + (~PKT_TX_TCP_SEG); > + gso_segments[nb_segments++] =3D > + pkts_burst[j]; > + } > + for (; j < nb_rx; j++) > + rte_pktmbuf_free(pkts_burst[j]); > + break; > + } > + ret =3D rte_gso_segment(pkts_burst[i], gso_ctx, > + &gso_segments[nb_segments], > + GSO_MAX_PKT_BURST - nb_segments); > + if (ret >=3D 1) > + nb_segments +=3D ret; > + else if (ret < 0) { > + /* > + * insufficient MBUFs or space in > + * gso_segments, stop GSO. > + */ > + for (j =3D i; j < nb_rx; j++) { > + pkts_burst[j]->ol_flags &=3D > + (~PKT_TX_TCP_SEG); > + gso_segments[nb_segments++] =3D > + pkts_burst[j]; > + } > + break; > + } > + } > + tx_pkts_burst =3D gso_segments; > + nb_rx =3D nb_segments; > + } > + > nb_prep =3D rte_eth_tx_prepare(fs->tx_port, fs->tx_queue, > - pkts_burst, nb_rx); > + tx_pkts_burst, nb_rx); > if (nb_prep !=3D nb_rx) > printf("Preparing packet burst to transmit failed: %s\n", > rte_strerror(rte_errno)); >=20 > - nb_tx =3D rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, > + nb_tx =3D rte_eth_tx_burst(fs->tx_port, fs->tx_queue, tx_pkts_burst, > nb_prep); >=20 > /* > @@ -868,7 +926,7 @@ struct simple_gre_hdr { > while (nb_tx < nb_rx && retry++ < burst_tx_retry_num) { > rte_delay_us(burst_tx_delay_time); > nb_tx +=3D rte_eth_tx_burst(fs->tx_port, fs->tx_queue, > - &pkts_burst[nb_tx], nb_rx - nb_tx); > + &tx_pkts_burst[nb_tx], nb_rx - nb_tx); > } > } > fs->tx_packets +=3D nb_tx; > @@ -881,9 +939,10 @@ struct simple_gre_hdr { > if (unlikely(nb_tx < nb_rx)) { > fs->fwd_dropped +=3D (nb_rx - nb_tx); > do { > - rte_pktmbuf_free(pkts_burst[nb_tx]); > + rte_pktmbuf_free(tx_pkts_burst[nb_tx]); > } while (++nb_tx < nb_rx); > } > + > #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES > end_tsc =3D rte_rdtsc(); > core_cycles =3D (end_tsc - start_tsc); > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c > index e097ee0..97e349d 100644 > --- a/app/test-pmd/testpmd.c > +++ b/app/test-pmd/testpmd.c > @@ -400,6 +400,9 @@ static int eth_event_callback(uint8_t port_id, > */ > static int all_ports_started(void); >=20 > +struct gso_status gso_ports[RTE_MAX_ETHPORTS]; > +uint16_t gso_max_segment_size =3D ETHER_MAX_LEN - ETHER_CRC_LEN; > + > /* > * Helper function to check if socket is already discovered. > * If yes, return positive value. If not, return zero. > @@ -570,6 +573,7 @@ static int eth_event_callback(uint8_t port_id, > unsigned int nb_mbuf_per_pool; > lcoreid_t lc_id; > uint8_t port_per_socket[RTE_MAX_NUMA_NODES]; > + uint32_t gso_types =3D 0; >=20 > memset(port_per_socket,0,RTE_MAX_NUMA_NODES); >=20 > @@ -654,6 +658,8 @@ static int eth_event_callback(uint8_t port_id, >=20 > init_port_config(); >=20 > + gso_types =3D DEV_TX_OFFLOAD_TCP_TSO | DEV_TX_OFFLOAD_VXLAN_TNL_TSO | > + DEV_TX_OFFLOAD_GRE_TNL_TSO; > /* > * Records which Mbuf pool to use by each logical core, if needed. > */ > @@ -664,6 +670,13 @@ static int eth_event_callback(uint8_t port_id, > if (mbp =3D=3D NULL) > mbp =3D mbuf_pool_find(0); > fwd_lcores[lc_id]->mbp =3D mbp; > + /* initialize GSO context */ > + fwd_lcores[lc_id]->gso_ctx.direct_pool =3D mbp; > + fwd_lcores[lc_id]->gso_ctx.indirect_pool =3D mbp; > + fwd_lcores[lc_id]->gso_ctx.gso_types =3D gso_types; > + fwd_lcores[lc_id]->gso_ctx.gso_size =3D ETHER_MAX_LEN - > + ETHER_CRC_LEN; > + fwd_lcores[lc_id]->gso_ctx.ipid_flag =3D !RTE_GSO_IPID_FIXED; Just fwd_lcores[lc_id]->gso_ctx.ipid_flag =3D 0; should do here. Konstantin