netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: stephen@networkplumber.org
Cc: dsahern@gmail.com, jhs@mojatatu.com, netdev@vger.kernel.org,
	Johannes Berg <johannes@sipsolutions.net>
Subject: Re: [PATCH iproute2] genl: print caps for all families
Date: Thu, 23 Feb 2023 17:57:08 -0800	[thread overview]
Message-ID: <20230223175708.51e593f0@kernel.org> (raw)
In-Reply-To: <20230224015234.1626025-1-kuba@kernel.org>

On Thu, 23 Feb 2023 17:52:34 -0800 Jakub Kicinski wrote:
> Back in 2006 kernel commit 334c29a64507 ("[GENETLINK]: Move
> command capabilities to flags.") removed some attributes and
> moved the capabilities to flags. Corresponding iproute2
> commit 26328fc3933f ("Add controller support for new features
> exposed") added the ability to print those caps.
> 
> Printing is gated on version of the family, but we're checking
> the version of each individual family rather than the control
> family. The format of attributes in the control family
> is dictated by the version of the control family alone.
> 
> Families can't use flags for random things, anyway,
> because kernel core has a fixed interpretation.
> 
> Thanks to this change caps will be shown for all families
> (assuming kernel newer than 2.6.19), not just those which
> by coincidence have their local version >= 2.
> 
> For instance devlink, before:
> 
>   $ genl ctrl get name devlink
>   Name: devlink
> 	ID: 0x15  Version: 0x1  header size: 0  max attribs: 179
> 	commands supported:
> 		#1:  ID-0x1
> 		#2:  ID-0x5
> 		#3:  ID-0x6
> 		...
> 
> after:
> 
>   $ genl ctrl get name devlink
>   Name: devlink
> 	ID: 0x15  Version: 0x1  header size: 0  max attribs: 179
> 	commands supported:
> 		#1:  ID-0x1
> 		Capabilities (0xe):
>  		  can doit; can dumpit; has policy
> 
> 		#2:  ID-0x5
> 		Capabilities (0xe):
>  		  can doit; can dumpit; has policy
> 
> 		#3:  ID-0x6
> 		Capabilities (0xb):
>  		  requires admin permission; can doit; has policy
> 
> Leave ctrl_v as 0 if we fail to read the version. Old code used 1
> as the default, but 0 or 1 - does not matter, checks are for >= 2.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> Not really sure if this is a fix or not..

Adding Johannes, that's probably everyone who ever used this 
command on CC? ;)

> diff --git a/genl/ctrl.c b/genl/ctrl.c
> index a2d87af0ad07..1fcb3848f137 100644
> --- a/genl/ctrl.c
> +++ b/genl/ctrl.c
> @@ -21,6 +21,8 @@
>  #define GENL_MAX_FAM_OPS	256
>  #define GENL_MAX_FAM_GRPS	256
>  
> +static unsigned int ctrl_v;
> +
>  static int usage(void)
>  {
>  	fprintf(stderr,"Usage: ctrl <CMD>\n" \
> @@ -109,7 +111,6 @@ static int print_ctrl(struct rtnl_ctrl_data *ctrl,
>  	int len = n->nlmsg_len;
>  	struct rtattr *attrs;
>  	FILE *fp = (FILE *) arg;
> -	__u32 ctrl_v = 0x1;
>  
>  	if (n->nlmsg_type !=  GENL_ID_CTRL) {
>  		fprintf(stderr, "Not a controller message, nlmsg_len=%d "
> @@ -148,7 +149,6 @@ static int print_ctrl(struct rtnl_ctrl_data *ctrl,
>  	if (tb[CTRL_ATTR_VERSION]) {
>  		__u32 *v = RTA_DATA(tb[CTRL_ATTR_VERSION]);
>  		fprintf(fp, " Version: 0x%x ",*v);
> -		ctrl_v = *v;
>  	}
>  	if (tb[CTRL_ATTR_HDRSIZE]) {
>  		__u32 *h = RTA_DATA(tb[CTRL_ATTR_HDRSIZE]);
> @@ -241,6 +241,55 @@ static int print_ctrl2(struct nlmsghdr *n, void *arg)
>  	return print_ctrl(NULL, n, arg);
>  }
>  
> +static void ctrl_load_ctrl_version(struct rtnl_handle *rth)
> +{
> +	struct rtattr *tb[CTRL_ATTR_MAX + 1];
> +	struct genlmsghdr *ghdr;
> +	struct rtattr *attrs;
> +	struct {
> +		struct nlmsghdr         n;
> +		struct genlmsghdr	g;
> +		char                    buf[128];
> +	} req = {
> +		.n.nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN),
> +		.n.nlmsg_flags = NLM_F_REQUEST,
> +		.n.nlmsg_type = GENL_ID_CTRL,
> +		.g.cmd = CTRL_CMD_GETFAMILY,
> +	};
> +	struct nlmsghdr *nlh = &req.n;
> +	struct nlmsghdr *answer = NULL;
> +	__u16 id = GENL_ID_CTRL;
> +	int len;
> +
> +	addattr_l(nlh, 128, CTRL_ATTR_FAMILY_ID, &id, 2);
> +
> +	if (rtnl_talk(rth, nlh, &answer) < 0) {
> +		fprintf(stderr, "Error talking to the kernel\n");
> +		return;
> +	}
> +
> +	ghdr = NLMSG_DATA(answer);
> +	if (answer->nlmsg_type != GENL_ID_CTRL ||
> +	    ghdr->cmd != CTRL_CMD_NEWFAMILY) {
> +		fprintf(stderr, "Unexpected reply nlmsg_type=%u cmd=%u\n",
> +			answer->nlmsg_type, ghdr->cmd);
> +		return;
> +	}
> +
> +	len = answer->nlmsg_len;
> +	len -= NLMSG_LENGTH(GENL_HDRLEN);
> +	if (len < 0) {
> +		fprintf(stderr, "wrong controller message len %d\n", len);
> +		return;
> +	}
> +
> +	attrs = (struct rtattr *) ((char *) ghdr + GENL_HDRLEN);
> +	parse_rtattr_flags(tb, CTRL_ATTR_MAX, attrs, len, NLA_F_NESTED);
> +
> +	if (tb[CTRL_ATTR_VERSION])
> +		ctrl_v = rta_getattr_u16(tb[CTRL_ATTR_VERSION]);
> +}
> +
>  static int ctrl_list(int cmd, int argc, char **argv)
>  {
>  	struct rtnl_handle rth;
> @@ -264,6 +313,9 @@ static int ctrl_list(int cmd, int argc, char **argv)
>  		exit(1);
>  	}
>  
> +	if (!ctrl_v)
> +		ctrl_load_ctrl_version(&rth);
> +
>  	if (cmd == CTRL_CMD_GETFAMILY || cmd == CTRL_CMD_GETPOLICY) {
>  		req.g.cmd = cmd;
>  


  reply	other threads:[~2023-02-24  1:57 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-24  1:52 [PATCH iproute2] genl: print caps for all families Jakub Kicinski
2023-02-24  1:57 ` Jakub Kicinski [this message]
2023-02-24  8:33   ` Johannes Berg
2023-02-24 15:15     ` Jamal Hadi Salim
2023-02-24 15:22       ` Jamal Hadi Salim
2023-02-24 17:10         ` Jakub Kicinski
2023-02-24 17:46           ` Jamal Hadi Salim
2023-02-24 18:30             ` Jakub Kicinski
2023-02-24  3:27 ` Stephen Hemminger
2023-02-24 17:11   ` Jakub Kicinski
2023-02-24 17:47     ` Jamal Hadi Salim
2023-02-24 18:29       ` Jakub Kicinski
2023-02-24 22:33         ` Stephen Hemminger
2023-02-25  0:56           ` Jakub Kicinski
2023-02-25 17:21             ` Jamal Hadi Salim
  -- strict thread matches above, loose matches on Subject: below --
2023-02-25  0:37 Jakub Kicinski
2023-02-25 16:41 ` Jamal Hadi Salim
2023-03-04  2:20 ` patchwork-bot+netdevbpf

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=20230223175708.51e593f0@kernel.org \
    --to=kuba@kernel.org \
    --cc=dsahern@gmail.com \
    --cc=jhs@mojatatu.com \
    --cc=johannes@sipsolutions.net \
    --cc=netdev@vger.kernel.org \
    --cc=stephen@networkplumber.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).