diff --git a/doc/netplan-yaml.md b/doc/netplan-yaml.md index 2857e7eab..f4e0e94ec 100644 --- a/doc/netplan-yaml.md +++ b/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) diff --git a/src/nm.c b/src/nm.c index 3dbb85bfc..6c4e33480 100644 --- a/src/nm.c +++ b/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); diff --git a/src/validation.c b/src/validation.c index a0371a8fe..bbd45023e 100644 --- a/src/validation.c +++ b/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; diff --git a/tests/generator/test_routing.py b/tests/generator/test_routing.py index 704a59a26..2612d2e96 100644 --- a/tests/generator/test_routing.py +++ b/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): diff --git a/tests/generator/test_vrfs.py b/tests/generator/test_vrfs.py index 23d0966e6..529551334 100644 --- a/tests/generator/test_vrfs.py +++ b/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) diff --git a/tests/integration/routing.py b/tests/integration/routing.py index 6afd75c6d..2f8841eb0 100644 --- a/tests/integration/routing.py +++ b/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)