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

Sriharsha Basavapatna sriharsha.basavapatna at broadcom.com
Tue Oct 20 10:12:29 UTC 2020


On Tue, Oct 20, 2020 at 2:40 PM Ilya Maximets <i.maximets at ovn.org> wrote:
>
> On 10/20/20 10:51 AM, Sriharsha Basavapatna wrote:
> > On Mon, Oct 19, 2020 at 7:59 PM Ilya Maximets <i.maximets at ovn.org> wrote:
> >>
> >> On 10/18/20 9:10 AM, Eli Britstein wrote:
> >>> 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. Fix it by checking if tunnel is
> >>> present.
> >>>
> >>> Signed-off-by: Eli Britstein <elibr at nvidia.com>
> >>> ---
> >>
> >> Thanks, Eli.
> >>
> >> Harsha, Emma, could you, please, review/test this version?
> >>
> >> Best regards, Ilya Maximets.
> >
> > Reviewed this change. Do we even need this backported to 2.13, since
> > we don't support clone/tnl-push actions with offload ?
>
> I think we could have metadata partially set even if we're not performing
> clone/tnl-push actions.  Maybe something like this:
>
>   table=0,in_port=1,ip,actions=load:0xbba->NXM_NX_TUN_ID[],goto_table(1)
>   table=1,ip,nw_src=192.168.0.1,actions=load:0xa566c10->NXM_NX_TUN_IPV4_DST[],<out to tunnel>
>   table=1,ip,nw_src=192.168.0.2,actions=drop
>
> In this scenario packet that goes to 'drop' action might have tun_id set
> in the metadata and we will not offload it.  I didn't test that though.
> Does that make sense?

I tested with the above set of rules. It doesn't fail in
validate_flow() even without this fix, since it checks
match_zero_wc.flow.tunnel and not match_zero_wc.masks.tunnel which has
the tun_id (mask) set. So, this fix doesn't make any difference
(validate succeeds with or without it).

(gdb) p /x match_zero_wc.flow.tunnel.tun_id
$4 = 0x0
(gdb) p /x match_zero_wc.wc.masks.tunnel.tun_id
$5 = 0xffffffffffffffff

Thanks,
-Harsha
>
> >
> > Thanks,
> > -Harsha
> >>
> >>>  lib/netdev-offload-dpdk.c | 3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> >>> index 4538baf5e..c68d539ea 100644
> >>> --- a/lib/netdev-offload-dpdk.c
> >>> +++ b/lib/netdev-offload-dpdk.c
> >>> @@ -1092,7 +1092,8 @@ netdev_offload_dpdk_validate_flow(const struct match *match)
> >>>      /* Create a wc-zeroed version of flow. */
> >>>      match_init(&match_zero_wc, &match->flow, &match->wc);
> >>>
> >>> -    if (!is_all_zeros(&match_zero_wc.flow.tunnel,
> >>> +    if (flow_tnl_dst_is_set(&match->flow.tunnel) &&
> >>> +        !is_all_zeros(&match_zero_wc.flow.tunnel,
> >>>                        sizeof match_zero_wc.flow.tunnel)) {
> >>>          goto err;
> >>>      }
> >>>
> >>
>


More information about the dev mailing list