From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 73A92C3CE for ; Mon, 3 Aug 2015 09:41:11 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP; 03 Aug 2015 00:41:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,599,1432623600"; d="scan'208";a="760669148" Received: from pgsmsx103.gar.corp.intel.com ([10.221.44.82]) by fmsmga001.fm.intel.com with ESMTP; 03 Aug 2015 00:41:09 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by PGSMSX103.gar.corp.intel.com (10.221.44.82) with Microsoft SMTP Server (TLS) id 14.3.224.2; Mon, 3 Aug 2015 15:41:07 +0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.126]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.45]) with mapi id 14.03.0224.002; Mon, 3 Aug 2015 15:41:06 +0800 From: "Liang, Cunming" To: "Ananyev, Konstantin" , Zoltan Kiss , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v1] ixgbe: remove vector pmd burst size restriction Thread-Index: AQHQy3qiQkxqTpGWFEC+z7BWNhCDRp35lIWQ Date: Mon, 3 Aug 2015 07:41:05 +0000 Message-ID: References: <1438330669-25942-1-git-send-email-cunming.liang@intel.com> <55BB47FB.3000409@linaro.org> <2601191342CEEE43887BDE71AB97725836A6B79C@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB97725836A6B79C@irsmsx105.ger.corp.intel.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: 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 v1] ixgbe: remove vector pmd burst size restriction X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Aug 2015 07:41:13 -0000 Hi,=20 [...] > > > uint16_t > > > ixgbe_recv_scattered_pkts_vec(void *rx_queue, struct rte_mbuf **rx_= pkts, > > > uint16_t nb_pkts) > > > { > > > struct ixgbe_rx_queue *rxq =3D rx_queue; > > > - uint8_t split_flags[RTE_IXGBE_VPMD_RX_BURST] =3D {0}; > > > + uint8_t split_flags[nb_pkts]; > > > + > > > + memset(split_flags, 0, nb_pkts); > > > > > > /* get some new buffers */ > > > uint16_t nb_bufs =3D _recv_raw_pkts_vec(rxq, rx_pkts, nb_pkts, > > > > After this _recv_raw_pkts_vec it checks 32 bytes in split_flags (4x8 > > bytes), that can overrun or miss some flags. > > Btw. Bruce just fixed that part in "ixgbe: fix check for split packets" >=20 > Ah yes, missed that when reviewing, that code would be broken if nb_bufs = > 32: >=20 > const uint64_t *split_fl64 =3D (uint64_t *)split_flags; > if (rxq->pkt_first_seg =3D=3D NULL && > split_fl64[0] =3D=3D 0 && split_fl64[1] =3D=3D 0 = && > split_fl64[2] =3D=3D 0 && split_fl64[3] =3D=3D 0) > return nb_bufs; >=20 > right? We can either rollback and only allow 'nb_pkts<=3D32', or do some broken fi= x as below diff. By the result of performance test (4*10GE 64B burst_size(32) iofwd by scatt= ered_pkts_vec), there's no drop. But I'm not sure it is important or not to allow burst size larger than 32.= Your comments will be important. diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec.c b/drivers/net/ixgbe/ixgbe_r= xtx_vec.c index e94c68b..8f34236 100644 --- a/drivers/net/ixgbe/ixgbe_rxtx_vec.c +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec.c @@ -537,26 +537,35 @@ uint16_t ixgbe_recv_scattered_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) { +#define NB_SPLIT_ELEM (8) struct ixgbe_rx_queue *rxq =3D rx_queue; uint8_t split_flags[nb_pkts]; + uint32_t i, nb_scan; + uint16_t nb_bufs; + uint64_t *split_fl64 =3D (uint64_t *)split_flags; memset(split_flags, 0, nb_pkts); /* get some new buffers */ - uint16_t nb_bufs =3D _recv_raw_pkts_vec(rxq, rx_pkts, nb_pkts, - split_flags); + nb_bufs =3D _recv_raw_pkts_vec(rxq, rx_pkts, nb_pkts, + split_flags); if (nb_bufs =3D=3D 0) return 0; /* happy day case, full burst + no packets to be joined */ - const uint64_t *split_fl64 =3D (uint64_t *)split_flags; - if (rxq->pkt_first_seg =3D=3D NULL && - split_fl64[0] =3D=3D 0 && split_fl64[1] =3D=3D 0 && - split_fl64[2] =3D=3D 0 && split_fl64[3] =3D=3D 0) + nb_scan =3D RTE_ALIGN(nb_bufs, NB_SPLIT_ELEM); + if (rxq->pkt_first_seg =3D=3D NULL) { + for (i =3D 0; i < nb_scan; + i +=3D NB_SPLIT_ELEM, split_fl64++) { + if (*split_fl64 !=3D 0) + goto reassemble; + } return nb_bufs; + } +reassemble: /* reassemble any packets that need reassembly*/ - unsigned i =3D 0; + i =3D 0; if (rxq->pkt_first_seg =3D=3D NULL) { /* find the first split flag, and only reassemble then*/ while (i < nb_bufs && !split_flags[i]) /Steve >=20 > Another thing, that I just thought about: > Right now we invoke ixgbe_rxq_rearm() only at the start of > _recv_raw_pkts_vec(). > Before it was ok, as _recv_raw_pkts_vec() would never try to read more th= en 32 > RXDs. > But what would happen if nb_pkts > rxq->nb_desc and rxq->rxrearm_nb =3D= =3D 0? > I suppose, _recv_raw_pkts_vec() can wrpa around RXD ring and 'receive' s= ame > packet twice? > So we probably better still limit nb_pkts <=3D 32 at _recv_raw_pkts_vec()= . The _recv_raw_pkts_vec() won't wrap around RXD ring. When it reaches the la= st one, the DD bit of padding desc. always 0. So in the case nb_pkts > rxq->nb_desc, the '_recv_raw_pkts_vec()' can only = get no more than 'rxq->nb_desc' packets. >=20 > Konstantin >=20 > >