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 4C845A0C45; Tue, 5 Oct 2021 21:04:15 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 11C2441262; Tue, 5 Oct 2021 21:04:15 +0200 (CEST) Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com [66.111.4.29]) by mails.dpdk.org (Postfix) with ESMTP id 8628C410E5 for ; Tue, 5 Oct 2021 21:04:13 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id E0B455C02CD; Tue, 5 Oct 2021 15:04:11 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Tue, 05 Oct 2021 15:04:11 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm2; bh= LST4IA59eLlf6c5WF3OX8JrqMZNku0lkP1BOEC048oc=; b=m1GXa5nQ2+i8R/gL 53druSVtMdcrjl6n0LVhDtIkaPKF9SXWUnrZ6GNcqjEoFbZWicyZhR8tMhhbzwzf Ctb0MkRfzH4uNRPKZXvasKpqZVB/FGh6HwfnM3acIdo9LQKhde1z343OmicMWWO0 PMw5XY16PP8NNzT7QsxJsZgNhJz14W5M+1sJ4tA9WEgKgdgVwh58S9Kpa99IEKaM sVaTMLIOQQmWQ9e35RbQMBQkLd0pM+B/WxmyaDDKeo0uRuYPaxWrISXrsRKPEGzj HKzOMYQeosbu8kuDhsI9eIqT/xo5mbAqrx3KhXbDZfWhCUi5uT3pB3RV/o8YSEGO mtj5qA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=LST4IA59eLlf6c5WF3OX8JrqMZNku0lkP1BOEC048 oc=; b=nvdeRKcEwbzMT1XbwNhBKkaxMtkPhMyrT4yXl3CL2zujRkNrX2liJYZsH QDFd+sVQlcOxUsy/25ezsntUdDojNYiWjT3g6fA3SxTgp1byqlg2/aMxfiPqfwOj DuXfdmsVwnN2Qagcz8+kfoHco2HKu0aIDmAU40apyVzZvk62Vv5UmOPJ8cNIiWwH lfT38ctXbqi6JVzZKc8RC/Me3MYN4oBDcL8iJwt94bCfLfDuTdRmeTE55xq+cfoN oCgXAQ6WKQOJr5ZBpWmdm4KvX67M2ePgmae6XukmH18uFxguF5hdgfwBDYQMTWRE Q3eyBWYATkQ9Qegp5LTL6v1TvOfQg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrudelgedgudefvdcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc ggtffrrghtthgvrhhnpedugefgvdefudfftdefgeelgffhueekgfffhfeujedtteeutdej ueeiiedvffegheenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfh hrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 5 Oct 2021 15:04:09 -0400 (EDT) From: Thomas Monjalon To: Ivan Malov Cc: dev@dpdk.org, Andrew Rybchenko , Xiaoyun Li , Ori Kam , Ferruh Yigit , Ray Kinsella Date: Tue, 05 Oct 2021 21:04:06 +0200 Message-ID: <2245996.DLApj8tiry@thomas> In-Reply-To: References: <20210907125157.3843-1-ivan.malov@oktetlabs.ru> <8088650.46bvbtDH9c@thomas> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH] ethdev: let apps find transfer admin port for a given ethdev 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" 05/10/2021 20:22, Ivan Malov: > Hi Thomas, > > Thanks for joining the review. > > On 05/10/2021 20:56, Thomas Monjalon wrote: > > 05/10/2021 02:36, Ivan Malov: > >> Introduce a helper API to let applications find transfer > >> admin port for a given ethdev to avoid failures when > >> managing "transfer" flows through unprivileged ports. > > > > Please explain what is transfer admin port. > > We may find a simpler wording. > > It's an ethdev which has the privilege to control the embedded switch. > Typically, it's a PF ethdev. > > Flows with "transfer" attribute apply to the e-switch level. So > "transfer admin port" reads the same as "e-switch admin port". Please explain this in the v2. > >> --- a/lib/ethdev/rte_flow.h > >> +++ b/lib/ethdev/rte_flow.h > >> + * > >> + * Communicating "transfer" flows via unprivileged ethdevs may not > >> + * be possible. In order to pick the ethdev suitable for that, the > > > > ethdev -> port > > Why? "Ethdev" is a clearer term for "port". Because the API mention ports, not ethdevs. Example: port_id, not ethdev_id. > >> + * application should use @p rte_flow_pick_transfer_proxy(). > >> */ > >> uint32_t transfer:1; > > [...] > >> +/** > > > > Please insert an one-line description here. > > Do you want me to make the first line of the description stand out? Like > a brief summary / title? Yes like a title. A proposal: "Get the proxy port able to manage transfer rules in e-switch." > >> + * An application receiving packets on a given ethdev may want to have their > >> + * processing offloaded to the e-switch lying beneath this ethdev by means > > > > Is "e-switch" a common word? Or should we say "embedded switch"? > > Term "e-switch" is a contraction for "embedded switch". I'd go for the > short version. > > >> + * of maintaining "transfer" flows. However, it need never use this exact > >> + * ethdev as an entry point for such flows to be managed through. More to > >> + * that, this particular ethdev may be short of privileges to control the > >> + * e-switch. Instead, the application should find an admin ethdev sitting > >> + * on top of the same e-switch to be used as the entry point (a "proxy"). > > > > I recognize the nice right-alignment, but I think this text can be shorter. > > No right-alignment here: the last line in the block is not aligned. I > wasn't looking to make this paragraph shorter or longer. I was looking > to introduce the problem clearly. If you believe that I failed to do so, > could you please highlight the parts which sound misleading to you. The words misleading me are: "application", "lying beneath", "never", "sitting on top", "entry point". I prefer not talking about application, and avoid story telling style. > >> + * > >> + * This API is a helper to find such "transfer proxy" for a given ethdev. I suggest something like this: " The transfer flow must be managed by privileged ports. For some devices, all ports are privileged, while some allow only one port to control the e-switch. This function returns a port (proxy) in the same e-switch domain, usable for managing transfer rules. " > >> + * > >> + * @note > >> + * If the PMD serving @p port_id doesn't have the corresponding method > >> + * implemented, the API will return @p port_id via @p proxy_port_id. > >> + * > >> + * @param port_id > >> + * ID of the ethdev in question > > > > The rest of the API says "port", not "ethdev". > > Here I would suggest "ID of the port to look from." > > The argument name is "port_id". This is for consistency with other API > signatures across DPDK. But, in comments, I'd stick with "ethdev". It's > clearer than "port". > > How about "ID of the ethdev to find the proxy for"? I like it except "ethdev" :)