[ovs-dev] [PATCH v3] conntrack: Reset ct_state when entering a new zone.

[Ilya Maximets]

On 3/20/20 9:44 PM, Aaron Conole wrote:
> Dumitru Ceara <dceara at redhat.com> writes:
> 
>> When a new conntrack zone is entered, the ct_state field is zeroed in
>> order to avoid using state information from different zones.
>>
>> One such scenario is when a packet is double NATed. Assuming two zones
>> and 3 flows performing the following actions in order on the packet:
>> 1. ct(zone=5,nat), recirc
>> 2. ct(zone=1), recirc
>> 3. ct(zone=1,nat)
>>
>> If at step #1 the packet matches an existing NAT entry, it will get
>> translated and pkt->md.ct_state is set to CS_DST_NAT or CS_SRC_NAT.
>> At step #2 the new tuple might match an existing connection and
>> pkt->md.ct_zone is set to 1.
>> If at step #3 the packet matches an existing NAT entry in zone 1,
>> handle_nat() will be called to perform the translation but it will
>> return early because the packet's zone matches the conntrack zone and
>> the ct_state field still contains CS_DST_NAT or CS_SRC_NAT from the
>> translations in zone 5.
>>
>> In order to reliably detect when a packet enters a new conntrack zone
>> we also need to make sure that the pkt->md.ct_zone is properly
>> initialized if pkt->md.ct_state is non-zero. This already happens for
>> most cases. The only exception is when matched conntrack connection is
>> of type CT_CONN_TYPE_UN_NAT and the master connection is missing. To
>> cover this path we now call write_ct_md() in that case too. Remove
>> setting the CS_TRACKED flag as in this case as it will be done by the
>> new call to write_ct_md().
>>
>> CC: Darrell Ball <dlu998 at gmail.com>
>> Fixes: 286de2729955 ("dpdk: Userspace Datapath: Introduce NAT Support.")
>> Acked-by: Ilya Maximets <i.maximets at ovn.org>
>> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
>>
>> ---
>> V3:
>> - Add Ilya's ack and fix "Fixes" tag.
>> - Remove NULL pointer dereference fix as there's already a patch for it:
>>   https://patchwork.ozlabs.org/patch/1257010/
>> V2:
>> - Address Ilya's comments:
>>     - revert changes to pkt_metadata_init().
>>     - update ct_state in process_one() only if ct_state is already
>>       non-zero.
>> - Make sure pkt->md.ct_zone is always initialized when pkt->md.ct_state
>>   is non-zero.
>> - Fix NULL pointer dereference in process_one() if conn_type is
>>   CT_CONN_TYPE_UN_NAT and master conn is not found.
>> ---
> 
> Acked-by: Aaron Conole <aconole at redhat.com>
> 


Thanks, Dumitru and Aaron!
Applied to master and backported down to 2.8.

Best regards, Ilya Maximets.