[dpdk-stable] patch 'net/mlx5: fix TC rule handle assignment' has been queued to LTS release 18.11.1

Kevin Traynor ktraynor at redhat.com
Thu Feb 7 14:26:03 CET 2019
Previous message: [dpdk-stable] patch 'net/virtio-user: fix used ring in cvq handling' has been queued to LTS release 18.11.1
Next message: [dpdk-stable] patch 'net/mlx5: fix VXLAN port registration race condition' has been queued to LTS release 18.11.1
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Hi,

FYI, your patch has been queued to LTS release 18.11.1

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 02/14/19. So please
shout if anyone has objections.

Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.

Thanks.

Kevin Traynor

---
>From a81e12468e1e834705750b8ba37876977f101ba9 Mon Sep 17 00:00:00 2001
From: Viacheslav Ovsiienko <viacheslavo at mellanox.com>
Date: Tue, 22 Jan 2019 12:05:46 +0000
Subject: [PATCH] net/mlx5: fix TC rule handle assignment

[ upstream commit 0deb984fd7e59746625e8a16b1b28b7b048971ee ]

When tc rule is created via Netlink message application
can provide the unique rule value which can be accepted
by the kernel. Than rule is managed with this assigned
handle. It was found that kernel can reject the proposed
handle and assign its own handle value, the rule control
is lost, because application uses its initially prorosed
rule handle and knows nothing about handle been repleced.

The kernel can assign handle automatically, the application
can get the assigned handle value by specifying NLM_F_ECHO
flag in Netlink message when rule is being created. The
kernel sends back the full descriptor of rule and handle
can be retrieved from and stored by application for further
rule management.

Fixes: 57123c00c1b8 ("net/mlx5: add Linux TC flower driver for E-Switch flow")

Signed-off-by: Viacheslav Ovsiienko <viacheslavo at mellanox.com>
Acked-by: Shahaf Shuler <shahafs at mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_tcf.c | 97 ++++++++++++++++++++------------
 1 file changed, 60 insertions(+), 37 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index 14896e64d..94421c5a1 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -2666,25 +2666,4 @@ action_of_vlan:
 }
 
-/**
- * Brand rtnetlink buffer with unique handle.
- *
- * This handle should be unique for a given network interface to avoid
- * collisions.
- *
- * @param nlh
- *   Pointer to Netlink message.
- * @param handle
- *   Unique 32-bit handle to use.
- */
-static void
-flow_tcf_nl_brand(struct nlmsghdr *nlh, uint32_t handle)
-{
-	struct tcmsg *tcm = mnl_nlmsg_get_payload(nlh);
-
-	tcm->tcm_handle = handle;
-	DRV_LOG(DEBUG, "Netlink msg %p is branded with handle %x",
-		(void *)nlh, handle);
-}
-
 /**
  * Prepare a flow object for Linux TC flower. It calculates the maximum size of
@@ -2777,18 +2756,4 @@ flow_tcf_prepare(const struct rte_flow_attr *attr,
 	else if (action_flags & MLX5_FLOW_ACTION_VXLAN_ENCAP)
 		dev_flow->tcf.tunnel->type = FLOW_TCF_TUNACT_VXLAN_ENCAP;
-	/*
-	 * Generate a reasonably unique handle based on the address of the
-	 * target buffer.
-	 *
-	 * This is straightforward on 32-bit systems where the flow pointer can
-	 * be used directly. Otherwise, its least significant part is taken
-	 * after shifting it by the previous power of two of the pointed buffer
-	 * size.
-	 */
-	if (sizeof(dev_flow) <= 4)
-		flow_tcf_nl_brand(nlh, (uintptr_t)dev_flow);
-	else
-		flow_tcf_nl_brand(nlh, (uintptr_t)dev_flow >>
-				       rte_log2_u32(rte_align32prevpow2(size)));
 	return dev_flow;
 }
@@ -5349,4 +5314,5 @@ flow_tcf_remove(struct rte_eth_dev *dev, struct rte_flow *flow)
 	struct mlx5_flow *dev_flow;
 	struct nlmsghdr *nlh;
+	struct tcmsg *tcm;
 
 	if (!flow)
@@ -5369,8 +5335,51 @@ flow_tcf_remove(struct rte_eth_dev *dev, struct rte_flow *flow)
 			dev_flow->tcf.tunnel->vtep = NULL;
 		}
+		/* Cleanup the rule handle value. */
+		tcm = mnl_nlmsg_get_payload(nlh);
+		tcm->tcm_handle = 0;
 		dev_flow->tcf.applied = 0;
 	}
 }
 
+/**
+ * Fetch the applied rule handle. This is callback routine called by
+ * libmnl mnl_cb_run() in loop for every message in received packet.
+ * When the NLM_F_ECHO flag i sspecified the kernel sends the created
+ * rule descriptor back to the application and we can retrieve the
+ * actual rule handle from updated descriptor.
+ *
+ * @param[in] nlh
+ *   Pointer to reply header.
+ * @param[in, out] arg
+ *   Context pointer for this callback.
+ *
+ * @return
+ *   A positive, nonzero value on success (required by libmnl
+ *   to continue messages processing).
+ */
+static int
+flow_tcf_collect_apply_cb(const struct nlmsghdr *nlh, void *arg)
+{
+	struct nlmsghdr *nlhrq = arg;
+	struct tcmsg *tcmrq = mnl_nlmsg_get_payload(nlhrq);
+	struct tcmsg *tcm = mnl_nlmsg_get_payload(nlh);
+	struct nlattr *na;
+
+	if (nlh->nlmsg_type != RTM_NEWTFILTER ||
+	    nlh->nlmsg_seq != nlhrq->nlmsg_seq)
+		return 1;
+	mnl_attr_for_each(na, nlh, sizeof(*tcm)) {
+		switch (mnl_attr_get_type(na)) {
+		case TCA_KIND:
+			if (strcmp(mnl_attr_get_payload(na), "flower")) {
+				/* Not flower filter, drop entire message. */
+				return 1;
+			}
+			tcmrq->tcm_handle = tcm->tcm_handle;
+			return 1;
+		}
+	}
+	return 1;
+}
 /**
  * Apply flow to E-Switch by sending Netlink message.
@@ -5394,4 +5403,6 @@ flow_tcf_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
 	struct mlx5_flow *dev_flow;
 	struct nlmsghdr *nlh;
+	struct tcmsg *tcm;
+	int ret;
 
 	dev_flow = LIST_FIRST(&flow->dev_flows);
@@ -5402,5 +5413,9 @@ flow_tcf_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
 	nlh = dev_flow->tcf.nlh;
 	nlh->nlmsg_type = RTM_NEWTFILTER;
-	nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL;
+	nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE |
+			   NLM_F_EXCL | NLM_F_ECHO;
+	tcm = mnl_nlmsg_get_payload(nlh);
+	/* Allow kernel to assign handle on its own. */
+	tcm->tcm_handle = 0;
 	if (dev_flow->tcf.tunnel) {
 		/*
@@ -5423,5 +5438,13 @@ flow_tcf_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
 			dev_flow->tcf.tunnel->vtep->ifindex;
 	}
-	if (!flow_tcf_nl_ack(ctx, nlh, NULL, NULL)) {
+	ret = flow_tcf_nl_ack(ctx, nlh, flow_tcf_collect_apply_cb, nlh);
+	if (!ret) {
+		if (!tcm->tcm_handle) {
+			flow_tcf_remove(dev, flow);
+			return rte_flow_error_set
+				(error, ENOENT,
+				 RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+				 "netlink: rule zero handle returned");
+		}
 		dev_flow->tcf.applied = 1;
 		if (*dev_flow->tcf.ptc_flags & TCA_CLS_FLAGS_SKIP_SW)
-- 
2.19.0

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2019-02-07 13:19:56.920261543 +0000
+++ 0057-net-mlx5-fix-TC-rule-handle-assignment.patch	2019-02-07 13:19:55.000000000 +0000
@@ -1,8 +1,10 @@
-From 0deb984fd7e59746625e8a16b1b28b7b048971ee Mon Sep 17 00:00:00 2001
+From a81e12468e1e834705750b8ba37876977f101ba9 Mon Sep 17 00:00:00 2001
 From: Viacheslav Ovsiienko <viacheslavo at mellanox.com>
 Date: Tue, 22 Jan 2019 12:05:46 +0000
 Subject: [PATCH] net/mlx5: fix TC rule handle assignment
 
+[ upstream commit 0deb984fd7e59746625e8a16b1b28b7b048971ee ]
+
 When tc rule is created via Netlink message application
 can provide the unique rule value which can be accepted
 by the kernel. Than rule is managed with this assigned
@@ -19,7 +21,6 @@
 rule management.
 
 Fixes: 57123c00c1b8 ("net/mlx5: add Linux TC flower driver for E-Switch flow")
-Cc: stable at dpdk.org
 
 Signed-off-by: Viacheslav Ovsiienko <viacheslavo at mellanox.com>
 Acked-by: Shahaf Shuler <shahafs at mellanox.com>
@@ -28,10 +29,10 @@
  1 file changed, 60 insertions(+), 37 deletions(-)
 
 diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
-index cf34742d5..859e99a9a 100644
+index 14896e64d..94421c5a1 100644
 --- a/drivers/net/mlx5/mlx5_flow_tcf.c
 +++ b/drivers/net/mlx5/mlx5_flow_tcf.c
-@@ -2778,25 +2778,4 @@ action_of_vlan:
+@@ -2666,25 +2666,4 @@ action_of_vlan:
  }
  
 -/**
@@ -57,7 +58,7 @@
 -
  /**
   * Prepare a flow object for Linux TC flower. It calculates the maximum size of
-@@ -2889,18 +2868,4 @@ flow_tcf_prepare(const struct rte_flow_attr *attr,
+@@ -2777,18 +2756,4 @@ flow_tcf_prepare(const struct rte_flow_attr *attr,
  	else if (action_flags & MLX5_FLOW_ACTION_VXLAN_ENCAP)
  		dev_flow->tcf.tunnel->type = FLOW_TCF_TUNACT_VXLAN_ENCAP;
 -	/*
@@ -76,13 +77,13 @@
 -				       rte_log2_u32(rte_align32prevpow2(size)));
  	return dev_flow;
  }
-@@ -5594,4 +5559,5 @@ flow_tcf_remove(struct rte_eth_dev *dev, struct rte_flow *flow)
+@@ -5349,4 +5314,5 @@ flow_tcf_remove(struct rte_eth_dev *dev, struct rte_flow *flow)
  	struct mlx5_flow *dev_flow;
  	struct nlmsghdr *nlh;
 +	struct tcmsg *tcm;
  
  	if (!flow)
-@@ -5614,8 +5580,51 @@ flow_tcf_remove(struct rte_eth_dev *dev, struct rte_flow *flow)
+@@ -5369,8 +5335,51 @@ flow_tcf_remove(struct rte_eth_dev *dev, struct rte_flow *flow)
  			dev_flow->tcf.tunnel->vtep = NULL;
  		}
 +		/* Cleanup the rule handle value. */
@@ -134,14 +135,14 @@
 +}
  /**
   * Apply flow to E-Switch by sending Netlink message.
-@@ -5639,4 +5648,6 @@ flow_tcf_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
+@@ -5394,4 +5403,6 @@ flow_tcf_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
  	struct mlx5_flow *dev_flow;
  	struct nlmsghdr *nlh;
 +	struct tcmsg *tcm;
 +	int ret;
  
  	dev_flow = LIST_FIRST(&flow->dev_flows);
-@@ -5647,5 +5658,9 @@ flow_tcf_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
+@@ -5402,5 +5413,9 @@ flow_tcf_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
  	nlh = dev_flow->tcf.nlh;
  	nlh->nlmsg_type = RTM_NEWTFILTER;
 -	nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL;
@@ -152,7 +153,7 @@
 +	tcm->tcm_handle = 0;
  	if (dev_flow->tcf.tunnel) {
  		/*
-@@ -5668,5 +5683,13 @@ flow_tcf_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
+@@ -5423,5 +5438,13 @@ flow_tcf_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
  			dev_flow->tcf.tunnel->vtep->ifindex;
  	}
 -	if (!flow_tcf_nl_ack(ctx, nlh, NULL, NULL)) {
Previous message: [dpdk-stable] patch 'net/virtio-user: fix used ring in cvq handling' has been queued to LTS release 18.11.1
Next message: [dpdk-stable] patch 'net/mlx5: fix VXLAN port registration race condition' has been queued to LTS release 18.11.1
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
More information about the stable mailing list