netdev - Re: [PATCH net-next v2 12/14] sfc: set EF100 VF MAC address through representor

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: <5a4d22f2-e315-b6f4-5fb5-31134960c430@gmail.com>
Date:   Thu, 28 Jul 2022 21:23:23 +0100
From:   Edward Cree <ecree.xilinx@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     ecree@...inx.com, davem@...emloft.net, pabeni@...hat.com,
        linux-net-drivers@....com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2 12/14] sfc: set EF100 VF MAC address through
 representor

On 28/07/2022 20:27, Jakub Kicinski wrote:
> On Thu, 28 Jul 2022 19:54:21 +0100 Edward Cree wrote:
>> The user determines which repr corresponds to which VF by looking in
>>  /sys/class/net/$VFREP/phys_port_name (e.g. "p0pf0vf0").
> 
> .. and that would also most likely be what the devlink port ID would be.
AFAICT the devlink port index is just an integer.  The example in the
 man page is
    devlink port function set pci/0000:01:00.0/1 hw_addr 00:00:00:11:22:33
Moreover, struct devlink_port has `unsigned int index`.
Though it does also have `struct devlink_port_attrs` which appears to
 encode the PF and VF numbers; I think those can be read with `devlink
 port show`.
But the whole devlink port abstraction is unnecessary when we already
 *have* an object to represent the port.

>> Indeed.  I agree that .ndo_set_mac_address() is the wrong interface.
>> But the interface I have in mind would be something like
>>     int (*ndo_set_partner_mac_address)(struct net_device *, void *);
>>  and would only be implemented by representor netdevs.
>> Idk what the uAPI/UI for that would be; probably a new `ip link set`
>>  parameter.
> 
> Yup... If only you were there during the fight over this uAPI.
> Now it's the devlink "port function" thing.
Sadly I was too busy with EF100 bring-up, and naïvely assumed that I
 could safely ignore devlink port stuff as it was so obviously going
 to be a classic Mellanox design: tasteless, overweight, and not
 cleanly mappable onto any other vendor.  Which seems to have been
 true but they've managed to make it the standard anyway by virtue
 of being there first, as usual :'(
(Yeah, I probably shouldn't publicly say things like that about
 another vendor's devs.  But I'm getting frustrated at this recurring
 pattern.)

Devlink port function *would* be useful for administering functions
 that don't have a representor.  I just can't see any good reason
 why such things should ever exist.
Maybe it's not too late to introduce my API to exist alongside it…
 though I have no idea how much work it would take to teach the
 orchestration frameworks to use it :/

-ed