| Message ID | 20191202084153.14412-15-elibr@mellanox.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | netdev datapath actions offload | expand |
On 02.12.2019 09:41, Eli Britstein wrote: > Signed-off-by: Eli Britstein <elibr@mellanox.com> > Reviewed-by: Oz Shlomo <ozsh@mellanox.com> This patch has a side effect of installing the COUNT action. That needs to be described in the commit message. > --- > Documentation/howto/dpdk.rst | 8 ++-- > NEWS | 1 + > lib/netdev-offload-dpdk-flow.c | 91 ++++++++++++++++++++++++++++++++++++++++-- > 3 files changed, 93 insertions(+), 7 deletions(-) > > diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst > index 766a7950c..de779d78f 100644 > --- a/Documentation/howto/dpdk.rst > +++ b/Documentation/howto/dpdk.rst > @@ -370,8 +370,10 @@ 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 have the relevant rte_flow actions support. Could you, please, keep the information on what are the 'relevant' actions. > > The validated NICs are: > > @@ -380,7 +382,7 @@ The validated NICs are: > > Supported protocols for hardware offload are: > - L2: Ethernet, VLAN > -- L3: IPv4, IPv6 > +- L3: IPv4 It seems that you need to have separate 'Supported' sections for partial and full offloading (or for match and actions). > - L4: TCP, UDP, SCTP, ICMP > > Further Reading > diff --git a/NEWS b/NEWS > index 222280b1a..33882d2af 100644 > --- a/NEWS > +++ b/NEWS > @@ -26,6 +26,7 @@ Post-v2.12.0 > releases. > * OVS validated with DPDK 18.11.5, due to the inclusion of a fix for > CVE-2019-14818, this DPDK version is strongly recommended to be used. > + * Add hardware offload support for output actions (experimental). > > v2.12.0 - 03 Sep 2019 > --------------------- > diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c > index b03ce2c41..b633bc129 100644 > --- a/lib/netdev-offload-dpdk-flow.c > +++ b/lib/netdev-offload-dpdk-flow.c > @@ -274,6 +274,28 @@ ds_put_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); > } > @@ -531,19 +553,80 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns *patterns, > return 0; > } > > +static void > +netdev_dpdk_flow_add_count_action(struct flow_actions *actions) > +{ > + struct rte_flow_action_count *count = xzalloc(sizeof *count); > + > + count->shared = 0; > + /* Each flow has a single count action, so no need of id */ Comments in OVS should be a full sentence, i.e. end with a period. This is for all the patches in this series. > + count->id = 0; > + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count); > +} > + > +static int > +netdev_dpdk_flow_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 > +netdev_dpdk_flow_add_output_action(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(&error_rl, > + "Cannot find netdev for odp port %d", port); > + return -1; > + } > + if (netdev_dpdk_flow_add_port_id_action(actions, outdev)) { > + VLOG_DBG_RL(&error_rl, > + "Output to %s cannot be offloaded", > + netdev_get_name(outdev)); > + ret = -1; > + } > + netdev_close(outdev); > + return ret; > +} > + > int > netdev_dpdk_flow_actions_add(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; > > + netdev_dpdk_flow_add_count_action(actions); > NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) { > - VLOG_DBG_RL(&error_rl, > - "Unsupported action type %d", nl_attr_type(nla)); > - return -1; > + if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) { > + if (!(left <= NLA_ALIGN(nla->nla_len)) || > + netdev_dpdk_flow_add_output_action(actions, nla, info )) { > + return -1; > + } > + } else { > + VLOG_DBG_RL(&error_rl, > + "Unsupported action type %d", nl_attr_type(nla)); > + return -1; > + } > } > > if (nl_actions_len == 0) { >
On 12/4/2019 5:28 PM, Ilya Maximets wrote: > On 02.12.2019 09:41, Eli Britstein wrote: >> Signed-off-by: Eli Britstein <elibr@mellanox.com> >> Reviewed-by: Oz Shlomo <ozsh@mellanox.com> > This patch has a side effect of installing the COUNT action. > That needs to be described in the commit message. > >> --- >> Documentation/howto/dpdk.rst | 8 ++-- >> NEWS | 1 + >> lib/netdev-offload-dpdk-flow.c | 91 ++++++++++++++++++++++++++++++++++++++++-- >> 3 files changed, 93 insertions(+), 7 deletions(-) >> >> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst >> index 766a7950c..de779d78f 100644 >> --- a/Documentation/howto/dpdk.rst >> +++ b/Documentation/howto/dpdk.rst >> @@ -370,8 +370,10 @@ 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 have the relevant rte_flow actions support. > Could you, please, keep the information on what are the 'relevant' actions. OK, I'll update it here (as well in NEWS). > >> >> The validated NICs are: >> >> @@ -380,7 +382,7 @@ The validated NICs are: >> >> Supported protocols for hardware offload are: >> - L2: Ethernet, VLAN >> -- L3: IPv4, IPv6 >> +- L3: IPv4 > It seems that you need to have separate 'Supported' sections for > partial and full offloading (or for match and actions). IPv6 was not supported before (also for partial), so this is a kind of a fix, but so minor I thought squashing it to this commit. I'll rephrase it. > >> - L4: TCP, UDP, SCTP, ICMP >> >> Further Reading >> diff --git a/NEWS b/NEWS >> index 222280b1a..33882d2af 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -26,6 +26,7 @@ Post-v2.12.0 >> releases. >> * OVS validated with DPDK 18.11.5, due to the inclusion of a fix for >> CVE-2019-14818, this DPDK version is strongly recommended to be used. >> + * Add hardware offload support for output actions (experimental). >> >> v2.12.0 - 03 Sep 2019 >> --------------------- >> diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c >> index b03ce2c41..b633bc129 100644 >> --- a/lib/netdev-offload-dpdk-flow.c >> +++ b/lib/netdev-offload-dpdk-flow.c >> @@ -274,6 +274,28 @@ ds_put_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); >> } >> @@ -531,19 +553,80 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns *patterns, >> return 0; >> } >> >> +static void >> +netdev_dpdk_flow_add_count_action(struct flow_actions *actions) >> +{ >> + struct rte_flow_action_count *count = xzalloc(sizeof *count); >> + >> + count->shared = 0; >> + /* Each flow has a single count action, so no need of id */ > Comments in OVS should be a full sentence, i.e. end with a period. > This is for all the patches in this series. > >> + count->id = 0; >> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count); >> +} >> + >> +static int >> +netdev_dpdk_flow_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 >> +netdev_dpdk_flow_add_output_action(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(&error_rl, >> + "Cannot find netdev for odp port %d", port); >> + return -1; >> + } >> + if (netdev_dpdk_flow_add_port_id_action(actions, outdev)) { >> + VLOG_DBG_RL(&error_rl, >> + "Output to %s cannot be offloaded", >> + netdev_get_name(outdev)); >> + ret = -1; >> + } >> + netdev_close(outdev); >> + return ret; >> +} >> + >> int >> netdev_dpdk_flow_actions_add(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; >> >> + netdev_dpdk_flow_add_count_action(actions); >> NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) { >> - VLOG_DBG_RL(&error_rl, >> - "Unsupported action type %d", nl_attr_type(nla)); >> - return -1; >> + if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) { >> + if (!(left <= NLA_ALIGN(nla->nla_len)) || >> + netdev_dpdk_flow_add_output_action(actions, nla, info )) { >> + return -1; >> + } >> + } else { >> + VLOG_DBG_RL(&error_rl, >> + "Unsupported action type %d", nl_attr_type(nla)); >> + return -1; >> + } >> } >> >> if (nl_actions_len == 0) { >>
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst index 766a7950c..de779d78f 100644 --- a/Documentation/howto/dpdk.rst +++ b/Documentation/howto/dpdk.rst @@ -370,8 +370,10 @@ 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 have the relevant rte_flow actions support. The validated NICs are: @@ -380,7 +382,7 @@ The validated NICs are: Supported protocols for hardware offload are: - L2: Ethernet, VLAN -- L3: IPv4, IPv6 +- L3: IPv4 - L4: TCP, UDP, SCTP, ICMP Further Reading diff --git a/NEWS b/NEWS index 222280b1a..33882d2af 100644 --- a/NEWS +++ b/NEWS @@ -26,6 +26,7 @@ Post-v2.12.0 releases. * OVS validated with DPDK 18.11.5, due to the inclusion of a fix for CVE-2019-14818, this DPDK version is strongly recommended to be used. + * Add hardware offload support for output actions (experimental). v2.12.0 - 03 Sep 2019 --------------------- diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c index b03ce2c41..b633bc129 100644 --- a/lib/netdev-offload-dpdk-flow.c +++ b/lib/netdev-offload-dpdk-flow.c @@ -274,6 +274,28 @@ ds_put_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); } @@ -531,19 +553,80 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns *patterns, return 0; } +static void +netdev_dpdk_flow_add_count_action(struct flow_actions *actions) +{ + struct rte_flow_action_count *count = xzalloc(sizeof *count); + + count->shared = 0; + /* Each flow has a single count action, so no need of id */ + count->id = 0; + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count); +} + +static int +netdev_dpdk_flow_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 +netdev_dpdk_flow_add_output_action(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(&error_rl, + "Cannot find netdev for odp port %d", port); + return -1; + } + if (netdev_dpdk_flow_add_port_id_action(actions, outdev)) { + VLOG_DBG_RL(&error_rl, + "Output to %s cannot be offloaded", + netdev_get_name(outdev)); + ret = -1; + } + netdev_close(outdev); + return ret; +} + int netdev_dpdk_flow_actions_add(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; + netdev_dpdk_flow_add_count_action(actions); NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) { - VLOG_DBG_RL(&error_rl, - "Unsupported action type %d", nl_attr_type(nla)); - return -1; + if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) { + if (!(left <= NLA_ALIGN(nla->nla_len)) || + netdev_dpdk_flow_add_output_action(actions, nla, info )) { + return -1; + } + } else { + VLOG_DBG_RL(&error_rl, + "Unsupported action type %d", nl_attr_type(nla)); + return -1; + } } if (nl_actions_len == 0) {