Hello, [email protected] writes:
> From: wenxu <[email protected]> > > It is better to choose the origin select port as current port > for each port search round with new address. > > Signed-off-by: wenxu <[email protected]> > --- 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.: - 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
