diff mbox series

app/testpmd: cleanup port resources after implicit close

Message ID 20220615231212.44122-1-dkozlyuk@nvidia.com (mailing list archive)
State Accepted, archived
Delegated to: Andrew Rybchenko
Headers
Series app/testpmd: cleanup port resources after implicit close |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/github-robot: build success github build: passed
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Dmitry Kozlyuk June 15, 2022, 11:12 p.m. UTC 
  When a port was closed implicitly closed by the PMD, for example,
if it was a representor port and its master port was detached,
flow indirect actions could remain with their handles no longer valid.
If a newly attached device was assigned the same ID as the closed port,
those indirect actions became accessible again.
Any attempt to use them resulted in an undefined behavior.
Flow flex items had no such issue on close, but had it on detach.

Introduce flush_port_owned_resources() function
for consistent cleanup and call it when a port is closed or detached.
Make it flush flow rules and multicast addresses too
because they logically belong to the port being removed.

Fixes: 55509e3a49fb ("app/testpmd: support shared flow action")
Fixes: 59f3a8acbcdb ("app/testpmd: add flex item commands")
Cc: stable@dpdk.org

Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 app/test-pmd/testpmd.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

Andrew Rybchenko June 23, 2022, 11:17 a.m. UTC | #1 
On 6/16/22 02:12, Dmitry Kozlyuk wrote:
> When a port was closed implicitly closed by the PMD, for example,
> if it was a representor port and its master port was detached,
> flow indirect actions could remain with their handles no longer valid.
> If a newly attached device was assigned the same ID as the closed port,
> those indirect actions became accessible again.
> Any attempt to use them resulted in an undefined behavior.
> Flow flex items had no such issue on close, but had it on detach.
> 
> Introduce flush_port_owned_resources() function
> for consistent cleanup and call it when a port is closed or detached.
> Make it flush flow rules and multicast addresses too
> because they logically belong to the port being removed.
> 
> Fixes: 55509e3a49fb ("app/testpmd: support shared flow action")
> Fixes: 59f3a8acbcdb ("app/testpmd: add flex item commands")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

Resolved merge conflict on apply to dpdk-next-net/main,
applied, thanks.
diff mbox series

Patch

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 04c39adc21..fe125e40e8 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3202,6 +3202,15 @@  remove_invalid_ports(void)
 	nb_cfg_ports = nb_fwd_ports;
 }
 
+static void
+flush_port_owned_resources(portid_t pi)
+{
+	mcast_addr_pool_destroy(pi);
+	port_flow_flush(pi);
+	port_flex_item_flush(pi);
+	port_action_handle_flush(pi);
+}
+
 void
 close_port(portid_t pid)
 {
@@ -3238,10 +3247,7 @@  close_port(portid_t pid)
 		}
 
 		if (is_proc_primary()) {
-			mcast_addr_pool_destroy(pi);
-			port_flow_flush(pi);
-			port_flex_item_flush(pi);
-			port_action_handle_flush(pi);
+			flush_port_owned_resources(pi);
 			rte_eth_dev_close(pi);
 		}
 
@@ -3386,7 +3392,7 @@  detach_device(struct rte_device *dev)
 					sibling);
 				return;
 			}
-			port_flow_flush(sibling);
+			flush_port_owned_resources(sibling);
 		}
 	}
 
@@ -3453,7 +3459,7 @@  detach_devargs(char *identifier)
 				rte_devargs_reset(&da);
 				return;
 			}
-			port_flow_flush(port_id);
+			flush_port_owned_resources(port_id);
 		}
 	}