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 <b...@ovn.org> 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 <cpp.code...@gmail.com> > > 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 { > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
