networkmanager: add support for "routing-policy" (LP: #2086544)

doc/netplan-yaml.md

@@ -879,6 +879,9 @@ network:
     > Specify a priority for the routing policy rule, to influence the order
     > in which routing rules are processed. A higher number means lower
     > priority: rules are processed in order by increasing priority number.
+    > Specifying an explicit, unique, priority for each routing policy rule
+    > is strongly recommended and is mandatory on the `NetworkManager`
+    > back-end.
 
   - **`mark`** (scalar)
 

src/nm.c

@@ -254,6 +254,64 @@ write_routes_nm(const NetplanNetDefinition* def, GKeyFile *kf, gint family, GErr
             j++;
         }
     }
+
+    return TRUE;
+}
+
+STATIC gboolean
+write_ip_rules_nm(const NetplanNetDefinition* def, GKeyFile *kf, gint family, GError** error)
+{
+    const gchar* group = NULL;
+    gchar* tmp_key = NULL;
+    GString* tmp_val = NULL;
+
+    if (family == AF_INET)
+        group = "ipv4";
+    else if (family == AF_INET6)
+        group = "ipv6";
+    g_assert(group != NULL);
+
+    if (def->ip_rules != NULL) {
+        for (unsigned i = 0, j = 1; i < def->ip_rules->len; ++i) {
+            const NetplanIPRule *cur_rule = g_array_index(def->ip_rules, NetplanIPRule*, i);
+
+            if (cur_rule->family != family)
+                continue;
+
+            /* NetworkManager requires that priority be specified.  This is
+             * also in-line with the iproute2 guidance that "Each rule should
+             * have an explicitly set unique priority value"[1].
+             * [1]http://www.policyrouting.org/iproute2.doc.html#ss9.6.1 */
+            if (cur_rule->priority == NETPLAN_IP_RULE_PRIO_UNSPEC) {
+                g_set_error(error, NETPLAN_BACKEND_ERROR, NETPLAN_ERROR_UNSUPPORTED,
+                            "ERROR: %s: The priority setting is mandatory for NetworkManager routing-policy\n", def->id);
+                return FALSE;
+            }
+
+            tmp_key = g_strdup_printf("routing-rule%u", j);
+            tmp_val = g_string_sized_new(200);
+
+            g_string_printf(tmp_val, "priority %u", cur_rule->priority);
+
+            if (cur_rule->from)
+                g_string_append_printf(tmp_val, " from %s", cur_rule->from);
+            if (cur_rule->to)
+                g_string_append_printf(tmp_val, " to %s", cur_rule->to);
+            if (cur_rule->tos != NETPLAN_IP_RULE_TOS_UNSPEC)
+                g_string_append_printf(tmp_val, " tos %u", cur_rule->tos);
+            if (cur_rule->fwmark != NETPLAN_IP_RULE_FW_MARK_UNSPEC)
+                g_string_append_printf(tmp_val, " fwmark %u", cur_rule->fwmark);
+            if (cur_rule->table != NETPLAN_ROUTE_TABLE_UNSPEC)
+                g_string_append_printf(tmp_val, " table %u", cur_rule->table);
+
+            g_key_file_set_string(kf, group, tmp_key, tmp_val->str);
+            g_free(tmp_key);
+            g_string_free(tmp_val, TRUE);
+
+            j++;
+        }
+    }
+
     return TRUE;
 }
 
@@ -716,6 +774,8 @@ write_nm_conf_access_point(const NetplanNetDefinition* def, const char* rootdir,
             g_key_file_set_uint64(kf, "vrf", "table", def->vrf_table);
             if (!write_routes_nm(def, kf, AF_INET, error) || !write_routes_nm(def, kf, AF_INET6, error))
                 return FALSE;
+            if (!write_ip_rules_nm(def, kf, AF_INET, error) || !write_ip_rules_nm(def, kf, AF_INET6, error))
+                return FALSE;
         }
 
         if (def->type == NETPLAN_DEF_TYPE_VETH && def->veth_peer_link) {
@@ -871,6 +931,8 @@ write_nm_conf_access_point(const NetplanNetDefinition* def, const char* rootdir,
         write_search_domains(def, "ipv4", kf);
         if (!write_routes_nm(def, kf, AF_INET, error))
             return FALSE;
+        if (!write_ip_rules_nm(def, kf, AF_INET, error))
+            return FALSE;
     }
 
     if (!def->dhcp4_overrides.use_routes) {
@@ -917,6 +979,9 @@ write_nm_conf_access_point(const NetplanNetDefinition* def, const char* rootdir,
         if (!write_routes_nm(def, kf, AF_INET6, error))
             return FALSE;
 
+        if (!write_ip_rules_nm(def, kf, AF_INET6, error))
+            return FALSE;
+
         if (!def->dhcp6_overrides.use_routes) {
             g_key_file_set_boolean(kf, "ipv6", "ignore-auto-routes", TRUE);
             g_key_file_set_boolean(kf, "ipv6", "never-default", TRUE);

src/validation.c

@@ -523,10 +523,13 @@ adopt_and_validate_vrf_routes(__unused const NetplanParser *npp, GHashTable *net
         if (nd->ip_rules) {
             for (size_t i = 0; i < nd->ip_rules->len; i++) {
                 NetplanIPRule* r = g_array_index(nd->ip_rules, NetplanIPRule*, i);
+                if (r->priority == NETPLAN_IP_RULE_PRIO_UNSPEC) {
+                    g_warning("%s: No priority specified for routing-policy %zu", nd->id, i);
+                }
                 if (r->table == nd->vrf_table) {
                     g_debug("%s: Ignoring redundant routing-policy table %d (matches VRF table)", nd->id, r->table);
                     continue;
-                } else if (r->table != NETPLAN_ROUTE_TABLE_UNSPEC && r->table != nd->vrf_table) {
+                } else if (r->table != NETPLAN_ROUTE_TABLE_UNSPEC) {
                     g_set_error(error, NETPLAN_VALIDATION_ERROR, NETPLAN_ERROR_CONFIG_GENERIC,
                             "%s: VRF routing-policy table mismatch (%d != %d)", nd->id, nd->vrf_table, r->table);
                     return FALSE;

tests/generator/test_routing.py

@@ -1071,6 +1071,122 @@ def test_route_v6_default(self):
 address1=2001:dead:beef::2/64
 ip6-privacy=0
 route1=::/0,2001:beef:beef::1
+'''})
+
+    def test_ip_rule_missing_priority_fails_ipv4(self):
+        err = self.generate('''network:
+  version: 2
+  renderer: NetworkManager
+  ethernets:
+    engreen:
+      addresses: ["192.168.14.2/24"]
+      routing-policy:
+        - to: 10.10.10.0/24
+          table: 100
+          ''', expect_fail=True)
+        self.assertIn("ERROR: engreen: The priority setting is mandatory for NetworkManager routing-policy", err)
+
+    def test_ip_rule_missing_priority_fails_ipv6(self):
+        err = self.generate('''network:
+  version: 2
+  renderer: NetworkManager
+  ethernets:
+    engreen:
+      addresses: ["2001:FFfe::1/64"]
+      routing-policy:
+        - to: 2001:FFfe::1/64
+          table: 100
+          ''', expect_fail=True)
+        self.assertIn("ERROR: engreen: The priority setting is mandatory for NetworkManager routing-policy", err)
+
+    def test_ip_rule_table(self):
+        self.generate('''network:
+  version: 2
+  renderer: NetworkManager
+  ethernets:
+    engreen:
+      addresses: ["192.168.14.2/24"]
+      routing-policy:
+        - to: 10.10.10.0/24
+          table: 100
+          priority: 99
+          ''')
+
+        self.assert_nm({'engreen': '''[connection]
+id=netplan-engreen
+type=ethernet
+interface-name=engreen
+
+[ethernet]
+wake-on-lan=0
+
+[ipv4]
+method=manual
+address1=192.168.14.2/24
+routing-rule1=priority 99 to 10.10.10.0/24 table 100
+
+[ipv6]
+method=ignore
+'''})
+
+    def test_ip_rule_fwmark(self):
+        self.generate('''network:
+  version: 2
+  renderer: NetworkManager
+  ethernets:
+    engreen:
+      addresses: ["192.168.14.2/24"]
+      routing-policy:
+        - from: 10.10.10.0/24
+          mark: 50
+          priority: 99
+          ''')
+
+        self.assert_nm({'engreen': '''[connection]
+id=netplan-engreen
+type=ethernet
+interface-name=engreen
+
+[ethernet]
+wake-on-lan=0
+
+[ipv4]
+method=manual
+address1=192.168.14.2/24
+routing-rule1=priority 99 from 10.10.10.0/24 fwmark 50
+
+[ipv6]
+method=ignore
+'''})
+
+    def test_ip_rule_tos(self):
+        self.generate('''network:
+  version: 2
+  renderer: NetworkManager
+  ethernets:
+    engreen:
+      addresses: ["192.168.14.2/24"]
+      routing-policy:
+        - to: 10.10.10.0/24
+          type-of-service: 250
+          priority: 99
+          ''')
+
+        self.assert_nm({'engreen': '''[connection]
+id=netplan-engreen
+type=ethernet
+interface-name=engreen
+
+[ethernet]
+wake-on-lan=0
+
+[ipv4]
+method=manual
+address1=192.168.14.2/24
+routing-rule1=priority 99 to 10.10.10.0/24 tos 250
+
+[ipv6]
+method=ignore
 '''})
 
     def test_routes_mixed(self):

tests/generator/test_vrfs.py

@@ -37,7 +37,8 @@ def test_vrf_set_table(self):
       - to: default
         via: 1.2.3.4
       routing-policy:
-      - from: 2.3.4.5''')
+      - from: 2.3.4.5
+        priority: 99''')
 
         self.assert_nm({'eth0': '''[connection]
 id=netplan-eth0
@@ -66,6 +67,7 @@ def test_vrf_set_table(self):
 [ipv4]
 route1=0.0.0.0/0,1.2.3.4
 route1_options=table=1005
+routing-rule1=priority 99 from 2.3.4.5 table 1005
 method=link-local
 
 [ipv6]
@@ -224,3 +226,14 @@ def test_vrf_without_routes(self):
 Table=1005
 ''',
                               'vrf1005.netdev': ND_VRF % ('vrf1005', 1005)})
+
+    def test_vrf_routing_policy_missing_priority_nm(self):
+        err = self.generate('''network:
+  version: 2
+  renderer: NetworkManager
+  vrfs:
+    vrf1005:
+      table: 1005
+      routing-policy:
+      - from: 2.3.4.5''', expect_fail=True)
+        self.assertIn("ERROR: vrf1005: The priority setting is mandatory for NetworkManager routing-policy", err)

tests/integration/routing.py

@@ -317,6 +317,7 @@ def test_vrf_basic(self):
         via: 10.10.10.1
       routing-policy:
       - from: 10.10.10.42
+        priority: 99
 ''' % {'r': self.backend, 'ec': self.dev_e_client})
         self.generate_and_settle([self.dev_e_client])
         self.assert_iface_up(self.dev_e_client, ['inet 10.10.10.22', 'master vrf0'])  # wokeignore:rule=master
@@ -332,10 +333,8 @@ def test_vrf_basic(self):
         self.assertIn('11.11.11.0/24 via 10.10.10.2 dev {}'.format(self.dev_e_client), out)
 
         # verify routing policy was setup correctly to the VRF's table
-        # 'routing-policy' is not supported on NetworkManager
-        if self.backend == 'networkd':
-            out = subprocess.check_output(['ip', 'rule', 'show'], text=True)
-            self.assertIn('from 10.10.10.42 lookup 1000', out)
+        out = subprocess.check_output(['ip', 'rule', 'show'], text=True)
+        self.assertIn('from 10.10.10.42 lookup 1000', out)
 
     def test_route_advmss_v6(self):
         self.setup_eth(None)