[ovs-dev] [PATCH v5 1/3] conntrack: restore the origin sport for each round with new address

Paolo Valerio pvalerio at redhat.com
Thu Oct 7 15:40:53 UTC 2021


Hi Wenxu,

sorry for the late review.
This patch is mostly a preparation for the #3.
Your last patch needs that both src and dst get restored.

My previous suggestion was to squash patch #1 with #3 as it mostly made
sense in that context, considering that without #3 restoring dst was
redundant in all cases and restoring src was redundant in all cases but
one.

When I asked if we needed dst, the comment was about patch #1 as a
standalone. In patch #1 we didn't need it whereas in patch #3 we did.

I still believe that squashing src and dst restoration in #3 is more
consistent. All in all, I think it would result in a cleaner history.

wenxu at ucloud.cn writes:

> From: wenxu <wenxu at ucloud.cn>
>
> It is better to choose the origin select sport as current sport
> for each port search round with new 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 33a1a92..76c466c 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -2416,10 +2416,10 @@ nat_get_unique_tuple(struct conntrack *ct, const struct conn *conn,
>      union ct_addr min_addr = {0}, max_addr = {0}, curr_addr = {0},
>                    guard_addr = {0};
>      uint32_t hash = nat_range_hash(conn, ct->hash_basis, nat_info);
> +    uint16_t min_sport, max_sport, curr_sport, orig_sport;
>      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;
>  
>      min_addr = nat_info->min_addr;
>      max_addr = nat_info->max_addr;
> @@ -2431,7 +2431,7 @@ 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(nat_info, &conn->key, hash, &curr_sport,
> +    set_sport_range(nat_info, &conn->key, hash, &orig_sport,
>                      &min_sport, &max_sport);
>      set_dport_range(nat_info, &conn->key, hash, &curr_dport,
>                      &min_dport, &max_dport);
> @@ -2449,6 +2449,8 @@ another_round:
>          goto next_addr;
>      }
>  
> +    curr_sport = orig_sport;
> +
>      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