netdev - Re: [PATCH net-next 0/2] Add ethtool set regs support

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]

Date:   Sun, 11 Dec 2016 14:41:27 +0200
From:   Saeed Mahameed <saeedm@....mellanox.co.il>
To:     David Miller <davem@...emloft.net>
Cc:     andrew@...n.ch, Saeed Mahameed <saeedm@...lanox.com>,
        Linux Netdev List <netdev@...r.kernel.org>,
        "John W. Linville" <linville@...driver.com>
Subject: Re: [PATCH net-next 0/2] Add ethtool set regs support

On Wed, Dec 7, 2016 at 4:57 AM, David Miller <davem@...emloft.net> wrote:
> From: Andrew Lunn <andrew@...n.ch>
> Date: Wed, 7 Dec 2016 03:41:43 +0100
>
>> On Wed, Dec 07, 2016 at 12:33:08AM +0200, Saeed Mahameed wrote:
>>> Hi Dave,
>>>
>>> This series adds the support for setting device registers from user
>>> space ethtool.
>>
>> Is this not the start of allowing binary only drivers in user space?
>>
>> Do we want this?
>
> I don't think we do.
>

Dave, i think we do have a case here, can you please check my response
to Stephen and Andrew,
I mention some examples and I think I made the purpose of this patches
more clear.

>>
>>> mlx5 driver have registers allowed access list and will check the user
>>> Request validity before forwarding it to HW registers. Mlx5 will allow only mlx5 specific
>>> configurations to be set (e.g. Device Diag Counters for HW performance debugging and analysis)
>>> which has no standard API to access it.
>>
>> Would it not be better to define an flexible API to do this? We have
>> lots of HW performance counters for CPUs. Why is it not possible to do
>> this for a network device?
>
> So if this isn't for raw PIO register access, then we should define
> an appropriate interface for it.
>

It is not, I hope i made it more clear in my responses to Stephen and Andrew.

>
> The ethtool regs stuff is untyped, and arbitrary.
>
> Please create something properly structured, and typed, which would
> allow accessing the information you want the user to be able to
> access.
>
> That way the kernel can tell what the user is reading or writing,
> and thus properly control access.

Thanks for your input,

As i explained to Stephen and Andrew, Those registers differs from
vendor to vendor and cross HW updates. Making those registers
strongly typed is wrong in my opinion, it is not scalable and as
flexible as you think it would be.  There are many HW vendors and each
has lots of registers.

Having a set_eeprom like API will do the job. you can think about this
as ethtool_flash_device kinda tool but more lightweight.

We simply would like to configure/tweak/monitor/debug HW internal
units activities for NIC netdev sake, like the usage we currently
have/suggest,
to have flexibility to enable/disable/query some HW internal
performance registers (should be off on production), both set and
query
will use ethtool_set/get_regs, obviously.

We also took into account future usages for other registers/formats
and to have flexibility across different NICs.

I will be glad if you can give this some thought and provide me with
more concrete direction/suggestion on how to make this less untyped.

The only thing i can think of right now is providing a list of write
allowed registers (address + type + layout length) to ethtool, but i
am not sure how
to handle the "type" information in the kernel, it will create more
restrictions than needed, and what types of register will we need to
add support for at first ?