Hi wenxu, [email protected] writes:
> From: wenxu <[email protected]> > > Splitting the two port range iterations instead of keeping it nested. And > the dst port (in case of DNAT) range would have precedence over the src > manipulation in the resolution. > > Signed-off-by: wenxu <[email protected]> > --- > lib/conntrack.c | 65 > +++++++++++++++++++++++++++------------------------------ > 1 file changed, 31 insertions(+), 34 deletions(-) > > diff --git a/lib/conntrack.c b/lib/conntrack.c > index 44f99f3..a6784ba 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -1036,6 +1036,17 @@ conn_not_found(struct conntrack *ct, struct dp_packet > *pkt, > nat_action_info); > > if (!nat_res) { > + if (nat_action_info->nat_action & NAT_ACTION_DST) { > + struct nat_action_info_t tmp_nat_info; > + > + memset(&tmp_nat_info, 0, sizeof tmp_nat_info); > + tmp_nat_info.nat_action = NAT_ACTION_SRC; > + nat_res = nat_get_unique_tuple(ct, nc, nat_conn, > + &tmp_nat_info); > + if (!nat_res) { > + goto nat_res_exhaustion; > + } > + } > goto nat_res_exhaustion; > } is there any particular reason that brought you to call nat_get_unique_tuple() twice instead of splitting the FOR_EACH with a single call of nat_get_unique_tuple() ? Something like: FOR_EACH_PORT_IN_RANGE(curr_dport, min_dport, max_dport) { ... } FOR_EACH_PORT_IN_RANGE(curr_sport, min_sport, max_sport) { ... } That was my initial understanding. I suspect that approach would require fewer changes as well. > > @@ -2258,12 +2269,12 @@ nat_range_hash(const struct conn *conn, uint32_t > basis, > > /* Ports are stored in host byte order for convenience. */ > static void > -set_sport_range(const struct nat_action_info_t *ni, const struct conn_key *k, > - uint32_t hash, uint16_t *curr, uint16_t *min, > - uint16_t *max) > +set_port_range(const struct nat_action_info_t *ni, const struct conn_key *k, > + uint32_t hash, uint16_t *curr, uint16_t *min, > + uint16_t *max) > { > - if (((ni->nat_action & NAT_ACTION_SNAT_ALL) == NAT_ACTION_SRC) || > - ((ni->nat_action & NAT_ACTION_DST))) { > + if ((ni->nat_action & NAT_ACTION_SRC) && > + (!(ni->nat_action & NAT_ACTION_SRC_PORT))) { > *curr = ntohs(k->src.port); > if (*curr < 512) { > *min = 1; > @@ -2275,6 +2286,10 @@ set_sport_range(const struct nat_action_info_t *ni, > const struct conn_key *k, > *min = MIN_NAT_EPHEMERAL_PORT; > *max = MAX_NAT_EPHEMERAL_PORT; > } > + } else if ((ni->nat_action & NAT_ACTION_DST) && > + (!(ni->nat_action & NAT_ACTION_DST_PORT))) { > + *curr = ntohs(k->dst.port); > + *min = *max = *curr; > } else { > *min = ni->min_port; > *max = ni->max_port; > @@ -2282,21 +2297,6 @@ set_sport_range(const struct nat_action_info_t *ni, > const struct conn_key *k, > } > } > > -static void > -set_dport_range(const struct nat_action_info_t *ni, const struct conn_key *k, > - uint32_t hash, uint16_t *curr, uint16_t *min, > - uint16_t *max) > -{ > - if (ni->nat_action & NAT_ACTION_DST_PORT) { > - *min = ni->min_port; > - *max = ni->max_port; > - *curr = *min + (hash % ((*max - *min) + 1)); > - } else { > - *curr = ntohs(k->dst.port); > - *min = *max = *curr; > - } > -} > - > /* Gets the initial in range address based on the hash. > * Addresses are kept in network order. */ > static void > @@ -2426,8 +2426,7 @@ nat_get_unique_tuple(struct conntrack *ct, const struct > conn *conn, > uint32_t hash = nat_range_hash(conn, ct->hash_basis, nat_info); > 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_port, max_port, curr_port; > > min_addr = nat_info->min_addr; > max_addr = nat_info->max_addr; > @@ -2439,10 +2438,8 @@ 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, > - &min_sport, &max_sport); > - set_dport_range(nat_info, &conn->key, hash, &curr_dport, > - &min_dport, &max_dport); > + set_port_range(nat_info, &conn->key, hash, &curr_port, > + &min_port, &max_port); > > another_round: > store_addr_to_key(&curr_addr, &nat_conn->rev_key, > @@ -2457,14 +2454,14 @@ another_round: > goto next_addr; > } > > - 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) { > - nat_conn->rev_key.dst.port = htons(curr_sport); > - if (!conn_lookup(ct, &nat_conn->rev_key, > - time_msec(), NULL, NULL)) { > - return true; > - } > + FOR_EACH_PORT_IN_RANGE(curr_port, min_port, max_port) { > + if (nat_info->nat_action & NAT_ACTION_SRC) { > + nat_conn->rev_key.dst.port = htons(curr_port); > + } else { > + nat_conn->rev_key.src.port = htons(curr_port); > + } > + if (!conn_lookup(ct, &nat_conn->rev_key, time_msec(), NULL, NULL)) { > + return true; > } > } > > -- > 1.8.3.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
