[ovs-dev,V5,14/18] netdev-offload-dpdk: Support offload of output action
diff mbox series

Message ID 20191218144110.18653-15-elibr@mellanox.com
State Changes Requested
Delegated to: Ilya Maximets
Headers show
Series
  • netdev datapath actions offload
Related show

Commit Message

Eli Britstein Dec. 18, 2019, 2:41 p.m. UTC
Support offload of output action, also configuring count action for
allowing query statistics of HW offloaded flows.

Signed-off-by: Eli Britstein <elibr@mellanox.com>
Reviewed-by: Oz Shlomo <ozsh@mellanox.com>
---
 Documentation/howto/dpdk.rst | 17 ++++++---
 NEWS                         |  1 +
 lib/netdev-offload-dpdk.c    | 85 +++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 95 insertions(+), 8 deletions(-)

Comments

Ilya Maximets Dec. 18, 2019, 10:08 p.m. UTC | #1
On 18.12.2019 15:41, Eli Britstein wrote:
> Support offload of output action, also configuring count action for
> allowing query statistics of HW offloaded flows.
> 
> Signed-off-by: Eli Britstein <elibr@mellanox.com>
> Reviewed-by: Oz Shlomo <ozsh@mellanox.com>
> ---
>  Documentation/howto/dpdk.rst | 17 ++++++---
>  NEWS                         |  1 +
>  lib/netdev-offload-dpdk.c    | 85 +++++++++++++++++++++++++++++++++++++++++---
>  3 files changed, 95 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
> index 766a7950c..f0fe2bef7 100644
> --- a/Documentation/howto/dpdk.rst
> +++ b/Documentation/howto/dpdk.rst
> @@ -370,19 +370,28 @@ The flow hardware offload is disabled by default and can be enabled by::
>  
>      $ ovs-vsctl set Open_vSwitch . other_config:hw-offload=true
>  
> -So far only partial flow offload is implemented. Moreover, it only works
> -with PMD drivers have the rte_flow action "MARK + RSS" support.
> +Matches and actions are programmed into HW to achieve full offload of
> +the flow. If not all actions are supported, fallback to partial flow
> +offload (offloading matches only). Moreover, it only works with PMD
> +drivers that support the configured rte_flow actions.
> +Partial flow offload requires support of "MARK + RSS" actions. Full
> +hardware offload requires support of the actions listed below.
>  
>  The validated NICs are:
>  
>  - Mellanox (ConnectX-4, ConnectX-4 Lx, ConnectX-5)
>  - Napatech (NT200B01)
>  
> -Supported protocols for hardware offload are:
> +Supported protocols for hardware offload matches are:
> +
>  - L2: Ethernet, VLAN
> -- L3: IPv4, IPv6
> +- L3: IPv4
>  - L4: TCP, UDP, SCTP, ICMP
>  
> +Supported actions for hardware offload are:
> +
> +- Output.
> +
>  Further Reading
>  ---------------
>  
> diff --git a/NEWS b/NEWS
> index 85c4a4812..8b8bcab9a 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -26,6 +26,7 @@ Post-v2.12.0
>       * DPDK ring ports (dpdkr) are deprecated and will be removed in next
>         releases.
>       * Add support for DPDK 19.11.
> +     * Add hardware offload support for output actions (experimental).
>     - 'ovs-appctl dpctl/dump-flows' can now show offloaded=partial for
>       partially offloaded flows, dp:dpdk for fully offloaded by dpdk, and
>       type filter supports new filters: "dpdk" and "partially-offloaded".
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index 861ceb9fa..6b3ca9f7d 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -347,6 +347,26 @@ dump_flow_action(struct ds *s, const struct rte_flow_action *actions)
>          } else {
>              ds_put_cstr(s, "  RSS = null\n");
>          }
> +    } else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
> +        const struct rte_flow_action_count *count = actions->conf;
> +
> +        ds_put_cstr(s, "rte flow count action:\n");
> +        if (count) {
> +            ds_put_format(s, "  Count: shared=%d, id=%d\n", count->shared,
> +                          count->id);
> +        } else {
> +            ds_put_cstr(s, "  Count = null\n");
> +        }
> +    } else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
> +        const struct rte_flow_action_port_id *port_id = actions->conf;
> +
> +        ds_put_cstr(s, "rte flow port-id action:\n");
> +        if (port_id) {
> +            ds_put_format(s, "  Port-id: original=%d, id=%d\n",
> +                          port_id->original, port_id->id);
> +        } else {
> +            ds_put_cstr(s, "  Port-id = null\n");
> +        }
>      } else {
>          ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
>      }
> @@ -719,19 +739,76 @@ netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns,
>      return flow;
>  }
>  
> +static void
> +add_count_action(struct flow_actions *actions)
> +{
> +    struct rte_flow_action_count *count = xzalloc(sizeof *count);
> +
> +    add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
> +}
> +
>  static int
> -parse_flow_actions(struct netdev *netdev OVS_UNUSED,
> +add_port_id_action(struct flow_actions *actions,
> +                   struct netdev *outdev)
> +{
> +    struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
> +    int outdev_id;
> +
> +    outdev_id = netdev_dpdk_get_port_id(outdev);
> +    if (outdev_id < 0) {
> +        return -1;

Leaking the port_id here.

> +    }
> +    port_id->id = outdev_id;
> +    add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
> +    return 0;
> +}
> +
> +static int
> +add_output_action(struct netdev *netdev,
> +                  struct flow_actions *actions,
> +                  const struct nlattr *nla,
> +                  struct offload_info *info)
> +{
> +    struct netdev *outdev;
> +    odp_port_t port;
> +    int ret = 0;
> +
> +    port = nl_attr_get_odp_port(nla);
> +    outdev = netdev_ports_get(port, info->dpif_class);
> +    if (outdev == NULL) {
> +        VLOG_DBG_RL(&rl, "Cannot find netdev for odp port %d", port);

It's better to use PRIu32 for odp_port_t. At least, it's unsigned.

> +        return -1;
> +    }
> +    if (!netdev_flow_api_equals(netdev, outdev) ||
> +        add_port_id_action(actions, outdev)) {
> +        VLOG_DBG_RL(&rl, "Output to %s cannot be offloaded",
> +                    netdev_get_name(outdev));

It might be good to print the source netdev name here too.
e.g. "%s: Output to port \'%s\' cannot be offloaded.",
     netdev_get_name(netdev), netdev_get_name(outdev));

> +        ret = -1;
> +    }
> +    netdev_close(outdev);
> +    return ret;
> +}
> +
> +static int
> +parse_flow_actions(struct netdev *netdev,
>                     struct flow_actions *actions,
>                     struct nlattr *nl_actions,
>                     size_t nl_actions_len,
> -                   struct offload_info *info OVS_UNUSED)
> +                   struct offload_info *info)
>  {
>      struct nlattr *nla;
>      size_t left;
>  
> +    add_count_action(actions);
>      NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
> -        VLOG_DBG_RL(&rl, "Unsupported action type %d", nl_attr_type(nla));
> -        return -1;
> +        if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) {
> +            if (add_output_action(netdev, actions, nla, info)) {
> +                return -1;
> +            }
> +        } else {
> +            VLOG_DBG_RL(&rl, "Unsupported action type %d", nl_attr_type(nla));
> +            return -1;
> +        }
>      }
>  
>      if (nl_actions_len == 0) {
>

Patch
diff mbox series

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index 766a7950c..f0fe2bef7 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -370,19 +370,28 @@  The flow hardware offload is disabled by default and can be enabled by::
 
     $ ovs-vsctl set Open_vSwitch . other_config:hw-offload=true
 
-So far only partial flow offload is implemented. Moreover, it only works
-with PMD drivers have the rte_flow action "MARK + RSS" support.
+Matches and actions are programmed into HW to achieve full offload of
+the flow. If not all actions are supported, fallback to partial flow
+offload (offloading matches only). Moreover, it only works with PMD
+drivers that support the configured rte_flow actions.
+Partial flow offload requires support of "MARK + RSS" actions. Full
+hardware offload requires support of the actions listed below.
 
 The validated NICs are:
 
 - Mellanox (ConnectX-4, ConnectX-4 Lx, ConnectX-5)
 - Napatech (NT200B01)
 
-Supported protocols for hardware offload are:
+Supported protocols for hardware offload matches are:
+
 - L2: Ethernet, VLAN
-- L3: IPv4, IPv6
+- L3: IPv4
 - L4: TCP, UDP, SCTP, ICMP
 
+Supported actions for hardware offload are:
+
+- Output.
+
 Further Reading
 ---------------
 
diff --git a/NEWS b/NEWS
index 85c4a4812..8b8bcab9a 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,7 @@  Post-v2.12.0
      * DPDK ring ports (dpdkr) are deprecated and will be removed in next
        releases.
      * Add support for DPDK 19.11.
+     * Add hardware offload support for output actions (experimental).
    - 'ovs-appctl dpctl/dump-flows' can now show offloaded=partial for
      partially offloaded flows, dp:dpdk for fully offloaded by dpdk, and
      type filter supports new filters: "dpdk" and "partially-offloaded".
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 861ceb9fa..6b3ca9f7d 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -347,6 +347,26 @@  dump_flow_action(struct ds *s, const struct rte_flow_action *actions)
         } else {
             ds_put_cstr(s, "  RSS = null\n");
         }
+    } else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
+        const struct rte_flow_action_count *count = actions->conf;
+
+        ds_put_cstr(s, "rte flow count action:\n");
+        if (count) {
+            ds_put_format(s, "  Count: shared=%d, id=%d\n", count->shared,
+                          count->id);
+        } else {
+            ds_put_cstr(s, "  Count = null\n");
+        }
+    } else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
+        const struct rte_flow_action_port_id *port_id = actions->conf;
+
+        ds_put_cstr(s, "rte flow port-id action:\n");
+        if (port_id) {
+            ds_put_format(s, "  Port-id: original=%d, id=%d\n",
+                          port_id->original, port_id->id);
+        } else {
+            ds_put_cstr(s, "  Port-id = null\n");
+        }
     } else {
         ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
     }
@@ -719,19 +739,76 @@  netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns,
     return flow;
 }
 
+static void
+add_count_action(struct flow_actions *actions)
+{
+    struct rte_flow_action_count *count = xzalloc(sizeof *count);
+
+    add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
+}
+
 static int
-parse_flow_actions(struct netdev *netdev OVS_UNUSED,
+add_port_id_action(struct flow_actions *actions,
+                   struct netdev *outdev)
+{
+    struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
+    int outdev_id;
+
+    outdev_id = netdev_dpdk_get_port_id(outdev);
+    if (outdev_id < 0) {
+        return -1;
+    }
+    port_id->id = outdev_id;
+    add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
+    return 0;
+}
+
+static int
+add_output_action(struct netdev *netdev,
+                  struct flow_actions *actions,
+                  const struct nlattr *nla,
+                  struct offload_info *info)
+{
+    struct netdev *outdev;
+    odp_port_t port;
+    int ret = 0;
+
+    port = nl_attr_get_odp_port(nla);
+    outdev = netdev_ports_get(port, info->dpif_class);
+    if (outdev == NULL) {
+        VLOG_DBG_RL(&rl, "Cannot find netdev for odp port %d", port);
+        return -1;
+    }
+    if (!netdev_flow_api_equals(netdev, outdev) ||
+        add_port_id_action(actions, outdev)) {
+        VLOG_DBG_RL(&rl, "Output to %s cannot be offloaded",
+                    netdev_get_name(outdev));
+        ret = -1;
+    }
+    netdev_close(outdev);
+    return ret;
+}
+
+static int
+parse_flow_actions(struct netdev *netdev,
                    struct flow_actions *actions,
                    struct nlattr *nl_actions,
                    size_t nl_actions_len,
-                   struct offload_info *info OVS_UNUSED)
+                   struct offload_info *info)
 {
     struct nlattr *nla;
     size_t left;
 
+    add_count_action(actions);
     NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
-        VLOG_DBG_RL(&rl, "Unsupported action type %d", nl_attr_type(nla));
-        return -1;
+        if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) {
+            if (add_output_action(netdev, actions, nla, info)) {
+                return -1;
+            }
+        } else {
+            VLOG_DBG_RL(&rl, "Unsupported action type %d", nl_attr_type(nla));
+            return -1;
+        }
     }
 
     if (nl_actions_len == 0) {