All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ido Schimmel <idosch@nvidia.com>
To: Nikolay Aleksandrov <razor@blackwall.org>
Cc: netdev@vger.kernel.org, bridge@lists.linux-foundation.org,
	edumazet@google.com, mlxsw@nvidia.com, roopa@nvidia.com,
	kuba@kernel.org, pabeni@redhat.com, davem@davemloft.net
Subject: Re: [Bridge] [PATCH net-next 09/14] bridge: mcast: Add support for (*, G) with a source list and filter mode
Date: Sat, 10 Dec 2022 15:33:09 +0200	[thread overview]
Message-ID: <Y5SKlVl6cuSp1Ib1@shredder> (raw)
In-Reply-To: <22583106-8f39-0c7e-1c61-47ec5c614418@blackwall.org>

On Fri, Dec 09, 2022 at 09:41:05AM +0200, Nikolay Aleksandrov wrote:
> On 08/12/2022 17:28, Ido Schimmel wrote:
> > In preparation for allowing user space to add (*, G) entries with a
> > source list and associated filter mode, add the necessary plumbing to
> > handle such requests.
> > 
> > Extend the MDB configuration structure with a currently empty source
> > array and filter mode that is currently hard coded to EXCLUDE.
> > 
> > Add the source entries and the corresponding (S, G) entries before
> > making the new (*, G) port group entry visible to the data path.
> > 
> > Handle the creation of each source entry in a similar fashion to how it
> > is created from the data path in response to received Membership
> > Reports: Create the source entry, arm the source timer (if needed), add
> > a corresponding (S, G) forwarding entry and finally mark the source
> > entry as installed (by user space).
> > 
> > Add the (S, G) entry by populating an MDB configuration structure and
> > calling br_mdb_add_group_sg() as if a new entry is created by user
> > space, with the sole difference that the 'src_entry' field is set to
> > make sure that the group timer of such entries is never armed.
> > 
> > Note that it is not currently possible to add more than 32 source
> > entries to a port group entry. If this proves to be a problem we can
> > either increase 'PG_SRC_ENT_LIMIT' or avoid forcing a limit on entries
> > created by user space.
> > 
> 
> That can be tricky wrt EHT. If the limit is increased we have to consider the
> complexity and runtime, we might have to optimize it. In practice I think it's
> rare to have so many sources, but evpn might change that. :)

Yea, we don't currently have data as to whether this limit is OK or not.
Once we do we can make a more informed decision. Some options:

1. Slightly increase the current limit.
2. Remove the limit and move to an RB tree instead of a list.
3. Only install (*, G) EXCLUDE entries on the VXLAN port and let the
VXLAN MDB do more fine-grained filtering.

> 
> > Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> > ---
> > 
> > Notes:
> >     v1:
> >     * Use an array instead of a list to store source entries.
> > 
> >  net/bridge/br_mdb.c     | 128 +++++++++++++++++++++++++++++++++++++++-
> >  net/bridge/br_private.h |   7 +++
> >  2 files changed, 132 insertions(+), 3 deletions(-)
> > 
> 
> Acked-by: Nikolay Aleksandrov <razor@blackwall.org>

Thanks!

WARNING: multiple messages have this Message-ID (diff)
From: Ido Schimmel <idosch@nvidia.com>
To: Nikolay Aleksandrov <razor@blackwall.org>
Cc: netdev@vger.kernel.org, bridge@lists.linux-foundation.org,
	davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com,
	edumazet@google.com, roopa@nvidia.com, mlxsw@nvidia.com
Subject: Re: [PATCH net-next 09/14] bridge: mcast: Add support for (*, G) with a source list and filter mode
Date: Sat, 10 Dec 2022 15:33:09 +0200	[thread overview]
Message-ID: <Y5SKlVl6cuSp1Ib1@shredder> (raw)
In-Reply-To: <22583106-8f39-0c7e-1c61-47ec5c614418@blackwall.org>

On Fri, Dec 09, 2022 at 09:41:05AM +0200, Nikolay Aleksandrov wrote:
> On 08/12/2022 17:28, Ido Schimmel wrote:
> > In preparation for allowing user space to add (*, G) entries with a
> > source list and associated filter mode, add the necessary plumbing to
> > handle such requests.
> > 
> > Extend the MDB configuration structure with a currently empty source
> > array and filter mode that is currently hard coded to EXCLUDE.
> > 
> > Add the source entries and the corresponding (S, G) entries before
> > making the new (*, G) port group entry visible to the data path.
> > 
> > Handle the creation of each source entry in a similar fashion to how it
> > is created from the data path in response to received Membership
> > Reports: Create the source entry, arm the source timer (if needed), add
> > a corresponding (S, G) forwarding entry and finally mark the source
> > entry as installed (by user space).
> > 
> > Add the (S, G) entry by populating an MDB configuration structure and
> > calling br_mdb_add_group_sg() as if a new entry is created by user
> > space, with the sole difference that the 'src_entry' field is set to
> > make sure that the group timer of such entries is never armed.
> > 
> > Note that it is not currently possible to add more than 32 source
> > entries to a port group entry. If this proves to be a problem we can
> > either increase 'PG_SRC_ENT_LIMIT' or avoid forcing a limit on entries
> > created by user space.
> > 
> 
> That can be tricky wrt EHT. If the limit is increased we have to consider the
> complexity and runtime, we might have to optimize it. In practice I think it's
> rare to have so many sources, but evpn might change that. :)

Yea, we don't currently have data as to whether this limit is OK or not.
Once we do we can make a more informed decision. Some options:

1. Slightly increase the current limit.
2. Remove the limit and move to an RB tree instead of a list.
3. Only install (*, G) EXCLUDE entries on the VXLAN port and let the
VXLAN MDB do more fine-grained filtering.

> 
> > Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> > ---
> > 
> > Notes:
> >     v1:
> >     * Use an array instead of a list to store source entries.
> > 
> >  net/bridge/br_mdb.c     | 128 +++++++++++++++++++++++++++++++++++++++-
> >  net/bridge/br_private.h |   7 +++
> >  2 files changed, 132 insertions(+), 3 deletions(-)
> > 
> 
> Acked-by: Nikolay Aleksandrov <razor@blackwall.org>

Thanks!

  reply	other threads:[~2022-12-10 13:33 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-08 15:28 [Bridge] [PATCH net-next 00/14] bridge: mcast: Extensions for EVPN Ido Schimmel
2022-12-08 15:28 ` Ido Schimmel
2022-12-08 15:28 ` [Bridge] [PATCH net-next 01/14] bridge: mcast: Do not derive entry type from its filter mode Ido Schimmel
2022-12-08 15:28   ` Ido Schimmel
2022-12-09  7:27   ` [Bridge] " Nikolay Aleksandrov
2022-12-09  7:27     ` Nikolay Aleksandrov
2022-12-08 15:28 ` [Bridge] [PATCH net-next 02/14] bridge: mcast: Split (*, G) and (S, G) addition into different functions Ido Schimmel
2022-12-08 15:28   ` Ido Schimmel
2022-12-09  7:29   ` [Bridge] " Nikolay Aleksandrov
2022-12-09  7:29     ` Nikolay Aleksandrov
2022-12-08 15:28 ` [Bridge] [PATCH net-next 03/14] bridge: mcast: Place netlink policy before validation functions Ido Schimmel
2022-12-08 15:28   ` Ido Schimmel
2022-12-09  7:30   ` [Bridge] " Nikolay Aleksandrov
2022-12-09  7:30     ` Nikolay Aleksandrov
2022-12-08 15:28 ` [Bridge] [PATCH net-next 04/14] bridge: mcast: Add a centralized error path Ido Schimmel
2022-12-08 15:28   ` Ido Schimmel
2022-12-09  7:30   ` [Bridge] " Nikolay Aleksandrov
2022-12-09  7:30     ` Nikolay Aleksandrov
2022-12-08 15:28 ` [Bridge] [PATCH net-next 05/14] bridge: mcast: Expose br_multicast_new_group_src() Ido Schimmel
2022-12-08 15:28   ` Ido Schimmel
2022-12-09  7:30   ` [Bridge] " Nikolay Aleksandrov
2022-12-09  7:30     ` Nikolay Aleksandrov
2022-12-08 15:28 ` [Bridge] [PATCH net-next 06/14] bridge: mcast: Expose __br_multicast_del_group_src() Ido Schimmel
2022-12-08 15:28   ` Ido Schimmel
2022-12-09  7:31   ` [Bridge] " Nikolay Aleksandrov
2022-12-09  7:31     ` Nikolay Aleksandrov
2022-12-08 15:28 ` [Bridge] [PATCH net-next 07/14] bridge: mcast: Add a flag for user installed source entries Ido Schimmel
2022-12-08 15:28   ` Ido Schimmel
2022-12-09  7:32   ` [Bridge] " Nikolay Aleksandrov
2022-12-09  7:32     ` Nikolay Aleksandrov
2022-12-08 15:28 ` [Bridge] [PATCH net-next 08/14] bridge: mcast: Avoid arming group timer when (S, G) corresponds to a source Ido Schimmel
2022-12-08 15:28   ` Ido Schimmel
2022-12-09  7:32   ` [Bridge] " Nikolay Aleksandrov
2022-12-09  7:32     ` Nikolay Aleksandrov
2022-12-08 15:28 ` [Bridge] [PATCH net-next 09/14] bridge: mcast: Add support for (*, G) with a source list and filter mode Ido Schimmel
2022-12-08 15:28   ` Ido Schimmel
2022-12-09  7:41   ` [Bridge] " Nikolay Aleksandrov
2022-12-09  7:41     ` Nikolay Aleksandrov
2022-12-10 13:33     ` Ido Schimmel [this message]
2022-12-10 13:33       ` Ido Schimmel
2022-12-08 15:28 ` [Bridge] [PATCH net-next 10/14] bridge: mcast: Allow user space to add " Ido Schimmel
2022-12-08 15:28   ` Ido Schimmel
2022-12-09  7:52   ` [Bridge] " Nikolay Aleksandrov
2022-12-09  7:52     ` Nikolay Aleksandrov
2022-12-08 15:28 ` [Bridge] [PATCH net-next 11/14] bridge: mcast: Allow user space to specify MDB entry routing protocol Ido Schimmel
2022-12-08 15:28   ` Ido Schimmel
2022-12-09  7:53   ` [Bridge] " Nikolay Aleksandrov
2022-12-09  7:53     ` Nikolay Aleksandrov
2022-12-08 15:28 ` [Bridge] [PATCH net-next 12/14] bridge: mcast: Support replacement of MDB port group entries Ido Schimmel
2022-12-08 15:28   ` Ido Schimmel
2022-12-09  8:08   ` [Bridge] " Nikolay Aleksandrov
2022-12-09  8:08     ` Nikolay Aleksandrov
2022-12-10 13:59     ` [Bridge] " Ido Schimmel
2022-12-10 13:59       ` Ido Schimmel
2022-12-08 15:28 ` [Bridge] [PATCH net-next 13/14] selftests: forwarding: Rename bridge_mdb test Ido Schimmel
2022-12-08 15:28   ` Ido Schimmel
2022-12-09  8:09   ` [Bridge] " Nikolay Aleksandrov
2022-12-09  8:09     ` Nikolay Aleksandrov
2022-12-08 15:28 ` [Bridge] [PATCH net-next 14/14] selftests: forwarding: Add bridge MDB test Ido Schimmel
2022-12-08 15:28   ` Ido Schimmel
2022-12-09  8:09   ` [Bridge] " Nikolay Aleksandrov
2022-12-09  8:09     ` Nikolay Aleksandrov

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=Y5SKlVl6cuSp1Ib1@shredder \
    --to=idosch@nvidia.com \
    --cc=bridge@lists.linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=mlxsw@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=razor@blackwall.org \
    --cc=roopa@nvidia.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.