On 6/5/24 16:54, Aaron Conole wrote:
> Mike Pattrick <[email protected]> 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 <[email protected]>
>> ---
> 
> 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.

Though updating the dictionary along with the patch that is using the word 
sounds OK
to me as well.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to