From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 759CE374F for ; Tue, 4 Jul 2017 18:03:11 +0200 (CEST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Jul 2017 09:03:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,309,1496127600"; d="scan'208";a="104379287" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga004.jf.intel.com with ESMTP; 04 Jul 2017 09:03:10 -0700 Received: from fmsmsx158.amr.corp.intel.com (10.18.116.75) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 4 Jul 2017 09:03:09 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx158.amr.corp.intel.com (10.18.116.75) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 4 Jul 2017 09:03:10 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.146]) by SHSMSX104.ccr.corp.intel.com ([10.239.4.70]) with mapi id 14.03.0319.002; Wed, 5 Jul 2017 00:03:07 +0800 From: "Hu, Jiayu" To: Yuanhan Liu CC: "dev@dpdk.org" , "Ananyev, Konstantin" , "stephen@networkplumber.org" , "Tan, Jianfeng" , "Wu, Jingjing" , "Yao, Lei A" , "Bie, Tiwei" Thread-Topic: [PATCH v10 2/3] lib/gro: add TCP/IPv4 GRO support Thread-Index: AQHS8lo/MRljckFCyEKPm8VDcdhCIaJC3d8AgAD7FfA= Date: Tue, 4 Jul 2017 16:03:06 +0000 Message-ID: References: <1498805618-63649-1-git-send-email-jiayu.hu@intel.com> <1498907323-17563-1-git-send-email-jiayu.hu@intel.com> <1498907323-17563-3-git-send-email-jiayu.hu@intel.com> <20170704090304.GL11626@yliu-home> In-Reply-To: <20170704090304.GL11626@yliu-home> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v10 2/3] lib/gro: add TCP/IPv4 GRO support 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: Tue, 04 Jul 2017 16:03:12 -0000 Hi Yuanhan, > -----Original Message----- > From: Yuanhan Liu [mailto:yliu@fridaylinux.org] > Sent: Tuesday, July 4, 2017 5:03 PM > To: Hu, Jiayu > Cc: dev@dpdk.org; Ananyev, Konstantin ; > stephen@networkplumber.org; Tan, Jianfeng ; Wu, > Jingjing ; Yao, Lei A ; Bie, > Tiwei > Subject: Re: [PATCH v10 2/3] lib/gro: add TCP/IPv4 GRO support >=20 > Again, just some quick comments after a glimpse. >=20 > On Sat, Jul 01, 2017 at 07:08:42PM +0800, Jiayu Hu wrote: > > + for (i =3D 0; i < nb_pkts; i++) { > > + if (RTE_ETH_IS_IPV4_HDR(pkts[i]->packet_type) && > > + (pkts[i]->packet_type & > RTE_PTYPE_L4_TCP)) { > > + ret =3D gro_tcp4_reassemble(pkts[i], > > + &tcp_tbl, > > + param->max_packet_size, > > + current_time); > > + if (ret > 0) > > + /* merge successfully */ > > + nb_after_gro--; > > + else if (ret < 0) > > + unprocess_pkts[unprocess_num++] =3D > > + pkts[i]; >=20 > Even it's just one statement, if the statement is spawned more than > one line, including the comment, the {} should be used. >=20 > Section 1.6.2. Control Statements and Loops of: > http://dpdk.org/doc/guides/contributing/coding_style.html >=20 > > + } else > > + unprocess_pkts[unprocess_num++] =3D > > + pkts[i]; >=20 > Besides, why breaking it to two lines, judging that it can be fit into > one line smaller than 80 chars. Thanks, I will add {}. >=20 > > + } > > + > > + /* re-arrange GROed packets */ > > + if (nb_after_gro < nb_pkts) { > > + i =3D gro_tcp4_tbl_timeout_flush(&tcp_tbl, 0, > > + pkts, nb_pkts); > > + if (unprocess_num > 0) > > + memcpy(&pkts[i], unprocess_pkts, > > + sizeof(struct rte_mbuf *) * > > + unprocess_num); >=20 > Ditto. >=20 > > +void *gro_tcp4_tbl_create(uint16_t socket_id, > > + uint16_t max_flow_num, > > + uint16_t max_item_per_flow) > > +{ > > + size_t size; > > + uint32_t entries_num; > > + struct gro_tcp4_tbl *tbl; > > + > > + entries_num =3D max_flow_num * max_item_per_flow; > > + entries_num =3D entries_num > GRO_TCP4_TBL_MAX_ITEM_NUM ? > > + GRO_TCP4_TBL_MAX_ITEM_NUM : entries_num; > > + > > + if (entries_num =3D=3D 0) > > + return NULL; > > + > > + tbl =3D (struct gro_tcp4_tbl *)rte_zmalloc_socket( > > + __func__, > > + sizeof(struct gro_tcp4_tbl), > > + RTE_CACHE_LINE_SIZE, > > + socket_id); >=20 > Again, the cast (from void *) is unnessary and should be dropped. >=20 > > + memcpy(&(tbl->keys[key_idx].key), > > + &key, sizeof(struct tcp4_key)); >=20 > Again, I believe they two can be fit into one single line. Thanks, I will fix these issues. BRs, Jiayu >=20 > --yliu