[ovs-dev] [PATCH v7 1/2] netdev-dpdk: negotiate delivery of per-packet Rx metadata
This may be required by some PMDs in offload scenarios.
Signed-off-by: Ivan Malov
---
lib/netdev-dpdk.c | 51 +++
1 file changed, 51 insertions(+)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 63dac689e..d9d1b43f6 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -492,6 +492,9 @@ struct netdev_dpdk {
/* Array of vhost rxq states, see vring_state_changed. */
bool *vhost_rxq_enabled;
+
+/* Ensures that Rx metadata delivery is configured only once. */
+bool rx_metadata_delivery_configured;
);
PADDED_MEMBERS(CACHE_LINE_SIZE,
@@ -1187,6 +1190,42 @@ dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev)
OVS_REQUIRES(dev->mutex)
}
}
+static void
+dpdk_eth_dev_init_rx_metadata(struct netdev_dpdk *dev)
+{
+uint64_t rx_metadata = 0;
+int ret;
+
+if (dev->rx_metadata_delivery_configured) {
+return;
+}
+
+/* For the fallback offload (non-"transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_USER_MARK;
+/* For the full offload ("transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_TUNNEL_ID;
+
+ret = rte_eth_rx_metadata_negotiate(dev->port_id, &rx_metadata);
+if (ret == 0) {
+if (!(rx_metadata & RTE_ETH_RX_METADATA_USER_MARK)) {
+VLOG_DBG("The NIC will not provide per-packet USER_MARK on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+if (!(rx_metadata & RTE_ETH_RX_METADATA_TUNNEL_ID)) {
+VLOG_DBG("The NIC will not provide per-packet TUNNEL_ID on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+} else if (ret == -ENOTSUP) {
+VLOG_DBG("Rx metadata negotiate procedure is not supported on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+} else {
+VLOG_WARN("Cannot negotiate Rx metadata on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+
+dev->rx_metadata_delivery_configured = true;
+}
+
static int
dpdk_eth_dev_init(struct netdev_dpdk *dev)
OVS_REQUIRES(dev->mutex)
@@ -1200,6 +1239,16 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
+/*
+ * Full tunnel offload requires that tunnel ID metadata be
+ * delivered with "miss" packets from the hardware to the
+ * PMD. The same goes for megaflow mark metadata which is
+ * used in MARK + RSS offload scenario.
+ *
+ * Request delivery of such metadata.
+ */
+dpdk_eth_dev_init_rx_metadata(dev);
+
rte_eth_dev_info_get(dev->port_id, &info);
if (strstr(info.driver_name, "vf") != NULL) {
@@ -1382,6 +1431,8 @@ common_construct(struct netdev *netdev, dpdk_port_t
port_no,
/* Initilize the hardware offload flags to 0 */
dev->hw_ol_features = 0;
+dev->rx_metadata_delivery_configured = false;
+
dev->flags = NETDEV_UP | NETDEV_PROMISC;
ovs_list_push_back(&dpdk_list, &dev->list_node);
--
2.39.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v7 1/2] netdev-dpdk: negotiate delivery of per-packet Rx metadata
On 6/30/23 04:46, Ivan Malov wrote:
> This may be required by some PMDs in offload scenarios.
>
> Signed-off-by: Ivan Malov
Hi, Ivan. Thanks for the patch!
I suppose, it can be considered as a bug fix. Could you add
a Fixes tag to the commit message?
> ---
> lib/netdev-dpdk.c | 51 +++
> 1 file changed, 51 insertions(+)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 63dac689e..d9d1b43f6 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -492,6 +492,9 @@ struct netdev_dpdk {
>
> /* Array of vhost rxq states, see vring_state_changed. */
> bool *vhost_rxq_enabled;
> +
> +/* Ensures that Rx metadata delivery is configured only once. */
> +bool rx_metadata_delivery_configured;
> );
>
> PADDED_MEMBERS(CACHE_LINE_SIZE,
> @@ -1187,6 +1190,42 @@ dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev)
> OVS_REQUIRES(dev->mutex)
> }
> }
>
> +static void
> +dpdk_eth_dev_init_rx_metadata(struct netdev_dpdk *dev)
> +{
> +uint64_t rx_metadata = 0;
> +int ret;
> +
> +if (dev->rx_metadata_delivery_configured) {
> +return;
> +}
> +
> +/* For the fallback offload (non-"transfer" rules) */
> +rx_metadata |= RTE_ETH_RX_METADATA_USER_MARK;
> +/* For the full offload ("transfer" rules) */
This doesn't seem to be correct. This flag is only needed for
tunnel offload. Should also be under experimental api ifdef?
> +rx_metadata |= RTE_ETH_RX_METADATA_TUNNEL_ID;
> +
> +ret = rte_eth_rx_metadata_negotiate(dev->port_id, &rx_metadata);
> +if (ret == 0) {
> +if (!(rx_metadata & RTE_ETH_RX_METADATA_USER_MARK)) {
> +VLOG_DBG("The NIC will not provide per-packet USER_MARK on port "
> + DPDK_PORT_ID_FMT, dev->port_id);
> +}
> +if (!(rx_metadata & RTE_ETH_RX_METADATA_TUNNEL_ID)) {
> +VLOG_DBG("The NIC will not provide per-packet TUNNEL_ID on port "
> + DPDK_PORT_ID_FMT, dev->port_id);
> +}
> +} else if (ret == -ENOTSUP) {
> +VLOG_DBG("Rx metadata negotiate procedure is not supported on port "
> + DPDK_PORT_ID_FMT, dev->port_id);
> +} else {
> +VLOG_WARN("Cannot negotiate Rx metadata on port "
> + DPDK_PORT_ID_FMT, dev->port_id);
For all the logs above, use the following format instead:
VLOG_*("%s: ...", netdev_get_name(&dev->up));
For the last two, you may use:
VLOG(ret == -ENOTSUP ? VLL_DBG : VLL_WARN,
"%s: Cannot negotiate Rx metadata: %s",
netdev_get_name(&dev->up), rte_strerror(-ret));
> +}
> +
> +dev->rx_metadata_delivery_configured = true;
> +}
> +
> static int
> dpdk_eth_dev_init(struct netdev_dpdk *dev)
> OVS_REQUIRES(dev->mutex)
> @@ -1200,6 +1239,16 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
> RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
> RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
>
> +/*
> + * Full tunnel offload requires that tunnel ID metadata be
> + * delivered with "miss" packets from the hardware to the
> + * PMD. The same goes for megaflow mark metadata which is
> + * used in MARK + RSS offload scenario.
> + *
> + * Request delivery of such metadata.
> + */
> +dpdk_eth_dev_init_rx_metadata(dev);
It's a bit strange that we request metadata regardless of hw-offload
being enabled. Should this be under netdev_is_flow_api_enabled() ?
> +
> rte_eth_dev_info_get(dev->port_id, &info);
>
> if (strstr(info.driver_name, "vf") != NULL) {
> @@ -1382,6 +1431,8 @@ common_construct(struct netdev *netdev, dpdk_port_t
> port_no,
> /* Initilize the hardware offload flags to 0 */
> dev->hw_ol_features = 0;
>
> +dev->rx_metadata_delivery_configured = false;
> +
> dev->flags = NETDEV_UP | NETDEV_PROMISC;
>
> ovs_list_push_back(&dpdk_list, &dev->list_node);
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v7 1/2] netdev-dpdk: negotiate delivery of per-packet Rx metadata
Hi Ilya,
Thanks for reviewing this. PSB.
On Fri, 14 Jul 2023, Ilya Maximets wrote:
On 6/30/23 04:46, Ivan Malov wrote:
This may be required by some PMDs in offload scenarios.
Signed-off-by: Ivan Malov
Hi, Ivan. Thanks for the patch!
I suppose, it can be considered as a bug fix. Could you add
a Fixes tag to the commit message?
---
lib/netdev-dpdk.c | 51 +++
1 file changed, 51 insertions(+)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 63dac689e..d9d1b43f6 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -492,6 +492,9 @@ struct netdev_dpdk {
/* Array of vhost rxq states, see vring_state_changed. */
bool *vhost_rxq_enabled;
+
+/* Ensures that Rx metadata delivery is configured only once. */
+bool rx_metadata_delivery_configured;
);
PADDED_MEMBERS(CACHE_LINE_SIZE,
@@ -1187,6 +1190,42 @@ dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev)
OVS_REQUIRES(dev->mutex)
}
}
+static void
+dpdk_eth_dev_init_rx_metadata(struct netdev_dpdk *dev)
+{
+uint64_t rx_metadata = 0;
+int ret;
+
+if (dev->rx_metadata_delivery_configured) {
+return;
+}
+
+/* For the fallback offload (non-"transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_USER_MARK;
+/* For the full offload ("transfer" rules) */
This doesn't seem to be correct. This flag is only needed for
tunnel offload. Should also be under experimental api ifdef?
My guess, it's comment which is incorrect. I'm going to fix it, yes.
But, regarding the experimental api ifdef, according to review [1]
from Eli, it might be unneeded. Furthermore, the dpdk api being
invoked is no longer experimental. What do you think?
[1]
https://patchwork.ozlabs.org/project/openvswitch/patch/20220720121823.2497727-4-ivan.ma...@oktetlabs.ru/#2935847
+rx_metadata |= RTE_ETH_RX_METADATA_TUNNEL_ID;
+
+ret = rte_eth_rx_metadata_negotiate(dev->port_id, &rx_metadata);
+if (ret == 0) {
+if (!(rx_metadata & RTE_ETH_RX_METADATA_USER_MARK)) {
+VLOG_DBG("The NIC will not provide per-packet USER_MARK on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+if (!(rx_metadata & RTE_ETH_RX_METADATA_TUNNEL_ID)) {
+VLOG_DBG("The NIC will not provide per-packet TUNNEL_ID on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+} else if (ret == -ENOTSUP) {
+VLOG_DBG("Rx metadata negotiate procedure is not supported on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+} else {
+VLOG_WARN("Cannot negotiate Rx metadata on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
For all the logs above, use the following format instead:
VLOG_*("%s: ...", netdev_get_name(&dev->up));
For the last two, you may use:
VLOG(ret == -ENOTSUP ? VLL_DBG : VLL_WARN,
"%s: Cannot negotiate Rx metadata: %s",
netdev_get_name(&dev->up), rte_strerror(-ret));
+}
+
+dev->rx_metadata_delivery_configured = true;
+}
+
static int
dpdk_eth_dev_init(struct netdev_dpdk *dev)
OVS_REQUIRES(dev->mutex)
@@ -1200,6 +1239,16 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
+/*
+ * Full tunnel offload requires that tunnel ID metadata be
+ * delivered with "miss" packets from the hardware to the
+ * PMD. The same goes for megaflow mark metadata which is
+ * used in MARK + RSS offload scenario.
+ *
+ * Request delivery of such metadata.
+ */
+dpdk_eth_dev_init_rx_metadata(dev);
It's a bit strange that we request metadata regardless of hw-offload
being enabled. Should this be under netdev_is_flow_api_enabled() ?
+
rte_eth_dev_info_get(dev->port_id, &info);
if (strstr(info.driver_name, "vf") != NULL) {
@@ -1382,6 +1431,8 @@ common_construct(struct netdev *netdev, dpdk_port_t
port_no,
/* Initilize the hardware offload flags to 0 */
dev->hw_ol_features = 0;
+dev->rx_metadata_delivery_configured = false;
+
dev->flags = NETDEV_UP | NETDEV_PROMISC;
ovs_list_push_back(&dpdk_list, &dev->list_node);
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v7 1/2] netdev-dpdk: negotiate delivery of per-packet Rx metadata
Hi Ilya,
A quick follow-up from me: I made a new version of this patch [1].
[1]
https://patchwork.ozlabs.org/project/openvswitch/patch/20230716115720.6789-1-ivan.ma...@arknetworks.am/
Thank you.
On Fri, 14 Jul 2023, Ilya Maximets wrote:
On 6/30/23 04:46, Ivan Malov wrote:
This may be required by some PMDs in offload scenarios.
Signed-off-by: Ivan Malov
Hi, Ivan. Thanks for the patch!
I suppose, it can be considered as a bug fix. Could you add
a Fixes tag to the commit message?
---
lib/netdev-dpdk.c | 51 +++
1 file changed, 51 insertions(+)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 63dac689e..d9d1b43f6 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -492,6 +492,9 @@ struct netdev_dpdk {
/* Array of vhost rxq states, see vring_state_changed. */
bool *vhost_rxq_enabled;
+
+/* Ensures that Rx metadata delivery is configured only once. */
+bool rx_metadata_delivery_configured;
);
PADDED_MEMBERS(CACHE_LINE_SIZE,
@@ -1187,6 +1190,42 @@ dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev)
OVS_REQUIRES(dev->mutex)
}
}
+static void
+dpdk_eth_dev_init_rx_metadata(struct netdev_dpdk *dev)
+{
+uint64_t rx_metadata = 0;
+int ret;
+
+if (dev->rx_metadata_delivery_configured) {
+return;
+}
+
+/* For the fallback offload (non-"transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_USER_MARK;
+/* For the full offload ("transfer" rules) */
This doesn't seem to be correct. This flag is only needed for
tunnel offload. Should also be under experimental api ifdef?
+rx_metadata |= RTE_ETH_RX_METADATA_TUNNEL_ID;
+
+ret = rte_eth_rx_metadata_negotiate(dev->port_id, &rx_metadata);
+if (ret == 0) {
+if (!(rx_metadata & RTE_ETH_RX_METADATA_USER_MARK)) {
+VLOG_DBG("The NIC will not provide per-packet USER_MARK on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+if (!(rx_metadata & RTE_ETH_RX_METADATA_TUNNEL_ID)) {
+VLOG_DBG("The NIC will not provide per-packet TUNNEL_ID on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+} else if (ret == -ENOTSUP) {
+VLOG_DBG("Rx metadata negotiate procedure is not supported on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+} else {
+VLOG_WARN("Cannot negotiate Rx metadata on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
For all the logs above, use the following format instead:
VLOG_*("%s: ...", netdev_get_name(&dev->up));
For the last two, you may use:
VLOG(ret == -ENOTSUP ? VLL_DBG : VLL_WARN,
"%s: Cannot negotiate Rx metadata: %s",
netdev_get_name(&dev->up), rte_strerror(-ret));
+}
+
+dev->rx_metadata_delivery_configured = true;
+}
+
static int
dpdk_eth_dev_init(struct netdev_dpdk *dev)
OVS_REQUIRES(dev->mutex)
@@ -1200,6 +1239,16 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
+/*
+ * Full tunnel offload requires that tunnel ID metadata be
+ * delivered with "miss" packets from the hardware to the
+ * PMD. The same goes for megaflow mark metadata which is
+ * used in MARK + RSS offload scenario.
+ *
+ * Request delivery of such metadata.
+ */
+dpdk_eth_dev_init_rx_metadata(dev);
It's a bit strange that we request metadata regardless of hw-offload
being enabled. Should this be under netdev_is_flow_api_enabled() ?
+
rte_eth_dev_info_get(dev->port_id, &info);
if (strstr(info.driver_name, "vf") != NULL) {
@@ -1382,6 +1431,8 @@ common_construct(struct netdev *netdev, dpdk_port_t
port_no,
/* Initilize the hardware offload flags to 0 */
dev->hw_ol_features = 0;
+dev->rx_metadata_delivery_configured = false;
+
dev->flags = NETDEV_UP | NETDEV_PROMISC;
ovs_list_push_back(&dpdk_list, &dev->list_node);
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
