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 22667A04FF; Tue, 24 May 2022 10:18:19 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EF6284067B; Tue, 24 May 2022 10:18:18 +0200 (CEST) Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by mails.dpdk.org (Postfix) with ESMTP id E47F5400D6 for ; Tue, 24 May 2022 10:18:17 +0200 (CEST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id C94FD320091A; Tue, 24 May 2022 04:18:14 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Tue, 24 May 2022 04:18:15 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm1; t=1653380294; x= 1653466694; bh=eEofN0tsUPow8J6OaPH67dWa0LQBpQfw+V5TKUaKpW0=; b=a oQYwFXbi2pJ/oaQf5bYUtF/YVJyBLr31yJoy7XCgYgTr2cSfHNNR09B6XfiJY3jI 5KS5pcKUPuXD0LPvXWK9Jr/Rl8pg+jRuyr/MHKSO1WAjkGS40QuVx9A+jR0T0Dl4 VyLk1UC4eKuH6VlZJhgZJJqYgARbEo7w36C0AIwtMO5DBTv//Vtuio9NXiHm1qWT AMgPsrxpfwJcd95zja0boF9KQgScIk/+NU8F87uQH4ChSbm7m9lgW/ygD48qwBS4 /aZNQJ/UZ8tf9aK+GJij/3s+tmVNG/e5LvtCkEAvHBdUcV85bu5wCmWaMYKyfLQT xiwe+69Hcokahh7fXR57A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1653380294; x= 1653466694; bh=eEofN0tsUPow8J6OaPH67dWa0LQBpQfw+V5TKUaKpW0=; b=p AxLXfKyMH9asp+bInm6hnada6YwatfhfXyineCSGknbw2TwV9+mvLWz9xtHorc3J kSbImw1gzbg8i7zIeU6eCwxmNQqco+pWGpT7l1LiAaIX8ff+JE4QA7zcRkrWi0k0 ddNAFGUJZGu30vPJzoFgiF7SDE+CHVWRmJ3FoDsPrpLplcZn77aQmIvTPS9eimsv mgRp0zN626bQHBW1ICA/qknfxo3oTlYVYEB1nvaWwuqarChHmXpa8Hil7wayUcO3 tUZYINiYjhrZ887uGBjzYnsPlCuSdHoLRZuuiq7fXjuFNrmHRhfsKfdvGiBABU48 5w8FVmsrGKoaSH8pKMgwA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrjeefgddtudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvfevufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnheptdejieeifeehtdffgfdvleetueeffeehueejgfeuteeftddtieek gfekudehtdfgnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 24 May 2022 04:18:11 -0400 (EDT) From: Thomas Monjalon To: Stephen Hemminger , Spike Du Cc: "dev@dpdk.org" , Matan Azrad , Slava Ovsiienko , Ori Kam , Raslan Darawsheh , "ferruh.yigit@amd.com" , "andrew.rybchenko@oktetlabs.ru" Subject: Re: [RFC v2 3/7] ethdev: introduce Rx queue based limit watermark Date: Tue, 24 May 2022 10:18:02 +0200 Message-ID: <31797171.aRNtrjHk3s@thomas> In-Reply-To: References: <20220506035645.4101714-1-spiked@nvidia.com> <1836405.jbyF5MZJ3u@thomas> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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 24/05/2022 04:50, Spike Du: > From: Thomas Monjalon > > 23/05/2022 05:01, Spike Du: > > > From: Stephen Hemminger > > > > Spike Du wrote: > > > > > --- a/lib/ethdev/rte_ethdev.h > > > > > +++ b/lib/ethdev/rte_ethdev.h > > > > > @@ -1249,7 +1249,16 @@ struct rte_eth_rxconf { > > > > > */ > > > > > union rte_eth_rxseg *rx_seg; > > > > > > > > > > - uint64_t reserved_64s[2]; /**< Reserved for future fields */ > > > > > + /** > > > > > + * Per-queue Rx limit watermark defined as percentage of Rx queue > > > > > + * size. If Rx queue receives traffic higher than this percentage, > > > > > + * the event RTE_ETH_EVENT_RX_LWM is triggered. > > > > > + */ > > > > > + uint8_t lwm; > > > > > + > > > > > + uint8_t reserved_bits[3]; > > > > > + uint32_t reserved_32s; > > > > > + uint64_t reserved_64s; > > > > > > > > Ok but, this is an ABI risk about this because reserved stuff was > > > > never required before. > > > > An ABI compatibility issue would be for an application compiled with an old > > DPDK, and loading a new DPDK at runtime. > > Let's think what would happen in such a case. > > > > > > Whenever is a reserved field is introduced the code (in this case > > > > rte_ethdev_configure). > > > > rte_eth_rx_queue_setup() is called with rx_conf->lwm not initialized. > > Then the library and drivers may interpret a wrong value. > > > > > > Best practice would have been to have the code require all reserved > > > > fields be > > > > 0 in earlier releases. In this case an application is like to define > > > > a watermark of zero; how will your code handle it. > > > > > > Having watermark of 0 is desired, which is the default. LWM of 0 means > > > the Rx Queue's watermark is not monitored, hence no LWM event is > > generated. > > > > The problem is to have a value not initialized. > > I think the best approach is to not expose the LWM value through this > > configuration structure. > > If the need is to get the current value, we should better add a field in the > > struct rte_eth_rxq_info. > > At least from all the dpdk app/example code, rxconf is initialized to 0 then setup > The Rx queue, if user follows these examples we should not have ABI issue. > Since many people are concerned about rxconf change, it's ok to remove the LWM > Field there. > Yes, I think we can add lwm into rte_eth_rxq_info. If we can set Rx queue's attribute, > We should have a way to get it. Unfortunately we cannot rely on examples for ABI compatibility. My suggestion of moving the field in rte_eth_rxq_info is not obvious because it could change the size of the struct. But thanks to __rte_cache_min_aligned, it is OK. Running pahole on this struct shows we have 50 bytes free: /* size: 128, cachelines: 2, members: 6 */ /* padding: 50 */ The other option would be to get the LWM value with a "get" function. What others prefer?