On 1/4/24 12:53, Ilya Maximets wrote:
> On 1/4/24 12:34, Simon Horman wrote:
>> On Tue, Dec 19, 2023 at 02:26:36PM +0100, Dumitru Ceara wrote:
>>> On 12/12/23 18:43, Mike Pattrick wrote:
>>>> In the case of a weak reference, clearing all change_seqno's can delete
>>>> useful information. Instead of clearing all seqno's when removing
>>>> track_node, only clear those values in cases including row insertion,
>>>> and row deleting if no dst_arcs remain.
>>>>
>>>> Fixes: 95689f166818 ("ovsdb-idl: Preserve references for deleted rows.")
>>>> Reported-at: https://issues.redhat.com/browse/FDP-193
>>>> Signed-off-by: Mike Pattrick <[email protected]>
>>>>
>>>> ---
>>>
>>> Thanks a lot for fixing this, Mike!
>>>
>>> Acked-by: Dumitru Ceara <[email protected]>
>>>
>>> I think I'd squash patch 2/2 into this one though. In which case, if
>>> maintainers agree, I'd ask them to remove my signed-off/co-authored-by
>>> from 2/2 and keep my ack. The test case was just the easiest way of
>>> creating a sane bug report. :)
>>
>> Can do.
>> I will work on applying the patch-set in the way you describe.
>
> Hi, Simon, others.
>
> + [[000: simple6: conditions unchanged
> +000: simple: conditions unchanged
> +001: table simple6: inserted row: name=row0_s6 weak_ref=[<0>] uuid=<1>
> +001: table simple6: updated columns: name weak_ref
> +001: table simple: inserted row: i=0 r=0 b=false s=row0_s u=<2> ia=[] ra=[]
> ba=[] sa=[] ua=[] uuid=<0>
> +001: table simple: updated columns: s
> +002: {"error":null,"result":[{"uuid":["uuid","<3>"]},{"count":1}]}
> +003: {"error":null,"result":[{"count":1}]}
> +004: table simple6: name=row0_s6 weak_ref=[<0>] uuid=<1>
>
> Yesterday we discussed the test with Mike off-list and I thought that
> the row in simple6 is still referencing the deleted row. With it
> being potentially a result of the other IDL issue we have:
>
> https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/
> (This one needs a a better fix, I'll ask if the author is still working
> on a new version.)
>
> But it actually references a previously added row with uuid=<0>, which
> is fine.
>
> Mike, Dumitru, can you confirm?
>
I confirm, thanks for double checking.
> If that's so, then the test should be fine as-is.
> FWIW, the fix will need to be backported all the way down to 2.17.
> And I agree that it's better to squash the patches into one.
>
Cool.
> Best regards, Ilya Maximets.
>
Thanks,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
