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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CALzJLG9WXBBQVdGpopgrAg_yF=eL3W5tU0E5u1xK5q8+0qahww@mail.gmail.com>
Date:   Sun, 11 Dec 2016 13:56:42 +0200
From:   Saeed Mahameed <saeedm@....mellanox.co.il>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     Saeed Mahameed <saeedm@...lanox.com>,
        "David S. Miller" <davem@...emloft.net>,
        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 12:45 AM, Stephen Hemminger
<stephen@...workplumber.org> wrote:
> On Wed,  7 Dec 2016 00:33:08 +0200
> Saeed Mahameed <saeedm@...lanox.com> wrote:
>
>> This simple ethool change will give HW vendors the flexibility to set
>> pure HW configurations (not directly related to netdev resources states
>> and rings), without the need of vendor proprietary tools and hacks.
>
>
> The danger is you need to restrict the kernel to only allow setting
> safe registers (and this is HW dependent).  There are cases like secure
> boot where it is expected that even root is not allowed to modify
> all memory.
>
> Also supporting closed format of device registers is not in the interest
> of promoting open source.
>

This is not totally close format, it is only a generic API to support
setting some of the HW registers.
the format is open in
1. http://www.mellanox.com/related-docs/user_manuals/Ethernet_Adapters_Programming_Manual.pdf
2. http://lxr.free-electrons.com/source/include/linux/mlx5/mlx5_ifc.h

We just want to let the user to configure HW dependent registers
without the need to add new defines/callback each time to the kernel
ABI/API.

> I am not saying I fundamentally disagree with supporting this, but it
> is a bigger step than you make it out to be.

I have to disagree, it is not as big as it seems, there are already at
least two places in ethtool API that do the same,

http://lxr.free-electrons.com/source/include/linux/ethtool.h
192  * @set_eeprom: Write data to the device EEPROM.
193  *      Should validate the magic field.  Don't need to check len for zero
194  *      or wraparound.  Update len to the amount written.  Returns an error
195  *      or zero.
http://lxr.free-electrons.com/source/net/core/ethtool.c#L1582

234  * @flash_device: Write a firmware image to device's flash memory.
235  *      Returns a negative error code or zero.

Both accept a binary array from user and forward to HW/FW.
Simply we want to to the same for HW registers, you already have a way
to get them (even parse them in user ethtool), but you can't set them.