netdev - Re: [PATCHv2 net-next 2/4] net: dsa: mv88e6xxx: Monitor and Management tables

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:   Fri, 2 Dec 2016 21:56:56 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Vivien Didelot <vivien.didelot@...oirfairelinux.com>
Cc:     David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCHv2 net-next 2/4] net: dsa: mv88e6xxx: Monitor and
 Management tables

On Fri, Dec 02, 2016 at 02:32:39PM -0500, Vivien Didelot wrote:
> Hi Andrew,
> 
> Andrew Lunn <andrew@...n.ch> writes:
> 
> > @@ -3184,6 +3186,8 @@ static const struct mv88e6xxx_ops mv88e6085_ops = {
> >  	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
> >  	.stats_get_strings = mv88e6095_stats_get_strings,
> >  	.stats_get_stats = mv88e6095_stats_get_stats,
> > +	.g1_set_cpu_port = mv88e6095_g1_set_cpu_port,
> > +	.g1_set_egress_port = mv88e6095_g1_set_egress_port,
> >  };
> 
> I like the implementation in this version better. But please explain me
> why you are prefixing these operations with g1_?

The prefix gives some basic grouping. port_ indicates it operates on a
port, and is likely to be found in port.c. stats_ indicates it
operates on statistics, ppu that is operates on the phy polling unit.

We are going to have some things which don't fall into a simple
category, like these two. But it would however be nice to group them,
so i picked which register bank they are in. These operations are
always in g1. It is a useful hint as to where to find the different
variants.

> But let's imagine we can set the CPU port in some Global 2 registers.
> You are going to wrap this in chip.c with something like:
> 
>     int mv88e6xxx_set_cpu_port(struct mv88e6xxx_chip *chip, int port)
>     {
>         if (chip->info->ops->g2_set_cpu_port)
>             return chip->info->ops->g2_set_cpu_port(chip, port);
>         else if (chip->info->ops->g1_set_cpu_port)
>             return chip->info->ops->g1_set_cpu_port(chip, port);
>         else
>             return -EOPNOTSUPP;
>     }

I answered in one of my other emails. Frames with reserved MAC
addresses can be forwarded to the CPU. For most devices, this is a g2
operation. However, for 6390, it is a g1. In that case, my code does
not use a prefix. Not having a prefix, when all the others do, also
gives you information. It means the ops are spread around and you need
to make a bigger effort to go find them.

   Andrew