diff mbox series

[iwl-next,v4,8/8] ice: allow to activate and deactivate subfunction

Message ID 20240417142028.2171-9-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: 926 this patch: 926
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 6 maintainers not CCed: pabeni@redhat.com jesse.brandeburg@intel.com kuba@kernel.org edumazet@google.com piotr.raczynski@intel.com anthony.l.nguyen@intel.com
netdev/build_clang success Errors and warnings before: 937 this patch: 937
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: 937 this patch: 937
netdev/checkpatch warning WARNING: line length of 84 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 2 this patch: 2
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>

Use previously implemented SF aux driver. It is probe during SF
activation and remove after deactivation.

Signed-off-by: Piotr Raczynski <piotr.raczynski@intel.com>
Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
---
 .../ethernet/intel/ice/devlink/devlink_port.c | 106 ++++++++++++++++++
 .../ethernet/intel/ice/devlink/devlink_port.h |   5 +
 drivers/net/ethernet/intel/ice/ice_sf_eth.c   | 104 +++++++++++++++++
 drivers/net/ethernet/intel/ice/ice_sf_eth.h   |   3 +
 4 files changed, 218 insertions(+)

Comments

Shay Drori April 18, 2024, 8:12 a.m. UTC | #1 
resend as plain test

On 18/04/2024 10:53, Shay Drori wrote:
> On 17/04/2024 17:20, Michal Swiatkowski wrote:
>> +/**
>> + * ice_devlink_port_fn_state_get - devlink handler for port state get
>> + * @port: pointer to devlink port
>> + * @state: admin configured state of the port
>> + * @opstate: current port operational state
>> + * @extack: extack for reporting error messages
>> + *
>> + * Gets port state.
>> + *
>> + * Return: zero on success or an error code on failure.
>> + */
>> +static int
>> +ice_devlink_port_fn_state_get(struct devlink_port *port,
>> +			      enum devlink_port_fn_state *state,
>> +			      enum devlink_port_fn_opstate *opstate,
>> +			      struct netlink_ext_ack *extack)
>> +{
>> +	struct ice_dynamic_port *dyn_port;
>> +
>> +	dyn_port = ice_devlink_port_to_dyn(port);
>> +
>> +	if (dyn_port->active) {
>> +		*state = DEVLINK_PORT_FN_STATE_ACTIVE;
>> +		*opstate = DEVLINK_PORT_FN_OPSTATE_ATTACHED;
> 
> 
> DEVLINK_PORT_FN_OPSTATE_ATTACHED means the SF is up/bind[1].
> ice is using auxiliary bus for SFs, which means user can unbind it
> via the auxiliary sysfs (/sys/bus/auxiliary/drivers/ice_sf/unbind).
> In this case[2], you need to return:
> *state = DEVLINK_PORT_FN_STATE_ACTIVE;
> *opstate = DEVLINK_PORT_FN_OPSTATE_DETACHED;
> 
> 
> [1]
> Documentation from include/uapi/linux/devlink.h:
> 
> * @DEVLINK_PORT_FN_OPSTATE_ATTACHED: Driver is attached to the function. 
> <...>
> * @DEVLINK_PORT_FN_OPSTATE_DETACHED: Driver is detached from the function.
> 
>> +	} else {
>> +		*state = DEVLINK_PORT_FN_STATE_INACTIVE;
>> +		*opstate = DEVLINK_PORT_FN_OPSTATE_DETACHED;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
Michal Swiatkowski April 18, 2024, 11:55 a.m. UTC | #2 
On Thu, Apr 18, 2024 at 11:12:47AM +0300, Shay Drori wrote:
> resend as plain test
> 
> On 18/04/2024 10:53, Shay Drori wrote:
> > On 17/04/2024 17:20, Michal Swiatkowski wrote:
> > > +/**
> > > + * ice_devlink_port_fn_state_get - devlink handler for port state get
> > > + * @port: pointer to devlink port
> > > + * @state: admin configured state of the port
> > > + * @opstate: current port operational state
> > > + * @extack: extack for reporting error messages
> > > + *
> > > + * Gets port state.
> > > + *
> > > + * Return: zero on success or an error code on failure.
> > > + */
> > > +static int
> > > +ice_devlink_port_fn_state_get(struct devlink_port *port,
> > > +			      enum devlink_port_fn_state *state,
> > > +			      enum devlink_port_fn_opstate *opstate,
> > > +			      struct netlink_ext_ack *extack)
> > > +{
> > > +	struct ice_dynamic_port *dyn_port;
> > > +
> > > +	dyn_port = ice_devlink_port_to_dyn(port);
> > > +
> > > +	if (dyn_port->active) {
> > > +		*state = DEVLINK_PORT_FN_STATE_ACTIVE;
> > > +		*opstate = DEVLINK_PORT_FN_OPSTATE_ATTACHED;
> > 
> > 
> > DEVLINK_PORT_FN_OPSTATE_ATTACHED means the SF is up/bind[1].
> > ice is using auxiliary bus for SFs, which means user can unbind it
> > via the auxiliary sysfs (/sys/bus/auxiliary/drivers/ice_sf/unbind).
> > In this case[2], you need to return:
> > *state = DEVLINK_PORT_FN_STATE_ACTIVE;
> > *opstate = DEVLINK_PORT_FN_OPSTATE_DETACHED;
> > 

Thanks, I didn't think about unbinding/binding the aux driver via sysfs.

To be sure:
- user create the subfunction:
INACTIVE, DETACHED
- user activate it:
ACTIVE, ATTACHED
- user unbind driver:
ACTIVE, DETACHED
- user can bind it again as long as subfunction port is ACTIVE
is it right?

I will fix the comment from previous patch and add state tracking for
ATTACHED/DETACHED.

Thanks,
Michal

> > 
> > [1]
> > Documentation from include/uapi/linux/devlink.h:
> > 
> > * @DEVLINK_PORT_FN_OPSTATE_ATTACHED: Driver is attached to the function.
> > <...>
> > * @DEVLINK_PORT_FN_OPSTATE_DETACHED: Driver is detached from the function.
> > 
> > > +	} else {
> > > +		*state = DEVLINK_PORT_FN_STATE_INACTIVE;
> > > +		*opstate = DEVLINK_PORT_FN_OPSTATE_DETACHED;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
Shay Drori April 18, 2024, 12:20 p.m. UTC | #3 
On 18/04/2024 14:55, Michal Swiatkowski wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Thu, Apr 18, 2024 at 11:12:47AM +0300, Shay Drori wrote:
>> resend as plain test
>>
>> On 18/04/2024 10:53, Shay Drori wrote:
>>> On 17/04/2024 17:20, Michal Swiatkowski wrote:
>>>> +/**
>>>> + * ice_devlink_port_fn_state_get - devlink handler for port state get
>>>> + * @port: pointer to devlink port
>>>> + * @state: admin configured state of the port
>>>> + * @opstate: current port operational state
>>>> + * @extack: extack for reporting error messages
>>>> + *
>>>> + * Gets port state.
>>>> + *
>>>> + * Return: zero on success or an error code on failure.
>>>> + */
>>>> +static int
>>>> +ice_devlink_port_fn_state_get(struct devlink_port *port,
>>>> +                       enum devlink_port_fn_state *state,
>>>> +                       enum devlink_port_fn_opstate *opstate,
>>>> +                       struct netlink_ext_ack *extack)
>>>> +{
>>>> + struct ice_dynamic_port *dyn_port;
>>>> +
>>>> + dyn_port = ice_devlink_port_to_dyn(port);
>>>> +
>>>> + if (dyn_port->active) {
>>>> +         *state = DEVLINK_PORT_FN_STATE_ACTIVE;
>>>> +         *opstate = DEVLINK_PORT_FN_OPSTATE_ATTACHED;
>>>
>>>
>>> DEVLINK_PORT_FN_OPSTATE_ATTACHED means the SF is up/bind[1].
>>> ice is using auxiliary bus for SFs, which means user can unbind it
>>> via the auxiliary sysfs (/sys/bus/auxiliary/drivers/ice_sf/unbind).
>>> In this case[2], you need to return:
>>> *state = DEVLINK_PORT_FN_STATE_ACTIVE;
>>> *opstate = DEVLINK_PORT_FN_OPSTATE_DETACHED;
>>>
> 
> Thanks, I didn't think about unbinding/binding the aux driver via sysfs. >
> To be sure:
> - user create the subfunction:
> INACTIVE, DETACHED
> - user activate it:
> ACTIVE, ATTACHED
> - user unbind driver:
> ACTIVE, DETACHED
> - user can bind it again as long as subfunction port is ACTIVE
> is it right?


yes.


> 
> I will fix the comment from previous patch and add state tracking for
> ATTACHED/DETACHED.
> 
> Thanks,
> Michal
> 
>>>
>>> [1]
>>> Documentation from include/uapi/linux/devlink.h:
>>>
>>> * @DEVLINK_PORT_FN_OPSTATE_ATTACHED: Driver is attached to the function.
>>> <...>
>>> * @DEVLINK_PORT_FN_OPSTATE_DETACHED: Driver is detached from the function.
>>>
>>>> + } else {
>>>> +         *state = DEVLINK_PORT_FN_STATE_INACTIVE;
>>>> +         *opstate = DEVLINK_PORT_FN_OPSTATE_DETACHED;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink_port.c b/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
index 0ab4d687e311..efff286f9e3e 100644
--- a/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
+++ b/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
@@ -482,6 +482,42 @@  void ice_devlink_destroy_sf_dev_port(struct ice_sf_dev *sf_dev)
 	devl_port_unregister(&sf_dev->priv->devlink_port);
 }
 
+/**
+ * ice_activate_dynamic_port - Activate a dynamic port
+ * @dyn_port: dynamic port instance to activate
+ * @extack: extack for reporting error messages
+ *
+ * Activate the dynamic port based on its flavour.
+ *
+ * Return: zero on success or an error code on failure.
+ */
+static int
+ice_activate_dynamic_port(struct ice_dynamic_port *dyn_port,
+			  struct netlink_ext_ack *extack)
+{
+	int err;
+
+	err = ice_sf_eth_activate(dyn_port, extack);
+	if (err)
+		return err;
+
+	dyn_port->active = true;
+
+	return 0;
+}
+
+/**
+ * ice_deactivate_dynamic_port - Deactivate a dynamic port
+ * @dyn_port: dynamic port instance to deactivate
+ *
+ * Undo activation of a dynamic port.
+ */
+static void ice_deactivate_dynamic_port(struct ice_dynamic_port *dyn_port)
+{
+	ice_sf_eth_deactivate(dyn_port);
+	dyn_port->active = false;
+}
+
 /**
  * ice_dealloc_dynamic_port - Deallocate and remove a dynamic port
  * @dyn_port: dynamic port instance to deallocate
@@ -494,6 +530,9 @@  static void ice_dealloc_dynamic_port(struct ice_dynamic_port *dyn_port)
 	struct devlink_port *devlink_port = &dyn_port->devlink_port;
 	struct ice_pf *pf = dyn_port->pf;
 
+	if (dyn_port->active)
+		ice_deactivate_dynamic_port(dyn_port);
+
 	xa_erase(&pf->sf_nums, devlink_port->attrs.pci_sf.sf);
 	devl_port_unregister(devlink_port);
 	ice_vsi_free(dyn_port->vsi);
@@ -638,10 +677,77 @@  ice_devlink_port_fn_hw_addr_get(struct devlink_port *port, u8 *hw_addr,
 	return 0;
 }
 
+/**
+ * ice_devlink_port_fn_state_set - devlink handler for port state set
+ * @port: pointer to devlink port
+ * @state: state to set
+ * @extack: extack for reporting error messages
+ *
+ * Activates or deactivates the port.
+ *
+ * Return: zero on success or an error code on failure.
+ */
+static int
+ice_devlink_port_fn_state_set(struct devlink_port *port,
+			      enum devlink_port_fn_state state,
+			      struct netlink_ext_ack *extack)
+{
+	struct ice_dynamic_port *dyn_port;
+
+	dyn_port = ice_devlink_port_to_dyn(port);
+
+	switch (state) {
+	case DEVLINK_PORT_FN_STATE_ACTIVE:
+		if (!dyn_port->active)
+			return ice_activate_dynamic_port(dyn_port, extack);
+		break;
+	case DEVLINK_PORT_FN_STATE_INACTIVE:
+		if (dyn_port->active)
+			ice_deactivate_dynamic_port(dyn_port);
+		break;
+	}
+
+	return 0;
+}
+
+/**
+ * ice_devlink_port_fn_state_get - devlink handler for port state get
+ * @port: pointer to devlink port
+ * @state: admin configured state of the port
+ * @opstate: current port operational state
+ * @extack: extack for reporting error messages
+ *
+ * Gets port state.
+ *
+ * Return: zero on success or an error code on failure.
+ */
+static int
+ice_devlink_port_fn_state_get(struct devlink_port *port,
+			      enum devlink_port_fn_state *state,
+			      enum devlink_port_fn_opstate *opstate,
+			      struct netlink_ext_ack *extack)
+{
+	struct ice_dynamic_port *dyn_port;
+
+	dyn_port = ice_devlink_port_to_dyn(port);
+
+	if (dyn_port->active) {
+		*state = DEVLINK_PORT_FN_STATE_ACTIVE;
+		*opstate = DEVLINK_PORT_FN_OPSTATE_ATTACHED;
+	} else {
+		*state = DEVLINK_PORT_FN_STATE_INACTIVE;
+		*opstate = DEVLINK_PORT_FN_OPSTATE_DETACHED;
+	}
+
+	return 0;
+}
+
 static const struct devlink_port_ops ice_devlink_port_sf_ops = {
 	.port_del = ice_devlink_port_del,
 	.port_fn_hw_addr_get = ice_devlink_port_fn_hw_addr_get,
 	.port_fn_hw_addr_set = ice_devlink_port_fn_hw_addr_set,
+	.port_fn_state_get = ice_devlink_port_fn_state_get,
+	.port_fn_state_set = ice_devlink_port_fn_state_set,
 };
 
 /**
diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink_port.h b/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
index 33f07cec6bd6..3692b8617bd5 100644
--- a/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
+++ b/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
@@ -14,6 +14,7 @@ 
  * @devlink_port: the associated devlink port structure
  * @pf: pointer to the PF private structure
  * @vsi: the VSI associated with this port
+ * @sf_dev: pointer to the subfunction device
  *
  * An instance of a dynamically added devlink port. Each port flavour
  */
@@ -23,6 +24,10 @@  struct ice_dynamic_port {
 	struct devlink_port devlink_port;
 	struct ice_pf *pf;
 	struct ice_vsi *vsi;
+	/* Flavour-specific implementation data */
+	union {
+		struct ice_sf_dev *sf_dev;
+	};
 };
 
 void ice_dealloc_all_dynamic_ports(struct ice_pf *pf);
diff --git a/drivers/net/ethernet/intel/ice/ice_sf_eth.c b/drivers/net/ethernet/intel/ice/ice_sf_eth.c
index ecef869c20a1..663bfdcf68b5 100644
--- a/drivers/net/ethernet/intel/ice/ice_sf_eth.c
+++ b/drivers/net/ethernet/intel/ice/ice_sf_eth.c
@@ -202,6 +202,8 @@  static struct auxiliary_driver ice_sf_driver = {
 	.id_table = ice_sf_dev_id_table
 };
 
+static DEFINE_XARRAY_ALLOC1(ice_sf_aux_id);
+
 /**
  * ice_sf_driver_register - Register new auxiliary subfunction driver
  *
@@ -221,3 +223,105 @@  void ice_sf_driver_unregister(void)
 {
 	auxiliary_driver_unregister(&ice_sf_driver);
 }
+
+/**
+ * ice_sf_dev_release - Release device associated with auxiliary device
+ * @device: pointer to the device
+ *
+ * Since most of the code for subfunction deactivation is handled in
+ * the remove handler, here just free tracking resources.
+ */
+static void ice_sf_dev_release(struct device *device)
+{
+	struct auxiliary_device *adev = to_auxiliary_dev(device);
+	struct ice_sf_dev *sf_dev = ice_adev_to_sf_dev(adev);
+
+	xa_erase(&ice_sf_aux_id, adev->id);
+	kfree(sf_dev);
+}
+
+/**
+ * ice_sf_eth_activate - Activate Ethernet subfunction port
+ * @dyn_port: the dynamic port instance for this subfunction
+ * @extack: extack for reporting error messages
+ *
+ * Activate the dynamic port as an Ethernet subfunction. Setup the netdev
+ * resources associated and initialize the auxiliary device.
+ *
+ * Return: zero on success or an error code on failure.
+ */
+int
+ice_sf_eth_activate(struct ice_dynamic_port *dyn_port,
+		    struct netlink_ext_ack *extack)
+{
+	struct ice_pf *pf = dyn_port->pf;
+	struct ice_sf_dev *sf_dev;
+	struct pci_dev *pdev;
+	int err;
+	u32 id;
+
+	err  = xa_alloc(&ice_sf_aux_id, &id, NULL, xa_limit_32b,
+			GFP_KERNEL);
+	if (err) {
+		NL_SET_ERR_MSG_MOD(extack, "Could not allocate subfunction ID");
+		return err;
+	}
+
+	sf_dev = kzalloc(sizeof(*sf_dev), GFP_KERNEL);
+	if (!sf_dev) {
+		err = -ENOMEM;
+		NL_SET_ERR_MSG_MOD(extack, "Could not allocate sf_dev memory");
+		goto xa_erase;
+	}
+	pdev = pf->pdev;
+
+	sf_dev->dyn_port = dyn_port;
+	sf_dev->adev.id = id;
+	sf_dev->adev.name = "sf";
+	sf_dev->adev.dev.release = ice_sf_dev_release;
+	sf_dev->adev.dev.parent = &pdev->dev;
+
+	err = auxiliary_device_init(&sf_dev->adev);
+	if (err) {
+		NL_SET_ERR_MSG_MOD(extack, "Failed to initialize auxiliary device");
+		goto sf_dev_free;
+	}
+
+	err = auxiliary_device_add(&sf_dev->adev);
+	if (err) {
+		NL_SET_ERR_MSG_MOD(extack, "Auxiliary device failed to probe");
+		goto aux_dev_uninit;
+	}
+
+	dyn_port->sf_dev = sf_dev;
+
+	return 0;
+
+aux_dev_uninit:
+	auxiliary_device_uninit(&sf_dev->adev);
+sf_dev_free:
+	kfree(sf_dev);
+xa_erase:
+	xa_erase(&ice_sf_aux_id, id);
+
+	return err;
+}
+
+/**
+ * ice_sf_eth_deactivate - Deactivate Ethernet subfunction port
+ * @dyn_port: the dynamic port instance for this subfunction
+ *
+ * Deactivate the Ethernet subfunction, removing its auxiliary device and the
+ * associated resources.
+ */
+void ice_sf_eth_deactivate(struct ice_dynamic_port *dyn_port)
+{
+	struct ice_sf_dev *sf_dev = dyn_port->sf_dev;
+
+	if (sf_dev) {
+		auxiliary_device_delete(&sf_dev->adev);
+		auxiliary_device_uninit(&sf_dev->adev);
+	}
+
+	dyn_port->sf_dev = NULL;
+}
diff --git a/drivers/net/ethernet/intel/ice/ice_sf_eth.h b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
index e972c50f96c9..c558cad0a183 100644
--- a/drivers/net/ethernet/intel/ice/ice_sf_eth.h
+++ b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
@@ -27,4 +27,7 @@  ice_sf_dev *ice_adev_to_sf_dev(struct auxiliary_device *adev)
 int ice_sf_driver_register(void);
 void ice_sf_driver_unregister(void);
 
+int ice_sf_eth_activate(struct ice_dynamic_port *dyn_port,
+			struct netlink_ext_ack *extack);
+void ice_sf_eth_deactivate(struct ice_dynamic_port *dyn_port);
 #endif /* _ICE_SF_ETH_H_ */