Merge pull request #18589 from yuwata/network-nexthop-drop-unnecessary-nexthops

network: drop unnecessary nexthops

src/libsystemd/sd-netlink/rtnl-message.c

@@ -329,6 +329,8 @@ int sd_rtnl_message_nexthop_get_family(const sd_netlink_message *m, uint8_t *fam
 
         assert_return(m, -EINVAL);
         assert_return(m->hdr, -EINVAL);
+        assert_return(rtnl_message_type_is_nexthop(m->hdr->nlmsg_type), -EINVAL);
+        assert_return(family, -EINVAL);
 
         nhm = NLMSG_DATA(m->hdr);
         *family = nhm->nh_family;
@@ -336,6 +338,20 @@ int sd_rtnl_message_nexthop_get_family(const sd_netlink_message *m, uint8_t *fam
         return 0;
 }
 
+int sd_rtnl_message_nexthop_get_protocol(const sd_netlink_message *m, uint8_t *protocol) {
+        struct nhmsg *nhm;
+
+        assert_return(m, -EINVAL);
+        assert_return(m->hdr, -EINVAL);
+        assert_return(rtnl_message_type_is_nexthop(m->hdr->nlmsg_type), -EINVAL);
+        assert_return(protocol, -EINVAL);
+
+        nhm = NLMSG_DATA(m->hdr);
+        *protocol = nhm->nh_protocol;
+
+        return 0;
+}
+
 int sd_rtnl_message_neigh_set_flags(sd_netlink_message *m, uint8_t flags) {
         struct ndmsg *ndm;
 

src/network/networkd-link.c

@@ -1980,6 +1980,10 @@ static int link_drop_foreign_config(Link *link) {
         if (k < 0 && r >= 0)
                 r = k;
 
+        k = link_drop_foreign_nexthops(link);
+        if (k < 0 && r >= 0)
+                r = k;
+
         k = manager_drop_foreign_routing_policy_rules(link->manager);
         if (k < 0 && r >= 0)
                 r = k;
@@ -2003,6 +2007,10 @@ static int link_drop_config(Link *link) {
         if (k < 0 && r >= 0)
                 r = k;
 
+        k = link_drop_nexthops(link);
+        if (k < 0 && r >= 0)
+                r = k;
+
         k = manager_drop_routing_policy_rules(link->manager, link);
         if (k < 0 && r >= 0)
                 r = k;

src/network/networkd-nexthop.c

@@ -102,6 +102,7 @@ static int nexthop_new_static(Network *network, const char *filename, unsigned s
 static void nexthop_hash_func(const NextHop *nexthop, struct siphash *state) {
         assert(nexthop);
 
+        siphash24_compress(&nexthop->protocol, sizeof(nexthop->protocol), state);
         siphash24_compress(&nexthop->id, sizeof(nexthop->id), state);
         siphash24_compress(&nexthop->blackhole, sizeof(nexthop->blackhole), state);
         siphash24_compress(&nexthop->family, sizeof(nexthop->family), state);
@@ -121,6 +122,10 @@ static void nexthop_hash_func(const NextHop *nexthop, struct siphash *state) {
 static int nexthop_compare_func(const NextHop *a, const NextHop *b) {
         int r;
 
+        r = CMP(a->protocol, b->protocol);
+        if (r != 0)
+                return r;
+
         r = CMP(a->id, b->id);
         if (r != 0)
                 return r;
@@ -146,12 +151,23 @@ DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(
                 nexthop_compare_func,
                 nexthop_free);
 
+static bool nexthop_equal(const NextHop *a, const NextHop *b) {
+        if (a == b)
+                return true;
+
+        if (!a || !b)
+                return false;
+
+        return nexthop_compare_func(a, b) == 0;
+}
+
 static void nexthop_copy(NextHop *dest, const NextHop *src) {
         assert(dest);
         assert(src);
 
         /* This only copies entries used in the above hash and compare functions. */
 
+        dest->protocol = src->protocol;
         dest->id = src->id;
         dest->blackhole = src->blackhole;
         dest->family = src->family;
@@ -339,6 +355,56 @@ static void log_nexthop_debug(const NextHop *nexthop, uint32_t id, const char *s
         }
 }
 
+static int nexthop_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
+        int r;
+
+        assert(m);
+
+        /* Note that link may be NULL. */
+        if (link && IN_SET(link->state, LINK_STATE_FAILED, LINK_STATE_LINGER))
+                return 1;
+
+        r = sd_netlink_message_get_errno(m);
+        if (r < 0 && r != -ENOENT)
+                log_link_message_warning_errno(link, m, r, "Could not drop nexthop, ignoring");
+
+        return 1;
+}
+
+static int nexthop_remove(const NextHop *nexthop, Manager *manager, Link *link) {
+        _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL;
+        int r;
+
+        assert(nexthop);
+        assert(manager);
+
+        /* link may be NULL. */
+
+        if (nexthop->id == 0) {
+                log_link_debug(link, "Cannot remove nexthop without valid ID, ignoring.");
+                return 0;
+        }
+
+        log_nexthop_debug(nexthop, nexthop->id, "Removing", link);
+
+        r = sd_rtnl_message_new_nexthop(manager->rtnl, &req, RTM_DELNEXTHOP, AF_UNSPEC, RTPROT_UNSPEC);
+        if (r < 0)
+                return log_link_error_errno(link, r, "Could not create RTM_DELNEXTHOP message: %m");
+
+        r = sd_netlink_message_append_u32(req, NHA_ID, nexthop->id);
+        if (r < 0)
+                return log_link_error_errno(link, r, "Could not append NHA_ID attribute: %m");
+
+        r = netlink_call_async(manager->rtnl, NULL, req, nexthop_remove_handler,
+                               link_netlink_destroy_callback, link);
+        if (r < 0)
+                return log_link_error_errno(link, r, "Could not send rtnetlink message: %m");
+
+        link_ref(link); /* link may be NULL, link_ref() is OK with that */
+
+        return 0;
+}
+
 static int nexthop_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
         int r;
 
@@ -472,6 +538,124 @@ int link_set_nexthops(Link *link) {
         return 0;
 }
 
+static bool link_has_nexthop(const Link *link, const NextHop *nexthop) {
+        NextHop *net_nexthop;
+
+        assert(link);
+        assert(nexthop);
+
+        if (!link->network)
+                return false;
+
+        HASHMAP_FOREACH(net_nexthop, link->network->nexthops_by_section)
+                if (nexthop_equal(net_nexthop, nexthop))
+                        return true;
+
+        return false;
+}
+
+static bool links_have_nexthop(const Manager *manager, const NextHop *nexthop, const Link *except) {
+        Link *link;
+
+        assert(manager);
+
+        HASHMAP_FOREACH(link, manager->links) {
+                if (link == except)
+                        continue;
+
+                if (link_has_nexthop(link, nexthop))
+                        return true;
+        }
+
+        return false;
+}
+
+static int manager_drop_nexthops_internal(Manager *manager, bool foreign, const Link *except) {
+        NextHop *nexthop;
+        Set *nexthops;
+        int k, r = 0;
+
+        assert(manager);
+
+        nexthops = foreign ? manager->nexthops_foreign : manager->nexthops;
+        SET_FOREACH(nexthop, nexthops) {
+                /* do not touch nexthop created by the kernel */
+                if (nexthop->protocol == RTPROT_KERNEL)
+                        continue;
+
+                /* The nexthop will be configured later, or already configured by a link. */
+                if (links_have_nexthop(manager, nexthop, except))
+                        continue;
+
+                /* The existing links do not have the nexthop. Let's drop this now. It may be
+                 * re-configured later. */
+                k = nexthop_remove(nexthop, manager, NULL);
+                if (k < 0 && r >= 0)
+                        r = k;
+        }
+
+        return r;
+}
+
+static int manager_drop_foreign_nexthops(Manager *manager) {
+        return manager_drop_nexthops_internal(manager, true, NULL);
+}
+
+static int manager_drop_nexthops(Manager *manager, const Link *except) {
+        return manager_drop_nexthops_internal(manager, false, except);
+}
+
+int link_drop_foreign_nexthops(Link *link) {
+        NextHop *nexthop;
+        int k, r = 0;
+
+        assert(link);
+        assert(link->manager);
+
+        SET_FOREACH(nexthop, link->nexthops_foreign) {
+                /* do not touch nexthop created by the kernel */
+                if (nexthop->protocol == RTPROT_KERNEL)
+                        continue;
+
+                if (link_has_nexthop(link, nexthop))
+                        k = nexthop_add(link, nexthop, NULL);
+                else
+                        k = nexthop_remove(nexthop, link->manager, link);
+                if (k < 0 && r >= 0)
+                        r = k;
+        }
+
+        k = manager_drop_foreign_nexthops(link->manager);
+        if (k < 0 && r >= 0)
+                r = k;
+
+        return r;
+}
+
+int link_drop_nexthops(Link *link) {
+        NextHop *nexthop;
+        int k, r = 0;
+
+        assert(link);
+        assert(link->manager);
+
+        SET_FOREACH(nexthop, link->nexthops) {
+                /* do not touch nexthop created by the kernel */
+                if (nexthop->protocol == RTPROT_KERNEL)
+                        continue;
+
+                k = nexthop_remove(nexthop, link->manager, link);
+                if (k < 0 && r >= 0)
+                        r = k;
+        }
+
+        k = manager_drop_nexthops(link->manager, link);
+        if (k < 0 && r >= 0)
+                r = k;
+
+        return r;
+}
+
 int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, Manager *m) {
         _cleanup_(nexthop_freep) NextHop *tmp = NULL;
         NextHop *nexthop = NULL;
@@ -530,6 +714,12 @@ int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message,
         } else if (!IN_SET(tmp->family, AF_INET, AF_INET6))
                 return log_link_debug(link, "rtnl: received nexthop message with invalid family %d, ignoring.", tmp->family);
 
+        r = sd_rtnl_message_nexthop_get_protocol(message, &tmp->protocol);
+        if (r < 0) {
+                log_link_warning_errno(link, r, "rtnl: could not get nexthop protocol, ignoring: %m");
+                return 0;
+        }
+
         r = netlink_message_read_in_addr_union(message, NHA_GATEWAY, tmp->family, &tmp->gw);
         if (r < 0 && r != -ENODATA) {
                 log_link_warning_errno(link, r, "rtnl: could not get NHA_GATEWAY attribute, ignoring: %m");

src/network/networkd-nexthop.h

@@ -23,7 +23,7 @@ typedef struct NextHop {
         Manager *manager;
         Link *link;
 
-        unsigned char protocol;
+        uint8_t protocol;
 
         uint32_t id;
         bool blackhole;
@@ -37,6 +37,8 @@ NextHop *nexthop_free(NextHop *nexthop);
 void network_drop_invalid_nexthops(Network *network);
 
 int link_set_nexthops(Link *link);
+int link_drop_nexthops(Link *link);
+int link_drop_foreign_nexthops(Link *link);
 
 int manager_get_nexthop_by_id(Manager *manager, uint32_t id, NextHop **ret);
 int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, Manager *m);

src/systemd/sd-netlink.h

@@ -184,6 +184,7 @@ int sd_rtnl_message_route_get_type(const sd_netlink_message *m, unsigned char *t
 int sd_rtnl_message_new_nexthop(sd_netlink *rtnl, sd_netlink_message **ret, uint16_t nhmsg_type, int nh_family, unsigned char nh_protocol);
 int sd_rtnl_message_nexthop_set_flags(sd_netlink_message *m, uint8_t flags);
 int sd_rtnl_message_nexthop_get_family(const sd_netlink_message *m, uint8_t *family);
+int sd_rtnl_message_nexthop_get_protocol(const sd_netlink_message *m, uint8_t *protocol);
 
 int sd_rtnl_message_new_neigh(sd_netlink *nl, sd_netlink_message **ret, uint16_t msg_type, int index, int nda_family);
 int sd_rtnl_message_neigh_set_flags(sd_netlink_message *m, uint8_t flags);

test/test-network/conf/25-nexthop-nothing.network

@@ -0,0 +1,8 @@
+[Match]
+Name=veth99
+
+[Network]
+IPv6AcceptRA=no
+Address=2001:1234:5:8f63::1/120
+Address=192.168.5.10/24
+Gateway=192.168.5.1

test/test-network/systemd-networkd-tests.py

@@ -1801,6 +1801,7 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities):
         '25-neighbor-ipv6.network',
         '25-neighbor-ip-dummy.network',
         '25-neighbor-ip.network',
+        '25-nexthop-nothing.network',
         '25-nexthop.network',
         '25-qdisc-cake.network',
         '25-qdisc-clsact-and-htb.network',
@@ -2868,6 +2869,33 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities):
         print(output)
         self.assertEqual('blackhole 2001:1234:5:8f62::2 nhid 7 dev lo proto static metric 1024 pref medium', output)
 
+        remove_unit_from_networkd_path(['25-nexthop.network'])
+        copy_unit_to_networkd_unit_path('25-nexthop-nothing.network')
+        rc = call(*networkctl_cmd, 'reload', env=env)
+        self.assertEqual(rc, 0)
+        time.sleep(1)
+
+        output = check_output('ip nexthop list dev veth99')
+        print(output)
+        self.assertEqual(output, '')
+        output = check_output('ip nexthop list dev lo')
+        print(output)
+        self.assertEqual(output, '')
+
+        remove_unit_from_networkd_path(['25-nexthop-nothing.network'])
+        copy_unit_to_networkd_unit_path('25-nexthop.network')
+        rc = call(*networkctl_cmd, 'reload', env=env)
+        self.assertEqual(rc, 0)
+        time.sleep(1)
+
+        rc = call('ip link del veth99')
+        self.assertEqual(rc, 0)
+        time.sleep(2)
+
+        output = check_output('ip nexthop list dev lo')
+        print(output)
+        self.assertEqual(output, '')
+
     def test_qdisc(self):
         copy_unit_to_networkd_unit_path('25-qdisc-clsact-and-htb.network', '12-dummy.netdev',
                                         '25-qdisc-ingress-netem-compat.network', '11-dummy.netdev')