diff options
author | Eli Britstein <elibr@nvidia.com> | 2022-08-31 12:59:55 +0300 |
---|---|---|
committer | Ilya Maximets <i.maximets@ovn.org> | 2022-10-25 21:35:51 +0200 |
commit | 76ab364ea8facd73366411916d7d0f5ff611daed (patch) | |
tree | ae2dfb392293d0bc4ece105b46a5f7a84198764e /lib | |
parent | 31db0e043119cf597d720d94f70ec19cf5b8b7d4 (diff) | |
download | openvswitch-76ab364ea8facd73366411916d7d0f5ff611daed.tar.gz |
netdev-offload: Set 'miss_api_supported' to be under netdev.
Cited commit 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.
Fixes: 6e50c1651869 ("dpif-netdev: Avoid hw_miss_packet_recover() for devices with no support.")
Signed-off-by: Eli Britstein <elibr@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Diffstat (limited to 'lib')
-rw-r--r-- | lib/dpif-netdev.c | 18 | ||||
-rw-r--r-- | lib/netdev-offload.c | 28 | ||||
-rw-r--r-- | lib/netdev-offload.h | 2 | ||||
-rw-r--r-- | lib/netdev.c | 1 |
4 files changed, 33 insertions, 16 deletions
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index a45b46014..2c08a71c8 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]; @@ -5416,7 +5415,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) { @@ -8034,17 +8032,15 @@ dp_netdev_hw_flow(const struct dp_netdev_pmd_thread *pmd, #ifdef ALLOW_EXPERIMENTAL_API /* Packet restoration API required. */ /* Restore the packet if HW processing was terminated before completion. */ struct dp_netdev_rxq *rxq = pmd->ctx.last_rxq; + bool miss_api_supported; - if (rxq->hw_miss_api_supported) { + atomic_read_relaxed(&rxq->port->netdev->hw_info.miss_api_supported, + &miss_api_supported); + if (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 9fde5f7a9..4592262bd 100644 --- a/lib/netdev-offload.c +++ b/lib/netdev-offload.c @@ -183,6 +183,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); + atomic_store_relaxed(&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); @@ -191,6 +192,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); } + atomic_store_relaxed(&netdev->hw_info.miss_api_supported, false); VLOG_INFO("%s: No suitable flow API found.", netdev_get_name(netdev)); return -1; @@ -322,12 +324,28 @@ 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; + bool miss_api_supported; + int rv; + + atomic_read_relaxed(&netdev->hw_info.miss_api_supported, + &miss_api_supported); + if (!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. */ + atomic_store_relaxed(&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 180d3f95f..edc843cd9 100644 --- a/lib/netdev-offload.h +++ b/lib/netdev-offload.h @@ -20,6 +20,7 @@ #include "openvswitch/netdev.h" #include "openvswitch/types.h" +#include "ovs-atomic.h" #include "ovs-rcu.h" #include "ovs-thread.h" #include "openvswitch/ofp-meter.h" @@ -46,6 +47,7 @@ struct ovs_action_push_tnl; /* Offload-capable (HW) netdev information */ struct netdev_hw_info { bool oor; /* Out of Offload Resources ? */ + atomic_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. */ diff --git a/lib/netdev.c b/lib/netdev.c index ce0d4117a..c79778378 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -431,6 +431,7 @@ netdev_open(const char *name, const char *type, struct netdev **netdevp) seq_read(netdev->reconfigure_seq); ovsrcu_set(&netdev->flow_api, NULL); netdev->hw_info.oor = false; + atomic_init(&netdev->hw_info.miss_api_supported, false); netdev->node = shash_add(&netdev_shash, name, netdev); /* By default enable one tx and rx queue per netdev. */ |