On Mon, May 10, 2021 at 05:19:07PM +0200, Jan Tluka wrote:
Mon, May 10, 2021 at 12:30:42PM CEST, jtluka(a)redhat.com wrote:
>Fri, May 07, 2021 at 03:50:15PM CEST, olichtne(a)redhat.com wrote:
>>On Tue, May 04, 2021 at 11:21:16AM +0200, Jan Tluka wrote:
>>> The addr parameter of the _ip_add_one() method can now take either a string
>>> specifying a device address or a tuple that includes both the device
address
>>> and peer address to support point-to-point networks.
>>>
>>> Signed-off-by: Jan Tluka <jtluka(a)redhat.com>
>>> ---
>>> lnst/Devices/Device.py | 29 ++++++++++++++++++++++-------
>>> 1 file changed, 22 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/lnst/Devices/Device.py b/lnst/Devices/Device.py
>>> index 68b5f1af..b98d9235 100644
>>> --- a/lnst/Devices/Device.py
>>> +++ b/lnst/Devices/Device.py
>>> @@ -569,15 +569,30 @@ class Device(object, metaclass=DeviceMeta):
>>> self._ip_addrs = []
>>>
>>> def _ip_add_one(self, addr):
>>> - ip = ipaddress(addr)
>>> + if type(addr) is tuple:
>>> + ip = ipaddress(addr[0])
>>> + peer = ipaddress(addr[1])
>>> + else:
>>> + ip = ipaddress(addr)
>>> + peer = None
>>> +
>>> if ip not in self.ips:
>>> - self._ipr_wrapper("addr", "add",
index=self.ifindex,
>>> - address=str(ip), mask=ip.prefixlen)
>>> + kwargs = dict(
>>> + index=self.ifindex,
>>> + local=str(ip),
>>> + address=str(ip),
>>
>>I think you have a copy paste typo here, both local and address are str(ip)
>>
>>-Ondrej
>>
>
>Good catch! Will fix that.
>
Actually no. This is not a typo.
In the next 'if peer' block this gets overwritten. I believe I did this
intentionally to coever both IPv4 and IPv6 configuration.
If you think I should rework this a bit so that this is more readable,
I'll do that.
J.
Ah, yeah... I'm not sure i fully understand this local vs address vs
peer and ipv4 and ipv6 situation so I'm going to leave the decision to
you, whatever makes the most sense.
Going just by how the code "looks"I think maybe in the "init" part of
the method we could do:
if type(addr) is tuple:
ip = ipaddress(addr[0])
peer = ipaddress(addr[1])
else:
ip = ipaddress(addr)
peer = ipaddress(addr) # and a comment explaining this here?
And then you can use ip and peer in the rest of the method without any
conditions?
There's also an option to just use a ternary operator:
address=str(ip) if peer is None else str(peer),
But like I've said, I'm not sure what the correct logic here is...
-Ondrej