From: Johannes Berg <johannes@sipsolutions.net>
To: Jakub Kicinski <kuba@kernel.org>, davem@davemloft.net
Cc: netdev@vger.kernel.org, andrew@lunn.ch, jiri@resnulli.us,
mkubecek@suse.cz, dsahern@kernel.org, pablo@netfilter.org
Subject: Re: [PATCH net-next 9/9] genetlink: allow dumping command-specific policy
Date: Thu, 01 Oct 2020 23:03:01 +0200 [thread overview]
Message-ID: <dcaeaedaa179c558cab0d417277fea9ac29d8b79.camel@sipsolutions.net> (raw)
In-Reply-To: <20201001183016.1259870-10-kuba@kernel.org>
On Thu, 2020-10-01 at 11:30 -0700, Jakub Kicinski wrote:
>
> +++ b/net/netlink/genetlink.c
> @@ -1119,6 +1119,7 @@ static const struct nla_policy ctrl_policy_policy[] = {
> [CTRL_ATTR_FAMILY_ID] = { .type = NLA_U16 },
> [CTRL_ATTR_FAMILY_NAME] = { .type = NLA_NUL_STRING,
> .len = GENL_NAMSIZ - 1 },
> + [CTRL_ATTR_OP] = { .type = NLA_U8 },
I slightly wonder if we shouldn't make this wider. There's no real
*benefit* to using a u8 since, due to padding, the attribute actually
has the same size anyway (up to U32), and we also still need to validate
it actually exists.
However, if we ever run out of command numbers in some family (nl80211
is almost half way :-) ) then I believe we still have some reserved
space in the genl header that we could use for extensions, but if then
we have to also go around and change a bunch of other interfaces like
this, it'll just be so much harder, and ... we'd probably just be
tempted to multiplex onto an "extension command" or something? Or maybe
then we should have a separate genl family at that point?
(Internal interfaces are much easier to change)
Dunno. Just a thought. We probably won't ever get there, it just sort of
rubs me the wrong way that we'd be restricting this in an "unfixable"
API for no real reason :)
> - if (!rt->policy)
> + if (tb[CTRL_ATTR_OP]) {
> + err = genl_get_cmd(nla_get_u8(tb[CTRL_ATTR_OP]), rt, &op);
OK, so maybe if we make that wider we also have to make the argument to
genl_get_cmd() wider so we don't erroneously return op 1 if you ask for
257, but that's in an at least 32-bit register anyway, presumably?
johannes
next prev parent reply other threads:[~2020-10-01 21:03 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-01 18:30 [PATCH net-next 0/9] genetlink: support per-command policy dump Jakub Kicinski
2020-10-01 18:30 ` [PATCH net-next 1/9] genetlink: reorg struct genl_family Jakub Kicinski
2020-10-01 18:30 ` [PATCH net-next 2/9] genetlink: add small version of ops Jakub Kicinski
2020-10-01 18:30 ` [PATCH net-next 3/9] genetlink: move to smaller ops wherever possible Jakub Kicinski
2020-10-01 18:30 ` [PATCH net-next 4/9] genetlink: add a structure for dump state Jakub Kicinski
2020-10-01 20:46 ` Johannes Berg
2020-10-01 18:30 ` [PATCH net-next 5/9] genetlink: use .start callback for dumppolicy Jakub Kicinski
2020-10-01 18:30 ` [PATCH net-next 6/9] genetlink: bring back per op policy Jakub Kicinski
2020-10-01 18:30 ` [PATCH net-next 7/9] taskstats: move specifying netlink policy back to ops Jakub Kicinski
2020-10-01 20:48 ` Johannes Berg
2020-10-01 18:30 ` [PATCH net-next 8/9] genetlink: use per-op policy for CTRL_CMD_GETPOLICY Jakub Kicinski
2020-10-01 20:55 ` Johannes Berg
2020-10-01 21:09 ` Jakub Kicinski
2020-10-01 21:10 ` Johannes Berg
2020-10-01 18:30 ` [PATCH net-next 9/9] genetlink: allow dumping command-specific policy Jakub Kicinski
2020-10-01 21:03 ` Johannes Berg [this message]
2020-10-01 21:11 ` Jakub Kicinski
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=dcaeaedaa179c558cab0d417277fea9ac29d8b79.camel@sipsolutions.net \
--to=johannes@sipsolutions.net \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=jiri@resnulli.us \
--cc=kuba@kernel.org \
--cc=mkubecek@suse.cz \
--cc=netdev@vger.kernel.org \
--cc=pablo@netfilter.org \
/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 NNTP newsgroup(s).