netdev - Re: [RFC net-next PATCH 3/3] mv88e6xxx: rmon: Use RMU to collect rmon data.

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 for Android: free password hash cracker in your pocket

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

Message-ID: <Yv5cbL7xUFRo02Bu@lunn.ch>
Date:   Thu, 18 Aug 2022 17:36:12 +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>
Subject: Re: [RFC net-next PATCH 3/3] mv88e6xxx: rmon: Use RMU to collect
 rmon data.

> @@ -1310,16 +1323,22 @@ static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
>  	struct mv88e6xxx_chip *chip = ds->priv;
>  	int ret;
>  
> -	mv88e6xxx_reg_lock(chip);
> +	if (chip->rmu.ops && chip->rmu.ops->get_rmon) {
> +		ret = chip->rmu.ops->get_rmon(chip, port, data);
> +		if (ret == -ETIMEDOUT)
> +			return;
> +	} else {
>  
> -	ret = mv88e6xxx_stats_snapshot(chip, port);
> -	mv88e6xxx_reg_unlock(chip);
> +		mv88e6xxx_reg_lock(chip);
>  
> -	if (ret < 0)
> -		return;
> +		ret = mv88e6xxx_stats_snapshot(chip, port);
> +		mv88e6xxx_reg_unlock(chip);
>  
> -	mv88e6xxx_get_stats(chip, port, data);
> +		if (ret < 0)
> +			return;
>  
> +		mv88e6xxx_get_stats(chip, port, data);
> +	}
>  }

I don't particularly like the way this is all mixed together.

Could you try to split it, so there is an MDIO set of functions and an
RMU set of functions. Maybe you have some helpers which are used by
both.

I would also suggest you try to think about ATU dump and VTU dump. You
ideally want a code structure which is very similar for all these dump
operations. Take a look at how qca8k-8xxx.c does things.

Is it documented in the datasheet that when RMU is used a snapshot is
not required?

    Andrew