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

[Ilya Maximets]

On 6/16/21 1:30 AM, Ansis wrote:
> On Tue, Jun 15, 2021 at 3:02 PM William Tu <u9012063 at gmail.com> wrote:
>>
>> 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.
> 
> ok in that case I am pushing it to master.


Hi, Ansis.  Thanks for taking care of this patch!
Since it's a bug fix, could you, please, also backport it down
to 2.13 LTS (including all newer branches, of course) ?

Best regards, Ilya Maximets.