From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 260C6A00C2;
	Mon, 23 May 2022 23:45:56 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id C07474067B;
	Mon, 23 May 2022 23:45:55 +0200 (CEST)
Received: from wout4-smtp.messagingengine.com (wout4-smtp.messagingengine.com
 [64.147.123.20]) by mails.dpdk.org (Postfix) with ESMTP id 02DFF40156
 for <dev@dpdk.org>; Mon, 23 May 2022 23:45:53 +0200 (CEST)
Received: from compute5.internal (compute5.nyi.internal [10.202.2.45])
 by mailout.west.internal (Postfix) with ESMTP id D680C32009E3;
 Mon, 23 May 2022 17:45:51 -0400 (EDT)
Received: from mailfrontend2 ([10.202.2.163])
 by compute5.internal (MEProxy); Mon, 23 May 2022 17:45:52 -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=1653342351; x=
 1653428751; bh=BdhzbBjqe0C9yGHVnzf+nqMFne+fNaVwH+WudWLp4Vo=; b=u
 Bq7BjmyLecil0FUQ93db0lJqdBpRjhdjuwparoTBxaUwdW3QtXjjXrRgkRdTNuIq
 70EGsJAk9XKZ5aMnFCxsUYk3YRfmOjbDasaLqm4bJVGsXQ0QEW3UBAuFnTZPet9+
 vG4gZTvoZDfzfPdlmk8saYozRvKW9i3PxtL87m+3kFyv4n26UyQUSa9rpq/59Kgv
 g3YT6DFCj/a1BokBg9Hssupp935BXDJJx7kywm6cIcesgphYbvQXj/qOHnLN2Y4L
 UD9Yi/MEH8PspQDMNYTnoEC+gfKOIYK+3FBUTgT47nRfE6yyQJyQJpXEU1goa3cx
 bi0fSX50+CAb2eAvnrP5A==
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=1653342351; x=
 1653428751; bh=BdhzbBjqe0C9yGHVnzf+nqMFne+fNaVwH+WudWLp4Vo=; b=I
 FU7LOKxV8df55mgfrzlrbdTsHwJUMNQvwKn3GlxYmltkXwcNpTw5/22c6y1ApaDq
 7mf93SpATkYLmSeGWzYulgUCWaHVn5Eoufgmv4wRJzrTGlFwbc7vJpAnfJ078BfD
 qlHCuNl9uJxdclSianpB6K23BbFqfwOEYeLlcHoM6xMVwSWno3kEsKBo/9ns/kJd
 jiPBgzha6JVqeua+fKCcphkT/lGcSBY/Zfn3N7/2LC7U9W++I5IREdbJEUSFiH9j
 HTxS7tKdF66KnRXTjGhxTcr8Ri6ZKXUZ0wIn+6OvD98TkUaP68HqsGJ4QtvPrUL1
 JMB+Vt1aY/MejlvY8N2YQ==
X-ME-Sender: <xms:jgCMYguWjnx06NTVrHcBKthUrM5S1dtbTwT3mnE_DIVQtFrqZwQovg>
 <xme:jgCMYtfgnUck3XiKTUDb98yTA3-GNU9peiLPApagP38XvMnCR-R9hXvPLGfENSSI1
 9u6Vax1CHRI3okMvw>
X-ME-Received: <xmr:jgCMYrw9zgXY-DR2UgtS0amFs74QMoLr9cpgrjDGCv3oWrSn4kt5S8xv6jGYBSNhWi7HiYcTNjj7RRO0NQ>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrjedvgddtvdcutefuodetggdotefrodftvf
 curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu
 uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc
 fjughrpefhvfevufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr
 shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg
 ftrfgrthhtvghrnheptdejieeifeehtdffgfdvleetueeffeehueejgfeuteeftddtieek
 gfekudehtdfgnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh
 homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth
X-ME-Proxy: <xmx:jgCMYjOdevrihBGNeOpxBcy06arrv_c4wv4yANeB5pvC0NdVdfRzQQ>
 <xmx:jgCMYg-FuDDEN4sBNvssB6oL5_MsY4yYjYQID5Ar6DeARHKkvgXXPw>
 <xmx:jgCMYrW91rI4SszelpUV2eZV22Jx4w-fP43bMNQguv6I4LBAeErtWg>
 <xmx:jwCMYtNOrd5hDUu83Rdyhb9ZR7rUixsiww68U8Er5GksTejFRec-hg>
Feedback-ID: i47234305:Fastmail
Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon,
 23 May 2022 17:45:47 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: Stephen Hemminger <stephen@networkplumber.org>,
 Spike Du <spiked@nvidia.com>
Cc: dev@dpdk.org, Matan Azrad <matan@nvidia.com>,
 Slava Ovsiienko <viacheslavo@nvidia.com>, Ori Kam <orika@nvidia.com>,
 Raslan Darawsheh <rasland@nvidia.com>, ferruh.yigit@amd.com,
 andrew.rybchenko@oktetlabs.ru
Subject: Re: [RFC v2 3/7] ethdev: introduce Rx queue based limit watermark
Date: Mon, 23 May 2022 23:45:38 +0200
Message-ID: <1836405.jbyF5MZJ3u@thomas>
In-Reply-To: <MN2PR12MB3647125617695A255DB326DCA8D49@MN2PR12MB3647.namprd12.prod.outlook.com>
References: <20220506035645.4101714-1-spiked@nvidia.com>
 <20220522082321.3cdb7693@hermes.local>
 <MN2PR12MB3647125617695A255DB326DCA8D49@MN2PR12MB3647.namprd12.prod.outlook.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

23/05/2022 05:01, Spike Du:
> From: Stephen Hemminger <stephen@networkplumber.org>
> > Spike Du <spiked@nvidia.com> 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.

[...]
> 
> > Why introduce new query/set operations? This should just be part of the
> > overall device configuration.

Thanks to the "set" function, we can avoid the ABI compat issue.

> Due to different implementation. LWM can be a dynamic configuration which can help user design a flexible flow control.
> User may feel ok with LWM of 80% to get high throughput, or later on with 50% to throttle the traffic responsively by handling LWM event in order to reduce drop.
> Some driver like mlx5 may implement LWM event as one-time shot. When you receive LWM event, you need to reconfigure LWM in order to receive the event again, thus you will
> not likely to be overwhelmed by the events.
> These all require set operation.

Yes it is better to allow dynamic watermark configuration,
not using the function rte_eth_rx_queue_setup().

> For the query operation. The rte_event API rte_eth_dev_callback_process() is per-port API, it doesn't carry much information when an event happens.
> When a LWM event happens, we need to know in which Rx queue it happens or optionally what's the current LWM percentage of this queue.
> The query operation serves this purpose.

Yes "query" has to be called in the event handler
because event structure is not specific to any event type.