[ovs-dev] [PATCH 1/1] netdev-offload-dpdk: Support vxlan encap offload with load actions

[Sriharsha Basavapatna]

On Mon, Oct 12, 2020 at 4:48 PM Eli Britstein <elibr at nvidia.com> wrote:
>
>
> On 10/12/2020 10:20 AM, Sriharsha Basavapatna wrote:
> > On Sun, Sep 6, 2020 at 5:51 PM Eli Britstein <elibr at nvidia.com> wrote:
> >> ping
> >>
> >> On 7/30/2020 1:58 PM, Eli Britstein wrote:
> >>> From: Lei Wang <leiw at mellanox.com>
> >>>
> >>> Struct match has the tunnel values/masks in
> >>> match->flow.tunnel/match->wc.masks.tunnel.
> >>> Load actions such as load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
> >>> load:0xbba->NXM_NX_TUN_ID[] are utilizing the tunnel masks fields,
> >>> but those should not be used for matching.
> >>> Offloading fails if masks is not clear. Clear it if no tunnel used.
> >>>
> >>> Signed-off-by: Lei Wang <leiw at mellanox.com>
> >>> Reviewed-by: Eli Britstein <elibr at mellanox.com>
> >>> Reviewed-by: Gaetan Rivet <gaetanr at mellanox.com>
> >>> Signed-off-by: Eli Britstein <elibr at mellanox.com>

Acked-by: Sriharsha Basavapatna <sriharsha.basavapatna at broadcom.com>

See my comment below.

> >>> ---
> >>>    lib/netdev-offload-dpdk.c | 4 ++++
> >>>    1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> >>> index de6101e4d..0d23e4879 100644
> >>> --- a/lib/netdev-offload-dpdk.c
> >>> +++ b/lib/netdev-offload-dpdk.c
> >>> @@ -682,6 +682,10 @@ parse_flow_match(struct flow_patterns *patterns,
> >>>
> >>>        consumed_masks = &match->wc.masks;
> >>>
> >>> +    if (!flow_tnl_dst_is_set(&match->flow.tunnel)) {
> >>> +        memset(&match->wc.masks.tunnel, 0, sizeof match->wc.masks.tunnel);
> >>> +    }
> >>> +
> > This fix looks good to me.  Did you take a look to see if this can be
> > fixed in the code that generates these invalid masks in the first
> > place ?
> I wouldn't say it's "invalid masks". OVS takes those masks into
> consideration with such usage of flow_tnl_dst_is_set, that is done in
> several places in the code. For example odp-util.c, lib/netdev-offload-tc.c.

It is invalid because the datapath rule is specifying tunnel masks
when we are not really matching on them. In the context of encap
actions (which is what this patch is fixing), tunnel masks shouldn't
be set.

> >
> > Thanks,
> > -Harsha
> >>>        memset(&consumed_masks->in_port, 0, sizeof consumed_masks->in_port);
> >>>        /* recirc id must be zero. */
> >>>        if (match->wc.masks.recirc_id & match->flow.recirc_id) {
> >> _______________________________________________
> >> dev mailing list
> >> dev at openvswitch.org
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev