Commit [1] introduced a flag in dpif-netdev level, to optimize
performance and avoid hw_miss_packet_recover() for devices with no such
support.
However, there is a race condition between traffic processing and
assigning a 'flow_api' object to the netdev. In such case, EOPNOTSUPP is
returned by netdev_hw_miss_packet_recover() in netdev-offload.c layer
because 'flow_api' is not yet initialized. As a result, the flag is
falsely disabled, and subsequent packets won't be recovered, though they
should.
In order to fix it, move the flag to be in netdev-offload layer, to
avoid that race.
[1]: 6e50c1651869 ("dpif-netdev: Avoid hw_miss_packet_recover() for devices
with no support.")
Fixes: 6e50c1651869 ("dpif-netdev: Avoid hw_miss_packet_recover() for devices
with no support.")
Signed-off-by: Eli Britstein <el...@nvidia.com>
---
lib/dpif-netdev.c | 15 ++++-----------
lib/netdev-offload.c | 25 ++++++++++++++++++++-----
lib/netdev-offload.h | 1 +
3 files changed, 25 insertions(+), 16 deletions(-)
Github actions:
- v1: https://github.com/elibritstein/OVS/actions/runs/2587743230
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index f46b9fe183..a286050b57 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -431,7 +431,6 @@ struct dp_netdev_rxq {
unsigned intrvl_idx; /* Write index for 'cycles_intrvl'. */
struct dp_netdev_pmd_thread *pmd; /* pmd thread that polls this queue. */
bool is_vhost; /* Is rxq of a vhost port. */
- bool hw_miss_api_supported; /* hw_miss_packet_recover() supported.*/
/* Counters of cycles spent successfully polling and processing pkts. */
atomic_ullong cycles[RXQ_N_CYCLES];
@@ -5421,7 +5420,6 @@ port_reconfigure(struct dp_netdev_port *port)
port->rxqs[i].port = port;
port->rxqs[i].is_vhost = !strncmp(port->type, "dpdkvhost", 9);
- port->rxqs[i].hw_miss_api_supported = true;
err = netdev_rxq_open(netdev, &port->rxqs[i].rx, i);
if (err) {
@@ -8039,16 +8037,11 @@ dp_netdev_hw_flow(const struct dp_netdev_pmd_thread
*pmd,
/* Restore the packet if HW processing was terminated before completion. */
struct dp_netdev_rxq *rxq = pmd->ctx.last_rxq;
- if (rxq->hw_miss_api_supported) {
+ if (rxq->port->netdev->hw_info.miss_api_supported) {
int err = netdev_hw_miss_packet_recover(rxq->port->netdev, packet);
- if (err) {
- if (err != EOPNOTSUPP) {
- COVERAGE_INC(datapath_drop_hw_miss_recover);
- return -1;
- } else {
- /* API unsupported by the port; avoid subsequent calls. */
- rxq->hw_miss_api_supported = false;
- }
+ if (err && err != EOPNOTSUPP) {
+ COVERAGE_INC(datapath_drop_hw_miss_recover);
+ return -1;
}
}
#endif
diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
index fb108c0d50..50c5076a63 100644
--- a/lib/netdev-offload.c
+++ b/lib/netdev-offload.c
@@ -182,6 +182,7 @@ netdev_assign_flow_api(struct netdev *netdev)
CMAP_FOR_EACH (rfa, cmap_node, &netdev_flow_apis) {
if (!rfa->flow_api->init_flow_api(netdev)) {
ovs_refcount_ref(&rfa->refcnt);
+ netdev->hw_info.miss_api_supported = true;
ovsrcu_set(&netdev->flow_api, rfa->flow_api);
VLOG_INFO("%s: Assigned flow API '%s'.",
netdev_get_name(netdev), rfa->flow_api->type);
@@ -190,6 +191,7 @@ netdev_assign_flow_api(struct netdev *netdev)
VLOG_DBG("%s: flow API '%s' is not suitable.",
netdev_get_name(netdev), rfa->flow_api->type);
}
+ netdev->hw_info.miss_api_supported = false;
VLOG_INFO("%s: No suitable flow API found.", netdev_get_name(netdev));
return -1;
@@ -263,12 +265,25 @@ int
netdev_hw_miss_packet_recover(struct netdev *netdev,
struct dp_packet *packet)
{
- const struct netdev_flow_api *flow_api =
- ovsrcu_get(const struct netdev_flow_api *, &netdev->flow_api);
+ const struct netdev_flow_api *flow_api;
+ int rv;
+
+ if (!netdev->hw_info.miss_api_supported) {
+ return EOPNOTSUPP;
+ }
+
+ flow_api = ovsrcu_get(const struct netdev_flow_api *, &netdev->flow_api);
+ if (!flow_api || !flow_api->hw_miss_packet_recover) {
+ return EOPNOTSUPP;
+ }
+
+ rv = flow_api->hw_miss_packet_recover(netdev, packet);
+ if (rv == EOPNOTSUPP) {
+ /* API unsupported by the port; avoid subsequent calls. */
+ netdev->hw_info.miss_api_supported = false;
+ }
- return (flow_api && flow_api->hw_miss_packet_recover)
- ? flow_api->hw_miss_packet_recover(netdev, packet)
- : EOPNOTSUPP;
+ return rv;
}
int
diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h
index 8237a85ddb..c56f9d49ad 100644
--- a/lib/netdev-offload.h
+++ b/lib/netdev-offload.h
@@ -45,6 +45,7 @@ struct ovs_action_push_tnl;
/* Offload-capable (HW) netdev information */
struct netdev_hw_info {
bool oor; /* Out of Offload Resources ? */
+ bool miss_api_supported; /* hw_miss_packet_recover() supported.*/
int offload_count; /* Pending (non-offloaded) flow count */
int pending_count; /* Offloaded flow count */
OVSRCU_TYPE(void *) offload_data; /* Offload metadata. */
--
2.26.2.1730.g385c171
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
