diff mbox series

[iwl-next,v4,6/8] ice: base subfunction aux driver

Message ID 20240417142028.2171-7-michal.swiatkowski@linux.intel.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series ice: support devlink subfunction | expand

Checks

Context Check Description
netdev/series_format warning Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 932 this patch: 932
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 5 maintainers not CCed: pabeni@redhat.com jesse.brandeburg@intel.com kuba@kernel.org edumazet@google.com anthony.l.nguyen@intel.com
netdev/build_clang success Errors and warnings before: 938 this patch: 938
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 944 this patch: 944
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 116 this patch: 116
netdev/source_inline success Was 0 now: 0

Commit Message

Michal Swiatkowski April 17, 2024, 2:20 p.m. UTC 
From: Piotr Raczynski <piotr.raczynski@intel.com>

Implement subfunction driver. It is probe when subfunction port is
activated.

VSI is already created. During the probe VSI is being configured.
MAC unicast and broadcast filter is added to allow traffic to pass.

Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Piotr Raczynski <piotr.raczynski@intel.com>
Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
---
 drivers/net/ethernet/intel/ice/Makefile     |   1 +
 drivers/net/ethernet/intel/ice/ice_main.c   |  10 ++
 drivers/net/ethernet/intel/ice/ice_sf_eth.c | 140 ++++++++++++++++++++
 drivers/net/ethernet/intel/ice/ice_sf_eth.h |   9 ++
 4 files changed, 160 insertions(+)
 create mode 100644 drivers/net/ethernet/intel/ice/ice_sf_eth.c

Comments

Shay Drori April 18, 2024, 7:57 a.m. UTC | #1 
On 17/04/2024 17:20, Michal Swiatkowski wrote:
> From: Piotr Raczynski <piotr.raczynski@intel.com>
>
> Implement subfunction driver. It is probe when subfunction port is
> activated.
>
> VSI is already created. During the probe VSI is being configured.
> MAC unicast and broadcast filter is added to allow traffic to pass.
>
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> Signed-off-by: Piotr Raczynski <piotr.raczynski@intel.com>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> ---
>   drivers/net/ethernet/intel/ice/Makefile     |   1 +
>   drivers/net/ethernet/intel/ice/ice_main.c   |  10 ++
>   drivers/net/ethernet/intel/ice/ice_sf_eth.c | 140 ++++++++++++++++++++
>   drivers/net/ethernet/intel/ice/ice_sf_eth.h |   9 ++
>   4 files changed, 160 insertions(+)
>   create mode 100644 drivers/net/ethernet/intel/ice/ice_sf_eth.c


<...>

> +
> +/**
> + * ice_sf_driver_register - Register new auxiliary subfunction driver
> + *
> + * Return: zero on success or an error code on failure.
> + */
> +int ice_sf_driver_register(void)
> +{
> +	return auxiliary_driver_register(&ice_sf_driver);
> +}
> +
> +/**
> + * ice_sf_driver_unregister - Unregister new auxiliary subfunction driver
> + *
> + * Return: zero on success or an error code on failure.


this function doesn't return anything...

> + */
> +void ice_sf_driver_unregister(void)
> +{
> +	auxiliary_driver_unregister(&ice_sf_driver);
> +}
> diff --git a/drivers/net/ethernet/intel/ice/ice_sf_eth.h b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
> index a08f8b2bceef..e972c50f96c9 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sf_eth.h
> +++ b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
> @@ -18,4 +18,13 @@ struct ice_sf_priv {
>   	struct devlink_port devlink_port;
>   };
>   
> +static inline struct
> +ice_sf_dev *ice_adev_to_sf_dev(struct auxiliary_device *adev)
> +{
> +	return container_of(adev, struct ice_sf_dev, adev);
> +}
> +
> +int ice_sf_driver_register(void);
> +void ice_sf_driver_unregister(void);
> +
>   #endif /* _ICE_SF_ETH_H_ */
Jiri Pirko April 18, 2024, 1:02 p.m. UTC | #2 
Wed, Apr 17, 2024 at 04:20:26PM CEST, michal.swiatkowski@linux.intel.com wrote:
>From: Piotr Raczynski <piotr.raczynski@intel.com>

[...]


>+static int ice_sf_dev_probe(struct auxiliary_device *adev,
>+			    const struct auxiliary_device_id *id)
>+{
>+	struct ice_sf_dev *sf_dev = ice_adev_to_sf_dev(adev);
>+	struct ice_dynamic_port *dyn_port = sf_dev->dyn_port;
>+	struct ice_vsi_cfg_params params = {};
>+	struct ice_vsi *vsi = dyn_port->vsi;
>+	struct ice_pf *pf = dyn_port->pf;
>+	struct device *dev = &adev->dev;
>+	struct ice_sf_priv *priv;
>+	struct devlink *devlink;
>+	int err;
>+
>+	params.type = ICE_VSI_SF;
>+	params.pi = pf->hw.port_info;
>+	params.flags = ICE_VSI_FLAG_INIT;
>+
>+	priv = ice_allocate_sf(&adev->dev);
>+	if (!priv) {
>+		dev_err(dev, "Subfunction devlink alloc failed");
>+		return -ENOMEM;
>+	}
>+
>+	priv->dev = sf_dev;
>+	sf_dev->priv = priv;
>+	devlink = priv_to_devlink(priv);
>+
>+	devlink_register(devlink);

Do register at the very end. Btw, currently the error path seems to be
broken, leaving devlink instance allocated and registered.


>+	devl_lock(devlink);
>+
>+	err = ice_vsi_cfg(vsi, &params);
>+	if (err) {
>+		dev_err(dev, "Subfunction vsi config failed");
>+		goto err_devlink_unlock;
>+	}
>+
>+	err = ice_devlink_create_sf_dev_port(sf_dev);
>+	if (err) {
>+		dev_err(dev, "Cannot add ice virtual devlink port for subfunction");
>+		goto err_vsi_decfg;
>+	}
>+
>+	err = ice_fltr_add_mac_and_broadcast(vsi, vsi->netdev->dev_addr,
>+					     ICE_FWD_TO_VSI);
>+	if (err) {
>+		dev_err(dev, "can't add MAC filters %pM for VSI %d\n",
>+			vsi->netdev->dev_addr, vsi->idx);
>+		goto err_devlink_destroy;
>+	}
>+
>+	ice_napi_add(vsi);
>+	devl_unlock(devlink);
>+
>+	return 0;
>+
>+err_devlink_destroy:
>+	ice_devlink_destroy_sf_dev_port(sf_dev);
>+err_vsi_decfg:
>+	ice_vsi_decfg(vsi);
>+err_devlink_unlock:
>+	devl_unlock(devlink);
>+	return err;

[...]
Michal Swiatkowski April 18, 2024, 2:47 p.m. UTC | #3 
On Thu, Apr 18, 2024 at 03:02:21PM +0200, Jiri Pirko wrote:
> Wed, Apr 17, 2024 at 04:20:26PM CEST, michal.swiatkowski@linux.intel.com wrote:
> >From: Piotr Raczynski <piotr.raczynski@intel.com>
> 
> [...]
> 
> 
> >+static int ice_sf_dev_probe(struct auxiliary_device *adev,
> >+			    const struct auxiliary_device_id *id)
> >+{
> >+	struct ice_sf_dev *sf_dev = ice_adev_to_sf_dev(adev);
> >+	struct ice_dynamic_port *dyn_port = sf_dev->dyn_port;
> >+	struct ice_vsi_cfg_params params = {};
> >+	struct ice_vsi *vsi = dyn_port->vsi;
> >+	struct ice_pf *pf = dyn_port->pf;
> >+	struct device *dev = &adev->dev;
> >+	struct ice_sf_priv *priv;
> >+	struct devlink *devlink;
> >+	int err;
> >+
> >+	params.type = ICE_VSI_SF;
> >+	params.pi = pf->hw.port_info;
> >+	params.flags = ICE_VSI_FLAG_INIT;
> >+
> >+	priv = ice_allocate_sf(&adev->dev);
> >+	if (!priv) {
> >+		dev_err(dev, "Subfunction devlink alloc failed");
> >+		return -ENOMEM;
> >+	}
> >+
> >+	priv->dev = sf_dev;
> >+	sf_dev->priv = priv;
> >+	devlink = priv_to_devlink(priv);
> >+
> >+	devlink_register(devlink);
> 
> Do register at the very end. Btw, currently the error path seems to be
> broken, leaving devlink instance allocated and registered.
> 

Sure, I will fix it.

> 
> >+	devl_lock(devlink);
> >+
> >+	err = ice_vsi_cfg(vsi, &params);
> >+	if (err) {
> >+		dev_err(dev, "Subfunction vsi config failed");
> >+		goto err_devlink_unlock;
> >+	}
> >+
> >+	err = ice_devlink_create_sf_dev_port(sf_dev);
> >+	if (err) {
> >+		dev_err(dev, "Cannot add ice virtual devlink port for subfunction");
> >+		goto err_vsi_decfg;
> >+	}
> >+
> >+	err = ice_fltr_add_mac_and_broadcast(vsi, vsi->netdev->dev_addr,
> >+					     ICE_FWD_TO_VSI);
> >+	if (err) {
> >+		dev_err(dev, "can't add MAC filters %pM for VSI %d\n",
> >+			vsi->netdev->dev_addr, vsi->idx);
> >+		goto err_devlink_destroy;
> >+	}
> >+
> >+	ice_napi_add(vsi);
> >+	devl_unlock(devlink);
> >+
> >+	return 0;
> >+
> >+err_devlink_destroy:
> >+	ice_devlink_destroy_sf_dev_port(sf_dev);
> >+err_vsi_decfg:
> >+	ice_vsi_decfg(vsi);
> >+err_devlink_unlock:
> >+	devl_unlock(devlink);
> >+	return err;
> 
> [...]
Michal Swiatkowski April 18, 2024, 2:47 p.m. UTC | #4 
On Thu, Apr 18, 2024 at 10:57:17AM +0300, Shay Drori wrote:
> 
> On 17/04/2024 17:20, Michal Swiatkowski wrote:
> > From: Piotr Raczynski <piotr.raczynski@intel.com>
> > 
> > Implement subfunction driver. It is probe when subfunction port is
> > activated.
> > 
> > VSI is already created. During the probe VSI is being configured.
> > MAC unicast and broadcast filter is added to allow traffic to pass.
> > 
> > Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> > Signed-off-by: Piotr Raczynski <piotr.raczynski@intel.com>
> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > ---
> >   drivers/net/ethernet/intel/ice/Makefile     |   1 +
> >   drivers/net/ethernet/intel/ice/ice_main.c   |  10 ++
> >   drivers/net/ethernet/intel/ice/ice_sf_eth.c | 140 ++++++++++++++++++++
> >   drivers/net/ethernet/intel/ice/ice_sf_eth.h |   9 ++
> >   4 files changed, 160 insertions(+)
> >   create mode 100644 drivers/net/ethernet/intel/ice/ice_sf_eth.c
> 
> 
> <...>
> 
> > +
> > +/**
> > + * ice_sf_driver_register - Register new auxiliary subfunction driver
> > + *
> > + * Return: zero on success or an error code on failure.
> > + */
> > +int ice_sf_driver_register(void)
> > +{
> > +	return auxiliary_driver_register(&ice_sf_driver);
> > +}
> > +
> > +/**
> > + * ice_sf_driver_unregister - Unregister new auxiliary subfunction driver
> > + *
> > + * Return: zero on success or an error code on failure.
> 
> 
> this function doesn't return anything...
> 

Thanks, will remove it.

> > + */
> > +void ice_sf_driver_unregister(void)
> > +{
> > +	auxiliary_driver_unregister(&ice_sf_driver);
> > +}
> > diff --git a/drivers/net/ethernet/intel/ice/ice_sf_eth.h b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
> > index a08f8b2bceef..e972c50f96c9 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_sf_eth.h
> > +++ b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
> > @@ -18,4 +18,13 @@ struct ice_sf_priv {
> >   	struct devlink_port devlink_port;
> >   };
> > +static inline struct
> > +ice_sf_dev *ice_adev_to_sf_dev(struct auxiliary_device *adev)
> > +{
> > +	return container_of(adev, struct ice_sf_dev, adev);
> > +}
> > +
> > +int ice_sf_driver_register(void);
> > +void ice_sf_driver_unregister(void);
> > +
> >   #endif /* _ICE_SF_ETH_H_ */
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/Makefile b/drivers/net/ethernet/intel/ice/Makefile
index 03500e28ac99..4d987f5dcdc1 100644
--- a/drivers/net/ethernet/intel/ice/Makefile
+++ b/drivers/net/ethernet/intel/ice/Makefile
@@ -31,6 +31,7 @@  ice-y := ice_main.o	\
 	 ice_idc.o	\
 	 devlink/devlink.o	\
 	 devlink/devlink_port.o \
+	 ice_sf_eth.o	\
 	 ice_ddp.o	\
 	 ice_fw_update.o \
 	 ice_lag.o	\
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 29552598ddb6..f55e3340b608 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -15,6 +15,7 @@ 
 #include "ice_dcb_nl.h"
 #include "devlink/devlink.h"
 #include "devlink/devlink_port.h"
+#include "ice_sf_eth.h"
 #include "ice_hwmon.h"
 /* Including ice_trace.h with CREATE_TRACE_POINTS defined will generate the
  * ice tracepoint functions. This must be done exactly once across the
@@ -5857,8 +5858,16 @@  static int __init ice_module_init(void)
 		goto err_dest_lag_wq;
 	}
 
+	status = ice_sf_driver_register();
+	if (status) {
+		pr_err("Failed to register SF driver, err %d\n", status);
+		goto err_sf_driver;
+	}
+
 	return 0;
 
+err_sf_driver:
+	pci_unregister_driver(&ice_driver);
 err_dest_lag_wq:
 	destroy_workqueue(ice_lag_wq);
 	ice_debugfs_exit();
@@ -5876,6 +5885,7 @@  module_init(ice_module_init);
  */
 static void __exit ice_module_exit(void)
 {
+	ice_sf_driver_unregister();
 	pci_unregister_driver(&ice_driver);
 	ice_debugfs_exit();
 	destroy_workqueue(ice_wq);
diff --git a/drivers/net/ethernet/intel/ice/ice_sf_eth.c b/drivers/net/ethernet/intel/ice/ice_sf_eth.c
new file mode 100644
index 000000000000..6ecc13217544
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_sf_eth.c
@@ -0,0 +1,140 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024, Intel Corporation. */
+#include "ice.h"
+#include "ice_lib.h"
+#include "ice_fltr.h"
+#include "ice_sf_eth.h"
+#include "devlink/devlink_port.h"
+#include "devlink/devlink.h"
+
+/**
+ * ice_sf_dev_probe - subfunction driver probe function
+ * @adev: pointer to the auxiliary device
+ * @id: pointer to the auxiliary_device id
+ *
+ * Configure VSI and netdev resources for the subfunction device.
+ *
+ * Return: zero on success or an error code on failure.
+ */
+static int ice_sf_dev_probe(struct auxiliary_device *adev,
+			    const struct auxiliary_device_id *id)
+{
+	struct ice_sf_dev *sf_dev = ice_adev_to_sf_dev(adev);
+	struct ice_dynamic_port *dyn_port = sf_dev->dyn_port;
+	struct ice_vsi_cfg_params params = {};
+	struct ice_vsi *vsi = dyn_port->vsi;
+	struct ice_pf *pf = dyn_port->pf;
+	struct device *dev = &adev->dev;
+	struct ice_sf_priv *priv;
+	struct devlink *devlink;
+	int err;
+
+	params.type = ICE_VSI_SF;
+	params.pi = pf->hw.port_info;
+	params.flags = ICE_VSI_FLAG_INIT;
+
+	priv = ice_allocate_sf(&adev->dev);
+	if (!priv) {
+		dev_err(dev, "Subfunction devlink alloc failed");
+		return -ENOMEM;
+	}
+
+	priv->dev = sf_dev;
+	sf_dev->priv = priv;
+	devlink = priv_to_devlink(priv);
+
+	devlink_register(devlink);
+	devl_lock(devlink);
+
+	err = ice_vsi_cfg(vsi, &params);
+	if (err) {
+		dev_err(dev, "Subfunction vsi config failed");
+		goto err_devlink_unlock;
+	}
+
+	err = ice_devlink_create_sf_dev_port(sf_dev);
+	if (err) {
+		dev_err(dev, "Cannot add ice virtual devlink port for subfunction");
+		goto err_vsi_decfg;
+	}
+
+	err = ice_fltr_add_mac_and_broadcast(vsi, vsi->netdev->dev_addr,
+					     ICE_FWD_TO_VSI);
+	if (err) {
+		dev_err(dev, "can't add MAC filters %pM for VSI %d\n",
+			vsi->netdev->dev_addr, vsi->idx);
+		goto err_devlink_destroy;
+	}
+
+	ice_napi_add(vsi);
+	devl_unlock(devlink);
+
+	return 0;
+
+err_devlink_destroy:
+	ice_devlink_destroy_sf_dev_port(sf_dev);
+err_vsi_decfg:
+	ice_vsi_decfg(vsi);
+err_devlink_unlock:
+	devl_unlock(devlink);
+	return err;
+}
+
+/**
+ * ice_sf_dev_remove - subfunction driver remove function
+ * @adev: pointer to the auxiliary device
+ *
+ * Deinitalize VSI and netdev resources for the subfunction device.
+ */
+static void ice_sf_dev_remove(struct auxiliary_device *adev)
+{
+	struct ice_sf_dev *sf_dev = ice_adev_to_sf_dev(adev);
+	struct ice_dynamic_port *dyn_port = sf_dev->dyn_port;
+	struct ice_vsi *vsi = dyn_port->vsi;
+	struct devlink *devlink;
+
+	devlink = priv_to_devlink(sf_dev->priv);
+	devl_lock(devlink);
+
+	ice_vsi_close(vsi);
+
+	ice_devlink_destroy_sf_dev_port(sf_dev);
+	devl_unregister(devlink);
+	devl_unlock(devlink);
+	devlink_free(devlink);
+	ice_vsi_decfg(vsi);
+}
+
+static const struct auxiliary_device_id ice_sf_dev_id_table[] = {
+	{ .name = "ice.sf", },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(auxiliary, ice_sf_dev_id_table);
+
+static struct auxiliary_driver ice_sf_driver = {
+	.name = "sf",
+	.probe = ice_sf_dev_probe,
+	.remove = ice_sf_dev_remove,
+	.id_table = ice_sf_dev_id_table
+};
+
+/**
+ * ice_sf_driver_register - Register new auxiliary subfunction driver
+ *
+ * Return: zero on success or an error code on failure.
+ */
+int ice_sf_driver_register(void)
+{
+	return auxiliary_driver_register(&ice_sf_driver);
+}
+
+/**
+ * ice_sf_driver_unregister - Unregister new auxiliary subfunction driver
+ *
+ * Return: zero on success or an error code on failure.
+ */
+void ice_sf_driver_unregister(void)
+{
+	auxiliary_driver_unregister(&ice_sf_driver);
+}
diff --git a/drivers/net/ethernet/intel/ice/ice_sf_eth.h b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
index a08f8b2bceef..e972c50f96c9 100644
--- a/drivers/net/ethernet/intel/ice/ice_sf_eth.h
+++ b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
@@ -18,4 +18,13 @@  struct ice_sf_priv {
 	struct devlink_port devlink_port;
 };
 
+static inline struct
+ice_sf_dev *ice_adev_to_sf_dev(struct auxiliary_device *adev)
+{
+	return container_of(adev, struct ice_sf_dev, adev);
+}
+
+int ice_sf_driver_register(void);
+void ice_sf_driver_unregister(void);
+
 #endif /* _ICE_SF_ETH_H_ */