From 33fc8bc7f30646fa7a4cee324d26e6979aa60400 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 14 Oct 2021 21:06:38 +0300 Subject: [PATCH 1/9] bfdd: cleanup bfd_session_enable Well, there are some weird and duplicated checks there... All we need is two simple checks: - VRF existence. We must have it to enable the session. - Interface existence. If it's configured for the session, we have to bind the session to the interface. This commit implements these checks and removes unnecessary duplication. Signed-off-by: Igor Ryzhov --- bfdd/bfd.c | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/bfdd/bfd.c b/bfdd/bfd.c index c66fccb853..a4091534f5 100644 --- a/bfdd/bfd.c +++ b/bfdd/bfd.c @@ -315,45 +315,28 @@ int bfd_session_enable(struct bfd_session *bs) vrf = vrf_lookup_by_name(bs->key.vrfname); if (vrf == NULL) { zlog_err( - "session-enable: specified VRF doesn't exists."); + "session-enable: specified VRF %s doesn't exists.", + bs->key.vrfname); return 0; } + } else { + vrf = vrf_lookup_by_id(VRF_DEFAULT); } - if (!vrf_is_backend_netns() && vrf && vrf->vrf_id != VRF_DEFAULT - && !if_lookup_by_name(vrf->name, vrf->vrf_id)) { - zlog_err("session-enable: vrf interface %s not available yet", - vrf->name); - return 0; - } + assert(vrf); if (bs->key.ifname[0]) { - if (vrf) - ifp = if_lookup_by_name(bs->key.ifname, vrf->vrf_id); - else - ifp = if_lookup_by_name_all_vrf(bs->key.ifname); + ifp = if_lookup_by_name(bs->key.ifname, vrf->vrf_id); if (ifp == NULL) { zlog_err( "session-enable: specified interface %s (VRF %s) doesn't exist.", - bs->key.ifname, vrf ? vrf->name : ""); + bs->key.ifname, vrf->name); return 0; } - if (bs->key.ifname[0] && !vrf) { - vrf = vrf_lookup_by_id(ifp->vrf_id); - if (vrf == NULL) { - zlog_err( - "session-enable: specified VRF %u doesn't exist.", - ifp->vrf_id); - return 0; - } - } } /* Assign interface/VRF pointers. */ bs->vrf = vrf; - if (bs->vrf == NULL) - bs->vrf = vrf_lookup_by_id(VRF_DEFAULT); - assert(bs->vrf); /* Assign interface pointer (if any). */ bs->ifp = ifp; From de4f1a66fbdd368baab4ba3a517d253e5ec6da1e Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 14 Oct 2021 21:06:38 +0300 Subject: [PATCH 2/9] bgpd: don't use if_lookup_by_name_all_vrf if_lookup_by_name_all_vrf doesn't work correctly with netns VRF backend as the same index may be used in multiple netns simultaneously. Use the appropriate VRF when looking for the interface. Signed-off-by: Igor Ryzhov --- bgpd/bgp_routemap.c | 4 ++-- bgpd/bgp_zebra.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 2f37367a8f..12a5e1d696 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -1688,10 +1688,10 @@ route_match_interface(void *rule, const struct prefix *prefix, void *object) path = object; - if (!path) + if (!path || !path->peer || !path->peer->bgp) return RMAP_NOMATCH; - ifp = if_lookup_by_name_all_vrf((char *)rule); + ifp = if_lookup_by_name((char *)rule, path->peer->bgp->vrf_id); if (ifp == NULL || ifp->ifindex != path->attr->nh_ifindex) return RMAP_NOMATCH; diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 0249d53f02..0546c63869 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -3506,7 +3506,7 @@ void bgp_zebra_announce_default(struct bgp *bgp, struct nexthop *nh, /* create default route with interface * with nexthop-vrf */ - ifp = if_lookup_by_name_all_vrf(vrf->name); + ifp = if_lookup_by_name_vrf(vrf->name, vrf); if (!ifp) return; api_nh->vrf_id = nh->vrf_id; From 5d8694aee7cc8bf1c30ac890b12ea507c50a7e37 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 14 Oct 2021 21:06:38 +0300 Subject: [PATCH 3/9] lib: remove wrong setting of interface configured flag The fact that the interface name is used in some nexthop config doesn't mean that the interface is configured. Signed-off-by: Igor Ryzhov --- lib/nexthop_group.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c index 97d70189ff..af9cd2d79a 100644 --- a/lib/nexthop_group.c +++ b/lib/nexthop_group.c @@ -953,12 +953,6 @@ DEFPY(ecmp_nexthops, ecmp_nexthops_cmd, nhg_hooks.add_nexthop(nhgc, nh); } - if (intf) { - struct interface *ifp = if_lookup_by_name_all_vrf(intf); - - if (ifp) - ifp->configured = true; - } return CMD_SUCCESS; } @@ -1265,7 +1259,6 @@ void nexthop_group_interface_state_change(struct interface *ifp, if (ifp->ifindex != nhop.ifindex) continue; - ifp->configured = true; nh = nexthop_new(); memcpy(nh, &nhop, sizeof(nhop)); From 4ff97453d0c5064da279c76e0602fdaf75f2a675 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 14 Oct 2021 21:06:38 +0300 Subject: [PATCH 4/9] pbrd: fix "set nexthop" for netns With netns VRF backend, we may have multiple interfaces with the same name. Currently, the function is not deterministic in this case as it uses the first interface that it finds in the list. Be more restrictive and ask the user to provide the VRF name. Signed-off-by: Igor Ryzhov --- pbrd/pbr_vty.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/pbrd/pbr_vty.c b/pbrd/pbr_vty.c index d083b9d2b0..bccbf9eaf1 100644 --- a/pbrd/pbr_vty.c +++ b/pbrd/pbr_vty.c @@ -485,14 +485,37 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd, nhop.vrf_id = vrf->vrf_id; if (intf) { - struct interface *ifp; + struct interface *ifp = NULL; + struct interface *ifptmp; + struct vrf *vrftmp; + int count = 0; + + if (vrf_is_backend_netns() && vrf_name) { + ifp = if_lookup_by_name_vrf(intf, vrf); + } else { + RB_FOREACH (vrftmp, vrf_name_head, &vrfs_by_name) { + ifptmp = if_lookup_by_name_vrf(intf, vrftmp); + if (ifptmp) { + ifp = ifptmp; + count++; + if (!vrf_is_backend_netns()) + break; + } + } + } - ifp = if_lookup_by_name_all_vrf(intf); if (!ifp) { vty_out(vty, "Specified Intf %s does not exist\n", intf); return CMD_WARNING_CONFIG_FAILED; } + if (count > 1) { + vty_out(vty, + "Specified Intf %s exists in multiple VRFs\n", + intf); + vty_out(vty, "You must specify the nexthop-vrf\n"); + return CMD_WARNING_CONFIG_FAILED; + } if (ifp->vrf_id != vrf->vrf_id) { struct vrf *actual; From 31eab818f69a3b4d47b9819e6c10f24c14e134b2 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 14 Oct 2021 21:06:38 +0300 Subject: [PATCH 5/9] zebra: fix "show interface IFNAME" for netns With netns VRF backend, we may have multiple interfaces with the same name. Currently, the function output is not deterministic in this case, it returns the first interface that it finds in the list. Be more explicit and tell the user that we need the VRF name. Signed-off-by: Igor Ryzhov --- zebra/interface.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/zebra/interface.c b/zebra/interface.c index a68d00d55c..b9605e138a 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -2485,12 +2485,24 @@ DEFPY (show_interface_name_vrf_all, VRF_ALL_CMD_HELP_STR JSON_STR) { - struct interface *ifp; + struct interface *ifp = NULL; + struct interface *ifptmp; + struct vrf *vrf; json_object *json = NULL; + int count = 0; interface_update_stats(); - ifp = if_lookup_by_name_all_vrf(ifname); + RB_FOREACH (vrf, vrf_name_head, &vrfs_by_name) { + ifptmp = if_lookup_by_name_vrf(ifname, vrf); + if (ifptmp) { + ifp = ifptmp; + count++; + if (!vrf_is_backend_netns()) + break; + } + } + if (ifp == NULL) { if (uj) vty_out(vty, "{}\n"); @@ -2498,6 +2510,17 @@ DEFPY (show_interface_name_vrf_all, vty_out(vty, "%% Can't find interface %s\n", ifname); return CMD_WARNING; } + if (count > 1) { + if (uj) { + vty_out(vty, "{}\n"); + } else { + vty_out(vty, + "%% There are multiple interfaces with name %s\n", + ifname); + vty_out(vty, "%% You must specify the VRF name\n"); + } + return CMD_WARNING; + } if (uj) json = json_object_new_object(); From 198ef12aefac2ccc746064c39de5c61cd4dcd68f Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 14 Oct 2021 21:06:38 +0300 Subject: [PATCH 6/9] ospf6d: don't use if_lookup_by_name_all_vrf if_lookup_by_name_all_vrf doesn't work correctly with netns VRF backend as the same index may be used in multiple netns simultaneously. Use the appropriate VRF when looking for the interface. Signed-off-by: Igor Ryzhov --- ospf6d/ospf6_asbr.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ospf6d/ospf6_asbr.c b/ospf6d/ospf6_asbr.c index cd2791fc48..3629942f17 100644 --- a/ospf6d/ospf6_asbr.c +++ b/ospf6d/ospf6_asbr.c @@ -2042,10 +2042,12 @@ ospf6_routemap_rule_match_interface(void *rule, const struct prefix *prefix, void *object) { struct interface *ifp; + struct ospf6_route *route; struct ospf6_external_info *ei; - ei = ((struct ospf6_route *)object)->route_option; - ifp = if_lookup_by_name_all_vrf((char *)rule); + route = object; + ei = route->route_option; + ifp = if_lookup_by_name((char *)rule, route->ospf6->vrf_id); if (ifp != NULL && ei->ifindex == ifp->ifindex) return RMAP_MATCH; From 4030e1867bd8598b17408b199ea9ba358611f69e Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 14 Oct 2021 21:06:38 +0300 Subject: [PATCH 7/9] ospfd: don't use if_lookup_by_name_all_vrf if_lookup_by_name_all_vrf doesn't work correctly with netns VRF backend as the same index may be used in multiple netns simultaneously. Use the appropriate VRF when looking for the interface. Signed-off-by: Igor Ryzhov --- ospfd/ospf_asbr.c | 5 +++-- ospfd/ospf_asbr.h | 5 ++++- ospfd/ospf_routemap.c | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/ospfd/ospf_asbr.c b/ospfd/ospf_asbr.c index 53d2ec538c..982fad63ec 100644 --- a/ospfd/ospf_asbr.c +++ b/ospfd/ospf_asbr.c @@ -72,12 +72,13 @@ void ospf_external_route_remove(struct ospf *ospf, struct prefix_ipv4 *p) } /* Add an External info for AS-external-LSA. */ -struct external_info *ospf_external_info_new(uint8_t type, +struct external_info *ospf_external_info_new(struct ospf *ospf, uint8_t type, unsigned short instance) { struct external_info *new; new = XCALLOC(MTYPE_OSPF_EXTERNAL_INFO, sizeof(struct external_info)); + new->ospf = ospf; new->type = type; new->instance = instance; new->to_be_processed = 0; @@ -138,7 +139,7 @@ ospf_external_info_add(struct ospf *ospf, uint8_t type, unsigned short instance, } /* Create new External info instance. */ - new = ospf_external_info_new(type, instance); + new = ospf_external_info_new(ospf, type, instance); new->p = p; new->ifindex = ifindex; new->nexthop = nexthop; diff --git a/ospfd/ospf_asbr.h b/ospfd/ospf_asbr.h index d3e50903ef..160883144f 100644 --- a/ospfd/ospf_asbr.h +++ b/ospfd/ospf_asbr.h @@ -29,6 +29,8 @@ struct route_map_set_values { /* Redistributed external information. */ struct external_info { + struct ospf *ospf; + /* Type of source protocol. */ uint8_t type; @@ -107,7 +109,8 @@ struct ospf_external_aggr_rt { #define OSPF_ASBR_NSSA_REDIST_UPDATE_DELAY 9 extern void ospf_external_route_remove(struct ospf *, struct prefix_ipv4 *); -extern struct external_info *ospf_external_info_new(uint8_t, unsigned short); +extern struct external_info *ospf_external_info_new(struct ospf *, uint8_t, + unsigned short); extern void ospf_reset_route_map_set_values(struct route_map_set_values *); extern int ospf_route_map_set_compare(struct route_map_set_values *, struct route_map_set_values *); diff --git a/ospfd/ospf_routemap.c b/ospfd/ospf_routemap.c index 2525c1cf3a..b1216626c4 100644 --- a/ospfd/ospf_routemap.c +++ b/ospfd/ospf_routemap.c @@ -320,7 +320,7 @@ route_match_interface(void *rule, const struct prefix *prefix, void *object) struct external_info *ei; ei = object; - ifp = if_lookup_by_name_all_vrf((char *)rule); + ifp = if_lookup_by_name((char *)rule, ei->ospf->vrf_id); if (ifp == NULL || ifp->ifindex != ei->ifindex) return RMAP_NOMATCH; From f13fdc673c1a51df0cda0952da390f4d10a0ecd7 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 14 Oct 2021 21:06:38 +0300 Subject: [PATCH 8/9] zebra: fix ptm message processing When PTM sends a "cbl status" message it specifies the interface name but not the VRF name. It is fine for VRF-lite, but doesn't work for netns because it's possible to have multiple interfaces with the same name. Be more restrictive in this case and return an error instead of randomly using of the interface with the specified name. Signed-off-by: Igor Ryzhov --- zebra/zebra_ptm.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/zebra/zebra_ptm.c b/zebra/zebra_ptm.c index e17465b112..ec68f585e3 100644 --- a/zebra/zebra_ptm.c +++ b/zebra/zebra_ptm.c @@ -609,7 +609,17 @@ static int zebra_ptm_handle_msg_cb(void *arg, void *in_ctxt) } if (strcmp(ZEBRA_PTM_INVALID_PORT_NAME, port_str)) { - ifp = if_lookup_by_name_all_vrf(port_str); + struct vrf *vrf; + int count = 0; + + RB_FOREACH (vrf, vrf_id_head, &vrfs_by_id) { + ifp = if_lookup_by_name_vrf(ifname, vrf); + if (ifp) { + count++; + if (!vrf_is_backend_netns()) + break; + } + } if (!ifp) { flog_warn(EC_ZEBRA_UNKNOWN_INTERFACE, @@ -617,6 +627,12 @@ static int zebra_ptm_handle_msg_cb(void *arg, void *in_ctxt) __func__, port_str); return -1; } + if (count > 1) { + flog_warn(EC_ZEBRA_UNKNOWN_INTERFACE, + "%s: multiple interface with name %s", + __func__, port_str); + return -1; + } } ptm_lib_find_key_in_msg(in_ctxt, ZEBRA_PTM_CBL_STR, cbl_str); From 0df2e1888b1e8011bc1e3ec6ed6c34a9e6fcaae9 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 14 Oct 2021 21:06:38 +0300 Subject: [PATCH 9/9] lib: make if_lookup_by_name_all_vrf internal This function doesn't work correctly with netns VRF backend as the same ifname may be used in multiple netns simultaneously. So let's hide it from the public API to reduce temptation to use it instead of writing the correct code. Signed-off-by: Igor Ryzhov --- lib/if.c | 2 +- lib/if.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/if.c b/lib/if.c index 4d7862022c..446b40fb46 100644 --- a/lib/if.c +++ b/lib/if.c @@ -431,7 +431,7 @@ struct interface *if_lookup_by_name_vrf(const char *name, struct vrf *vrf) return RB_FIND(if_name_head, &vrf->ifaces_by_name, &if_tmp); } -struct interface *if_lookup_by_name_all_vrf(const char *name) +static struct interface *if_lookup_by_name_all_vrf(const char *name) { struct vrf *vrf; struct interface *ifp; diff --git a/lib/if.h b/lib/if.h index 59e75d8b68..60d571b54e 100644 --- a/lib/if.h +++ b/lib/if.h @@ -529,7 +529,6 @@ size_t if_lookup_by_hwaddr(const uint8_t *hw_addr, size_t addrsz, struct interface ***result, vrf_id_t vrf_id); struct vrf; -extern struct interface *if_lookup_by_name_all_vrf(const char *ifname); extern struct interface *if_lookup_by_name_vrf(const char *name, struct vrf *vrf); extern struct interface *if_lookup_by_name(const char *ifname, vrf_id_t vrf_id); extern struct interface *if_get_by_name(const char *ifname, vrf_id_t vrf_id);