little-dude / netlink

netlink libraries for rust
Other
329 stars 89 forks source link

bridge: Fix decode error on bridge with IFLA_BR_MCAST_QUERIER_STATE #247

Closed cathay4t closed 2 years ago

cathay4t commented 2 years ago

With kernel supporting IFLA_BR_MCAST_QUERIER_STATE, we got error:

Decode error occurred: Failed to parse message with type 16

After painfull debug, this is caused by IFLA_BRIDGE_FLAGS holding the same value as IFLA_BR_MCAST_QUERIER_STATE and expecting the payload been a u16.

This is incorrect:

I have no project needing the support IFLA_BR_MCAST_QUERIER_STATE of it yet, so this patch just remove IFLA_BRIDGE_FLAGS, IFLA_BRIDGE_VLAN_INFO, and their associates.

Also reordering the order of parsing IFLA_LINKINFO for bridge, to match the numeric order. So the developer could easily compare it to /usr/include/linux/if_link.h.

cathay4t commented 2 years ago

@little-dude This bug is breaking my project. Please review and tag new release afterwards. The use of anyhow::context make the debug life horrible, it discard all the error in the middle, but showing top level meaningless message. Any idea how to improve it?

cathay4t commented 2 years ago

@ffmancera The impact of this bug: it stop nmstate/nispor on all linux bridge related tasks. In case you ran into also.

little-dude commented 2 years ago

Regarding anyhow: you get all the context if you print it as debug: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=832a92287ed29ef27954a25d3b32750a

I'm on vacation for two weeks so I won't be able to review this before, sorry. Is this a case where different kernels will expect a different message format as in https://github.com/little-dude/netlink/issues/44 ?

cathay4t commented 2 years ago

No, it is not a kernel version specific difference. The last modifcation regarding IFLA_BRIDGE_FLAGS is:

[2469ffd72] net: set and query VEB/VEPA bridge mode via PF_BRIDGE John Fastabend (2012-10-24 04:13:03 PM)

Which is 10 years ago.

cathay4t commented 2 years ago

Regarding the anyhow preserving lower level errors, it is not happening in this project. This is the full debug log: https://gist.github.com/cathay4t/78d7550ae97db8254ce2891f49cda237

It only contains the error: failed to decode packet <package_u8_array_dump_omitted>: Decode error occurred: Failed to parse message with type 16, nothing else.

stbuehler commented 2 years ago

It seems #222 also removes IFLA_BRIDGE_FLAGS and IFLA_BRIDGE_VLAN_INFO from InfoBridge, and tries to implement IFLA_AF_SPEC and nested key AF_BRIDGE with those instead.

cathay4t commented 2 years ago

Close in favor of #222