From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B3DAAA0C47; Tue, 6 Jul 2021 10:33:53 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7EB2A40688; Tue, 6 Jul 2021 10:33:53 +0200 (CEST) Received: from smtpbgau1.qq.com (smtpbgau1.qq.com [54.206.16.166]) by mails.dpdk.org (Postfix) with ESMTP id 030C34067E for ; Tue, 6 Jul 2021 10:33:50 +0200 (CEST) X-QQ-mid: bizesmtp50t1625560425tfrlgn7w Received: from jiawenwu (unknown [183.129.236.74]) by esmtp6.qq.com (ESMTP) with id ; Tue, 06 Jul 2021 16:33:42 +0800 (CST) X-QQ-SSF: 01400000002000D0E000B00A0000000 X-QQ-FEAT: PAtBXkQ2bGAl6E/4odw1JT8lgshFGusQ+KLErkBw2N911btIdAJEdudpxukrj PoRDrsWicpedHlT9ClSkmc6QCUnAKFFzVlDbjb6v/L8T/QL5yO4uhBrxjcktv79LOzoJEOp QW0tPpw9/o7/E3uZJyZXWOuCLZ+e5/nA6kbiUkYOs3auRdHXFU/H9MJLt47QM4DVvya29Qx KvgNU8wQ2o/eg0eQF0x070gm87FjBw5KfJqnaXuBTWnoeu1JTUCxceo6gKIIErXKi03Dc2d q73RXrtJgyzAwHLJ+RoLnFLpWn4hCeXnIbwR5XmRbeGegZU50wW+17wrxRAZWFxQfWTG4PD LMsbyIkxpyb+6TUC1dwNLX2YNdaESyP79k1Qi05 X-QQ-GoodBg: 2 From: "Jiawen Wu" To: "'Andrew Rybchenko'" , References: <20210617110005.4132926-1-jiawenwu@trustnetic.com> <20210617110005.4132926-13-jiawenwu@trustnetic.com> <6d34d736-eaf6-a69a-47b5-9f42e0c09b5c@oktetlabs.ru> <00a801d77178$d6d645e0$8482d1a0$@trustnetic.com> <00ac01d7723c$0cce7460$266b5d20$@trustnetic.com> In-Reply-To: Date: Tue, 6 Jul 2021 16:33:42 +0800 Message-ID: <00ae01d77241$a2317570$e6946050$@trustnetic.com>+0834504FFFD04DD8 MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQEx9tyhu8851cQlQUtl5PIyQvroAQHfIcqsAmvVc5ABTezltwHFWsfZAYGpxRgB82qbeqwp+DUg Content-Language: zh-cn X-QQ-SENDSIZE: 520 Feedback-ID: bizesmtp:trustnetic.com:qybgforeign:qybgforeign2 X-QQ-Bgrelay: 1 Subject: Re: [dpdk-dev] [PATCH v6 12/19] net/ngbe: add Rx queue setup and release X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On July 6, 2021 4:06 PM, Andrew Rybchenko wrote: > On 7/6/21 10:53 AM, Jiawen Wu wrote: > > On July 5, 2021 5:08 PM, Andrew Rybchenko wrote: > >> On 7/5/21 11:36 AM, Jiawen Wu wrote: > >>> On July 3, 2021 12:36 AM, Andrew Rybchenko wrote: > >>>> On 6/17/21 1:59 PM, Jiawen Wu wrote: > >>>>> Setup device Rx queue and release Rx queue. > >>>>> > >>>>> Signed-off-by: Jiawen Wu > >>>>> --- > >>>>> drivers/net/ngbe/meson.build | 1 + > >>>>> drivers/net/ngbe/ngbe_ethdev.c | 37 +++- > >>>>> drivers/net/ngbe/ngbe_ethdev.h | 16 ++ > >>>>> drivers/net/ngbe/ngbe_rxtx.c | 308 > +++++++++++++++++++++++++++++++++ > >>>>> drivers/net/ngbe/ngbe_rxtx.h | 96 ++++++++++ > >>>>> 5 files changed, 457 insertions(+), 1 deletion(-) create mode > >>>>> 100644 drivers/net/ngbe/ngbe_rxtx.c create mode 100644 > >>>>> drivers/net/ngbe/ngbe_rxtx.h > >>>>> > >>>>> a/drivers/net/ngbe/ngbe_ethdev.c = b/drivers/net/ngbe/ngbe_ethdev.c > >>>>> index c952023e8b..e73606c5f3 100644 > >>>>> --- a/drivers/net/ngbe/ngbe_ethdev.c > >>>>> +++ b/drivers/net/ngbe/ngbe_ethdev.c > >>>>> @@ -37,6 +38,12 @@ static const struct rte_pci_id = pci_id_ngbe_map[] > =3D { > >>>>> { .vendor_id =3D 0, /* sentinel */ }, }; > >>>>> > >>>>> +static const struct rte_eth_desc_lim rx_desc_lim =3D { > >>>>> + .nb_max =3D NGBE_RING_DESC_MAX, > >>>>> + .nb_min =3D NGBE_RING_DESC_MIN, > >>>>> + .nb_align =3D NGBE_RXD_ALIGN, > >>>>> +}; > >>>>> + > >>>>> static const struct eth_dev_ops ngbe_eth_dev_ops; > >>>>> > >>>>> static inline void > >>>>> @@ -266,11 +280,30 @@ ngbe_dev_close(struct rte_eth_dev *dev) > >>>>> static int ngbe_dev_info_get(struct rte_eth_dev *dev, struct > >>>>> rte_eth_dev_info *dev_info) { > >>>>> - RTE_SET_USED(dev); > >>>>> + struct ngbe_hw *hw =3D ngbe_dev_hw(dev); > >>>>> + > >>>>> + dev_info->max_rx_queues =3D (uint16_t)hw->mac.max_rx_queues; > >>>>> + > >>>>> + dev_info->default_rxconf =3D (struct rte_eth_rxconf) { > >>>>> + .rx_thresh =3D { > >>>>> + .pthresh =3D NGBE_DEFAULT_RX_PTHRESH, > >>>>> + .hthresh =3D NGBE_DEFAULT_RX_HTHRESH, > >>>>> + .wthresh =3D NGBE_DEFAULT_RX_WTHRESH, > >>>>> + }, > >>>>> + .rx_free_thresh =3D NGBE_DEFAULT_RX_FREE_THRESH, > >>>>> + .rx_drop_en =3D 0, > >>>>> + .offloads =3D 0, > >>>>> + }; > >>>>> + > >>>>> + dev_info->rx_desc_lim =3D rx_desc_lim; > >>>>> > > <...> > >>>>> +int __rte_cold > >>>>> +ngbe_dev_rx_queue_setup(struct rte_eth_dev *dev, > >>>>> + uint16_t queue_idx, > >>>>> + uint16_t nb_desc, > >>>>> + unsigned int socket_id, > >>>>> + const struct rte_eth_rxconf *rx_conf, > >>>>> + struct rte_mempool *mp) > >>>>> +{ > >>>>> + const struct rte_memzone *rz; > >>>>> + struct ngbe_rx_queue *rxq; > >>>>> + struct ngbe_hw *hw; > >>>>> + uint16_t len; > >>>>> + struct ngbe_adapter *adapter =3D ngbe_dev_adapter(dev); > >>>>> + > >>>>> + PMD_INIT_FUNC_TRACE(); > >>>>> + hw =3D ngbe_dev_hw(dev); > >>>>> + > >>>>> + /* > >>>>> + * Validate number of receive descriptors. > >>>>> + * It must not exceed hardware maximum, and must be multiple > >>>>> + * of NGBE_ALIGN. > >>>>> + */ > >>>>> + if (nb_desc % NGBE_RXD_ALIGN !=3D 0 || > >>>>> + nb_desc > NGBE_RING_DESC_MAX || > >>>>> + nb_desc < NGBE_RING_DESC_MIN) { > >>>>> + return -EINVAL; > >>>>> + } > >>>> > >>>> rte_eth_rx_queue_setup cares about it > >>>> > >>> > >>> I don't quite understand. > >> > >> ethdev does the check based dev_info provided by the driver: > >> > >> if (nb_rx_desc > dev_info.rx_desc_lim.nb_max || > >> nb_rx_desc < dev_info.rx_desc_lim.nb_min || > >> nb_rx_desc % dev_info.rx_desc_lim.nb_align !=3D 0) { > >> RTE_ETHDEV_LOG(ERR, > >> "Invalid value for nb_rx_desc(=3D%hu), should be: <=3D = %hu, > >> >=3D %hu, and a product of %hu\n", > >> nb_rx_desc, dev_info.rx_desc_lim.nb_max, > >> dev_info.rx_desc_lim.nb_min, > >> dev_info.rx_desc_lim.nb_align); > >> return -EINVAL; > >> } > > > > 'dev_info.rx_desc_lim' was set up in function 'ngbe_dev_info_get' = with the > struct 'rx_desc_lim' defined. > > So I think it is appropriate to check with macros. However, the log > > can be > Sorry, but I don't understand why do you want to duplicate check which = is > already done in a generic code. This check can be omitted, thanks. NGBE driver code was ported from some earlier code, there may some = redundant code left.