mbox series

[v4,net-next,0/6] IGMP snooping for local traffic

Message ID 1510265462-25077-1-git-send-email-andrew@lunn.ch
Headers show
Series IGMP snooping for local traffic | expand

Message

Andrew Lunn Nov. 9, 2017, 10:10 p.m. UTC
The linux bridge supports IGMP snooping. It will listen to IGMP
reports on bridge ports and keep track of which groups have been
joined on an interface. It will then forward multicast based on this
group membership.

When the bridge adds or removed groups from an interface, it uses
switchdev to request the hardware add an mdb to a port, so the
hardware can perform the selective forwarding between ports.

What is not covered by the current bridge code, is IGMP joins/leaves
from the host on the brX interface. These are not reported via
switchdev so that hardware knows the local host is interested in the
multicast frames.

Luckily, the bridge does track joins/leaves on the brX interface. The
code is obfusticated, which is why i missed it with my first attempt.
So the first patch tries to remove this obfustication. Currently,
there is no notifications sent when the bridge interface joins a
group. The second patch adds them. bridge monitor then shows
joins/leaves in the same way as for other ports of the bridge.

Then starts the work passing down to the hardware that the host has
joined/left a group. The existing switchdev mdb object cannot be used,
since the semantics are different. The existing
SWITCHDEV_OBJ_ID_PORT_MDB is used to indicate a specific multicast
group should be forwarded out that port of the switch. However here we
require the exact opposite. We want multicast frames for the group
received on the port to the forwarded to the host. Hence add a new
object SWITCHDEV_OBJ_ID_HOST_MDB, a multicast database entry to
forward to the host. This new object is then propagated through the
DSA layers. No DSA driver changes should be needed, this should just
work...

This version fixes up the nitpick from Nikolay, removes an unrelated
white space change, and adds in a patch adding a few const attributes
to a couple of functions taking a port parameter, in order to stop the
following patch produces warnings.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Andrew Lunn (6):
  net: bridge: Rename mglist to host_joined
  net: bridge: Send notification when host join/leaves a group
  net: bridge: Add/del switchdev object on host join/leave
  net: dsa: slave: Handle switchdev host mdb add/del
  net: dsa: add more const attributes
  net: dsa: switch: Don't add CPU port to an mdb by default

 include/net/switchdev.h   |  1 +
 net/bridge/br_input.c     |  2 +-
 net/bridge/br_mdb.c       | 54 +++++++++++++++++++++++++++++++++++++++++++----
 net/bridge/br_multicast.c | 18 ++++++++++------
 net/bridge/br_private.h   |  2 +-
 net/dsa/dsa_priv.h        |  4 ++--
 net/dsa/port.c            |  6 +++---
 net/dsa/slave.c           | 13 ++++++++++++
 net/dsa/switch.c          |  2 +-
 net/switchdev/switchdev.c |  2 ++
 10 files changed, 85 insertions(+), 19 deletions(-)

Comments

Nikolay Aleksandrov Nov. 10, 2017, 2:03 a.m. UTC | #1
On 10.11.2017 00:10, Andrew Lunn wrote:
> The linux bridge supports IGMP snooping. It will listen to IGMP
> reports on bridge ports and keep track of which groups have been
> joined on an interface. It will then forward multicast based on this
> group membership.
> 
> When the bridge adds or removed groups from an interface, it uses
> switchdev to request the hardware add an mdb to a port, so the
> hardware can perform the selective forwarding between ports.
> 
> What is not covered by the current bridge code, is IGMP joins/leaves
> from the host on the brX interface. These are not reported via
> switchdev so that hardware knows the local host is interested in the
> multicast frames.
> 
> Luckily, the bridge does track joins/leaves on the brX interface. The
> code is obfusticated, which is why i missed it with my first attempt.
> So the first patch tries to remove this obfustication. Currently,
> there is no notifications sent when the bridge interface joins a
> group. The second patch adds them. bridge monitor then shows
> joins/leaves in the same way as for other ports of the bridge.
> 
> Then starts the work passing down to the hardware that the host has
> joined/left a group. The existing switchdev mdb object cannot be used,
> since the semantics are different. The existing
> SWITCHDEV_OBJ_ID_PORT_MDB is used to indicate a specific multicast
> group should be forwarded out that port of the switch. However here we
> require the exact opposite. We want multicast frames for the group
> received on the port to the forwarded to the host. Hence add a new
> object SWITCHDEV_OBJ_ID_HOST_MDB, a multicast database entry to
> forward to the host. This new object is then propagated through the
> DSA layers. No DSA driver changes should be needed, this should just
> work...
> 
> This version fixes up the nitpick from Nikolay, removes an unrelated
> white space change, and adds in a patch adding a few const attributes
> to a couple of functions taking a port parameter, in order to stop the
> following patch produces warnings.
> 
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> Andrew Lunn (6):
>   net: bridge: Rename mglist to host_joined
>   net: bridge: Send notification when host join/leaves a group
>   net: bridge: Add/del switchdev object on host join/leave
>   net: dsa: slave: Handle switchdev host mdb add/del
>   net: dsa: add more const attributes
>   net: dsa: switch: Don't add CPU port to an mdb by default
> 
>  include/net/switchdev.h   |  1 +
>  net/bridge/br_input.c     |  2 +-
>  net/bridge/br_mdb.c       | 54 +++++++++++++++++++++++++++++++++++++++++++----
>  net/bridge/br_multicast.c | 18 ++++++++++------
>  net/bridge/br_private.h   |  2 +-
>  net/dsa/dsa_priv.h        |  4 ++--
>  net/dsa/port.c            |  6 +++---
>  net/dsa/slave.c           | 13 ++++++++++++
>  net/dsa/switch.c          |  2 +-
>  net/switchdev/switchdev.c |  2 ++
>  10 files changed, 85 insertions(+), 19 deletions(-)
> 

Andrew, overall looks good to me, thanks for keeping the acks and
incorporating the changes. Just one note - in the future please add the
reviewers to the CC list. I've been reviewing and suggesting changes to
this set since its RFC/WIP version but it's making it harder to track
when I'm not receiving the new versions and have to search for them on
netdev.

Thanks,
 Nik
David Miller Nov. 10, 2017, 4:42 a.m. UTC | #2
From: Andrew Lunn <andrew@lunn.ch>
Date: Thu,  9 Nov 2017 23:10:56 +0100

> The linux bridge supports IGMP snooping. It will listen to IGMP
> reports on bridge ports and keep track of which groups have been
> joined on an interface. It will then forward multicast based on this
> group membership.
> 
> When the bridge adds or removed groups from an interface, it uses
> switchdev to request the hardware add an mdb to a port, so the
> hardware can perform the selective forwarding between ports.
> 
> What is not covered by the current bridge code, is IGMP joins/leaves
> from the host on the brX interface. These are not reported via
> switchdev so that hardware knows the local host is interested in the
> multicast frames.
> 
> Luckily, the bridge does track joins/leaves on the brX interface. The
> code is obfusticated, which is why i missed it with my first attempt.
> So the first patch tries to remove this obfustication. Currently,
> there is no notifications sent when the bridge interface joins a
> group. The second patch adds them. bridge monitor then shows
> joins/leaves in the same way as for other ports of the bridge.
> 
> Then starts the work passing down to the hardware that the host has
> joined/left a group. The existing switchdev mdb object cannot be used,
> since the semantics are different. The existing
> SWITCHDEV_OBJ_ID_PORT_MDB is used to indicate a specific multicast
> group should be forwarded out that port of the switch. However here we
> require the exact opposite. We want multicast frames for the group
> received on the port to the forwarded to the host. Hence add a new
> object SWITCHDEV_OBJ_ID_HOST_MDB, a multicast database entry to
> forward to the host. This new object is then propagated through the
> DSA layers. No DSA driver changes should be needed, this should just
> work...
> 
> This version fixes up the nitpick from Nikolay, removes an unrelated
> white space change, and adds in a patch adding a few const attributes
> to a couple of functions taking a port parameter, in order to stop the
> following patch produces warnings.
> 
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Series applied, thanks Andrew.