On Wed, Nov 20, 2019 at 9:07 PM Eli Britstein <el...@mellanox.com> wrote:
>
> Signed-off-by: Eli Britstein <el...@mellanox.com>
> Reviewed-by: Oz Shlomo <o...@mellanox.com>
> ---
> NEWS | 2 +
> lib/netdev-offload-dpdk-flow.c | 87
> ++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 85 insertions(+), 4 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 88b818948..ca9c2b230 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,8 @@ Post-v2.12.0
> if supported by libbpf.
> * Add option to enable, disable and query TCP sequence checking in
> conntrack.
> + - DPDK:
> + * Add hardware offload support for output actions.
>
> v2.12.0 - 03 Sep 2019
> ---------------------
> diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
> index dbbc45e99..6e7efb315 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,76 @@ 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 void
> +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);
> +
> + port_id->id = netdev_dpdk_get_port_id(outdev);
> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
> +}
> +
> +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;
> +
> + 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_api_supported(outdev)) {
> + VLOG_DBG_RL(&error_rl,
> + "Output to %s cannot be offloaded",
> + netdev_get_name(outdev));
> + return -1;
> + }
> +
> + netdev_dpdk_flow_add_count_action(actions);
> + netdev_dpdk_flow_add_port_id_action(actions, outdev);
> + netdev_close(outdev);
> +
> + return 0;
> +}
> +
> 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;
>
> 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 &&
> + left <= NLA_ALIGN(nla->nla_len)) {
> + if (netdev_dpdk_flow_add_output_action(actions, nla, info )) {
> + return -1;
> + }
The check in netdev_dpdk_flow_add_output_action() is insufficient to
decide whether the device is capable of full-offload, since it assumes
that the output action is supported if netdev_dpdk type is
DPDK_DEV_ETH.
There are a couple of issues with this approach:
1. For a device that is not capable of full-offload, it results in 2
calls to the PMD for every flow-offload request. The first attempt for
full-offload fails, followed by another call for partial-offload.
2. Even for devices that support full-offload, the offload might fail
if the in-port and out-port are not in the same switch-domain. For
example, if only partial-offload is configured (vhost-user ports) in
the vSwitch, then the PMD should check if the in-port and out-port are
on the same switch (domain-id must match) and fail the request
otherwise. This check is important to alert the user of
misconfigurations. If OVS doesn't do this check, all pmd drivers will
have to do this check and that duplication is not desirable.
Here's how this could be addressed, by identifying whether the device
is capable of full-offload or partial-offload.
1. Check if the target device (in-port) is full-offload capable. This
could be done by checking if rte_flow api is supported, along with a
valid switch-domain-id. We could even improve this further, similar to
the kernel switchdev framework in which the device mode is
configurable (legacy/eswitch modes) and available to consumers, but
this might require some more DPDK framework changes.
2. If the device is full-offload capable, and if output action is
specified, check if in-port and out-port are on the same switch
(switch-domain-id match mentioned above).
If both 1 and 2 succeed, then the device is capable of full-offload
and we can proceed with full-offload item/action setup, followed by
rte_flow_create().
Otherwise, we fallback to partial-offload code path. Here, we can
handle the case where a vhost-user interface is either the in-port or
out-port of the flow being offloaded. Depending on whether the
vhost-user interface is the in-port or out-port, if the other port
associated with the flow is offload capable (DPDK_DEV_ETH), then we
offload the flow to that device. This would help us build further
extensions to the existing partial-offload framework by adding more
actions such as VLAN and Tunnel actions (apart from MARK/RSS), except
the output action that still needs to be processed in software.
Thanks,
-Harsha
> + } else {
> + VLOG_DBG_RL(&error_rl,
> + "Unsupported action type %d", nl_attr_type(nla));
> + return -1;
> + }
> }
>
> add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
> --
> 2.14.5
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
