Skip to content

Commit

Permalink
ice: Fix ASSERT_RTNL() warning during certain scenarios
Browse files Browse the repository at this point in the history
Commit 91fdbce ("ice: Add support in the driver for associating
queue with napi") invoked the netif_queue_set_napi() call. This
kernel function requires to be called with rtnl_lock taken,
otherwise ASSERT_RTNL() warning will be triggered. ice_vsi_rebuild()
initiating this call is under rtnl_lock when the rebuild is in
response to configuration changes from external interfaces (such as
tc, ethtool etc. which holds the lock). But, the VSI rebuild
generated from service tasks and resets (PFR/CORER/GLOBR) is not
under rtnl lock protection. Handle these cases as well to hold lock
before the kernel call (by setting the 'locked' boolean to false).

netif_queue_set_napi() is also used to clear previously set napi
in the q_vector unroll flow. Handle this for locked/lockless execution
paths.

Fixes: 91fdbce ("ice: Add support in the driver for associating queue with napi")
Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
  • Loading branch information
anambiarin authored and anguy11 committed Feb 20, 2024
1 parent ee89921 commit 080b0c8
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 26 deletions.
10 changes: 4 additions & 6 deletions drivers/net/ethernet/intel/ice/ice_base.c
Original file line number Diff line number Diff line change
Expand Up @@ -190,15 +190,13 @@ static void ice_free_q_vector(struct ice_vsi *vsi, int v_idx)
q_vector = vsi->q_vectors[v_idx];

ice_for_each_tx_ring(tx_ring, q_vector->tx) {
if (vsi->netdev)
netif_queue_set_napi(vsi->netdev, tx_ring->q_index,
NETDEV_QUEUE_TYPE_TX, NULL);
ice_queue_set_napi(vsi, tx_ring->q_index, NETDEV_QUEUE_TYPE_TX,
NULL);
tx_ring->q_vector = NULL;
}
ice_for_each_rx_ring(rx_ring, q_vector->rx) {
if (vsi->netdev)
netif_queue_set_napi(vsi->netdev, rx_ring->q_index,
NETDEV_QUEUE_TYPE_RX, NULL);
ice_queue_set_napi(vsi, rx_ring->q_index, NETDEV_QUEUE_TYPE_RX,
NULL);
rx_ring->q_vector = NULL;
}

Expand Down
86 changes: 69 additions & 17 deletions drivers/net/ethernet/intel/ice/ice_lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -2426,7 +2426,7 @@ ice_vsi_cfg_def(struct ice_vsi *vsi, struct ice_vsi_cfg_params *params)
ice_vsi_map_rings_to_vectors(vsi);

/* Associate q_vector rings to napi */
ice_vsi_set_napi_queues(vsi, true);
ice_vsi_set_napi_queues(vsi);

vsi->stat_offsets_loaded = false;

Expand Down Expand Up @@ -2904,19 +2904,19 @@ void ice_vsi_dis_irq(struct ice_vsi *vsi)
}

/**
* ice_queue_set_napi - Set the napi instance for the queue
* __ice_queue_set_napi - Set the napi instance for the queue
* @dev: device to which NAPI and queue belong
* @queue_index: Index of queue
* @type: queue type as RX or TX
* @napi: NAPI context
* @locked: is the rtnl_lock already held
*
* Set the napi instance for the queue
* Set the napi instance for the queue. Caller indicates the lock status.
*/
static void
ice_queue_set_napi(struct net_device *dev, unsigned int queue_index,
enum netdev_queue_type type, struct napi_struct *napi,
bool locked)
__ice_queue_set_napi(struct net_device *dev, unsigned int queue_index,
enum netdev_queue_type type, struct napi_struct *napi,
bool locked)
{
if (!locked)
rtnl_lock();
Expand All @@ -2926,46 +2926,98 @@ ice_queue_set_napi(struct net_device *dev, unsigned int queue_index,
}

/**
* ice_q_vector_set_napi_queues - Map queue[s] associated with the napi
* ice_queue_set_napi - Set the napi instance for the queue
* @vsi: VSI being configured
* @queue_index: Index of queue
* @type: queue type as RX or TX
* @napi: NAPI context
*
* Set the napi instance for the queue. The rtnl lock state is derived from the
* execution path.
*/
void
ice_queue_set_napi(struct ice_vsi *vsi, unsigned int queue_index,
enum netdev_queue_type type, struct napi_struct *napi)
{
struct ice_pf *pf = vsi->back;

if (!vsi->netdev)
return;

if (current_work() == &pf->serv_task ||
test_bit(ICE_PREPARED_FOR_RESET, pf->state) ||
test_bit(ICE_DOWN, pf->state) ||
test_bit(ICE_SUSPENDED, pf->state))
__ice_queue_set_napi(vsi->netdev, queue_index, type, napi,
false);
else
__ice_queue_set_napi(vsi->netdev, queue_index, type, napi,
true);
}

/**
* __ice_q_vector_set_napi_queues - Map queue[s] associated with the napi
* @q_vector: q_vector pointer
* @locked: is the rtnl_lock already held
*
* Associate the q_vector napi with all the queue[s] on the vector.
* Caller indicates the lock status.
*/
void __ice_q_vector_set_napi_queues(struct ice_q_vector *q_vector, bool locked)
{
struct ice_rx_ring *rx_ring;
struct ice_tx_ring *tx_ring;

ice_for_each_rx_ring(rx_ring, q_vector->rx)
__ice_queue_set_napi(q_vector->vsi->netdev, rx_ring->q_index,
NETDEV_QUEUE_TYPE_RX, &q_vector->napi,
locked);

ice_for_each_tx_ring(tx_ring, q_vector->tx)
__ice_queue_set_napi(q_vector->vsi->netdev, tx_ring->q_index,
NETDEV_QUEUE_TYPE_TX, &q_vector->napi,
locked);
/* Also set the interrupt number for the NAPI */
netif_napi_set_irq(&q_vector->napi, q_vector->irq.virq);
}

/**
* ice_q_vector_set_napi_queues - Map queue[s] associated with the napi
* @q_vector: q_vector pointer
*
* Associate the q_vector napi with all the queue[s] on the vector
*/
void ice_q_vector_set_napi_queues(struct ice_q_vector *q_vector, bool locked)
void ice_q_vector_set_napi_queues(struct ice_q_vector *q_vector)
{
struct ice_rx_ring *rx_ring;
struct ice_tx_ring *tx_ring;

ice_for_each_rx_ring(rx_ring, q_vector->rx)
ice_queue_set_napi(q_vector->vsi->netdev, rx_ring->q_index,
NETDEV_QUEUE_TYPE_RX, &q_vector->napi,
locked);
ice_queue_set_napi(q_vector->vsi, rx_ring->q_index,
NETDEV_QUEUE_TYPE_RX, &q_vector->napi);

ice_for_each_tx_ring(tx_ring, q_vector->tx)
ice_queue_set_napi(q_vector->vsi->netdev, tx_ring->q_index,
NETDEV_QUEUE_TYPE_TX, &q_vector->napi,
locked);
ice_queue_set_napi(q_vector->vsi, tx_ring->q_index,
NETDEV_QUEUE_TYPE_TX, &q_vector->napi);
/* Also set the interrupt number for the NAPI */
netif_napi_set_irq(&q_vector->napi, q_vector->irq.virq);
}

/**
* ice_vsi_set_napi_queues
* @vsi: VSI pointer
* @locked: is the rtnl_lock already held
*
* Associate queue[s] with napi for all vectors
*/
void ice_vsi_set_napi_queues(struct ice_vsi *vsi, bool locked)
void ice_vsi_set_napi_queues(struct ice_vsi *vsi)
{
int i;

if (!vsi->netdev)
return;

ice_for_each_q_vector(vsi, i)
ice_q_vector_set_napi_queues(vsi->q_vectors[i], locked);
ice_q_vector_set_napi_queues(vsi->q_vectors[i]);
}

/**
Expand Down
10 changes: 8 additions & 2 deletions drivers/net/ethernet/intel/ice/ice_lib.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,15 @@ void ice_vsi_cfg_netdev_tc(struct ice_vsi *vsi, u8 ena_tc);
struct ice_vsi *
ice_vsi_setup(struct ice_pf *pf, struct ice_vsi_cfg_params *params);

void ice_q_vector_set_napi_queues(struct ice_q_vector *q_vector, bool locked);
void
ice_queue_set_napi(struct ice_vsi *vsi, unsigned int queue_index,
enum netdev_queue_type type, struct napi_struct *napi);

void __ice_q_vector_set_napi_queues(struct ice_q_vector *q_vector, bool locked);

void ice_q_vector_set_napi_queues(struct ice_q_vector *q_vector);

void ice_vsi_set_napi_queues(struct ice_vsi *vsi, bool locked);
void ice_vsi_set_napi_queues(struct ice_vsi *vsi);

int ice_vsi_release(struct ice_vsi *vsi);

Expand Down
3 changes: 2 additions & 1 deletion drivers/net/ethernet/intel/ice/ice_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -3495,7 +3495,7 @@ static void ice_napi_add(struct ice_vsi *vsi)
ice_for_each_q_vector(vsi, v_idx) {
netif_napi_add(vsi->netdev, &vsi->q_vectors[v_idx]->napi,
ice_napi_poll);
ice_q_vector_set_napi_queues(vsi->q_vectors[v_idx], false);
__ice_q_vector_set_napi_queues(vsi->q_vectors[v_idx], false);
}
}

Expand Down Expand Up @@ -5447,6 +5447,7 @@ static int ice_reinit_interrupt_scheme(struct ice_pf *pf)
if (ret)
goto err_reinit;
ice_vsi_map_rings_to_vectors(pf->vsi[v]);
ice_vsi_set_napi_queues(pf->vsi[v]);
}

ret = ice_req_irq_msix_misc(pf);
Expand Down

0 comments on commit 080b0c8

Please sign in to comment.