[ovs-dev] [PATCH v2 1/2] conntrack: restore the origin port for each round with new address

wenxu wenxu at ucloud.cn
Tue Sep 7 02:23:57 UTC 2021









From: Paolo Valerio <pvalerio at redhat.com>
Date: 2021-09-06 19:00:37
To:  wenxu at ucloud.cn,i.maximets at ovn.org,dlu998 at gmail.com,aconole at redhat.com
Cc:  dev at openvswitch.org
Subject: Re: [PATCH v2 1/2] conntrack: restore the origin port for each round with new address>Hello,
>
>wenxu at ucloud.cn writes:
>
>> From: wenxu <wenxu at ucloud.cn>
>>
>> It is better to choose the origin select port as current port
>> for each port search round with new address.
>>
>> Signed-off-by: wenxu <wenxu at ucloud.cn>
>> ---
>
>This should happen normally.
>It doesn't happen in the case of source port manipulation when the
>default ephemeral range is used and the packet source port is below
>1024. In that case the first IP iteration uses the packet source port,
>whereas the others don't.
>
>if we want to change this behavior, there are some more ways we can
>consider, e.g.:


This can be done for avoding keep old_sport for source ports < 1024 case. 
>
>- Using 1024 as initial curr_sport for source ports < 1024.
>- picking different default ranges
>  - e.g. the kernel uses the range [1, 511], if the source port is < 512,
>    [600, 1023], if it's >= 512 and < 1024, and [1024,65535] for the
>    remaining ports.
>
>What do you think if we do something like the third bullet and squash
>this change, if needed, in your next patch?
>
>In any case, there are a couple of small nits/questions, see inline.
>
>>  lib/conntrack.c | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/conntrack.c b/lib/conntrack.c
>> index 551c206..2d14205 100644
>> --- a/lib/conntrack.c
>> +++ b/lib/conntrack.c
>> @@ -2412,8 +2412,8 @@ nat_get_unique_tuple(struct conntrack *ct, const struct conn *conn,
>>      uint32_t hash = nat_range_hash(conn, ct->hash_basis);
>>      bool pat_proto = conn->key.nw_proto == IPPROTO_TCP ||
>>                       conn->key.nw_proto == IPPROTO_UDP;
>> -    uint16_t min_dport, max_dport, curr_dport;
>> -    uint16_t min_sport, max_sport, curr_sport;
>> +    uint16_t min_dport, max_dport, curr_dport, orig_dport;
>> +    uint16_t min_sport, max_sport, curr_sport, orig_sport;
>
>can we keep the reverse xmas tree style here?
>
>>  
>>      min_addr = conn->nat_info->min_addr;
>>      max_addr = conn->nat_info->max_addr;
>> @@ -2425,9 +2425,9 @@ nat_get_unique_tuple(struct conntrack *ct, const struct conn *conn,
>>       * we can stop once we reach it. */
>>      guard_addr = curr_addr;
>>  
>> -    set_sport_range(conn->nat_info, &conn->key, hash, &curr_sport,
>> +    set_sport_range(conn->nat_info, &conn->key, hash, &orig_sport,
>>                      &min_sport, &max_sport);
>> -    set_dport_range(conn->nat_info, &conn->key, hash, &curr_dport,
>> +    set_dport_range(conn->nat_info, &conn->key, hash, &orig_dport,
>>                      &min_dport, &max_dport);
>>  
>>  another_round:
>> @@ -2443,6 +2443,9 @@ another_round:
>>          goto next_addr;
>>      }
>>  
>> +    curr_sport = orig_sport;
>> +    curr_dport = orig_dport;
>> +
>
>can this happen with dport?
>
>>      FOR_EACH_PORT_IN_RANGE(curr_dport, min_dport, max_dport) {
>>          nat_conn->rev_key.src.port = htons(curr_dport);
>>          FOR_EACH_PORT_IN_RANGE(curr_sport, min_sport, max_sport) {
>> -- 
>> 1.8.3.1
>






More information about the dev mailing list