On Thu, Feb 13, 2025 at 02:33:44PM +0100, Martin Kalcok wrote: > From: Dumitru Ceara <[email protected]> > > We can do this early, in the en_lr_nat node. That allows future commits > to reuse the parsed information later in the I-P run without having to, > reprocess NB contents. > > While at it clean up a bit the nat entry parsing and checking. We were > re-parsing external_ips, for example, twice even though we already had > the parsed IPs available in the internal record. Also the code now > tries to separate parsing and checking a bit as they were inter-twined > and very hard to understand.
Acked-By: Felix Huettner <[email protected]> > > Signed-off-by: Dumitru Ceara <[email protected]> > --- > northd/en-lr-nat.c | 234 +++++++++++++++++++++++++++++++++++---- > northd/en-lr-nat.h | 24 ++-- > northd/northd.c | 270 ++++++++++++--------------------------------- > northd/northd.h | 3 + > 4 files changed, 292 insertions(+), 239 deletions(-) > > diff --git a/northd/en-lr-nat.c b/northd/en-lr-nat.c > index bdbb2c860..44009ca6f 100644 > --- a/northd/en-lr-nat.c > +++ b/northd/en-lr-nat.c > @@ -43,17 +43,21 @@ static void lr_nat_table_init(struct lr_nat_table *); > static void lr_nat_table_clear(struct lr_nat_table *); > static void lr_nat_table_destroy(struct lr_nat_table *); > static void lr_nat_table_build(struct lr_nat_table *, > - const struct ovn_datapaths *lr_datapaths); > + const struct ovn_datapaths *lr_datapaths, > + const struct hmap *lr_ports); > static struct lr_nat_record *lr_nat_table_find_by_index_( > const struct lr_nat_table *, size_t od_index); > > -static struct lr_nat_record *lr_nat_record_create( > - struct lr_nat_table *, const struct ovn_datapath *); > +static struct lr_nat_record *lr_nat_record_create(struct lr_nat_table *, > + const struct ovn_datapath > *, > + const struct hmap > *lr_ports); > static void lr_nat_record_init(struct lr_nat_record *, > - const struct ovn_datapath *); > + const struct ovn_datapath *, > + const struct hmap *lr_ports); > static void lr_nat_record_clear(struct lr_nat_record *); > static void lr_nat_record_reinit(struct lr_nat_record *, > - const struct ovn_datapath *); > + const struct ovn_datapath *, > + const struct hmap *lr_ports); > static void lr_nat_record_destroy(struct lr_nat_record *); > > static bool get_force_snat_ip(const struct ovn_datapath *, > @@ -105,7 +109,8 @@ en_lr_nat_run(struct engine_node *node, void *data_) > > stopwatch_start(LR_NAT_RUN_STOPWATCH_NAME, time_msec()); > lr_nat_table_clear(&data->lr_nats); > - lr_nat_table_build(&data->lr_nats, &northd_data->lr_datapaths); > + lr_nat_table_build(&data->lr_nats, &northd_data->lr_datapaths, > + &northd_data->lr_ports); > > stopwatch_stop(LR_NAT_RUN_STOPWATCH_NAME, time_msec()); > engine_set_node_state(node, EN_UPDATED); > @@ -133,7 +138,7 @@ lr_nat_northd_handler(struct engine_node *node, void > *data_) > od = hmapx_node->data; > lrnat_rec = lr_nat_table_find_by_index_(&data->lr_nats, od->index); > ovs_assert(lrnat_rec); > - lr_nat_record_reinit(lrnat_rec, od); > + lr_nat_record_reinit(lrnat_rec, od, &northd_data->lr_ports); > > /* Add the lrnet rec to the tracking data. */ > hmapx_add(&data->trk_data.crupdated, lrnat_rec); > @@ -169,14 +174,15 @@ lr_nat_table_clear(struct lr_nat_table *table) > > static void > lr_nat_table_build(struct lr_nat_table *table, > - const struct ovn_datapaths *lr_datapaths) > + const struct ovn_datapaths *lr_datapaths, > + const struct hmap *lr_ports) > { > table->array = xrealloc(table->array, > ods_size(lr_datapaths) * sizeof *table->array); > > const struct ovn_datapath *od; > HMAP_FOR_EACH (od, key_node, &lr_datapaths->datapaths) { > - lr_nat_record_create(table, od); > + lr_nat_record_create(table, od, lr_ports); > } > } > > @@ -198,12 +204,13 @@ lr_nat_table_find_by_index_(const struct lr_nat_table > *table, > > static struct lr_nat_record * > lr_nat_record_create(struct lr_nat_table *table, > - const struct ovn_datapath *od) > + const struct ovn_datapath *od, > + const struct hmap *lr_ports) > { > ovs_assert(od->nbr); > > struct lr_nat_record *lrnat_rec = xzalloc(sizeof *lrnat_rec); > - lr_nat_record_init(lrnat_rec, od); > + lr_nat_record_init(lrnat_rec, od, lr_ports); > > hmap_insert(&table->entries, &lrnat_rec->key_node, > uuid_hash(&od->nbr->header_.uuid)); > @@ -211,9 +218,142 @@ lr_nat_record_create(struct lr_nat_table *table, > return lrnat_rec; > } > > +/* Returns true if a 'nat_entry' is valid, i.e.: > + * - parsing was successful. > + * - the string yielded exactly one IPv4 address or exactly one IPv6 address. > + */ > +static bool > +lr_nat_entry_ext_addrs_valid(const struct ovn_nat *nat_entry) > +{ > + const struct lport_addresses *ext_addrs = &nat_entry->ext_addrs; > + > + return (ext_addrs->n_ipv4_addrs == 1 && ext_addrs->n_ipv6_addrs == 0) || > + (ext_addrs->n_ipv4_addrs == 0 && ext_addrs->n_ipv6_addrs == 1); > +} > + > +/* Populates 'nat_entry->logical_ip_cidr_bits'. SNAT rules can have > + * subnets as logical IPs. Their prefix length is used to generate > + * NAT priority (LPM). */ > +static bool > +lr_nat_entry_set_logical_ip_cidr_bits(const struct ovn_datapath *od, > + struct ovn_nat *nat_entry) > +{ > + struct in6_addr ipv6, mask_v6, v6_exact = IN6ADDR_EXACT_INIT; > + const struct nbrec_nat *nat = nat_entry->nb; > + bool is_v6 = nat_entry_is_v6(nat_entry); > + ovs_be32 ip, mask; > + char *error = NULL; > + if (is_v6) { > + error = ipv6_parse_masked(nat->logical_ip, &ipv6, &mask_v6); > + nat_entry->logical_ip_cidr_bits = ipv6_count_cidr_bits(&mask_v6); > + } else { > + error = ip_parse_masked(nat->logical_ip, &ip, &mask); > + nat_entry->logical_ip_cidr_bits = ip_count_cidr_bits(mask); > + } > + if (nat_entry->type == SNAT) { > + if (error) { > + /* Invalid for both IPv4 and IPv6 */ > + static struct vlog_rate_limit rl = > + VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, "bad ip network or ip %s for snat " > + "in router " UUID_FMT, > + nat->logical_ip, UUID_ARGS(&od->key)); > + free(error); > + return false; > + } > + } else { > + if (error || (!is_v6 && mask != OVS_BE32_MAX) > + || (is_v6 && memcmp(&mask_v6, &v6_exact, sizeof mask_v6))) { > + /* Invalid for both IPv4 and IPv6 */ > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, "bad ip %s for dnat in router " > + UUID_FMT, > + nat->logical_ip, UUID_ARGS(&od->key)); > + free(error); > + return false; > + } > + } > + return true; > +} > + > +/* Populates 'nat_entry->l3dgw_port' with the corresponding DGW port > + * (distributed gateway port) for the NAT entry, if any. Sets > + * 'nat_entry->is_distributed' to true if the NAT entry is fully > + * distributed, that is, dnat_and_snat with valid DGW port and mac set. > + * It sets 'nat_entry->is_distributed' to false otherwise. > + * > + * Returns false on failure to find an adequate DGW port. > + * Returns true otherwise. > + * */ > +static bool > +lr_nat_entry_set_dgw_port(const struct ovn_datapath *od, > + struct ovn_nat *nat_entry, > + const struct hmap *lr_ports) > +{ > + const struct nbrec_nat *nat = nat_entry->nb; > + > + /* Validate gateway_port of NAT rule. */ > + nat_entry->l3dgw_port = NULL; > + if (nat->gateway_port == NULL) { > + if (od->n_l3dgw_ports == 1) { > + nat_entry->l3dgw_port = od->l3dgw_ports[0]; > + } else if (od->n_l3dgw_ports > 1) { > + /* Find the DGP reachable for the NAT external IP. */ > + for (size_t i = 0; i < od->n_l3dgw_ports; i++) { > + if (lrp_find_member_ip(od->l3dgw_ports[i], nat->external_ip)) > { > + nat_entry->l3dgw_port = od->l3dgw_ports[i]; > + break; > + } > + } > + if (nat_entry->l3dgw_port == NULL) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, > 1); > + VLOG_WARN_RL(&rl, "Unable to determine gateway_port for NAT " > + "with external_ip: %s configured on logical " > + "router: %s with multiple distributed gateway " > + "ports", nat->external_ip, od->nbr->name); > + return false; > + } > + } > + } else { > + nat_entry->l3dgw_port = > + ovn_port_find(lr_ports, nat->gateway_port->name); > + > + if (!nat_entry->l3dgw_port || nat_entry->l3dgw_port->od != od || > + !lrp_is_l3dgw(nat_entry->l3dgw_port)) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, "gateway_port: %s of NAT configured on " > + "logical router: %s is not a valid distributed " > + "gateway port on that router", > + nat->gateway_port->name, od->nbr->name); > + return false; > + } > + } > + > + /* For distributed router NAT, determine whether this NAT rule > + * satisfies the conditions for distributed NAT processing. */ > + nat_entry->is_distributed = false; > + > + /* NAT cannnot be distributed if the DGP's peer > + * has a chassisredirect port (as the routing is centralized > + * on the gateway chassis for the DGP's networks/subnets.) > + */ > + struct ovn_port *l3dgw_port = nat_entry->l3dgw_port; > + if (l3dgw_port && l3dgw_port->peer && l3dgw_port->peer->cr_port) { > + return true; > + } > + > + if (od->n_l3dgw_ports && nat_entry->type == DNAT_AND_SNAT && > + nat->logical_port && nat->external_mac) { > + nat_entry->is_distributed = true; > + } > + > + return true; > +} > + > static void > lr_nat_record_init(struct lr_nat_record *lrnat_rec, > - const struct ovn_datapath *od) > + const struct ovn_datapath *od, > + const struct hmap *lr_ports) > { > lrnat_rec->lr_index = od->index; > lrnat_rec->nbr_uuid = od->nbr->header_.uuid; > @@ -285,16 +425,64 @@ lr_nat_record_init(struct lr_nat_record *lrnat_rec, > > nat_entry->nb = nat; > nat_entry->is_router_ip = false; > + nat_entry->is_valid = true; > + > + if (!strcmp(nat->type, "snat")) { > + nat_entry->type = SNAT; > + } else if (!strcmp(nat->type, "dnat_and_snat")) { > + nat_entry->type = DNAT_AND_SNAT; > + } else { > + nat_entry->type = DNAT; > + } > + > + if (!extract_ip_addresses(nat->external_ip, &nat_entry->ext_addrs)) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > + > + VLOG_WARN_RL(&rl, "Failed to extract ip address %s " > + "in nat configuration for router %s", > + nat->external_ip, od->nbr->name); > + nat_entry->is_valid = false; > + continue; > + } > + > + if (nat->external_mac && !eth_addr_from_string(nat->external_mac, > + &nat_entry->mac)) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, "bad mac %s for dnat in router " > + ""UUID_FMT"", nat->external_mac, > UUID_ARGS(&od->key)); > + nat_entry->is_valid = false; > + continue; > + } > > - if (!extract_ip_addresses(nat->external_ip, > - &nat_entry->ext_addrs) || > - !nat_entry_is_valid(nat_entry)) { > + if (!lr_nat_entry_ext_addrs_valid(nat_entry)) { > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > > - VLOG_WARN_RL(&rl, > - "Bad ip address %s in nat configuration " > - "for router %s", nat->external_ip, > - od->nbr->name); > + VLOG_WARN_RL(&rl, "Invalid ip address %s in nat configuration " > + "for router %s", > + nat->external_ip, od->nbr->name); > + nat_entry->is_valid = false; > + continue; > + } > + > + if (nat->allowed_ext_ips && nat->exempted_ext_ips) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > + VLOG_WARN_RL(&rl, "NAT rule: " UUID_FMT " not applied, since " > + "both allowed and exempt external ips set", > + UUID_ARGS(&(nat->header_.uuid))); > + nat_entry->is_valid = false; > + continue; > + } > + > + /* Set nat_entry->logical_ip_cidr_bits (SNAT logical_ip can be > + * a subnet). */ > + if (!lr_nat_entry_set_logical_ip_cidr_bits(od, nat_entry)) { > + nat_entry->is_valid = false; > + continue; > + } > + > + /* Set nat_entry->l3dgw_port and nat_entry->is_distributed. */ > + if (!lr_nat_entry_set_dgw_port(od, nat_entry, lr_ports)) { > + nat_entry->is_valid = false; > continue; > } > > @@ -313,11 +501,8 @@ lr_nat_record_init(struct lr_nat_record *lrnat_rec, > nat_entry->ext_addrs.ipv6_addrs[0].addr_s, > nat_entry); > } > - nat_entry->type = SNAT; > } else { > - nat_entry->type = DNAT; > if (!strcmp(nat->type, "dnat_and_snat")) { > - nat_entry->type = DNAT_AND_SNAT; > if (nat->logical_port && nat->external_mac) { > lrnat_rec->has_distributed_nat = true; > } > @@ -348,10 +533,11 @@ lr_nat_record_clear(struct lr_nat_record *lrnat_rec) > > static void > lr_nat_record_reinit(struct lr_nat_record *lrnat_rec, > - const struct ovn_datapath *od) > + const struct ovn_datapath *od, > + const struct hmap *lr_ports) > { > lr_nat_record_clear(lrnat_rec); > - lr_nat_record_init(lrnat_rec, od); > + lr_nat_record_init(lrnat_rec, od, lr_ports); > } > > static void > diff --git a/northd/en-lr-nat.h b/northd/en-lr-nat.h > index 120ceeca9..a73933390 100644 > --- a/northd/en-lr-nat.h > +++ b/northd/en-lr-nat.h > @@ -38,13 +38,22 @@ enum ovn_nat_type { > /* Contains a NAT entry with the external addresses pre-parsed. */ > struct ovn_nat { > const struct nbrec_nat *nb; > - struct lport_addresses ext_addrs; > + struct lport_addresses ext_addrs; /* Parsed NB.NAT.external_ip. */ > + struct eth_addr mac; /* Parsed NB.NAT.external_mac. */ > + int logical_ip_cidr_bits; /* Parsed NB.NATlogical_ip prefix len. > */ > struct ovs_list ext_addr_list_node; /* Linkage in the per-external IP > * list of nat entries. Currently > * only used for SNAT. > */ > bool is_router_ip; /* Indicates if the NAT external_ip is also one of > * router's lrp ip. Can be 'true' only for SNAT. */ > + > + struct ovn_port *l3dgw_port; /* If non-NULL, the distributed gateway port > + * this NAT will use. NULL for gateway > + * routers. */ > + bool is_distributed; /* True if this NAT record is fully > + * distributed. */ > + bool is_valid; /* True if the configuration of this entry is valid. */ > enum ovn_nat_type type; > }; > > @@ -114,19 +123,6 @@ void en_lr_nat_run(struct engine_node *, void *data); > bool lr_nat_logical_router_handler(struct engine_node *, void *data); > bool lr_nat_northd_handler(struct engine_node *, void *data); > > -/* Returns true if a 'nat_entry' is valid, i.e.: > - * - parsing was successful. > - * - the string yielded exactly one IPv4 address or exactly one IPv6 address. > - */ > -static inline bool > -nat_entry_is_valid(const struct ovn_nat *nat_entry) > -{ > - const struct lport_addresses *ext_addrs = &nat_entry->ext_addrs; > - > - return (ext_addrs->n_ipv4_addrs == 1 && ext_addrs->n_ipv6_addrs == 0) || > - (ext_addrs->n_ipv4_addrs == 0 && ext_addrs->n_ipv6_addrs == 1); > -} > - > static inline bool > nat_entry_is_v6(const struct ovn_nat *nat_entry) > { > diff --git a/northd/northd.c b/northd/northd.c > index f0733182b..1c9433e96 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -1554,9 +1554,6 @@ ipam_add_port_addresses(struct ovn_datapath *od, struct > ovn_port *op) > } > } > > -static const char *find_lrp_member_ip(const struct ovn_port *op, > - const char *ip_s); > - > /* Returns true if the given router port 'op' (assumed to be a distributed > * gateway port) is the relevant DGP where the NAT rule of the router needs > to > * be applied. */ > @@ -1564,7 +1561,7 @@ static bool > is_nat_gateway_port(const struct nbrec_nat *nat, const struct ovn_port *op) > { > if (op->od->n_l3dgw_ports > 1 > - && ((!nat->gateway_port && !find_lrp_member_ip(op, nat->external_ip)) > + && ((!nat->gateway_port && !lrp_find_member_ip(op, nat->external_ip)) > || (nat->gateway_port && nat->gateway_port != op->nbrp))) { > return false; > } > @@ -8854,7 +8851,7 @@ build_lswitch_rport_arp_req_flows_for_lbnats( > &lr_stateful_rec->lrnat_rec->nat_entries[i]; > const struct nbrec_nat *nat = nat_entry->nb; > > - if (!nat_entry_is_valid(nat_entry)) { > + if (!nat_entry->is_valid) { > continue; > } > > @@ -10581,8 +10578,8 @@ build_bfd_map(const struct nbrec_bfd_table > *nbrec_bfd_table, > * overlaps with 'ip_s". If one is not found, returns NULL. > * > * The caller must not free the returned string. */ > -static const char * > -find_lrp_member_ip(const struct ovn_port *op, const char *ip_s) > +const char * > +lrp_find_member_ip(const struct ovn_port *op, const char *ip_s) > { > return find_lport_address(&op->lrp_networks, ip_s); > } > @@ -10601,7 +10598,7 @@ get_outport_for_routing_policy_nexthop(struct > ovn_datapath *od, > struct nbrec_logical_router_port *lrp = od->nbr->ports[i]; > > struct ovn_port *out_port = ovn_port_find(lr_ports, lrp->name); > - if (out_port && find_lrp_member_ip(out_port, nexthop)) { > + if (out_port && lrp_find_member_ip(out_port, nexthop)) { > return out_port; > } > } > @@ -10691,7 +10688,7 @@ build_routing_policy_flow(struct lflow_table *lflows, > struct ovn_datapath *od, > return; > } > > - const char *lrp_addr_s = find_lrp_member_ip(out_port, nexthop); > + const char *lrp_addr_s = lrp_find_member_ip(out_port, nexthop); > if (!lrp_addr_s) { > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > VLOG_WARN_RL(&rl, "lrp_addr not found for routing policy " > @@ -10792,7 +10789,7 @@ build_ecmp_routing_policy_flows(struct lflow_table > *lflows, > } > > const char *lrp_addr_s = > - find_lrp_member_ip(out_port, rp->valid_nexthops[i]); > + lrp_find_member_ip(out_port, rp->valid_nexthops[i]); > if (!lrp_addr_s) { > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > VLOG_WARN_RL(&rl, "lrp_addr not found for routing policy " > @@ -11537,7 +11534,7 @@ find_route_outport(const struct hmap *lr_ports, const > char *output_port, > return false; > } > if (nexthop[0]) { > - *lrp_addr_s = find_lrp_member_ip(*out_port, nexthop); > + *lrp_addr_s = lrp_find_member_ip(*out_port, nexthop); > } > if (!*lrp_addr_s) { > if (!force_out_port) { > @@ -11582,7 +11579,7 @@ find_static_route_outport(const struct ovn_datapath > *od, > * router port matching the next hop. */ > HMAP_FOR_EACH (out_port, dp_node, &od->ports) { > if (route->nexthop[0]) { > - lrp_addr_s = find_lrp_member_ip(out_port, route->nexthop); > + lrp_addr_s = lrp_find_member_ip(out_port, route->nexthop); > } > if (lrp_addr_s) { > break; > @@ -14526,7 +14523,7 @@ build_arp_resolve_flows_for_lsp( > continue; > } > > - if (!find_lrp_member_ip(peer, ip_s)) { > + if (!lrp_find_member_ip(peer, ip_s)) { > continue; > } > > @@ -14558,7 +14555,7 @@ build_arp_resolve_flows_for_lsp( > continue; > } > > - if (!find_lrp_member_ip(peer, ip_s)) { > + if (!lrp_find_member_ip(peer, ip_s)) { > continue; > } > > @@ -15550,7 +15547,7 @@ build_lrouter_arp_nd_for_datapath(const struct > ovn_datapath *od, > struct ovn_nat *nat_entry = &lrnat_rec->nat_entries[i]; > > /* Skip entries we failed to parse. */ > - if (!nat_entry_is_valid(nat_entry)) { > + if (!nat_entry->is_valid) { > continue; > } > > @@ -15857,7 +15854,7 @@ build_lrouter_ipv4_ip_input_for_lbnats( > &lr_stateful_rec->lrnat_rec->nat_entries[i]; > > /* Skip entries we failed to parse. */ > - if (!nat_entry_is_valid(nat_entry)) { > + if (!nat_entry->is_valid) { > continue; > } > > @@ -16499,145 +16496,6 @@ build_lrouter_ingress_flow(struct lflow_table > *lflows, > } > } > > -static int > -lrouter_check_nat_entry(const struct ovn_datapath *od, > - const struct ovn_nat *nat_entry, > - const struct hmap *lr_ports, ovs_be32 *mask, > - bool *is_v6, int *cidr_bits, struct eth_addr *mac, > - bool *distributed, struct ovn_port **nat_l3dgw_port) > -{ > - const struct nbrec_nat *nat = nat_entry->nb; > - struct in6_addr ipv6, mask_v6, v6_exact = IN6ADDR_EXACT_INIT; > - ovs_be32 ip; > - > - if (nat->allowed_ext_ips && nat->exempted_ext_ips) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > - VLOG_WARN_RL(&rl, "NAT rule: "UUID_FMT" not applied, since " > - "both allowed and exempt external ips set", > - UUID_ARGS(&(nat->header_.uuid))); > - return -EINVAL; > - } > - > - char *error = ip_parse_masked(nat->external_ip, &ip, mask); > - *is_v6 = false; > - > - if (error || *mask != OVS_BE32_MAX) { > - free(error); > - error = ipv6_parse_masked(nat->external_ip, &ipv6, &mask_v6); > - if (error || memcmp(&mask_v6, &v6_exact, sizeof(mask_v6))) { > - /* Invalid for both IPv4 and IPv6 */ > - static struct vlog_rate_limit rl = > - VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, "bad external ip %s for nat", > - nat->external_ip); > - free(error); > - return -EINVAL; > - } > - /* It was an invalid IPv4 address, but valid IPv6. > - * Treat the rest of the handling of this NAT rule > - * as IPv6. */ > - *is_v6 = true; > - } > - > - /* Validate gateway_port of NAT rule. */ > - *nat_l3dgw_port = NULL; > - if (nat->gateway_port == NULL) { > - if (od->n_l3dgw_ports == 1) { > - *nat_l3dgw_port = od->l3dgw_ports[0]; > - } else if (od->n_l3dgw_ports > 1) { > - /* Find the DGP reachable for the NAT external IP. */ > - for (size_t i = 0; i < od->n_l3dgw_ports; i++) { > - if (find_lrp_member_ip(od->l3dgw_ports[i], nat->external_ip)) > { > - *nat_l3dgw_port = od->l3dgw_ports[i]; > - break; > - } > - } > - if (*nat_l3dgw_port == NULL) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, > 1); > - VLOG_WARN_RL(&rl, "Unable to determine gateway_port for NAT " > - "with external_ip: %s configured on logical " > - "router: %s with multiple distributed gateway " > - "ports", nat->external_ip, od->nbr->name); > - return -EINVAL; > - } > - } > - } else { > - *nat_l3dgw_port = ovn_port_find(lr_ports, nat->gateway_port->name); > - > - if (!(*nat_l3dgw_port) || (*nat_l3dgw_port)->od != od || > - !lrp_is_l3dgw(*nat_l3dgw_port)) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, "gateway_port: %s of NAT configured on " > - "logical router: %s is not a valid distributed " > - "gateway port on that router", > - nat->gateway_port->name, od->nbr->name); > - return -EINVAL; > - } > - } > - > - /* Check the validity of nat->logical_ip. 'logical_ip' can > - * be a subnet when the type is "snat". */ > - if (*is_v6) { > - error = ipv6_parse_masked(nat->logical_ip, &ipv6, &mask_v6); > - *cidr_bits = ipv6_count_cidr_bits(&mask_v6); > - } else { > - error = ip_parse_masked(nat->logical_ip, &ip, mask); > - *cidr_bits = ip_count_cidr_bits(*mask); > - } > - if (nat_entry->type == SNAT) { > - if (error) { > - /* Invalid for both IPv4 and IPv6 */ > - static struct vlog_rate_limit rl = > - VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, "bad ip network or ip %s for snat " > - "in router "UUID_FMT"", > - nat->logical_ip, UUID_ARGS(&od->key)); > - free(error); > - return -EINVAL; > - } > - } else { > - if (error || (*is_v6 == false && *mask != OVS_BE32_MAX) > - || (*is_v6 && memcmp(&mask_v6, &v6_exact, > - sizeof mask_v6))) { > - /* Invalid for both IPv4 and IPv6 */ > - static struct vlog_rate_limit rl = > - VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, "bad ip %s for dnat in router " > - ""UUID_FMT"", nat->logical_ip, UUID_ARGS(&od->key)); > - free(error); > - return -EINVAL; > - } > - } > - > - /* For distributed router NAT, determine whether this NAT rule > - * satisfies the conditions for distributed NAT processing. */ > - *distributed = false; > - > - /* NAT cannnot be distributed if the DGP's peer > - * has a chassisredirect port (as the routing is centralized > - * on the gateway chassis for the DGP's networks/subnets.) > - */ > - struct ovn_port *l3dgw_port = *nat_l3dgw_port; > - if (l3dgw_port && l3dgw_port->peer && l3dgw_port->peer->cr_port) { > - return 0; > - } > - > - if (od->n_l3dgw_ports && nat_entry->type == DNAT_AND_SNAT && > - nat->logical_port && nat->external_mac) { > - if (eth_addr_from_string(nat->external_mac, mac)) { > - *distributed = true; > - } else { > - static struct vlog_rate_limit rl = > - VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, "bad mac %s for dnat in router " > - ""UUID_FMT"", nat->external_mac, UUID_ARGS(&od->key)); > - return -EINVAL; > - } > - } > - > - return 0; > -} > - > /* NAT, Defrag and load balancing. */ > static void build_lr_nat_defrag_and_lb_default_flows( > struct ovn_datapath *od, struct lflow_table *lflows, > @@ -16680,7 +16538,7 @@ static void > build_lrouter_nat_defrag_and_lb( > const struct lr_stateful_record *lr_stateful_rec, > const struct ovn_datapath *od, struct lflow_table *lflows, > - const struct hmap *ls_ports, const struct hmap *lr_ports, > + const struct hmap *ls_ports, > struct ds *match, struct ds *actions, > const struct shash *meter_groups, > const struct chassis_features *features, > @@ -16784,20 +16642,16 @@ build_lrouter_nat_defrag_and_lb( > for (size_t i = 0; i < lrnat_rec->n_nat_entries; i++) { > struct ovn_nat *nat_entry = &lrnat_rec->nat_entries[i]; > const struct nbrec_nat *nat = nat_entry->nb; > - struct eth_addr mac = eth_addr_broadcast; > - bool is_v6, distributed_nat; > - ovs_be32 mask; > - int cidr_bits; > - struct ovn_port *l3dgw_port; > > - bool stateless = lrouter_dnat_and_snat_is_stateless(nat_entry); > - > - if (lrouter_check_nat_entry(od, nat_entry, lr_ports, &mask, &is_v6, > - &cidr_bits, > - &mac, &distributed_nat, &l3dgw_port) < > 0) { > + /* Skip invalid entries. */ > + if (!nat_entry->is_valid) { > continue; > } > > + bool stateless = lrouter_dnat_and_snat_is_stateless(nat_entry); > + bool is_v6 = nat_entry_is_v6(nat_entry); > + unsigned int cidr_bits = nat_entry->logical_ip_cidr_bits; > + > /* S_ROUTER_IN_UNSNAT > * Ingress UNSNAT table: It is for already established connections' > * reverse traffic. i.e., SNAT has already been done in egress > @@ -16810,22 +16664,27 @@ build_lrouter_nat_defrag_and_lb( > * egress pipeline. */ > if (stateless) { > build_lrouter_in_unsnat_stateless_flow(lflows, od, nat_entry, > - match, distributed_nat, > - is_v6, l3dgw_port, > + match, > + nat_entry->is_distributed, > + is_v6, > + nat_entry->l3dgw_port, > lflow_ref); > } else if (lrouter_use_common_zone(od)) { > build_lrouter_in_unsnat_in_czone_flow(lflows, od, nat_entry, > match, > - distributed_nat, is_v6, > - l3dgw_port, lflow_ref); > + nat_entry->is_distributed, > + is_v6, > nat_entry->l3dgw_port, > + lflow_ref); > } else { > build_lrouter_in_unsnat_flow(lflows, od, nat_entry, match, > - distributed_nat, is_v6, l3dgw_port, > + nat_entry->is_distributed, > + is_v6, nat_entry->l3dgw_port, > lflow_ref); > } > /* S_ROUTER_IN_DNAT */ > build_lrouter_in_dnat_flow(lflows, od, lrnat_rec, nat_entry, match, > - actions, distributed_nat, cidr_bits, > is_v6, > - l3dgw_port, stateless, lflow_ref); > + actions, nat_entry->is_distributed, > + cidr_bits, is_v6, nat_entry->l3dgw_port, > + stateless, lflow_ref); > > /* ARP resolve for NAT IPs. */ > if (!od->is_gw_router) { > @@ -16837,7 +16696,8 @@ build_lrouter_nat_defrag_and_lb( > ds_clear(match); > ds_put_format( > match, "inport == %s && outport == %s && ip%s.dst == %s", > - l3dgw_port->json_key, l3dgw_port->json_key, > + nat_entry->l3dgw_port->json_key, > + nat_entry->l3dgw_port->json_key, > is_v6 ? "6" : "4", nat->external_ip); > ovn_lflow_add_with_hint(lflows, od, > S_ROUTER_IN_ARP_RESOLVE, > @@ -16851,21 +16711,21 @@ build_lrouter_nat_defrag_and_lb( > ds_clear(match); > ds_put_format( > match, "outport == %s && %s == %s", > - l3dgw_port->json_key, > + nat_entry->l3dgw_port->json_key, > is_v6 ? REG_NEXT_HOP_IPV6 : REG_NEXT_HOP_IPV4, > nat->external_ip); > ds_clear(actions); > ds_put_format( > actions, "eth.dst = %s; next;", > - distributed_nat ? nat->external_mac : > - l3dgw_port->lrp_networks.ea_s); > + nat_entry->is_distributed ? nat->external_mac : > + nat_entry->l3dgw_port->lrp_networks.ea_s); > ovn_lflow_add_with_hint(lflows, od, > S_ROUTER_IN_ARP_RESOLVE, > 100, ds_cstr(match), > ds_cstr(actions), > &nat->header_, > lflow_ref); > - if (od->redirect_bridged && distributed_nat) { > + if (od->redirect_bridged && nat_entry->is_distributed) { > ds_clear(match); > ds_put_format( > match, > @@ -16895,13 +16755,16 @@ build_lrouter_nat_defrag_and_lb( > if (use_common_zone) { > /* S_ROUTER_OUT_DNAT_LOCAL */ > build_lrouter_out_is_dnat_local(lflows, od, nat, match, actions, > - distributed_nat, is_v6, > - l3dgw_port, lflow_ref); > + nat_entry->is_distributed, > + is_v6, nat_entry->l3dgw_port, > + lflow_ref); > } > > /* S_ROUTER_OUT_UNDNAT */ > build_lrouter_out_undnat_flow(lflows, od, nat_entry, match, actions, > - distributed_nat, mac, is_v6, > l3dgw_port, > + nat_entry->is_distributed, > + nat_entry->mac, is_v6, > + nat_entry->l3dgw_port, > stateless, lflow_ref); > /* S_ROUTER_OUT_SNAT > * Egress SNAT table: Packets enter the egress pipeline with > @@ -16909,23 +16772,30 @@ build_lrouter_nat_defrag_and_lb( > * address. */ > if (stateless) { > build_lrouter_out_snat_stateless_flow(lflows, od, nat_entry, > match, > - actions, distributed_nat, > - mac, cidr_bits, is_v6, > - l3dgw_port, lflow_ref); > + actions, > + nat_entry->is_distributed, > + nat_entry->mac, cidr_bits, > + is_v6, > nat_entry->l3dgw_port, > + lflow_ref); > } else if (lrouter_use_common_zone(od)) { > build_lrouter_out_snat_in_czone_flow(lflows, od, nat_entry, > match, > - actions, distributed_nat, > mac, > - cidr_bits, is_v6, > l3dgw_port, > + actions, > + nat_entry->is_distributed, > + nat_entry->mac, cidr_bits, > + is_v6, > nat_entry->l3dgw_port, > lflow_ref); > } else { > build_lrouter_out_snat_flow(lflows, od, nat_entry, match, > actions, > - distributed_nat, mac, cidr_bits, > is_v6, > - l3dgw_port, lflow_ref, features); > + nat_entry->is_distributed, > + nat_entry->mac, cidr_bits, > + is_v6, nat_entry->l3dgw_port, > + lflow_ref, features); > } > > /* S_ROUTER_IN_ADMISSION - S_ROUTER_IN_IP_INPUT */ > - build_lrouter_ingress_flow(lflows, od, nat_entry, match, actions, > mac, > - distributed_nat, is_v6, l3dgw_port, > + build_lrouter_ingress_flow(lflows, od, nat_entry, match, actions, > + nat_entry->mac, nat_entry->is_distributed, > + is_v6, nat_entry->l3dgw_port, > meter_groups, lflow_ref); > > /* Ingress Gateway Redirect Table: For NAT on a distributed > @@ -16937,13 +16807,13 @@ build_lrouter_nat_defrag_and_lb( > * generated in the following stage is sent out with proper IP/MAC > * src addresses. > */ > - if (distributed_nat) { > + if (nat_entry->is_distributed) { > ds_clear(match); > ds_clear(actions); > ds_put_format(match, > "ip%s.src == %s && outport == %s", > is_v6 ? "6" : "4", nat->logical_ip, > - l3dgw_port->json_key); > + nat_entry->l3dgw_port->json_key); > /* Add a rule to drop traffic from a distributed NAT if > * the virtual port has not claimed yet becaused otherwise > * the traffic will be centralized misconfiguring the TOR switch. > @@ -16979,10 +16849,10 @@ build_lrouter_nat_defrag_and_lb( > ds_put_format(match, "ip%s.dst == %s && outport == %s", > is_v6 ? "6" : "4", > nat->external_ip, > - l3dgw_port->json_key); > - if (!distributed_nat) { > + nat_entry->l3dgw_port->json_key); > + if (!nat_entry->is_distributed) { > ds_put_format(match, " && is_chassis_resident(%s)", > - l3dgw_port->cr_port->json_key); > + nat_entry->l3dgw_port->cr_port->json_key); > } else { > ds_put_format(match, " && is_chassis_resident(\"%s\")", > nat->logical_port); > @@ -17280,7 +17150,6 @@ build_lr_stateful_flows(const struct > lr_stateful_record *lr_stateful_rec, > const struct ovn_datapaths *lr_datapaths, > struct lflow_table *lflows, > const struct hmap *ls_ports, > - const struct hmap *lr_ports, > struct ds *match, > struct ds *actions, > const struct shash *meter_groups, > @@ -17292,7 +17161,7 @@ build_lr_stateful_flows(const struct > lr_stateful_record *lr_stateful_rec, > ovs_assert(uuid_equals(&od->nbr->header_.uuid, > &lr_stateful_rec->nbr_uuid)); > build_lrouter_nat_defrag_and_lb(lr_stateful_rec, od, lflows, ls_ports, > - lr_ports, match, actions, meter_groups, > + match, actions, meter_groups, > features, lr_stateful_rec->lflow_ref); > build_lr_gateway_redirect_flows_for_nats(od, lr_stateful_rec->lrnat_rec, > lflows, match, actions, > @@ -17620,8 +17489,7 @@ build_lflows_thread(void *arg) > } > build_lr_stateful_flows(lr_stateful_rec, > lsi->lr_datapaths, > lsi->lflows, lsi->ls_ports, > - lsi->lr_ports, &lsi->match, > - &lsi->actions, > + &lsi->match, &lsi->actions, > lsi->meter_groups, > lsi->features); > } > @@ -17847,7 +17715,7 @@ build_lswitch_and_lrouter_flows( > stopwatch_start(LFLOWS_LR_STATEFUL_STOPWATCH_NAME, time_msec()); > LR_STATEFUL_TABLE_FOR_EACH (lr_stateful_rec, lr_stateful_table) { > build_lr_stateful_flows(lr_stateful_rec, lsi.lr_datapaths, > - lsi.lflows, lsi.ls_ports, lsi.lr_ports, > + lsi.lflows, lsi.ls_ports, > &lsi.match, &lsi.actions, > lsi.meter_groups, lsi.features); > } > @@ -18250,7 +18118,7 @@ lflow_handle_lr_stateful_changes(struct ovsdb_idl_txn > *ovnsb_txn, > /* Generate new lflows. */ > build_lr_stateful_flows(lr_stateful_rec, lflow_input->lr_datapaths, > lflows, lflow_input->ls_ports, > - lflow_input->lr_ports, &match, &actions, > + &match, &actions, > lflow_input->meter_groups, > lflow_input->features); > > diff --git a/northd/northd.h b/northd/northd.h > index eaf3d9455..b984e124d 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -967,6 +967,8 @@ lsp_is_router(const struct nbrec_logical_switch_port > *nbsp) > return !strcmp(nbsp->type, "router"); > } > > +const char *lrp_find_member_ip(const struct ovn_port *op, const char *ip_s); > + > /* This function returns true if 'op' is a gateway router port. > * False otherwise. > * For 'op' to be a gateway router port. > @@ -985,6 +987,7 @@ lrp_is_l3dgw(const struct ovn_port *op) > } > > struct ovn_port *ovn_port_find(const struct hmap *ports, const char *name); > + > void build_igmp_lflows(struct hmap *igmp_groups, > const struct hmap *ls_datapaths, > struct lflow_table *lflows, > -- > 2.43.0 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
