netdev - Re: [PATCH v2 net-next 1/3] net: dsa: lan9303: Refactor lan9303_xxx_packet

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]

Date:   Tue, 1 Aug 2017 16:02:03 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Egil Hjelmeland <privat@...l-hjelmeland.no>
Cc:     vivien.didelot@...oirfairelinux.com, f.fainelli@...il.com,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel@...gutronix.de
Subject: Re: [PATCH v2 net-next 1/3] net: dsa: lan9303: Refactor
 lan9303_xxx_packet_processing()

On Tue, Aug 01, 2017 at 03:50:14PM +0200, Egil Hjelmeland wrote:
> On 01. aug. 2017 15:39, Andrew Lunn wrote:
> >>@@ -704,7 +710,7 @@ static void lan9303_get_ethtool_stats(struct dsa_switch *ds, int port,
> >>  	unsigned int u, poff;
> >>  	int ret;
> >>-	poff = port * 0x400;
> >>+	poff = LAN9303_SWITCH_PORT_REG(port, 0);
> >>  	for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) {
> >>  		ret = lan9303_read_switch_reg(chip,
> >
> >So the actual code is:
> >
> >	for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) {
> >        		ret = lan9303_read_switch_reg(chip,
> >			      		      lan9303_mib[u].offset + poff,
> >					      &reg);
> >
> >Could this be written as
> >
> >	for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) {
> >		ret = lan9303_read_switch_port(chip, port, lan9303_mib[u].offset, &reg);
> >
> >It is then clear you are reading the statistics from a port register.
> >
> >    Andrew
> >
> 
> Yes it can. Since it is (insignificantly) less efficient, I
> chose not to touch it. But I can do it if you like.

I doubt it is less efficient. The compiler has seen
lan9303_read_switch_port() and will probably inline it.  So what the
optimiser gets to see is probably the same in both cases.

Try generating the assembler listing in both cases, and compare them

make drivers/net/dsa/lan9303-core.lst

     Andrew