2013-08-12 16:37:23

by Asbjørn Sloth Tønnesen

[permalink] [raw]
Subject: Bridge VLAN kernel/iproute2 incompatibility

Hi,

Let's start with a little history:

Feb 20: Vlad Yasevich got his VLAN-aware bridge patchset included in
the 3.9 merge window.
In the kernel commit 6cbdceeb, he added attribute support to
bridge GETLINK requests sent with rtgenmsg.

Mar 6th: Vlad got this iproute2 reference implementation of the bridge
vlan netlink interface accepted (iproute2 9eff0e5c)

Apr 25th: iproute2 switched from using rtgenmsg to ifinfomsg (63338dca)
http://patchwork.ozlabs.org/patch/239602/
http://marc.info/?t=136680900700007

Apr 28th: Linus released 3.9

Apr 30th: Stephen released iproute2 3.9.0

The `bridge vlan show` command haven't been working since the switch to
ifinfomsg, or in a released version of iproute2. Since the kernel side
only supports rtgenmsg, which iproute2 switched away from just prior to
the iproute2 3.9.0 release.

I haven't been able to find any documentation, about neither rtgenmsg
nor ifinfomsg, and in which situation to use which, but kernel commit
88c5b5ce seams to suggest that ifinfomsg should be used.

Fixing this in kernel will break compatibility, but I doubt that anybody
have been using it due to this bug in the user space reference
implementation, at least not without noticing this bug. That said the
functionality is still fully functional in 3.9, when reversing iproute2
commit 63338dca.

This could also be fixed in iproute2, but thats an ugly patch that would
reintroduce rtgenmsg in iproute2, and from searching in netdev it seams
like rtgenmsg usage is discouraged. I'm assuming that the only reason
that Vlad implemented the kernel side to use rtgenmsg, was because
iproute2 was using it at the time.

I will reply with kernel patch, I can also post the alternative iproute2
patch if requested.

--
Best regards
Asbjørn Sloth Tønnesen
Network engineer
Fiberby ApS - AS42541


2013-08-12 16:37:28

by Asbjørn Sloth Tønnesen

[permalink] [raw]
Subject: [PATCH] rtnetlink: rtnl_bridge_getlink: Call nlmsg_find_attr() with ifinfomsg header

Fix the iproute2 command `bridge vlan show`, after switching from
rtgenmsg to ifinfomsg.

Signed-off-by: Asbjoern Sloth Toennesen <[email protected]>
---
net/core/rtnetlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 3de7408..a043171 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2384,7 +2384,7 @@ static int rtnl_bridge_getlink(struct sk_buff *skb, struct netlink_callback *cb)
struct nlattr *extfilt;
u32 filter_mask = 0;

- extfilt = nlmsg_find_attr(cb->nlh, sizeof(struct rtgenmsg),
+ extfilt = nlmsg_find_attr(cb->nlh, sizeof(struct ifinfomsg),
IFLA_EXT_MASK);
if (extfilt)
filter_mask = nla_get_u32(extfilt);
--
1.8.4.rc1

2013-08-12 19:57:43

by Vlad Yasevich

[permalink] [raw]
Subject: Re: [PATCH] rtnetlink: rtnl_bridge_getlink: Call nlmsg_find_attr() with ifinfomsg header

On 08/12/2013 12:30 PM, Asbjoern Sloth Toennesen wrote:
> Fix the iproute2 command `bridge vlan show`, after switching from
> rtgenmsg to ifinfomsg.
>
> Signed-off-by: Asbjoern Sloth Toennesen <[email protected]>


Thanks.. I've still been using an older iproute version and didn't see
this.


Reviewed-by: Vlad Yasevich <[email protected]>
> ---
> net/core/rtnetlink.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 3de7408..a043171 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -2384,7 +2384,7 @@ static int rtnl_bridge_getlink(struct sk_buff *skb, struct netlink_callback *cb)
> struct nlattr *extfilt;
> u32 filter_mask = 0;
>
> - extfilt = nlmsg_find_attr(cb->nlh, sizeof(struct rtgenmsg),
> + extfilt = nlmsg_find_attr(cb->nlh, sizeof(struct ifinfomsg),
> IFLA_EXT_MASK);
> if (extfilt)
> filter_mask = nla_get_u32(extfilt);
>

2013-08-13 22:54:46

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] rtnetlink: rtnl_bridge_getlink: Call nlmsg_find_attr() with ifinfomsg header

From: Vlad Yasevich <[email protected]>
Date: Mon, 12 Aug 2013 15:57:29 -0400

> On 08/12/2013 12:30 PM, Asbjoern Sloth Toennesen wrote:
>> Fix the iproute2 command `bridge vlan show`, after switching from
>> rtgenmsg to ifinfomsg.
>>
>> Signed-off-by: Asbjoern Sloth Toennesen <[email protected]>
>
>
> Thanks.. I've still been using an older iproute version and didn't
> see this.
>
>
> Reviewed-by: Vlad Yasevich <[email protected]>

What introduced this regression?

2013-08-13 23:06:46

by Vlad Yasevich

[permalink] [raw]
Subject: Re: [PATCH] rtnetlink: rtnl_bridge_getlink: Call nlmsg_find_attr() with ifinfomsg header

On 08/13/2013 06:54 PM, David Miller wrote:
> From: Vlad Yasevich <[email protected]>
> Date: Mon, 12 Aug 2013 15:57:29 -0400
>
>> On 08/12/2013 12:30 PM, Asbjoern Sloth Toennesen wrote:
>>> Fix the iproute2 command `bridge vlan show`, after switching from
>>> rtgenmsg to ifinfomsg.
>>>
>>> Signed-off-by: Asbjoern Sloth Toennesen <[email protected]>
>>
>>
>> Thanks.. I've still been using an older iproute version and didn't
>> see this.
>>
>>
>> Reviewed-by: Vlad Yasevich <[email protected]>
>
> What introduced this regression?
>

ast explained it in his header message (Bridge VLAN kernel/iproute2
incompatibility)

-vlad

2013-08-13 23:31:30

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] rtnetlink: rtnl_bridge_getlink: Call nlmsg_find_attr() with ifinfomsg header

From: Vlad Yasevich <[email protected]>
Date: Tue, 13 Aug 2013 19:06:37 -0400

> ast explained it in his header message (Bridge VLAN kernel/iproute2
> incompatibility)

That's not a header message.

Header messages have a subject prefix of the form "[PATCH 0/N]".
If he had done this I wouldn't have had to ask such silly
questions.

2013-08-14 02:10:15

by David Miller

[permalink] [raw]
Subject: Re: Bridge VLAN kernel/iproute2 incompatibility

From: Asbj?rn Sloth T?nnesen <[email protected]>
Date: Mon, 12 Aug 2013 16:24:06 +0000

> Let's start with a little history:

I've applied your kernel patch, but the detailed analysis you put
here, belongs in the commit message.

Thanks.