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 52CD829CF for ; Mon, 19 Jun 2017 05:21:29 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga104.jf.intel.com with ESMTP; 18 Jun 2017 20:21:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,359,1493708400"; d="scan'208";a="100971213" Received: from unknown (HELO localhost.localdomain) ([10.239.128.239]) by orsmga002.jf.intel.com with ESMTP; 18 Jun 2017 20:21:27 -0700 Date: Mon, 19 Jun 2017 11:22:38 +0800 From: Jiayu Hu To: "Wu, Jingjing" Cc: "dev@dpdk.org" , "Ananyev, Konstantin" , "yliu@fridaylinux.org" , "Wiles, Keith" , "Tan, Jianfeng" , "Bie, Tiwei" , "Yao, Lei A" Message-ID: <20170619032238.GA40216@localhost.localdomain> References: <1496833731-53653-1-git-send-email-jiayu.hu@intel.com> <1497770469-16661-1-git-send-email-jiayu.hu@intel.com> <1497770469-16661-4-git-send-email-jiayu.hu@intel.com> <9BB6961774997848B5B42BEC655768F810DA943D@SHSMSX103.ccr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9BB6961774997848B5B42BEC655768F810DA943D@SHSMSX103.ccr.corp.intel.com> User-Agent: Mutt/1.7.1 (2016-10-04) Subject: Re: [dpdk-dev] [PATCH v5 3/3] app/testpmd: enable TCP/IPv4 GRO 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: Mon, 19 Jun 2017 03:21:30 -0000 On Mon, Jun 19, 2017 at 10:27:05AM +0800, Wu, Jingjing wrote: > > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jiayu Hu > > Sent: Sunday, June 18, 2017 3:21 PM > > To: dev@dpdk.org > > Cc: Ananyev, Konstantin ; yliu@fridaylinux.org; > > Wiles, Keith ; Tan, Jianfeng ; > > Bie, Tiwei ; Yao, Lei A ; Hu, Jiayu > > > > Subject: [dpdk-dev] [PATCH v5 3/3] app/testpmd: enable TCP/IPv4 GRO > > > > This patch demonstrates the usage of GRO library in testpmd. By default, GRO > > is turned off. Command, "gro on (port_id)", turns on GRO for the given port; > > command, "gro off (port_id)", turns off GRO for the given port. Currently, GRO > > only supports to process TCP/IPv4 packets and works in IO forward mode. > > Besides, only GRO lightweight mode is enabled. > > > > Signed-off-by: Jiayu Hu > > --- > > app/test-pmd/cmdline.c | 45 > > +++++++++++++++++++++++++++++++++++++++++++++ > > app/test-pmd/config.c | 29 +++++++++++++++++++++++++++++ > > app/test-pmd/iofwd.c | 6 ++++++ > > app/test-pmd/testpmd.c | 3 +++ > > app/test-pmd/testpmd.h | 11 +++++++++++ > > 5 files changed, 94 insertions(+) > > > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index > > 105c71f..d1ca8df 100644 > > --- a/app/test-pmd/cmdline.c > > +++ b/app/test-pmd/cmdline.c > > @@ -76,6 +76,7 @@ > > #include > > #include > > #include > > +#include > > > > #include > > #include > > @@ -423,6 +424,9 @@ static void cmd_help_long_parsed(void *parsed_result, > > "tso show (portid)" > > " Display the status of TCP Segmentation > > Offload.\n\n" > > > > + "gro (on|off) (port_id)" > > + " Enable or disable Generic Receive Offload.\n\n" > > + > > "set fwd (%s)\n" > > " Set packet forwarding mode.\n\n" > > > > @@ -3831,6 +3835,46 @@ cmdline_parse_inst_t cmd_tunnel_tso_show = { > > }, > > }; > > > > +/* *** SET GRO FOR A PORT *** */ > > +struct cmd_gro_result { > > + cmdline_fixed_string_t cmd_keyword; > > + cmdline_fixed_string_t mode; > > + uint8_t port_id; > > +}; > > + > > +static void > > +cmd_set_gro_parsed(void *parsed_result, > > + __attribute__((unused)) struct cmdline *cl, > > + __attribute__((unused)) void *data) > > +{ > > + struct cmd_gro_result *res; > > + > > + res = parsed_result; > > + setup_gro(res->mode, res->port_id); > > +} > > + > > +cmdline_parse_token_string_t cmd_gro_keyword = > > + TOKEN_STRING_INITIALIZER(struct cmd_gro_result, > > + cmd_keyword, "gro"); > > +cmdline_parse_token_string_t cmd_gro_mode = > > + TOKEN_STRING_INITIALIZER(struct cmd_gro_result, > > + mode, "on#off"); > > +cmdline_parse_token_num_t cmd_gro_pid = > > + TOKEN_NUM_INITIALIZER(struct cmd_gro_result, > > + port_id, UINT8); > > + > > +cmdline_parse_inst_t cmd_set_gro = { > > + .f = cmd_set_gro_parsed, > > + .data = NULL, > > + .help_str = "gro (on|off) (port_id)", > > + .tokens = { > > + (void *)&cmd_gro_keyword, > > + (void *)&cmd_gro_mode, > > + (void *)&cmd_gro_pid, > > + NULL, > > + }, > > +}; > > + > > /* *** ENABLE/DISABLE FLUSH ON RX STREAMS *** */ struct > > cmd_set_flush_rx { > > cmdline_fixed_string_t set; > > @@ -13710,6 +13754,7 @@ cmdline_parse_ctx_t main_ctx[] = { > > (cmdline_parse_inst_t *)&cmd_tso_show, > > (cmdline_parse_inst_t *)&cmd_tunnel_tso_set, > > (cmdline_parse_inst_t *)&cmd_tunnel_tso_show, > > + (cmdline_parse_inst_t *)&cmd_set_gro, > > (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 3cd4f31..858342d > > 100644 > > --- a/app/test-pmd/config.c > > +++ b/app/test-pmd/config.c > > @@ -71,6 +71,7 @@ > > #ifdef RTE_LIBRTE_BNXT_PMD > > #include > > #endif > > +#include > > > > #include "testpmd.h" > > > > @@ -2414,6 +2415,34 @@ set_tx_pkt_segments(unsigned *seg_lengths, > > unsigned nb_segs) > > tx_pkt_nb_segs = (uint8_t) nb_segs; > > } > > > > +void > > +setup_gro(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 enable GRO," > > + " please stop forwarding first\n"); > > + return; > > + } > > + gro_ports[port_id].enable = 1; > > + gro_ports[port_id].param.max_flow_num = 4; > > + gro_ports[port_id].param.max_item_per_flow = 32; > Is 4 and 32 the default value for GRO? If so, how about to define the MACRO in rte_gro.h The values of param.max_flow_num and param.max_item_per_flow should depend on the specific usage scenarios. In testpmd case, the number of packets returned by rte_eth_rx_burst each time is always less than or equal to 32. So I simply hard-code this value. Besides, I assume the flow number per queue receives is less than 4. So does the value of param.max_flow_num. Maybe I should add commands in testpmd to enable users to set these two values dynamically. How do you think? > > > + gro_ports[port_id].param.desired_gro_types = GRO_TCP_IPV4; > > + gro_ports[port_id].param.max_packet_size = UINT16_MAX; > > + } else if (strcmp(mode, "off") == 0) { > > "else" is enough, no need to compare it with "off". Thanks. I will modify it in next patch. > > > + if (test_done == 0) { > > + printf("before disable GRO," > > + " please stop forwarding first\n"); > > + return; > > + } > > How about to move the test_done before the mode checking? Thanks. I will modify it in next patch. > > > + gro_ports[port_id].enable = 0; > > + } > > +} > > + > > char* > > list_pkt_forwarding_modes(void) > > { > > diff --git a/app/test-pmd/iofwd.c b/app/test-pmd/iofwd.c index > > 15cb4a2..d9ec528 100644 > > --- a/app/test-pmd/iofwd.c > > +++ b/app/test-pmd/iofwd.c > > @@ -65,6 +65,7 @@ > > #include > > #include > > #include > > +#include > > > > #include "testpmd.h" > > > > @@ -99,6 +100,11 @@ pkt_burst_io_forward(struct fwd_stream *fs) > > pkts_burst, nb_pkt_per_burst); > > if (unlikely(nb_rx == 0)) > > return; > > + if (unlikely(gro_ports[fs->rx_port].enable)) { > > + nb_rx = rte_gro_reassemble_burst(pkts_burst, > > + nb_rx, > > + gro_ports[fs->rx_port].param); > > + } > > fs->rx_packets += nb_rx; > > > > #ifdef RTE_TEST_PMD_RECORD_BURST_STATS > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index > > b29328a..ed27c7a 100644 > > --- a/app/test-pmd/testpmd.c > > +++ b/app/test-pmd/testpmd.c > > @@ -90,6 +90,7 @@ > > #ifdef RTE_LIBRTE_LATENCY_STATS > > #include > > #endif > > +#include > > > > #include "testpmd.h" > > > > @@ -378,6 +379,8 @@ lcoreid_t bitrate_lcore_id; uint8_t bitrate_enabled; > > #endif > > > > +struct gro_status gro_ports[RTE_MAX_ETHPORTS]; > > + > > /* Forward function declarations */ > > static void map_port_queue_stats_mapping_registers(uint8_t pi, struct > > rte_port *port); static void check_all_ports_link_status(uint32_t port_mask); > > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index > > 364502d..0471e99 100644 > > --- a/app/test-pmd/testpmd.h > > +++ b/app/test-pmd/testpmd.h > > @@ -34,6 +34,8 @@ > > #ifndef _TESTPMD_H_ > > #define _TESTPMD_H_ > > > > +#include > > + > > #define RTE_PORT_ALL (~(portid_t)0x0) > > > > #define RTE_TEST_RX_DESC_MAX 2048 > > @@ -109,6 +111,8 @@ struct fwd_stream { > > queueid_t tx_queue; /**< TX queue to send forwarded packets */ > > streamid_t peer_addr; /**< index of peer ethernet address of packets > > */ > > > > + uint16_t tbl_idx; /**< TCP IPv4 GRO lookup tale index */ > > + > > unsigned int retry_enabled; > > > > /* "read-write" results */ > > @@ -428,6 +432,12 @@ extern struct ether_addr > > peer_eth_addrs[RTE_MAX_ETHPORTS]; extern uint32_t burst_tx_delay_time; > > /**< Burst tx delay time(us) for mac-retry. */ extern uint32_t > > burst_tx_retry_num; /**< Burst tx retry number for mac-retry. */ > > > > +struct gro_status { > > + struct rte_gro_param param; > > + uint8_t enable; > > +}; > > +extern struct gro_status gro_ports[RTE_MAX_ETHPORTS]; > > + > > static inline unsigned int > > lcore_num(void) > > { > > @@ -626,6 +636,7 @@ void get_2tuple_filter(uint8_t port_id, uint16_t index); > > void get_5tuple_filter(uint8_t port_id, uint16_t index); 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); > > > > /* Functions to manage the set of filtered Multicast MAC addresses */ void > > mcast_addr_add(uint8_t port_id, struct ether_addr *mc_addr); > > -- > > 2.7.4 > > Looks fine to me, just don't forget the doc update "doc/guides/testpmd_app_ug/" due to new command line Thanks for you comments. I will update it. BRs, Jiayu