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 48E92429FF; Wed, 26 Apr 2023 09:00:15 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2D01B410DC; Wed, 26 Apr 2023 09:00:15 +0200 (CEST) Received: from EUR05-DB8-obe.outbound.protection.outlook.com (mail-db8eur05on2049.outbound.protection.outlook.com [40.107.20.49]) by mails.dpdk.org (Postfix) with ESMTP id 7276A40DDA for ; Wed, 26 Apr 2023 09:00:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=nKydDyzICtG5mY80maxVDmHi75ok2iFs3c16IxRYe6g=; b=9iNIm0JxfnYIyxkdtv04NjBhviGhIs/L4si0kei4X/t03DlASVGx1gF1n3E7ExVNP47sBui8C+GAWz5FBt7rBOzs4eQHTYDHwEJ+wjR1+OlN5NQNRVd+G8UwjzvfuWp4Hq6rTM1QXgrINxqO5m+XT0xDnuXCXRB47GKxqVd9uTA= Received: from DUZPR01CA0031.eurprd01.prod.exchangelabs.com (2603:10a6:10:468::16) by VI1PR08MB5358.eurprd08.prod.outlook.com (2603:10a6:803:13c::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6340.21; Wed, 26 Apr 2023 07:00:10 +0000 Received: from DBAEUR03FT012.eop-EUR03.prod.protection.outlook.com (2603:10a6:10:468:cafe::f1) by DUZPR01CA0031.outlook.office365.com (2603:10a6:10:468::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6319.34 via Frontend Transport; Wed, 26 Apr 2023 07:00:10 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; pr=C Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by DBAEUR03FT012.mail.protection.outlook.com (100.127.142.126) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6340.20 via Frontend Transport; Wed, 26 Apr 2023 07:00:10 +0000 Received: ("Tessian outbound 8b05220b4215:v136"); Wed, 26 Apr 2023 07:00:09 +0000 X-CR-MTA-TID: 64aa7808 Received: from ba6498ae9356.2 by 64aa7808-outbound-1.mta.getcheckrecipient.com id D6B13E93-23CB-4AC4-934F-B57A6A22ED01.1; Wed, 26 Apr 2023 06:59:59 +0000 Received: from EUR01-HE1-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id ba6498ae9356.2 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Wed, 26 Apr 2023 06:59:59 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=cbIo/TEMrF8o44olhOlG3xme9NT5X7+0LWXZmGSUb4KshZygmP5BCqM2byvixgHB396pWRFYGPCvUrblkYpiow1cjiHSUGfq1ouzX7mfJ3Cu9+mzGsee1zNz49qG5+RiIPIufsJ6z0d13k4+pzIy25guXiBQMKUaJfkdT3ghLr7y5toVllfjGHwvYetRJaGAdQBtHPN9js6x+cLL20tepdramUeVfx3YhlqFTBxF0L4w1q9nq54twWdve0bsQQnH5c+iioeIW/GeIs2jiMoNo8IG60oW8cSp9WGtLQRKcbSZSBjx8N/Y8/CAnFoJoelZ2Oy0vlrJqcpCvCpFlUnLuw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=nKydDyzICtG5mY80maxVDmHi75ok2iFs3c16IxRYe6g=; b=CSxAPlbp0jm5sT4xAwqUIRiqvzqFis5mF8eBjrZZiDpkl9i++wJlh49T1dUFjPo+U2bqXDu+bNjcM8z1KCyi2sLobzPl1KzAQW5TVOVQnepZiE7n74poSvR7V+IQOZg79FtMo63/hD9nQIM+8c4J1kU1azedEAtYZEnsKNAG5hgH5D7ubYzTfZYsesPyWrIL2CXduTNJdGEM+3zQfr7K/CeCBME25mXk/krb4xXNHLNPmJjqqFBDYpVnwuQEHKwjFpaXUq72OoFls66bXyPvl3lQDlgZRPsd20bEEznki+cYcVjgbeZyiVJaj/G+NaYb4pf+SabJLPoiCf6AEYGXpQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=nKydDyzICtG5mY80maxVDmHi75ok2iFs3c16IxRYe6g=; b=9iNIm0JxfnYIyxkdtv04NjBhviGhIs/L4si0kei4X/t03DlASVGx1gF1n3E7ExVNP47sBui8C+GAWz5FBt7rBOzs4eQHTYDHwEJ+wjR1+OlN5NQNRVd+G8UwjzvfuWp4Hq6rTM1QXgrINxqO5m+XT0xDnuXCXRB47GKxqVd9uTA= Received: from AS8PR08MB7718.eurprd08.prod.outlook.com (2603:10a6:20b:50a::22) by VI1PR08MB10049.eurprd08.prod.outlook.com (2603:10a6:800:1c5::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6319.34; Wed, 26 Apr 2023 06:59:51 +0000 Received: from AS8PR08MB7718.eurprd08.prod.outlook.com ([fe80::7878:ff5f:b10:d4a6]) by AS8PR08MB7718.eurprd08.prod.outlook.com ([fe80::7878:ff5f:b10:d4a6%7]) with mapi id 15.20.6340.021; Wed, 26 Apr 2023 06:59:51 +0000 From: Feifei Wang To: =?iso-8859-1?Q?Morten_Br=F8rup?= , "thomas@monjalon.net" , Ferruh Yigit , Andrew Rybchenko CC: "dev@dpdk.org" , "konstantin.v.ananyev@yandex.ru" , nd , Honnappa Nagarahalli , Ruifeng Wang , nd , nd Subject: RE: [PATCH v5 1/3] ethdev: add API for buffer recycle mode Thread-Topic: [PATCH v5 1/3] ethdev: add API for buffer recycle mode Thread-Index: AQHZYtEPO3yD/9FA0E+nLV0H3CV5M68S6kqAgAAXoRCAAGCxwIAAFbcwgCnYg/A= Date: Wed, 26 Apr 2023 06:59:51 +0000 Message-ID: References: <20211224164613.32569-1-feifei.wang2@arm.com> <20230330062939.1206267-1-feifei.wang2@arm.com> <20230330062939.1206267-2-feifei.wang2@arm.com> <98CBD80474FA8B44BF855DF32C47DC35D8781F@smartserver.smartshare.dk> <98CBD80474FA8B44BF855DF32C47DC35D8782F@smartserver.smartshare.dk> In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D8782F@smartserver.smartshare.dk> Accept-Language: zh-CN, en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ts-tracking-id: 999D18719B58634591FC18BF01F64258.0 x-checkrecipientchecked: true Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; x-ms-traffictypediagnostic: AS8PR08MB7718:EE_|VI1PR08MB10049:EE_|DBAEUR03FT012:EE_|VI1PR08MB5358:EE_ X-MS-Office365-Filtering-Correlation-Id: e9e3e908-7ac6-4c57-a565-08db4623e014 x-ld-processed: f34e5979-57d9-4aaa-ad4d-b122a662184d,ExtAddr x-checkrecipientrouted: true nodisclaimer: true X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: flr43ddrg+g5Oe7t7kqKHZ3TgDDkVNHmAuoNCEFrkYnPzNKFAzptjEFd930MylFsmZzmDveBhETGXufaM4hxTgSsArQQm/xEt6Ulo98LWBUoNoehL0/V1rj3Dhd5dPXpd/VJwKqH4hu4d3yaXo5+Dhus69fOl9Led2d2JClS9QCqoxPno9Aai7YLyqkSxZiUrGGI50bd0baOumo2ZOhjffTJhrcVUExcXCXrggCYovVk2YDJc3XuvVwZ7eowJ6947ynmyPnE3f0sCkezF1NFMbC2ZEqimIn0IW4oThoZcdMSAAThG4I7ZyYiExAIJU/023a+hoXAVsqypfxP/S9CFqqn7WspIec71waCnCd4gic0ALgZJKoNaKxV3mZ9h2rO6cGRr2WNuzXeFVx0W25UsUWVYkcs8+IQG0p1IG8P8Dn/1WYeYsfPSkKiqZoOx2xZpu4YpMXJw5s64c6LSKD5P4P9GvXJ+6I5cF1MflxVpw+a16g8wkS+TSY6aJcKe+zxS0S/TJUl8ac1G918k9I9Wm6NsgmSGIfjzJH21/ykTw6UFZQgaV8FaZ08o2yziIvBMVV3BPR2yeBTok7PmjgeuwBY0Y41/iKz9FAxvXbjjuvZHV6oeDYcs4eLsb3EqygE29h4oLTp0wL+BhJ3PK5kAQ== X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:AS8PR08MB7718.eurprd08.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230028)(4636009)(346002)(376002)(396003)(39850400004)(366004)(136003)(451199021)(478600001)(83380400001)(66556008)(55016003)(26005)(9686003)(53546011)(6506007)(71200400001)(7696005)(76116006)(110136005)(66946007)(54906003)(186003)(66574015)(41300700001)(122000001)(38100700002)(52536014)(5660300002)(316002)(4326008)(66446008)(64756008)(86362001)(66476007)(38070700005)(2906002)(30864003)(8676002)(8936002)(33656002)(23180200003); DIR:OUT; SFP:1101; Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR08MB10049 Original-Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: DBAEUR03FT012.eop-EUR03.prod.protection.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: 98d4ab07-d551-433b-0939-08db4623d4e1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: aYgqUeW9+1JvXWZSF8PZN0wdRMTHcCdI3VHnF6bz+dkKTnV7GWjHLSG03ZJ/UysKl08QX9DEoS1Ey2EQOAzQCs4wmjCqPHSusNr+9Z6t6orpKdeCB1nvHkCYkDPl7swzJGOr+O6lHeFplkjy7wUFZ2QlSTbxhkT3ov4MDt3/M5pgwMd1SmUaR4zafjnsjGyLBKGBEg0Eom3JxUSE0AJIpcD3lAACgcMU3LK18myjMugSANwf6hhYN9Qdd5RFmW/7eadLG+hDyLbdyFOV/Pza6OJ76dCwEr5tkM3LyFFEat/Av+HThIaRgAXXpNieOG+jHQjGf8Vx0RF6ZDQwGtUyjMcJyIfxZHp1tZv9WBO3pfAHJ35+JOv1I2S5Ke3enRhZ2I+V6nj9vOgISoTPGPUGgJQcmsjf/ai5UP1ItCSFavMA0NpYFbpiyBYWafCOKeMpvo4caFbRFSJyQhkoW31jwiWjfLI2d60TQ1LYEsoeMWQzaV1eXIjVYvrKAlwRWBf22JVjxi0mz67OTnWoYfQucgfNVdN/p0fnJXyMM+9YDlR++631TOhhgKjE2teOjAQXtpIFBuuW/7ejEFuhE4KAAAwd17OEWmjRcS9FOrWNLPURVtnTDvDAbex+c2LzqdI7786xzKOlsAUK3NdfPhcAHvrfFLJaLgUdGjmR/RUY8XMH0efqUVHMu0KUVjMFK5WNOhpDld6Didipi0KaySwMlHxyvcTo79wXI22lky+rr4IW9txuWOXlLgkMZrN8B+JwgIonx7e7hwJztLGNHDL0Hw== X-Forefront-Antispam-Report: CIP:63.35.35.123; CTRY:IE; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:64aa7808-outbound-1.mta.getcheckrecipient.com; PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com; CAT:NONE; SFS:(13230028)(4636009)(396003)(136003)(376002)(346002)(39850400004)(451199021)(46966006)(36840700001)(40470700004)(7696005)(33656002)(5660300002)(52536014)(40460700003)(82740400003)(30864003)(2906002)(356005)(81166007)(40480700001)(478600001)(55016003)(82310400005)(316002)(41300700001)(336012)(4326008)(70206006)(70586007)(86362001)(186003)(66574015)(47076005)(6506007)(53546011)(36860700001)(9686003)(26005)(54906003)(83380400001)(8936002)(8676002)(34070700002)(110136005)(23180200003); DIR:OUT; SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Apr 2023 07:00:10.0150 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: e9e3e908-7ac6-4c57-a565-08db4623e014 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d; Ip=[63.35.35.123]; Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: DBAEUR03FT012.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR08MB5358 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 > -----Original Message----- > From: Morten Br=F8rup > Sent: Thursday, March 30, 2023 11:59 PM > To: Feifei Wang ; thomas@monjalon.net; Ferruh > Yigit ; Andrew Rybchenko > > Cc: dev@dpdk.org; konstantin.v.ananyev@yandex.ru; nd ; > Honnappa Nagarahalli ; Ruifeng Wang > ; nd > Subject: RE: [PATCH v5 1/3] ethdev: add API for buffer recycle mode >=20 > > From: Morten Br=F8rup > > Sent: Thursday, 30 March 2023 17.15 > > > > > From: Feifei Wang [mailto:Feifei.Wang2@arm.com] > > > Sent: Thursday, 30 March 2023 11.31 > > > > > > > From: Morten Br=F8rup > > > > Sent: Thursday, March 30, 2023 3:19 PM > > > > > > > > > From: Feifei Wang [mailto:feifei.wang2@arm.com] > > > > > Sent: Thursday, 30 March 2023 08.30 > > > > > > > > > > > > > [...] > > > > > > > > > +/** > > > > > + * @internal > > > > > + * Rx routine for rte_eth_dev_buf_recycle(). > > > > > + * Refill Rx descriptors in buffer recycle mode. > > > > > + * > > > > > + * @note > > > > > + * This API can only be called by rte_eth_dev_buf_recycle(). > > > > > + * Before calling this API, rte_eth_tx_buf_stash() should be > > > > > + * called to stash Tx used buffers into Rx buffer ring. > > > > > + * > > > > > + * When this functionality is not implemented in the driver, > > > > > +the return > > > > > + * buffer number is 0. > > > > > + * > > > > > + * @param port_id > > > > > + * The port identifier of the Ethernet device. > > > > > + * @param queue_id > > > > > + * The index of the receive queue. > > > > > + * The value must be in the range [0, nb_rx_queue - 1] previou= sly > > > > supplied > > > > > + * to rte_eth_dev_configure(). > > > > > + *@param nb > > > > > + * The number of Rx descriptors to be refilled. > > > > > + * @return > > > > > + * The number Rx descriptors correct to be refilled. > > > > > + * - ENODEV: bad port or queue (only if compiled with debug). > > > > > > > > If you want errors reported by the return value, the function > > > > return type cannot be uint16_t. > > > Agree. Actually, in the code path, if errors happen, the function > > > will > > return > > > 0. > > > For this description line, I refer to 'rte_eth_tx_prepare' notes. > > > Maybe we should delete this line. > > > > > > > > > > > > + */ > > > > > +static inline uint16_t rte_eth_rx_descriptors_refill(uint16_t po= rt_id, > > > > > + uint16_t queue_id, uint16_t nb) { > > > > > + struct rte_eth_fp_ops *p; > > > > > + void *qd; > > > > > + > > > > > +#ifdef RTE_ETHDEV_DEBUG_RX > > > > > + if (port_id >=3D RTE_MAX_ETHPORTS || > > > > > + queue_id >=3D RTE_MAX_QUEUES_PER_PORT) { > > > > > + RTE_ETHDEV_LOG(ERR, > > > > > + "Invalid port_id=3D%u or queue_id=3D%u\n", > > > > > + port_id, queue_id); > > > > > + rte_errno =3D ENODEV; > > > > > + return 0; > > > > > > > > If p->rx_descriptors_refill() is likely to return 0, this function > > > > should > > > not use 0 > > > > as return value to indicate errors. > > > However, refer to dpdk code style in ethdev, most of API write like t= his. > > > For example, 'rte_eth_rx/tx_burst', 'rte_eth_tx_prep'. > > > > > > I'm also confused what's return type for this due to I want to > > > indicate errors and show the processed buffer number. > > > > OK. Thanks for the references. > > > > Looking at rte_eth_rx/tx_burst(), you could follow the same > > conventions here, > > i.e.: > > - Use uint16_t as return type. > > - Return 0 on error. > > - Do not set rte_errno. > > - Remove the "ENODEV" line from the @return description. > > - Use RTE_ETHDEV_LOG(ERR,...) as the only method to indicate errors. > > > > I now see that you follow the convention of rte_eth_tx_prepare(). This > > is also perfectly fine; then you just need to update the description > > of @return to mention that the error value is set in rte_errno if a > > value less than 'nb' is returned. >=20 > After further consideration, I have changed my mind: >=20 > The primary purpose of rte_eth_tx_prepare() is to test if a packet burst = is valid, > so the ability to return an error value is a natural requirement. >=20 > This is not the purpose your functions. The purpose of your functions > resemble rte_eth_rx/tx_burst(), where there is no requirement to return a= n > error value. So you should follow the convention of rte_eth_rx/tx_burst()= , as I > just suggested. Agree. >=20 > > > > > > > > > > > > > > + } > > > > > +#endif > > > > > + > > > > > + p =3D &rte_eth_fp_ops[port_id]; > > > > > + qd =3D p->rxq.data[queue_id]; > > > > > + > > > > > +#ifdef RTE_ETHDEV_DEBUG_RX > > > > > + if (!rte_eth_dev_is_valid_port(port_id)) { > > > > > + RTE_ETHDEV_LOG(ERR, "Invalid Rx port_id=3D%u\n", > port_id); > > > > > + rte_errno =3D ENODEV; > > > > > + return 0; > > > > > + > > > > > + if (qd =3D=3D NULL) { > > > > > + RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=3D%u for > > > > port_id=3D%u\n", > > > > > + queue_id, port_id); > > > > > + rte_errno =3D ENODEV; > > > > > + return 0; > > > > > + } > > > > > +#endif > > > > > + > > > > > + if (p->rx_descriptors_refill =3D=3D NULL) > > > > > + return 0; > > > > > + > > > > > + return p->rx_descriptors_refill(qd, nb); } > > > > When does p->rx_descriptors_refill() return anything else than 'nb'? > > > > If p->rx_descriptors_refill() always succeeds (and thus always returns > > 'nb'), you could make its return type void. And thus, you could also > > make the return type of rte_eth_rx_descriptors_refill() void. > > > > > > > + > > > > > /**@{@name Rx hardware descriptor states > > > > > * @see rte_eth_rx_descriptor_status > > > > > */ > > > > > @@ -6483,6 +6597,122 @@ rte_eth_tx_buffer(uint16_t port_id, > > > > > uint16_t > > > > queue_id, > > > > > return rte_eth_tx_buffer_flush(port_id, queue_id, buffer); } > > > > > > > > > > +/** > > > > > + * @internal > > > > > + * Tx routine for rte_eth_dev_buf_recycle(). > > > > > + * Stash Tx used buffers into Rx buffer ring in buffer recycle m= ode. > > > > > + * > > > > > + * @note > > > > > + * This API can only be called by rte_eth_dev_buf_recycle(). > > > > > + * After calling this API, rte_eth_rx_descriptors_refill() > > > > > +should be > > > > > + * called to refill Rx ring descriptors. > > > > > + * > > > > > + * When this functionality is not implemented in the driver, > > > > > +the return > > > > > + * buffer number is 0. > > > > > + * > > > > > + * @param port_id > > > > > + * The port identifier of the Ethernet device. > > > > > + * @param queue_id > > > > > + * The index of the transmit queue. > > > > > + * The value must be in the range [0, nb_tx_queue - 1] previou= sly > > > > supplied > > > > > + * to rte_eth_dev_configure(). > > > > > + * @param rxq_buf_recycle_info > > > > > + * A pointer to a structure of Rx queue buffer ring informatio= n in > > > buffer > > > > > + * recycle mode. > > > > > + * > > > > > + * @return > > > > > + * The number buffers correct to be filled in the Rx buffer ri= ng. > > > > > + * - ENODEV: bad port or queue (only if compiled with debug). > > > > > > > > If you want errors reported by the return value, the function > > > > return type cannot be uint16_t. > > > > I now see that you follow the convention of rte_eth_tx_prepare() here t= oo. > > > > This is perfectly fine; then you just need to update the description > > of @return to mention that the error value is set in rte_errno if a > > value less than 'nb' is returned. >=20 > Same comment about conventions as above: This function is more like > rte_eth_rx/tx_burst() than rte_eth_tx_prepare(), so follow the convention= of > rte_eth_rx/tx_burst() instead. >=20 > > > > > > > > > > > + */ > > > > > +static inline uint16_t rte_eth_tx_buf_stash(uint16_t port_id, > > > > > +uint16_t > > > > > queue_id, > > > > > + struct rte_eth_rxq_buf_recycle_info > *rxq_buf_recycle_info) > > > > { > > > > > + struct rte_eth_fp_ops *p; > > > > > + void *qd; > > > > > + > > > > > +#ifdef RTE_ETHDEV_DEBUG_TX > > > > > + if (port_id >=3D RTE_MAX_ETHPORTS || > > > > > + queue_id >=3D RTE_MAX_QUEUES_PER_PORT) { > > > > > + RTE_ETHDEV_LOG(ERR, > > > > > + "Invalid port_id=3D%u or queue_id=3D%u\n", > > > > > + port_id, queue_id); > > > > > + rte_errno =3D ENODEV; > > > > > + return 0; > > > > > > > > If p->tx_buf_stash() is likely to return 0, this function should > > > > not use 0 > > > as > > > > return value to indicate errors. > > > > I now see that you follow the convention of rte_eth_tx_prepare() here t= oo. > > Then please ignore my comment about using 0 as return value on errors > > for this function. >=20 > Same comment about conventions as above: This function is more like > rte_eth_rx/tx_burst() than rte_eth_tx_prepare(), so follow the convention= of > rte_eth_rx/tx_burst() instead. >=20 > > > > > > > > > > > + } > > > > > +#endif > > > > > + > > > > > + p =3D &rte_eth_fp_ops[port_id]; > > > > > + qd =3D p->txq.data[queue_id]; > > > > > + > > > > > +#ifdef RTE_ETHDEV_DEBUG_TX > > > > > + if (!rte_eth_dev_is_valid_port(port_id)) { > > > > > + RTE_ETHDEV_LOG(ERR, "Invalid Tx port_id=3D%u\n", > port_id); > > > > > + rte_errno =3D ENODEV; > > > > > + return 0; > > > > > + > > > > > + if (qd =3D=3D NULL) { > > > > > + RTE_ETHDEV_LOG(ERR, "Invalid Tx queue_id=3D%u for > > > > port_id=3D%u\n", > > > > > + queue_id, port_id); > > > > > + rte_erno =3D ENODEV; > > > > > + return 0; > > > > > + } > > > > > +#endif > > > > > + > > > > > + if (p->tx_buf_stash =3D=3D NULL) > > > > > + return 0; > > > > > + > > > > > + return p->tx_buf_stash(qd, rxq_buf_recycle_info); } > > > > > + > > > > > +/** > > > > > + * @warning > > > > > + * @b EXPERIMENTAL: this API may change, or be removed, without > > > > > +prior notice > > > > > + * > > > > > + * Buffer recycle mode can let Tx queue directly put used > > > > > +buffers into Rx > > > > > buffer > > > > > + * ring. This avoids freeing buffers into mempool and > > > > > + allocating buffers from > > > > > + * mempool. > > > > > > > > This function description generally describes the buffer recycle mo= de. > > > > > > > > Please update it to describe what this specific function does. > > > Ack. > > > > > > > > > + * > > > > > + * @param rx_port_id > > > > > + * Port identifying the receive side. > > > > > + * @param rx_queue_id > > > > > + * The index of the receive queue identifying the receive side= . > > > > > + * The value must be in the range [0, nb_rx_queue - 1] previou= sly > > > > supplied > > > > > + * to rte_eth_dev_configure(). > > > > > + * @param tx_port_id > > > > > + * Port identifying the transmit side. > > > > > + * @param tx_queue_id > > > > > + * The index of the transmit queue identifying the transmit si= de. > > > > > + * The value must be in the range [0, nb_tx_queue - 1] previou= sly > > > > supplied > > > > > + * to rte_eth_dev_configure(). > > > > > + * @param rxq_recycle_info > > > > > + * A pointer to a structure of type *rte_eth_txq_rearm_data* t= o be > > > filled. > > > > > + * @return > > > > > + * - (0) on success or no recycling buffer. > > > > > > > > Why not return the return value of rte_eth_rx_descriptors_refill() > > > > instead > > > of > > > > 0 on success? (This is a question, not a suggestion.) > > > > > > > > Or, if rxq_buf_recycle_info must be valid, the function return > > > > type could > > be > > > > void instead of int. > > > > > > > In some time, users may forget to allocate the room for ' > > rxq_buf_recycle_info > > > ' > > > and call 'rte_rxq_buf_recycle_info_get' API. Thus, I think we need > > > to check with this. > > > > If the user forgets to allocate the rxq_buf_recycle_info, it is a > > serious bug in the user's application. > > > > We don't need to handle such bugs at runtime. > > > > > > > > Furthermore, the return value of this API should return success or no= t. > > > > If rxq_buf_recycle_info is not NULL, this function will always > > succeed. So there is no need for a return value. > > > > If you want this function to return something, it could return nb_buf, > > so the application can it use for telemetry purposes or similar. > > > > > > > > > > + * - (-EINVAL) rxq_recycle_info is NULL. > > > > > + */ > > > > > +__rte_experimental > > > > > +static inline int > > > > > +rte_eth_dev_buf_recycle(uint16_t rx_port_id, uint16_t rx_queue_i= d, > > > > > + uint16_t tx_port_id, uint16_t tx_queue_id, > > > > > + struct rte_eth_rxq_buf_recycle_info > *rxq_buf_recycle_info) > > > > { > > > > > + /* The number of recycling buffers. */ > > > > > + uint16_t nb_buf; > > > > > + > > > > > + if (!rxq_buf_recycle_info) > > > > > + return -EINVAL; > > > > > > > > This is a fast path function. In which situation is this function > > > > called > > > with > > > > rxq_buf_recycle_info =3D=3D NULL? > > > > > > > > If this function can genuinely be called with rxq_buf_recycle_info > > > > =3D=3D > > NULL, > > > > you should test for (rxq_buf_recycle_info =3D=3D NULL), not (! > > > > rxq_buf_recycle_info). Otherwise, I think > > > > RTE_ASSERT(rxq_buf_recycle_info !=3D NULL) is more appropriate. > > > Agree. We should use ' RTE_ASSERT(rxq_buf_recycle_info !=3D NULL)'. > > > > > > > > > + > > > > > + /* Stash Tx used buffers into Rx buffer ring */ > > > > > + nb_buf =3D rte_eth_tx_buf_stash(tx_port_id, tx_queue_id, > > > > > + rxq_buf_recycle_info); > > > > > + /* If there are recycling buffers, refill Rx queue descriptors. > > */ > > > > > + if (nb_buf) > > > > > + rte_eth_rx_descriptors_refill(rx_port_id, rx_queue_id, > > > > > + nb_buf); > > > > > + > > > > > + return 0; > > > > > +} > > > > > + > > > > > /** > > > > > * @warning > > > > > * @b EXPERIMENTAL: this API may change without prior notice > > > > > diff --git a/lib/ethdev/rte_ethdev_core.h > > > > > b/lib/ethdev/rte_ethdev_core.h index dcf8adab92..a138fd4dbc > > > > > 100644 > > > > > --- a/lib/ethdev/rte_ethdev_core.h > > > > > +++ b/lib/ethdev/rte_ethdev_core.h > > > > > @@ -56,6 +56,13 @@ typedef int > > > > > (*eth_rx_descriptor_status_t)(void > > > > > *rxq, uint16_t offset); > > > > > /** @internal Check the status of a Tx descriptor */ typedef > > > > > int (*eth_tx_descriptor_status_t)(void *txq, uint16_t offset); > > > > > > > > > > +/** @internal Stash Tx used buffers into RX ring in buffer > > > > > +recycle mode */ typedef uint16_t (*eth_tx_buf_stash_t)(void *txq= , > > > > > + struct rte_eth_rxq_buf_recycle_info > > *rxq_buf_recycle_info); > > > > > + > > > > > +/** @internal Refill Rx descriptors in buffer recycle mode */ > > > > > +typedef uint16_t (*eth_rx_descriptors_refill_t)(void *rxq, > > > > > +uint16_t nb); > > > > > > > > Please add proper function descriptions for the two callbacks above= . > > > Ack. > > > > > > > > > + > > > > > /** > > > > > * @internal > > > > > * Structure used to hold opaque pointers to internal ethdev > > > > > Rx/Tx @@ > > > > > -90,9 +97,11 @@ struct rte_eth_fp_ops { > > > > > eth_rx_queue_count_t rx_queue_count; > > > > > /** Check the status of a Rx descriptor. */ > > > > > eth_rx_descriptor_status_t rx_descriptor_status; > > > > > + /** Refill Rx descriptors in buffer recycle mode */ > > > > > + eth_rx_descriptors_refill_t rx_descriptors_refill; > > > > > /** Rx queues data. */ > > > > > struct rte_ethdev_qdata rxq; > > > > > - uintptr_t reserved1[3]; > > > > > + uintptr_t reserved1[4]; > > > > > > > > You added a function pointer above, so to keep the structure > > > > alignment, > > you > > > > must remove one here, not add one: > > > > > > > > - uintptr_t reserved1[3]; > > > > + uintptr_t reserved1[2]; > > > > > > > Ack. > > > > > /**@}*/ > > > > > > > > > > /**@{*/ > > > > > @@ -106,9 +115,11 @@ struct rte_eth_fp_ops { > > > > > eth_tx_prep_t tx_pkt_prepare; > > > > > /** Check the status of a Tx descriptor. */ > > > > > eth_tx_descriptor_status_t tx_descriptor_status; > > > > > + /** Stash Tx used buffers into RX ring in buffer recycle mode *= / > > > > > + eth_tx_buf_stash_t tx_buf_stash; > > > > > /** Tx queues data. */ > > > > > struct rte_ethdev_qdata txq; > > > > > - uintptr_t reserved2[3]; > > > > > + uintptr_t reserved2[4]; > > > > > > > > You added a function pointer above, so to keep the structure > > > > alignment, > > you > > > > must remove one here, not add one: > > > > > > > > - uintptr_t reserved1[3]; > > > > + uintptr_t reserved1[2]; > > > > > > > Ack. > > > > > /**@}*/ > > > > > > > > > > } __rte_cache_aligned;