All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Horatiu Vultur <horatiu.vultur@microchip.com>
Cc: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>,
	netdev@vger.kernel.org, roopa@cumulusnetworks.com,
	bridge@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
	allan.nielsen@microchip.com, davem@davemloft.net
Subject: Re: [Bridge] [PATCH] net: bridge: Allow bridge to joing multicast groups
Date: Fri, 26 Jul 2019 15:46:13 +0200	[thread overview]
Message-ID: <20190726134613.GD18223@lunn.ch> (raw)
In-Reply-To: <20190726120214.c26oj5vks7g5ntwu@soft-dev3.microsemi.net>

On Fri, Jul 26, 2019 at 02:02:15PM +0200, Horatiu Vultur wrote:
> Hi Nikolay,
> 
> The 07/26/2019 12:26, Nikolay Aleksandrov wrote:
> > External E-Mail
> > 
> > 
> > On 26/07/2019 11:41, Nikolay Aleksandrov wrote:
> > > On 25/07/2019 17:21, Horatiu Vultur wrote:
> > >> Hi Nikolay,
> > >>
> > >> The 07/25/2019 16:21, Nikolay Aleksandrov wrote:
> > >>> External E-Mail
> > >>>
> > >>>
> > >>> On 25/07/2019 16:06, Nikolay Aleksandrov wrote:
> > >>>> On 25/07/2019 14:44, Horatiu Vultur wrote:
> > >>>>> There is no way to configure the bridge, to receive only specific link
> > >>>>> layer multicast addresses. From the description of the command 'bridge
> > >>>>> fdb append' is supposed to do that, but there was no way to notify the
> > >>>>> network driver that the bridge joined a group, because LLADDR was added
> > >>>>> to the unicast netdev_hw_addr_list.
> > >>>>>
> > >>>>> Therefore update fdb_add_entry to check if the NLM_F_APPEND flag is set
> > >>>>> and if the source is NULL, which represent the bridge itself. Then add
> > >>>>> address to multicast netdev_hw_addr_list for each bridge interfaces.
> > >>>>> And then the .ndo_set_rx_mode function on the driver is called. To notify
> > >>>>> the driver that the list of multicast mac addresses changed.
> > >>>>>
> > >>>>> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > >>>>> ---
> > >>>>>  net/bridge/br_fdb.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
> > >>>>>  1 file changed, 46 insertions(+), 3 deletions(-)
> > >>>>>
> > >>>>
> > >>>> Hi,
> > >>>> I'm sorry but this patch is wrong on many levels, some notes below. In general
> > >>>> NLM_F_APPEND is only used in vxlan, the bridge does not handle that flag at all.
> > >>>> FDB is only for *unicast*, nothing is joined and no multicast should be used with fdbs.
> > >>>> MDB is used for multicast handling, but both of these are used for forwarding.
> > >>>> The reason the static fdbs are added to the filter is for non-promisc ports, so they can
> > >>>> receive traffic destined for these FDBs for forwarding.
> > >>>> If you'd like to join any multicast group please use the standard way, if you'd like to join
> > >>>> it only on a specific port - join it only on that port (or ports) and the bridge and you'll
> > >>>
> > >>> And obviously this is for the case where you're not enabling port promisc mode (non-default).
> > >>> In general you'll only need to join the group on the bridge to receive traffic for it
> > >>> or add it as an mdb entry to forward it.
> > >>>
> > >>>> have the effect that you're describing. What do you mean there's no way ?
> > >>
> > >> Thanks for the explanation.
> > >> There are few things that are not 100% clear to me and maybe you can
> > >> explain them, not to go totally in the wrong direction. Currently I am
> > >> writing a network driver on which I added switchdev support. Then I was
> > >> looking for a way to configure the network driver to copy link layer
> > >> multicast address to the CPU port.
> > >>
> > >> If I am using bridge mdb I can do it only for IP multicast addreses,
> > >> but how should I do it if I want non IP frames with link layer multicast
> > >> address to be copy to CPU? For example: all frames with multicast
> > >> address '01-21-6C-00-00-01' to be copy to CPU. What is the user space
> > >> command for that?
> > >>
> > > 
> > > Check SIOCADDMULTI (ip maddr from iproute2), f.e. add that mac to the port
> > > which needs to receive it and the bridge will send it up automatically since
> > > it's unknown mcast (note that if there's a querier, you'll have to make the
> > > bridge mcast router if it is not the querier itself). It would also flood it to all
> > 
> > Actually you mentioned non-IP traffic, so the querier stuff is not a problem. This
> > traffic will always be flooded by the bridge (and also a copy will be locally sent up).
> > Thus only the flooding may need to be controlled.
> 
> OK, I see, but the part which is not clear to me is, which bridge
> command(from iproute2) to use so the bridge would notify the network
> driver(using swichdev or not) to configure the HW to copy all the frames
> with dmac '01-21-6C-00-00-01' to CPU? So that the bridge can receive
> those frames and then just to pass them up.

Hi Horatiu

Something to keep in mind.

My default, multicast should be flooded, and that includes the CPU
port for a DSA driver. Adding an MDB entry allows for optimisations,
limiting which ports a multicast frame goes out of. But it is just an
optimisation.

	Andrew

WARNING: multiple messages have this Message-ID (diff)
From: Andrew Lunn <andrew@lunn.ch>
To: Horatiu Vultur <horatiu.vultur@microchip.com>
Cc: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>,
	roopa@cumulusnetworks.com, davem@davemloft.net,
	bridge@lists.linux-foundation.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, allan.nielsen@microchip.com
Subject: Re: [PATCH] net: bridge: Allow bridge to joing multicast groups
Date: Fri, 26 Jul 2019 15:46:13 +0200	[thread overview]
Message-ID: <20190726134613.GD18223@lunn.ch> (raw)
In-Reply-To: <20190726120214.c26oj5vks7g5ntwu@soft-dev3.microsemi.net>

On Fri, Jul 26, 2019 at 02:02:15PM +0200, Horatiu Vultur wrote:
> Hi Nikolay,
> 
> The 07/26/2019 12:26, Nikolay Aleksandrov wrote:
> > External E-Mail
> > 
> > 
> > On 26/07/2019 11:41, Nikolay Aleksandrov wrote:
> > > On 25/07/2019 17:21, Horatiu Vultur wrote:
> > >> Hi Nikolay,
> > >>
> > >> The 07/25/2019 16:21, Nikolay Aleksandrov wrote:
> > >>> External E-Mail
> > >>>
> > >>>
> > >>> On 25/07/2019 16:06, Nikolay Aleksandrov wrote:
> > >>>> On 25/07/2019 14:44, Horatiu Vultur wrote:
> > >>>>> There is no way to configure the bridge, to receive only specific link
> > >>>>> layer multicast addresses. From the description of the command 'bridge
> > >>>>> fdb append' is supposed to do that, but there was no way to notify the
> > >>>>> network driver that the bridge joined a group, because LLADDR was added
> > >>>>> to the unicast netdev_hw_addr_list.
> > >>>>>
> > >>>>> Therefore update fdb_add_entry to check if the NLM_F_APPEND flag is set
> > >>>>> and if the source is NULL, which represent the bridge itself. Then add
> > >>>>> address to multicast netdev_hw_addr_list for each bridge interfaces.
> > >>>>> And then the .ndo_set_rx_mode function on the driver is called. To notify
> > >>>>> the driver that the list of multicast mac addresses changed.
> > >>>>>
> > >>>>> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > >>>>> ---
> > >>>>>  net/bridge/br_fdb.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
> > >>>>>  1 file changed, 46 insertions(+), 3 deletions(-)
> > >>>>>
> > >>>>
> > >>>> Hi,
> > >>>> I'm sorry but this patch is wrong on many levels, some notes below. In general
> > >>>> NLM_F_APPEND is only used in vxlan, the bridge does not handle that flag at all.
> > >>>> FDB is only for *unicast*, nothing is joined and no multicast should be used with fdbs.
> > >>>> MDB is used for multicast handling, but both of these are used for forwarding.
> > >>>> The reason the static fdbs are added to the filter is for non-promisc ports, so they can
> > >>>> receive traffic destined for these FDBs for forwarding.
> > >>>> If you'd like to join any multicast group please use the standard way, if you'd like to join
> > >>>> it only on a specific port - join it only on that port (or ports) and the bridge and you'll
> > >>>
> > >>> And obviously this is for the case where you're not enabling port promisc mode (non-default).
> > >>> In general you'll only need to join the group on the bridge to receive traffic for it
> > >>> or add it as an mdb entry to forward it.
> > >>>
> > >>>> have the effect that you're describing. What do you mean there's no way ?
> > >>
> > >> Thanks for the explanation.
> > >> There are few things that are not 100% clear to me and maybe you can
> > >> explain them, not to go totally in the wrong direction. Currently I am
> > >> writing a network driver on which I added switchdev support. Then I was
> > >> looking for a way to configure the network driver to copy link layer
> > >> multicast address to the CPU port.
> > >>
> > >> If I am using bridge mdb I can do it only for IP multicast addreses,
> > >> but how should I do it if I want non IP frames with link layer multicast
> > >> address to be copy to CPU? For example: all frames with multicast
> > >> address '01-21-6C-00-00-01' to be copy to CPU. What is the user space
> > >> command for that?
> > >>
> > > 
> > > Check SIOCADDMULTI (ip maddr from iproute2), f.e. add that mac to the port
> > > which needs to receive it and the bridge will send it up automatically since
> > > it's unknown mcast (note that if there's a querier, you'll have to make the
> > > bridge mcast router if it is not the querier itself). It would also flood it to all
> > 
> > Actually you mentioned non-IP traffic, so the querier stuff is not a problem. This
> > traffic will always be flooded by the bridge (and also a copy will be locally sent up).
> > Thus only the flooding may need to be controlled.
> 
> OK, I see, but the part which is not clear to me is, which bridge
> command(from iproute2) to use so the bridge would notify the network
> driver(using swichdev or not) to configure the HW to copy all the frames
> with dmac '01-21-6C-00-00-01' to CPU? So that the bridge can receive
> those frames and then just to pass them up.

Hi Horatiu

Something to keep in mind.

My default, multicast should be flooded, and that includes the CPU
port for a DSA driver. Adding an MDB entry allows for optimisations,
limiting which ports a multicast frame goes out of. But it is just an
optimisation.

	Andrew

  parent reply	other threads:[~2019-07-26 13:46 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-25 11:44 [Bridge] [PATCH] net: bridge: Allow bridge to joing multicast groups Horatiu Vultur
2019-07-25 11:44 ` Horatiu Vultur
2019-07-25 13:06 ` [Bridge] " Nikolay Aleksandrov
2019-07-25 13:06   ` Nikolay Aleksandrov
2019-07-25 13:21   ` [Bridge] " Nikolay Aleksandrov
2019-07-25 13:21     ` Nikolay Aleksandrov
2019-07-25 14:21     ` [Bridge] " Horatiu Vultur
2019-07-25 14:21       ` Horatiu Vultur
2019-07-26  8:41       ` [Bridge] " Nikolay Aleksandrov
2019-07-26  8:41         ` Nikolay Aleksandrov
2019-07-26  9:26         ` [Bridge] " Nikolay Aleksandrov
2019-07-26  9:26           ` Nikolay Aleksandrov
2019-07-26 12:02           ` [Bridge] " Horatiu Vultur
2019-07-26 12:02             ` Horatiu Vultur
2019-07-26 12:31             ` [Bridge] " Nikolay Aleksandrov
2019-07-26 12:31               ` Nikolay Aleksandrov
2019-07-29 12:14               ` [Bridge] " Allan W. Nielsen
2019-07-29 12:14                 ` Allan W. Nielsen
2019-07-29 12:22                 ` [Bridge] " Nikolay Aleksandrov
2019-07-29 12:22                   ` Nikolay Aleksandrov
2019-07-29 12:50                   ` [Bridge] " Nikolay Aleksandrov
2019-07-29 12:50                     ` Nikolay Aleksandrov
2019-07-29 13:14                   ` [Bridge] " Allan W. Nielsen
2019-07-29 13:14                     ` Allan W. Nielsen
2019-07-29 13:42                     ` [Bridge] " Nikolay Aleksandrov
2019-07-29 13:42                       ` Nikolay Aleksandrov
2019-07-29 13:52                       ` [Bridge] " Allan W. Nielsen
2019-07-29 13:52                         ` Allan W. Nielsen
2019-07-29 14:21                         ` [Bridge] " Nikolay Aleksandrov
2019-07-29 14:21                           ` Nikolay Aleksandrov
2019-07-29 14:35                           ` [Bridge] " Allan W. Nielsen
2019-07-29 14:35                             ` Allan W. Nielsen
2019-07-29 17:51                             ` [Bridge] " Ido Schimmel
2019-07-29 17:51                               ` Ido Schimmel
2019-07-30  6:27                               ` [Bridge] " Allan W. Nielsen
2019-07-30  6:27                                 ` Allan W. Nielsen
2019-07-30  7:06                                 ` [Bridge] " Ido Schimmel
2019-07-30  7:06                                   ` Ido Schimmel
2019-07-30  8:30                                   ` [Bridge] " Allan W. Nielsen
2019-07-30  8:30                                     ` Allan W. Nielsen
2019-07-30  8:58                                     ` [Bridge] " Nikolay Aleksandrov
2019-07-30  8:58                                       ` Nikolay Aleksandrov
2019-07-30  9:21                                       ` [Bridge] " Allan W. Nielsen
2019-07-30  9:21                                         ` Allan W. Nielsen
2019-07-30  9:55                                         ` [Bridge] " Nikolay Aleksandrov
2019-07-30  9:55                                           ` Nikolay Aleksandrov
2019-07-30 11:23                                           ` [Bridge] " Allan W. Nielsen
2019-07-30 11:23                                             ` Allan W. Nielsen
2019-07-30 10:04                                     ` [Bridge] " Ido Schimmel
2019-07-30 10:04                                       ` Ido Schimmel
2019-07-30 12:19                                       ` [Bridge] " Allan W. Nielsen
2019-07-30 12:19                                         ` Allan W. Nielsen
2019-07-30 14:34                                 ` [Bridge] " Andrew Lunn
2019-07-30 14:34                                   ` Andrew Lunn
2019-07-30 19:00                                   ` [Bridge] " Allan W. Nielsen
2019-07-30 19:00                                     ` Allan W. Nielsen
2019-07-31  3:31                                     ` [Bridge] " Andrew Lunn
2019-07-31  3:31                                       ` Andrew Lunn
2019-07-31  8:01                                       ` [Bridge] " Allan W. Nielsen
2019-07-31  8:01                                         ` Allan W. Nielsen
2019-07-31 13:45                                         ` [Bridge] " Andrew Lunn
2019-07-31 13:45                                           ` Andrew Lunn
2019-07-31 19:38                                           ` [Bridge] " Allan W. Nielsen
2019-07-31 19:38                                             ` Allan W. Nielsen
2019-08-01 14:22               ` [Bridge] " Allan W. Nielsen
2019-08-01 14:22                 ` Allan W. Nielsen
2019-07-26 13:46             ` Andrew Lunn [this message]
2019-07-26 13:46               ` Andrew Lunn
2019-07-26 19:50               ` [Bridge] " Allan W. Nielsen
2019-07-26 19:50                 ` Allan W. Nielsen
2019-07-27  3:02                 ` [Bridge] " Andrew Lunn
2019-07-27  3:02                   ` Andrew Lunn
2019-07-28 19:15                   ` [Bridge] " Allan W. Nielsen
2019-07-28 19:15                     ` Allan W. Nielsen
2019-07-28 23:07                     ` [Bridge] " Andrew Lunn
2019-07-28 23:07                       ` Andrew Lunn
2019-07-29  6:09                     ` [Bridge] " Ido Schimmel
2019-07-29  6:09                       ` Ido Schimmel
2019-07-29 12:43                       ` [Bridge] " Allan W. Nielsen
2019-07-29 12:43                         ` Allan W. Nielsen
2019-08-01 19:17 ` [Bridge] " Vivien Didelot
2019-08-01 19:17   ` Vivien Didelot
2019-08-01 19:48   ` [Bridge] " Horatiu Vultur
2019-08-01 19:48     ` Horatiu Vultur
2019-08-01 20:08     ` [Bridge] " Vivien Didelot
2019-08-01 20:08       ` Vivien Didelot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190726134613.GD18223@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=allan.nielsen@microchip.com \
    --cc=bridge@lists.linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=horatiu.vultur@microchip.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nikolay@cumulusnetworks.com \
    --cc=roopa@cumulusnetworks.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.