[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAM0EoMk_XOeMPXf8o165Z5UGGFHufKO4+_q8UxdVnS3z_imVpg@mail.gmail.com>
Date: Sat, 25 Feb 2023 11:41:48 -0500
From: Jamal Hadi Salim <jhs@...atatu.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: stephen@...workplumber.org, dsahern@...il.com,
netdev@...r.kernel.org, johannes@...solutions.net
Subject: Re: [PATCH iproute2] genl: print caps for all families
On Fri, Feb 24, 2023 at 7:38 PM Jakub Kicinski <kuba@...nel.org> 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.
>
> In fact the entire version check is not strictly necessary.
> The code is not using the old attributes, so on older kernels
> it will simply print nothing either way.
>
> Families can't use flags for random things, 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
>
> Fixes: 26328fc3933f ("Add controller support for new features exposed")
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
> genl/ctrl.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/genl/ctrl.c b/genl/ctrl.c
> index a2d87af0ad07..8d2e944802ba 100644
> --- a/genl/ctrl.c
> +++ b/genl/ctrl.c
> @@ -57,7 +57,7 @@ static void print_ctrl_cmd_flags(FILE *fp, __u32 fl)
> fprintf(fp, "\n");
> }
>
> -static int print_ctrl_cmds(FILE *fp, struct rtattr *arg, __u32 ctrl_ver)
> +static int print_ctrl_cmds(FILE *fp, struct rtattr *arg)
> {
> struct rtattr *tb[CTRL_ATTR_OP_MAX + 1];
>
> @@ -70,7 +70,7 @@ static int print_ctrl_cmds(FILE *fp, struct rtattr *arg, __u32 ctrl_ver)
> fprintf(fp, " ID-0x%x ",*id);
> }
> /* we are only gonna do this for newer version of the controller */
> - if (tb[CTRL_ATTR_OP_FLAGS] && ctrl_ver >= 0x2) {
> + if (tb[CTRL_ATTR_OP_FLAGS]) {
> __u32 *fl = RTA_DATA(tb[CTRL_ATTR_OP_FLAGS]);
> print_ctrl_cmd_flags(fp, *fl);
> }
> @@ -78,7 +78,7 @@ static int print_ctrl_cmds(FILE *fp, struct rtattr *arg, __u32 ctrl_ver)
>
> }
>
> -static int print_ctrl_grp(FILE *fp, struct rtattr *arg, __u32 ctrl_ver)
> +static int print_ctrl_grp(FILE *fp, struct rtattr *arg)
> {
> struct rtattr *tb[CTRL_ATTR_MCAST_GRP_MAX + 1];
>
> @@ -109,7 +109,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 +147,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]);
> @@ -198,7 +196,7 @@ static int print_ctrl(struct rtnl_ctrl_data *ctrl,
> for (i = 0; i < GENL_MAX_FAM_OPS; i++) {
> if (tb2[i]) {
> fprintf(fp, "\t\t#%d: ", i);
> - if (0 > print_ctrl_cmds(fp, tb2[i], ctrl_v)) {
> + if (0 > print_ctrl_cmds(fp, tb2[i])) {
> fprintf(fp, "Error printing command\n");
> }
> /* for next command */
> @@ -221,7 +219,7 @@ static int print_ctrl(struct rtnl_ctrl_data *ctrl,
> for (i = 0; i < GENL_MAX_FAM_GRPS; i++) {
> if (tb2[i]) {
> fprintf(fp, "\t\t#%d: ", i);
> - if (0 > print_ctrl_grp(fp, tb2[i], ctrl_v))
> + if (0 > print_ctrl_grp(fp, tb2[i]))
> fprintf(fp, "Error printing group\n");
> /* for next group */
> fprintf(fp,"\n");
> --
> 2.39.2
>
Reviewed-by: Jamal Hadi Salim <jhs@...atatu.com>
cheers,
jamal
Powered by blists - more mailing lists