netdev - Re: [PATCH net-next v14 5/7] net: dsa: mv88e6xxx: rmu: Add functionality to get RMON

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:   Wed, 21 Sep 2022 17:50:15 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Mattias Forsblad <mattias.forsblad@...il.com>
Cc:     Vladimir Oltean <olteanv@...il.com>, netdev@...r.kernel.org,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...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 v14 5/7] net: dsa: mv88e6xxx: rmu: Add
 functionality to get RMON

> I understand want you want but I can see a lot of risks and pitfalls with moving
> ordinary read and writes to RMU, which I wanted to avoid by first doing
> RMON dump and then dump ATU and at a later stage with a better architecture
> for write/read combining doing that, instead of forcing through read/writes
> with all associated testing it would require. Can we please do this in
> steps?

If we are going to fall back to MDIO when RMU fails, we need a
different code structure for these operations. That different code
structure should also help solve the messy _ops structure stuff.

RMU affects us in two different locations:

ATU and MIB dump: Controlled by struct mv88e6xxx_ops

register read/write: Controlled by struct mv88e6xxx_bus_ops

We could add to struct mv88e6xxx_ops:

        int (*stats_rmu_get_sset_count)(struct mv88e6xxx_chip *chip);
        int (*stats_rmu_get_strings)(struct mv88e6xxx_chip *chip,  uint8_t *data);
        int (*stats_rmu_get_stats)(struct mv88e6xxx_chip *chip,  int port,
                                   uint64_t *data);

and then mv88e6xxx_get_stats() would become something like:

static void mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port,
                                uint64_t *data)
{
        int count = 0;
	int err;

	if (chip->info->ops->stats_rmu_get_stats && mv88e6xxx_rmu_enabled(chip)) {
		err = chip->info->ops->stats_rmu_get_stats(chip, port, data)
		if (!err)
			return;
	}
			
        if (chip->info->ops->stats_get_stats)
                count = chip->info->ops->stats_get_stats(chip, port, data);

We then get fall back to MDIO, and clean, separate implementations of
RMU and MDIO stats operations.

I hope the ATU/fdb dump can be done in a similar way.

register read/writes, we probably need to extend mv88e6xxx_smi_read()
and mv88e6xxx_smi_write(). Try RMU first, and then fall back to MDIO.

Please try something in this direction. But please, lots of small,
simple patches with good commit messages.

       Andrew