Skip to content

Commit

Permalink
staticd: fix nht memory leak
Browse files Browse the repository at this point in the history
When a static route with a gateway nexthop is created, the nexthop is
sent to zebra for NHT, and added to a local hash. When the nexthop's VRF
is deleted from kernel, nexthop still stays in the hash. This is a
memory leak, because it is never deleted from there. Even if the VRF is
recreated in kernel, it is assigned with a new `vrf_id` so the old hash
entry is not reused, and a new one is created. To fix the issue, remove
all nexthops from the hash when the corresponding VRF is deleted, do not
add nexthops to the hash if their corresponding VRF doesn't exist in
kernel and don't add the same nexthop to the hash multiple times.

Signed-off-by: Igor Ryzhov <[email protected]>
  • Loading branch information
idryzhov committed Feb 3, 2024
1 parent 352b2a6 commit 09bac97
Showing 1 changed file with 6 additions and 35 deletions.
41 changes: 6 additions & 35 deletions staticd/static_routes.c
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,6 @@ void zebra_stable_node_cleanup(struct route_table *table,
/* Install static path into rib. */
void static_install_path(struct static_path *pn)
{
struct static_nexthop *nh;

frr_each(static_nexthop_list, &pn->nexthop_list, nh)
static_zebra_nht_register(nh, true);

if (static_nexthop_list_count(&pn->nexthop_list))
static_zebra_route_add(pn, true);
}
Expand Down Expand Up @@ -492,7 +487,6 @@ static void static_fixup_vrf(struct vrf *vrf, struct route_table *stable,
continue;

nh->nh_vrf_id = vrf->vrf_id;
nh->nh_registered = false;
if (nh->ifname[0]) {
ifp = if_lookup_by_name(nh->ifname,
nh->nh_vrf_id);
Expand All @@ -502,7 +496,7 @@ static void static_fixup_vrf(struct vrf *vrf, struct route_table *stable,
continue;
}

static_install_path(pn);
static_install_nexthop(nh);
}
}
}
Expand All @@ -520,31 +514,15 @@ static void static_fixup_vrf(struct vrf *vrf, struct route_table *stable,
static void static_enable_vrf(struct route_table *stable, afi_t afi, safi_t safi)
{
struct route_node *rn;
struct static_nexthop *nh;
struct interface *ifp;
struct static_path *pn;
struct static_route_info *si;

for (rn = route_top(stable); rn; rn = route_next(rn)) {
si = static_route_info_from_rnode(rn);
if (!si)
continue;
frr_each(static_path_list, &si->path_list, pn) {
frr_each(static_nexthop_list, &pn->nexthop_list, nh) {
if (nh->nh_vrf_id == VRF_UNKNOWN)
continue;
if (nh->ifname[0]) {
ifp = if_lookup_by_name(nh->ifname,
nh->nh_vrf_id);
if (ifp)
nh->ifindex = ifp->ifindex;
else
continue;
}

static_install_path(pn);
}
}
frr_each(static_path_list, &si->path_list, pn)
static_install_path(pn);
}
}

Expand Down Expand Up @@ -606,7 +584,7 @@ static void static_cleanup_vrf(struct vrf *vrf, struct route_table *stable,
if (strcmp(vrf->name, nh->nh_vrfname) != 0)
continue;

static_uninstall_path(pn);
static_uninstall_nexthop(nh);

nh->nh_vrf_id = VRF_UNKNOWN;
nh->ifindex = IFINDEX_INTERNAL;
Expand All @@ -627,22 +605,15 @@ static void static_disable_vrf(struct route_table *stable,
afi_t afi, safi_t safi)
{
struct route_node *rn;
struct static_nexthop *nh;
struct static_path *pn;
struct static_route_info *si;

for (rn = route_top(stable); rn; rn = route_next(rn)) {
si = static_route_info_from_rnode(rn);
if (!si)
continue;
frr_each(static_path_list, &si->path_list, pn) {
frr_each(static_nexthop_list, &pn->nexthop_list, nh) {
if (nh->nh_vrf_id == VRF_UNKNOWN)
continue;

static_uninstall_path(pn);
}
}
frr_each(static_path_list, &si->path_list, pn)
static_uninstall_path(pn);
}
}

Expand Down

0 comments on commit 09bac97

Please sign in to comment.