bridge.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Petr Machata <petrm@nvidia.com>
To: Johannes Nixdorf <jnixdorf-oss@avm.de>
Cc: David Ahern <dsahern@gmail.com>, Ido Schimmel <idosch@nvidia.com>,
	Nikolay Aleksandrov <razor@blackwall.org>,
	bridge@lists.linux-foundation.org, netdev@vger.kernel.org,
	Roopa Prabhu <roopa@nvidia.com>
Subject: Re: [Bridge] [PATCH iproute2-next v3] iplink: bridge: Add support for bridge FDB learning limits
Date: Wed, 6 Sep 2023 12:32:32 +0200	[thread overview]
Message-ID: <87msxzmv5q.fsf@nvidia.com> (raw)
In-Reply-To: <20230905-fdb_limit-v3-1-34bb124556d8@avm.de>

(I pruned the CC list, hopefully I didn't leave out anybody who cares.)

Johannes Nixdorf via Bridge <bridge@lists.linux-foundation.org> writes:

> Support setting the FDB limit through ip link. The arguments is:
>  - fdb_max_learned_entries: A 32-bit unsigned integer specifying the
>                             maximum number of learned FDB entries, with 0
>                             disabling the limit.

...

> Signed-off-by: Johannes Nixdorf <jnixdorf-oss@avm.de>

Code looks good to me. A couple points though:

- The corresponding kernel changes have not yet been merged, have they?
  This patch should obviously only be merged after that happens.

- The UAPI changes should not be part of the patch, the maintainers will
  update themselves.

- The names fdb_n_learned_entries, fdb_max_learned_entries... they are
  somewhat unwieldy IMHO. Just for consideration, I don't feel strongly
  about this:

  Your kconfig option is named BRIDGE_DEFAULT_FDB_MAX_LEARNED, and that
  makes sense to me, because yeah, given the word "learned" in context
  of FDB, "entries" is the obvious continuation, so why mention it
  explicitly. Consider following suit with the other source artifacts --
  the attribute names, struct fields, keywords in this patch.
  "fdb_n_learned", "fdb_max_learned" is IMHO just as understandable and
  will be easier to type.

      reply	other threads:[~2023-09-06 10:32 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-05 11:47 [Bridge] [PATCH iproute2-next v3] iplink: bridge: Add support for bridge FDB learning limits Johannes Nixdorf
2023-09-06 10:32 ` Petr Machata [this message]

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=87msxzmv5q.fsf@nvidia.com \
    --to=petrm@nvidia.com \
    --cc=bridge@lists.linux-foundation.org \
    --cc=dsahern@gmail.com \
    --cc=idosch@nvidia.com \
    --cc=jnixdorf-oss@avm.de \
    --cc=netdev@vger.kernel.org \
    --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).