On 12 Oct 2021, at 9:05, Chris Mi wrote:
> On 10/12/2021 2:53 PM, Eelco Chaudron wrote: >> >> On 9 Oct 2021, at 10:16, Chris Mi wrote: >> >>> On 10/1/2021 5:43 PM, Eelco Chaudron wrote: >>>> On 1 Oct 2021, at 11:35, Eelco Chaudron wrote: >>>> >>>>> Hi Chris, >>>>> >>>>> I just started to review this patchset, but as some of the v14 >>>>> discussions have not finished, I’ll copy them over to v15. This way, all >>>>> the open items are contained in a single thread/revision. I would suggest >>>>> clear up the open items before you send a new revision. >>>>> >>>>> This is only a visual review, as I did not test the code, assuming we >>>>> need a new revision anyway with the test cases integrated. >>>>> >>>>> Cheers, >>>>> >>>>> Eelco >>>> Forgot about this issue (see below), was this fixed in v15? I did not see >>>> any reply to the email chain? >>> I still can't reproduce the issue locally, so I didn't reply the email >>> explicitly. But I revised the code in v15. >>> I think the change should fix it. In function dpif_sflow_attr_equal() of >>> v14, only ufid is compared. In v15, >>> all the fields are compared. That will prevent the gid mapping to be reused >>> wrongly when the flow updates >>> the action for the same ufid. Since this issue is a corner case, in >>> dpif_sflow_attr_hash(), only ufid is hashed to save CPU cycles. >>> Hopefully that will fix the issue. 😁 >> I did some testing, and I can no longer replicate the issue with v15 (and it >> comes back as soon as I load v14). So I guess the change you mention fixed >> it. >> >> //Eelco >> >> <SNIP> >> > Thanks a lot for verifying it, Eelco. > > I'll send v16 with the test case integrated today for further review. Ack, will try to review and test it next week. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
