On Tue, Apr 08, 2025 at 07:58:40AM +0200, Ales Musil via dev wrote: > On Mon, Apr 7, 2025 at 2:00 PM Felix Huettner <felix.huettner@stackit.cloud> > wrote: > > > On Thu, Apr 03, 2025 at 01:25:10PM +0200, Ales Musil wrote: > > > On Thu, Mar 20, 2025 at 1:02 PM Felix Huettner via dev < > > > ovs-dev@openvswitch.org> wrote: > > > > > > > Previously on each call to pinctrl_run all addresses that need to be > > > > announced via garps or rarps where computed newly. On larger external > > > > networks that might take significant time. > > > > However the content of the data can only change in a run of the I+P > > > > engine. So we move the whole logic to an engine node and only handle > > the > > > > minimal needed set in pinctrl_run. > > > > > > > > Acked-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com> > > > > Co-Authored-by: Max Lamprecht <max.lamprecht@mail.schwarz> > > > > Signed-off-by: Max Lamprecht <max.lamprecht@mail.schwarz> > > > > Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud> > > > > --- > > > > > > > > > > Hi Felix, > > > > > > thank you for the patch. I have a few comments down below. > > > > Hi Ales, > > > > Hi Felix, > > thank you for working on this.
No problem, it eats a lot of performance for us. > > > > > > > > v6->v7: added ack > > > > > > > > controller/automake.mk | 4 +- > > > > controller/garp_rarp.c | 626 ++++++++++++++++++++++++++++++++++++ > > > > controller/garp_rarp.h | 61 ++++ > > > > controller/ovn-controller.c | 243 +++++++++++++- > > > > controller/pinctrl.c | 559 ++------------------------------ > > > > controller/pinctrl.h | 16 +- > > > > 6 files changed, 967 insertions(+), 542 deletions(-) > > > > create mode 100644 controller/garp_rarp.c > > > > create mode 100644 controller/garp_rarp.h > > > > > > > > diff --git a/controller/automake.mk b/controller/automake.mk > > > > index 067c2e6e4..f0638ea97 100644 > > > > --- a/controller/automake.mk > > > > +++ b/controller/automake.mk > > > > @@ -59,7 +59,9 @@ controller_ovn_controller_SOURCES = \ > > > > controller/route-exchange.h \ > > > > controller/route-table-notify.h \ > > > > controller/route.h \ > > > > - controller/route.c > > > > + controller/route.c \ > > > > + controller/garp_rarp.h \ > > > > + controller/garp_rarp.c > > > > > > > > if HAVE_NETLINK > > > > controller_ovn_controller_SOURCES += \ > > > > diff --git a/controller/garp_rarp.c b/controller/garp_rarp.c > > > > new file mode 100644 > > > > index 000000000..916ff896a > > > > --- /dev/null > > > > +++ b/controller/garp_rarp.c > > > > @@ -0,0 +1,626 @@ > > > > +/* > > > > + * Copyright (c) 2025, STACKIT GmbH & Co. KG > > > > + * > > > > + * Licensed under the Apache License, Version 2.0 (the "License"); > > > > + * you may not use this file except in compliance with the License. > > > > + * You may obtain a copy of the License at: > > > > + * > > > > + * http://www.apache.org/licenses/LICENSE-2.0 > > > > + * > > > > + * Unless required by applicable law or agreed to in writing, software > > > > + * distributed under the License is distributed on an "AS IS" BASIS, > > > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or > > > > implied. > > > > + * See the License for the specific language governing permissions and > > > > + * limitations under the License. > > > > + */ > > > > + > > > > +#include <config.h> > > > > + > > > > +#include <net/if.h> > > > > +#include <stdbool.h> > > > > +#include <stddef.h> > > > > + > > > > +#include "controller/encaps.h" > > > > +#include "controller/local_data.h" > > > > +#include "controller/lport.h" > > > > +#include "hash.h" > > > > +#include "mac-binding-index.h" > > > > +#include "openvswitch/hmap.h" > > > > +#include "openvswitch/vlog.h" > > > > +#include "ovn/lex.h" > > > > +#include "packets.h" > > > > +#include "smap.h" > > > > +#include "sset.h" > > > > +#include "openvswitch/shash.h" > > > > +#include "garp_rarp.h" > > > > +#include "ovn-sb-idl.h" > > > > +#include "vswitch-idl.h" > > > > + > > > > +VLOG_DEFINE_THIS_MODULE(garp_rarp); > > > > +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); > > > > + > > > > +/* Get localnet vifs, local l3gw ports and ofport for localnet patch > > > > ports. */ > > > > +static void > > > > +get_localnet_vifs_l3gwports( > > > > + struct ovsdb_idl_index *sbrec_port_binding_by_datapath, > > > > + struct ovsdb_idl_index *sbrec_port_binding_by_name, > > > > + const struct ovsrec_bridge *br_int, > > > > + const struct sbrec_chassis *chassis, > > > > + const struct hmap *local_datapaths, > > > > + struct sset *localnet_vifs, > > > > + struct sset *local_l3gw_ports) > > > > +{ > > > > + for (int i = 0; i < br_int->n_ports; i++) { > > > > + const struct ovsrec_port *port_rec = br_int->ports[i]; > > > > + if (!strcmp(port_rec->name, br_int->name)) { > > > > + continue; > > > > + } > > > > + const char *tunnel_id = smap_get(&port_rec->external_ids, > > > > + "ovn-chassis-id"); > > > > + if (tunnel_id && > > > > + encaps_tunnel_id_match(tunnel_id, chassis->name, NULL, > > > > NULL)) { > > > > + continue; > > > > + } > > > > + const char *localnet = smap_get(&port_rec->external_ids, > > > > + "ovn-localnet-port"); > > > > + if (localnet) { > > > > + continue; > > > > + } > > > > + for (int j = 0; j < port_rec->n_interfaces; j++) { > > > > + const struct ovsrec_interface *iface_rec = > > > > port_rec->interfaces[j]; > > > > + if (!iface_rec->n_ofport) { > > > > + continue; > > > > + } > > > > + /* Get localnet vif. */ > > > > + const char *iface_id = smap_get(&iface_rec->external_ids, > > > > + "iface-id"); > > > > + if (!iface_id) { > > > > + continue; > > > > + } > > > > + const struct sbrec_port_binding *pb > > > > + = lport_lookup_by_name(sbrec_port_binding_by_name, > > > > iface_id); > > > > + if (!pb || pb->chassis != chassis) { > > > > + continue; > > > > + } > > > > + if (!iface_rec->link_state || > > > > + strcmp(iface_rec->link_state, "up")) { > > > > + continue; > > > > + } > > > > + struct local_datapath *ld > > > > + = get_local_datapath(local_datapaths, > > > > + pb->datapath->tunnel_key); > > > > + if (ld && ld->localnet_port) { > > > > + sset_add(localnet_vifs, iface_id); > > > > + } > > > > + } > > > > + } > > > > > > > > > > I know that this is old code but it makes me wonder if there's a better > > > way to find local vifs that are connected to localnet port. I think > > > it should be doable through bindings. > > > > Actually it seems like we can get this easily in the local_datapaths > > loop below. I will update this in the next version. It is so simple that > > i am unsure if i missed something. > > > This part of pinctrl is really old at this point, back then there > probably wasn't anything like local_datapaths. And by using them > we can actually remove the dependency on "en_ovs_open_vswitch" and > "en_ovs_bridge" which is a good thing. Less dependencies that can > cause recomputes is better. Yep "en_ovs_bridge" can be removed. We still need "en_ovs_open_vswitch" to get the chassis record. Maybe we should rework this as well, it seems strange to have nearly all engine nodes get it. > > > > > > > > > > + > > > > + struct sbrec_port_binding *target = > > sbrec_port_binding_index_init_row( > > > > + sbrec_port_binding_by_datapath); > > > > + > > > > + const struct local_datapath *ld; > > > > + HMAP_FOR_EACH (ld, hmap_node, local_datapaths) { > > > > + const struct sbrec_port_binding *pb; > > > > + > > > > + if (!ld->localnet_port) { > > > > + continue; > > > > + } > > > > + > > > > + /* Get l3gw ports. Consider port bindings with type > > "l3gateway" > > > > + * that connect to gateway routers (if local), and consider > > port > > > > + * bindings of type "patch" since they might connect to > > > > + * distributed gateway ports with NAT addresses. */ > > > > + > > > > + sbrec_port_binding_index_set_datapath(target, ld->datapath); > > > > + SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target, > > > > + > > > > sbrec_port_binding_by_datapath) { > > > > + if ((!strcmp(pb->type, "l3gateway") && pb->chassis == > > chassis) > > > > + || !strcmp(pb->type, "patch")) { > > > > + sset_add(local_l3gw_ports, pb->logical_port); > > > > + } > > > > + } > > > > + } > > > > + sbrec_port_binding_index_destroy_row(target); > > > > +} > > > > + > > > > + > > > > +/* Extracts the mac, IPv4 and IPv6 addresses, and logical port from > > > > + * 'addresses' which should be of the format 'MAC [IP1 IP2 ..] > > > > + * [is_chassis_resident("LPORT_NAME")]', where IPn should be a valid > > IPv4 > > > > + * or IPv6 address, and stores them in the 'ipv4_addrs' and > > 'ipv6_addrs' > > > > + * fields of 'laddrs'. The logical port name is stored in 'lport'. > > > > + * > > > > + * Returns true if at least 'MAC' is found in 'address', false > > otherwise. > > > > + * > > > > + * The caller must call destroy_lport_addresses() and free(*lport). */ > > > > +static bool > > > > +extract_addresses_with_port(const char *addresses, > > > > + struct lport_addresses *laddrs, > > > > + char **lport) > > > > +{ > > > > + int ofs; > > > > + if (!extract_addresses(addresses, laddrs, &ofs)) { > > > > + return false; > > > > + } else if (!addresses[ofs]) { > > > > + return true; > > > > + } > > > > + > > > > + struct lexer lexer; > > > > + lexer_init(&lexer, addresses + ofs); > > > > + lexer_get(&lexer); > > > > + > > > > + if (lexer.error || lexer.token.type != LEX_T_ID > > > > + || !lexer_match_id(&lexer, "is_chassis_resident")) { > > > > + VLOG_INFO_RL(&rl, "invalid syntax '%s' in address", > > addresses); > > > > + lexer_destroy(&lexer); > > > > + return true; > > > > + } > > > > + > > > > + if (!lexer_match(&lexer, LEX_T_LPAREN)) { > > > > + VLOG_INFO_RL(&rl, "Syntax error: expecting '(' after " > > > > + "'is_chassis_resident' in address '%s'", > > > > addresses); > > > > + lexer_destroy(&lexer); > > > > + return false; > > > > + } > > > > + > > > > + if (lexer.token.type != LEX_T_STRING) { > > > > + VLOG_INFO_RL(&rl, > > > > + "Syntax error: expecting quoted string after " > > > > + "'is_chassis_resident' in address '%s'", > > addresses); > > > > + lexer_destroy(&lexer); > > > > + return false; > > > > + } > > > > + > > > > + *lport = xstrdup(lexer.token.s); > > > > + > > > > + lexer_get(&lexer); > > > > + if (!lexer_match(&lexer, LEX_T_RPAREN)) { > > > > + VLOG_INFO_RL(&rl, "Syntax error: expecting ')' after quoted > > > > string in " > > > > + "'is_chassis_resident()' in address '%s'", > > > > + addresses); > > > > + lexer_destroy(&lexer); > > > > + return false; > > > > + } > > > > + > > > > + lexer_destroy(&lexer); > > > > + return true; > > > > +} > > > > + > > > > +static void > > > > +consider_nat_address(struct ovsdb_idl_index > > *sbrec_port_binding_by_name, > > > > + const char *nat_address, > > > > + const struct sbrec_port_binding *pb, > > > > + struct sset *nat_address_keys, > > > > + const struct sbrec_chassis *chassis, > > > > + const struct sset *active_tunnels, > > > > + struct shash *nat_addresses, > > > > + struct sset *non_local_lports, > > > > + struct sset *local_lports) > > > > +{ > > > > + struct lport_addresses *laddrs = xmalloc(sizeof *laddrs); > > > > + char *lport = NULL; > > > > + if (!extract_addresses_with_port(nat_address, laddrs, &lport) > > > > + || (!lport && !strcmp(pb->type, "patch"))) { > > > > + destroy_lport_addresses(laddrs); > > > > + free(laddrs); > > > > + free(lport); > > > > + return; > > > > + } > > > > + if (lport) { > > > > + if (!lport_is_chassis_resident(sbrec_port_binding_by_name, > > > > + chassis, active_tunnels, lport)) { > > > > + sset_add(non_local_lports, lport); > > > > + destroy_lport_addresses(laddrs); > > > > + free(laddrs); > > > > + free(lport); > > > > + return; > > > > + } else { > > > > + sset_add(local_lports, lport); > > > > + } > > > > + } > > > > + free(lport); > > > > + > > > > + int i; > > > > + for (i = 0; i < laddrs->n_ipv4_addrs; i++) { > > > > + char *name = xasprintf("%s-%s", pb->logical_port, > > > > + laddrs->ipv4_addrs[i].addr_s); > > > > + sset_add(nat_address_keys, name); > > > > + free(name); > > > > + } > > > > + if (laddrs->n_ipv4_addrs == 0) { > > > > + char *name = xasprintf("%s-noip", pb->logical_port); > > > > + sset_add(nat_address_keys, name); > > > > + free(name); > > > > + } > > > > + shash_add(nat_addresses, pb->logical_port, laddrs); > > > > +} > > > > + > > > > +static void > > > > +get_nat_addresses_and_keys(struct ovsdb_idl_index > > > > *sbrec_port_binding_by_name, > > > > + struct sset *nat_address_keys, > > > > + struct sset *local_l3gw_ports, > > > > + const struct sbrec_chassis *chassis, > > > > + const struct sset *active_tunnels, > > > > + struct shash *nat_addresses, > > > > + struct sset *non_local_lports, > > > > + struct sset *local_lports) > > > > +{ > > > > + const char *gw_port; > > > > + SSET_FOR_EACH (gw_port, local_l3gw_ports) { > > > > + const struct sbrec_port_binding *pb; > > > > + > > > > + pb = lport_lookup_by_name(sbrec_port_binding_by_name, > > gw_port); > > > > + if (!pb) { > > > > + continue; > > > > + } > > > > + > > > > + if (pb->n_nat_addresses) { > > > > + for (int i = 0; i < pb->n_nat_addresses; i++) { > > > > + consider_nat_address(sbrec_port_binding_by_name, > > > > + pb->nat_addresses[i], pb, > > > > + nat_address_keys, chassis, > > > > + active_tunnels, > > > > + nat_addresses, > > > > + non_local_lports, > > > > + local_lports); > > > > + } > > > > + } else { > > > > + /* Continue to support options:nat-addresses for version > > > > + * upgrade. */ > > > > + const char *nat_addresses_options = smap_get(&pb->options, > > > > + > > "nat-addresses"); > > > > + if (nat_addresses_options) { > > > > + consider_nat_address(sbrec_port_binding_by_name, > > > > + nat_addresses_options, pb, > > > > + nat_address_keys, chassis, > > > > + active_tunnels, > > > > + nat_addresses, > > > > + non_local_lports, > > > > + local_lports); > > > > + } > > > > + } > > > > + } > > > > +} > > > > + > > > > +static uint32_t > > > > +garp_rarp_node_hash(const struct eth_addr *ea, uint32_t dp_key, > > > > + uint32_t port_key) > > > > +{ > > > > + return hash_bytes(ea, sizeof *ea, hash_2words(dp_key, port_key)); > > > > +} > > > > + > > > > +static uint32_t > > > > +garp_rarp_node_hash_struct(const struct garp_rarp_node *n) > > > > +{ > > > > + return garp_rarp_node_hash(&n->ea, n->dp_key, n->port_key); > > > > +} > > > > + > > > > +static void > > > > +garp_rarp_node_add(struct hmap *map, const struct eth_addr ea, > > ovs_be32 > > > > ip, > > > > + uint32_t dp_key, uint32_t port_key) > > > > +{ > > > > + struct garp_rarp_node *grn = xmalloc(sizeof *grn); > > > > + grn->ea = ea; > > > > + grn->ipv4 = ip; > > > > + grn->announce_time = time_msec() + 1000; > > > > + grn->backoff = 1000; /* msec. */ > > > > + grn->dp_key = dp_key; > > > > + grn->port_key = port_key; > > > > + hmap_insert(map, &grn->hmap_node, > > garp_rarp_node_hash_struct(grn)); > > > > +} > > > > + > > > > +/* Searches for a given garp_rarp_node in a hmap. Ignores the > > > > announce_time > > > > + * and backoff field since they might be different based on runtime. > > */ > > > > +static struct garp_rarp_node* > > > > +garp_rarp_lookup(const struct hmap *map, const struct garp_rarp_node > > > > *search) > > > > +{ > > > > + struct garp_rarp_node *n; > > > > + uint32_t hash = garp_rarp_node_hash_struct(search); > > > > + HMAP_FOR_EACH_WITH_HASH (n, hmap_node, hash, map) { > > > > + if (!eth_addr_equals(search->ea, n->ea)) { > > > > + continue; > > > > + } > > > > + > > > > + if (search->ipv4 != n->ipv4) { > > > > + continue; > > > > + } > > > > + > > > > + if (search->dp_key != n->dp_key) { > > > > + continue; > > > > + } > > > > + > > > > + if (search->port_key != n->port_key) { > > > > + continue; > > > > + } > > > > + > > > > + return n; > > > > + } > > > > + return NULL; > > > > +} > > > > + > > > > +/* Update or add an IP-MAC binding for 'logical_port'. > > > > + * Caller should make sure that 'ovnsb_idl_txn' is valid. */ > > > > +static void > > > > +mac_binding_add_to_sb(struct ovsdb_idl_txn *ovnsb_idl_txn, > > > > + struct ovsdb_idl_index > > > > *sbrec_mac_binding_by_lport_ip, > > > > + const char *logical_port, > > > > + const struct sbrec_datapath_binding *dp, > > > > + struct eth_addr ea, const char *ip, > > > > + bool update_only) > > > > +{ > > > > + /* Convert ethernet argument to string form for database. */ > > > > + char mac_string[ETH_ADDR_STRLEN + 1]; > > > > + snprintf(mac_string, sizeof mac_string, ETH_ADDR_FMT, > > > > ETH_ADDR_ARGS(ea)); > > > > + > > > > + const struct sbrec_mac_binding *b = > > > > + mac_binding_lookup(sbrec_mac_binding_by_lport_ip, > > > > + logical_port, ip); > > > > + if (!b) { > > > > + if (update_only) { > > > > + return; > > > > + } > > > > + b = sbrec_mac_binding_insert(ovnsb_idl_txn); > > > > + sbrec_mac_binding_set_logical_port(b, logical_port); > > > > + sbrec_mac_binding_set_ip(b, ip); > > > > + sbrec_mac_binding_set_datapath(b, dp); > > > > + } > > > > + > > > > + if (strcmp(b->mac, mac_string)) { > > > > + sbrec_mac_binding_set_mac(b, mac_string); > > > > + > > > > + /* For backward compatibility check if timestamp column is > > > > available > > > > + * in SB DB. */ > > > > + if (sbrec_server_has_mac_binding_table_col_timestamp( > > > > + ovsdb_idl_txn_get_idl(ovnsb_idl_txn))) { > > > > + VLOG_DBG("Setting MAC binding timestamp for " > > > > + "ip:%s mac:%s port:%s to %lld", > > > > + b->ip, b->mac, logical_port, time_wall_msec()); > > > > + sbrec_mac_binding_set_timestamp(b, time_wall_msec()); > > > > + } > > > > + } > > > > +} > > > > > > > > > > Unless I've missed something this is duplicated from pinctrl.c, how about > > > moving > > > this function to a module where it could be used by both? > > > > Sorry for missing that. Will be centralized in the next version. > > > > > > > > > > > > + > > > > +/* Simulate the effect of a GARP on local datapaths, i.e., create > > > > MAC_Bindings > > > > + * on peer router datapaths. > > > > + */ > > > > +static void > > > > +send_garp_locally(const struct garp_rarp_ctx_in *r_ctx_in, > > > > + const struct sbrec_port_binding *in_pb, > > > > + struct eth_addr ea, ovs_be32 ip) > > > > +{ > > > > + if (!r_ctx_in->ovnsb_idl_txn) { > > > > + return; > > > > + } > > > > + > > > > + const struct local_datapath *ldp = > > > > + get_local_datapath(r_ctx_in->local_datapaths, > > > > + in_pb->datapath->tunnel_key); > > > > + > > > > + ovs_assert(ldp); > > > > + for (size_t i = 0; i < ldp->n_peer_ports; i++) { > > > > + const struct sbrec_port_binding *local = > > ldp->peer_ports[i].local; > > > > + const struct sbrec_port_binding *remote = > > > > ldp->peer_ports[i].remote; > > > > + > > > > + /* Skip "ingress" port. */ > > > > + if (local == in_pb) { > > > > + continue; > > > > + } > > > > + > > > > + bool update_only = > > !smap_get_bool(&remote->datapath->external_ids, > > > > + > > "always_learn_from_arp_request", > > > > + true); > > > > + > > > > + struct ds ip_s = DS_EMPTY_INITIALIZER; > > > > + > > > > + ip_format_masked(ip, OVS_BE32_MAX, &ip_s); > > > > + mac_binding_add_to_sb(r_ctx_in->ovnsb_idl_txn, > > > > + r_ctx_in->sbrec_mac_binding_by_lport_ip, > > > > + remote->logical_port, remote->datapath, > > > > + ea, ds_cstr(&ip_s), update_only); > > > > + ds_destroy(&ip_s); > > > > + } > > > > +} > > > > + > > > > +/* Add or update a vif for which GARPs need to be announced. */ > > > > +static void > > > > +send_garp_rarp_update(const struct garp_rarp_ctx_in *r_ctx_in, > > > > + const struct sbrec_port_binding *binding_rec, > > > > + struct shash *nat_addresses, > > > > + struct hmap *garp_rarp_data) > > > > +{ > > > > + /* Skip localports as they don't need to be announced */ > > > > + if (!strcmp(binding_rec->type, "localport")) { > > > > + return; > > > > + } > > > > + > > > > + /* Update GARP for NAT IP if it exists. Consider port bindings > > with > > > > type > > > > + * "l3gateway" for logical switch ports attached to gateway > > routers, > > > > and > > > > + * port bindings with type "patch" for logical switch ports > > attached > > > > to > > > > + * distributed gateway ports. */ > > > > + if (!strcmp(binding_rec->type, "l3gateway") > > > > + || !strcmp(binding_rec->type, "patch")) { > > > > + struct lport_addresses *laddrs = NULL; > > > > + while ((laddrs = shash_find_and_delete(nat_addresses, > > > > + > > > > binding_rec->logical_port))) { > > > > + int i; > > > > + for (i = 0; i < laddrs->n_ipv4_addrs; i++) { > > > > + garp_rarp_node_add(garp_rarp_data, laddrs->ea, > > > > + laddrs->ipv4_addrs[i].addr, > > > > + binding_rec->datapath->tunnel_key, > > > > + binding_rec->tunnel_key); > > > > + send_garp_locally(r_ctx_in, binding_rec, laddrs->ea, > > > > + laddrs->ipv4_addrs[i].addr); > > > > + } > > > > + /* > > > > + * Send RARPs even if we do not have a ipv4 address as it > > e.g. > > > > + * happens on ipv6 only ports. > > > > + */ > > > > + if (laddrs->n_ipv4_addrs == 0) { > > > > + garp_rarp_node_add(garp_rarp_data, laddrs->ea, > > > > + 0, binding_rec->datapath->tunnel_key, > > > > + binding_rec->tunnel_key); > > > > + } > > > > + destroy_lport_addresses(laddrs); > > > > + free(laddrs); > > > > + } > > > > + return; > > > > + } > > > > + > > > > + /* Add GARP for new vif. */ > > > > + int i; > > > > + for (i = 0; i < binding_rec->n_mac; i++) { > > > > + struct lport_addresses laddrs; > > > > + ovs_be32 ip = 0; > > > > + if (!extract_lsp_addresses(binding_rec->mac[i], &laddrs)) { > > > > + continue; > > > > + } > > > > + > > > > + if (laddrs.n_ipv4_addrs) { > > > > + ip = laddrs.ipv4_addrs[0].addr; > > > > + } > > > > + > > > > + garp_rarp_node_add(garp_rarp_data, > > > > + laddrs.ea, ip, > > > > + binding_rec->datapath->tunnel_key, > > > > + binding_rec->tunnel_key); > > > > + if (ip) { > > > > + send_garp_locally(r_ctx_in, binding_rec, laddrs.ea, ip); > > > > + } > > > > + > > > > + destroy_lport_addresses(&laddrs); > > > > + break; > > > > + } > > > > +} > > > > + > > > > +static struct garp_rarp_node* > > > > +garp_rarp_node_clone(const struct garp_rarp_node *src) > > > > +{ > > > > + struct garp_rarp_node *dst = xzalloc(sizeof *dst); > > > > + dst->ea = src->ea; > > > > + dst->ipv4 = src->ipv4; > > > > + dst->announce_time = time_msec() + 1000; > > > > + dst->backoff = 1000; > > > > + dst->dp_key = src->dp_key; > > > > + dst->port_key = src->port_key; > > > > + return dst; > > > > +} > > > > + > > > > +static void > > > > +garp_rarp_clear(struct garp_rarp_ctx_in *r_ctx_in, > > > > + struct garp_rarp_ctx_out *r_ctx_out) > > > > +{ > > > > + struct garp_rarp_node *grn; > > > > + HMAP_FOR_EACH_POP (grn, hmap_node, r_ctx_out->garp_rarp_data) { > > > > + garp_rarp_node_free(grn); > > > > + } > > > > + hmap_clear(r_ctx_out->garp_rarp_data); > > > > + sset_clear(r_ctx_in->non_local_lports); > > > > + sset_clear(r_ctx_in->local_lports); > > > > +} > > > > + > > > > +void > > > > +garp_rarp_run(struct garp_rarp_ctx_in *r_ctx_in, > > > > + struct garp_rarp_ctx_out *r_ctx_out) > > > > +{ > > > > + garp_rarp_clear(r_ctx_in, r_ctx_out); > > > > + > > > > + struct sset localnet_vifs = SSET_INITIALIZER(&localnet_vifs); > > > > + struct sset local_l3gw_ports = > > SSET_INITIALIZER(&local_l3gw_ports); > > > > + struct sset nat_ip_keys = SSET_INITIALIZER(&nat_ip_keys); > > > > + struct shash nat_addresses; > > > > + > > > > + shash_init(&nat_addresses); > > > > + > > > > + > > get_localnet_vifs_l3gwports(r_ctx_in->sbrec_port_binding_by_datapath, > > > > + r_ctx_in->sbrec_port_binding_by_name, > > > > + r_ctx_in->br_int, r_ctx_in->chassis, > > > > + r_ctx_in->local_datapaths, > > > > + &localnet_vifs, &local_l3gw_ports); > > > > + > > > > + get_nat_addresses_and_keys(r_ctx_in->sbrec_port_binding_by_name, > > > > + &nat_ip_keys, &local_l3gw_ports, > > > > + r_ctx_in->chassis, > > > > r_ctx_in->active_tunnels, > > > > + &nat_addresses, > > r_ctx_in->non_local_lports, > > > > + r_ctx_in->local_lports); > > > > + > > > > + /* Update send_garp_rarp_data. */ > > > > + const char *iface_id; > > > > + SSET_FOR_EACH (iface_id, &localnet_vifs) { > > > > + const struct sbrec_port_binding *pb = lport_lookup_by_name( > > > > + r_ctx_in->sbrec_port_binding_by_name, iface_id); > > > > + if (pb && !smap_get_bool(&pb->options, "disable_garp_rarp", > > > > false)) { > > > > + send_garp_rarp_update(r_ctx_in, pb, &nat_addresses, > > > > + r_ctx_out->garp_rarp_data); > > > > + } > > > > + } > > > > + > > > > + /* Update send_garp_rarp_data for nat-addresses. */ > > > > + const char *gw_port; > > > > + SSET_FOR_EACH (gw_port, &local_l3gw_ports) { > > > > + const struct sbrec_port_binding *pb = lport_lookup_by_name( > > > > + r_ctx_in->sbrec_port_binding_by_name, gw_port); > > > > + if (pb && !smap_get_bool(&pb->options, "disable_garp_rarp", > > > > false)) { > > > > + send_garp_rarp_update(r_ctx_in, pb, &nat_addresses, > > > > + r_ctx_out->garp_rarp_data); > > > > + } > > > > + } > > > > + > > > > + sset_destroy(&localnet_vifs); > > > > + sset_destroy(&local_l3gw_ports); > > > > + > > > > + struct shash_node *iter; > > > > + SHASH_FOR_EACH_SAFE (iter, &nat_addresses) { > > > > + struct lport_addresses *laddrs = iter->data; > > > > + destroy_lport_addresses(laddrs); > > > > + shash_delete(&nat_addresses, iter); > > > > + free(laddrs); > > > > + } > > > > + shash_destroy(&nat_addresses); > > > > + > > > > + sset_destroy(&nat_ip_keys); > > > > +} > > > > + > > > > +/* Syncs a hmap of garp_rarp_node to another hmap. The elements in the > > > > + * destination hmap are initialized with valid announce times based > > on the > > > > + * current timestamp. > > > > + * If source is NULL then destination will at most change the timer if > > > > + * reset_timers is set. > > > > + * If reset_timers is true then the timers of all entries in the > > > > destination > > > > + * hmap will be reset to default values. > > > > + * Returns true if the destination hmap was chnaged. */ > > > > +bool > > > > +garp_rarp_sync(const struct hmap *source, struct hmap *dest, bool > > > > reset_timers) > > > > +{ > > > > + struct garp_rarp_node *grn; > > > > + bool changed = false; > > > > + > > > > + HMAP_FOR_EACH_SAFE (grn, hmap_node, dest) { > > > > + if (source && !garp_rarp_lookup(source, grn)) { > > > > + hmap_remove(dest, &grn->hmap_node); > > > > + garp_rarp_node_free(grn); > > > > + changed = true; > > > > + } else if (reset_timers) { > > > > + grn->announce_time = time_msec() + 1000; > > > > + grn->backoff = 1000; > > > > + changed = true; > > > > + } > > > > + } > > > > + > > > > + if (source) { > > > > + HMAP_FOR_EACH (grn, hmap_node, source) { > > > > + if (!garp_rarp_lookup(dest, grn)) { > > > > + uint32_t hash = garp_rarp_node_hash_struct(grn); > > > > + hmap_insert(dest, > > &garp_rarp_node_clone(grn)->hmap_node, > > > > hash); > > > > + changed = true; > > > > + } > > > > + } > > > > + } > > > > + > > > > + return changed; > > > > +} > > > > > > > > > > That's a lot of copying between the hmaps, why can't we indicate that the > > > announcement was sent for the last time and use just the status instead > > > of copying parts of the hmap? > > > > The idea was that any state that needs to be persistently kept is part > > of the pinctrl module. The engine node is then free to discard the hmap > > and generate it again from scratch. > > > > Also the data is needed in the pinctrl thread separately from the main > > thread. So copying seemed like the best solution to avoid needing to > > lock the data between pinctrl and the engine node. > > > > Yes the locking might be a problem, how about using CMAP instead? > That should get rid of the locking problem and we could avoid the > copies. I'll leave it up to you if you would like to do it as part > of this patch or as follow up. I'll include it in these changes. It seems to be cleaner when i rework it in one go. Thanks a lot, Felix > > > > > > > > > > > > > > + > > > > +void > > > > +garp_rarp_node_free(struct garp_rarp_node *garp_rarp) > > > > +{ > > > > + free(garp_rarp); > > > > +} > > > > diff --git a/controller/garp_rarp.h b/controller/garp_rarp.h > > > > new file mode 100644 > > > > index 000000000..b85e75a7c > > > > --- /dev/null > > > > +++ b/controller/garp_rarp.h > > > > @@ -0,0 +1,61 @@ > > > > +/* > > > > + * Copyright (c) 2025, STACKIT GmbH & Co. KG > > > > + * > > > > + * Licensed under the Apache License, Version 2.0 (the "License"); > > > > + * you may not use this file except in compliance with the License. > > > > + * You may obtain a copy of the License at: > > > > + * > > > > + * http://www.apache.org/licenses/LICENSE-2.0 > > > > + * > > > > + * Unless required by applicable law or agreed to in writing, software > > > > + * distributed under the License is distributed on an "AS IS" BASIS, > > > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or > > > > implied. > > > > + * See the License for the specific language governing permissions and > > > > + * limitations under the License. > > > > + */ > > > > + > > > > +#ifndef GARP_RARP_H > > > > +#define GARP_RARP_H 1 > > > > + > > > > +#include "openvswitch/hmap.h" > > > > +#include "openvswitch/types.h" > > > > + > > > > +/* Contains a single mac and ip address that should be announced. */ > > > > +struct garp_rarp_node { > > > > + struct hmap_node hmap_node; > > > > + struct eth_addr ea; /* Ethernet address of port. */ > > > > + ovs_be32 ipv4; /* Ipv4 address of port. */ > > > > + long long int announce_time; /* Next announcement in ms. > > > > + * If LLONG_MAX there should be no > > > > + * annoucement. */ > > > > + int backoff; /* Backoff timeout for the next > > > > + * announcement (in msecs). */ > > > > + uint32_t dp_key; /* Datapath used to output this > > GARP. */ > > > > + uint32_t port_key; /* Port to inject the GARP into. */ > > > > +}; > > > > + > > > > +struct garp_rarp_ctx_in { > > > > + struct ovsdb_idl_txn *ovnsb_idl_txn; > > > > + struct ovsdb_idl_index *sbrec_port_binding_by_datapath; > > > > + struct ovsdb_idl_index *sbrec_port_binding_by_name; > > > > + struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip; > > > > + const struct sbrec_ecmp_nexthop_table *ecmp_nh_table; > > > > + const struct ovsrec_bridge *br_int; > > > > + const struct sbrec_chassis *chassis; > > > > + const struct hmap *local_datapaths; > > > > + const struct sset *active_tunnels; > > > > + struct sset *non_local_lports; > > > > + struct sset *local_lports; > > > > +}; > > > > + > > > > +struct garp_rarp_ctx_out { > > > > + /* Contains struct garp_rarp_node. */ > > > > + struct hmap *garp_rarp_data; > > > > +}; > > > > + > > > > +void garp_rarp_run(struct garp_rarp_ctx_in *, struct > > garp_rarp_ctx_out *); > > > > +bool garp_rarp_sync(const struct hmap *source, struct hmap *dest, > > > > + bool reset_timers); > > > > +void garp_rarp_node_free(struct garp_rarp_node *); > > > > + > > > > +#endif /* GARP_RARP_H */ > > > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > > > > index cfed5282c..6f1b27876 100644 > > > > --- a/controller/ovn-controller.c > > > > +++ b/controller/ovn-controller.c > > > > @@ -92,6 +92,7 @@ > > > > #include "route.h" > > > > #include "route-exchange.h" > > > > #include "route-table-notify.h" > > > > +#include "garp_rarp.h" > > > > > > > > VLOG_DEFINE_THIS_MODULE(main); > > > > > > > > @@ -4952,6 +4953,14 @@ controller_output_route_exchange_handler(struct > > > > engine_node *node, > > > > return true; > > > > } > > > > > > > > +static bool > > > > +controller_output_garp_rarp_handler(struct engine_node *node, > > > > + void *data OVS_UNUSED) > > > > +{ > > > > + engine_set_node_state(node, EN_UPDATED); > > > > + return true; > > > > +} > > > > + > > > > /* Handles sbrec_chassis changes. > > > > * If a new chassis is added or removed return false, so that > > > > * flows are recomputed. For any updates, there is no need for > > > > @@ -5305,6 +5314,216 @@ en_route_table_notify_cleanup(void *data > > > > OVS_UNUSED) > > > > { > > > > } > > > > > > > > +struct ed_type_garp_rarp { > > > > + struct hmap garp_rarp_data; > > > > + > > > > + /* non_local_lports and local_lports are used in the incremental > > > > handlers > > > > + * to trigger updates if such a port changes. */ > > > > + struct sset non_local_lports; /* lports that we did not consider > > > > because > > > > + they where not local. */ > > > > + struct sset local_lports; /* lports where we did consider the > > > > addresses > > > > + because they where local. */ > > > > +}; > > > > + > > > > +static void > > > > +en_garp_rarp_run(struct engine_node *node, void *data_) > > > > +{ > > > > + struct ed_type_garp_rarp *data = data_; > > > > + > > > > + const struct ovsrec_open_vswitch_table *ovs_table = > > > > + EN_OVSDB_GET(engine_get_input("OVS_open_vswitch", node)); > > > > + const char *chassis_id = get_ovs_chassis_id(ovs_table); > > > > + ovs_assert(chassis_id); > > > > + > > > > + struct ovsdb_idl_index *sbrec_chassis_by_name = > > > > + engine_ovsdb_node_get_index( > > > > + engine_get_input("SB_chassis", node), > > > > + "name"); > > > > + const struct sbrec_chassis *chassis > > > > + = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); > > > > + ovs_assert(chassis); > > > > + > > > > + const struct ovsrec_bridge_table *bridge_table = > > > > + EN_OVSDB_GET(engine_get_input("OVS_bridge", node)); > > > > + const struct ovsrec_bridge *br_int = get_br_int(bridge_table, > > > > ovs_table); > > > > + > > > > + struct ovsdb_idl_index *sbrec_port_binding_by_datapath = > > > > + engine_ovsdb_node_get_index( > > > > + engine_get_input("SB_port_binding", node), > > > > + "datapath"); > > > > + struct ovsdb_idl_index *sbrec_port_binding_by_name = > > > > + engine_ovsdb_node_get_index( > > > > + engine_get_input("SB_port_binding", node), > > > > + "name"); > > > > + struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip = > > > > + engine_ovsdb_node_get_index( > > > > + engine_get_input("SB_mac_binding", node), > > > > + "lport_ip"); > > > > + > > > > + struct ovsdb_idl_txn *ovnsb_idl_txn = > > > > engine_get_context()->ovnsb_idl_txn; > > > > + > > > > + const struct sbrec_ecmp_nexthop_table *ecmp_nh_table = > > > > + > > > > sbrec_ecmp_nexthop_table_get(ovsdb_idl_txn_get_idl(ovnsb_idl_txn)); > > > > + > > > > + struct ed_type_runtime_data *rt_data = > > > > + engine_get_input_data("runtime_data", node); > > > > + > > > > + struct garp_rarp_ctx_in r_ctx_in = { > > > > + .ovnsb_idl_txn = ovnsb_idl_txn, > > > > + .sbrec_port_binding_by_datapath = > > sbrec_port_binding_by_datapath, > > > > + .sbrec_port_binding_by_name = sbrec_port_binding_by_name, > > > > + .sbrec_mac_binding_by_lport_ip = > > sbrec_mac_binding_by_lport_ip, > > > > + .ecmp_nh_table = ecmp_nh_table, > > > > + .br_int = br_int, > > > > + .chassis = chassis, > > > > + .active_tunnels = &rt_data->active_tunnels, > > > > + .local_datapaths = &rt_data->local_datapaths, > > > > + .non_local_lports = &data->non_local_lports, > > > > + .local_lports = &data->local_lports, > > > > + }; > > > > + > > > > + struct garp_rarp_ctx_out r_ctx_out = { > > > > + .garp_rarp_data = &data->garp_rarp_data, > > > > + }; > > > > + > > > > + garp_rarp_run(&r_ctx_in, &r_ctx_out); > > > > +} > > > > + > > > > + > > > > +static void * > > > > +en_garp_rarp_init(struct engine_node *node OVS_UNUSED, > > > > + struct engine_arg *arg OVS_UNUSED) > > > > +{ > > > > + struct ed_type_garp_rarp *gr = xmalloc(sizeof *gr); > > > > + hmap_init(&gr->garp_rarp_data); > > > > + sset_init(&gr->non_local_lports); > > > > + sset_init(&gr->local_lports); > > > > + return gr; > > > > +} > > > > + > > > > +static void > > > > +en_garp_rarp_cleanup(void *data_) > > > > +{ > > > > + struct ed_type_garp_rarp *data = data_; > > > > + struct garp_rarp_node *grn; > > > > + HMAP_FOR_EACH_POP (grn, hmap_node, &data->garp_rarp_data) { > > > > + garp_rarp_node_free(grn); > > > > + } > > > > + hmap_destroy(&data->garp_rarp_data); > > > > + sset_destroy(&data->non_local_lports); > > > > + sset_destroy(&data->local_lports); > > > > +} > > > > + > > > > +static bool > > > > +garp_rarp_sb_port_binding_handler(struct engine_node *node, > > > > + void *data_) > > > > +{ > > > > + /* We need to handle a change if there was change on a datapath > > with > > > > + * a localnet port. > > > > + * Also the ha_chassis status of a port binding might change. */ > > > > + struct ed_type_garp_rarp *data = data_; > > > > + > > > > + const struct ovsrec_open_vswitch_table *ovs_table = > > > > + EN_OVSDB_GET(engine_get_input("OVS_open_vswitch", node)); > > > > + const char *chassis_id = get_ovs_chassis_id(ovs_table); > > > > + ovs_assert(chassis_id); > > > > + > > > > + struct ovsdb_idl_index *sbrec_chassis_by_name = > > > > + engine_ovsdb_node_get_index( > > > > + engine_get_input("SB_chassis", node), > > > > + "name"); > > > > + const struct sbrec_chassis *chassis > > > > + = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); > > > > + ovs_assert(chassis); > > > > + > > > > + struct ed_type_runtime_data *rt_data = > > > > + engine_get_input_data("runtime_data", node); > > > > + const struct sbrec_port_binding_table *port_binding_table = > > > > + EN_OVSDB_GET(engine_get_input("SB_port_binding", node)); > > > > + struct ovsdb_idl_index *sbrec_port_binding_by_name = > > > > + engine_ovsdb_node_get_index( > > > > + engine_get_input("SB_port_binding", node), > > > > + "name"); > > > > + > > > > + const struct sbrec_port_binding *pb; > > > > + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, > > port_binding_table) { > > > > + struct local_datapath *ld = get_local_datapath( > > > > + &rt_data->local_datapaths, pb->datapath->tunnel_key); > > > > + > > > > + if (!ld || ld->localnet_port) { > > > > + /* XXX: actually handle this incrementally. */ > > > > + return false; > > > > + } > > > > + > > > > + if (sset_contains(&data->non_local_lports, pb->logical_port) > > && > > > > + lport_is_chassis_resident(sbrec_port_binding_by_name, > > chassis, > > > > + &rt_data->active_tunnels, > > > > + pb->logical_port)) { > > > > + /* XXX: actually handle this incrementally. */ > > > > + return false; > > > > + } > > > > + > > > > + if (sset_contains(&data->local_lports, pb->logical_port) && > > > > + !lport_is_chassis_resident(sbrec_port_binding_by_name, > > > > chassis, > > > > + &rt_data->active_tunnels, > > > > + pb->logical_port)) { > > > > + /* XXX: actually handle this incrementally. */ > > > > + return false; > > > > + } > > > > + } > > > > + > > > > + return true; > > > > +} > > > > + > > > > +static bool > > > > +garp_rarp_runtime_data_handler(struct engine_node *node, void *data > > > > OVS_UNUSED) > > > > +{ > > > > + /* We use two elements from rt_data: > > > > + * 1. active_tunnels: There is currently not incremental > > processing > > > > for > > > > + * this in runtime_data. So we just fall back to a recompute. > > > > + * 2. local_datapaths: This has incremental processing on the > > > > runtime_data > > > > + * side. We are only interested in datapaths with a localnet > > port > > > > so > > > > + * we just recompute if there is one in there. Otherwise the > > > > change is > > > > + * irrelevant for us. */ > > > > + > > > > + struct ed_type_runtime_data *rt_data = > > > > + engine_get_input_data("runtime_data", node); > > > > + > > > > + /* There are no tracked data. Fall back to full recompute. */ > > > > + if (!rt_data->tracked) { > > > > + return false; > > > > + } > > > > + > > > > + struct tracked_datapath *tdp; > > > > + HMAP_FOR_EACH (tdp, node, &rt_data->tracked_dp_bindings) { > > > > + if (tdp->tracked_type == TRACKED_RESOURCE_REMOVED) { > > > > + /* This is currently not handled incrementally in > > runtime_data > > > > + * so it should never happen. Recompute just in case. */ > > > > + return false; > > > > + } > > > > + > > > > + struct local_datapath *ld = get_local_datapath( > > > > + &rt_data->local_datapaths, tdp->dp->tunnel_key); > > > > + > > > > + if (!ld || ld->localnet_port) { > > > > + /* XXX: actually handle this incrementally. */ > > > > + return false; > > > > + } > > > > + > > > > + /* The localnet port might also have been removed. */ > > > > + struct tracked_lport *tlp; > > > > + struct shash_node *sn; > > > > + SHASH_FOR_EACH (sn, &tdp->lports) { > > > > + tlp = sn->data; > > > > + if (!strcmp(tlp->pb->type, "localnet")) { > > > > + return false; > > > > + } > > > > + } > > > > + } > > > > + > > > > + return true; > > > > +} > > > > + > > > > /* Returns false if the northd internal version stored in SB_Global > > > > * and ovn-controller internal version don't match. > > > > */ > > > > @@ -5614,6 +5833,7 @@ main(int argc, char *argv[]) > > > > ENGINE_NODE(route, "route"); > > > > ENGINE_NODE(route_table_notify, "route_table_notify"); > > > > ENGINE_NODE(route_exchange, "route_exchange"); > > > > + ENGINE_NODE(garp_rarp, "garp_rarp"); > > > > > > > > #define SB_NODE(NAME, NAME_STR) ENGINE_NODE_SB(NAME, NAME_STR); > > > > SB_NODES > > > > @@ -5826,6 +6046,15 @@ main(int argc, char *argv[]) > > > > engine_add_input(&en_dns_cache, &en_sb_dns, > > > > dns_cache_sb_dns_handler); > > > > > > > > + engine_add_input(&en_garp_rarp, &en_ovs_open_vswitch, NULL); > > > > + engine_add_input(&en_garp_rarp, &en_ovs_bridge, NULL); > > > > + engine_add_input(&en_garp_rarp, &en_sb_chassis, NULL); > > > > + engine_add_input(&en_garp_rarp, &en_sb_port_binding, > > > > + garp_rarp_sb_port_binding_handler); > > > > + engine_add_input(&en_garp_rarp, &en_sb_mac_binding, NULL); > > > > > > > > > > This dependency leads to recompute every time we insert MB locally. > > > It seems that we need this only for index, so it should be noop handler. > > > > Yes, thanks. > > > > > > > > > > > > + engine_add_input(&en_garp_rarp, &en_runtime_data, > > > > + garp_rarp_runtime_data_handler); > > > > + > > > > engine_add_input(&en_controller_output, &en_dns_cache, > > > > NULL); > > > > engine_add_input(&en_controller_output, &en_lflow_output, > > > > @@ -5838,6 +6067,8 @@ main(int argc, char *argv[]) > > > > controller_output_bfd_chassis_handler); > > > > engine_add_input(&en_controller_output, &en_route_exchange, > > > > controller_output_route_exchange_handler); > > > > + engine_add_input(&en_controller_output, &en_garp_rarp, > > > > + controller_output_garp_rarp_handler); > > > > > > > > engine_add_input(&en_acl_id, &en_sb_acl_id, NULL); > > > > engine_add_input(&en_controller_output, &en_acl_id, > > > > @@ -5874,6 +6105,8 @@ main(int argc, char *argv[]) > > > > > > > > sbrec_chassis_template_var_index_by_chassis); > > > > engine_ovsdb_node_add_index(&en_sb_learned_route, "datapath", > > > > > > sbrec_learned_route_index_by_datapath); > > > > + engine_ovsdb_node_add_index(&en_sb_mac_binding, "lport_ip", > > > > + sbrec_mac_binding_by_lport_ip); > > > > engine_ovsdb_node_add_index(&en_ovs_flow_sample_collector_set, > > "id", > > > > > > ovsrec_flow_sample_collector_set_by_id); > > > > engine_ovsdb_node_add_index(&en_ovs_port, "qos", > > ovsrec_port_by_qos); > > > > @@ -6290,9 +6523,12 @@ main(int argc, char *argv[]) > > > > stopwatch_start(PINCTRL_RUN_STOPWATCH_NAME, > > > > time_msec()); > > > > pinctrl_update(ovnsb_idl_loop.idl); > > > > + const struct ed_type_garp_rarp *grd = > > > > engine_get_data( > > > > + &en_garp_rarp); > > > > + const struct hmap *garp_rarp_data = > > > > + grd ? &grd->garp_rarp_data : NULL; > > > > pinctrl_run(ovnsb_idl_txn, > > > > sbrec_datapath_binding_by_key, > > > > - sbrec_port_binding_by_datapath, > > > > sbrec_port_binding_by_key, > > > > sbrec_port_binding_by_name, > > > > sbrec_mac_binding_by_lport_ip, > > > > @@ -6308,13 +6544,14 @@ main(int argc, char *argv[]) > > > > > > > > sbrec_bfd_table_get(ovnsb_idl_loop.idl), > > > > sbrec_ecmp_nexthop_table_get( > > > > ovnsb_idl_loop.idl), > > > > - br_int, chassis, > > > > + chassis, > > > > &runtime_data->local_datapaths, > > > > &runtime_data->active_tunnels, > > > > > > > > &runtime_data->local_active_ports_ipv6_pd, > > > > > > &runtime_data->local_active_ports_ras, > > > > ovsrec_open_vswitch_table_get( > > > > - ovs_idl_loop.idl)); > > > > + ovs_idl_loop.idl), > > > > + garp_rarp_data); > > > > stopwatch_stop(PINCTRL_RUN_STOPWATCH_NAME, > > > > time_msec()); > > > > mirror_run(ovs_idl_txn, > > > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > > > > index 1481b2547..6b4a4a520 100644 > > > > --- a/controller/pinctrl.c > > > > +++ b/controller/pinctrl.c > > > > @@ -64,6 +64,7 @@ > > > > #include "ip-mcast.h" > > > > #include "ovn-sb-idl.h" > > > > #include "ovn-dns.h" > > > > +#include "garp_rarp.h" > > > > > > > > VLOG_DEFINE_THIS_MODULE(pinctrl); > > > > > > > > @@ -233,15 +234,9 @@ static void destroy_send_arps_nds(void); > > > > static void send_garp_rarp_wait(long long int send_garp_rarp_time); > > > > static void send_arp_nd_wait(long long int send_arp_nd_time); > > > > static void send_garp_rarp_prepare( > > > > - struct ovsdb_idl_txn *ovnsb_idl_txn, > > > > - struct ovsdb_idl_index *sbrec_port_binding_by_datapath, > > > > - struct ovsdb_idl_index *sbrec_port_binding_by_name, > > > > - struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip, > > > > + const struct hmap *garp_rarp_data, > > > > const struct sbrec_ecmp_nexthop_table *ecmp_nh_table, > > > > - const struct ovsrec_bridge *, > > > > - const struct sbrec_chassis *, > > > > - const struct hmap *local_datapaths, > > > > - const struct sset *active_tunnels, > > > > + const struct sbrec_chassis *chassis, > > > > const struct ovsrec_open_vswitch_table *ovs_table) > > > > OVS_REQUIRES(pinctrl_mutex); > > > > static void send_garp_rarp_run(struct rconn *swconn, > > > > @@ -4066,7 +4061,6 @@ pinctrl_update(const struct ovsdb_idl *idl) > > > > void > > > > pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > > > > struct ovsdb_idl_index *sbrec_datapath_binding_by_key, > > > > - struct ovsdb_idl_index *sbrec_port_binding_by_datapath, > > > > struct ovsdb_idl_index *sbrec_port_binding_by_key, > > > > struct ovsdb_idl_index *sbrec_port_binding_by_name, > > > > struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip, > > > > @@ -4078,13 +4072,13 @@ pinctrl_run(struct ovsdb_idl_txn > > *ovnsb_idl_txn, > > > > const struct sbrec_mac_binding_table *mac_binding_table, > > > > const struct sbrec_bfd_table *bfd_table, > > > > const struct sbrec_ecmp_nexthop_table *ecmp_nh_table, > > > > - const struct ovsrec_bridge *br_int, > > > > const struct sbrec_chassis *chassis, > > > > const struct hmap *local_datapaths, > > > > const struct sset *active_tunnels, > > > > const struct shash *local_active_ports_ipv6_pd, > > > > const struct shash *local_active_ports_ras, > > > > - const struct ovsrec_open_vswitch_table *ovs_table) > > > > + const struct ovsrec_open_vswitch_table *ovs_table, > > > > + const struct hmap *garp_rarp_data) > > > > { > > > > ovs_mutex_lock(&pinctrl_mutex); > > > > run_put_mac_bindings(ovnsb_idl_txn, sbrec_datapath_binding_by_key, > > > > @@ -4092,11 +4086,8 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > > > > sbrec_mac_binding_by_lport_ip); > > > > run_put_vport_bindings(ovnsb_idl_txn, > > sbrec_datapath_binding_by_key, > > > > sbrec_port_binding_by_key, chassis); > > > > - send_garp_rarp_prepare(ovnsb_idl_txn, > > sbrec_port_binding_by_datapath, > > > > - sbrec_port_binding_by_name, > > > > - sbrec_mac_binding_by_lport_ip, > > ecmp_nh_table, > > > > - br_int, chassis, local_datapaths, > > > > active_tunnels, > > > > - ovs_table); > > > > + send_garp_rarp_prepare(garp_rarp_data, ecmp_nh_table, > > > > + chassis, ovs_table); > > > > prepare_ipv6_ras(local_active_ports_ras, > > sbrec_port_binding_by_name); > > > > prepare_ipv6_prefixd(ovnsb_idl_txn, sbrec_port_binding_by_name, > > > > local_active_ports_ipv6_pd, chassis, > > > > @@ -4785,47 +4776,6 @@ mac_binding_add_to_sb(struct ovsdb_idl_txn > > > > *ovnsb_idl_txn, > > > > } > > > > } > > > > > > > > -/* Simulate the effect of a GARP on local datapaths, i.e., create > > > > MAC_Bindings > > > > - * on peer router datapaths. > > > > - */ > > > > -static void > > > > -send_garp_locally(struct ovsdb_idl_txn *ovnsb_idl_txn, > > > > - struct ovsdb_idl_index > > *sbrec_mac_binding_by_lport_ip, > > > > - const struct hmap *local_datapaths, > > > > - const struct sbrec_port_binding *in_pb, > > > > - struct eth_addr ea, ovs_be32 ip) > > > > -{ > > > > - if (!ovnsb_idl_txn) { > > > > - return; > > > > - } > > > > - > > > > - const struct local_datapath *ldp = > > > > - get_local_datapath(local_datapaths, > > in_pb->datapath->tunnel_key); > > > > - > > > > - ovs_assert(ldp); > > > > - for (size_t i = 0; i < ldp->n_peer_ports; i++) { > > > > - const struct sbrec_port_binding *local = > > ldp->peer_ports[i].local; > > > > - const struct sbrec_port_binding *remote = > > > > ldp->peer_ports[i].remote; > > > > - > > > > - /* Skip "ingress" port. */ > > > > - if (local == in_pb) { > > > > - continue; > > > > - } > > > > - > > > > - bool update_only = > > !smap_get_bool(&remote->datapath->external_ids, > > > > - > > "always_learn_from_arp_request", > > > > - true); > > > > - > > > > - struct ds ip_s = DS_EMPTY_INITIALIZER; > > > > - > > > > - ip_format_masked(ip, OVS_BE32_MAX, &ip_s); > > > > - mac_binding_add_to_sb(ovnsb_idl_txn, > > > > sbrec_mac_binding_by_lport_ip, > > > > - remote->logical_port, remote->datapath, > > > > - ea, ds_cstr(&ip_s), update_only); > > > > - ds_destroy(&ip_s); > > > > - } > > > > -} > > > > - > > > > static void > > > > run_put_mac_binding(struct ovsdb_idl_txn *ovnsb_idl_txn, > > > > struct ovsdb_idl_index > > *sbrec_datapath_binding_by_key, > > > > @@ -4977,173 +4927,25 @@ wait_put_mac_bindings(struct ovsdb_idl_txn > > > > *ovnsb_idl_txn) > > > > * are needed for switches and routers on the broadcast segment to > > update > > > > * their port-mac and ARP tables. > > > > */ > > > > -struct garp_rarp_data { > > > > - struct eth_addr ea; /* Ethernet address of port. */ > > > > - ovs_be32 ipv4; /* Ipv4 address of port. */ > > > > - long long int announce_time; /* Next announcement in ms. */ > > > > - int backoff; /* Backoff timeout for the next > > > > - * announcement (in msecs). */ > > > > - uint32_t dp_key; /* Datapath used to output this > > GARP. */ > > > > - uint32_t port_key; /* Port to inject the GARP into. */ > > > > -}; > > > > > > > > -/* Contains GARPs/RARPs to be sent. Protected by pinctrl_mutex*/ > > > > -static struct shash send_garp_rarp_data; > > > > +/* Contains GARPs/RARPs to be sent in struct garp_rarp_node. > > > > + * Protected by pinctrl_mutex. */ > > > > +static struct hmap send_garp_rarp_data; > > > > > > > > static void > > > > init_send_garps_rarps(void) > > > > { > > > > - shash_init(&send_garp_rarp_data); > > > > + hmap_init(&send_garp_rarp_data); > > > > } > > > > > > > > static void > > > > destroy_send_garps_rarps(void) > > > > { > > > > - shash_destroy_free_data(&send_garp_rarp_data); > > > > -} > > > > - > > > > -/* Runs with in the main ovn-controller thread context. */ > > > > -static void > > > > -add_garp_rarp(const char *name, const struct eth_addr ea, ovs_be32 ip, > > > > - uint32_t dp_key, uint32_t port_key) > > > > -{ > > > > - struct garp_rarp_data *garp_rarp = xmalloc(sizeof *garp_rarp); > > > > - garp_rarp->ea = ea; > > > > - garp_rarp->ipv4 = ip; > > > > - garp_rarp->announce_time = time_msec() + 1000; > > > > - garp_rarp->backoff = 1000; /* msec. */ > > > > - garp_rarp->dp_key = dp_key; > > > > - garp_rarp->port_key = port_key; > > > > - shash_add(&send_garp_rarp_data, name, garp_rarp); > > > > - > > > > - /* Notify pinctrl_handler so that it can wakeup and process > > > > - * these GARP/RARP requests. */ > > > > - notify_pinctrl_handler(); > > > > -} > > > > - > > > > -/* Add or update a vif for which GARPs need to be announced. */ > > > > -static void > > > > -send_garp_rarp_update(struct ovsdb_idl_txn *ovnsb_idl_txn, > > > > - struct ovsdb_idl_index > > > > *sbrec_mac_binding_by_lport_ip, > > > > - const struct hmap *local_datapaths, > > > > - const struct sbrec_port_binding *binding_rec, > > > > - struct shash *nat_addresses, > > > > - long long int garp_max_timeout, > > > > - bool garp_continuous) > > > > -{ > > > > - volatile struct garp_rarp_data *garp_rarp = NULL; > > > > - > > > > - /* Skip localports as they don't need to be announced */ > > > > - if (!strcmp(binding_rec->type, "localport")) { > > > > - return; > > > > - } > > > > - > > > > - /* Update GARP for NAT IP if it exists. Consider port bindings > > with > > > > type > > > > - * "l3gateway" for logical switch ports attached to gateway > > routers, > > > > and > > > > - * port bindings with type "patch" for logical switch ports > > attached > > > > to > > > > - * distributed gateway ports. */ > > > > - if (!strcmp(binding_rec->type, "l3gateway") > > > > - || !strcmp(binding_rec->type, "patch")) { > > > > - struct lport_addresses *laddrs = NULL; > > > > - while ((laddrs = shash_find_and_delete(nat_addresses, > > > > - > > > > binding_rec->logical_port))) { > > > > - int i; > > > > - for (i = 0; i < laddrs->n_ipv4_addrs; i++) { > > > > - char *name = xasprintf("%s-%s", > > binding_rec->logical_port, > > > > - > > > > laddrs->ipv4_addrs[i].addr_s); > > > > - garp_rarp = shash_find_data(&send_garp_rarp_data, > > name); > > > > - if (garp_rarp) { > > > > - garp_rarp->dp_key = > > binding_rec->datapath->tunnel_key; > > > > - garp_rarp->port_key = binding_rec->tunnel_key; > > > > - if (garp_max_timeout != garp_rarp_max_timeout || > > > > - garp_continuous != garp_rarp_continuous) { > > > > - /* reset backoff */ > > > > - garp_rarp->announce_time = time_msec() + 1000; > > > > - garp_rarp->backoff = 1000; /* msec. */ > > > > - } > > > > - } else if (ovnsb_idl_txn) { > > > > - add_garp_rarp(name, laddrs->ea, > > > > - laddrs->ipv4_addrs[i].addr, > > > > - binding_rec->datapath->tunnel_key, > > > > - binding_rec->tunnel_key); > > > > - send_garp_locally(ovnsb_idl_txn, > > > > - sbrec_mac_binding_by_lport_ip, > > > > - local_datapaths, binding_rec, > > > > laddrs->ea, > > > > - laddrs->ipv4_addrs[i].addr); > > > > - > > > > - } > > > > - free(name); > > > > - } > > > > - /* > > > > - * Send RARPs even if we do not have a ipv4 address as it > > e.g. > > > > - * happens on ipv6 only ports. > > > > - */ > > > > - if (laddrs->n_ipv4_addrs == 0) { > > > > - char *name = xasprintf("%s-noip", > > > > - binding_rec->logical_port); > > > > - garp_rarp = shash_find_data(&send_garp_rarp_data, > > > > name); > > > > - if (garp_rarp) { > > > > - garp_rarp->dp_key = > > > > binding_rec->datapath->tunnel_key; > > > > - garp_rarp->port_key = binding_rec->tunnel_key; > > > > - if (garp_max_timeout != garp_rarp_max_timeout > > || > > > > - garp_continuous != garp_rarp_continuous) { > > > > - /* reset backoff */ > > > > - garp_rarp->announce_time = time_msec() + > > 1000; > > > > - garp_rarp->backoff = 1000; /* msec. */ > > > > - } > > > > - } else { > > > > - add_garp_rarp(name, laddrs->ea, > > > > - 0, > > > > binding_rec->datapath->tunnel_key, > > > > - binding_rec->tunnel_key); > > > > - } > > > > - free(name); > > > > - } > > > > - destroy_lport_addresses(laddrs); > > > > - free(laddrs); > > > > - } > > > > - return; > > > > - } > > > > - > > > > - /* Update GARP for vif if it exists. */ > > > > - garp_rarp = shash_find_data(&send_garp_rarp_data, > > > > - binding_rec->logical_port); > > > > - if (garp_rarp) { > > > > - garp_rarp->dp_key = binding_rec->datapath->tunnel_key; > > > > - garp_rarp->port_key = binding_rec->tunnel_key; > > > > - if (garp_max_timeout != garp_rarp_max_timeout || > > > > - garp_continuous != garp_rarp_continuous) { > > > > - /* reset backoff */ > > > > - garp_rarp->announce_time = time_msec() + 1000; > > > > - garp_rarp->backoff = 1000; /* msec. */ > > > > - } > > > > - return; > > > > - } > > > > - > > > > - /* Add GARP for new vif. */ > > > > - int i; > > > > - for (i = 0; i < binding_rec->n_mac; i++) { > > > > - struct lport_addresses laddrs; > > > > - ovs_be32 ip = 0; > > > > - if (!extract_lsp_addresses(binding_rec->mac[i], &laddrs)) { > > > > - continue; > > > > - } > > > > - > > > > - if (laddrs.n_ipv4_addrs) { > > > > - ip = laddrs.ipv4_addrs[0].addr; > > > > - } > > > > - > > > > - add_garp_rarp(binding_rec->logical_port, > > > > - laddrs.ea, ip, > > > > - binding_rec->datapath->tunnel_key, > > > > - binding_rec->tunnel_key); > > > > - if (ip) { > > > > - send_garp_locally(ovnsb_idl_txn, > > > > sbrec_mac_binding_by_lport_ip, > > > > - local_datapaths, binding_rec, laddrs.ea, > > > > ip); > > > > - } > > > > - > > > > - destroy_lport_addresses(&laddrs); > > > > - break; > > > > + struct garp_rarp_node *garp_rarp; > > > > + HMAP_FOR_EACH_POP (garp_rarp, hmap_node, &send_garp_rarp_data) { > > > > + garp_rarp_node_free(garp_rarp); > > > > } > > > > + hmap_destroy(&send_garp_rarp_data); > > > > } > > > > > > > > struct arp_nd_data { > > > > @@ -5289,16 +5091,6 @@ send_arp_nd_update(const struct > > sbrec_port_binding > > > > *pb, const char *nexthop, > > > > } > > > > } > > > > > > > > -/* Remove a vif from GARP announcements. */ > > > > -static void > > > > -send_garp_rarp_delete(const char *lport) > > > > -{ > > > > - struct garp_rarp_data *garp_rarp = shash_find_and_delete > > > > - (&send_garp_rarp_data, lport); > > > > - free(garp_rarp); > > > > - notify_pinctrl_handler(); > > > > -} > > > > - > > > > void > > > > send_self_originated_neigh_packet(struct rconn *swconn, > > > > uint32_t dp_key, uint32_t port_key, > > > > @@ -5350,7 +5142,7 @@ send_self_originated_neigh_packet(struct rconn > > > > *swconn, > > > > > > > > /* Called with in the pinctrl_handler thread context. */ > > > > static long long int > > > > -send_garp_rarp(struct rconn *swconn, struct garp_rarp_data *garp_rarp, > > > > +send_garp_rarp(struct rconn *swconn, struct garp_rarp_node *garp_rarp, > > > > long long int current_time) > > > > OVS_REQUIRES(pinctrl_mutex) > > > > { > > > > @@ -6419,242 +6211,12 @@ ip_mcast_querier_wait(long long int > > query_time) > > > > } > > > > } > > > > > > > > -/* Get localnet vifs, local l3gw ports and ofport for localnet patch > > > > ports. */ > > > > -static void > > > > -get_localnet_vifs_l3gwports( > > > > - struct ovsdb_idl_index *sbrec_port_binding_by_datapath, > > > > - struct ovsdb_idl_index *sbrec_port_binding_by_name, > > > > - const struct ovsrec_bridge *br_int, > > > > - const struct sbrec_chassis *chassis, > > > > - const struct hmap *local_datapaths, > > > > - struct sset *localnet_vifs, > > > > - struct sset *local_l3gw_ports) > > > > -{ > > > > - for (int i = 0; i < br_int->n_ports; i++) { > > > > - const struct ovsrec_port *port_rec = br_int->ports[i]; > > > > - if (!strcmp(port_rec->name, br_int->name)) { > > > > - continue; > > > > - } > > > > - const char *tunnel_id = smap_get(&port_rec->external_ids, > > > > - "ovn-chassis-id"); > > > > - if (tunnel_id && > > > > - encaps_tunnel_id_match(tunnel_id, chassis->name, NULL, > > > > NULL)) { > > > > - continue; > > > > - } > > > > - const char *localnet = smap_get(&port_rec->external_ids, > > > > - "ovn-localnet-port"); > > > > - if (localnet) { > > > > - continue; > > > > - } > > > > - for (int j = 0; j < port_rec->n_interfaces; j++) { > > > > - const struct ovsrec_interface *iface_rec = > > > > port_rec->interfaces[j]; > > > > - if (!iface_rec->n_ofport) { > > > > - continue; > > > > - } > > > > - /* Get localnet vif. */ > > > > - const char *iface_id = smap_get(&iface_rec->external_ids, > > > > - "iface-id"); > > > > - if (!iface_id) { > > > > - continue; > > > > - } > > > > - const struct sbrec_port_binding *pb > > > > - = lport_lookup_by_name(sbrec_port_binding_by_name, > > > > iface_id); > > > > - if (!pb || pb->chassis != chassis) { > > > > - continue; > > > > - } > > > > - if (!iface_rec->link_state || > > > > - strcmp(iface_rec->link_state, "up")) { > > > > - continue; > > > > - } > > > > - struct local_datapath *ld > > > > - = get_local_datapath(local_datapaths, > > > > - pb->datapath->tunnel_key); > > > > - if (ld && ld->localnet_port) { > > > > - sset_add(localnet_vifs, iface_id); > > > > - } > > > > - } > > > > - } > > > > - > > > > - struct sbrec_port_binding *target = > > sbrec_port_binding_index_init_row( > > > > - sbrec_port_binding_by_datapath); > > > > - > > > > - const struct local_datapath *ld; > > > > - HMAP_FOR_EACH (ld, hmap_node, local_datapaths) { > > > > - const struct sbrec_port_binding *pb; > > > > - > > > > - if (!ld->localnet_port) { > > > > - continue; > > > > - } > > > > - > > > > - /* Get l3gw ports. Consider port bindings with type > > "l3gateway" > > > > - * that connect to gateway routers (if local), and consider > > port > > > > - * bindings of type "patch" since they might connect to > > > > - * distributed gateway ports with NAT addresses. */ > > > > - > > > > - sbrec_port_binding_index_set_datapath(target, ld->datapath); > > > > - SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target, > > > > - > > > > sbrec_port_binding_by_datapath) { > > > > - if ((!strcmp(pb->type, "l3gateway") && pb->chassis == > > chassis) > > > > - || !strcmp(pb->type, "patch")) { > > > > - sset_add(local_l3gw_ports, pb->logical_port); > > > > - } > > > > - } > > > > - } > > > > - sbrec_port_binding_index_destroy_row(target); > > > > -} > > > > - > > > > - > > > > -/* Extracts the mac, IPv4 and IPv6 addresses, and logical port from > > > > - * 'addresses' which should be of the format 'MAC [IP1 IP2 ..] > > > > - * [is_chassis_resident("LPORT_NAME")]', where IPn should be a valid > > IPv4 > > > > - * or IPv6 address, and stores them in the 'ipv4_addrs' and > > 'ipv6_addrs' > > > > - * fields of 'laddrs'. The logical port name is stored in 'lport'. > > > > - * > > > > - * Returns true if at least 'MAC' is found in 'address', false > > otherwise. > > > > - * > > > > - * The caller must call destroy_lport_addresses() and free(*lport). */ > > > > -static bool > > > > -extract_addresses_with_port(const char *addresses, > > > > - struct lport_addresses *laddrs, > > > > - char **lport) > > > > -{ > > > > - int ofs; > > > > - if (!extract_addresses(addresses, laddrs, &ofs)) { > > > > - return false; > > > > - } else if (!addresses[ofs]) { > > > > - return true; > > > > - } > > > > - > > > > - struct lexer lexer; > > > > - lexer_init(&lexer, addresses + ofs); > > > > - lexer_get(&lexer); > > > > - > > > > - if (lexer.error || lexer.token.type != LEX_T_ID > > > > - || !lexer_match_id(&lexer, "is_chassis_resident")) { > > > > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > > > > - VLOG_INFO_RL(&rl, "invalid syntax '%s' in address", > > addresses); > > > > - lexer_destroy(&lexer); > > > > - return true; > > > > - } > > > > - > > > > - if (!lexer_match(&lexer, LEX_T_LPAREN)) { > > > > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > > > > - VLOG_INFO_RL(&rl, "Syntax error: expecting '(' after " > > > > - "'is_chassis_resident' in address '%s'", > > > > addresses); > > > > - lexer_destroy(&lexer); > > > > - return false; > > > > - } > > > > - > > > > - if (lexer.token.type != LEX_T_STRING) { > > > > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > > > > - VLOG_INFO_RL(&rl, > > > > - "Syntax error: expecting quoted string after " > > > > - "'is_chassis_resident' in address '%s'", > > addresses); > > > > - lexer_destroy(&lexer); > > > > - return false; > > > > - } > > > > - > > > > - *lport = xstrdup(lexer.token.s); > > > > - > > > > - lexer_get(&lexer); > > > > - if (!lexer_match(&lexer, LEX_T_RPAREN)) { > > > > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > > > > - VLOG_INFO_RL(&rl, "Syntax error: expecting ')' after quoted > > > > string in " > > > > - "'is_chassis_resident()' in address '%s'", > > > > - addresses); > > > > - lexer_destroy(&lexer); > > > > - return false; > > > > - } > > > > - > > > > - lexer_destroy(&lexer); > > > > - return true; > > > > -} > > > > - > > > > -static void > > > > -consider_nat_address(struct ovsdb_idl_index > > *sbrec_port_binding_by_name, > > > > - const char *nat_address, > > > > - const struct sbrec_port_binding *pb, > > > > - struct sset *nat_address_keys, > > > > - const struct sbrec_chassis *chassis, > > > > - const struct sset *active_tunnels, > > > > - struct shash *nat_addresses) > > > > -{ > > > > - struct lport_addresses *laddrs = xmalloc(sizeof *laddrs); > > > > - char *lport = NULL; > > > > - if (!extract_addresses_with_port(nat_address, laddrs, &lport) > > > > - || (!lport && !strcmp(pb->type, "patch")) > > > > - || (lport && !lport_is_chassis_resident( > > > > - sbrec_port_binding_by_name, chassis, > > > > - active_tunnels, lport))) { > > > > - destroy_lport_addresses(laddrs); > > > > - free(laddrs); > > > > - free(lport); > > > > - return; > > > > - } > > > > - free(lport); > > > > - > > > > - int i; > > > > - for (i = 0; i < laddrs->n_ipv4_addrs; i++) { > > > > - char *name = xasprintf("%s-%s", pb->logical_port, > > > > - laddrs->ipv4_addrs[i].addr_s); > > > > - sset_add(nat_address_keys, name); > > > > - free(name); > > > > - } > > > > - if (laddrs->n_ipv4_addrs == 0) { > > > > - char *name = xasprintf("%s-noip", pb->logical_port); > > > > - sset_add(nat_address_keys, name); > > > > - free(name); > > > > - } > > > > - shash_add(nat_addresses, pb->logical_port, laddrs); > > > > -} > > > > - > > > > -static void > > > > -get_nat_addresses_and_keys(struct ovsdb_idl_index > > > > *sbrec_port_binding_by_name, > > > > - struct sset *nat_address_keys, > > > > - struct sset *local_l3gw_ports, > > > > - const struct sbrec_chassis *chassis, > > > > - const struct sset *active_tunnels, > > > > - struct shash *nat_addresses) > > > > -{ > > > > - const char *gw_port; > > > > - SSET_FOR_EACH(gw_port, local_l3gw_ports) { > > > > - const struct sbrec_port_binding *pb; > > > > - > > > > - pb = lport_lookup_by_name(sbrec_port_binding_by_name, > > gw_port); > > > > - if (!pb) { > > > > - continue; > > > > - } > > > > - > > > > - if (pb->n_nat_addresses) { > > > > - for (int i = 0; i < pb->n_nat_addresses; i++) { > > > > - consider_nat_address(sbrec_port_binding_by_name, > > > > - pb->nat_addresses[i], pb, > > > > - nat_address_keys, chassis, > > > > - active_tunnels, > > > > - nat_addresses); > > > > - } > > > > - } else { > > > > - /* Continue to support options:nat-addresses for version > > > > - * upgrade. */ > > > > - const char *nat_addresses_options = smap_get(&pb->options, > > > > - > > "nat-addresses"); > > > > - if (nat_addresses_options) { > > > > - consider_nat_address(sbrec_port_binding_by_name, > > > > - nat_addresses_options, pb, > > > > - nat_address_keys, chassis, > > > > - active_tunnels, > > > > - nat_addresses); > > > > - } > > > > - } > > > > - } > > > > -} > > > > - > > > > static void > > > > send_garp_rarp_wait(long long int send_garp_rarp_time) > > > > { > > > > /* Set the poll timer for next garp/rarp only if there is data to > > > > * be sent. */ > > > > - if (!shash_is_empty(&send_garp_rarp_data)) { > > > > + if (!hmap_is_empty(&send_garp_rarp_data)) { > > > > poll_timer_wait_until(send_garp_rarp_time); > > > > } > > > > } > > > > @@ -6674,16 +6236,16 @@ static void > > > > send_garp_rarp_run(struct rconn *swconn, long long int > > > > *send_garp_rarp_time) > > > > OVS_REQUIRES(pinctrl_mutex) > > > > { > > > > - if (shash_is_empty(&send_garp_rarp_data)) { > > > > + if (hmap_is_empty(&send_garp_rarp_data)) { > > > > return; > > > > } > > > > > > > > /* Send GARPs, and update the next announcement. */ > > > > - struct shash_node *iter; > > > > long long int current_time = time_msec(); > > > > *send_garp_rarp_time = LLONG_MAX; > > > > - SHASH_FOR_EACH (iter, &send_garp_rarp_data) { > > > > - long long int next_announce = send_garp_rarp(swconn, > > iter->data, > > > > + struct garp_rarp_node *garp; > > > > + HMAP_FOR_EACH (garp, hmap_node, &send_garp_rarp_data) { > > > > + long long int next_announce = send_garp_rarp(swconn, garp, > > > > current_time); > > > > if (*send_garp_rarp_time > next_announce) { > > > > *send_garp_rarp_time = next_announce; > > > > @@ -6743,22 +6305,12 @@ send_arp_nd_run(struct rconn *swconn, long long > > > > int *send_arp_nd_time) > > > > /* Called by pinctrl_run(). Runs with in the main ovn-controller > > > > * thread context. */ > > > > static void > > > > -send_garp_rarp_prepare(struct ovsdb_idl_txn *ovnsb_idl_txn, > > > > - struct ovsdb_idl_index > > > > *sbrec_port_binding_by_datapath, > > > > - struct ovsdb_idl_index > > *sbrec_port_binding_by_name, > > > > - struct ovsdb_idl_index > > > > *sbrec_mac_binding_by_lport_ip, > > > > +send_garp_rarp_prepare(const struct hmap *garp_rarp_data, > > > > const struct sbrec_ecmp_nexthop_table > > > > *ecmp_nh_table, > > > > - const struct ovsrec_bridge *br_int, > > > > const struct sbrec_chassis *chassis, > > > > - const struct hmap *local_datapaths, > > > > - const struct sset *active_tunnels, > > > > const struct ovsrec_open_vswitch_table > > *ovs_table) > > > > OVS_REQUIRES(pinctrl_mutex) > > > > { > > > > - struct sset localnet_vifs = SSET_INITIALIZER(&localnet_vifs); > > > > - struct sset local_l3gw_ports = > > SSET_INITIALIZER(&local_l3gw_ports); > > > > - struct sset nat_ip_keys = SSET_INITIALIZER(&nat_ip_keys); > > > > - struct shash nat_addresses; > > > > unsigned long long garp_max_timeout = GARP_RARP_DEF_MAX_TIMEOUT; > > > > unsigned long long max_arp_nd_timeout = GARP_RARP_DEF_MAX_TIMEOUT; > > > > bool garp_continuous = false, continuous_arp_nd = true; > > > > @@ -6778,51 +6330,13 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn > > > > *ovnsb_idl_txn, > > > > continuous_arp_nd = !!max_arp_nd_timeout; > > > > } > > > > > > > > - shash_init(&nat_addresses); > > > > + bool reset_timers = (garp_max_timeout != garp_rarp_max_timeout || > > > > + garp_continuous != garp_rarp_continuous); > > > > > > > > - get_localnet_vifs_l3gwports(sbrec_port_binding_by_datapath, > > > > - sbrec_port_binding_by_name, > > > > - br_int, chassis, local_datapaths, > > > > - &localnet_vifs, &local_l3gw_ports); > > > > - > > > > - get_nat_addresses_and_keys(sbrec_port_binding_by_name, > > > > - &nat_ip_keys, &local_l3gw_ports, > > > > - chassis, active_tunnels, > > > > - &nat_addresses); > > > > - > > > > - /* For deleted ports and deleted nat ips, remove from > > > > - * send_garp_rarp_data. */ > > > > - struct shash_node *iter; > > > > - SHASH_FOR_EACH_SAFE (iter, &send_garp_rarp_data) { > > > > - if (!sset_contains(&localnet_vifs, iter->name) && > > > > - !sset_contains(&nat_ip_keys, iter->name)) { > > > > - send_garp_rarp_delete(iter->name); > > > > - } > > > > - } > > > > - > > > > - /* Update send_garp_rarp_data. */ > > > > - const char *iface_id; > > > > - SSET_FOR_EACH (iface_id, &localnet_vifs) { > > > > - const struct sbrec_port_binding *pb = lport_lookup_by_name( > > > > - sbrec_port_binding_by_name, iface_id); > > > > - if (pb && !smap_get_bool(&pb->options, "disable_garp_rarp", > > > > false)) { > > > > - send_garp_rarp_update(ovnsb_idl_txn, > > > > - sbrec_mac_binding_by_lport_ip, > > > > - local_datapaths, pb, &nat_addresses, > > > > - garp_max_timeout, garp_continuous); > > > > - } > > > > - } > > > > - > > > > - /* Update send_garp_rarp_data for nat-addresses. */ > > > > - const char *gw_port; > > > > - SSET_FOR_EACH (gw_port, &local_l3gw_ports) { > > > > - const struct sbrec_port_binding *pb > > > > - = lport_lookup_by_name(sbrec_port_binding_by_name, > > gw_port); > > > > - if (pb && !smap_get_bool(&pb->options, "disable_garp_rarp", > > > > false)) { > > > > - send_garp_rarp_update(ovnsb_idl_txn, > > > > sbrec_mac_binding_by_lport_ip, > > > > - local_datapaths, pb, &nat_addresses, > > > > - garp_max_timeout, garp_continuous); > > > > - } > > > > + if (garp_rarp_sync(garp_rarp_data, &send_garp_rarp_data, > > > > reset_timers)) { > > > > + /* Notify pinctrl_handler so that it can wakeup and process > > > > + * these GARP/RARP requests. */ > > > > + notify_pinctrl_handler(); > > > > } > > > > > > > > arp_nd_sync_data(ecmp_nh_table); > > > > @@ -6836,21 +6350,6 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn > > > > *ovnsb_idl_txn, > > > > } > > > > } > > > > > > > > - /* pinctrl_handler thread will send the GARPs. */ > > > > - > > > > - sset_destroy(&localnet_vifs); > > > > - sset_destroy(&local_l3gw_ports); > > > > - > > > > - SHASH_FOR_EACH_SAFE (iter, &nat_addresses) { > > > > - struct lport_addresses *laddrs = iter->data; > > > > - destroy_lport_addresses(laddrs); > > > > - shash_delete(&nat_addresses, iter); > > > > - free(laddrs); > > > > - } > > > > - shash_destroy(&nat_addresses); > > > > - > > > > - sset_destroy(&nat_ip_keys); > > > > - > > > > garp_rarp_max_timeout = garp_max_timeout; > > > > garp_rarp_continuous = garp_continuous; > > > > > > > > @@ -6862,7 +6361,7 @@ static bool > > > > may_inject_pkts(void) > > > > { > > > > return (!shash_is_empty(&ipv6_ras) || > > > > - !shash_is_empty(&send_garp_rarp_data) || > > > > + !hmap_is_empty(&send_garp_rarp_data) || > > > > ipv6_prefixd_should_inject() || > > > > !ovs_list_is_empty(&mcast_query_list) || > > > > > > buffered_packets_ctx_is_ready_to_send(&buffered_packets_ctx) > > > > || > > > > diff --git a/controller/pinctrl.h b/controller/pinctrl.h > > > > index e33d7cbf5..f57f5c306 100644 > > > > --- a/controller/pinctrl.h > > > > +++ b/controller/pinctrl.h > > > > @@ -44,24 +44,24 @@ struct sbrec_mac_binding_table; > > > > void pinctrl_init(void); > > > > void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > > > > struct ovsdb_idl_index > > *sbrec_datapath_binding_by_key, > > > > - struct ovsdb_idl_index > > *sbrec_port_binding_by_datapath, > > > > struct ovsdb_idl_index *sbrec_port_binding_by_key, > > > > struct ovsdb_idl_index *sbrec_port_binding_by_name, > > > > struct ovsdb_idl_index > > *sbrec_mac_binding_by_lport_ip, > > > > struct ovsdb_idl_index *sbrec_igmp_groups, > > > > struct ovsdb_idl_index *sbrec_ip_multicast_opts, > > > > struct ovsdb_idl_index *sbrec_fdb_by_dp_key_mac, > > > > - const struct sbrec_controller_event_table *, > > > > - const struct sbrec_service_monitor_table *, > > > > - const struct sbrec_mac_binding_table *, > > > > - const struct sbrec_bfd_table *, > > > > - const struct sbrec_ecmp_nexthop_table *, > > > > - const struct ovsrec_bridge *, const struct > > sbrec_chassis > > > > *, > > > > + const struct sbrec_controller_event_table *ce_table, > > > > + const struct sbrec_service_monitor_table > > *svc_mon_table, > > > > + const struct sbrec_mac_binding_table > > *mac_binding_table, > > > > + const struct sbrec_bfd_table *bfd_table, > > > > + const struct sbrec_ecmp_nexthop_table *ecmp_nh_table, > > > > + const struct sbrec_chassis *chassis, > > > > > > > > > > nit: Unrelated change. > > > > will be cleaned up. > > > > Thanks a lot, > > Felix > > > > > > > > const struct hmap *local_datapaths, > > > > const struct sset *active_tunnels, > > > > const struct shash *local_active_ports_ipv6_pd, > > > > const struct shash *local_active_ports_ras, > > > > - const struct ovsrec_open_vswitch_table *ovs_table); > > > > + const struct ovsrec_open_vswitch_table *ovs_table, > > > > + const struct hmap *garp_rarp_data); > > > > void pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn); > > > > void pinctrl_destroy(void); > > > > > > > > -- > > > > 2.43.0 > > > > > > > > > > > > _______________________________________________ > > > > dev mailing list > > > > d...@openvswitch.org > > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > > > > > > > Thanks, > > > Ales > > > > > Thanks, > Ales > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
