[ovs-dev] [PATCH v15 7/7] netdev-offload-tc: Add offload support for sFlow

Eelco Chaudron echaudro at redhat.com
Fri Oct 1 13:55:26 UTC 2021


One small nit below.

On 15 Sep 2021, at 14:43, Chris Mi wrote:

> Create a unique group ID to map the sFlow info when offloading sFlow
> action to TC. When showing the offloaded datapath flows, translate the
> group ID from TC sample action to sFlow info using the mapping.
>
> Signed-off-by: Chris Mi <cmi at nvidia.com>
> Reviewed-by: Eli Britstein <elibr at nvidia.com>
> ---
>  NEWS                    |   1 +
>  lib/netdev-offload-tc.c | 220 ++++++++++++++++++++++++++++++++++++----
>  lib/tc.c                |  61 ++++++++++-
>  lib/tc.h                |  15 ++-
>  4 files changed, 275 insertions(+), 22 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 1f2adf718..974a3e8e0 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -8,6 +8,7 @@ Post-v2.16.0
>         by default.  'other_config:dpdk-socket-limit' can be set equal to
>         the 'other_config:dpdk-socket-mem' to preserve the legacy memory
>         limiting behavior.
> +   - Add sFlow offload support for kernel (netlink) datapath.
>
>
>  v2.16.0 - 16 Aug 2021
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index 5d817b6cf..4f603566c 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -20,6 +20,7 @@
>  #include <linux/if_ether.h>
>
>  #include "dpif.h"
> +#include "dpif-offload-provider.h"
>  #include "hash.h"
>  #include "openvswitch/hmap.h"
>  #include "openvswitch/match.h"
> @@ -1053,6 +1054,19 @@ parse_tc_flower_to_match(struct tc_flower *flower,
>          action = flower->actions;
>          for (i = 0; i < flower->action_count; i++, action++) {
>              switch (action->type) {
> +            case TC_ACT_SAMPLE: {
> +                const struct sgid_node *node;
> +
> +                node = sgid_find(action->sample.group_id);
> +                if (!node) {
> +                    VLOG_WARN("%s: sgid node is NULL, sgid:  ",
> +                              __func__, action->sample.group_id);

Can we remove the function call, and maybe tweak the message to something like (also note gid is unsigned, so needs %u) :

“Can’t find sample group ID data for ID: %u”

> +                    return ENOENT;
> +                }
> +                nl_msg_put(buf, node->sflow.action,
> +                           node->sflow.action->nla_len);
> +            }
> +            break;
>              case TC_ACT_VLAN_POP: {
>                  nl_msg_put_flag(buf, OVS_ACTION_ATTR_POP_VLAN);
>

<SNIP>



More information about the dev mailing list