netdev - RE: [PATCH net-next 06/11] net: dsa: debugfs: add port registers

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: <9235D6609DB808459E95D78E17F2E43D40AFD219@CHN-SV-EXMX02.mchp-main.com>
Date:   Tue, 15 Aug 2017 16:28:12 +0000
From:   <Woojung.Huh@...rochip.com>
To:     <vivien.didelot@...oirfairelinux.com>, <netdev@...r.kernel.org>
CC:     <linux-kernel@...r.kernel.org>, <kernel@...oirfairelinux.com>,
        <davem@...emloft.net>, <f.fainelli@...il.com>, <andrew@...n.ch>,
        <privat@...l-hjelmeland.no>, <john@...ozen.org>,
        <sean.wang@...iatek.com>, <volodymyr.bendiuga@...il.com>,
        <nikita.yoush@...entembedded.com>, <maxime@...ialet.com>,
        <cphealy@...il.com>, <muvarov@...il.com>, <eichest@...il.com>,
        <jcobham@...stertangent.com>, <jbe@...gutronix.de>,
        <tobias@...dekranz.com>
Subject: RE: [PATCH net-next 06/11] net: dsa: debugfs: add port registers

Vivien,

> Subject: [PATCH net-next 06/11] net: dsa: debugfs: add port registers
> 
> Add a debug filesystem "regs" entry to query a port's hardware registers
> through the .get_regs_len and .get_regs_len switch operations.
> 
> This is very convenient because it allows one to dump the registers of
> DSA links, which are not exposed to userspace.
This series will be very useful to get various debug information.
Do you have any plan to expand 32bit register access and switch-level registers
In addition to port-level registers?

> +static void dsa_debugfs_regs_read_count(struct dsa_switch *ds, int id,
> +					struct seq_file *seq, int count)
> +{
> +	u16 data[count * ETH_GSTRING_LEN];
I think this should be u16 data[count] because it is value of registers.

> +	struct ethtool_regs regs;
> +	int i;
> +
> +	ds->ops->get_regs(ds, id, &regs, data);
> +
> +	for (i = 0; i < count / 2; i++)
> +		seq_printf(seq, "%2d: %04x\n", i, data[i]);
> +}
> +
> +static int dsa_debugfs_regs_read(struct dsa_switch *ds, int id,
> +				 struct seq_file *seq)
> +{
> +	int count;
> +
> +	if (!ds->ops->get_regs_len || !ds->ops->get_regs)
> +		return -EOPNOTSUPP;
> +
> +	count = ds->ops->get_regs_len(ds, id);
> +	if (count < 0)
> +		return count;
Because get_regs_len returns length than count per mv88e6xxx/chip.c,
it requires some math before passing to dsa_debugfs_regs_read_count().

It does "count/2" in dsa-debugfs_regs_read_count(), however,
As commented above, "count" is used at "u16 data[...]".
So, it would be nice to match with name. Ie, count or size/length.

Thanks.
Woojung