[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 06:43:55 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.:


" In that case the first IP iteration uses the packet source port, whereas the others don't"


I think the above rule is not matter with the curr_sport picking different ranges?
So you means for source ports < 1024, each IP iteration should using different source port?




> >- 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