[dpdk-dev,v6,5/6] app/testpmd: enable TCP/IPv4, VxLAN and GRE GSO
Checks
Commit Message
From: Jiayu Hu <jiayu.hu@intel.com>
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.
GSO support may be toggled on a per-port basis, using the command:
"set port <port_id> gso on|off"
The maximum packet length (including the packet header and payload) for
GSO segments may be set with the command:
"set gso segsz <length>"
Show GSO configuration for a given port with the command:
"show port <port_id> gso"
Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
---
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(-)
Comments
> -----Original Message-----
> From: Kavanagh, Mark B
> Sent: Monday, October 2, 2017 5:46 PM
> To: dev@dpdk.org
> Cc: Hu, Jiayu <jiayu.hu@intel.com>; Tan, Jianfeng <jianfeng.tan@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Yigit,
> Ferruh <ferruh.yigit@intel.com>; thomas@monjalon.net; Kavanagh, Mark B <mark.b.kavanagh@intel.com>
> Subject: [PATCH v6 5/6] app/testpmd: enable TCP/IPv4, VxLAN and GRE GSO
>
> From: Jiayu Hu <jiayu.hu@intel.com>
>
> 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.
>
> GSO support may be toggled on a per-port basis, using the command:
>
> "set port <port_id> gso on|off"
>
> The maximum packet length (including the packet header and payload) for
> GSO segments may be set with the command:
>
> "set gso segsz <length>"
>
> Show GSO configuration for a given port with the command:
>
> "show port <port_id> gso"
>
> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
> ---
> 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(-)
>
> 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"
>
> + "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_type too.
> +
> + "show port (port_id) gso\n"
> + " Show GSO configuration.\n\n"
> +
> "set fwd (%s)\n"
> " Set packet forwarding mode.\n\n"
>
> @@ -3967,6 +3978,170 @@ struct cmd_gro_set_result {
> },
> };
>
> +/* *** 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 = 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 =
> + TOKEN_STRING_INITIALIZER(struct cmd_gso_enable_result,
> + cmd_set, "set");
> +cmdline_parse_token_string_t cmd_gso_enable_port =
> + TOKEN_STRING_INITIALIZER(struct cmd_gso_enable_result,
> + cmd_port, "port");
> +cmdline_parse_token_string_t cmd_gso_enable_keyword =
> + TOKEN_STRING_INITIALIZER(struct cmd_gso_enable_result,
> + cmd_keyword, "gso");
> +cmdline_parse_token_string_t cmd_gso_enable_mode =
> + TOKEN_STRING_INITIALIZER(struct cmd_gso_enable_result,
> + cmd_mode, "on#off");
> +cmdline_parse_token_num_t cmd_gso_enable_pid =
> + TOKEN_NUM_INITIALIZER(struct cmd_gso_enable_result,
> + cmd_pid, UINT8);
> +
> +cmdline_parse_inst_t cmd_gso_enable = {
> + .f = cmd_gso_enable_parsed,
> + .data = NULL,
> + .help_str = "set port <port_id> gso on|off",
> + .tokens = {
> + (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 = parsed_result;
> +
> + if (test_done == 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 == 0) {
As your gso_size includes packet header too, you probably shouldn't allow gso_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 = res->cmd_size;
> + }
> +}
> +
> +cmdline_parse_token_string_t cmd_gso_size_set =
> + TOKEN_STRING_INITIALIZER(struct cmd_gso_size_result,
> + cmd_set, "set");
> +cmdline_parse_token_string_t cmd_gso_size_keyword =
> + TOKEN_STRING_INITIALIZER(struct cmd_gso_size_result,
> + cmd_keyword, "gso");
> +cmdline_parse_token_string_t cmd_gso_size_segsz =
> + TOKEN_STRING_INITIALIZER(struct cmd_gso_size_result,
> + cmd_segsz, "segsz");
> +cmdline_parse_token_num_t cmd_gso_size_size =
> + TOKEN_NUM_INITIALIZER(struct cmd_gso_size_result,
> + cmd_size, UINT16);
> +
> +cmdline_parse_inst_t cmd_gso_size = {
> + .f = cmd_gso_size_parsed,
> + .data = NULL,
> + .help_str = "set gso segsz <length>",
> + .tokens = {
> + (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 = 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 =
> +TOKEN_STRING_INITIALIZER(struct cmd_gso_show_result,
> + cmd_show, "show");
> +cmdline_parse_token_string_t cmd_gso_show_port =
> +TOKEN_STRING_INITIALIZER(struct cmd_gso_show_result,
> + cmd_port, "port");
> +cmdline_parse_token_string_t cmd_gso_show_keyword =
> + TOKEN_STRING_INITIALIZER(struct cmd_gso_show_result,
> + cmd_keyword, "gso");
> +cmdline_parse_token_num_t cmd_gso_show_pid =
> + TOKEN_NUM_INITIALIZER(struct cmd_gso_show_result,
> + cmd_pid, UINT8);
> +
> +cmdline_parse_inst_t cmd_gso_show = {
> + .f = cmd_gso_show_parsed,
> + .data = NULL,
> + .help_str = "show port <port_id> gso",
> + .tokens = {
> + (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 {
> }
> }
>
> +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") == 0) {
> + if (test_done == 0) {
> + printf("before enabling GSO,"
> + " please stop forwarding first\n");
> + return;
> + }
> + gso_ports[port_id].enable = 1;
> + } else if (strcmp(mode, "off") == 0) {
> + if (test_done == 0) {
> + printf("before disabling GSO,"
> + " please stop forwarding first\n");
> + return;
> + }
> + gso_ports[port_id].enable = 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 <rte_string_fns.h>
> #include <rte_flow.h>
> #include <rte_gro.h>
> +#include <rte_gso.h>
> +
> #include "testpmd.h"
>
> #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 |= PKT_TX_TCP_SEG;
> } else if (info->l4_proto == IPPROTO_SCTP) {
> sctp_hdr = (struct sctp_hdr *)((char *)l3_hdr + info->l3_len);
> sctp_hdr->cksum = 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 = 0;
> + int ret;
>
> #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 = txp->tso_segsz;
> info.tunnel_tso_segsz = txp->tunnel_tso_segsz;
> + if (gso_ports[fs->tx_port].enable)
> + info.gso_enable = 1;
>
> for (i = 0; i < nb_rx; i++) {
> if (likely(i < nb_rx - 1))
> @@ -851,13 +863,59 @@ struct simple_gre_hdr {
> }
> }
>
> + if (gso_ports[fs->tx_port].enable == 0)
> + tx_pkts_burst = pkts_burst;
> + else {
> + gso_ctx = &(current_fwd_lcore()->gso_ctx);
> + gso_ctx->gso_size = gso_max_segment_size;
> + for (i = 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 = 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 += ret;
> + if (unlikely(nb_rx - i >= GSO_MAX_PKT_BURST -
> + nb_segments)) {
> + /*
> + * insufficient space in gso_segments,
> + * stop GSO.
> + */
> + for (j = i; j < GSO_MAX_PKT_BURST -
> + nb_segments; j++) {
> + pkts_burst[j]->ol_flags &=
> + (~PKT_TX_TCP_SEG);
> + gso_segments[nb_segments++] =
> + pkts_burst[j];
> + }
> + for (; j < nb_rx; j++)
> + rte_pktmbuf_free(pkts_burst[j]);
> + break;
> + }
> + ret = rte_gso_segment(pkts_burst[i], gso_ctx,
> + &gso_segments[nb_segments],
> + GSO_MAX_PKT_BURST - nb_segments);
> + if (ret >= 1)
> + nb_segments += ret;
> + else if (ret < 0) {
> + /*
> + * insufficient MBUFs or space in
> + * gso_segments, stop GSO.
> + */
> + for (j = i; j < nb_rx; j++) {
> + pkts_burst[j]->ol_flags &=
> + (~PKT_TX_TCP_SEG);
> + gso_segments[nb_segments++] =
> + pkts_burst[j];
> + }
> + break;
> + }
> + }
> + tx_pkts_burst = gso_segments;
> + nb_rx = nb_segments;
> + }
> +
> nb_prep = rte_eth_tx_prepare(fs->tx_port, fs->tx_queue,
> - pkts_burst, nb_rx);
> + tx_pkts_burst, nb_rx);
> if (nb_prep != nb_rx)
> printf("Preparing packet burst to transmit failed: %s\n",
> rte_strerror(rte_errno));
>
> - nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst,
> + nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, tx_pkts_burst,
> nb_prep);
>
> /*
> @@ -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 += 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 += nb_tx;
> @@ -881,9 +939,10 @@ struct simple_gre_hdr {
> if (unlikely(nb_tx < nb_rx)) {
> fs->fwd_dropped += (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 = rte_rdtsc();
> core_cycles = (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);
>
> +struct gso_status gso_ports[RTE_MAX_ETHPORTS];
> +uint16_t gso_max_segment_size = 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 = 0;
>
> memset(port_per_socket,0,RTE_MAX_NUMA_NODES);
>
> @@ -654,6 +658,8 @@ static int eth_event_callback(uint8_t port_id,
>
> init_port_config();
>
> + gso_types = 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 == NULL)
> mbp = mbuf_pool_find(0);
> fwd_lcores[lc_id]->mbp = mbp;
> + /* initialize GSO context */
> + fwd_lcores[lc_id]->gso_ctx.direct_pool = mbp;
> + fwd_lcores[lc_id]->gso_ctx.indirect_pool = mbp;
> + fwd_lcores[lc_id]->gso_ctx.gso_types = gso_types;
> + fwd_lcores[lc_id]->gso_ctx.gso_size = ETHER_MAX_LEN -
> + ETHER_CRC_LEN;
> + fwd_lcores[lc_id]->gso_ctx.ipid_flag = !RTE_GSO_IPID_FIXED;
Just
fwd_lcores[lc_id]->gso_ctx.ipid_flag = 0;
should do here.
Konstantin
>-----Original Message-----
>From: Ananyev, Konstantin
>Sent: Wednesday, October 4, 2017 4:09 PM
>To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; dev@dpdk.org
>Cc: Hu, Jiayu <jiayu.hu@intel.com>; Tan, Jianfeng <jianfeng.tan@intel.com>;
>Yigit, Ferruh <ferruh.yigit@intel.com>; thomas@monjalon.net
>Subject: RE: [PATCH v6 5/6] app/testpmd: enable TCP/IPv4, VxLAN and GRE GSO
>
>
>
>> -----Original Message-----
>> From: Kavanagh, Mark B
>> Sent: Monday, October 2, 2017 5:46 PM
>> To: dev@dpdk.org
>> Cc: Hu, Jiayu <jiayu.hu@intel.com>; Tan, Jianfeng <jianfeng.tan@intel.com>;
>Ananyev, Konstantin <konstantin.ananyev@intel.com>; Yigit,
>> Ferruh <ferruh.yigit@intel.com>; thomas@monjalon.net; Kavanagh, Mark B
><mark.b.kavanagh@intel.com>
>> Subject: [PATCH v6 5/6] app/testpmd: enable TCP/IPv4, VxLAN and GRE GSO
>>
>> From: Jiayu Hu <jiayu.hu@intel.com>
>>
>> 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.
>>
>> GSO support may be toggled on a per-port basis, using the command:
>>
>> "set port <port_id> gso on|off"
>>
>> The maximum packet length (including the packet header and payload) for
>> GSO segments may be set with the command:
>>
>> "set gso segsz <length>"
>>
>> Show GSO configuration for a given port with the command:
>>
>> "show port <port_id> gso"
>>
>> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
>> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>> ---
>> 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(-)
>>
>> 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"
>>
>> + "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_type
>too.
Would you like to see that change implemented in time for the 17.11 release?
>
>> +
>> + "show port (port_id) gso\n"
>> + " Show GSO configuration.\n\n"
>> +
>> "set fwd (%s)\n"
>> " Set packet forwarding mode.\n\n"
>>
>> @@ -3967,6 +3978,170 @@ struct cmd_gro_set_result {
>> },
>> };
>>
>> +/* *** 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 = 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 =
>> + TOKEN_STRING_INITIALIZER(struct cmd_gso_enable_result,
>> + cmd_set, "set");
>> +cmdline_parse_token_string_t cmd_gso_enable_port =
>> + TOKEN_STRING_INITIALIZER(struct cmd_gso_enable_result,
>> + cmd_port, "port");
>> +cmdline_parse_token_string_t cmd_gso_enable_keyword =
>> + TOKEN_STRING_INITIALIZER(struct cmd_gso_enable_result,
>> + cmd_keyword, "gso");
>> +cmdline_parse_token_string_t cmd_gso_enable_mode =
>> + TOKEN_STRING_INITIALIZER(struct cmd_gso_enable_result,
>> + cmd_mode, "on#off");
>> +cmdline_parse_token_num_t cmd_gso_enable_pid =
>> + TOKEN_NUM_INITIALIZER(struct cmd_gso_enable_result,
>> + cmd_pid, UINT8);
>> +
>> +cmdline_parse_inst_t cmd_gso_enable = {
>> + .f = cmd_gso_enable_parsed,
>> + .data = NULL,
>> + .help_str = "set port <port_id> gso on|off",
>> + .tokens = {
>> + (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 = parsed_result;
>> +
>> + if (test_done == 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 == 0) {
>
>As your gso_size includes packet header too, you probably shouldn't allow
>gso_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.
Okay - I'll take a look at this, thanks.
>
>> + printf("gso_size should be larger than 0."
>> + " Please input a legal value\n");
>> + } else
>> + gso_max_segment_size = res->cmd_size;
>> + }
>> +}
>> +
>> +cmdline_parse_token_string_t cmd_gso_size_set =
>> + TOKEN_STRING_INITIALIZER(struct cmd_gso_size_result,
>> + cmd_set, "set");
>> +cmdline_parse_token_string_t cmd_gso_size_keyword =
>> + TOKEN_STRING_INITIALIZER(struct cmd_gso_size_result,
>> + cmd_keyword, "gso");
>> +cmdline_parse_token_string_t cmd_gso_size_segsz =
>> + TOKEN_STRING_INITIALIZER(struct cmd_gso_size_result,
>> + cmd_segsz, "segsz");
>> +cmdline_parse_token_num_t cmd_gso_size_size =
>> + TOKEN_NUM_INITIALIZER(struct cmd_gso_size_result,
>> + cmd_size, UINT16);
>> +
>> +cmdline_parse_inst_t cmd_gso_size = {
>> + .f = cmd_gso_size_parsed,
>> + .data = NULL,
>> + .help_str = "set gso segsz <length>",
>> + .tokens = {
>> + (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 = 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 =
>> +TOKEN_STRING_INITIALIZER(struct cmd_gso_show_result,
>> + cmd_show, "show");
>> +cmdline_parse_token_string_t cmd_gso_show_port =
>> +TOKEN_STRING_INITIALIZER(struct cmd_gso_show_result,
>> + cmd_port, "port");
>> +cmdline_parse_token_string_t cmd_gso_show_keyword =
>> + TOKEN_STRING_INITIALIZER(struct cmd_gso_show_result,
>> + cmd_keyword, "gso");
>> +cmdline_parse_token_num_t cmd_gso_show_pid =
>> + TOKEN_NUM_INITIALIZER(struct cmd_gso_show_result,
>> + cmd_pid, UINT8);
>> +
>> +cmdline_parse_inst_t cmd_gso_show = {
>> + .f = cmd_gso_show_parsed,
>> + .data = NULL,
>> + .help_str = "show port <port_id> gso",
>> + .tokens = {
>> + (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 {
>> }
>> }
>>
>> +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") == 0) {
>> + if (test_done == 0) {
>> + printf("before enabling GSO,"
>> + " please stop forwarding first\n");
>> + return;
>> + }
>> + gso_ports[port_id].enable = 1;
>> + } else if (strcmp(mode, "off") == 0) {
>> + if (test_done == 0) {
>> + printf("before disabling GSO,"
>> + " please stop forwarding first\n");
>> + return;
>> + }
>> + gso_ports[port_id].enable = 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 <rte_string_fns.h>
>> #include <rte_flow.h>
>> #include <rte_gro.h>
>> +#include <rte_gso.h>
>> +
>> #include "testpmd.h"
>>
>> #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 |= PKT_TX_TCP_SEG;
>> } else if (info->l4_proto == IPPROTO_SCTP) {
>> sctp_hdr = (struct sctp_hdr *)((char *)l3_hdr + info->l3_len);
>> sctp_hdr->cksum = 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 = 0;
>> + int ret;
>>
>> #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 = txp->tso_segsz;
>> info.tunnel_tso_segsz = txp->tunnel_tso_segsz;
>> + if (gso_ports[fs->tx_port].enable)
>> + info.gso_enable = 1;
>>
>> for (i = 0; i < nb_rx; i++) {
>> if (likely(i < nb_rx - 1))
>> @@ -851,13 +863,59 @@ struct simple_gre_hdr {
>> }
>> }
>>
>> + if (gso_ports[fs->tx_port].enable == 0)
>> + tx_pkts_burst = pkts_burst;
>> + else {
>> + gso_ctx = &(current_fwd_lcore()->gso_ctx);
>> + gso_ctx->gso_size = gso_max_segment_size;
>> + for (i = 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 = 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 += ret;
Ditto - thanks!
>
>
>
>> + if (unlikely(nb_rx - i >= GSO_MAX_PKT_BURST -
>> + nb_segments)) {
>> + /*
>> + * insufficient space in gso_segments,
>> + * stop GSO.
>> + */
>> + for (j = i; j < GSO_MAX_PKT_BURST -
>> + nb_segments; j++) {
>> + pkts_burst[j]->ol_flags &=
>> + (~PKT_TX_TCP_SEG);
>> + gso_segments[nb_segments++] =
>> + pkts_burst[j];
>> + }
>> + for (; j < nb_rx; j++)
>> + rte_pktmbuf_free(pkts_burst[j]);
>> + break;
>> + }
>> + ret = rte_gso_segment(pkts_burst[i], gso_ctx,
>> + &gso_segments[nb_segments],
>> + GSO_MAX_PKT_BURST - nb_segments);
>> + if (ret >= 1)
>> + nb_segments += ret;
>> + else if (ret < 0) {
>> + /*
>> + * insufficient MBUFs or space in
>> + * gso_segments, stop GSO.
>> + */
>> + for (j = i; j < nb_rx; j++) {
>> + pkts_burst[j]->ol_flags &=
>> + (~PKT_TX_TCP_SEG);
>> + gso_segments[nb_segments++] =
>> + pkts_burst[j];
>> + }
>> + break;
>> + }
>> + }
>> + tx_pkts_burst = gso_segments;
>> + nb_rx = nb_segments;
>> + }
>> +
>> nb_prep = rte_eth_tx_prepare(fs->tx_port, fs->tx_queue,
>> - pkts_burst, nb_rx);
>> + tx_pkts_burst, nb_rx);
>> if (nb_prep != nb_rx)
>> printf("Preparing packet burst to transmit failed: %s\n",
>> rte_strerror(rte_errno));
>>
>> - nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst,
>> + nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, tx_pkts_burst,
>> nb_prep);
>>
>> /*
>> @@ -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 += 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 += nb_tx;
>> @@ -881,9 +939,10 @@ struct simple_gre_hdr {
>> if (unlikely(nb_tx < nb_rx)) {
>> fs->fwd_dropped += (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 = rte_rdtsc();
>> core_cycles = (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);
>>
>> +struct gso_status gso_ports[RTE_MAX_ETHPORTS];
>> +uint16_t gso_max_segment_size = 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 = 0;
>>
>> memset(port_per_socket,0,RTE_MAX_NUMA_NODES);
>>
>> @@ -654,6 +658,8 @@ static int eth_event_callback(uint8_t port_id,
>>
>> init_port_config();
>>
>> + gso_types = 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 == NULL)
>> mbp = mbuf_pool_find(0);
>> fwd_lcores[lc_id]->mbp = mbp;
>> + /* initialize GSO context */
>> + fwd_lcores[lc_id]->gso_ctx.direct_pool = mbp;
>> + fwd_lcores[lc_id]->gso_ctx.indirect_pool = mbp;
>> + fwd_lcores[lc_id]->gso_ctx.gso_types = gso_types;
>> + fwd_lcores[lc_id]->gso_ctx.gso_size = ETHER_MAX_LEN -
>> + ETHER_CRC_LEN;
>> + fwd_lcores[lc_id]->gso_ctx.ipid_flag = !RTE_GSO_IPID_FIXED;
>
>Just
>fwd_lcores[lc_id]->gso_ctx.ipid_flag = 0;
>should do here.
Agreed.
Thanks again,
Mark
>
>Konstantin
> -----Original Message-----
> From: Kavanagh, Mark B
> Sent: Wednesday, October 4, 2017 5:23 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> Cc: Hu, Jiayu <jiayu.hu@intel.com>; Tan, Jianfeng <jianfeng.tan@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; thomas@monjalon.net
> Subject: RE: [PATCH v6 5/6] app/testpmd: enable TCP/IPv4, VxLAN and GRE GSO
>
>
>
> >-----Original Message-----
> >From: Ananyev, Konstantin
> >Sent: Wednesday, October 4, 2017 4:09 PM
> >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; dev@dpdk.org
> >Cc: Hu, Jiayu <jiayu.hu@intel.com>; Tan, Jianfeng <jianfeng.tan@intel.com>;
> >Yigit, Ferruh <ferruh.yigit@intel.com>; thomas@monjalon.net
> >Subject: RE: [PATCH v6 5/6] app/testpmd: enable TCP/IPv4, VxLAN and GRE GSO
> >
> >
> >
> >> -----Original Message-----
> >> From: Kavanagh, Mark B
> >> Sent: Monday, October 2, 2017 5:46 PM
> >> To: dev@dpdk.org
> >> Cc: Hu, Jiayu <jiayu.hu@intel.com>; Tan, Jianfeng <jianfeng.tan@intel.com>;
> >Ananyev, Konstantin <konstantin.ananyev@intel.com>; Yigit,
> >> Ferruh <ferruh.yigit@intel.com>; thomas@monjalon.net; Kavanagh, Mark B
> ><mark.b.kavanagh@intel.com>
> >> Subject: [PATCH v6 5/6] app/testpmd: enable TCP/IPv4, VxLAN and GRE GSO
> >>
> >> From: Jiayu Hu <jiayu.hu@intel.com>
> >>
> >> 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.
> >>
> >> GSO support may be toggled on a per-port basis, using the command:
> >>
> >> "set port <port_id> gso on|off"
> >>
> >> The maximum packet length (including the packet header and payload) for
> >> GSO segments may be set with the command:
> >>
> >> "set gso segsz <length>"
> >>
> >> Show GSO configuration for a given port with the command:
> >>
> >> "show port <port_id> gso"
> >>
> >> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> >> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
> >> ---
> >> 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(-)
> >>
> >> 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"
> >>
> >> + "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_type
> >too.
>
> Would you like to see that change implemented in time for the 17.11 release?
I think that's too late for such change in 17.11.
My thought was about 18.02 here.
Konstantin
>-----Original Message-----
>From: Ananyev, Konstantin
>Sent: Wednesday, October 4, 2017 5:27 PM
>To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; dev@dpdk.org
>Cc: Hu, Jiayu <jiayu.hu@intel.com>; Tan, Jianfeng <jianfeng.tan@intel.com>;
>Yigit, Ferruh <ferruh.yigit@intel.com>; thomas@monjalon.net
>Subject: RE: [PATCH v6 5/6] app/testpmd: enable TCP/IPv4, VxLAN and GRE GSO
>
>
>
>> -----Original Message-----
>> From: Kavanagh, Mark B
>> Sent: Wednesday, October 4, 2017 5:23 PM
>> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
>> Cc: Hu, Jiayu <jiayu.hu@intel.com>; Tan, Jianfeng <jianfeng.tan@intel.com>;
>Yigit, Ferruh <ferruh.yigit@intel.com>; thomas@monjalon.net
>> Subject: RE: [PATCH v6 5/6] app/testpmd: enable TCP/IPv4, VxLAN and GRE GSO
>>
>>
>>
>> >-----Original Message-----
>> >From: Ananyev, Konstantin
>> >Sent: Wednesday, October 4, 2017 4:09 PM
>> >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; dev@dpdk.org
>> >Cc: Hu, Jiayu <jiayu.hu@intel.com>; Tan, Jianfeng <jianfeng.tan@intel.com>;
>> >Yigit, Ferruh <ferruh.yigit@intel.com>; thomas@monjalon.net
>> >Subject: RE: [PATCH v6 5/6] app/testpmd: enable TCP/IPv4, VxLAN and GRE GSO
>> >
>> >
>> >
>> >> -----Original Message-----
>> >> From: Kavanagh, Mark B
>> >> Sent: Monday, October 2, 2017 5:46 PM
>> >> To: dev@dpdk.org
>> >> Cc: Hu, Jiayu <jiayu.hu@intel.com>; Tan, Jianfeng
><jianfeng.tan@intel.com>;
>> >Ananyev, Konstantin <konstantin.ananyev@intel.com>; Yigit,
>> >> Ferruh <ferruh.yigit@intel.com>; thomas@monjalon.net; Kavanagh, Mark B
>> ><mark.b.kavanagh@intel.com>
>> >> Subject: [PATCH v6 5/6] app/testpmd: enable TCP/IPv4, VxLAN and GRE GSO
>> >>
>> >> From: Jiayu Hu <jiayu.hu@intel.com>
>> >>
>> >> 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.
>> >>
>> >> GSO support may be toggled on a per-port basis, using the command:
>> >>
>> >> "set port <port_id> gso on|off"
>> >>
>> >> The maximum packet length (including the packet header and payload) for
>> >> GSO segments may be set with the command:
>> >>
>> >> "set gso segsz <length>"
>> >>
>> >> Show GSO configuration for a given port with the command:
>> >>
>> >> "show port <port_id> gso"
>> >>
>> >> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
>> >> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>> >> ---
>> >> 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(-)
>> >>
>> >> 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"
>> >>
>> >> + "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_type
>> >too.
>>
>> Would you like to see that change implemented in time for the 17.11 release?
>
>I think that's too late for such change in 17.11.
>My thought was about 18.02 here.
>Konstantin
No problem - thanks Konstantin.
@@ -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"
+ "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"
+
+ "show port (port_id) gso\n"
+ " Show GSO configuration.\n\n"
+
"set fwd (%s)\n"
" Set packet forwarding mode.\n\n"
@@ -3967,6 +3978,170 @@ struct cmd_gro_set_result {
},
};
+/* *** 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 = 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 =
+ TOKEN_STRING_INITIALIZER(struct cmd_gso_enable_result,
+ cmd_set, "set");
+cmdline_parse_token_string_t cmd_gso_enable_port =
+ TOKEN_STRING_INITIALIZER(struct cmd_gso_enable_result,
+ cmd_port, "port");
+cmdline_parse_token_string_t cmd_gso_enable_keyword =
+ TOKEN_STRING_INITIALIZER(struct cmd_gso_enable_result,
+ cmd_keyword, "gso");
+cmdline_parse_token_string_t cmd_gso_enable_mode =
+ TOKEN_STRING_INITIALIZER(struct cmd_gso_enable_result,
+ cmd_mode, "on#off");
+cmdline_parse_token_num_t cmd_gso_enable_pid =
+ TOKEN_NUM_INITIALIZER(struct cmd_gso_enable_result,
+ cmd_pid, UINT8);
+
+cmdline_parse_inst_t cmd_gso_enable = {
+ .f = cmd_gso_enable_parsed,
+ .data = NULL,
+ .help_str = "set port <port_id> gso on|off",
+ .tokens = {
+ (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 = parsed_result;
+
+ if (test_done == 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 == 0) {
+ printf("gso_size should be larger than 0."
+ " Please input a legal value\n");
+ } else
+ gso_max_segment_size = res->cmd_size;
+ }
+}
+
+cmdline_parse_token_string_t cmd_gso_size_set =
+ TOKEN_STRING_INITIALIZER(struct cmd_gso_size_result,
+ cmd_set, "set");
+cmdline_parse_token_string_t cmd_gso_size_keyword =
+ TOKEN_STRING_INITIALIZER(struct cmd_gso_size_result,
+ cmd_keyword, "gso");
+cmdline_parse_token_string_t cmd_gso_size_segsz =
+ TOKEN_STRING_INITIALIZER(struct cmd_gso_size_result,
+ cmd_segsz, "segsz");
+cmdline_parse_token_num_t cmd_gso_size_size =
+ TOKEN_NUM_INITIALIZER(struct cmd_gso_size_result,
+ cmd_size, UINT16);
+
+cmdline_parse_inst_t cmd_gso_size = {
+ .f = cmd_gso_size_parsed,
+ .data = NULL,
+ .help_str = "set gso segsz <length>",
+ .tokens = {
+ (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 = 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 =
+TOKEN_STRING_INITIALIZER(struct cmd_gso_show_result,
+ cmd_show, "show");
+cmdline_parse_token_string_t cmd_gso_show_port =
+TOKEN_STRING_INITIALIZER(struct cmd_gso_show_result,
+ cmd_port, "port");
+cmdline_parse_token_string_t cmd_gso_show_keyword =
+ TOKEN_STRING_INITIALIZER(struct cmd_gso_show_result,
+ cmd_keyword, "gso");
+cmdline_parse_token_num_t cmd_gso_show_pid =
+ TOKEN_NUM_INITIALIZER(struct cmd_gso_show_result,
+ cmd_pid, UINT8);
+
+cmdline_parse_inst_t cmd_gso_show = {
+ .f = cmd_gso_show_parsed,
+ .data = NULL,
+ .help_str = "show port <port_id> gso",
+ .tokens = {
+ (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,
@@ -2454,6 +2454,30 @@ struct igb_ring_desc_16_bytes {
}
}
+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") == 0) {
+ if (test_done == 0) {
+ printf("before enabling GSO,"
+ " please stop forwarding first\n");
+ return;
+ }
+ gso_ports[port_id].enable = 1;
+ } else if (strcmp(mode, "off") == 0) {
+ if (test_done == 0) {
+ printf("before disabling GSO,"
+ " please stop forwarding first\n");
+ return;
+ }
+ gso_ports[port_id].enable = 0;
+ }
+}
+
char*
list_pkt_forwarding_modes(void)
{
@@ -70,6 +70,8 @@
#include <rte_string_fns.h>
#include <rte_flow.h>
#include <rte_gro.h>
+#include <rte_gso.h>
+
#include "testpmd.h"
#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 |= PKT_TX_TCP_SEG;
} else if (info->l4_proto == IPPROTO_SCTP) {
sctp_hdr = (struct sctp_hdr *)((char *)l3_hdr + info->l3_len);
sctp_hdr->cksum = 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 = 0;
+ int ret;
#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 = txp->tso_segsz;
info.tunnel_tso_segsz = txp->tunnel_tso_segsz;
+ if (gso_ports[fs->tx_port].enable)
+ info.gso_enable = 1;
for (i = 0; i < nb_rx; i++) {
if (likely(i < nb_rx - 1))
@@ -851,13 +863,59 @@ struct simple_gre_hdr {
}
}
+ if (gso_ports[fs->tx_port].enable == 0)
+ tx_pkts_burst = pkts_burst;
+ else {
+ gso_ctx = &(current_fwd_lcore()->gso_ctx);
+ gso_ctx->gso_size = gso_max_segment_size;
+ for (i = 0; i < nb_rx; i++) {
+ if (unlikely(nb_rx - i >= GSO_MAX_PKT_BURST -
+ nb_segments)) {
+ /*
+ * insufficient space in gso_segments,
+ * stop GSO.
+ */
+ for (j = i; j < GSO_MAX_PKT_BURST -
+ nb_segments; j++) {
+ pkts_burst[j]->ol_flags &=
+ (~PKT_TX_TCP_SEG);
+ gso_segments[nb_segments++] =
+ pkts_burst[j];
+ }
+ for (; j < nb_rx; j++)
+ rte_pktmbuf_free(pkts_burst[j]);
+ break;
+ }
+ ret = rte_gso_segment(pkts_burst[i], gso_ctx,
+ &gso_segments[nb_segments],
+ GSO_MAX_PKT_BURST - nb_segments);
+ if (ret >= 1)
+ nb_segments += ret;
+ else if (ret < 0) {
+ /*
+ * insufficient MBUFs or space in
+ * gso_segments, stop GSO.
+ */
+ for (j = i; j < nb_rx; j++) {
+ pkts_burst[j]->ol_flags &=
+ (~PKT_TX_TCP_SEG);
+ gso_segments[nb_segments++] =
+ pkts_burst[j];
+ }
+ break;
+ }
+ }
+ tx_pkts_burst = gso_segments;
+ nb_rx = nb_segments;
+ }
+
nb_prep = rte_eth_tx_prepare(fs->tx_port, fs->tx_queue,
- pkts_burst, nb_rx);
+ tx_pkts_burst, nb_rx);
if (nb_prep != nb_rx)
printf("Preparing packet burst to transmit failed: %s\n",
rte_strerror(rte_errno));
- nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst,
+ nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, tx_pkts_burst,
nb_prep);
/*
@@ -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 += 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 += nb_tx;
@@ -881,9 +939,10 @@ struct simple_gre_hdr {
if (unlikely(nb_tx < nb_rx)) {
fs->fwd_dropped += (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 = rte_rdtsc();
core_cycles = (end_tsc - start_tsc);
@@ -400,6 +400,9 @@ static int eth_event_callback(uint8_t port_id,
*/
static int all_ports_started(void);
+struct gso_status gso_ports[RTE_MAX_ETHPORTS];
+uint16_t gso_max_segment_size = 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 = 0;
memset(port_per_socket,0,RTE_MAX_NUMA_NODES);
@@ -654,6 +658,8 @@ static int eth_event_callback(uint8_t port_id,
init_port_config();
+ gso_types = 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 == NULL)
mbp = mbuf_pool_find(0);
fwd_lcores[lc_id]->mbp = mbp;
+ /* initialize GSO context */
+ fwd_lcores[lc_id]->gso_ctx.direct_pool = mbp;
+ fwd_lcores[lc_id]->gso_ctx.indirect_pool = mbp;
+ fwd_lcores[lc_id]->gso_ctx.gso_types = gso_types;
+ fwd_lcores[lc_id]->gso_ctx.gso_size = ETHER_MAX_LEN -
+ ETHER_CRC_LEN;
+ fwd_lcores[lc_id]->gso_ctx.ipid_flag = !RTE_GSO_IPID_FIXED;
}
/* Configuration of packet forwarding streams. */
@@ -36,6 +36,7 @@
#include <rte_pci.h>
#include <rte_gro.h>
+#include <rte_gso.h>
#define RTE_PORT_ALL (~(portid_t)0x0)
@@ -205,6 +206,7 @@ struct rte_port {
* CPU id. configuration table.
*/
struct fwd_lcore {
+ struct rte_gso_ctx gso_ctx; /**< GSO context */
struct rte_mempool *mbp; /**< The mbuf pool to use by this core */
streamid_t stream_idx; /**< index of 1st stream in "fwd_streams" */
streamid_t stream_nb; /**< number of streams in "fwd_streams" */
@@ -442,6 +444,13 @@ struct gro_status {
};
extern struct gro_status gro_ports[RTE_MAX_ETHPORTS];
+#define GSO_MAX_PKT_BURST 2048
+struct gso_status {
+ uint8_t enable;
+};
+extern struct gso_status gso_ports[RTE_MAX_ETHPORTS];
+extern uint16_t gso_max_segment_size;
+
static inline unsigned int
lcore_num(void)
{
@@ -642,6 +651,7 @@ void port_rss_hash_key_update(portid_t port_id, char rss_type[],
int rx_queue_id_is_invalid(queueid_t rxq_id);
int tx_queue_id_is_invalid(queueid_t txq_id);
void setup_gro(const char *mode, uint8_t port_id);
+void setup_gso(const char *mode, uint8_t port_id);
/* Functions to manage the set of filtered Multicast MAC addresses */
void mcast_addr_add(uint8_t port_id, struct ether_addr *mc_addr);
@@ -932,6 +932,52 @@ number of packets a GRO table can store.
If current packet number is greater than or equal to the max value, GRO
will stop processing incoming packets.
+set port - gso
+~~~~~~~~~~~~~~
+
+Toggle per-port GSO support in ``csum`` forwarding engine::
+
+ testpmd> set port <port_id> gso on|off
+
+If enabled, the csum forwarding engine will perform GSO on supported IPv4
+packets, transmitted on the given port.
+
+If disabled, packets transmitted on the given port will not undergo GSO.
+By default, GSO is disabled for all ports.
+
+.. note::
+
+ When GSO is enabled on a port, supported IPv4 packets transmitted on that
+ port undergo GSO. Afterwards, the segmented packets are represented by
+ multi-segment mbufs; however, the csum forwarding engine doesn't calculation
+ of checksums for GSO'd segments in SW. As a result, if users want correct
+ checksums in GSO segments, they should enable HW checksum calculation for
+ GSO-enabled ports.
+
+ For example, HW checksum calculation for VxLAN GSO'd packets may be enabled
+ by setting the following options in the csum forwarding engine:
+
+ testpmd> csum set outer_ip hw <port_id>
+
+ testpmd> csum set ip hw <port_id>
+
+ testpmd> csum set tcp hw <port_id>
+
+set gso segsz
+~~~~~~~~~~~~~
+
+Set the maximum GSO segment size (measured in bytes), which includes the
+packet header and the packet payload for GSO-enabled ports (global)::
+
+ testpmd> set gso segsz <length>
+
+show port - gso
+~~~~~~~~~~~~~~~
+
+Display the status of Generic Segmentation Offload for a given port::
+
+ testpmd> show port <port_id> gso
+
mac_addr add
~~~~~~~~~~~~