All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] rtnetlink: Fix regression in bridge VLAN configuration
@ 2021-06-09 11:17 Ido Schimmel
  2021-06-09 22:00 ` patchwork-bot+netdevbpf
  2021-06-15 17:06 ` Joachim Wiberg
  0 siblings, 2 replies; 3+ messages in thread
From: Ido Schimmel @ 2021-06-09 11:17 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, nikolay, roopa, jiapeng.chong, mlxsw, Ido Schimmel

From: Ido Schimmel <idosch@nvidia.com>

Cited commit started returning errors when notification info is not
filled by the bridge driver, resulting in the following regression:

 # ip link add name br1 type bridge vlan_filtering 1
 # bridge vlan add dev br1 vid 555 self pvid untagged
 RTNETLINK answers: Invalid argument

As long as the bridge driver does not fill notification info for the
bridge device itself, an empty notification should not be considered as
an error. This is explained in commit 59ccaaaa49b5 ("bridge: dont send
notification when skb->len == 0 in rtnl_bridge_notify").

Fix by removing the error and add a comment to avoid future bugs.

Fixes: a8db57c1d285 ("rtnetlink: Fix missing error code in rtnl_bridge_notify()")
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Nikolay Aleksandrov <nikolay@nvidia.com>
---
 net/core/rtnetlink.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 3e84279c4123..ec931b080156 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -4842,10 +4842,12 @@ static int rtnl_bridge_notify(struct net_device *dev)
 	if (err < 0)
 		goto errout;
 
-	if (!skb->len) {
-		err = -EINVAL;
+	/* Notification info is only filled for bridge ports, not the bridge
+	 * device itself. Therefore, a zero notification length is valid and
+	 * should not result in an error.
+	 */
+	if (!skb->len)
 		goto errout;
-	}
 
 	rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, GFP_ATOMIC);
 	return 0;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] rtnetlink: Fix regression in bridge VLAN configuration
  2021-06-09 11:17 [PATCH net] rtnetlink: Fix regression in bridge VLAN configuration Ido Schimmel
@ 2021-06-09 22:00 ` patchwork-bot+netdevbpf
  2021-06-15 17:06 ` Joachim Wiberg
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-06-09 22:00 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: netdev, davem, kuba, nikolay, roopa, jiapeng.chong, mlxsw, idosch

Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Wed,  9 Jun 2021 14:17:53 +0300 you wrote:
> From: Ido Schimmel <idosch@nvidia.com>
> 
> Cited commit started returning errors when notification info is not
> filled by the bridge driver, resulting in the following regression:
> 
>  # ip link add name br1 type bridge vlan_filtering 1
>  # bridge vlan add dev br1 vid 555 self pvid untagged
>  RTNETLINK answers: Invalid argument
> 
> [...]

Here is the summary with links:
  - [net] rtnetlink: Fix regression in bridge VLAN configuration
    https://git.kernel.org/netdev/net/c/d2e381c49636

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] rtnetlink: Fix regression in bridge VLAN configuration
  2021-06-09 11:17 [PATCH net] rtnetlink: Fix regression in bridge VLAN configuration Ido Schimmel
  2021-06-09 22:00 ` patchwork-bot+netdevbpf
@ 2021-06-15 17:06 ` Joachim Wiberg
  1 sibling, 0 replies; 3+ messages in thread
From: Joachim Wiberg @ 2021-06-15 17:06 UTC (permalink / raw)
  To: Ido Schimmel, netdev
  Cc: davem, kuba, nikolay, roopa, jiapeng.chong, mlxsw, Ido Schimmel

On Wed, Jun 09, 2021 at 14:17, Ido Schimmel <idosch@idosch.org> wrote:
> Cited commit started returning errors when notification info is not
> filled by the bridge driver, resulting in the following regression:
>
>  # ip link add name br1 type bridge vlan_filtering 1
>  # bridge vlan add dev br1 vid 555 self pvid untagged
>  RTNETLINK answers: Invalid argument
>
> As long as the bridge driver does not fill notification info for the
> bridge device itself, an empty notification should not be considered as
> an error. This is explained in commit 59ccaaaa49b5 ("bridge: dont send
> notification when skb->len == 0 in rtnl_bridge_notify").
>
> Fix by removing the error and add a comment to avoid future bugs.
>
> Fixes: a8db57c1d285 ("rtnetlink: Fix missing error code in rtnl_bridge_notify()")
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> Reviewed-by: Nikolay Aleksandrov <nikolay@nvidia.com>

Fix does indeed solve the same problem I encountered myself yesterday,
and bisected today.  Tested successfully on a bridge setup with Marvell
88E6097 (DSA switch) for bridge offloading (not that it matters for this
particular fix).

Tested-by: Joachim Wiberg <troglobit@gmail.com>

Best regards
 /Joachim

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-06-15 17:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-09 11:17 [PATCH net] rtnetlink: Fix regression in bridge VLAN configuration Ido Schimmel
2021-06-09 22:00 ` patchwork-bot+netdevbpf
2021-06-15 17:06 ` Joachim Wiberg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.