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

[Aaron Conole]

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

Hi Wenxu,

Paolo has done a good job reviewing, so I won't look too much at the
code, but I think we might want to include a test or two in the
system-traffic.at file that can catch these NAT collision cases.  WDYT?

>  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;
>  
>      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;
> +
>      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) {