Fri, May 07, 2021 at 03:31:18PM CEST, olichtne(a)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(a)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