On Mon, Nov 23, 2020 at 6:45 AM Cpp Code <[email protected]> wrote: > > I would expect such checks to be commented as indeed it was not clear why > it was there. I looked in similar places where FLOW_TNL_F_UDPIF is used > and there is no such check. The commit which created this condition is > quite large so I am not able to conclude from that. > I assume this is an optimization because if we don't set > OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS there is less code to be executed later > when the parsing is done. I believe the assumption was made to exclude this > without considering specifics of Geneve related FLOW_TNL_F_UDPIF flag. > > > On Fri, Nov 20, 2020 at 8:52 AM Ben Pfaff <[email protected]> wrote: > > > On Fri, Nov 20, 2020 at 05:12:46AM -0800, Toms Atteka 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. > > > > > > Reported-at: https://github.com/vmware-tanzu/antrea/issues/897 > > > Signed-off-by: Toms Atteka <[email protected]> > > > > Why was the check there? It seems strange to have a very specific "if" > > test and then just remove it without some idea of why it was there to > > begin with. > > > > ... > > > > > - } else if (flow->metadata.present.len || is_mask) { > > > + } else { > >
Hi Toms and Ben, I think the root cause for the reported issue observed on Antrea is because the netdev native tunnel always sets FLOW_TNL_F_UDPIF no matter the geneve option is present or not. So Toms's fix here will always generate OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS so that FLOW_TNL_F_UDPIF can be set when we convert the netlink attributes back to flow key. Instead of fixing the issue on this function, I think we can fix on the tunnel receive function, and only set FLOW_TNL_F_UDPIF when the geneve tunnel metadata is present. Here is the proposed fix: https://mail.openvswitch.org/pipermail/ovs-dev/2020-December/378711.html Thanks, -Yi-Hung _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
