[ovs-dev] [PATCH v4] netlink: removed incorrect optimization

[William Tu]

On Tue, Jun 15, 2021 at 11:43 AM Ansis <ansisatteka at gmail.com> wrote:
>
> On Mon, Jun 14, 2021 at 10:22 PM Ansis <ansisatteka at gmail.com> wrote:
> >
> > On Mon, Jun 7, 2021 at 1:31 PM Toms Atteka <cpp.code.lv at gmail.com> wrote:
> > >
> > > This optimization caused FLOW_TNL_F_UDPIF flag not to be used in
> > > hash calculation for geneve tunnel when revalidating flows which
> > > resulted in different cache hash values and incorrect behaviour.
> > >
> > > Added test to prevent regression.
> > >
> > > CC: Jesse Gross <jesse at nicira.com>
> > > Fixes: 6728d578f64e ("dpif-netdev: Translate Geneve options per-flow, not per-packet.")
> > > Reported-at: https://github.com/vmware-tanzu/antrea/issues/897
> > > Signed-off-by: Toms Atteka <cpp.code.lv at gmail.com>
> > > ---
> > >  lib/tun-metadata.c      |  2 +-
> > >  tests/system-traffic.at | 54 +++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 55 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/tun-metadata.c b/lib/tun-metadata.c
> > > index c0b0ae044..af0bcbde8 100644
> > > --- a/lib/tun-metadata.c
> > > +++ b/lib/tun-metadata.c
> > > @@ -828,7 +828,7 @@ tun_metadata_to_geneve_nlattr(const struct flow_tnl *tun,
> > >          } else {
> > >              tun_metadata_to_geneve_nlattr_mask(key, tun, flow, b);
> > >          }
> > > -    } else if (flow->metadata.present.len || is_mask) {
> > > +    } else {
> >
> > I reverted the line above, but the regression prevention test you
> > added below still seems to pass. So - does this regression prevention
> > test serve any purpose or am I doing something wrong? Here is proof:
> >
> >  18: datapath - n ok
>
> I take it back. I ran make check-kernel. I had to run make
> check-system-userspace to see your regression test in action.
>
> William, do you have any comments for this patch? You indicated you
> wanted to look at it too.
>
Don't have any comments.
Thanks.
William