Skip to content
This repository has been archived by the owner on Oct 26, 2022. It is now read-only.

bridge: Fix decode error on bridge with IFLA_BR_MCAST_QUERIER_STATE #247

Closed
wants to merge 1 commit into from

Conversation

cathay4t
Copy link
Collaborator

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:

  • IFLA_BRIDGE_FLAGS is 0, and for IFLA_AF_SPEC, not IFLA_LINKINFO.
  • IFLA_BR_MCAST_QUERIER_STATE is a nested netlink attribute, not u16
    which cause this failure.

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.

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:
 * `IFLA_BRIDGE_FLAGS` is 0, and for `IFLA_AF_SPEC`, not `IFLA_LINKINFO`.
 * `IFLA_BR_MCAST_QUERIER_STATE` is a nested netlink attribute, not u16
   which cause this failure.

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`.

Signed-off-by: Gris Ge <[email protected]>
@cathay4t cathay4t force-pushed the fix_bridge_link_info branch from 73bbdd7 to f86f418 Compare March 18, 2022 05:34
@cathay4t
Copy link
Collaborator Author

@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
Copy link
Collaborator Author

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

@little-dude
Copy link
Owner

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 #44 ?

@cathay4t
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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.

cathay4t added a commit to cathay4t/nispor that referenced this pull request Mar 22, 2022
Due to issue of little-dude/netlink#247 , nispor
could not show contain bridge on kernel supporting
`IFLA_BR_MCAST_QUERIER_STATE`.

Instead of waiting rust-netlink to tag new release, we are including
patched `netlink-package-route` into this project and switch to upstream
once they have new release.

Signed-off-by: Gris Ge <[email protected]>
cathay4t added a commit to cathay4t/nispor that referenced this pull request Mar 22, 2022
Due to issue of little-dude/netlink#247 , nispor
could not show contain bridge with `IFLA_BR_MCAST_QUERIER_STATE` data.

Instead of waiting rust-netlink to tag new release, we are including
patched `netlink-package-route` into this project and switch to upstream
once they have new release.

Signed-off-by: Gris Ge <[email protected]>
cathay4t added a commit to nispor/nispor that referenced this pull request Mar 22, 2022
Due to issue of little-dude/netlink#247 , nispor
could not show contain bridge with `IFLA_BR_MCAST_QUERIER_STATE` data.

Instead of waiting rust-netlink to tag new release, we are including
patched `netlink-package-route` into this project and switch to upstream
once they have new release.

Signed-off-by: Gris Ge <[email protected]>
@stbuehler
Copy link
Contributor

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
Copy link
Collaborator Author

cathay4t commented May 6, 2022

Close in favor of #222

@cathay4t cathay4t closed this May 6, 2022
@cathay4t cathay4t deleted the fix_bridge_link_info branch May 6, 2022 00:05
cathay4t added a commit to cathay4t/nmstate that referenced this pull request Jun 24, 2022
The nispor 1.2.6 fixed the linux bridge bug:

    little-dude/netlink#247

Which is important to nmstate

Signed-off-by: Gris Ge <[email protected]>
cathay4t added a commit to cathay4t/nmstate that referenced this pull request Jun 27, 2022
The nispor 1.2.6 fixed the linux bridge bug:

    little-dude/netlink#247

Which is important to nmstate

Signed-off-by: Gris Ge <[email protected]>
cathay4t added a commit to cathay4t/nmstate that referenced this pull request Jun 27, 2022
The nispor 1.2.6 fixed the linux bridge bug:

    little-dude/netlink#247

Which is important to nmstate

Signed-off-by: Gris Ge <[email protected]>
cathay4t added a commit to cathay4t/nmstate that referenced this pull request Jun 29, 2022
The nispor 1.2.7 fixed the linux bridge bug:

    little-dude/netlink#247

Which is important to nmstate

Signed-off-by: Gris Ge <[email protected]>
cathay4t added a commit to cathay4t/nmstate that referenced this pull request Jun 29, 2022
The nispor 1.2.7 fixed the linux bridge bug:

    little-dude/netlink#247

Which is important to nmstate

Signed-off-by: Gris Ge <[email protected]>
cathay4t added a commit to cathay4t/nmstate that referenced this pull request Jun 29, 2022
The nispor 1.2.7 fixed the linux bridge bug:

    little-dude/netlink#247

Which is important to nmstate

Signed-off-by: Gris Ge <[email protected]>
ffmancera pushed a commit to cathay4t/nmstate that referenced this pull request Jun 29, 2022
The nispor 1.2.7 fixed the linux bridge bug:

    little-dude/netlink#247

Which is important to nmstate

Signed-off-by: Gris Ge <[email protected]>
cathay4t added a commit to nmstate/nmstate that referenced this pull request Jun 29, 2022
The nispor 1.2.7 fixed the linux bridge bug:

    little-dude/netlink#247

Which is important to nmstate

Signed-off-by: Gris Ge <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants