diff mbox series

[ovs-dev,V3,1/2] netdev-offload-dpdk: Fix flushing of a physdev

Message ID 20230611155827.1957023-1-elibr@nvidia.com
State Accepted
Commit 0aeb06e1fa7e2b97d338e317eb9ee63c28b8490b
Headers show
Series [ovs-dev,V3,1/2] netdev-offload-dpdk: Fix flushing of a physdev | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Eli Britstein June 11, 2023, 3:58 p.m. UTC
Vport's offloads are done on the tracked orig-in-port, but the flow itself
is associated in the vport's map.

Removing the physdev will flush all the ports that are on its map, but
not the ones on other netdevs' maps. Since flows take reference count on
both their vport and their physdev, the physdev still has references on.
Trying to remove it and re-add it fails with "already in use" error.

Fix it by flushing the physdev's offload flows in all related netdevs,
e.g. the netdev itself, or for physical devices, all vports.

Fixes: adbd4301a249 ("netdev-offload-dpdk: Use per-netdev offload metadata.")
Reported-by: 15895987278 <wuxi_seu@163.com>
Signed-off-by: Eli Britstein <elibr@nvidia.com>
---
 lib/netdev-offload-dpdk.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

Comments

Simon Horman June 29, 2023, 2:08 p.m. UTC | #1
On Sun, Jun 11, 2023 at 06:58:26PM +0300, Eli Britstein via dev wrote:
> Vport's offloads are done on the tracked orig-in-port, but the flow itself
> is associated in the vport's map.
> 
> Removing the physdev will flush all the ports that are on its map, but
> not the ones on other netdevs' maps. Since flows take reference count on
> both their vport and their physdev, the physdev still has references on.
> Trying to remove it and re-add it fails with "already in use" error.
> 
> Fix it by flushing the physdev's offload flows in all related netdevs,
> e.g. the netdev itself, or for physical devices, all vports.
> 
> Fixes: adbd4301a249 ("netdev-offload-dpdk: Use per-netdev offload metadata.")
> Reported-by: 15895987278 <wuxi_seu@163.com>
> Signed-off-by: Eli Britstein <elibr@nvidia.com>

Acked-by: Simon Horman <simon.horman@corigine.com>
Ilya Maximets Oct. 9, 2023, 9:57 p.m. UTC | #2
On 6/29/23 16:08, Simon Horman wrote:
> On Sun, Jun 11, 2023 at 06:58:26PM +0300, Eli Britstein via dev wrote:
>> Vport's offloads are done on the tracked orig-in-port, but the flow itself
>> is associated in the vport's map.
>>
>> Removing the physdev will flush all the ports that are on its map, but
>> not the ones on other netdevs' maps. Since flows take reference count on
>> both their vport and their physdev, the physdev still has references on.
>> Trying to remove it and re-add it fails with "already in use" error.
>>
>> Fix it by flushing the physdev's offload flows in all related netdevs,
>> e.g. the netdev itself, or for physical devices, all vports.
>>
>> Fixes: adbd4301a249 ("netdev-offload-dpdk: Use per-netdev offload metadata.")
>> Reported-by: 15895987278 <wuxi_seu@163.com>
>> Signed-off-by: Eli Britstein <elibr@nvidia.com>
> 
> Acked-by: Simon Horman <simon.horman@corigine.com>
> 

Thanks, Eli and Simon!  And David for checking the previous versions!

I applied this one patch for now.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 14bc87771..992627fa2 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -2537,15 +2537,15 @@  out:
     return ret;
 }
 
-static int
-netdev_offload_dpdk_flow_flush(struct netdev *netdev)
+static void
+flush_netdev_flows_in_related(struct netdev *netdev, struct netdev *related)
 {
-    struct cmap *map = offload_data_map(netdev);
-    struct ufid_to_rte_flow_data *data;
     unsigned int tid = netdev_offload_thread_id();
+    struct cmap *map = offload_data_map(related);
+    struct ufid_to_rte_flow_data *data;
 
     if (!map) {
-        return -1;
+        return;
     }
 
     CMAP_FOR_EACH (data, node, map) {
@@ -2556,6 +2556,31 @@  netdev_offload_dpdk_flow_flush(struct netdev *netdev)
             netdev_offload_dpdk_flow_destroy(data);
         }
     }
+}
+
+static bool
+flush_in_vport_cb(struct netdev *vport,
+                  odp_port_t odp_port OVS_UNUSED,
+                  void *aux)
+{
+    struct netdev *netdev = aux;
+
+    /* Only vports are related to physical devices. */
+    if (netdev_vport_is_vport_class(vport->netdev_class)) {
+        flush_netdev_flows_in_related(netdev, vport);
+    }
+
+    return false;
+}
+
+static int
+netdev_offload_dpdk_flow_flush(struct netdev *netdev)
+{
+    flush_netdev_flows_in_related(netdev, netdev);
+
+    if (!netdev_vport_is_vport_class(netdev->netdev_class)) {
+        netdev_ports_traverse(netdev->dpif_type, flush_in_vport_cb, netdev);
+    }
 
     return 0;
 }