Bug 1436531 - [RFE] Support multiple routing tables to enable sourceroute.
Summary: [RFE] Support multiple routing tables to enable sourceroute.
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: NetworkManager
Version: 7.3
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: rc
: ---
Assignee: Thomas Haller
QA Contact: Desktop QE
Ioanna Gkioka
URL:
Whiteboard:
: 1456592 (view as bug list)
Depends On:
Blocks: 1477926 1467520 1470965 1491311
TreeView+ depends on / blocked
 
Reported: 2017-03-28 06:26 UTC by Edward Haas
Modified: 2018-06-06 09:37 UTC (History)
17 users (show)

Fixed In Version:
Doc Type: Release Note
Doc Text:
*NetworkManager* now supports multiple routing tables to enable source routing This update adds a new "table" attribute for IPv4 and IPv6 routes which can be configured manually by the user. For each manual static route, a routing table can be selected. As a result, configuring the table of a route has the effect of configuring the route in that table. Additionally, the default routing table of a connection profile can be configured via the new `ipv4.route-table` and `ipv6.route-table` settings for IPv4 and IPv6 respectively. These settings determine in which table the routes are placed, except manual routes that explicitly overwrite this setting.
Clone Of:
: 1491311 (view as bug list)
Environment:
Last Closed: 2018-04-10 13:22:08 UTC
Target Upstream Version:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2018:0778 0 None None None 2018-04-10 13:23:39 UTC

Description Edward Haas 2017-03-28 06:26:38 UTC
In a scenario where a host is connected to multiple networks, managing routings for each network may be required.

In its simplest form, for each network a default route can be set, allowing traffic from different source addresses to be forwarded by default to a different next hop. This is the current usage for RHV and the trigger for requesting such an option from NM.

The basic request of this RFE is to allow the user to define a gateway per ip address.

RHV is utilizing the multiple routing tables that linux provides, other solutions (like a VRF) utilize the network namespace.

Comment 2 Thomas Haller 2017-05-30 09:06:29 UTC
*** Bug 1456592 has been marked as a duplicate of this bug. ***

Comment 3 Thomas Haller 2017-09-19 13:32:18 UTC
The plan for the first step is (later we can expand beyond that):

 - no support for rules. You have to manage rules outside NM any way you want. 
   NM ignores them.

 - routes from DHCP/DHCP6/SLAAC are still only added to the "main" table. 
   The feature only matters for manually added IPv4 and IPv6 routes.

 - manually configured IPv4 and IPv6 routes can have a new optional 
   attribute "table $TABLEID". Obviously, configuring the table of a route
   has the effect of configuring the route in that table.

That's about it. In step 1, the only new thing will be the new table property, and it only works for manual routes.




One additional thing: currently NM only manages the main table and ignores all other tables.
NM syncs the routes it wants to configure with kernel (in the main table for the interface in question). The important part here is that "sync" means to remove all routes from the main table (for that interface) which it doesn't expect. NM does not only add routes, it also deletes existing routes.

When adding support to policy-routing, that sync behavior could be harmful for existing setups. Instead, NM also a new option ipv4.route-table-sync and ipv6.route-table-sync that can have 3 values:

  - "none": means NM will never delete any routes that it finds. It only adds 
    new routes.
  - "main" (default): NM will delete routes from the main table, but in all 
    other tables it will accept existing routes without deleting them.
  - "full": NM will sync all tables (except the local table #255).

Comment 4 Beniamino Galvani 2017-09-19 13:40:59 UTC
Sounds good to me.

Comment 5 Edward Haas 2017-09-19 15:08:08 UTC
Sounds good to me too, it covers what we do today with the exception of the table ID generation.
Currently VDSM is generating the table ID, perhaps NM can help with that as well.
(It uses the IP address 32 bits for the table ID)

Comment 6 Edward Haas 2017-09-19 15:11:09 UTC
Question: Have netns discussed? At some level, it may cover this need as well, but I'm not completely sure.

Comment 7 Thomas Haller 2017-09-19 17:42:59 UTC
(In reply to Edward Haas from comment #5)
> Sounds good to me too, it covers what we do today with the exception of the
> table ID generation.
> Currently VDSM is generating the table ID, perhaps NM can help with that as
> well.
> (It uses the IP address 32 bits for the table ID)

There isn't a "generation" of the table ID. You pick a number, that's it.
It's up to the user (you) to pick that number. Using the IPv4 address sounds like a good idea.

iproute2 also supports aliases from /etc/iproute2/rt_tables. I don't think NM should accept such names, because then the connection profile is no longer self contained. E.g. export + import on another host might result in a broken configuration, because the table name is missing.


(In reply to Edward Haas from comment #6)
> Question: Have netns discussed? At some level, it may cover this need as
> well, but I'm not completely sure.

Yes, but it's entirely independent from policy routing. Links, addresses and routes all life inside one netns, there is no overlap. NM manages addresses/routes per netns. If it supports multiple netns, it manages them entirely separately. Especially there is no direct relation with policy-routing.

For afnetns that might be slightly different, because here the addresses of one interface might life in separate afnetns.

Comment 8 Thomas Haller 2017-09-22 14:09:24 UTC
please review a first version of th/policy-routing-pt1-rh1436531

Comment 9 Thomas Haller 2017-09-25 16:46:01 UTC
(In reply to Edward Haas from comment #0)

> In its simplest form, for each network a default route can be set, allowing
> traffic from different source addresses to be forwarded by default to a
> different next hop. This is the current usage for RHV and the trigger for
> requesting such an option from NM.

Hm, the current branch adds support for routing tables for manually added routes.

Notably, NetworkManager does not treat the default-route like normal routes (which I think is a mistake, but done historically). Hence, the branch would not yet allow you to configure the default route in another table.

Do you explicitly require to configure the default route as well?

Comment 10 Beniamino Galvani 2017-09-26 07:41:39 UTC
> device: do full update_ext_ip_config() during merge-and-apply

This looks suspicious to me...

	if (commit) {
		ensure_con_ip4_config (self);
+               if (priv->queued_ip4_config_id)
+                       update_ext_ip_config (self, AF_INET, FALSE);
	}

When there is a queued ip4 configuration, ensure_con_ip4_config()
creates a new priv->con_ip4_config from the setting and then
update_ext_ip_config() will intersect it with the external
configuration, removing everything (because we have not committed the
configuration yet).


> libnm,cli: add IP setting "route-table-sync"

Please document somewhere what the default value is, for example here:

	 <varlistentry>
+          <term><varname>ipv4.route-table-sync</varname></term>
+        </varlistentry>
+        <varlistentry>


Pushed a trivial fix.

Comment 11 Thomas Haller 2017-09-26 09:27:19 UTC
(In reply to Beniamino Galvani from comment #10)
> > device: do full update_ext_ip_config() during merge-and-apply

addressed all (?) and repushed.

How about now?

Comment 12 Beniamino Galvani 2017-09-26 09:49:44 UTC
LGTM

Comment 13 Francesco Giudici 2017-09-26 14:52:18 UTC
+
+       /**
+        * NMSettingIPConfig:route-table-sync:
+        *
+        * The mode how to sync the routes. In general, when NetworkManager manages
+        * a device it will remove extraneous routes from the routing tables. The
+        * sync parameter specifies which tables are synced this way. Meaning, from
+        * which routing table NetworkManager will remove routes that it doesn't
+        * expect there. The default value can be overwritten via global configuration.
+        * If unspecified, the default value is 2 (main).

I think here would be great to have also briefly listed the available values that the property can get. Something like:

"A value of 1 (none) means that route tables will not be synced, 2 (main) means that only the main table will be synced, 3 (all) will sync all the route tables. A value of zero (or unspecified) means to use the default value, which is 2 (main) if not specified via global configuration" 


commit msg typo in:
"device: refactor update-ip-config for device"
[...]
"And also, I think it's an antipattern [two-->TO] have two mirroring functions"


Apart from this imperfections, branch looks great!

Comment 14 Thomas Haller 2017-09-26 18:11:37 UTC
thanks. Merged branch th/policy-routing-pt1-rh1436531 to master:

https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=e6292bcb2a1326dd2ff4f53933acb76b8724e6d9


What is still missing is support for configuring the default-route in a different table. I think that shall be done by accepting default-routes like regular routes in ipv4.routes:

  nmcli connection modify "$CON" +ipv4.routes 'default 192.168.5.1 table=55'


Currently the addition of the default route depends on ipv4.never-default, ipv4.gateway, and whatever comes via ipv4.method.
Specifying default routes in ipv4.routes would be in addition to those methods.

Comment 15 Thomas Haller 2017-09-28 17:22:56 UTC
Please review th/policy-routing-pt2-rh1436531.

It now configures also the routing table for DHCP, default-route, everything.

This breaks API/ABI on current master branch (unstable).

Comment 16 Beniamino Galvani 2017-09-29 15:39:47 UTC
(In reply to Thomas Haller from comment #15)
> Please review th/policy-routing-pt2-rh1436531.
> 
> It now configures also the routing table for DHCP, default-route, everything.

Looks good to me (pushed a trivial fixup).

Comment 17 Beniamino Galvani 2017-10-09 15:23:50 UTC
> platform: mark static nla_policy variables as const

-       err = nla_parse_nested (tb, IFLA_INET6_MAX, attr, policy);
+       err = nla_parse_nested (tb, IFLA_INET6_MAX, attr, (struct nla_policy *) policy);

The cast is not needed.


> core: refactor parsing resolve.conf

gboolean
+nm_utils_resolve_conf_parse (int addr_family,
+                             const char *rc_contents,
+                             GArray *nameservers,
+                             GPtrArray *dns_options)
+{
+       guint i;
+       gboolean changed = FALSE;
+       gs_free const char **lines;

gs_free const char **lines = NULL;


+       g_return_val_if_fail (   nameservers
+                             && (   (   addr_family == AF_INET
+                                     && g_array_get_element_size (nameservers) == sizeof (in_addr_t))
+                                 || (   addr_family == AF_INET6
+                                     && g_array_get_element_size (nameservers) == sizeof (struct in6_addr))), FALSE);

Maybe, for readability split the expression in:

       g_return_val_if_fail (nameservers);
       g_return_val_if_fail (... the rest ...);

The rest LGTM.

Comment 18 Thomas Haller 2017-10-09 15:45:00 UTC
(In reply to Beniamino Galvani from comment #17)

addresses all and repushed.

thanks!

Comment 19 Francesco Giudici 2017-10-09 17:23:46 UTC
lgtm

Comment 24 Vladimir Benes 2018-01-25 15:00:21 UTC
running in CI since now

Comment 27 errata-xmlrpc 2018-04-10 13:22:08 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2018:0778

Comment 28 Mai Ling 2018-06-06 09:37:35 UTC
(In reply to Thomas Haller from comment #3)
> The plan for the first step is (later we can expand beyond that):
> 
>  - no support for rules. You have to manage rules outside NM any way you
> want. 
>    NM ignores them.
[...]

What is the issue number to track progress on adding support for rules?
Having to keep one configuration in n-m and one outside breaks all the confort of using nmcli to configure everything.


Note You need to log in before you can comment on or make changes to this bug.