On Mon, Apr 14, 2025 at 1:38 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 v8. I have some comments down below. v7->v8: > * addressed review comments > * changed to cmap for data storage > v6->v7: added ack > > controller/automake.mk | 4 +- > controller/garp_rarp.c | 542 +++++++++++++++++++++++++++++++ > controller/garp_rarp.h | 70 +++++ > controller/ovn-controller.c | 206 +++++++++++- > controller/pinctrl.c | 612 ++---------------------------------- > controller/pinctrl.h | 3 +- > lib/ovn-util.c | 43 +++ > lib/ovn-util.h | 6 + > 8 files changed, 889 insertions(+), 597 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..5f924bad6 > --- /dev/null > +++ b/controller/garp_rarp.c > @@ -0,0 +1,542 @@ > +/* > + * 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 "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" > + > +VLOG_DEFINE_THIS_MODULE(garp_rarp); > +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); > + > +static atomic_bool garp_rarp_data_changed = false; > This doesn't have to be atomic, all updates and reads are happening in the main thread (engine_run(), send_garp_rarp_prepare()). > +static struct cmap garp_rarp_data; > + > +/* 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, > + const struct sbrec_chassis *chassis, > + const struct hmap *local_datapaths, > + struct sset *localnet_vifs, > + struct sset *local_l3gw_ports) > +{ > + 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; > + } > + > + sbrec_port_binding_index_set_datapath(target, ld->datapath); > + SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target, > + > sbrec_port_binding_by_datapath) { > + /* 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. */ > + if ((!strcmp(pb->type, "l3gateway") && pb->chassis == chassis) > + || !strcmp(pb->type, "patch")) { > + sset_add(local_l3gw_ports, pb->logical_port); > + } > + > + /* Get all vifs that are directly connected to a localnet > port. */ > + if (!strcmp(pb->type, "") && pb->chassis == chassis) { > + sset_add(localnet_vifs, 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); > +} > + > +/* 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 eth_addr ea, ovs_be32 ipv4, uint32_t dp_key, > + uint32_t port_key) > +{ > + struct garp_rarp_node *grn; > + uint32_t hash = garp_rarp_node_hash(&ea, dp_key, port_key); > + CMAP_FOR_EACH_WITH_HASH (grn, cmap_node, hash, &garp_rarp_data) { > + if (!eth_addr_equals(ea, grn->ea)) { > + continue; > + } > + > + if (ipv4 != grn->ipv4) { > + continue; > + } > + > + if (dp_key != grn->dp_key) { > + continue; > + } > + > + if (port_key != grn->port_key) { > + continue; > + } > + > + return grn; > + } > + return NULL; > +} > + > +static void > +garp_rarp_node_add(const struct eth_addr ea, ovs_be32 ip, > + uint32_t dp_key, uint32_t port_key) > +{ > + struct garp_rarp_node *grn = garp_rarp_lookup(ea, ip, dp_key, > port_key); > + if (grn) { > + grn->stale = false; > + return; > + } > + > + 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; > + grn->stale = false; > + cmap_insert(&garp_rarp_data, &grn->cmap_node, > + garp_rarp_node_hash_struct(grn)); > + atomic_store(&garp_rarp_data_changed, true); > +} > + > +/* 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) > +{ > + /* 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(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(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++) { > nit: size_t i should be used instead. + 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(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 void > +garp_rarp_clear(struct garp_rarp_ctx_in *r_ctx_in) > +{ > + 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) > +{ > + garp_rarp_clear(r_ctx_in); > + > + 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_INITIALIZER(&nat_addresses); > + > + struct garp_rarp_node *grn; > + CMAP_FOR_EACH (grn, cmap_node, &garp_rarp_data) { > + grn->stale = true; > + } > + > + get_localnet_vifs_l3gwports(r_ctx_in->sbrec_port_binding_by_datapath, > + 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); > + } > + } > + > + /* 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); > + } > + } > + > + 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); > + > + CMAP_FOR_EACH (grn, cmap_node, &garp_rarp_data) { > + if (grn->stale) { > + cmap_remove(&garp_rarp_data, &grn->cmap_node, > + garp_rarp_node_hash_struct(grn)); > + ovsrcu_postpone(garp_rarp_node_free, grn); > + } > + } > +} > + > +const struct cmap * > +garp_rarp_get_data(void) { > + return &garp_rarp_data; > +} > + > +bool garp_rarp_data_sync(bool reset_timers) { > + bool ret; > + atomic_read(&garp_rarp_data_changed, &ret); > + atomic_store_relaxed(&garp_rarp_data_changed, false); > + > + if (reset_timers) { > + struct garp_rarp_node *grn; > + CMAP_FOR_EACH (grn, cmap_node, &garp_rarp_data) { > + grn->announce_time = time_msec() + 1000; > + grn->backoff = 1000; > + } > + ret = true; > + } > + return ret; > +} > + > +void > +garp_rarp_node_free(struct garp_rarp_node *garp_rarp) > +{ > + free(garp_rarp); > +} > + > +struct ed_type_garp_rarp * > +garp_rarp_init(void) > +{ > + cmap_init(&garp_rarp_data); > + struct ed_type_garp_rarp *gr = xmalloc(sizeof *gr); > + sset_init(&gr->non_local_lports); > + sset_init(&gr->local_lports); > + return gr; > +} > + > +void > +garp_rarp_cleanup(struct ed_type_garp_rarp *data) > +{ > + struct garp_rarp_node *grn; > + CMAP_FOR_EACH (grn, cmap_node, &garp_rarp_data) { > + cmap_remove(&garp_rarp_data, &grn->cmap_node, > + garp_rarp_node_hash_struct(grn)); > + garp_rarp_node_free(grn); > + } > + sset_destroy(&data->non_local_lports); > + sset_destroy(&data->local_lports); > +} > diff --git a/controller/garp_rarp.h b/controller/garp_rarp.h > new file mode 100644 > index 000000000..047313ddd > --- /dev/null > +++ b/controller/garp_rarp.h > @@ -0,0 +1,70 @@ > +/* > + * 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 "cmap.h" > +#include "sset.h" > +#include "openvswitch/types.h" > + > +/* Contains a single mac and ip address that should be announced. */ > +struct garp_rarp_node { > + struct cmap_node cmap_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. */ > + bool stale; /* Used during sync to remove stale > + * information. */ > +}; > + > +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 sbrec_chassis *chassis; > + const struct hmap *local_datapaths; > + const struct sset *active_tunnels; > + struct sset *non_local_lports; > + struct sset *local_lports; > +}; > + > +struct ed_type_garp_rarp { > + /* 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. */ > +}; > + > +void garp_rarp_run(struct garp_rarp_ctx_in *); > +void garp_rarp_node_free(struct garp_rarp_node *); > +const struct cmap * garp_rarp_get_data(void); > nit: Wrong formatting +bool garp_rarp_data_sync(bool reset_timers); > + > +struct ed_type_garp_rarp * garp_rarp_init(void); > nit: Wrong formatting > +void garp_rarp_cleanup(struct ed_type_garp_rarp *); > + > +#endif /* GARP_RARP_H */ > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 716644084..c6b471ca8 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); > > @@ -4964,6 +4965,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 > @@ -5317,6 +5326,185 @@ en_route_table_notify_cleanup(void *data > OVS_UNUSED) > { > } > > +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); > AFAIK we will never do engine_run without proper chassis, so those asserts on chassis and chassis_id might be redundant. + > + 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, > + .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, > + }; > + > + garp_rarp_run(&r_ctx_in); > +} > + > + > +static void * > +en_garp_rarp_init(struct engine_node *node OVS_UNUSED, > + struct engine_arg *arg OVS_UNUSED) > +{ > + return garp_rarp_init(); > +} > + > +static void > +en_garp_rarp_cleanup(void *data) > +{ > + garp_rarp_cleanup(data); > +} > + > +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. > */ > @@ -5626,6 +5814,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 > @@ -5838,6 +6027,16 @@ 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_sb_chassis, NULL); > + engine_add_input(&en_garp_rarp, &en_sb_port_binding, > + garp_rarp_sb_port_binding_handler); > + /* The mac_binding data is just used in an index to filter duplicates > when > + * inserting data to the southbound. */ > + engine_add_input(&en_garp_rarp, &en_sb_mac_binding, > engine_noop_handler); > + 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, > @@ -5850,6 +6049,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, > @@ -5886,6 +6087,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); > @@ -6321,7 +6524,6 @@ main(int argc, char *argv[]) > pinctrl_update(ovnsb_idl_loop.idl); > 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, > @@ -6337,7 +6539,7 @@ 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, > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index bdb619b4d..a4aaec306 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); > > @@ -226,27 +227,17 @@ static void run_activated_ports( > struct ovsdb_idl_index *sbrec_port_binding_by_name, > const struct sbrec_chassis *chassis); > > -static void init_send_garps_rarps(void); > static void init_send_arps_nds(void); > -static void destroy_send_garps_rarps(void); > 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 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, > - long long int *send_garp_rarp_time) > - OVS_REQUIRES(pinctrl_mutex); > + long long int *send_garp_rarp_time); > static void send_arp_nd_run(struct rconn *swconn, > long long int *send_arp_nd_time) > OVS_REQUIRES(pinctrl_mutex); > @@ -560,7 +551,6 @@ void > pinctrl_init(void) > { > init_put_mac_bindings(); > - init_send_garps_rarps(); > init_send_arps_nds(); > init_ipv6_ras(); > init_ipv6_prefixd(); > @@ -3957,7 +3947,6 @@ pinctrl_handler(void *arg_) > > if (may_inject_pkts()) { > if (!ovs_mutex_trylock(&pinctrl_mutex)) { > - send_garp_rarp_run(swconn, &send_garp_rarp_time); > send_arp_nd_run(swconn, &send_arp_nd_time); > send_ipv6_ras(swconn, &send_ipv6_ra_time); > send_ipv6_prefixd(swconn, &send_prefixd_time); > @@ -3967,6 +3956,7 @@ pinctrl_handler(void *arg_) > } else { > lock_failed = true; > } > + send_garp_rarp_run(swconn, &send_garp_rarp_time); > ip_mcast_querier_run(swconn, &send_mcast_query_time); > } > > @@ -4066,7 +4056,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,7 +4067,6 @@ 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, > @@ -4092,11 +4080,7 @@ 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(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, > @@ -4637,7 +4621,6 @@ pinctrl_destroy(void) > pthread_join(pinctrl.pinctrl_thread, NULL); > latch_destroy(&pinctrl.pinctrl_thread_exit); > rconn_destroy(pinctrl.swconn); > - destroy_send_garps_rarps(); > destroy_send_arps_nds(); > destroy_ipv6_ras(); > destroy_ipv6_prefixd(); > @@ -4744,88 +4727,6 @@ send_mac_binding_buffered_pkts(struct rconn *swconn) > ovs_list_init(&buffered_packets_ctx.ready_packets_data); > } > > -/* 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 (pinctrl.mac_binding_can_timestamp) { > - 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()); > - } > - } > -} > - > -/* 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,174 +4878,6 @@ 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; > - > -static void > -init_send_garps_rarps(void) > -{ > - shash_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 arp_nd_data { > struct hmap_node hmap_node; > @@ -5289,16 +5022,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,9 +5073,8 @@ 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) > { > if (current_time < garp_rarp->announce_time) { > return garp_rarp->announce_time; > @@ -6419,242 +6141,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 (!cmap_is_empty(garp_rarp_get_data())) { > poll_timer_wait_until(send_garp_rarp_time); > } > } > @@ -6672,18 +6164,18 @@ send_arp_nd_wait(long long int send_arp_nd_time) > /* Called with in the pinctrl_handler thread context. */ > 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)) { > + const struct cmap *send_garp_rarp_data = garp_rarp_get_data(); > + if (cmap_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; > + CMAP_FOR_EACH (garp, cmap_node, send_garp_rarp_data) { > + long long int next_announce = send_garp_rarp(swconn, garp, > current_time); > send_garp_rarp() modifies the underlying struct which creates race condition here. There are 2 options on how to address that, use cmap_replace() which would be pretty costly or we can convert the announce_time and backoff into atomic. > if (*send_garp_rarp_time > next_announce) { > *send_garp_rarp_time = next_announce; > @@ -6743,22 +6235,11 @@ 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, > - const struct sbrec_ecmp_nexthop_table > *ecmp_nh_table, > - const struct ovsrec_bridge *br_int, > +send_garp_rarp_prepare(const struct sbrec_ecmp_nexthop_table > *ecmp_nh_table, > 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 +6259,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_data_sync(reset_timers)) { > We might not need the sync call at all, the "garp_rarp_max_timeout" and "garp_rarp_continuous" should be actually part of the new node as they are taken from ovs table and we have a dependency on it anyway. So all of those timer resets could be handled during the node run. + /* 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 +6279,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 +6290,7 @@ static bool > may_inject_pkts(void) > { > return (!shash_is_empty(&ipv6_ras) || > - !shash_is_empty(&send_garp_rarp_data) || > + !cmap_is_empty(garp_rarp_get_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..4a9a56c8b 100644 > --- a/controller/pinctrl.h > +++ b/controller/pinctrl.h > @@ -44,7 +44,6 @@ 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, > @@ -56,7 +55,7 @@ void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > 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_chassis *chassis, > const struct hmap *local_datapaths, > const struct sset *active_tunnels, > const struct shash *local_active_ports_ipv6_pd, > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > index f89523502..5582e1b60 100644 > --- a/lib/ovn-util.c > +++ b/lib/ovn-util.c > @@ -21,6 +21,7 @@ > > #include "daemon.h" > #include "include/ovn/actions.h" > +#include "mac-binding-index.h" > #include "openvswitch/ofp-parse.h" > #include "openvswitch/rconn.h" > #include "openvswitch/vlog.h" > @@ -1422,3 +1423,45 @@ ovn_debug_commands_register(void) > unixctl_command_register("debug/enable-timewarp", "", 0, 0, > ovn_enable_timewarp, NULL); > } > + > +/* Update or add an IP-MAC binding for 'logical_port'. > + * Caller should make sure that 'ovnsb_idl_txn' is valid. */ > +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) > I'm slightly on edge if this actually shouldn't be in mac-binding-index.c/.h where we have other MAC binding DB related stuff. > +{ > + /* 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()); > + } > + } > +} > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > index 0fff9b463..c05f067eb 100644 > --- a/lib/ovn-util.h > +++ b/lib/ovn-util.h > @@ -496,4 +496,10 @@ const struct sbrec_port_binding *lport_lookup_by_name( > struct ovsdb_idl_index *sbrec_port_binding_by_name, > const char *name); > > +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); > + > #endif /* OVN_UTIL_H */ > -- > 2.43.0 > > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thanks, Ales _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
