Re: [PATCH net 2/4] ice: Correctly handle aux device when num channels change

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07 Dec 13:10, Tony Nguyen wrote:
From: Dave Ertman <david.m.ertman@xxxxxxxxx>

When the number of channels/queues changes on an interface, it is necessary
to change how those resources are distributed to the auxiliary device for
maintaining RDMA functionality.  To do this, the best way is to unplug, and

Can you please explain how an ethtool can affect RDMA functionality ?
don't you have full bifurcation between the two eth and rdma interfaces ..
then re-plug the auxiliary device.  This will cause all current resource
allocation to be released, and then re-requested under the new state.


I find this really disruptive, changing number of netdev queues to cause
full aux devs restart !

Since the set_channel command from ethtool comes in while holding the RTNL
lock, it is necessary to offset the plugging and unplugging of auxiliary
device to another context.  For this purpose, set the flags for UNPLUG and
PLUG in the PF state, then respond to them in the service task.

Also, since the auxiliary device will be unplugged/plugged at the end of
the flow, it is better to not send the event for TCs changing in the
middle of the flow.  This will prevent a timing issue between the events
and the probe/release calls conflicting.

Fixes: 348048e724a0 ("ice: Implement iidc operations")
Signed-off-by: Dave Ertman <david.m.ertman@xxxxxxxxx>
Tested-by: Gurucharan G <gurucharanx.g@xxxxxxxxx> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@xxxxxxxxx>
---
drivers/net/ethernet/intel/ice/ice.h         | 2 ++
drivers/net/ethernet/intel/ice/ice_ethtool.c | 6 ++++++
drivers/net/ethernet/intel/ice/ice_idc.c     | 3 +++
drivers/net/ethernet/intel/ice/ice_main.c    | 3 +++
4 files changed, 14 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index 001500afc4a6..092e572768fe 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -281,6 +281,7 @@ enum ice_pf_state {
	ICE_FLTR_OVERFLOW_PROMISC,
	ICE_VF_DIS,
	ICE_CFG_BUSY,
+	ICE_SET_CHANNELS,
	ICE_SERVICE_SCHED,
	ICE_SERVICE_DIS,
	ICE_FD_FLUSH_REQ,
@@ -485,6 +486,7 @@ enum ice_pf_flags {
	ICE_FLAG_VF_VLAN_PRUNING,
	ICE_FLAG_LINK_LENIENT_MODE_ENA,
	ICE_FLAG_PLUG_AUX_DEV,
+	ICE_FLAG_UNPLUG_AUX_DEV,
	ICE_FLAG_MTU_CHANGED,
	ICE_FLAG_GNSS,			/* GNSS successfully initialized */
	ICE_PF_FLAGS_NBITS		/* must be last */
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index b7be84bbe72d..37e174a19860 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -3536,6 +3536,8 @@ static int ice_set_channels(struct net_device *dev, struct ethtool_channels *ch)
		return -EINVAL;
	}

+	set_bit(ICE_SET_CHANNELS, pf->state);
+
	ice_vsi_recfg_qs(vsi, new_rx, new_tx);

	if (!netif_is_rxfh_configured(dev))
@@ -3543,6 +3545,10 @@ static int ice_set_channels(struct net_device *dev, struct ethtool_channels *ch)

	/* Update rss_size due to change in Rx queues */
	vsi->rss_size = ice_get_valid_rss_size(&pf->hw, new_rx);
+	clear_bit(ICE_SET_CHANNELS, pf->state);
+

you just set this new state a few lines ago, clearing the bit in the same
function few lines later seems to be an abuse of the pf state machine, couldn't you just pass a parameter to the functions which needed this information ?
+	set_bit(ICE_FLAG_UNPLUG_AUX_DEV, pf->flags);
+	set_bit(ICE_FLAG_PLUG_AUX_DEV, pf->flags);

	return 0;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_idc.c b/drivers/net/ethernet/intel/ice/ice_idc.c
index 895c32bcc8b5..9bf6fa5ed4c8 100644
--- a/drivers/net/ethernet/intel/ice/ice_idc.c
+++ b/drivers/net/ethernet/intel/ice/ice_idc.c
@@ -37,6 +37,9 @@ void ice_send_event_to_aux(struct ice_pf *pf, struct iidc_event *event)
	if (WARN_ON_ONCE(!in_task()))
		return;

+	if (test_bit(ICE_SET_CHANNELS, pf->state))
+		return;
+
	mutex_lock(&pf->adev_mutex);
	if (!pf->adev)
		goto finish;
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index d0f14e73e8da..d58f55a72ab3 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -2300,6 +2300,9 @@ static void ice_service_task(struct work_struct *work)
		}
	}

+	if (test_and_clear_bit(ICE_FLAG_UNPLUG_AUX_DEV, pf->flags))
+		ice_unplug_aux_dev(pf);
+
	if (test_bit(ICE_FLAG_PLUG_AUX_DEV, pf->flags)) {
		/* Plug aux device per request */
		ice_plug_aux_dev(pf);
--
2.35.1




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux