Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly parse IFLA_VLAN_PROTOCOL in IPDB interface #393

Merged
merged 1 commit into from
Jul 14, 2017

Conversation

yonirom
Copy link
Contributor

@yonirom yonirom commented Jul 13, 2017

In issue #361 the example file qinq.zip works with IPRoute.

When trying the same with IPDB, parsing the result when committing an interface with vlan_protocol fails:

Traceback (most recent call last):
  File "qinq.py", line 9, in <module>
    ipdb.create(kind="vlan", ifname="qtest.50", link=hidx, vlan_id=50, vlan_protocol=0x88a8).commit()
  File "/usr/local/lib/python2.7/dist-packages/pyroute2/pyroute2/ipdb/interfaces.py", line 1060, in commit
    raise error
pyroute2.ipdb.exceptions.CommitException: target vlan_protocol is not set

This fixes the issue for me.

This is the modified qinq.py:

from pyroute2 import IPDB

ipdb = IPDB()
# create host dummy
ipdb.create(kind="dummy", ifname="qtest").commit()
hidx = ipdb.interfaces['qtest']

# create 802.1ad s-vlan interface (external tag)
ipdb.create(kind="vlan", ifname="qtest.50", link=hidx, vlan_id=50, vlan_protocol=0x88a8).commit()

# create 802.1q c-vlan interface (internal tag)
ipdb.create(kind="vlan", ifname="qtest.50.120", link=hidx, vlan_id=120, vlan_protocol=0x8100).commit()

# print the links
for link in (ipdb.interfaces['qtest.50'], ipdb.interfaces['qtest.50.120']):
    print(link.ifname)
    print("\t%s" % (link.vlan_protocol))


# cleanup
hidx.remove().commit()

@svinota
Copy link
Owner

svinota commented Jul 13, 2017

Thanks a lot! Will be merged tomorrow.

@yonirom
Copy link
Contributor Author

yonirom commented Jul 13, 2017

Can you please verify that if this has any impact on #361

My tests might be inconsistent and messy, but compared to pyroute2 0.4.18 this seems to work:

This is qinq.py (with ipdb):

from pyroute2 import IPDB, NetNS

ipdb = IPDB()
# create host dummy
ipdb.create(kind="dummy", ifname="qtest").commit()
hidx = ipdb.interfaces['qtest']

# create 802.1ad s-vlan interface (external tag)
ipdb.create(kind="vlan", ifname="qtest.50", link=hidx, vlan_id=50, vlan_protocol=0x88a8).commit()

# create 802.1q c-vlan interface (internal tag)
ipdb.create(kind="vlan", ifname="qtest.50.120", link=hidx, vlan_id=120, vlan_protocol=0x8100).commit()

# print the links
for link in (ipdb.interfaces['qtest.50'], ipdb.interfaces['qtest.50.120']):
    print(link.ifname)
    print("\t%s" % (link.vlan_protocol))

Vanilla pyroute 0.4.18

root@ubuntu# ip -d link
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 promiscuity 0
2: gre0@NONE: <NOARP> mtu 1476 qdisc noop state DOWN mode DEFAULT group default qlen 1
    link/gre 0.0.0.0 brd 0.0.0.0 promiscuity 0
    gre remote any local any ttl inherit nopmtudisc
3: gretap0@NONE: <BROADCAST,MULTICAST> mtu 1462 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether 00:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff promiscuity 0
    gretap remote any local any ttl inherit nopmtudisc
2956: eth0@if2957: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default
    link/ether 02:42:ac:13:00:06 brd ff:ff:ff:ff:ff:ff promiscuity 0
    veth
root@ubuntu# python qinq.py
qtest.50
        34984
qtest.50.120
        33024
root@ubuntu# ip -d link
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 promiscuity 0
2: gre0@NONE: <NOARP> mtu 1476 qdisc noop state DOWN mode DEFAULT group default qlen 1
    link/gre 0.0.0.0 brd 0.0.0.0 promiscuity 0
    gre remote any local any ttl inherit nopmtudisc
3: gretap0@NONE: <BROADCAST,MULTICAST> mtu 1462 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether 00:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff promiscuity 0
    gretap remote any local any ttl inherit nopmtudisc
261: qtest: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether 66:e0:fb:47:a4:54 brd ff:ff:ff:ff:ff:ff promiscuity 0
    dummy
262: qtest.50@qtest: <BROADCAST,NOARP,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether 66:e0:fb:47:a4:54 brd ff:ff:ff:ff:ff:ff promiscuity 0
    vlan protocol 802.1Q id 50 <REORDER_HDR>
263: qtest.50.120@qtest: <BROADCAST,NOARP,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether 66:e0:fb:47:a4:54 brd ff:ff:ff:ff:ff:ff promiscuity 0
    vlan protocol 802.1Q id 120 <REORDER_HDR>
2956: eth0@if2957: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default
    link/ether 02:42:ac:13:00:06 brd ff:ff:ff:ff:ff:ff promiscuity 0
    veth

notice both devices created as type 802.1Q (even though qinq.py displays otherwise)

After applying the patch (git master + pr):

root@ubuntu# ip -d link
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 promiscuity 0
2: gre0@NONE: <NOARP> mtu 1476 qdisc noop state DOWN mode DEFAULT group default qlen 1
    link/gre 0.0.0.0 brd 0.0.0.0 promiscuity 0
    gre remote any local any ttl inherit nopmtudisc
3: gretap0@NONE: <BROADCAST,MULTICAST> mtu 1462 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether 00:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff promiscuity 0
    gretap remote any local any ttl inherit nopmtudisc
2956: eth0@if2957: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default
    link/ether 02:42:ac:13:00:06 brd ff:ff:ff:ff:ff:ff promiscuity 0
    veth
root@ubuntu# python qinq.py
qtest.50
        34984
qtest.50.120
        33024
root@ubuntu# ip -d link
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 promiscuity 0
2: gre0@NONE: <NOARP> mtu 1476 qdisc noop state DOWN mode DEFAULT group default qlen 1
    link/gre 0.0.0.0 brd 0.0.0.0 promiscuity 0
    gre remote any local any ttl inherit nopmtudisc
3: gretap0@NONE: <BROADCAST,MULTICAST> mtu 1462 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether 00:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff promiscuity 0
    gretap remote any local any ttl inherit nopmtudisc
264: qtest: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether c6:53:b7:e8:8e:27 brd ff:ff:ff:ff:ff:ff promiscuity 0
    dummy
265: qtest.50@qtest: <BROADCAST,NOARP,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether c6:53:b7:e8:8e:27 brd ff:ff:ff:ff:ff:ff promiscuity 0
    vlan protocol 802.1ad id 50 <REORDER_HDR>
266: qtest.50.120@qtest: <BROADCAST,NOARP,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether c6:53:b7:e8:8e:27 brd ff:ff:ff:ff:ff:ff promiscuity 0
    vlan protocol 802.1Q id 120 <REORDER_HDR>
2956: eth0@if2957: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default
    link/ether 02:42:ac:13:00:06 brd ff:ff:ff:ff:ff:ff promiscuity 0
    veth

** both devices created correctly**

I would love to hear your opinion regarding this issue.
I have not gone deep enough into the code to know if this is the cause.

Thanks!

@svinota svinota merged commit 6b51a1e into svinota:master Jul 14, 2017
svinota added a commit that referenced this pull request Jul 14, 2017
@svinota
Copy link
Owner

svinota commented Jul 14, 2017

Since there is no automatic mapping between NLAs — that may have really weird layout sometimes, as there is no standard — and «flat» IPDB dictionaries, we have to explicitly tell, how to load some specific attributes. Your solution is exactly what I would do to fix the issue.

Thanks a lot!

Also, notice the test code added to cover the issue: 9a2e02c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants