netdev - Re: [PATCH v2] net: dsa: mv88e6xxx: add write access to debugfs regs file

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]

Message-ID: <1092744390.7730.1436639772848.JavaMail.zimbra@savoirfairelinux.com>
Date:	Sat, 11 Jul 2015 14:36:12 -0400 (EDT)
From:	Vivien Didelot <vivien.didelot@...oirfairelinux.com>
To:	David <davem@...emloft.net>
Cc:	netdev <netdev@...r.kernel.org>,
	Guenter Roeck <linux@...ck-us.net>, andrew@...n.ch,
	linux-kernel <linux-kernel@...r.kernel.org>,
	kernel <kernel@...oirfairelinux.com>
Subject: Re: [PATCH v2] net: dsa: mv88e6xxx: add write access to debugfs
 regs file

Hi David,

On Jul 11, 2015, at 2:01 AM, David davem@...emloft.net wrote:

> From: Vivien Didelot <vivien.didelot@...oirfairelinux.com>
> Date: Thu,  9 Jul 2015 17:13:29 -0400
> 
>> Allow write access to the regs file in the debugfs interface, with the
>> following parameters:
>> 
>>     echo <name> <reg> <value> > regs
>> 
>> Where "name" is the register name (as shown in the header row), "reg" is
>> the register address (as shown in the first column) and "value" is the
>> 16-bit value. e.g.:
>> 
>>     echo GLOBAL 1a 5550 > regs
>> 
>> Signed-off-by: Vivien Didelot <vivien.didelot@...oirfairelinux.com>
> 
> I don't know about this.
> 
> This starts to smell like a back door for proprietary userspace SDKs to
> program the switch hardware.
> 
> Yes, they can do it via other mechanisms, but we don't have to make it
> any eaiser for them either.

I agree with you and I wouldn't want that neither.

> If you want to poke registers, hack the module just like any other
> person with appropriate privileges can do.

I'm not sure what you mean. Keeping some custom patches in our local tree?

> Frankly, all of this debugfs crap in the DSA drivers smells like poo.
> I don't like it _AT_ _ALL_, and I shouldn't have allowed any of it
> into the tree in the first place.
> 
> I might just remove it all myself, it bothers me so much.
> 
> Fetching information should be done by well typed, generic, interfaces
> that apply to any similar device or object.  All of this debugfs stuff
> smells of hacks and special case crap that's only usable for one
> device type and that makes it the single most terrible interface to
> give to users.

In the meantime, this is really useful for development. i.e. ensuring a good
switchdev/DSA interaction without being able to read and write directly the
hardware VLAN table, is a bit a PITA. A dynamic debugfs looked appropriate.

On the other hand, the mv88e6xxx driver gets cluttered with all this code. I'd
gladly move all this code in a mv88e6xxx-debugfs.c file, and conditionally
compile it with:

    mv88e6xxx_drv-$(CONFIG_DEBUG_FS) += mv88e6xxx-debugfs.o

similar to what the i2400m driver does.

Would that be appreciated?

Thanks,
-v
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html