On Thu, Nov 14, 2024 at 11:47 AM Max Lamprecht via dev < [email protected]> wrote:
> This patch moves the nat_addresses calculation to an own engine node. > > Previously the nat_address calculation was executed at every pinctrl_run > (send_garp_rarp_prepare). This can cause high cpu usage and delayed > actions on gateway-chassis(500 LRPs+). > > perf trace: > - 98.83% main > - 79.69% pinctrl_run > - 77.71% send_garp_rarp_prepare > - 55.49% get_nat_addresses_and_keys > - 36.76% consider_nat_address > - 18.13% lport_lookup_by_name > - 13.58% sset_destroy > > Co-authored-by: Ihtisham ul Haq <[email protected]> > Signed-off-by: Ihtisham ul Haq <[email protected]> > Signed-off-by: Max Lamprecht <[email protected]> > --- > Hello Max, thank you for your patience and sorry for the delay in reviews. I have a couple of comments down below. controller/ovn-controller.c | 341 +++++++++++++++++++++++++++++++- > controller/pinctrl.c | 381 ++++++------------------------------ > controller/pinctrl.h | 14 +- > 3 files changed, 410 insertions(+), 326 deletions(-) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 98b144699..49bc6d86c 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -3359,6 +3359,319 @@ en_dns_cache_cleanup(void *data OVS_UNUSED) > ovn_dns_cache_destroy(); > } > > +static void * > +en_nat_addresses_init(struct engine_node *node OVS_UNUSED, > + struct engine_arg *arg OVS_UNUSED) > +{ > + struct ed_type_nat_addresses *data = xzalloc(sizeof *data); > nit: No need to zero alloc if you init everything right away. > + shash_init(&data->nat_addresses); > + sset_init(&data->localnet_vifs); > + sset_init(&data->local_l3gw_ports); > + sset_init(&data->nat_ip_keys); > + return data; > +} > + > +/* 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); > + } > + } > + } > +} > + > +/* 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); > +} > + > +static void > +en_nat_addresses_run(struct engine_node *node, void *data OVS_UNUSED) > +{ > + struct ed_type_nat_addresses *nat_addresses = 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); > + 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); > + > + struct ovsdb_idl_index *sbrec_pb_by_datapath = > + engine_ovsdb_node_get_index( > + engine_get_input("SB_port_binding", node), > + "datapath"); > + struct ovsdb_idl_index *sbrec_pb_by_name = > + engine_ovsdb_node_get_index( > + engine_get_input("SB_port_binding", node), > + "name"); > + const struct ovsrec_bridge_table *bridge_table = > + EN_OVSDB_GET(engine_get_input("OVS_bridge", node)); > + const struct ovsrec_bridge *br_int = get_br_int(bridge_table, > ovs_table); > + > + struct ed_type_runtime_data *rt_data = > + engine_get_input_data("runtime_data", node); > + const struct hmap *local_datapaths = &rt_data->local_datapaths; > + const struct sset *active_tunnels = &rt_data->active_tunnels; > + > + struct shash_node *nat_node; > + SHASH_FOR_EACH (nat_node, &nat_addresses->nat_addresses) { > + struct lport_addresses *laddrs = nat_node->data; > + destroy_lport_addresses(laddrs); > + free(laddrs); > + } > + shash_clear(&nat_addresses->nat_addresses); > + sset_clear(&nat_addresses->localnet_vifs); > + sset_clear(&nat_addresses->local_l3gw_ports); > + sset_clear(&nat_addresses->nat_ip_keys); > + > + get_localnet_vifs_l3gwports(sbrec_pb_by_datapath, > + sbrec_pb_by_name, > + br_int, chassis, local_datapaths, > + &nat_addresses->localnet_vifs, > + &nat_addresses->local_l3gw_ports); > + > + get_nat_addresses_and_keys(sbrec_pb_by_name, > + &nat_addresses->nat_ip_keys, > + &nat_addresses->local_l3gw_ports, > + chassis, active_tunnels, > + &nat_addresses->nat_addresses); > + > + engine_set_node_state(node, EN_UPDATED); > +} > + > +static bool > +nat_addresses_change_handler(struct engine_node *node, void *data) > +{ > + en_nat_addresses_run(node, data); > + return true; > +} > There is no need for separate handler, engine will always run the "<NODE>_run()" function if handler is NULL. + > +static void > +en_nat_addresses_cleanup(void *data OVS_UNUSED){ > + struct ed_type_nat_addresses *nat_addresses = data; > + shash_destroy(&nat_addresses->nat_addresses); > We are leaking all the "struct lport_addresses" stored within this shash. > + sset_destroy(&nat_addresses->localnet_vifs); > + sset_destroy(&nat_addresses->local_l3gw_ports); > + sset_destroy(&nat_addresses->nat_ip_keys); > +} > > /* Engine node which is used to handle the Non VIF data like > * - OVS patch ports > @@ -4779,6 +5092,14 @@ controller_output_bfd_chassis_handler(struct > engine_node *node, > return true; > } > > +static bool > +controller_output_nat_addresses_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 > @@ -5093,6 +5414,7 @@ main(int argc, char *argv[]) > ENGINE_NODE(mac_cache, "mac_cache"); > ENGINE_NODE(bfd_chassis, "bfd_chassis"); > ENGINE_NODE(dns_cache, "dns_cache"); > + ENGINE_NODE(nat_addresses, "nat_addresses"); > > #define SB_NODE(NAME, NAME_STR) ENGINE_NODE_SB(NAME, NAME_STR); > SB_NODES > @@ -5137,6 +5459,14 @@ main(int argc, char *argv[]) > engine_add_input(&en_bfd_chassis, &en_sb_chassis, NULL); > engine_add_input(&en_bfd_chassis, &en_sb_ha_chassis_group, NULL); > > + engine_add_input(&en_nat_addresses, &en_ovs_open_vswitch, NULL); > + engine_add_input(&en_nat_addresses, &en_ovs_bridge, NULL); > + engine_add_input(&en_nat_addresses, &en_sb_chassis, NULL); > + engine_add_input(&en_nat_addresses, &en_sb_port_binding, > + nat_addresses_change_handler); > + engine_add_input(&en_nat_addresses, &en_runtime_data, > + nat_addresses_change_handler); > + > /* Note: The order of inputs is important, all OVS interface changes > must > * be handled before any ct_zone changes. > */ > @@ -5300,6 +5630,8 @@ main(int argc, char *argv[]) > controller_output_mac_cache_handler); > engine_add_input(&en_controller_output, &en_bfd_chassis, > controller_output_bfd_chassis_handler); > + engine_add_input(&en_controller_output, &en_nat_addresses, > + controller_output_nat_addresses_handler); > > struct engine_arg engine_arg = { > .sb_idl = ovnsb_idl_loop.idl, > @@ -5346,6 +5678,8 @@ main(int argc, char *argv[]) > engine_get_internal_data(&en_ct_zones); > struct ed_type_bfd_chassis *bfd_chassis_data = > engine_get_internal_data(&en_bfd_chassis); > + struct ed_type_nat_addresses *nat_addresses_data = > + engine_get_internal_data(&en_nat_addresses); > struct ed_type_runtime_data *runtime_data = > engine_get_internal_data(&en_runtime_data); > struct ed_type_template_vars *template_vars_data = > @@ -5682,6 +6016,7 @@ main(int argc, char *argv[]) > } > > runtime_data = engine_get_data(&en_runtime_data); > + nat_addresses_data = > engine_get_data(&en_nat_addresses); > if (runtime_data) { > stopwatch_start(PATCH_RUN_STOPWATCH_NAME, > time_msec()); > patch_run(ovs_idl_txn, > @@ -5729,7 +6064,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, > @@ -5743,13 +6077,14 @@ main(int argc, char *argv[]) > sbrec_mac_binding_table_get( > ovnsb_idl_loop.idl), > > sbrec_bfd_table_get(ovnsb_idl_loop.idl), > - br_int, chassis, > + chassis, > &runtime_data->local_datapaths, > &runtime_data->active_tunnels, > > &runtime_data->local_active_ports_ipv6_pd, > &runtime_data->local_active_ports_ras, > ovsrec_open_vswitch_table_get( > - ovs_idl_loop.idl)); > + ovs_idl_loop.idl), > + nat_addresses_data); > stopwatch_stop(PINCTRL_RUN_STOPWATCH_NAME, > time_msec()); > mirror_run(ovs_idl_txn, > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index 3fb7e2fd7..e8c4ff77b 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -227,14 +227,11 @@ static void destroy_send_garps_rarps(void); > static void send_garp_rarp_wait(long long int send_garp_rarp_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 ovsrec_bridge *, > - const struct sbrec_chassis *, > const struct hmap *local_datapaths, > - const struct sset *active_tunnels, > - const struct ovsrec_open_vswitch_table *ovs_table) > + const struct ovsrec_open_vswitch_table *ovs_table, > + struct ed_type_nat_addresses *nat_addresses) > OVS_REQUIRES(pinctrl_mutex); > static void send_garp_rarp_run(struct rconn *swconn, > long long int *send_garp_rarp_time) > @@ -4008,7 +4005,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, > @@ -4019,13 +4015,13 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > const struct sbrec_service_monitor_table *svc_mon_table, > const struct sbrec_mac_binding_table *mac_binding_table, > const struct sbrec_bfd_table *bfd_table, > - const struct ovsrec_bridge *br_int, > const struct sbrec_chassis *chassis, > const struct hmap *local_datapaths, > const struct sset *active_tunnels, > const struct shash *local_active_ports_ipv6_pd, > const struct shash *local_active_ports_ras, > - const struct ovsrec_open_vswitch_table *ovs_table) > + const struct ovsrec_open_vswitch_table *ovs_table, > + struct ed_type_nat_addresses *nat_addresses) > { > ovs_mutex_lock(&pinctrl_mutex); > run_put_mac_bindings(ovnsb_idl_txn, sbrec_datapath_binding_by_key, > @@ -4033,10 +4029,11 @@ 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, > + send_garp_rarp_prepare(ovnsb_idl_txn, > sbrec_port_binding_by_name, > - sbrec_mac_binding_by_lport_ip, br_int, chassis, > - local_datapaths, active_tunnels, ovs_table); > + sbrec_mac_binding_by_lport_ip, > + local_datapaths, > + ovs_table, nat_addresses); > 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, > @@ -6175,236 +6172,6 @@ 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) > { > @@ -6441,96 +6208,70 @@ send_garp_rarp_run(struct rconn *swconn, long long > int *send_garp_rarp_time) > * 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 ovsrec_bridge *br_int, > - const struct sbrec_chassis *chassis, > const struct hmap *local_datapaths, > - const struct sset *active_tunnels, > - const struct ovsrec_open_vswitch_table *ovs_table) > + const struct ovsrec_open_vswitch_table *ovs_table, > + struct ed_type_nat_addresses *nat_addresses) > 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; > - bool garp_continuous = false; > - const struct ovsrec_open_vswitch *cfg = > - ovsrec_open_vswitch_table_first(ovs_table); > - if (cfg) { > - garp_max_timeout = smap_get_ullong( > - &cfg->external_ids, "garp-max-timeout-sec", 0) * 1000; > - garp_continuous = !!garp_max_timeout; > - if (!garp_max_timeout) { > - garp_max_timeout = GARP_RARP_DEF_MAX_TIMEOUT; > - } > - } > - > - shash_init(&nat_addresses); > - > - 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); > + if (nat_addresses) { > + unsigned long long garp_max_timeout = GARP_RARP_DEF_MAX_TIMEOUT; > + bool garp_continuous = false; > + > + const struct ovsrec_open_vswitch *cfg = > + ovsrec_open_vswitch_table_first(ovs_table); > + if (cfg) { > + garp_max_timeout = smap_get_ullong( > + &cfg->external_ids, "garp-max-timeout-sec", 0) * 1000; > + garp_continuous = !!garp_max_timeout; > + if (!garp_max_timeout) { > + garp_max_timeout = GARP_RARP_DEF_MAX_TIMEOUT; > + } > } > - } > - > - /* 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); > + /* 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(&nat_addresses->localnet_vifs, iter->name) > && > + !sset_contains(&nat_addresses->nat_ip_keys, iter->name)) { > + send_garp_rarp_delete(iter->name); > + } > } > - } > - > - /* 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); > + /* Update send_garp_rarp_data. */ > + const char *iface_id; > + SSET_FOR_EACH (iface_id, &nat_addresses->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->nat_addresses, > + garp_max_timeout, garp_continuous); > + } > } > - } > > - /* pinctrl_handler thread will send the GARPs. */ > - > - sset_destroy(&localnet_vifs); > - sset_destroy(&local_l3gw_ports); > + /* Update send_garp_rarp_data for nat-addresses. */ > + const char *gw_port; > + SSET_FOR_EACH (gw_port, &nat_addresses->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->nat_addresses, > + garp_max_timeout, garp_continuous); > + } > + } > > - SHASH_FOR_EACH_SAFE (iter, &nat_addresses) { > - struct lport_addresses *laddrs = iter->data; > - destroy_lport_addresses(laddrs); > - shash_delete(&nat_addresses, iter); > - free(laddrs); > + /* pinctrl_handler thread will send the GARPs. */ > + garp_rarp_max_timeout = garp_max_timeout; > + garp_rarp_continuous = garp_continuous; > } > - shash_destroy(&nat_addresses); > - > - sset_destroy(&nat_ip_keys); > - > - garp_rarp_max_timeout = garp_max_timeout; > - garp_rarp_continuous = garp_continuous; > } > > static bool > diff --git a/controller/pinctrl.h b/controller/pinctrl.h > index 846afe0a4..c6a7423b8 100644 > --- a/controller/pinctrl.h > +++ b/controller/pinctrl.h > @@ -20,6 +20,7 @@ > #include <stdint.h> > > #include "lib/sset.h" > +#include "openvswitch/shash.h" > #include "openvswitch/list.h" > #include "openvswitch/meta-flow.h" > > @@ -39,10 +40,16 @@ struct sbrec_bfd_table; > struct sbrec_port_binding; > struct sbrec_mac_binding_table; > > +struct ed_type_nat_addresses { > + struct shash nat_addresses; > + struct sset localnet_vifs; > + struct sset local_l3gw_ports; > + struct sset nat_ip_keys; > +}; > This shouldn't be part of pinctrl.h as it doesn't have anything to do with pinctrl. In fact all functions that are handling the NAT addresses should be in a separate module. + > 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, > @@ -53,12 +60,13 @@ void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > const struct sbrec_service_monitor_table *, > const struct sbrec_mac_binding_table *, > const struct sbrec_bfd_table *, > - const struct ovsrec_bridge *, const struct sbrec_chassis > *, > + const struct sbrec_chassis *, > const struct hmap *local_datapaths, > const struct sset *active_tunnels, > const struct shash *local_active_ports_ipv6_pd, > const struct shash *local_active_ports_ras, > - const struct ovsrec_open_vswitch_table *ovs_table); > + const struct ovsrec_open_vswitch_table *ovs_table, > + struct ed_type_nat_addresses *nat_addresses); > void pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn); > void pinctrl_destroy(void); > > -- > 2.43.0 > > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thanks, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
