Message-ID: <e6e7814e-3d8b-3410-483b-df581a26e389@cumulusnetworks.com>
Date:   Fri, 10 Nov 2017 04:03:08 +0200
From:   Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To:     Andrew Lunn <andrew@...n.ch>, David Miller <davem@...emloft.net>
Cc:     Vivien Didelot <vivien.didelot@...oirfairelinux.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH v4 net-next 0/6] IGMP snooping for local traffic

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@...workplumber.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

Powered by blists - more mailing lists