On Thu, Apr 24, 2025 at 01:02:36PM +0200, Ales Musil wrote: > 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.
Hi Ales, thanks for the review. > > 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()). Ah, thanks. > > > > +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. thanks fixed. > > + 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 thanks fixed both. > > > > +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. I just copied that from the other engine nodes. But it seems like a lot of noise. Maybe we should just pass in the chassis generally. But i would keep it for now, then they are all the same. > > + > > + 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. I changed them to 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. I moved this logic into the garp_rarp engine node. We still need a call here if data has changed so that we can wakeup the pinctrl thread. > > + /* 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. I'll move it there. Thanks a lot, Felix > > > > +{ > > + /* 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
