netdev - Re: [PATCH ethtool-next 5/5] pause: add support for dumping statistics

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200923163632.43269739@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Wed, 23 Sep 2020 16:36:32 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Michal Kubecek <mkubecek@...e.cz>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH ethtool-next 5/5] pause: add support for dumping
 statistics

On Thu, 24 Sep 2020 00:45:10 +0200 Michal Kubecek wrote:
> > +		if (mnl_attr_validate(tb[stats[i].attr], MNL_TYPE_U64)) {
> > +			fprintf(stderr, "malformed netlink message (statistic)\n");
> > +			goto err_close_stats;
> > +		}
> > +
> > +		n = snprintf(fmt, sizeof(fmt), "  %s: %%" PRId64 "\n",
> > +			     stats[i].name);  
> 
> The stats are unsigned so the format should be PRIu64 here.

Good catch.

> > @@ -173,8 +236,9 @@ int nl_gpause(struct cmd_context *ctx)
> >  		return 1;
> >  	}
> >  
> > +	flags = nlctx->ctx->show_stats ? ETHTOOL_FLAG_STATS : 0;
> >  	ret = nlsock_prep_get_request(nlsk, ETHTOOL_MSG_PAUSE_GET,
> > -				      ETHTOOL_A_PAUSE_HEADER, 0);
> > +				      ETHTOOL_A_PAUSE_HEADER, flags);
> >  	if (ret < 0)
> >  		return ret;
> >    
> 
> When the stats are supported by kernel but not provided by a device,
> the request will succeed and usual output without stats will be shown.
> However, when stats are requested on a pre-5.10 kernel not recognizing
> ETHTOOL_FLAG_STATS, the request will fail:
> 
>     mike@...n:~/work/git/ethtool> ./ethtool --debug 0x10 -I -a eth0  
>     netlink error: unrecognized request flags
>     netlink error: Operation not supported
>     offending message and attribute:
>         ETHTOOL_MSG_PAUSE_GET
>             ETHTOOL_A_PAUSE_HEADER
>                 ETHTOOL_A_HEADER_DEV_NAME = "eth0"
>     ===>        ETHTOOL_A_HEADER_FLAGS = 0x00000004  
> 
> We should probably repeat the request with flags=0 in this case but that
> would require keeping the offset of ETHTOOL_A_HEADER_FLAGS attribute and
> checking for -EOPNOTSUPP with this offset in nlsock_process_ack().

Makes sense, will do.