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] [RFC PATCH net-next 17/19] bridge: mcast: Allow user space to add (*, G) with a source list and filter mode
Date: Thu, 3 Nov 2022 11:09:30 +0200 [thread overview]
Message-ID: <Y2OFSq7AZRARG9ov@shredder> (raw)
In-Reply-To: <e3a74c46-0542-4f21-4975-5bd22bb62ab9@blackwall.org>
On Wed, Oct 19, 2022 at 04:28:23PM +0300, Nikolay Aleksandrov wrote:
> On 18/10/2022 15:04, Ido Schimmel wrote:
> > +static int br_mdb_config_src_list_init(struct nlattr *src_list,
> > + struct br_mdb_config *cfg,
> > + struct netlink_ext_ack *extack)
> > +{
> > + struct br_mdb_src_entry *src, *tmp;
> > + struct nlattr *src_entry;
> > + int rem, err;
> > +
> > + nla_for_each_nested(src_entry, src_list, rem) {
> > + err = br_mdb_config_src_entry_init(src_entry, cfg, extack);
>
> Hmm, since we know the exact number of these (due to attr embedding) can't we allocate
> all at once and drop the list? They should not be more than 32 (PG_SRC_ENT_LIMIT) IIRC,
> which makes it at most 1152 bytes. Might simplify the code a bit and reduce allocations.
I didn't see how I can reliably determine the exact number of source
entries without going all the 'MDBE_SRC_LIST_ENTRY' attributes. I mean,
the entries can have varying sizes in case user space provided mixed
IPv4/IPv6 sources (which will be rejected later on) and in the future we
might have more attributes per-source entry other than the address
(e.g., source timer), which might be specified only for a subset of the
entries.
So, I did end up converting to an array like you suggested, but I'm
going over the entries twice. Once to understand how large the array
should be and again to initialize it. It's control path so it should be
fine. The advantages are that the number of allocations are reduced and
that I can reject a too long source list before doing any processing:
if (cfg->num_src_entries >= PG_SRC_ENT_LIMIT) {
NL_SET_ERR_MSG_FMT_MOD(extack, "Exceeded maximum number of source entries (%u)",
PG_SRC_ENT_LIMIT);
return -EINVAL;
}
[...]
> > +static void br_mdb_config_fini(struct br_mdb_config *cfg)
> > +{
> > + br_mdb_config_attrs_fini(cfg);
> > +}
> > +
>
> Is there more coming to these two _fini helpers? If not, I think one would be enough, i.e.
> just call br_mdb_config_src_list_fini() from br_mdb_config_fini()
Done.
Thanks!
next prev parent reply other threads:[~2022-11-03 9:09 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-18 12:04 [Bridge] [RFC PATCH net-next 00/19] bridge: mcast: Extensions for EVPN Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 01/19] bridge: mcast: Centralize netlink attribute parsing Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 02/19] bridge: mcast: Remove redundant checks Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 03/19] bridge: mcast: Use MDB configuration structure where possible Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 04/19] bridge: mcast: Propagate MDB configuration structure further Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 05/19] bridge: mcast: Use MDB group key from configuration structure Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 06/19] bridge: mcast: Remove br_mdb_parse() Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 07/19] bridge: mcast: Move checks out of critical section Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 08/19] bridge: mcast: Remove redundant function arguments Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 09/19] bridge: mcast: Do not derive entry type from its filter mode Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 10/19] bridge: mcast: Split (*, G) and (S, G) addition into different functions Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 11/19] bridge: mcast: Place netlink policy before validation functions Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 12/19] bridge: mcast: Add a centralized error path Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 13/19] bridge: mcast: Expose br_multicast_new_group_src() Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 14/19] bridge: mcast: Add a flag for user installed source entries Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 15/19] bridge: mcast: Avoid arming group timer when (S, G) corresponds to a source Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 16/19] bridge: mcast: Add support for (*, G) with a source list and filter mode Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 17/19] bridge: mcast: Allow user space to add " Ido Schimmel
2022-10-19 13:28 ` Nikolay Aleksandrov
2022-11-03 9:09 ` Ido Schimmel [this message]
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 18/19] bridge: mcast: Allow user space to specify MDB entry routing protocol Ido Schimmel
2022-10-18 12:04 ` [Bridge] [RFC PATCH net-next 19/19] bridge: mcast: Support replacement of MDB port group entries Ido Schimmel
2022-10-18 19:21 ` [Bridge] [RFC PATCH net-next 00/19] bridge: mcast: Extensions for EVPN Jakub Kicinski
2022-10-25 10:53 ` Ido Schimmel
2022-10-19 13:15 ` 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=Y2OFSq7AZRARG9ov@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).