All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ophir Munk <ophirmu@mellanox.com>
To: dev@dpdk.org, Pascal Mazon <pascal.mazon@6wind.com>
Cc: Thomas Monjalon <thomas@monjalon.net>,
	Olga Shern <olgas@mellanox.com>,
	Ophir Munk <ophirmu@mellanox.com>,
	stable@dpdk.org
Subject: [PATCH v1] net/tap: fix isolation mode toggling
Date: Mon,  7 May 2018 08:36:40 +0000	[thread overview]
Message-ID: <1525682200-21821-1-git-send-email-ophirmu@mellanox.com> (raw)

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 raw 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
raw 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@dpdk.org

Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
---
 drivers/net/tap/tap_flow.c | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c
index aab9eef..91f15f6 100644
--- a/drivers/net/tap/tap_flow.c
+++ b/drivers/net/tap/tap_flow.c
@@ -1568,10 +1568,10 @@ tap_flow_isolate(struct rte_eth_dev *dev,
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
 
-	if (set)
-		pmd->flow_isolate = 1;
-	else
-		pmd->flow_isolate = 0;
+	/* if already in the right isolation mode - nothing to do */
+	if ((!!set ^ pmd->flow_isolate) == 0)
+		return 0;
+	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,21 +1579,30 @@ 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)
+		while (!LIST_EMPTY(&pmd->implicit_flows)) {
+			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)
-				break;
-			if (tap_flow_destroy_pmd(pmd, flow, NULL) < 0)
-				goto error;
+			if (remote_flow->msg.t.tcm_ifindex != pmd->if_index) {
+				/*
+				 * remove TC from kernel and
+				 * remote_flow from list
+				 */
+				if (tap_flow_destroy_pmd(pmd, remote_flow,
+						NULL) < 0)
+					goto error;
+			} else {
+				/* remove remote_flow from list */
+				LIST_REMOVE(remote_flow, next);
+				rte_free(remote_flow);
+			}
 		}
 		/* Switch the TC rule according to pmd->flow_isolate */
 		if (tap_flow_implicit_create(pmd, TAP_ISOLATE) == -1)
@@ -1739,8 +1748,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

             reply	other threads:[~2018-05-07  8:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-07  8:36 Ophir Munk [this message]
2018-05-14 12:32 ` [PATCH v1] net/tap: fix isolation mode toggling Wiles, Keith
2018-05-14 22:20   ` Ophir Munk
2018-05-14 22:28     ` Wiles, Keith
2018-05-14 22:11 ` [PATCH v2] " Ophir Munk
2018-05-14 22:18   ` Wiles, Keith
2018-05-14 22:19     ` Ophir Munk
2018-05-14 22:26   ` [PATCH v3] " Ophir Munk
2018-05-14 22:46     ` Wiles, Keith
2018-05-17 14:13       ` [dpdk-stable] " Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1525682200-21821-1-git-send-email-ophirmu@mellanox.com \
    --to=ophirmu@mellanox.com \
    --cc=dev@dpdk.org \
    --cc=olgas@mellanox.com \
    --cc=pascal.mazon@6wind.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.