[ovs-dev] [PATCH v6 7/8] dpif-netdev: Support partial offload of POP_VLAN action

Sriharsha Basavapatna sriharsha.basavapatna at broadcom.com
Sun Jul 12 19:26:24 UTC 2020


If the output-port is a vhost-user port and the action is POP_VLAN,
offload the action on the ingress device if it is offload capable.

Note:
- With ingress partial action offload, the flow must be added to the
  mark-to-flow table. Otherwise, since the action (e.g, POP_VLAN) is
  already performed in the HW, the flow can't be located in the datapath
  flow tables and caches.
- The mark action is offloaded implicitly to facilitate this mark-to-flow
  lookup.
- Add a new member 'partial_actions_offloaded' to the info structure passed
  to the offload layer. When the offload layer successfully offloads the
  partial action, it indicates this to the dpif-netdev layer through this
  flag. This is needed by the dpif-netdev layer to distinguish partial
  offload (i.e, classification offload or mark,rss actions) from partial
  actions offload (classification + some actions, e.g vlan-pop,mark actions)
  in ingress direction.

Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna at broadcom.com>
---
 lib/dpif-netdev.c             | 61 ++++++++++++++++++----
 lib/netdev-offload-dpdk.c     | 97 ++++++++++++++++++++++++++++++++---
 lib/netdev-offload-provider.h |  6 +++
 lib/netdev-offload.c          | 35 +++++++++++++
 lib/netdev-offload.h          |  4 ++
 5 files changed, 186 insertions(+), 17 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 53f07fc44..60de686bd 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -115,6 +115,7 @@ COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
 COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
 COVERAGE_DEFINE(datapath_skip_tunnel_push);
 COVERAGE_DEFINE(datapath_skip_vlan_push);
+COVERAGE_DEFINE(datapath_skip_vlan_pop);
 
 /* Protects against changes to 'dp_netdevs'. */
 static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
@@ -2551,6 +2552,28 @@ dp_netdev_flow_offload_del(struct dp_flow_offload_item *offload)
     }
 }
 
+/* This function determines if the given flow actions can be partially
+ * offloaded. Partial action offload is attempted when either the in-port
+ * or the out-port for the flow is a vhost-user port.
+ */
+static bool
+should_partial_offload(struct netdev *in_netdev, const char *dpif_type,
+                       struct match *match, struct nlattr *actions,
+                       size_t act_len, struct netdev **egress_netdev,
+                       odp_port_t *egress_port)
+{
+    if (netdev_partial_offload_ingress(in_netdev, dpif_type, match, actions,
+                                       act_len)) {
+        return true;
+    } else if (netdev_partial_offload_egress(in_netdev, dpif_type, match,
+                                             actions, act_len, egress_netdev,
+                                             egress_port)) {
+        return true;
+    } else {
+        return false;
+    }
+}
+
 static int
 dp_netdev_alloc_flow_mark(struct dp_netdev_flow *flow, bool modification,
                           uint32_t *markp)
@@ -2626,14 +2649,14 @@ dp_netdev_flow_offload_put(struct dp_flow_offload_item *offload)
 
     info.attr_egress = 0;
     info.partial_actions = 0;
-
-    if (unlikely(netdev_partial_offload_egress(netdev, dpif_type_str,
-                                               &offload->match,
-                                               CONST_CAST(struct nlattr *,
-                                               offload->actions),
-                                               offload->actions_len,
-                                               &egress_netdev,
-                                               &egress_port))) {
+    info.partial_actions_offloaded = 0;
+
+    if (unlikely(should_partial_offload(netdev, dpif_type_str, &offload->match,
+                                        CONST_CAST(struct nlattr *,
+                                        offload->actions),
+                                        offload->actions_len,
+                                        &egress_netdev,
+                                        &egress_port))) {
         if (egress_netdev) {
             netdev_close(netdev);
             netdev = egress_netdev;
@@ -2666,12 +2689,14 @@ dp_netdev_flow_offload_put(struct dp_flow_offload_item *offload)
         goto err_free;
     }
 
-    if (unlikely(info.partial_actions && egress_netdev)) {
+    if (unlikely(info.partial_actions && info.partial_actions_offloaded)) {
         VLOG_DBG_RL("%s: flow: %p mega_ufid: "UUID_FMT" pmd_id: %d\n",
                     __func__, flow, UUID_ARGS((struct uuid *)&flow->mega_ufid),
                     flow->pmd_id);
         flow->partial_actions_offloaded = true;
-    } else if (!modification) {
+    }
+
+    if (!modification && alloc_mark) {
         megaflow_to_mark_associate(&flow->mega_ufid, mark);
         mark_to_flow_associate(mark, flow);
     }
@@ -7529,6 +7554,7 @@ dp_netdev_assist_cb(void *dp OVS_UNUSED, const struct nlattr *a)
 
     switch (type) {
     case OVS_ACTION_ATTR_PUSH_VLAN:
+    case OVS_ACTION_ATTR_POP_VLAN:
         return true;
     default:
         return false;
@@ -7903,7 +7929,20 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
         break;
     }
 
-    case OVS_ACTION_ATTR_POP_VLAN:
+    case OVS_ACTION_ATTR_POP_VLAN: {
+        struct dp_packet *packet;
+
+        if (!dp_flow || !dp_flow->partial_actions_offloaded) {
+            DP_PACKET_BATCH_FOR_EACH (i, packet, packets_) {
+                eth_pop_vlan(packet);
+            }
+        } else {
+            packet_count = dp_packet_batch_size(packets_);
+            COVERAGE_ADD(datapath_skip_vlan_pop, packet_count);
+        }
+        break;
+    }
+
     case OVS_ACTION_ATTR_PUSH_MPLS:
     case OVS_ACTION_ATTR_POP_MPLS:
     case OVS_ACTION_ATTR_SET:
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 449333248..a4da03e62 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -1347,8 +1347,11 @@ parse_flow_actions(struct netdev *netdev,
     }
     NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
         if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) {
-            if (add_output_action(netdev, actions, nla)) {
-                return -1;
+            /* add output action only if full-offload */
+            if (!info->partial_actions) {
+                if (add_output_action(netdev, actions, nla)) {
+                    return -1;
+                }
             }
         } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_DROP) {
             add_flow_action(actions, RTE_FLOW_ACTION_TYPE_DROP, NULL);
@@ -1390,6 +1393,13 @@ parse_flow_actions(struct netdev *netdev,
         return -1;
     }
 
+    if (info->partial_actions && !info->attr_egress) {
+        struct rte_flow_action_mark *mark = xzalloc(sizeof *mark);
+
+        mark->id = info->flow_mark;
+        add_flow_action(actions, RTE_FLOW_ACTION_TYPE_MARK, mark);
+    }
+
     add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
     return 0;
 }
@@ -1407,9 +1417,11 @@ netdev_offload_dpdk_actions(struct netdev *netdev,
     struct rte_flow_error error;
     int ret;
 
-    if (info->attr_egress) {
-        flow_attr.ingress = 0;
-        flow_attr.egress = 1;
+    if (info->partial_actions) {
+        if (info->attr_egress) {
+            flow_attr.ingress = 0;
+            flow_attr.egress = 1;
+        }
         flow_attr.transfer = 0;
     }
 
@@ -1447,11 +1459,12 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
     flow = netdev_offload_dpdk_actions(netdev, &patterns, nl_actions,
                                        actions_len, info);
     if (flow) {
-        if (info->partial_actions && info->attr_egress) {
+        if (info->partial_actions) {
             /* actions_offloaded should be set to false with partial actions,
              * since it is still considered as partial-offload and not
              * full-offload. */
             actions_offloaded = false;
+            info->partial_actions_offloaded = 1;
         }
     } else if (!(info->partial_actions && info->attr_egress)) {
         /* If we failed to offload the rule actions fallback to MARK+RSS
@@ -1518,6 +1531,7 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match,
             VLOG_DBG_RL("%s: mega_ufid: "UUID_FMT" refcnt: %d\n", __func__,
                         UUID_ARGS((struct uuid *)ufid), rte_flow_data->refcnt);
             rte_flow_data->refcnt++;
+            info->partial_actions_offloaded = 1;
             return ret;
         } else {
             /*
@@ -1635,6 +1649,18 @@ enum num_action_attr_egress {
     MAX_ACTION_ATTRS_EGRESS = TUNNEL_PUSH_ATTRS
 };
 
+/*
+ * Maxium number of actions to be parsed while selecting a flow for ingress
+ * partial action offload. This number is currently based on the minimum
+ * number of attributes seen with the vlan pop action (vlan_pop, output).
+ * This number includes output action to a single vhost device (uplink) and
+ * does not support multiple output actions.
+ */
+enum num_action_attr_ingress {
+    VLAN_POP_ATTRS = 2,        /* vlan_pop, output */
+    MAX_ACTION_ATTRS_INGRESS = VLAN_POP_ATTRS
+};
+
 /*
  * This function parses the list of OVS "actions" of length "actions_len",
  * and returns them in an array of action "attrs", of size "max_attrs".
@@ -1682,6 +1708,64 @@ parse_nlattr_actions(struct nlattr *actions, size_t actions_len,
     return 0;
 }
 
+/* This function determines if the given flow should be partially offloaded
+ * on the ingress device, when the out-port is not offload-capable like a
+ * vhost-user port. The function currently supports offloading of only
+ * vlan-pop action.
+ */
+static bool
+netdev_offload_dpdk_ingress_partial(struct netdev *netdev,
+                                    struct match *match,
+                                    struct nlattr *actions,
+                                    size_t actions_len)
+{
+    struct action_attr attrs[MAX_ACTION_ATTRS_INGRESS];
+    odp_port_t out_port = ODPP_NONE;
+    struct netdev *out_netdev;
+    int num_attrs = 0;
+    int type;
+    int rc;
+
+    /* Support ingress partial-offload only
+     * when the in-port supports offloads.
+     */
+    if (!netdev_dpdk_flow_api_supported(netdev)) {
+         return false;
+    }
+
+    rc = parse_nlattr_actions(actions, actions_len, attrs,
+                              MAX_ACTION_ATTRS_INGRESS, &num_attrs);
+    if (rc == E2BIG) {
+        /* Action list too big; decline partial offload */
+        return false;
+    }
+
+    /* Minimum number of attrs expected (pop_vlan) */
+    if (num_attrs < VLAN_POP_ATTRS) {
+        return false;
+    }
+
+    if (num_attrs == VLAN_POP_ATTRS &&
+        (attrs[0].type != OVS_ACTION_ATTR_POP_VLAN ||
+         attrs[1].type != OVS_ACTION_ATTR_OUTPUT)) {
+        return false;
+    }
+
+    /* Ingress partial-offload needs an output action at the end. */
+    out_port = nl_attr_get_odp_port(attrs[num_attrs - 1].action);
+    if (out_port == ODPP_NONE) {
+        return false;
+    }
+
+    /* Support ingress partial-offload only if out-port is vhost-user. */
+    out_netdev = netdev_ports_get(out_port, netdev->dpif_type);
+    if (out_netdev && is_dpdk_vhost_netdev(out_netdev)) {
+        return true;
+    }
+
+    return false;
+}
+
 /* This function determines if the given flow should be partially offloaded
  * on the egress device, when the in-port is not offload-capable like a
  * vhost-user port. The function currently supports offloading of only
@@ -1754,4 +1838,5 @@ const struct netdev_flow_api netdev_offload_dpdk = {
     .init_flow_api = netdev_offload_dpdk_init_flow_api,
     .flow_get = netdev_offload_dpdk_flow_get,
     .flow_offload_egress_partial = netdev_offload_dpdk_egress_partial,
+    .flow_offload_ingress_partial = netdev_offload_dpdk_ingress_partial,
 };
diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
index 94973b24d..f0e944281 100644
--- a/lib/netdev-offload-provider.h
+++ b/lib/netdev-offload-provider.h
@@ -93,6 +93,12 @@ struct netdev_flow_api {
     bool (*flow_offload_egress_partial)(struct netdev *, struct match *,
                                         struct nlattr *, size_t,
                                         struct netdev **, odp_port_t *);
+
+    /* Determine if the flow should be partial offloaded to the ingress
+     * device and if yes return true; otherwise return false.
+     */
+    bool (*flow_offload_ingress_partial)(struct netdev *, struct match *,
+                                         struct nlattr *, size_t);
 };
 
 int netdev_register_flow_api_provider(const struct netdev_flow_api *);
diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
index 4fee717d9..9ce9bfb8b 100644
--- a/lib/netdev-offload.c
+++ b/lib/netdev-offload.c
@@ -733,3 +733,38 @@ netdev_partial_offload_egress(struct netdev *netdev, const char *dpif_type,
     netdev_close(flow_api_netdev);
     return true;
 }
+
+bool
+netdev_partial_offload_ingress(struct netdev *netdev, const char *dpif_type,
+                               struct match *match, struct nlattr *actions,
+                               size_t act_len)
+{
+    struct netdev *flow_api_netdev;
+    struct port_to_netdev_data *data;
+    struct netdev_flow_api *flow_api =
+        ovsrcu_get(const struct netdev_flow_api *, &netdev->flow_api);
+
+    /* Ingress netdev is not offload capable */
+    if (!flow_api) {
+        return false;
+    }
+
+    /* Ingress netdev must belong to the datapath specified */
+    if (netdev_get_dpif_type(netdev) != dpif_type) {
+        return false;
+    }
+
+    /* flow_api does not support ingress partial offload */
+    if (!flow_api->flow_offload_ingress_partial) {
+        return false;
+    }
+
+    /* Can the flow be partial offloaded to the ingress dev ? */
+    if (!flow_api->flow_offload_ingress_partial(netdev, match, actions,
+                                                act_len)) {
+        return false;
+    }
+
+    /* Success */
+    return true;
+}
diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h
index 66b31af59..bf782fa29 100644
--- a/lib/netdev-offload.h
+++ b/lib/netdev-offload.h
@@ -69,6 +69,7 @@ struct offload_info {
                                      * sync with datapath recirc ids. */
     uint8_t attr_egress;      /* Egress direction offload */
     uint8_t partial_actions;  /* Partial action offload; no forward action */
+    uint8_t partial_actions_offloaded; /* Success flag */
 
     /*
      * The flow mark id assigened to the flow. If any pkts hit the flow,
@@ -131,6 +132,9 @@ bool netdev_partial_offload_egress(struct netdev *netdev,
                                    struct nlattr *actions, size_t act_len,
                                    struct netdev **egress_netdev,
                                    odp_port_t *egress_port);
+bool netdev_partial_offload_ingress(struct netdev *netdev,
+                                    const char *dpif_type, struct match *match,
+                                    struct nlattr *actions, size_t act_len);
 #ifdef  __cplusplus
 }
 #endif
-- 
2.25.0.rc2



More information about the dev mailing list