| 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
| ||
|
Message-ID: <CO1PR11MB5089D91A3610E600ACEC8AF6D64E2@CO1PR11MB5089.namprd11.prod.outlook.com>
Date: Wed, 14 Feb 2024 19:41:11 +0000
From: "Keller, Jacob E" <jacob.e.keller@...el.com>
To: Jiri Pirko <jiri@...nulli.us>
CC: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>,
"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Kubiak, Michal"
<michal.kubiak@...el.com>, "Fijalkowski, Maciej"
<maciej.fijalkowski@...el.com>, "Samudrala, Sridhar"
<sridhar.samudrala@...el.com>, "Kitszel, Przemyslaw"
<przemyslaw.kitszel@...el.com>, "Drewek, Wojciech"
<wojciech.drewek@...el.com>, "pio.raczynski@...il.com"
<pio.raczynski@...il.com>, Piotr Raczynski <piotr.raczynski@...el.com>
Subject: RE: [iwl-next v1 07/15] ice: add auxiliary device sfnum attribute
> -----Original Message-----
> From: Jiri Pirko <jiri@...nulli.us>
> Sent: Wednesday, February 14, 2024 12:45 AM
> To: Keller, Jacob E <jacob.e.keller@...el.com>
> Cc: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>; intel-wired-
> lan@...ts.osuosl.org; netdev@...r.kernel.org; Kubiak, Michal
> <michal.kubiak@...el.com>; Fijalkowski, Maciej <maciej.fijalkowski@...el.com>;
> Samudrala, Sridhar <sridhar.samudrala@...el.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@...el.com>; Drewek, Wojciech
> <wojciech.drewek@...el.com>; pio.raczynski@...il.com; Piotr Raczynski
> <piotr.raczynski@...el.com>
> Subject: Re: [iwl-next v1 07/15] ice: add auxiliary device sfnum attribute
>
> Tue, Feb 13, 2024 at 11:04:00PM CET, jacob.e.keller@...el.com wrote:
> >
> >
> >On 2/13/2024 3:55 AM, Michal Swiatkowski wrote:
> >> On Tue, Feb 13, 2024 at 12:29:40PM +0100, Jiri Pirko wrote:
> >>> Tue, Feb 13, 2024 at 10:53:50AM CET, michal.swiatkowski@...ux.intel.com
> wrote:
> >>>> On Tue, Feb 13, 2024 at 09:59:14AM +0100, Jiri Pirko wrote:
> >>>>> Tue, Feb 13, 2024 at 08:27:16AM CET, michal.swiatkowski@...ux.intel.com
> wrote:
> >>>>>> From: Piotr Raczynski <piotr.raczynski@...el.com>
> >>>>>>
> >>>>>> Add read only sysfs attribute for each auxiliary subfunction
> >>>>>> device. This attribute is needed for orchestration layer
> >>>>>> to distinguish SF devices from each other since there is no
> >>>>>> native devlink mechanism to represent the connection between
> >>>>>> devlink instance and the devlink port created for the port
> >>>>>> representor.
> >>>>>>
> >>>>>> Reviewed-by: Wojciech Drewek <wojciech.drewek@...el.com>
> >>>>>> Signed-off-by: Piotr Raczynski <piotr.raczynski@...el.com>
> >>>>>> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> >>>>>> ---
> >>>>>> drivers/net/ethernet/intel/ice/ice_sf_eth.c | 31
> +++++++++++++++++++++
> >>>>>> 1 file changed, 31 insertions(+)
> >>>>>>
> >>>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_sf_eth.c
> b/drivers/net/ethernet/intel/ice/ice_sf_eth.c
> >>>>>> index ab90db52a8fc..abee733710a5 100644
> >>>>>> --- a/drivers/net/ethernet/intel/ice/ice_sf_eth.c
> >>>>>> +++ b/drivers/net/ethernet/intel/ice/ice_sf_eth.c
> >>>>>> @@ -224,6 +224,36 @@ static void ice_sf_dev_release(struct device
> *device)
> >>>>>> kfree(sf_dev);
> >>>>>> }
> >>>>>>
> >>>>>> +static ssize_t
> >>>>>> +sfnum_show(struct device *dev, struct device_attribute *attr, char *buf)
> >>>>>> +{
> >>>>>> + struct devlink_port_attrs *attrs;
> >>>>>> + struct auxiliary_device *adev;
> >>>>>> + struct ice_sf_dev *sf_dev;
> >>>>>> +
> >>>>>> + adev = to_auxiliary_dev(dev);
> >>>>>> + sf_dev = ice_adev_to_sf_dev(adev);
> >>>>>> + attrs = &sf_dev->dyn_port->devlink_port.attrs;
> >>>>>> +
> >>>>>> + return sysfs_emit(buf, "%u\n", attrs->pci_sf.sf);
> >>>>>> +}
> >>>>>> +
> >>>>>> +static DEVICE_ATTR_RO(sfnum);
> >>>>>> +
> >>>>>> +static struct attribute *ice_sf_device_attrs[] = {
> >>>>>> + &dev_attr_sfnum.attr,
> >>>>>> + NULL,
> >>>>>> +};
> >>>>>> +
> >>>>>> +static const struct attribute_group ice_sf_attr_group = {
> >>>>>> + .attrs = ice_sf_device_attrs,
> >>>>>> +};
> >>>>>> +
> >>>>>> +static const struct attribute_group *ice_sf_attr_groups[2] = {
> >>>>>> + &ice_sf_attr_group,
> >>>>>> + NULL
> >>>>>> +};
> >>>>>> +
> >>>>>> /**
> >>>>>> * ice_sf_eth_activate - Activate Ethernet subfunction port
> >>>>>> * @dyn_port: the dynamic port instance for this subfunction
> >>>>>> @@ -262,6 +292,7 @@ ice_sf_eth_activate(struct ice_dynamic_port
> *dyn_port,
> >>>>>> sf_dev->dyn_port = dyn_port;
> >>>>>> sf_dev->adev.id = id;
> >>>>>> sf_dev->adev.name = "sf";
> >>>>>> + sf_dev->adev.dev.groups = ice_sf_attr_groups;
> >>>>>
> >>>>> Ugh. Custom driver sysfs files like this are always very questionable.
> >>>>> Don't do that please. If you need to expose sfnum, please think about
> >>>>> some common way. Why exactly you need to expose it?
> >>>>
> >>>> Uh, hard question. I will drop it and check if it still needed to expose
> >>>> the sfnum, probably no, as I have never used this sysfs during testing.
> >>>>
> >>>> Should devlink be used for it?
> >>>
> >>> sfnum is exposed over devlink on the port representor. If you need to
> >>> expose it on the actual SF, we have to figure it out. But again, why?
> >>>
> >>>
> >
> >I vaguely remember some internal discussion about orchestration software
> >wanting to know which subfunction was associated with which auxiliary
> >device. However, I think a much better solution would be to expose the
> >auxiliary device ID out of devlink_port instead, through devlink port.
> >
> >I can't find any notes on this and it was quite some time ago so maybe
> >things have changed.
> >
> >If we enable support for user-space configurable sfnum, then we can just
> >have the orchestration software pick its sfnum (or check the netlink
> >return value from the port add), so probably this is not that useful.
>
> This is already solved by nested devlink. When you properly call
> devl_port_fn_devlink_set(), you link the SF devlink instance with the
> eswitch port representor. Then the user sees:
>
> $ devlink port
> pci/0000:08:00.1/98304: type eth netdev eth4 flavour pcisf controller 0 pfnum 1
> sfnum 109 splittable false
> function:
> hw_addr 00:00:00:00:00:00 state active opstate attached roce enable
> nested_devlink:
> auxiliary/mlx5_core.sf.2
>
Excellent. That looks to resolve this entirely then.
Thanks,
Jake
Powered by blists - more mailing lists