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 <ivan.ma...@oktetlabs.ru> > Reviewed-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru> I have to review it better before, but it is better to do it later than never. > Acked-by: Ori Kam <or...@nvidia.com> > --- > 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
