[dpdk-dev] [PATCH v3] net/tap: fix isolation mode toggling

[Ophir Munk]

Running testpmd command "flow isolae <port> 0" (i.e. disabling flow
isolation) followed by command "flow isolate <port> 1" (i.e. enabling
flow isolation) may result in a TAP error:
PMD: Kernel refused TC filter rule creation (17): File exists

Root cause analysis: when disabling flow isolation we keep the local
rule to redirect packets on TX (TAP_REMOTE_TX index) while we add it
again when enabling flow isolation. As a result this rule is added
two times in a row which results in "File exists" error.
The fix is to identify the "File exists" error and silently ignore it.

Another issue occurs when enabling isolation mode several times in a
row in which case the same tc rules are added consecutively and
rte_flow structs are added to a linked list before removing the
previous rte_flow structs.
The fix is to act upon isolation mode command only when there is a
change from "0" to "1" (or vice versa).

Fixes: f503d2694825 ("net/tap: support flow API isolated mode")
Cc: stable at dpdk.org

Reviewed-by: Raslan Darawsheh <rasland at mellanox.com>
Signed-off-by: Ophir Munk <ophirmu at mellanox.com>
---
v1:
initial release
v2:
Please ignore (due to typo errors)
v3:
1. Updates based on Keith Wiles review
2. Do not empty list of implicit TC rules (role back to legacy implementation)
   to ensure TC implicit rules cleanup during implicit rules flushing

 drivers/net/tap/tap_flow.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c
index aab9eef..6b60e6d 100644
--- a/drivers/net/tap/tap_flow.c
+++ b/drivers/net/tap/tap_flow.c
@@ -1568,10 +1568,14 @@ tap_flow_isolate(struct rte_eth_dev *dev,
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
 
+	/* normalize 'set' variable to contain 0 or 1 values */
 	if (set)
-		pmd->flow_isolate = 1;
-	else
-		pmd->flow_isolate = 0;
+		set = 1;
+	/* if already in the right isolation mode - nothing to do */
+	if ((set ^ pmd->flow_isolate) == 0)
+		return 0;
+	/* mark the isolation mode for tap_flow_implicit_create() */
+	pmd->flow_isolate = set;
 	/*
 	 * If netdevice is there, setup appropriate flow rules immediately.
 	 * Otherwise it will be set when bringing up the netdevice (tun_alloc).
@@ -1579,20 +1583,20 @@ tap_flow_isolate(struct rte_eth_dev *dev,
 	if (!pmd->rxq[0].fd)
 		return 0;
 	if (set) {
-		struct rte_flow *flow;
+		struct rte_flow *remote_flow;
 
 		while (1) {
-			flow = LIST_FIRST(&pmd->implicit_flows);
-			if (!flow)
+			remote_flow = LIST_FIRST(&pmd->implicit_flows);
+			if (!remote_flow)
 				break;
 			/*
 			 * Remove all implicit rules on the remote.
 			 * Keep the local rule to redirect packets on TX.
 			 * Keep also the last implicit local rule: ISOLATE.
 			 */
-			if (flow->msg.t.tcm_ifindex == pmd->if_index)
+			if (remote_flow->msg.t.tcm_ifindex == pmd->if_index)
 				break;
-			if (tap_flow_destroy_pmd(pmd, flow, NULL) < 0)
+			if (tap_flow_destroy_pmd(pmd, remote_flow, NULL) < 0)
 				goto error;
 		}
 		/* Switch the TC rule according to pmd->flow_isolate */
@@ -1739,8 +1743,8 @@ int tap_flow_implicit_create(struct pmd_internals *pmd,
 	}
 	err = tap_nl_recv_ack(pmd->nlsk_fd);
 	if (err < 0) {
-		/* Silently ignore re-entering remote promiscuous rule */
-		if (errno == EEXIST && idx == TAP_REMOTE_PROMISC)
+		/* Silently ignore re-entering existing rule */
+		if (errno == EEXIST)
 			goto success;
 		TAP_LOG(ERR,
 			"Kernel refused TC filter rule creation (%d): %s",
-- 
2.7.4