Re: [PATCH] ice: Add get/set hw address for VF representor ports

[Michal Swiatkowski <michal.swiatkowski@linux.intel.com>]

On Wed, Jan 31, 2024 at 01:00:04PM +0100, Jiri Pirko wrote:
> Wed, Jan 31, 2024 at 11:43:44AM CET, michal.swiatkowski@linux.intel.com wrote:
> >On Wed, Jan 31, 2024 at 01:38:47PM +0530, karthiksundaravel wrote:
> >> Changing the mac address of the VF representor ports are not
> >> available via devlink. Add the function handlers to set and get
> >> the HW address for the VF representor ports.
> >> 
> >> Signed-off-by: karthiksundaravel <ksundara@redhat.com>
> >> ---
> >>  drivers/net/ethernet/intel/ice/ice_devlink.c | 134 ++++++++++++++++++-
> >>  1 file changed, 132 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
> >> index 80dc5445b50d..56d81836c469 100644
> >> --- a/drivers/net/ethernet/intel/ice/ice_devlink.c
> >> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
> >> @@ -9,6 +9,8 @@
> >
> >As Jiri already wrote, you are not changing MAC of VF in your code. Try
> >to look at ice_set_vf_mac in ice_sriov.c. In current implementation you
> >nedd to set new MAC value for VF and reset it. You shouldn't use PF VSI.
> >
> >Pointer to VF you can get from representor struct (through parent VSI).
> 
> What if it is in a different host? Would you still be able to change the
> mac?
> 

In current VF MAC changing implementation yes, because it is done by
resetting the VF. After the reset new MAC will be sent via virtchnl.
But I think resetting VF may be incorrect here, as it leads to reset
also port representor.

> 
> >
> >You shouldn't manage the rules during MAC changing, as in switchdev
> >slow-path there shouldn't be VF MAC rules. It can be problematic as user
> >already can have MAC + sth rule (which also needs to be change). I will
> >leave it to user (most probably the MAC change happens before adding any
> >rules).
> 
> Rules are on the representor, not the VF, correct? Seems unrelated to
> me.
> 

I pointed it out because it was in the code. Rules added on representor
points to corresponding VF. My point was that there shouldn't be any
changes to rules after changing MAC.

> 
> >
> >In few days we will send patchset for subfunction support where the
> >subfunction MAC chaning is implementing from devlink API. I will add you
> >to the CC.
> >
> >Thanks for working on it, it is a gap in our solution.
> >
> >Thanks,
> >Michal
> >