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

[Edward Cree <ecree.xilinx@gmail.com>]

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.)

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').
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.

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.

-ed