On Thu, Feb 8, 2024 at 1:49 PM <[email protected]> wrote: > > From: Numan Siddique <[email protected]> > > If an SNAT external_ip belongs to the router IP, then there > is no need to generate ARP request responder flows in > build_lswitch_rport_arp_req_flows_for_lbnats() as these flows > for router ips are generated by build_lswitch_rport_arp_req_flows(). > Otherwise this results in the lflow_table_add_lflow() to be called > multiple times for the same match and actions and the ovn_lflow to > have multiple dp_refcnts. > > Fixes: fe1c5df98b6f ("northd: forward arp request to lrp snat on.") > Signed-off-by: Numan Siddique <[email protected]> > --- > northd/en-lr-nat.c | 6 ++++++ > northd/en-lr-nat.h | 2 ++ > northd/northd.c | 28 ++++++++++++++++++++++++---- > northd/northd.h | 1 + > 4 files changed, 33 insertions(+), 4 deletions(-) > > diff --git a/northd/en-lr-nat.c b/northd/en-lr-nat.c > index 7664ec0ca9..ad11025c69 100644 > --- a/northd/en-lr-nat.c > +++ b/northd/en-lr-nat.c > @@ -288,6 +288,8 @@ lr_nat_record_init(struct lr_nat_record *lrnat_rec, > struct ovn_nat *nat_entry = &lrnat_rec->nat_entries[i]; > > nat_entry->nb = nat; > + nat_entry->is_router_ip = false; > + > if (!extract_ip_addresses(nat->external_ip, > &nat_entry->ext_addrs) || > !nat_entry_is_valid(nat_entry)) { > @@ -302,6 +304,10 @@ lr_nat_record_init(struct lr_nat_record *lrnat_rec, > > /* If this is a SNAT rule add the IP to the set of unique SNAT IPs. */ > if (!strcmp(nat->type, "snat")) { > + if (sset_contains(&od->router_ips, nat->external_ip)) { > + nat_entry->is_router_ip = true; > + } > + > if (!nat_entry_is_v6(nat_entry)) { > snat_ip_add(lrnat_rec, > nat_entry->ext_addrs.ipv4_addrs[0].addr_s, > diff --git a/northd/en-lr-nat.h b/northd/en-lr-nat.h > index 16b166ee05..6d3b2b6d65 100644 > --- a/northd/en-lr-nat.h > +++ b/northd/en-lr-nat.h > @@ -37,6 +37,8 @@ struct ovn_nat { > * 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. Initialized only for SNAT. */ > }; > > /* Stores the list of SNAT entries referencing a unique SNAT IP address. > diff --git a/northd/northd.c b/northd/northd.c > index a5d5e67117..c59aa8d304 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -431,6 +431,7 @@ ovn_datapath_create(struct hmap *datapaths, const struct uuid *key, > hmap_insert(datapaths, &od->key_node, uuid_hash(&od->key)); > od->lr_group = NULL; > hmap_init(&od->ports); > + sset_init(&od->router_ips); > return od; > } > > @@ -459,6 +460,7 @@ ovn_datapath_destroy(struct hmap *datapaths, struct ovn_datapath *od) > free(od->l3dgw_ports); > destroy_mcast_info_for_datapath(od); > destroy_ports_for_datapath(od); > + sset_destroy(&od->router_ips); > free(od); > } > } > @@ -2190,6 +2192,19 @@ join_logical_ports(const struct sbrec_port_binding_table *sbrec_pb_table, > > op->lrp_networks = lrp_networks; > op->od = od; > + > + for (size_t j = 0; j < op->lrp_networks.n_ipv4_addrs; j++) { > + sset_add(&op->od->router_ips, > + op->lrp_networks.ipv4_addrs[j].addr_s); > + } > + for (size_t j = 0; j < op->lrp_networks.n_ipv6_addrs; j++) { > + /* Exclude the LLA. */ > + if (!in6_is_lla(&op->lrp_networks.ipv6_addrs[j].addr)) { > + sset_add(&op->od->router_ips, > + op->lrp_networks.ipv6_addrs[j].addr_s); > + } > + } > + > hmap_insert(&od->ports, &op->dp_node, > hmap_node_hash(&op->key_node)); > > @@ -8302,22 +8317,27 @@ build_lswitch_rport_arp_req_flows_for_lbnats( > struct ovn_nat *nat_entry = > CONTAINER_OF(ovs_list_front(&snat_ip->snat_entries), > struct ovn_nat, ext_addr_list_node); > + if (nat_entry->is_router_ip) { > + /* If its a router ip, then there is no need to add the ARP > + * request forwarder flows as it will be added by > + * build_lswitch_rport_arp_req_flows(). */ > + continue; > + } > + > const struct nbrec_nat *nat = nat_entry->nb; > > /* Check if the ovn port has a network configured on which we could > * expect ARP requests/NS for the SNAT external_ip. > */ > if (nat_entry_is_v6(nat_entry)) { > - if (!lr_stateful_rec || > - !sset_contains(&lr_stateful_rec->lb_ips->ips_v6, > + if (!sset_contains(&lr_stateful_rec->lb_ips->ips_v6, > nat->external_ip)) { > build_lswitch_rport_arp_req_flow( > nat->external_ip, AF_INET6, sw_op, sw_od, 80, lflows, > stage_hint, lflow_ref); > } > } else { > - if (!lr_stateful_rec || > - !sset_contains(&lr_stateful_rec->lb_ips->ips_v4, > + if (!sset_contains(&lr_stateful_rec->lb_ips->ips_v4, > nat->external_ip)) { > build_lswitch_rport_arp_req_flow( > nat->external_ip, AF_INET, sw_op, sw_od, 80, lflows, > diff --git a/northd/northd.h b/northd/northd.h > index b5c175929e..3f1cd83413 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -293,6 +293,7 @@ struct ovn_datapath { > struct ovn_datapath **ls_peers; > size_t n_ls_peers; > size_t n_allocated_ls_peers; > + struct sset router_ips; /* Router port IPs except the IPv6 LLAs. */ > > /* Logical switch data. */ > struct ovn_port **router_ports; > -- > 2.43.0 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Thanks Numan. Acked-by: Han Zhou <[email protected]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
