From 75d26fb3138a015a61919d9291f8791f519fdc0b Mon Sep 17 00:00:00 2001 From: sudhanshukumar22 Date: Thu, 12 Nov 2020 04:37:30 -0800 Subject: [PATCH] zebra: treat vrf add for existing vrf as update Description: When we get a new vrf add and vrf with same name, but different vrf-id already exists in the database, we should treat vrf add as update. This happens mostly when there are lots of vrf and other configuration being replayed. There may be a stale vrf delete followed by new vrf add. This can cause timing race condition where vrf delete could be missed and further same vrf add would get rejected instead of treating last arrived vrf add as update. Treat vrf add for existing vrf as update. Implicitly disable this VRF to cleanup routes and other functions as part of vrf disable. Update vrf_id for the vrf and update vrf_id tree. Re-enable VRF so that all routes are freshly installed. Above 3 steps are mandatory since it can happen that with config reload stale routes which are installed in vrf-1 table might contain routes from older vrf-0 table which might have got deleted due to missing vrf-0 in new configuration. Signed-off-by: sudhanshukumar22 --- lib/vrf.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++ lib/vrf.h | 1 + zebra/if_netlink.c | 8 ++------ 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/lib/vrf.c b/lib/vrf.c index 1a9cd7e451..07080daabf 100644 --- a/lib/vrf.c +++ b/lib/vrf.c @@ -214,6 +214,53 @@ struct vrf *vrf_get(vrf_id_t vrf_id, const char *name) return vrf; } +/* Update a VRF. If not found, create one. + * Arg: + * name - The name of the vrf. + * vrf_id - The vrf_id of the vrf. + * Description: This function first finds the vrf using its name. If the vrf is + * found and the vrf-id of the existing vrf does not match the new vrf id, it + * will disable the existing vrf and update it with new vrf-id. If the vrf is + * not found, it will create the vrf with given name and the new vrf id. + */ +struct vrf *vrf_update(vrf_id_t new_vrf_id, const char *name) +{ + struct vrf *vrf = NULL; + + /*Treat VRF add for existing vrf as update + * Update VRF ID and also update in VRF ID table + */ + if (name) + vrf = vrf_lookup_by_name(name); + if (vrf && new_vrf_id != VRF_UNKNOWN && vrf->vrf_id != VRF_UNKNOWN + && vrf->vrf_id != new_vrf_id) { + if (debug_vrf) { + zlog_debug( + "Vrf Update event: %s old id: %u, new id: %u", + name, vrf->vrf_id, new_vrf_id); + } + + /*Disable the vrf to simulate implicit delete + * so that all stale routes are deleted + * This vrf will be enabled down the line + */ + vrf_disable(vrf); + + + RB_REMOVE(vrf_id_head, &vrfs_by_id, vrf); + vrf->vrf_id = new_vrf_id; + RB_INSERT(vrf_id_head, &vrfs_by_id, vrf); + + } else { + + /* + * vrf_get is implied creation if it does not exist + */ + vrf = vrf_get(new_vrf_id, name); + } + return vrf; +} + /* Delete a VRF. This is called when the underlying VRF goes away, a * pre-configured VRF is deleted or when shutting down (vrf_terminate()). */ diff --git a/lib/vrf.h b/lib/vrf.h index c636b9ea7e..81473ae7e5 100644 --- a/lib/vrf.h +++ b/lib/vrf.h @@ -114,6 +114,7 @@ extern struct vrf_name_head vrfs_by_name; extern struct vrf *vrf_lookup_by_id(vrf_id_t); extern struct vrf *vrf_lookup_by_name(const char *); extern struct vrf *vrf_get(vrf_id_t, const char *); +extern struct vrf *vrf_update(vrf_id_t new_vrf_id, const char *name); extern const char *vrf_id_to_name(vrf_id_t vrf_id); extern vrf_id_t vrf_name_to_id(const char *); diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index e4dd745f42..00471b9645 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -301,7 +301,7 @@ static void netlink_vrf_change(struct nlmsghdr *h, struct rtattr *tb, struct ifinfomsg *ifi; struct rtattr *linkinfo[IFLA_INFO_MAX + 1]; struct rtattr *attr[IFLA_VRF_MAX + 1]; - struct vrf *vrf; + struct vrf *vrf = NULL; struct zebra_vrf *zvrf; uint32_t nl_table_id; @@ -350,11 +350,7 @@ static void netlink_vrf_change(struct nlmsghdr *h, struct rtattr *tb, } } - /* - * vrf_get is implied creation if it does not exist - */ - vrf = vrf_get((vrf_id_t)ifi->ifi_index, - name); // It would create vrf + vrf = vrf_update((vrf_id_t)ifi->ifi_index, name); if (!vrf) { flog_err(EC_LIB_INTERFACE, "VRF %s id %u not created", name, ifi->ifi_index); -- 2.39.5