| 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
| ||
|
Date: Tue, 28 May 2024 09:11:09 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>,
<intel-wired-lan@...ts.osuosl.org>
CC: <netdev@...r.kernel.org>, <jacob.e.keller@...el.com>,
<michal.kubiak@...el.com>, <maciej.fijalkowski@...el.com>,
<sridhar.samudrala@...el.com>, <wojciech.drewek@...el.com>,
<pio.raczynski@...il.com>, <jiri@...dia.com>, <mateusz.polchlopek@...el.com>,
<shayd@...dia.com>, <kalesh-anakkur.purayil@...adcom.com>
Subject: Re: [iwl-next v3 05/15] ice: allocate devlink for subfunction
On 5/28/24 06:38, Michal Swiatkowski wrote:
> From: Piotr Raczynski <piotr.raczynski@...el.com>
>
> Make devlink allocation function generic to use it for PF and for SF.
>
> Add function for SF devlink port creation. It will be used in next
> patch.
>
> Create header file for subfunction device. Define subfunction device
> structure there as it is needed for devlink allocation and port
> creation.
>
> Signed-off-by: Piotr Raczynski <piotr.raczynski@...el.com>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> ---
> .../net/ethernet/intel/ice/devlink/devlink.c | 33 +++++++++++++++
> .../net/ethernet/intel/ice/devlink/devlink.h | 1 +
> .../ethernet/intel/ice/devlink/devlink_port.c | 41 +++++++++++++++++++
> .../ethernet/intel/ice/devlink/devlink_port.h | 3 ++
> drivers/net/ethernet/intel/ice/ice_sf_eth.h | 21 ++++++++++
> 5 files changed, 99 insertions(+)
> create mode 100644 drivers/net/ethernet/intel/ice/ice_sf_eth.h
just two minor nitpicks, so:
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
>
> diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> index bfb3d5b59a62..58196c170b1b 100644
> --- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
> +++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> @@ -10,6 +10,7 @@
> #include "ice_eswitch.h"
> #include "ice_fw_update.h"
> #include "ice_dcb_lib.h"
> +#include "ice_sf_eth.h"
>
> /* context for devlink info version reporting */
> struct ice_info_ctx {
> @@ -1282,6 +1283,8 @@ static const struct devlink_ops ice_devlink_ops = {
> .port_new = ice_devlink_port_new,
> };
>
> +static const struct devlink_ops ice_sf_devlink_ops;
> +
> static int
> ice_devlink_enable_roce_get(struct devlink *devlink, u32 id,
> struct devlink_param_gset_ctx *ctx)
> @@ -1422,6 +1425,7 @@ static void ice_devlink_free(void *devlink_ptr)
> * Allocate a devlink instance for this device and return the private area as
> * the PF structure. The devlink memory is kept track of through devres by
> * adding an action to remove it when unwinding.
> + *
> */
> struct ice_pf *ice_allocate_pf(struct device *dev)
> {
> @@ -1438,6 +1442,35 @@ struct ice_pf *ice_allocate_pf(struct device *dev)
> return devlink_priv(devlink);
> }
>
> +/**
> + * ice_allocate_sf - Allocate devlink and return SF structure pointer
> + * @dev: the device to allocate for
> + * @pf: pointer to the PF structure
> + *
> + * Allocate a devlink instance for SF.
> + *
> + * Return: void pointer to allocated memory
nit: it's not void; you could add "or ERR_PTR in case of error"
> + */
> +struct ice_sf_priv *ice_allocate_sf(struct device *dev, struct ice_pf *pf)
> +{
> + struct devlink *devlink;
> + int err;
> +
> + devlink = devlink_alloc_ns(&ice_sf_devlink_ops,
> + sizeof(struct ice_sf_priv),
> + devlink_net(priv_to_devlink(pf)), dev);
> + if (!devlink)
> + return NULL;
ERR_PTR(-ENOMEM) would be more consistent with the other error exit path
> +
> + err = devl_nested_devlink_set(priv_to_devlink(pf), devlink);
> + if (err) {
> + devlink_free(devlink);
> + return ERR_PTR(err);
> + }
> +
> + return devlink_priv(devlink);
> +}
> +
> /**
> * ice_devlink_register - Register devlink interface for this PF
> * @pf: the PF to register the devlink for.
> diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.h b/drivers/net/ethernet/intel/ice/devlink/devlink.h
> index d291c0e2e17b..1af3b0763fbb 100644
> --- a/drivers/net/ethernet/intel/ice/devlink/devlink.h
> +++ b/drivers/net/ethernet/intel/ice/devlink/devlink.h
> @@ -5,6 +5,7 @@
> #define _ICE_DEVLINK_H_
>
> struct ice_pf *ice_allocate_pf(struct device *dev);
> +struct ice_sf_priv *ice_allocate_sf(struct device *dev, struct ice_pf *pf);
>
> void ice_devlink_register(struct ice_pf *pf);
> void ice_devlink_unregister(struct ice_pf *pf);
> diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink_port.c b/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
> index 5d1fe08e4bab..f06baabd0112 100644
> --- a/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
> +++ b/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
> @@ -489,6 +489,47 @@ void ice_devlink_destroy_vf_port(struct ice_vf *vf)
> devl_port_unregister(&vf->devlink_port);
> }
>
> +/**
> + * ice_devlink_create_sf_dev_port - Register virtual port for a subfunction
> + * @sf_dev: the subfunction device to create a devlink port for
> + *
> + * Register virtual flavour devlink port for the subfunction auxiliary device
> + * created after activating a dynamically added devlink port.
> + *
> + * Return: zero on success or an error code on failure.
> + */
> +int ice_devlink_create_sf_dev_port(struct ice_sf_dev *sf_dev)
> +{
> + struct devlink_port_attrs attrs = {};
> + struct ice_dynamic_port *dyn_port;
> + struct devlink_port *devlink_port;
> + struct devlink *devlink;
> + struct ice_vsi *vsi;
> +
> + dyn_port = sf_dev->dyn_port;
> + vsi = dyn_port->vsi;
> +
> + devlink_port = &sf_dev->priv->devlink_port;
> +
> + attrs.flavour = DEVLINK_PORT_FLAVOUR_VIRTUAL;
(just comment, not an issue)
we have (among others):
198│ enum devlink_port_flavour {
199│ DEVLINK_PORT_FLAVOUR_PHYSICAL, /* Any kind of a port physically
200│ * facing the user.
201│ */
210│ DEVLINK_PORT_FLAVOUR_PCI_VF, /* Represents eswitch port
211│ * for the PCI VF. It is an
internal
212│ * port that faces the PCI VF.
213│ */
214│ DEVLINK_PORT_FLAVOUR_VIRTUAL, /* Any virtual port facing
the user. */
216│ * is not used in any way.
217│ */
218│ DEVLINK_PORT_FLAVOUR_PCI_SF, /* Represents eswitch port
219│ * for the PCI SF. It is an
internal
220│ * port that faces the PCI SF.
221│ */
from that I conclude that _PCI_ ones are internal, and you are adding
user-facing port, so your choice is good, even if there is one with SF
in the name. Perhaps the enum should have this piece of documentation ;)
> +
> + devlink_port_attrs_set(devlink_port, &attrs);
> + devlink = priv_to_devlink(sf_dev->priv);
> +
> + return devl_port_register(devlink, devlink_port, vsi->idx);
> +}
> +
> +/**
> + * ice_devlink_destroy_sf_dev_port - Destroy virtual port for a subfunction
> + * @sf_dev: the subfunction device to create a devlink port for
> + *
> + * Unregisters the virtual port associated with this subfunction.
> + */
> +void ice_devlink_destroy_sf_dev_port(struct ice_sf_dev *sf_dev)
> +{
> + devl_port_unregister(&sf_dev->priv->devlink_port);
> +}
> +
> /**
> * ice_dealloc_dynamic_port - Deallocate and remove a dynamic port
> * @dyn_port: dynamic port instance to deallocate
> diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink_port.h b/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
> index 08ebf56664a5..97b21b58c300 100644
> --- a/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
> +++ b/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
> @@ -5,6 +5,7 @@
> #define _DEVLINK_PORT_H_
>
> #include "../ice.h"
> +#include "../ice_sf_eth.h"
>
> /**
> * struct ice_dynamic_port - Track dynamically added devlink port instance
> @@ -34,6 +35,8 @@ int ice_devlink_create_vf_port(struct ice_vf *vf);
> void ice_devlink_destroy_vf_port(struct ice_vf *vf);
> int ice_devlink_create_sf_port(struct ice_dynamic_port *dyn_port);
> void ice_devlink_destroy_sf_port(struct ice_dynamic_port *dyn_port);
> +int ice_devlink_create_sf_dev_port(struct ice_sf_dev *sf_dev);
> +void ice_devlink_destroy_sf_dev_port(struct ice_sf_dev *sf_dev);
>
> #define ice_devlink_port_to_dyn(port) \
> container_of(port, struct ice_dynamic_port, devlink_port)
> diff --git a/drivers/net/ethernet/intel/ice/ice_sf_eth.h b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
> new file mode 100644
> index 000000000000..a08f8b2bceef
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2024, Intel Corporation. */
> +
> +#ifndef _ICE_SF_ETH_H_
> +#define _ICE_SF_ETH_H_
> +
> +#include <linux/auxiliary_bus.h>
> +#include "ice.h"
> +
> +struct ice_sf_dev {
> + struct auxiliary_device adev;
> + struct ice_dynamic_port *dyn_port;
> + struct ice_sf_priv *priv;
> +};
> +
> +struct ice_sf_priv {
> + struct ice_sf_dev *dev;
> + struct devlink_port devlink_port;
> +};
> +
> +#endif /* _ICE_SF_ETH_H_ */
Powered by blists - more mailing lists