[ovs-dev,v3,6/9] netdev-dpdk: retry with queue action

Message ID 1506404199-23579-7-git-send-email-yliu@fridaylinux.org
State New
Headers show
Series
  • OVS-DPDK flow offload with rte_flow
Related show

Commit Message

Yuanhan Liu Sept. 26, 2017, 5:36 a.m.
From: Finn Christensen <fc@napatech.com>

AFAIK, most (if not all) NICs (including Mellanox and Intel) do not
support a pure MARK action.  It's required to be used together with
some other actions, like QUEUE.

To workaround it, retry with a queue action when first try failed.

Moreover, some Intel's NIC (say XL710) needs the QUEUE action set
before the MARK action.

Co-authored-by: Yuanhan Liu <yliu@fridaylinux.org>
Signed-off-by: Finn Christensen <fc@napatech.com>
Signed-off-by: Yuanhan Liu <yliu@fridaylinux.org>
---

v3: - retry when 2 specific errors are met

v2: - workarounded the queue action issue, which sets the queue index
      to 0 blindly.
    - added some comments regarding to the retry with queue action
---
 lib/netdev-dpdk.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 56 insertions(+), 7 deletions(-)

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 525536a..3e2b96b 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -3324,6 +3324,7 @@  static struct ovs_mutex ufid_lock = OVS_MUTEX_INITIALIZER;
 struct ufid_dpdk_flow_data {
     struct hmap_node node;
     ovs_u128 ufid;
+    int rxq;
     struct rte_flow *rte_flow;
 };
 
@@ -3369,7 +3370,8 @@  del_ufid_dpdk_flow_mapping(const ovs_u128 *ufid)
 
 /* Add ufid to dpdk_flow mapping */
 static inline void
-add_ufid_dpdk_flow_mapping(const ovs_u128 *ufid, struct rte_flow *rte_flow)
+add_ufid_dpdk_flow_mapping(const ovs_u128 *ufid, struct rte_flow *rte_flow,
+                           int rxq)
 {
     size_t hash = hash_bytes(ufid, sizeof(*ufid), 0);
     struct ufid_dpdk_flow_data *data = xzalloc(sizeof(*data));
@@ -3384,6 +3386,7 @@  add_ufid_dpdk_flow_mapping(const ovs_u128 *ufid, struct rte_flow *rte_flow)
 
     data->ufid = *ufid;
     data->rte_flow = rte_flow;
+    data->rxq = rxq;
 
     ovs_mutex_lock(&ufid_lock);
     hmap_insert(&ufid_dpdk_flow, &data->node, hash);
@@ -3665,15 +3668,55 @@  netdev_dpdk_add_rte_flow_offload(struct netdev *netdev,
     add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_MARK, &mark);
     add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL);
 
+    bool tried = 0;
+    struct rte_flow_action_queue queue;
+again:
     flow = rte_flow_create(dev->port_id, &flow_attr, patterns.items,
                            actions.actions, &error);
-    if (!flow) {
+    if (!flow && !tried &&
+        (error.type == RTE_FLOW_ERROR_TYPE_ACTION ||
+         error.type == RTE_FLOW_ERROR_TYPE_HANDLE)) {
+        /*
+         * Seems most (if not all) NICs do not support a pure MARK
+         * action. It's required to be used together with some other
+         * actions, such as QUEUE.
+         *
+         * Thus, if flow creation fails (more precisely, if above 2
+         * errors are met), here we try again with QUEUE action.
+         *
+         * The reason why there are 2 error codes is DPDK drivers are
+         * not quite consistent on error report for this case.
+         */
+        if (info->rxq < 0 || info->rxq >= netdev->n_rxq) {
+            VLOG_ERR("invalid queue index: %d\n", info->rxq);
+            ret = -1;
+            goto out;
+        }
+        queue.index = info->rxq;
+
+        /* re-build the action */
+        actions.cnt = 0;
+        /*
+         * NOTE: Intel PMD driver needs the QUEUE action set before
+         * the MARK action.
+         */
+        add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_QUEUE, &queue);
+        add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_MARK, &mark);
+        add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL);
+
+        VLOG_INFO("failed to create rte flow, "
+                  "try again with QUEUE action (queue index = %d)\n",
+                  info->rxq);
+        tried = true;
+
+        goto again;
+    } else if (!flow) {
         VLOG_ERR("rte flow creat error: %u : message : %s\n",
                  error.type, error.message);
         ret = -1;
         goto out;
     }
-    add_ufid_dpdk_flow_mapping(ufid, flow);
+    add_ufid_dpdk_flow_mapping(ufid, flow, info->rxq);
     VLOG_INFO("installed flow %p by ufid "UUID_FMT"\n",
               flow, UUID_ARGS((struct uuid *)ufid));
 
@@ -3807,17 +3850,23 @@  netdev_dpdk_flow_put(struct netdev *netdev, struct match *match,
                      const ovs_u128 *ufid, struct offload_info *info,
                      struct dpif_flow_stats *stats OVS_UNUSED)
 {
-    struct rte_flow *rte_flow;
+    struct ufid_dpdk_flow_data *flow_data;
     int ret;
 
     /*
      * If an old rte_flow exists, it means it's a flow modification.
      * Here destroy the old rte flow first before adding a new one.
      */
-    rte_flow = get_rte_flow_by_ufid(ufid);
-    if (rte_flow) {
+    flow_data = find_ufid_dpdk_flow_mapping(ufid);
+    if (flow_data && flow_data->rte_flow) {
+        /*
+         * there is no rxq given for flow modification. Instead, we
+         * retrieve it from the rxq firstly registered here (by flow
+         * add operation).
+         */
+        info->rxq = flow_data->rxq;
         ret = netdev_dpdk_destroy_rte_flow(netdev_dpdk_cast(netdev),
-                                           ufid, rte_flow);
+                                           ufid, flow_data->rte_flow);
         if (ret < 0) {
             return ret;
         }