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 5E10BA0C45; Wed, 6 Oct 2021 09:47:49 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4DE474114E; Wed, 6 Oct 2021 09:47:49 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 68E024114E for ; Wed, 6 Oct 2021 09:47:48 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (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 DFDAB7F50A; Wed, 6 Oct 2021 10:47:47 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru DFDAB7F50A DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1633506467; bh=NV1GPX9rCiPnWdR+hILb7FxEoUYuwFuU5LcwwoAqe30=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=udSvtElEhT1AwWVyQoJ+D5LcC9fnJXjSWELOM4LzHi3g8IueCsSoEvIffgnHWk/JU ADikXG1mMJn/yfEhuU/GbhLjRQ3Jh30zBdAl+2x0fGAECXAuFPLPGNlOwzFMbLQBP9 4XSEqJkg1icAVd6KCRoeHCE/hVZkZpYaA8OA04kc= To: Ivan Malov , dev@dpdk.org Cc: Thomas Monjalon , Ori Kam , Xiaoyun Li , Ferruh Yigit , Ray Kinsella References: <20211005003604.31684-1-ivan.malov@oktetlabs.ru> <20211005211045.29403-1-ivan.malov@oktetlabs.ru> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: Date: Wed, 6 Oct 2021 10:47:47 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: <20211005211045.29403-1-ivan.malov@oktetlabs.ru> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2] ethdev: add API to query proxy port to manage transfer flows 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" On 10/6/21 12:10 AM, Ivan Malov wrote: > Not all DPDK ports in a given e-switch domain may have the Search in the Internet does not provide e-switch as a shorter form of "embedded switch". So, I agree with Thomas, it is better to avoid it in the documentation. Especially taking into account that there is a company with a similar name. I.e. e-switch -> embedded switch > privilege to manage "transfer" flows. Add an API to find a > port with sufficient privileges by any port in the domain. > > Signed-off-by: Ivan Malov > Reviewed-by: Andrew Rybchenko I have to review it better before, but it is better to do it later than never. > Acked-by: Ori Kam > --- > Patch series [1] has reworked support for "transfer" flows. > This allows to elaborate on the idea which first appeared > in RFC [2]. Hence the patch in question. > > net/sfc driver is going to support the new API. The > corresponding patch is already in progress and will > be provided in the course of this release cycle. > > [1] https://patches.dpdk.org/project/dpdk/list/?series=19326 > [2] https://patches.dpdk.org/project/dpdk/list/?series=18737 [snip] > diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst > index 0787baed8c..5de30fad9c 100644 > --- a/doc/guides/rel_notes/release_21_11.rst > +++ b/doc/guides/rel_notes/release_21_11.rst > @@ -67,6 +67,9 @@ New Features > Added macros ETH_RSS_IPV4_CHKSUM and ETH_RSS_L4_CHKSUM, now IPv4 and > TCP/UDP/SCTP header checksum field can be used as input set for RSS. > > +* **Added an API to get a proxy port to manage "transfer" (e-switch) flows** Let's avoid (e-switch) here. I guess initial definition of the transfer intentionally avoid the term. > + A new API, ``rte_flow_pick_transfer_proxy()``, was added. > + > * **Updated Broadcom bnxt PMD.** > > * Added flow offload support for Thor. > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c > index 647bbf91ce..15e978f7f7 100644 > --- a/lib/ethdev/rte_flow.c > +++ b/lib/ethdev/rte_flow.c > @@ -1270,3 +1270,25 @@ rte_flow_tunnel_item_release(uint16_t port_id, > RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > NULL, rte_strerror(ENOTSUP)); > } > + > +int > +rte_flow_pick_transfer_proxy(uint16_t port_id, uint16_t *proxy_port_id, > + struct rte_flow_error *error) > +{ > + struct rte_eth_dev *dev = &rte_eth_devices[port_id]; Let's avoid initialization here since 'port_id' is not checked yet to be correct. Let's assign dev below just before usage. > + const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error); > + > + if (unlikely(ops == NULL)) > + return -rte_errno; > + > + if (likely(ops->pick_transfer_proxy != NULL)) { Yes, I see that it is the stile in the file to expect non-NULL callback, it is a bit unfair to say so when just driver implements it. So, I suggest to remove it. Also it is not an error or exception. It is documented behaviour. > + return flow_err(port_id, > + ops->pick_transfer_proxy(dev, proxy_port_id, > + error), > + error); > + } > + > + *proxy_port_id = port_id; I think code will have less lines and easier to read if we revert above if condition, do the assignment and return 0 from its body. > + > + return 0; > +} > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h > index f195aa7224..5405e2565a 100644 > --- a/lib/ethdev/rte_flow.h > +++ b/lib/ethdev/rte_flow.h [snip] > @@ -4427,6 +4430,34 @@ rte_flow_tunnel_item_release(uint16_t port_id, > struct rte_flow_item *items, > uint32_t num_of_items, > struct rte_flow_error *error); > + > +/** Don't we need experimental header here? > + * Get a proxy port to manage "transfer" (e-switch) flows. I suggest to have no (e-switch) here as well. "transfer" is sufficient. > + * > + * Managing "transfer" flows requires that the user communicate them > + * through a port which has the privilege to control the e-switch. e-switch -> embedded switch > + * For some vendors, all ports in a given e-switch domain have e-switch -> embedded switch, or "switching domain" > + * this privilege. For other vendors, it's only one port. > + * > + * This API indicates such a privileged port (a "proxy") > + * for a given port in the same e-switch domain. e-switch -> embedded switch, or "switching domain" > + * > + * @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 > + * Indicates the port to get a "proxy" for > + * @param[out] proxy_port_id > + * Indicates the "proxy" port I should notice it earlier, but 'error' parameter description is missing here. > + * > + * @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); > #ifdef __cplusplus > } > #endif