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: <20220728113231.26fdfab0@kernel.org>
Date:   Thu, 28 Jul 2022 11:32:31 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Edward Cree <ecree.xilinx@...il.com>
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 Thu, 28 Jul 2022 19:12:34 +0100 Edward Cree wrote:
> On 28/07/2022 17:20, Jakub Kicinski wrote:
> > It's set thru
> > 
> >  devlink port function set DEV/PORT_INDEX hw_addr ADDR
> > 
> > "port functions" is a weird object representing something 
> > in Mellanox FW. Hopefully it makes more sense to you than
> > it does to me.  
> Hmm that does look weird, looks like it acts on a PCI device
>  (DEV is a PCI address) and then I'm not sure what PORT_INDEX
>  is meant to mean (the man page doesn't describe it at all).
>  Possibly it doesn't have semantics as such and is just a
>  synthetic index into a list of ports…
> I can't say it makes sense to me either :shrug:
> 
> We did take a look at what nfp does, as well; they use the
>  old .ndo_set_vf_mac(), but they appear to support it both on
>  the PF and on the VF reprs — meaning that (AFAICT) it allows
>  to set the MAC address of VF 0 through the repr for VF 1.
> (There is no check that I can see in nfp_app_set_vf_mac()
>  that the value of `int vf` matches the caller.)

IIRC the reprs are all linked to the PCI device of the PF in sysfs,
and OpenStack would pick a device linked to the PCI parent almost
at random. So the VF reprs needed the legacy NDOs. At least that's
what I remember being told.

I think the legacy NDOs are acceptable, devlink way is preferred
(devlink way did not exist when NFP code was written).

> Our (SN1000) approach to the problem of configuring 'remote'
>  functions (VFs in VMs, PFs on the embedded SoC) is to use
>  representors for them all (VF reps as added in this & prev
>  series, PF reps coming in the future.  Similarly, if we
>  were ever to add Subfunctions, each SF would have a
>  corresponding SF representor that would work in much the
>  same way as VF reps).  At which point you should always be
>  able to configure an object through its associated rep,
>  and there should never be a need for an 'index' parameter
>  (be that 'VF index' or 'port index').

How do you map reprs to VFs? The PCI devices of the VF may be on 
a different system.

> While .ndo_set_mac_address() might be the Wrong Thing (if
>  we want to be able to set VF and VF-rep addresses
>  independently to different things), the Right Thing ought
>  to have the same signature (i.e. just taking a netdev and
>  a hwaddr).  Devlink seems to me like a needless
>  complication here.

But reps are like switch ports in a switch ASIC, and the PCI
device is the other side of the virtual wire. You would not be
configuring the MAC address of a peer to peer link by setting 
the local address.

> Anyway, since the proper direction is unclear, I'll respin
>  the series without patches 10-13 in the hope of getting
>  the rest of it in before the merge window.

SG