DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Hu, Jiayu" <jiayu.hu@intel.com>
To: Yuanhan Liu <yliu@fridaylinux.org>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	"stephen@networkplumber.org" <stephen@networkplumber.org>,
	"Tan, Jianfeng" <jianfeng.tan@intel.com>,
	"Wu, Jingjing" <jingjing.wu@intel.com>,
	"Yao, Lei A" <lei.a.yao@intel.com>,
	"Bie, Tiwei" <tiwei.bie@intel.com>
Subject: Re: [dpdk-dev] [PATCH v10 1/3] lib: add Generic Receive Offload API framework
Date: Tue, 4 Jul 2017 16:01:20 +0000	[thread overview]
Message-ID: <ED946F0BEFE0A141B63BABBD629A2A9B3876F9DD@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <20170704083714.GK11626@yliu-home>

Hi Yuanhan,

> -----Original Message-----
> From: Yuanhan Liu [mailto:yliu@fridaylinux.org]
> Sent: Tuesday, July 4, 2017 4:37 PM
> To: Hu, Jiayu <jiayu.hu@intel.com>
> Cc: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> stephen@networkplumber.org; Tan, Jianfeng <jianfeng.tan@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Yao, Lei A <lei.a.yao@intel.com>; Bie,
> Tiwei <tiwei.bie@intel.com>
> Subject: Re: [PATCH v10 1/3] lib: add Generic Receive Offload API framework
> 
> Haven't looked at the details yet, and below are some quick comments
> after a glimpse.
> 
> On Sat, Jul 01, 2017 at 07:08:41PM +0800, Jiayu Hu wrote:
> ...
> > +void *rte_gro_tbl_create(const
> > +		const struct rte_gro_param *param)
> 
> The DPDK style is:
> 
> void *
> rte_gro_tbl_destroy(...)
> 
> Also you should revisit all other functions, as I have seen quite many
> coding style issues like this.

Thanks, I will fix the style issues.

> 
> > +{
> > +	gro_tbl_create_fn create_tbl_fn;
> > +	gro_tbl_destroy_fn destroy_tbl_fn;
> > +	struct gro_tbl *gro_tbl;
> > +	uint64_t gro_type_flag = 0;
> > +	uint8_t i, j;
> > +
> > +	gro_tbl = rte_zmalloc_socket(__func__,
> > +			sizeof(struct gro_tbl),
> > +			RTE_CACHE_LINE_SIZE,
> > +			param->socket_id);
> > +	if (gro_tbl == NULL)
> > +		return NULL;
> > +	gro_tbl->max_packet_size = param->max_packet_size;
> > +	gro_tbl->max_timeout_cycles = param->max_timeout_cycles;
> > +	gro_tbl->desired_gro_types = param->desired_gro_types;
> > +
> > +	for (i = 0; i < RTE_GRO_TYPE_MAX_NUM; i++) {
> > +		gro_type_flag = 1 << i;
> > +
> > +		if ((param->desired_gro_types & gro_type_flag) == 0)
> > +			continue;
> > +		create_tbl_fn = tbl_create_functions[i];
> > +		if (create_tbl_fn == NULL)
> > +			continue;
> > +
> > +		gro_tbl->tbls[i] = create_tbl_fn(
> > +				param->socket_id,
> > +				param->max_flow_num,
> > +				param->max_item_per_flow);
> > +		if (gro_tbl->tbls[i] == NULL) {
> > +			/* destroy all allocated tables */
> > +			for (j = 0; j < i; j++) {
> > +				gro_type_flag = 1 << j;
> > +				if ((param->desired_gro_types &
> gro_type_flag) == 0)
> > +					continue;
> > +				destroy_tbl_fn = tbl_destroy_functions[j];
> > +				if (destroy_tbl_fn)
> > +					destroy_tbl_fn(gro_tbl->tbls[j]);
> > +			}
> > +			rte_free(gro_tbl);
> > +			return NULL;
> 
> The typical way to handle this is to re-use rte_gro_tbl_destroy() as
> much as possible. This saves duplicate code.

Thanks, I will change it.

> 
> > +		}
> > +	}
> > +	return gro_tbl;
> > +}
> > +
> > +void rte_gro_tbl_destroy(void *tbl)
> > +{
> > +	gro_tbl_destroy_fn destroy_tbl_fn;
> > +	struct gro_tbl *gro_tbl = (struct gro_tbl *)tbl;
> 
> The cast (from void *) is unnecessary and can be dropped.

Thanks, I will remove them.

> 
> ...
> > +/**
> > + * the max packets number that rte_gro_reassemble_burst can
> > + * process in each invocation.
> > + */
> > +#define RTE_GRO_MAX_BURST_ITEM_NUM 128UL
> > +
> > +/* max number of supported GRO types */
> > +#define RTE_GRO_TYPE_MAX_NUM 64
> > +#define RTE_GRO_TYPE_SUPPORT_NUM 0	/**< current supported
> GRO num */
> 
> The reason we need use comment style of "/**< ... */" is because this
> is what the doc generator (doxygen) recognizes. If not doing this, your
> comment won't be displayed at the generated doc page (for example,
> http://dpdk.org/doc/api/rte__ethdev_8h.html#ade7de72f6c0f8102d01a0b3
> 438856900).
> 
> The format, as far as I known, could be:
> 
>     /**< here is a comment */
>     #define A_MACRO		x
> 
> Or the one you did for RTE_GRO_TYPE_SUPPORT_NUM: put it at the end
> of the line.
> 
> That being said, the comments for RTE_GRO_MAX_BURST_ITEM_NUM and
> RTE_GRO_TYPE_MAX_NUM should be changed. Again, you should revisit
> other places.

Thanks, I will modify the comments style.

> 
> > +
> > +
> > +struct rte_gro_param {
> > +	uint64_t desired_gro_types;	/**< desired GRO types */
> > +	uint32_t max_packet_size;	/**< max length of merged packets
> */
> > +	uint16_t max_flow_num;	/**< max flow number */
> > +	uint16_t max_item_per_flow;	/**< max packet number per flow
> */
> > +
> > +	/* socket index where the Ethernet port connects to */
> 
> Ditto.
> 
> ...
> > +++ b/lib/librte_gro/rte_gro_version.map
> > @@ -0,0 +1,12 @@
> > +DPDK_17.08 {
> > +	global:
> > +
> > +	rte_gro_tbl_create;
> > +	rte_gro_tbl_destroy;
> > +	rte_gro_reassemble_burst;
> > +	rte_gro_reassemble;
> > +	rte_gro_timeout_flush;
> > +	rte_gro_tbl_item_num;
> 
> The undocumented habit is to list them in alpha order.

Thanks, I will change the order.

BRs,
Jiayu
> 
> 	--yliu

  reply	other threads:[~2017-07-04 16:01 UTC|newest]

Thread overview: 141+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-22  9:32 [dpdk-dev] [PATCH 0/2] lib: add TCP IPv4 GRO support Jiayu Hu
2017-03-22  9:32 ` [dpdk-dev] [PATCH 1/2] lib: add Generic Receive Offload support for TCP IPv4 packets Jiayu Hu
2017-03-22  9:32 ` [dpdk-dev] [PATCH 2/2] app/testpmd: provide TCP IPv4 GRO function in iofwd mode Jiayu Hu
     [not found] ` <1B893F1B-4DA8-4F88-9583-8C0BAA570832@intel.com>
     [not found]   ` <20170323021502.GA114662@localhost.localdomain>
     [not found]     ` <C830A6FC-F440-4E68-AB4E-2FD502722E3F@intel.com>
     [not found]       ` <20170323062433.GA120139@localhost.localdomain>
     [not found]         ` <59AF69C657FD0841A61C55336867B5B066729E3F@IRSMSX103.ger.corp.intel.com>
     [not found]           ` <20170323102135.GA124301@localhost.localdomain>
     [not found]             ` <2601191342CEEE43887BDE71AB9772583FAD410A@IRSMSX109.ger.corp.intel.com>
2017-03-24  2:23               ` [dpdk-dev] [PATCH 0/2] lib: add TCP IPv4 GRO support Jiayu Hu
2017-03-24  6:18                 ` Wiles, Keith
2017-03-24  7:22                   ` Yuanhan Liu
2017-03-24  8:06                     ` Jiayu Hu
2017-03-24 11:43                       ` Ananyev, Konstantin
2017-03-24 14:37                         ` Wiles, Keith
2017-03-24 14:59                           ` Olivier Matz
2017-03-24 15:07                             ` Wiles, Keith
2017-03-28 13:40                               ` Wiles, Keith
2017-03-28 13:57                                 ` Hu, Jiayu
2017-03-28 16:06                                   ` Wiles, Keith
2017-03-29 10:47                         ` Morten Brørup
2017-03-29 12:12                           ` Wiles, Keith
2017-04-04 12:31 ` [dpdk-dev] [PATCH v2 0/3] support GRO in DPDK Jiayu Hu
2017-04-04 12:31   ` [dpdk-dev] [PATCH v2 1/3] lib: add Generic Receive Offload API framework Jiayu Hu
2017-04-04 12:31   ` [dpdk-dev] [PATCH v2 2/3] lib/gro: add TCP/IPv4 GRO support Jiayu Hu
2017-04-04 12:31   ` [dpdk-dev] [PATCH v2 3/3] app/testpmd: enable GRO feature Jiayu Hu
2017-04-24  8:09   ` [dpdk-dev] [PATCH v3 0/3] support GRO in DPDK Jiayu Hu
2017-04-24  8:09     ` [dpdk-dev] [PATCH v3 1/3] lib: add Generic Receive Offload API framework Jiayu Hu
2017-05-22  9:19       ` Ananyev, Konstantin
2017-05-23 10:31         ` Jiayu Hu
2017-05-24 12:38           ` Ananyev, Konstantin
2017-05-26  7:26             ` Jiayu Hu
2017-05-26 23:10               ` Ananyev, Konstantin
2017-05-27  3:41                 ` Jiayu Hu
2017-05-27 11:12                   ` Ananyev, Konstantin
2017-05-27 14:09                     ` Jiayu Hu
2017-05-27 16:51                       ` Ananyev, Konstantin
2017-05-29 10:22                         ` Hu, Jiayu
2017-05-29 12:18                           ` Bruce Richardson
2017-05-30 14:10                             ` Hu, Jiayu
2017-05-29 12:51                           ` Ananyev, Konstantin
2017-05-30  5:29                             ` Hu, Jiayu
2017-05-30 11:56                               ` Ananyev, Konstantin
2017-04-24  8:09     ` [dpdk-dev] [PATCH v3 2/3] lib/gro: add TCP/IPv4 GRO support Jiayu Hu
2017-04-24  8:09     ` [dpdk-dev] [PATCH v3 3/3] app/testpmd: enable GRO feature Jiayu Hu
2017-06-07  9:24       ` Wu, Jingjing
2017-06-07 11:08     ` [dpdk-dev] [PATCH v4 0/3] Support TCP/IPv4 GRO in DPDK Jiayu Hu
2017-06-07 11:08       ` [dpdk-dev] [PATCH v4 1/3] lib: add Generic Receive Offload API framework Jiayu Hu
2017-06-07 11:08       ` [dpdk-dev] [PATCH v4 2/3] lib/gro: add TCP/IPv4 GRO support Jiayu Hu
2017-06-07 11:08       ` [dpdk-dev] [PATCH v4 3/3] app/testpmd: enable TCP/IPv4 GRO Jiayu Hu
2017-06-18  7:21       ` [dpdk-dev] [PATCH v5 0/3] Support TCP/IPv4 GRO in DPDK Jiayu Hu
2017-06-18  7:21         ` [dpdk-dev] [PATCH v5 1/3] lib: add Generic Receive Offload API framework Jiayu Hu
2017-06-19  4:03           ` Tiwei Bie
2017-06-19  5:16             ` Jiayu Hu
2017-06-19 15:43           ` Tan, Jianfeng
2017-06-19 15:55           ` Stephen Hemminger
2017-06-20  1:48             ` Jiayu Hu
2017-06-18  7:21         ` [dpdk-dev] [PATCH v5 2/3] lib/gro: add TCP/IPv4 GRO support Jiayu Hu
2017-06-19 15:43           ` Tan, Jianfeng
2017-06-20  3:22             ` Jiayu Hu
2017-06-20 15:15               ` Ananyev, Konstantin
2017-06-20 16:16                 ` Jiayu Hu
2017-06-20 15:21               ` Ananyev, Konstantin
2017-06-20 23:30               ` Tan, Jianfeng
2017-06-20 23:55                 ` Stephen Hemminger
2017-06-22  7:39                 ` Jiayu Hu
2017-06-22  8:18             ` Jiayu Hu
2017-06-22  9:35               ` Tan, Jianfeng
2017-06-22 13:55                 ` Jiayu Hu
2017-06-18  7:21         ` [dpdk-dev] [PATCH v5 3/3] app/testpmd: enable TCP/IPv4 GRO Jiayu Hu
2017-06-19  1:24           ` Yao, Lei A
2017-06-19  2:27           ` Wu, Jingjing
2017-06-19  3:22             ` Jiayu Hu
2017-06-19  1:39         ` [dpdk-dev] [PATCH v5 0/3] Support TCP/IPv4 GRO in DPDK Tan, Jianfeng
2017-06-19  3:07           ` Jiayu Hu
2017-06-19  5:12             ` Jiayu Hu
2017-06-23 14:43         ` [dpdk-dev] [PATCH v6 " Jiayu Hu
2017-06-23 14:43           ` [dpdk-dev] [PATCH v6 1/3] lib: add Generic Receive Offload API framework Jiayu Hu
2017-06-25 16:53             ` Tan, Jianfeng
2017-06-23 14:43           ` [dpdk-dev] [PATCH v6 2/3] lib/gro: add TCP/IPv4 GRO support Jiayu Hu
2017-06-25 16:53             ` Tan, Jianfeng
2017-06-26  1:58               ` Jiayu Hu
2017-06-23 14:43           ` [dpdk-dev] [PATCH v6 3/3] app/testpmd: enable TCP/IPv4 GRO Jiayu Hu
2017-06-24  8:01             ` Yao, Lei A
2017-06-25 16:03           ` [dpdk-dev] [PATCH v6 0/3] Support TCP/IPv4 GRO in DPDK Tan, Jianfeng
2017-06-26  1:35             ` Jiayu Hu
2017-06-26  6:43           ` [dpdk-dev] [PATCH v7 " Jiayu Hu
2017-06-26  6:43             ` [dpdk-dev] [PATCH v7 1/3] lib: add Generic Receive Offload API framework Jiayu Hu
2017-06-27 23:42               ` Ananyev, Konstantin
2017-06-28  2:17                 ` Jiayu Hu
2017-06-28 17:41                   ` Ananyev, Konstantin
2017-06-29  1:19                     ` Jiayu Hu
2017-06-26  6:43             ` [dpdk-dev] [PATCH v7 2/3] lib/gro: add TCP/IPv4 GRO support Jiayu Hu
2017-06-28 23:56               ` Ananyev, Konstantin
2017-06-29  2:26                 ` Jiayu Hu
2017-06-30 12:07                   ` Ananyev, Konstantin
2017-06-30 15:40                     ` Hu, Jiayu
2017-06-26  6:43             ` [dpdk-dev] [PATCH v7 3/3] app/testpmd: enable TCP/IPv4 GRO Jiayu Hu
2017-06-29 10:58             ` [dpdk-dev] [PATCH v8 0/3] Support TCP/IPv4 GRO in DPDK Jiayu Hu
2017-06-29 10:58               ` [dpdk-dev] [PATCH v8 1/3] lib: add Generic Receive Offload API framework Jiayu Hu
2017-06-29 10:58               ` [dpdk-dev] [PATCH v8 2/3] lib/gro: add TCP/IPv4 GRO support Jiayu Hu
2017-06-29 17:51                 ` Stephen Hemminger
2017-06-30  2:07                   ` Jiayu Hu
2017-06-29 10:59               ` [dpdk-dev] [PATCH v8 3/3] app/testpmd: enable TCP/IPv4 GRO Jiayu Hu
2017-06-30  2:26                 ` Wu, Jingjing
2017-06-30  6:53               ` [dpdk-dev] [PATCH v9 0/3] Support TCP/IPv4 GRO in DPDK Jiayu Hu
2017-06-30  6:53                 ` [dpdk-dev] [PATCH v9 1/3] lib: add Generic Receive Offload API framework Jiayu Hu
2017-06-30  6:53                 ` [dpdk-dev] [PATCH v9 2/3] lib/gro: add TCP/IPv4 GRO support Jiayu Hu
2017-06-30  6:53                 ` [dpdk-dev] [PATCH v9 3/3] app/testpmd: enable TCP/IPv4 GRO Jiayu Hu
2017-07-01 11:08                 ` [dpdk-dev] [PATCH v10 0/3] Support TCP/IPv4 GRO in DPDK Jiayu Hu
2017-07-01 11:08                   ` [dpdk-dev] [PATCH v10 1/3] lib: add Generic Receive Offload API framework Jiayu Hu
2017-07-02 10:19                     ` Tan, Jianfeng
2017-07-03  5:56                       ` Hu, Jiayu
2017-07-04  8:11                         ` Yuanhan Liu
2017-07-04  8:37                     ` Yuanhan Liu
2017-07-04 16:01                       ` Hu, Jiayu [this message]
2017-07-01 11:08                   ` [dpdk-dev] [PATCH v10 2/3] lib/gro: add TCP/IPv4 GRO support Jiayu Hu
2017-07-02 10:19                     ` Tan, Jianfeng
2017-07-03  5:13                       ` Hu, Jiayu
2017-07-04  9:03                     ` Yuanhan Liu
2017-07-04 16:03                       ` Hu, Jiayu
2017-07-01 11:08                   ` [dpdk-dev] [PATCH v10 3/3] app/testpmd: enable TCP/IPv4 GRO Jiayu Hu
2017-07-05  4:08                   ` [dpdk-dev] [PATCH v11 0/3] Support TCP/IPv4 GRO in DPDK Jiayu Hu
2017-07-05  4:08                     ` [dpdk-dev] [PATCH v11 1/3] lib: add Generic Receive Offload API framework Jiayu Hu
2017-07-07  6:55                       ` Tan, Jianfeng
2017-07-07  9:19                         ` Tan, Jianfeng
2017-07-05  4:08                     ` [dpdk-dev] [PATCH v11 2/3] lib/gro: add TCP/IPv4 GRO support Jiayu Hu
2017-07-07  6:55                       ` Tan, Jianfeng
2017-07-05  4:08                     ` [dpdk-dev] [PATCH v11 3/3] app/testpmd: enable TCP/IPv4 GRO Jiayu Hu
2017-07-07 10:39                     ` [dpdk-dev] [PATCH v12 0/3] Support TCP/IPv4 GRO in DPDK Jiayu Hu
2017-07-07 10:39                       ` [dpdk-dev] [PATCH v12 1/3] lib: add Generic Receive Offload API framework Jiayu Hu
2017-07-08 16:37                         ` Tan, Jianfeng
2017-07-07 10:39                       ` [dpdk-dev] [PATCH v12 2/3] lib/gro: add TCP/IPv4 GRO support Jiayu Hu
2017-07-08 16:37                         ` Tan, Jianfeng
2017-07-07 10:39                       ` [dpdk-dev] [PATCH v12 3/3] app/testpmd: enable TCP/IPv4 GRO Jiayu Hu
2017-07-09  1:13                       ` [dpdk-dev] [PATCH v13 0/3] Support TCP/IPv4 GRO in DPDK Jiayu Hu
2017-07-09  1:13                         ` [dpdk-dev] [PATCH v13 1/3] lib: add Generic Receive Offload API framework Jiayu Hu
2017-07-09  1:13                         ` [dpdk-dev] [PATCH v13 2/3] lib/gro: add TCP/IPv4 GRO support Jiayu Hu
2017-07-09  1:13                         ` [dpdk-dev] [PATCH v13 3/3] app/testpmd: enable TCP/IPv4 GRO Jiayu Hu
2017-07-09  5:46                         ` [dpdk-dev] [PATCH v14 0/3] Support TCP/IPv4 GRO in DPDK Jiayu Hu
2017-07-09  5:46                           ` [dpdk-dev] [PATCH v14 1/3] lib: add Generic Receive Offload API framework Jiayu Hu
2017-07-09  5:46                           ` [dpdk-dev] [PATCH v14 2/3] lib/gro: add TCP/IPv4 GRO support Jiayu Hu
2017-07-09  5:46                           ` [dpdk-dev] [PATCH v14 3/3] app/testpmd: enable TCP/IPv4 GRO Jiayu Hu
2017-07-09  7:59                             ` Yao, Lei A
2017-07-09 16:14                           ` [dpdk-dev] [PATCH v14 0/3] Support TCP/IPv4 GRO in DPDK Thomas Monjalon
2017-07-10  2:21                             ` Hu, Jiayu
2017-07-10  7:03                               ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ED946F0BEFE0A141B63BABBD629A2A9B3876F9DD@shsmsx102.ccr.corp.intel.com \
    --to=jiayu.hu@intel.com \
    --cc=dev@dpdk.org \
    --cc=jianfeng.tan@intel.com \
    --cc=jingjing.wu@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=lei.a.yao@intel.com \
    --cc=stephen@networkplumber.org \
    --cc=tiwei.bie@intel.com \
    --cc=yliu@fridaylinux.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).