Mon, May 10, 2021 at 12:25:27PM CEST, jtluka@redhat.com wrote:
Fri, May 07, 2021 at 03:31:18PM CEST, olichtne@redhat.com wrote:
On Tue, May 04, 2021 at 11:21:15AM +0200, Jan Tluka wrote:
The handler of RTM_NEWADDR incorrectly fetches the IFA_ADDRESS attribute to update the device address. This is not an issue for IPv6 or typical update of the device address. However it is a bug for the assignment of address with peer address included (typical for point to point networks), for example:
ip address add 192.168.100.1/24 peer 192.169.100.2 dev enp7s0
In this case, the IFA_ADDRESS attribute value is the peer address value and the device address is the value of IFA_LOCAL.
When the peer address is not specified, both IFA_LOCAL and IFA_ADDRESS contain the same value which is the reason this has been working without any issues previously.
The fix is to use the IFA_LOCAL attribute value to update the device address for IPv4 scenarios.
Signed-off-by: Jan Tluka jtluka@redhat.com
lnst/Devices/Device.py | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/lnst/Devices/Device.py b/lnst/Devices/Device.py index c31a018f..68b5f1af 100644 --- a/lnst/Devices/Device.py +++ b/lnst/Devices/Device.py @@ -25,7 +25,7 @@ from lnst.Common.ExecCmd import exec_cmd, ExecCmdFail from lnst.Common.DeviceError import DeviceError, DeviceDeleted, DeviceDisabled from lnst.Common.DeviceError import DeviceConfigError, DeviceConfigValueError from lnst.Common.DeviceError import DeviceFeatureNotSupported -from lnst.Common.IpAddress import ipaddress +from lnst.Common.IpAddress import ipaddress, AF_INET from lnst.Common.HWAddress import hwaddress
from pyroute2.netlink.rtnl import RTM_NEWLINK @@ -205,8 +205,23 @@ class Device(object, metaclass=DeviceMeta): if nl_msg['header']['type'] == RTM_NEWLINK: self._nl_msg = nl_msg elif nl_msg['header']['type'] == RTM_NEWADDR:
addr = ipaddress(nl_msg.get_attr('IFA_ADDRESS'),
flags=nl_msg.get_attr("IFA_FLAGS"))
if nl_msg['family'] == AF_INET:
"""
from if_addr.h:
/*
* Important comment:
* IFA_ADDRESS is prefix address, rather than local interface address.
* It makes no difference for normally configured broadcast interfaces,
* but for point-to-point IFA_ADDRESS is DESTINATION address,
* local address is supplied in IFA_LOCAL attribute.
*/
"""
addr = ipaddress(nl_msg.get_attr('IFA_LOCAL'),
flags=nl_msg.get_attr("IFA_FLAGS"))
else:
addr = ipaddress(nl_msg.get_attr('IFA_ADDRESS'),
flags=nl_msg.get_attr("IFA_FLAGS"))
Based on your explanation and the if_addr.h comment, shouldn't we **always** read the IFA_LOCAL attribute?
-Ondrej
Yes, we should. We should query IFA_ADDRESS only for IPv6 [1].
J.
[1] https://github.com/tgraf/libnl/blob/master/lib/route/addr.c#L259
FYI, I double checked this. The patch is correct.
For IPv4 we should read the IFA_LOCAL, but for IPv6 only the IFA_ADDRESS is populated:
$ ip -4 addr add dev lo 192.168.10.10/24
{'attrs': [('IFA_ADDRESS', '192.168.10.10'), ('IFA_LOCAL', '192.168.10.10'), ('IFA_LABEL', 'lo'), ('IFA_FLAGS', 128), [root@lnst-devel lnst]# ('IFA_CACHEINFO', {'ifa_preferred': 4294967295, 'ifa_valid': 4294967295, 'cstamp': 2507106, 'tstamp': 2507106})], 'event': 'RTM_NEWADDR', 'family': 2, 'flags': 128, 'header': {'error': None, 'flags': 0, 'length': 76, 'pid': 3993, 'sequence_number': 1620746196, 'stats': Stats(qsize=0, delta=0, delay=0), 'target': 'localhost', 'type': 20}, 'index': 1, 'prefixlen': 24, 'scope': 0}
$ ip -6 addr add dev lo fc00::6/64
{'attrs': [('IFA_ADDRESS', 'fc00::6'), ('IFA_CACHEINFO', {'ifa_preferred': 4294967295, 'ifa_valid': 4294967295, 'cstamp': 2475861, 'tstamp': 2475861}), ('IFA_FLAGS', 192)], 'event': 'RTM_NEWADDR', 'family': 10, 'flags': 192, 'header': {'error': None, 'flags': 0, 'length': 72, 'pid': 0, 'sequence_number': 0, 'stats': Stats(qsize=0, delta=0, delay=0), 'target': 'localhost', 'type': 20}, 'index': 1, 'prefixlen': 64, 'scope': 0}
-Jan