Ilya Maximets <i.maxim...@ovn.org> writes:
> On 6/5/24 16:54, Aaron Conole wrote:
>> Mike Pattrick <m...@redhat.com> writes:
>>
>>> When conntrack is reassembling packet fragments, the same reassembly
>>> context can be shared across multiple threads handling different packets
>>> simultaneously. Once a full packet is assembled, it is added to a packet
>>> batch for processing, in the case where there are multiple different pmd
>>> threads accessing conntrack simultaneously, there is a race condition
>>> where the reassembled packet may be added to an arbitrary batch even if
>>> the current batch is available.
>>>
>>> When this happens, the packet may be handled incorrectly as it is
>>> inserted into a random openflow execution pipeline, instead of the
>>> pipeline for that packets flow.
>>>
>>> This change makes a best effort attempt to try to add the defragmented
>>> packet to the current batch. directly. This should succeed most of the
>>> time.
>>>
>>> Fixes: 4ea96698f667 ("Userspace datapath: Add fragmentation handling.")
>>> Reported-at: https://issues.redhat.com/browse/FDP-560
>>> Signed-off-by: Mike Pattrick <m...@redhat.com>
>>> ---
>>
>> The patch overall looks good to me. I'm considering applying with the
>> following addition:
>>
>> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
>> index 6b293770dd..d9b9e0c23f 100755
>> --- a/utilities/checkpatch.py
>> +++ b/utilities/checkpatch.py
>> @@ -63,7 +63,8 @@ def open_spell_check_dict():
>> 'dhcpv6', 'opts', 'metadata', 'geneve', 'mutex',
>> 'netdev', 'netdevs', 'subtable', 'virtio',
>> 'qos',
>> 'policer', 'datapath', 'tunctl', 'attr',
>> 'ethernet',
>> - 'ether', 'defrag', 'defragment', 'loopback',
>> 'sflow',
>> + 'ether', 'defrag', 'defragment', 'defragmented',
>> + 'loopback', 'sflow',
>> 'acl', 'initializer', 'recirc', 'xlated',
>> 'unclosed',
>> 'netlink', 'msec', 'usec', 'nsec', 'ms', 'us',
>> 'ns',
>> 'kilobits', 'kbps', 'kilobytes', 'megabytes',
>> 'mbps',
>>
>>
>> unless anyone objects. This is to squelch:
>>
>> == Checking 16f6885353c2 ("ipf: Handle common case of ipf defragmentation.")
>> ==
>> WARNING: Possible misspelled word: "defragmented"
>> Did you mean: ['defragment ed', 'defragment-ed', 'defragment']
>> Lines checked: 129, Warnings: 1, Errors: 0
>
> It doesn't affect CI today, so can be a separate patch, I think. We
> have a few more
> words like this in relatively recent commits, like 'poller' or
> 'autovalidator', these
> can be bundled in that separate commit as well.
Good point - okay, I'll make a separate commit for it.
> Though updating the dictionary along with the patch that is using the
> word sounds OK
> to me as well.
Actually, I was thinking that it has the downside of sometimes making
backports more tricky as well, so I ended up not including this change.
> Best regards, Ilya Maximets.
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
