After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 769015 - IPv4 Routes show incorrect metric
IPv4 Routes show incorrect metric
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: nm-connection-editor
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Beniamino Galvani
NetworkManager maintainer(s)
Depends on:
Blocks: nm-review
 
 
Reported: 2016-07-21 01:52 UTC by Scott Palmer
Modified: 2016-10-14 11:39 UTC
See Also:
GNOME target: ---
GNOME version: 3.19/3.20


Attachments
Screenshot showing the incorrect route metric values (292.87 KB, image/png)
2016-07-21 01:52 UTC, Scott Palmer
  Details
[PATCH] editor: consider -1 as missing route metric (13.15 KB, patch)
2016-09-28 07:57 UTC, Beniamino Galvani
none Details | Review
[PATCH] editor: factor out common tree model helpers (16.61 KB, patch)
2016-10-04 11:51 UTC, Beniamino Galvani
none Details | Review

Description Scott Palmer 2016-07-21 01:52:12 UTC
Created attachment 331851 [details]
Screenshot showing the incorrect route metric values

[scott@localhost ~]$ route -n
Kernel IP routing table
Destination     Gateway         Genmask         Flags Metric Ref    Use Iface
0.0.0.0         10.65.0.1       0.0.0.0         UG    1      0        0 vlan20
0.0.0.0         192.168.50.253  0.0.0.0         UG    2      0        0 vlan50
10.0.0.0        10.65.0.1       255.0.0.0       UG    1      0        0 vlan20
10.65.0.0       0.0.0.0         255.255.255.0   U     1      0        0 vlan20
10.65.226.15    10.65.0.1       255.255.255.255 UGH   1      0        0 vlan20
172.16.0.0      192.168.50.253  255.255.0.0     UG    2      0        0 vlan50
192.168.0.0     192.168.50.253  255.255.0.0     UG    2      0        0 vlan50
192.168.50.0    0.0.0.0         255.255.255.0   U     2      0        0 vlan50


The metric shown in the GUI is 4294967295 for the following routes:
10.0.0.0        10.65.0.1       255.0.0.0       UG    1      0        0 vlan20
172.16.0.0      192.168.50.253  255.255.0.0     UG    2      0        0 vlan50
192.168.0.0     192.168.50.253  255.255.0.0     UG    2      0        0 vlan50


[scott@localhost ~]$ nmcli --version
nmcli tool, version 1.2.2-2.fc24


The network interfaces were created as follows:

# Create the Houle Domain interface
nmcli con add type vlan ifname vlan20 con-name "Houle Domain" autoconnect no dev enp0s25 id 20
nmcli con mod "Houle Domain" +ipv4.routes "10.0.0.0/8 10.65.0.1"
nmcli con mod "Houle Domain" ipv4.route-metric 1
nmcli con mod "Houle Domain" ipv6.method ignore
nmcli con mod "Houle Domain" connection.autoconnect yes

# Create the Houle Security interface
nmcli con add type vlan ifname vlan50 con-name "Houle Security" autoconnect no dev enp0s25 id 50
nmcli con mod "Houle Security" +ipv4.routes "192.168.0.0/16 192.168.50.253"
nmcli con mod "Houle Security" +ipv4.routes "172.16.0.0/16 192.168.50.253"
nmcli con mod "Houle Security" ipv4.route-metric 2
nmcli con mod "Houle Security" ipv6.method ignore
nmcli con mod "Houle Security" connection.autoconnect yes
Comment 1 Thomas Haller 2016-08-18 13:41:57 UTC
the UI looks like gnome-control-center, not nm-connection-editor. Reassigning.
Comment 2 Rui Matos 2016-08-18 17:34:18 UTC
g-c-c doesn't support vlans, this really is nm-connection-editor, which was spawned from g-c-c in the past for certain connection types like vlans.
Comment 3 Thomas Haller 2016-08-18 20:41:28 UTC
ah right... thanks Rui.

The metric of these routes is "-1", which means "fallback to the per-device metric" (which means use the value "ipv4.route-metric", or if that is -1 too, it depends on the device-type, like 100 for ethernet).

Anyway, the editor of nm-c-e currently does not support the special value -1. Needs fixing.
Comment 5 Beniamino Galvani 2016-09-28 07:57:28 UTC
Created attachment 336418 [details] [review]
[PATCH] editor: consider -1 as missing route metric
Comment 6 Thomas Haller 2016-10-03 19:16:17 UTC
(In reply to Beniamino Galvani from comment #5)
> Created attachment 336418 [details] [review] [review]
> [PATCH] editor: consider -1 as missing route metric

could we move get_one_metric()/get_one_int64() to a common place and re-use it?

maybe use _nm_utils_ascii_str_to_int64() instead of "long long" and strtoll().
Comment 7 Beniamino Galvani 2016-10-04 11:51:37 UTC
Created attachment 336895 [details] [review]
[PATCH] editor: factor out common tree model helpers

(In reply to Thomas Haller from comment #6)
> (In reply to Beniamino Galvani from comment #5)
> > Created attachment 336418 [details] [review] [review] [review]
> > [PATCH] editor: consider -1 as missing route metric
> 
> could we move get_one_metric()/get_one_int64() to a common place and re-use
> it?

How about this additional patch?

> maybe use _nm_utils_ascii_str_to_int64() instead of "long long" and
> strtoll().

In order to use _nm_utils_ascii_str_to_int64() to implement a get_one_int64() we need to define a fallback value outsize of the input range to detect errors. I think it's simpler to stick with current code.
Comment 8 Thomas Haller 2016-10-04 12:31:19 UTC
(In reply to Beniamino Galvani from comment #7)
> Created attachment 336895 [details] [review] [review]
> > maybe use _nm_utils_ascii_str_to_int64() instead of "long long" and
> > strtoll().
> 
> In order to use _nm_utils_ascii_str_to_int64() to implement a
> get_one_int64() we need to define a fallback value outsize of the input
> range to detect errors. I think it's simpler to stick with current code.

Check errno:

   val = _nm_utils_ascii_str_to_int64 (str, 10, G_MININT64, G_MAXINT64, default);
   if (errno)
       return FALSE;
   ...

(you don't have to initialize errno to 0. The function is guaranteed to always set errno).
Comment 9 Beniamino Galvani 2016-10-05 16:43:25 UTC
(In reply to Thomas Haller from comment #8)
> (In reply to Beniamino Galvani from comment #7)
> > Created attachment 336895 [details] [review] [review] [review]
> > > maybe use _nm_utils_ascii_str_to_int64() instead of "long long" and
> > > strtoll().
> > 
> > In order to use _nm_utils_ascii_str_to_int64() to implement a
> > get_one_int64() we need to define a fallback value outsize of the input
> > range to detect errors. I think it's simpler to stick with current code.
> 
> Check errno:
> 
>    val = _nm_utils_ascii_str_to_int64 (str, 10, G_MININT64, G_MAXINT64,
> default);
>    if (errno)
>        return FALSE;
>    ...
> 
> (you don't have to initialize errno to 0. The function is guaranteed to
> always set errno).

Ah, I missed that, thanks. Pushed branch bg/editor-route-metric-default-bgo769015.
Comment 10 Thomas Haller 2016-10-05 17:32:26 UTC
lgtm