netdev - Re: [PATCH net-next v2 5/7] net: dsa: mv88e6xxx: Add devlink regions

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] [day] [month] [year] [list]

Date:   Tue, 8 Sep 2020 12:30:36 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     David Miller <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>, Chris Healy <cphealy@...il.com>,
        Jiri Pirko <jiri@...nulli.us>
Subject: Re: [PATCH net-next v2 5/7] net: dsa: mv88e6xxx: Add devlink
 regions

On Tue, 8 Sep 2020 21:22:31 +0200 Andrew Lunn wrote:
> On Tue, Sep 08, 2020 at 12:01:06PM -0700, Jakub Kicinski wrote:
> > On Tue,  8 Sep 2020 02:51:53 +0200 Andrew Lunn wrote:  
> > > Allow ports, the global registers, and the ATU to be snapshot via
> > > devlink regions.
> > > 
> > > v2:
> > > Remove left over debug prints
> > > Comment ATU format is generic for mv88e6xxx, not wider
> > > 
> > > Signed-off-by: Andrew Lunn <andrew@...n.ch>  
> > 
> > Probably best CCing devlink maintainers on devlink patches.
> > 
> > Also - it's always useful to include show command outputs in the commit
> > message for devlink patches.  
> 
> Hi Jakub
> 
> root@rap:~# devlink region dump mdio_bus/gpio-0:00/port5 snapshot 42
> 0000000000000000 0f 10 03 00 00 00 01 39 7c 00 00 00 df 07 01 00 
> 0000000000000010 80 20 01 00 00 80 20 00 00 00 00 00 00 00 00 91 
> 0000000000000020 00 00 00 00 00 00 00 00 00 00 00 00 22 00 00 00 
> 0000000000000030 00 00 00 00 c0 01 00 80 00 00 00 00 00 00 00 00 
> 
> Not very informative. The whole point of devlink regions is that they
> are suppose to be specific to a device, and you need intimate
> knowledge of the device to decode it.

I meant the list of regions the device would create.

> > > +PORT_REGION_OPS(0);
> > > +PORT_REGION_OPS(1);
> > > +PORT_REGION_OPS(2);
> > > +PORT_REGION_OPS(3);
> > > +PORT_REGION_OPS(4);
> > > +PORT_REGION_OPS(5);
> > > +PORT_REGION_OPS(6);
> > > +PORT_REGION_OPS(7);
> > > +PORT_REGION_OPS(8);
> > > +PORT_REGION_OPS(9);
> > > +PORT_REGION_OPS(10);
> > > +PORT_REGION_OPS(11);
> > > +
> > > +static const struct devlink_region_ops *mv88e6xxx_region_port_ops[] = {
> > > +	&mv88e6xxx_region_port_0_ops,
> > > +	&mv88e6xxx_region_port_1_ops,
> > > +	&mv88e6xxx_region_port_2_ops,
> > > +	&mv88e6xxx_region_port_3_ops,
> > > +	&mv88e6xxx_region_port_4_ops,
> > > +	&mv88e6xxx_region_port_5_ops,
> > > +	&mv88e6xxx_region_port_6_ops,
> > > +	&mv88e6xxx_region_port_7_ops,
> > > +	&mv88e6xxx_region_port_8_ops,
> > > +	&mv88e6xxx_region_port_9_ops,
> > > +	&mv88e6xxx_region_port_10_ops,
> > > +	&mv88e6xxx_region_port_11_ops,
> > > +};  
> > 
> > Ahh, seems like regions will get a per-port incarnation as some point as
> > well..  
> 
> Again, i think this is back to the history of dumping firmware core.
> I guess the existing users don't have per port CPUs which could dump a
> core.

Ack, I'm referring to the fact that we recently converted health
reporters to per-port and now traps are getting the same treatment.