Skip to content

Commit

Permalink
nl_l3: only route l3 neighs if we have a route for them
Browse files Browse the repository at this point in the history
Make sure we do not add host entries for l3 neighs for which we do not
have a route until we a route gets added, and drop the host entries
again when the route gets removed.

To do this, we need to start keeping track of all l3 neighs we currently
handle. To make this easy, use a set, and reuse the nh_stub to store the
neighbor information. Due to IPv6 LL addresses being a bit more
complicated, and should never routable anyway, do not track them.

To allow nh_stub to be used in a set, we need to make it comparable, so
implement the < operator for it.

Using this we can then determine the routability of a l3 neigh we add by
asking the kernel for the route to it, then checking if it has a nexthop
that exist on one of our interfaces.

Only if this is true, add a host route for it and add it to the set of
routed l3 neighbours, in all other cases add it to the set of unrouted
l3 neighbours.

Then when adding a route, add all currently unrouted l3 neighbors
matching this route, and conversely, when removing a route remove all
matching l3 neighbors that are currently routed.

Fixes l3 neighbours learned on an additional on-link staying reachable
after removing the on-link route.

Signed-off-by: Jonas Gorski <jonas.gorski@bisdn.de>
  • Loading branch information
KanjiMonster committed Mar 10, 2022
1 parent f7d92dd commit 4719213
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 0 deletions.
109 changes: 109 additions & 0 deletions src/netlink/nl_l3.cc
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,7 @@ int nl_l3::add_l3_neigh(struct rtnl_neigh *n) {
return -EINVAL;

addr = rtnl_neigh_get_dst(n);

if (family == AF_INET6) {
auto p = nl_addr_alloc(16);
nl_addr_parse("fe80::/10", AF_INET6, &p);
Expand All @@ -729,6 +730,18 @@ int nl_l3::add_l3_neigh(struct rtnl_neigh *n) {
}

if (add_host_entry) {
// unless we have route, we should not route l3 neighbors
struct nh_stub nh {
addr, rtnl_neigh_get_ifindex(n)
};

if (is_l3_neigh_routable(n)) {
routable_l3_neighs.emplace(nh);
} else {
unroutable_l3_neighs.emplace(nh);
return -ENETUNREACH;
}

rv = add_l3_neigh_egress(n, &l3_interface_id, &vrf_id);
LOG(INFO) << __FUNCTION__ << ": adding l3 neigh egress for neigh "
<< OBJ_CAST(n);
Expand Down Expand Up @@ -827,6 +840,14 @@ int nl_l3::update_l3_neigh(struct rtnl_neigh *n_old, struct rtnl_neigh *n_new) {
VLOG(1) << __FUNCTION__ << ": neighbour ll changed: new neighbor "
<< n_ll_new << " ifindex=" << ifindex;

struct nh_stub nh {
rtnl_neigh_get_dst(n_old), ifindex
};
if (unroutable_l3_neighs.count(nh) > 0) {
VLOG(1) << __FUNCTION__ << ": ignoring update on unroutable neighbor";
return -EINVAL;
}

auto link = nl->get_link_by_ifindex(ifindex);

if (link.get() == nullptr) {
Expand Down Expand Up @@ -923,6 +944,17 @@ int nl_l3::del_l3_neigh(struct rtnl_neigh *n) {
if (!link)
return -EINVAL;

struct nh_stub nh {
addr, rtnl_neigh_get_ifindex(n)
};
if (unroutable_l3_neighs.erase(nh) > 0) {
VLOG(2) << __FUNCTION__ << ": l3 neigh was disabled, nothing to do for "
<< OBJ_CAST(n);
return 0;
}

routable_l3_neighs.erase(nh);

std::deque<rtnl_addr *> link_addresses;
get_l3_addrs(link.get(), &link_addresses);
for (auto i : link_addresses) {
Expand Down Expand Up @@ -1808,6 +1840,25 @@ int nl_l3::add_l3_unicast_route(rtnl_route *r, bool update_route) {
<< " rv=" << rv;
}

VLOG(2) << __FUNCTION__ << ": enabling l3 neighs reachable by route "
<< OBJ_CAST(r);
auto l3_neighs =
get_l3_neighs_of_prefix(unroutable_l3_neighs, rtnl_route_get_dst(r));
for (auto n : l3_neighs) {
auto neigh = nl->get_neighbour(n.ifindex, n.nh);
if (!neigh) {
// should we remove it from unroutable?
VLOG(2) << __FUNCTION__ << ": unknown l3 neigh ifindex=" << n.ifindex
<< ", addr=" << n.nh;
continue;
}

VLOG(2) << __FUNCTION__ << ": enabling l3 neigh " << OBJ_CAST(neigh);
unroutable_l3_neighs.erase(n);
add_l3_neigh(neigh);
rtnl_neigh_put(neigh);
}

// cleanup
for (auto n : neighs)
rtnl_neigh_put(n);
Expand Down Expand Up @@ -1844,6 +1895,27 @@ int nl_l3::del_l3_unicast_route(rtnl_route *r, bool keep_route) {
}
}

VLOG(2) << __FUNCTION__ << ": disabling l3 neighs reachable by route "
<< OBJ_CAST(r);
auto l3_neighs =
get_l3_neighs_of_prefix(routable_l3_neighs, rtnl_route_get_dst(r));
for (auto n : l3_neighs) {
auto neigh = nl->get_neighbour(n.ifindex, n.nh);
if (!neigh) {
// neigh is already purged from nl cache, so neigh will be
// removed when handling the DEL_NEIGH notification
VLOG(2) << __FUNCTION__
<< ": skipping non-existing l3 neigh ifindex=" << n.ifindex
<< ", addr=" << n.nh;
continue;
}

VLOG(2) << __FUNCTION__ << ": disabling l3 neigh " << OBJ_CAST(neigh);
del_l3_neigh(neigh);
rtnl_neigh_put(neigh);
unroutable_l3_neighs.emplace(n);
}

std::deque<struct rtnl_neigh *> neighs;
std::deque<nh_stub> unresolved_nh;
get_neighbours_of_route(r, &neighs, &unresolved_nh);
Expand Down Expand Up @@ -1934,4 +2006,41 @@ int nl_l3::del_l3_unicast_route(rtnl_route *r, bool keep_route) {
return rv;
}

bool nl_l3::is_l3_neigh_routable(struct rtnl_neigh *n) {
bool routable = false;
nl_route_query rq;

auto route = rq.query_route(rtnl_neigh_get_dst(n));
if (route) {
VLOG(2) << __FUNCTION__ << ": got route " << OBJ_CAST(route)
<< " for neigh " << OBJ_CAST(n);
if (rtnl_route_get_nnexthops(route) > 0) {
std::deque<struct rtnl_nexthop *> nhs;
get_nexthops_of_route(route, &nhs);
if (nhs.size() > 0) {
VLOG(2) << __FUNCTION__ << ": neigh is routable in by us";
routable = true;
}
}
nl_object_put(OBJ_CAST(route));
} else {
VLOG(2) << __FUNCTION__ << ": no route for neigh " << OBJ_CAST(n);
}

return routable;
};

std::deque<nh_stub> nl_l3::get_l3_neighs_of_prefix(std::set<nh_stub> &list,
nl_addr *prefix) {
std::deque<nh_stub> ret;

auto l3_neighs =
std::equal_range(list.begin(), list.end(), prefix, l3_prefix_comp);
std::deque<nh_stub> nhs;
for (auto n = l3_neighs.first; n != l3_neighs.second; ++n) {
ret.push_back(*n);
}
return ret;
}

} // namespace basebox
21 changes: 21 additions & 0 deletions src/netlink/nl_l3.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ class nl_vlan;
class nl_bridge;
class switch_interface;

class l3_prefix_comp final {
public:
bool operator()(const nh_stub &nh, const struct nl_addr *addr) const {
return nl_addr_cmp_prefix(nh.nh, addr) < 0;
}

bool operator()(const struct nl_addr *addr, const nh_stub &nh) const {
return nl_addr_cmp_prefix(addr, nh.nh) < 0;
}
};

class nl_l3 : public nh_reachable, public nh_unreachable {
public:
nl_l3(std::shared_ptr<nl_vlan> vlan, cnetlink *nl);
Expand Down Expand Up @@ -133,12 +144,22 @@ class nl_l3 : public nh_reachable, public nh_unreachable {
return !nl_addr_cmp_prefix(mc_addr.get(), addr);
}

bool is_l3_neigh_routable(struct rtnl_neigh *n);

std::deque<nh_stub> get_l3_neighs_of_prefix(std::set<nh_stub> &list,
nl_addr *prefix);

switch_interface *sw;
std::shared_ptr<nl_vlan> vlan;
cnetlink *nl;
std::list<std::pair<net_reachable *, net_params>> net_callbacks;
std::list<std::pair<nh_reachable *, nh_params>> nh_callbacks;
std::list<std::pair<nh_unreachable *, nh_params>> nh_unreach_callbacks;

std::set<nh_stub> routable_l3_neighs;
std::set<nh_stub> unroutable_l3_neighs;
struct l3_prefix_comp l3_prefix_comp;

const uint8_t MAIN_ROUTING_TABLE = 254;
};

Expand Down
7 changes: 7 additions & 0 deletions src/netlink/nl_l3_interfaces.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ struct nh_stub {
nl_addr_put(nh);
}

bool operator<(const nh_stub& other) const {
if (nl_addr_cmp(nh, other.nh) < 0)
return true;

return ifindex < other.ifindex;
}

nl_addr *nh;
int ifindex;
};
Expand Down

0 comments on commit 4719213

Please sign in to comment.