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 AFB52A0C4C; Tue, 5 Oct 2021 20:23:12 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 768C0413D0; Tue, 5 Oct 2021 20:23:12 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 66BAC410E5 for ; Tue, 5 Oct 2021 20:23:11 +0200 (CEST) Received: from [100.65.5.102] (unknown [5.144.120.119]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id CE2037F408; Tue, 5 Oct 2021 21:23:10 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru CE2037F408 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1633458190; bh=Ejq17h9B3kwuSNG5dnZYq8W3rixKed1Slz2RU2P+QBk=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=cnKqr4v7tQF/5IjCaJN/yYlyGX+qaW2C3Wp4HwIy+G7tHenIF2SH3XS5GAWt9hZ8u PKfBDL5kclvPL9cncTUFV3CwO2C4bDHfWShS0cLrxyscF3mFbVs39gbu1GAyAjy13y upBj2aWgb6V7Q0KFNkh4gXpWtlNzacwNa4kxxOk0= To: Thomas Monjalon Cc: dev@dpdk.org, Andrew Rybchenko , Xiaoyun Li , Ori Kam , Ferruh Yigit , Ray Kinsella References: <20210907125157.3843-1-ivan.malov@oktetlabs.ru> <20211005003604.31684-1-ivan.malov@oktetlabs.ru> <8088650.46bvbtDH9c@thomas> From: Ivan Malov Message-ID: Date: Tue, 5 Oct 2021 21:22:59 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <8088650.46bvbtDH9c@thomas> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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" 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". > >> --- 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". > >> + * 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? > >> + * 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. > >> + * >> + * This API is a helper to find such "transfer proxy" for a given ethdev. >> + * >> + * @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"? > >> + * @param[out] proxy_port_id >> + * ID of the "transfer proxy" >> + * >> + * @return >> + * 0 on success, a negative error code otherwise >> + */ >> +__rte_experimental >> +int >> +rte_flow_pick_transfer_proxy(uint16_t port_id, uint16_t *proxy_port_id, >> + struct rte_flow_error *error); > > -- Ivan M