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
| ||
|
Date: Thu, 15 Sep 2022 22:39:11 +0200 From: Andrew Lunn <andrew@...n.ch> To: Mattias Forsblad <mattias.forsblad@...il.com> Cc: netdev@...r.kernel.org, Vivien Didelot <vivien.didelot@...il.com>, Florian Fainelli <f.fainelli@...il.com>, Vladimir Oltean <olteanv@...il.com>, "David S . Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, linux@...linux.org.uk, ansuelsmth@...il.com Subject: Re: [PATCH net-next v12 5/6] net: dsa: mv88e6xxx: rmon: Use RMU for reading RMON data > + { "sw_in_discards", 4, 0x10, 0x81, STATS_TYPE_PORT, }, > + { "sw_in_filtered", 2, 0x12, 0x85, STATS_TYPE_PORT, }, > + { "sw_out_filtered", 2, 0x13, 0x89, STATS_TYPE_PORT, }, I agree with Florian here. Maybe add a table just for these three values? Or even a switch statement. > -static int mv88e6xxx_stats_get_stats(struct mv88e6xxx_chip *chip, int port, > - uint64_t *data, int types, > - u16 bank1_select, u16 histogram) > +static int mv88e6xxx_state_get_stats_rmu(struct mv88e6xxx_chip *chip, int port, > + uint64_t *data, int types, > + u16 bank1_select, u16 histogram) > +{ > + const u64 *stats = chip->ports[port].rmu_raw_stats; > + struct mv88e6xxx_hw_stat *stat; > + int offset = 0; > + u64 high; > + int i, j; > + > + for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) { > + stat = &mv88e6xxx_hw_stats[i]; > + if (stat->type & types) { > + if (stat->type & STATS_TYPE_PORT) { > + data[j] = stats[stat->rmu_reg]; > + } else { > + if (stat->type & STATS_TYPE_BANK1) > + offset = 32; > + > + data[j] = stats[stat->reg + offset]; > + if (stat->size == 8) { > + high = stats[stat->reg + offset + 1]; > + data[j] += (high << 32); > + } > + } > + > + j++; > + } > + } > + return j; > +} This is definitely getting better, closer to what i was thinking. But... > +static int mv88e6xxx_stats_get_stats(struct mv88e6xxx_chip *chip, int port, > + uint64_t *data, int types, > + u16 bank1_select, u16 histogram) > +{ > + if (mv88e6xxx_rmu_available(chip)) > + return mv88e6xxx_state_get_stats_rmu(chip, port, data, types, > + bank1_select, histogram); > + else > + return mv88e6xxx_stats_get_stats_mdio(chip, port, data, types, > + bank1_select, histogram); > +} I would still like to get rid of this. What i think the problem here is, you are trying to be 100% compatible. So you only show bank1 statistics, if they where available via MDIO. However, it seems like all the RMU implementations make all statistics available, both banks, and counters in the port space. So long as you keep the names consistent, you can return more statistics via RMU than via MDIO. Andrew
Powered by blists - more mailing lists