On Mon, Oct 12, 2020 at 4:48 PM Eli Britstein <[email protected]> wrote:
>
>
> On 10/12/2020 10:20 AM, Sriharsha Basavapatna wrote:
> > On Sun, Sep 6, 2020 at 5:51 PM Eli Britstein <[email protected]> wrote:
> >> ping
> >>
> >> On 7/30/2020 1:58 PM, Eli Britstein wrote:
> >>> From: Lei Wang <[email protected]>
> >>>
> >>> 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 <[email protected]>
> >>> Reviewed-by: Eli Britstein <[email protected]>
> >>> Reviewed-by: Gaetan Rivet <[email protected]>
> >>> Signed-off-by: Eli Britstein <[email protected]>

Acked-by: Sriharsha Basavapatna <[email protected]>

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
> >> [email protected]
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to