All of lore.kernel.org
 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 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.