Discussion: more ideomatic access to IFLA_* deep below

[crosser]

I suddenly found myself writing code like this:

vrflist = [(
    l.get('index'),
    dict(l.get('attrs', {})).get('IFLA_IFNAME'),
    dict(
        dict(
            dict(
                l.get('attrs', {})
            ).get('IFLA_LINKINFO', {}).get('attrs', {})
        ).get('IFLA_INFO_DATA', {}).get('attrs', {})
    ).get('IFLA_VRF_TABLE'),
)
for l in IPRoute().get_links()
if  dict(
        dict(
            l.get('attrs', {})
        ).get('IFLA_LINKINFO', {}).get('attrs', {})
    ).get('IFLA_INFO_KIND') == 'vrf'
]

(for my use case, I wanted None for nonexisting attributes), hence ...get(..., {})). This made me wish be able to reach netlink attributes as attributes of python objects, a la

l.IFLA_LINKINFO.IFLA_INFO_DATA.IFLA_VRF_TABLE

Obviously this would make it impossible to have "None for nonexisting attributes", but it is OK. Other thing is potentially multiple attributes with the same key. Currently they are represented as a set of two-tuples, making direct access to the value by the key awkward (see all the dict()-s in my example). It might be possible to represent all attributes with the same key as a list instead, making my "ideal" example look more like this:

l.IFLA_LINKINFO[0].IFLA_INFO_DATA[0].IFLA_VRF_TABLE[0]

I wonder if such approach would be possible or practical?..

[svinota]

Right now there are two methods: .get_attr() and .get_attrs(). The first one returns the first NLA attr with the name or None (or whatever specified as the second argument), the second one returns a list of all NLA with that name or an empty list:

ipr.get_links()[0].get_attr('IFLA_AF_SPEC')

The worst is that one can not chain .get_attr() unless one is sure that it returns an NLA object. Thus

(ipr
 .get_links()[0]
 .get_attr('IFLA_AF_WHATEVER', {})
 .get_attr('IFLA_NEXT_LEVEL'))

will not work as a dict has no .get_attr(). A possible workaround:

from pyroute2.netlink import nla

(ipr
 .get_links()[0]
 .get_attr('IFLA_AF_WHATEVER', nla())
 .get_attr('IFLA_NEXT_LEVEL'))

Not sure if it helps, but maybe. And I'll try to see if it's possible to implement your syntax w/o significant slowdown. If it is — it may be a good idea.

[crosser]

Well, get_attr() does not improve readability very much, does it?

I was thinking along the lines of defining __getattr__() or __getattribute__() method in the nla class. Some "convenient" semantics could be defined, like, e.g. it only looks for names that start with 'IFLA', and if no such attribute exists, returns a "dummy nla" object. This "dummy nla" can have __bool__() method returning False, and __getattr__() that again returns "dummy nla". This way, IFLA... attributes could be chained directly via '.' like in my example, and you would get a "false object" if the chain is broken at some point.

Just an idea.

[svinota]

Something like that:

(ipr
 .get_links()[0]
 .IFLA_LINKINFO
 .IFLA_INFO_DATA
 .IFLA_VRF_TABLE)

(see the branch)

The code is far not ready, but it's possible to play around. Pls notice that it's possible to use NLA refs both without indices (IFLA_INFO_DATA) and with them (IFLA_INFO_DATA[0]).

Benchmarking, testing etc. will follow, it's just a proof-of-concept for now.

[svinota]

Added type conversion to bool, so an empty nlmsg may be used in if statements.

[svinota]

So far overloading of __getattribute__() gives us ~20% performance drop according to benchmarks on NDB/IPDB. I can not say I like that.

Though some things may be ported back to master.

[crosser]

I've just converted my existing enumerating script to the new scheme and it looks much nicer! (And works just as well as before :laughing:)

I am attaching two versions of the script for comparison. getvrfs1.txt getvrfs2.txt

[svinota]

A slightly different approach, see the branch nla_access_621_1.

In order to support list indexing on retrieved NLAs, I had to require an explicit list vs. value referencing, so nlmsg.SOME_NLA_NAME always returns an NLA object.

So:

(ipr
 .get_links[0]
 .IFLA_LINKINFO
 .IFLA_INFO_DATA
 .IFLA_IP6GRE_LOCAL
 .value)

Does it look reasonable?

And there is still some performance penalty that makes me sad, really.

[crosser]

Looks less succinct, but would do, fair enough.

But:

>>> from pyroute2 import IPRoute
>>> for l in IPRoute().get_links(): print(l.IFLA_LINKINFO)
... 
Traceback (most recent call last):
  File "/home/crosser/src/pyroute2/pyroute2/netlink/__init__.py", line 1165, in __getattribute__
    return super(nlmsg_base, self).__getattribute__(key)
AttributeError: 'ifinfmsg' object has no attribute 'IFLA_LINKINFO'

??? (works from branch nla_access_621)

[crosser]

Actually rnm does contain the key 'IFLA_LINKINFO'

debug printout shows that the if rnm and key in rnm: is entered, but after chain = self.nla(key) chain=={}, so it does not enter the next if and does not return.

[svinota]

Yep, that's a leftover of a debug code :)

Fixed, pushed.

[crosser]

Ummm...

now dot-chaining does not seem to work? Or something else got broken.

>>> from pyroute2 import IPRoute
>>> for l in IPRoute().get_links(): print(l.IFLA_LINKINFO.IFLA_INFO_KIND)
... 
Traceback (most recent call last):
  File "/home/crosser/src/pyroute2/pyroute2/netlink/__init__.py", line 1165, in __getattribute__
    return super(nlmsg_base, self).__getattribute__(key)
AttributeError: 'nlmsg_base' object has no attribute 'IFLA_INFO_KIND'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/crosser/src/pyroute2/pyroute2/netlink/__init__.py", line 1170, in __getattribute__
    raise AttributeError(key)
AttributeError: IFLA_INFO_KIND

[svinota]

Nope, it's too strict logic that forces __getattribute__() to use only defined set of NLA names. Which set is empty for fake nla objects returned for non-existing NLA.

Let me see, how can we solve that w/o being too permissive.

[svinota]

from pyroute2 import IPRoute

with IPRoute() as ipr:
    for link in ipr.get_links():
        print(link
              .IFLA_IFNAME
              .value,
              link
              .IFLA_LINKINFO
              .IFLA_INFO_KIND
              .value)

$ python3 t1.py 
lo None
ip6tnl0 ip6tnl
eth0 None
eth1 None
docker0 bridge
wg0 wireguard
tp0 veth
ip_vti0 vti
ip6_vti0 vti6
gre0 gre
gretap0 gretap
erspan0 erspan
ip6gre0 ip6gre
dummy0 dummy

Now it's better, but doesn't use the NLA dict — as for chaining non-existing NLAs it's meaningless.

[svinota]

Personally I find those .value references completely unaesthetic, but I see no other way to use NLAs as lists to get sibling NLAs. Getting int or str instead of nlmsg we lose this feature.

And at least it's explicit, and any explicit thing is better than implicit one.

[crosser]

Nope, it's too strict logic that forces __getattribute__() to use only defined set of NLA names. Which set is empty for fake nla objects returned for non-existing NLA.

Let me see, how can we solve that w/o being too permissive.

Might this be a reasonable compromise? If there are valid attributes, check against the list. If we are dealing with "fake" nla already, then make any attribure return "fake" nla again?

diff --git a/pyroute2/netlink/__init__.py b/pyroute2/netlink/__init__.py
index 9e35f86b..1c3717a6 100644
--- a/pyroute2/netlink/__init__.py
+++ b/pyroute2/netlink/__init__.py
@@ -1165,8 +1165,11 @@ class nlmsg_base(dict):
             return super(nlmsg_base, self).__getattribute__(key)
         except AttributeError:
             rnm = self.__class__.__r_nla_map
-            if rnm and key in rnm:
-                return self.nla(key)
+            if rnm:
+                if key in rnm:
+                    return self.nla(key)
+            elif key[0].isupper():
+                return nla_base()
             raise AttributeError(key)

     def __getitem__(self, key):

[svinota]

Let me trace, how expensive it will be. But the idea is good.

[svinota]

For now merged to the master. Let's see if there will be any negative feedback about the performance.

[crosser]

:+1: thanks!