[ovs-dev] [PATCH v2] conntrack: fix src port selection for DNAT case

[wenxu]

Got it Thanks.



From: Paolo Valerio <pvalerio at redhat.com>
Date: 2021-08-31 22:25:10
To:  wenxu at ucloud.cn,i.maximets at ovn.org
Cc:  dev at openvswitch.org,"dceara at redhat.com" <dceara at redhat.com>
Subject: Re: [PATCH v2] conntrack: fix src port selection for DNAT case>Hello,
>
>wenxu at ucloud.cn writes:
>
>> From: wenxu <wenxu at ucloud.cn>
>>
>> For DNAT case the src port should never modified.
>>
>> Fixes: 61e48c2d1db2 ("conntrack: Handle SNAT with all-zero IP address")
>> Signed-off-by: wenxu <wenxu at ucloud.cn>
>> ---
>>  lib/conntrack.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/conntrack.c b/lib/conntrack.c
>> index 551c206..4566f65 100644
>> --- a/lib/conntrack.c
>> +++ b/lib/conntrack.c
>> @@ -2258,11 +2258,13 @@ set_sport_range(struct nat_action_info_t *ni, const struct conn_key *k,
>>                  uint32_t hash, uint16_t *curr, uint16_t *min,
>>                  uint16_t *max)
>>  {
>> -    if (((ni->nat_action & NAT_ACTION_SNAT_ALL) == NAT_ACTION_SRC) ||
>> -        ((ni->nat_action & NAT_ACTION_DST))) {
>
>the purpose here was to have a more consistent behavior between
>datapaths, allowing, only in case of collision, something like this:
>
>tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=10000,dport=80),reply=(src=10.1.1.2,dst=10.1.1.1,sport=80,dport=10000),protoinfo=(state=ESTABLISHED)
>tcp,orig=(src=10.1.1.1,dst=192.168.2.100,sport=10000,dport=80),reply=(src=10.1.1.2,dst=10.1.1.1,sport=80,dport=10001),protoinfo=(state=ESTABLISHED)
>
>originating both connections from 10.1.1.1 using the same source port.
>
>The kernel datapath does the same:
>
>tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=10000,dport=80),reply=(src=10.1.1.2,dst=10.1.1.1,sport=80,dport=10000),protoinfo=(state=ESTABLISHED)
>tcp,orig=(src=10.1.1.1,dst=192.168.2.100,sport=10000,dport=80),reply=(src=10.1.1.2,dst=10.1.1.1,sport=80,dport=49264),protoinfo=(state=ESTABLISHED)
>
>but picking a random port.
>
>Changing this wouldn't allow the second entry to get created:
>
>ovs-vswitchd[434250]: ovs|00051|conntrack|WARN|Unable to NAT due to tuple space exhaustion - if DoS attack, use firewalling and/or zone partitioning.
>
>is it something we want?
>
>> +    if ((ni->nat_action & NAT_ACTION_SNAT_ALL) == NAT_ACTION_SRC) {
>>          *curr = ntohs(k->src.port);
>>          *min = MIN_NAT_EPHEMERAL_PORT;
>>          *max = MAX_NAT_EPHEMERAL_PORT;
>> +    } else if (ni->nat_action & NAT_ACTION_DST) {
>> +        *curr = ntohs(k->src.port);
>> +        *min = *max = *curr;
>>      } else {
>>          *min = ni->min_port;
>>          *max = ni->max_port;
>> -- 
>> 1.8.3.1
>