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.malov@oktetlabs.ru>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
I have to review it better before, but it is better to do it
later than never.
> Acked-by: Ori Kam <orika@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
@@ -1505,10 +1505,25 @@ port_action_handle_create(portid_t port_id, uint32_t id,
struct port_indirect_action *pia;
int ret;
struct rte_flow_error error;
+ struct rte_port *port;
+
+ if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+ port_id == (portid_t)RTE_PORT_ALL)
+ return -EINVAL;
ret = action_alloc(port_id, id, &pia);
if (ret)
return ret;
+
+ port = &ports[port_id];
+
+ if (conf->transfer)
+ port_id = port->flow_transfer_proxy;
+
+ if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+ port_id == (portid_t)RTE_PORT_ALL)
+ return -EINVAL;
+
if (action->type == RTE_FLOW_ACTION_TYPE_AGE) {
struct rte_flow_action_age *age =
(struct rte_flow_action_age *)(uintptr_t)(action->conf);
@@ -1531,6 +1546,7 @@ port_action_handle_create(portid_t port_id, uint32_t id,
return port_flow_complain(&error);
}
pia->type = action->type;
+ pia->transfer = conf->transfer;
printf("Indirect action #%u created\n", pia->id);
return 0;
}
@@ -1557,9 +1573,18 @@ port_action_handle_destroy(portid_t port_id,
for (i = 0; i != n; ++i) {
struct rte_flow_error error;
struct port_indirect_action *pia = *tmp;
+ portid_t port_id_eff = port_id;
if (actions[i] != pia->id)
continue;
+
+ if (pia->transfer)
+ port_id_eff = port->flow_transfer_proxy;
+
+ if (port_id_is_invalid(port_id_eff, ENABLED_WARN) ||
+ port_id_eff == (portid_t)RTE_PORT_ALL)
+ return -EINVAL;
+
/*
* Poisoning to make sure PMDs update it in case
* of error.
@@ -1567,7 +1592,7 @@ port_action_handle_destroy(portid_t port_id,
memset(&error, 0x33, sizeof(error));
if (pia->handle && rte_flow_action_handle_destroy(
- port_id, pia->handle, &error)) {
+ port_id_eff, pia->handle, &error)) {
ret = port_flow_complain(&error);
continue;
}
@@ -1602,8 +1627,15 @@ port_action_handle_update(portid_t port_id, uint32_t id,
struct rte_flow_error error;
struct rte_flow_action_handle *action_handle;
struct port_indirect_action *pia;
+ struct rte_port *port;
const void *update;
+ if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+ port_id == (portid_t)RTE_PORT_ALL)
+ return -EINVAL;
+
+ port = &ports[port_id];
+
action_handle = port_action_handle_get_by_id(port_id, id);
if (!action_handle)
return -EINVAL;
@@ -1618,6 +1650,14 @@ port_action_handle_update(portid_t port_id, uint32_t id,
update = action;
break;
}
+
+ if (pia->transfer)
+ port_id = port->flow_transfer_proxy;
+
+ if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+ port_id == (portid_t)RTE_PORT_ALL)
+ return -EINVAL;
+
if (rte_flow_action_handle_update(port_id, action_handle, update,
&error)) {
return port_flow_complain(&error);
@@ -1636,6 +1676,14 @@ port_action_handle_query(portid_t port_id, uint32_t id)
struct rte_flow_query_age age;
struct rte_flow_action_conntrack ct;
} query;
+ portid_t port_id_eff = port_id;
+ struct rte_port *port;
+
+ if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+ port_id == (portid_t)RTE_PORT_ALL)
+ return -EINVAL;
+
+ port = &ports[port_id];
pia = action_get_by_id(port_id, id);
if (!pia)
@@ -1650,10 +1698,19 @@ port_action_handle_query(portid_t port_id, uint32_t id)
id, pia->type, port_id);
return -ENOTSUP;
}
+
+ if (pia->transfer)
+ port_id_eff = port->flow_transfer_proxy;
+
+ if (port_id_is_invalid(port_id_eff, ENABLED_WARN) ||
+ port_id_eff == (portid_t)RTE_PORT_ALL)
+ return -EINVAL;
+
/* Poisoning to make sure PMDs update it in case of error. */
memset(&error, 0x55, sizeof(error));
memset(&query, 0, sizeof(query));
- if (rte_flow_action_handle_query(port_id, pia->handle, &query, &error))
+ if (rte_flow_action_handle_query(port_id_eff, pia->handle, &query,
+ &error))
return port_flow_complain(&error);
switch (pia->type) {
case RTE_FLOW_ACTION_TYPE_AGE:
@@ -1872,6 +1929,20 @@ port_flow_validate(portid_t port_id,
{
struct rte_flow_error error;
struct port_flow_tunnel *pft = NULL;
+ struct rte_port *port;
+
+ if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+ port_id == (portid_t)RTE_PORT_ALL)
+ return -EINVAL;
+
+ port = &ports[port_id];
+
+ if (attr->transfer)
+ port_id = port->flow_transfer_proxy;
+
+ if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+ port_id == (portid_t)RTE_PORT_ALL)
+ return -EINVAL;
/* Poisoning to make sure PMDs update it in case of error. */
memset(&error, 0x11, sizeof(error));
@@ -1925,7 +1996,19 @@ port_flow_create(portid_t port_id,
struct port_flow_tunnel *pft = NULL;
struct rte_flow_action_age *age = age_action_get(actions);
+ if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+ port_id == (portid_t)RTE_PORT_ALL)
+ return -EINVAL;
+
port = &ports[port_id];
+
+ if (attr->transfer)
+ port_id = port->flow_transfer_proxy;
+
+ if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+ port_id == (portid_t)RTE_PORT_ALL)
+ return -EINVAL;
+
if (port->flow_list) {
if (port->flow_list->id == UINT32_MAX) {
fprintf(stderr,
@@ -1989,6 +2072,7 @@ port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule)
uint32_t i;
for (i = 0; i != n; ++i) {
+ portid_t port_id_eff = port_id;
struct rte_flow_error error;
struct port_flow *pf = *tmp;
@@ -1999,7 +2083,15 @@ port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule)
* of error.
*/
memset(&error, 0x33, sizeof(error));
- if (rte_flow_destroy(port_id, pf->flow, &error)) {
+
+ if (pf->rule.attr->transfer)
+ port_id_eff = port->flow_transfer_proxy;
+
+ if (port_id_is_invalid(port_id_eff, ENABLED_WARN) ||
+ port_id_eff == (portid_t)RTE_PORT_ALL)
+ return -EINVAL;
+
+ if (rte_flow_destroy(port_id_eff, pf->flow, &error)) {
ret = port_flow_complain(&error);
continue;
}
@@ -2133,6 +2225,14 @@ port_flow_query(portid_t port_id, uint32_t rule,
fprintf(stderr, "Flow rule #%u not found\n", rule);
return -ENOENT;
}
+
+ if (pf->rule.attr->transfer)
+ port_id = port->flow_transfer_proxy;
+
+ if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+ port_id == (portid_t)RTE_PORT_ALL)
+ return -EINVAL;
+
ret = rte_flow_conv(RTE_FLOW_CONV_OP_ACTION_NAME_PTR,
&name, sizeof(name),
(void *)(uintptr_t)action->type, &error);
@@ -533,6 +533,25 @@ int proc_id;
*/
unsigned int num_procs = 1;
+static void
+flow_pick_transfer_proxy_mp(uint16_t port_id)
+{
+ struct rte_port *port = &ports[port_id];
+ int ret;
+
+ port->flow_transfer_proxy = port_id;
+
+ if (!is_proc_primary())
+ return;
+
+ ret = rte_flow_pick_transfer_proxy(port_id, &port->flow_transfer_proxy,
+ NULL);
+ if (ret != 0) {
+ fprintf(stderr, "Error picking flow transfer proxy for port %u: %s - ignore\n",
+ port_id, rte_strerror(-ret));
+ }
+}
+
static int
eth_dev_configure_mp(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
const struct rte_eth_conf *dev_conf)
@@ -1489,6 +1508,8 @@ init_config_port_offloads(portid_t pid, uint32_t socket_id)
int ret;
int i;
+ flow_pick_transfer_proxy_mp(pid);
+
port->dev_conf.txmode = tx_mode;
port->dev_conf.rxmode = rx_mode;
@@ -173,6 +173,8 @@ struct port_indirect_action {
enum rte_flow_action_type type; /**< Action type. */
struct rte_flow_action_handle *handle; /**< Indirect action handle. */
enum age_action_context_type age_type; /**< Age action context type. */
+ /** If true, the action applies to "transfer" flows, and vice versa */
+ bool transfer;
};
struct port_flow_tunnel {
@@ -234,6 +236,8 @@ struct rte_port {
/**< dynamic flags. */
uint64_t mbuf_dynf;
const struct rte_eth_rxtx_callback *tx_set_dynf_cb[RTE_MAX_QUEUES_PER_PORT+1];
+ /** Associated port which is supposed to handle "transfer" flows */
+ portid_t flow_transfer_proxy;
};
/**
@@ -98,13 +98,16 @@ dump_pkt_burst(uint16_t port_id, uint16_t queue, struct rte_mbuf *pkts[],
int ret;
struct rte_flow_error error;
struct rte_flow_restore_info info = { 0, };
+ struct rte_port *port = &ports[port_id];
mb = pkts[i];
eth_hdr = rte_pktmbuf_read(mb, 0, sizeof(_eth_hdr), &_eth_hdr);
eth_type = RTE_BE_TO_CPU_16(eth_hdr->ether_type);
packet_type = mb->packet_type;
is_encapsulation = RTE_ETH_IS_TUNNEL_PKT(packet_type);
- ret = rte_flow_get_restore_info(port_id, mb, &info, &error);
+
+ ret = rte_flow_get_restore_info(port->flow_transfer_proxy,
+ mb, &info, &error);
if (!ret) {
MKDUMPSTR(print_buf, buf_size, cur_len,
"restore info:");
@@ -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**
+ A new API, ``rte_flow_pick_transfer_proxy()``, was added.
+
* **Updated Broadcom bnxt PMD.**
* Added flow offload support for Thor.
@@ -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];
+ 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)) {
+ return flow_err(port_id,
+ ops->pick_transfer_proxy(dev, proxy_port_id,
+ error),
+ error);
+ }
+
+ *proxy_port_id = port_id;
+
+ return 0;
+}
@@ -122,6 +122,9 @@ struct rte_flow_attr {
*
* In order to match traffic originating from specific source(s), the
* application should use pattern items ETHDEV and ESWITCH_PORT.
+ *
+ * Managing "transfer" flows requires that the user communicate them
+ * through a suitable port. @see rte_flow_pick_transfer_proxy().
*/
uint32_t transfer:1;
uint32_t reserved:29; /**< Reserved, must be zero. */
@@ -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);
+
+/**
+ * Get a proxy port to manage "transfer" (e-switch) flows.
+ *
+ * Managing "transfer" flows requires that the user communicate them
+ * through a port which has the privilege to control the e-switch.
+ * For some vendors, all ports in a given e-switch domain have
+ * 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.
+ *
+ * @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
+ *
+ * @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
@@ -139,6 +139,11 @@ struct rte_flow_ops {
struct rte_flow_item *pmd_items,
uint32_t num_of_items,
struct rte_flow_error *err);
+ /** See rte_flow_pick_transfer_proxy() */
+ int (*pick_transfer_proxy)
+ (struct rte_eth_dev *dev,
+ uint16_t *proxy_port_id,
+ struct rte_flow_error *error);
};
/**
@@ -247,6 +247,9 @@ EXPERIMENTAL {
rte_mtr_meter_policy_delete;
rte_mtr_meter_policy_update;
rte_mtr_meter_policy_validate;
+
+ # added in 21.11
+ rte_flow_pick_transfer_proxy;
};
INTERNAL {