| Message ID | 1688544973-2507-1-git-send-email-schakrabarti@linux.microsoft.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [V5,net] net: mana: Fix MANA VF unload when hardware is unresponsive | expand |
On Wed, 2023-07-05 at 01:16 -0700, Souradeep Chakrabarti wrote: > When unloading the MANA driver, mana_dealloc_queues() waits for the MANA > hardware to complete any inflight packets and set the pending send count > to zero. But if the hardware has failed, mana_dealloc_queues() > could wait forever. > > Fix this by adding a timeout to the wait. Set the timeout to 120 seconds, > which is a somewhat arbitrary value that is more than long enough for > functional hardware to complete any sends. > > Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)") > --- > V4 -> V5: > * Added fixes tag > * Changed the usleep_range from static to incremental value. > * Initialized timeout in the begining. > --- > Signed-off-by: Souradeep Chakrabarti <schakrabarti@linux.microsoft.com> the changelog should come after the SoB tag, and there should be no '-- - ' separator before the SoB. Please double-check your patch with the checkpatch script before the next submission, it should catch trivial issues as the above one. > --- > drivers/net/ethernet/microsoft/mana/mana_en.c | 30 ++++++++++++++++--- > 1 file changed, 26 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c > index a499e460594b..56b7074db1a2 100644 > --- a/drivers/net/ethernet/microsoft/mana/mana_en.c > +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c > @@ -2345,9 +2345,13 @@ int mana_attach(struct net_device *ndev) > static int mana_dealloc_queues(struct net_device *ndev) > { > struct mana_port_context *apc = netdev_priv(ndev); > + unsigned long timeout = jiffies + 120 * HZ; > struct gdma_dev *gd = apc->ac->gdma_dev; > struct mana_txq *txq; > + struct sk_buff *skb; > + struct mana_cq *cq; > int i, err; > + u32 tsleep; > > if (apc->port_is_up) > return -EINVAL; > @@ -2363,15 +2367,33 @@ static int mana_dealloc_queues(struct net_device *ndev) > * to false, but it doesn't matter since mana_start_xmit() drops any > * new packets due to apc->port_is_up being false. > * > - * Drain all the in-flight TX packets > + * Drain all the in-flight TX packets. > + * A timeout of 120 seconds for all the queues is used. > + * This will break the while loop when h/w is not responding. > + * This value of 120 has been decided here considering max > + * number of queues. > */ > + > for (i = 0; i < apc->num_queues; i++) { > txq = &apc->tx_qp[i].txq; > - > - while (atomic_read(&txq->pending_sends) > 0) > - usleep_range(1000, 2000); > + tsleep = 1000; > + while (atomic_read(&txq->pending_sends) > 0 && > + time_before(jiffies, timeout)) { > + usleep_range(tsleep, tsleep << 1); > + tsleep <<= 1; > + } > } > > + for (i = 0; i < apc->num_queues; i++) { > + txq = &apc->tx_qp[i].txq; > + cq = &apc->tx_qp[i].tx_cq; The above variable is unused, and causes a build warning. Please remove the assignment and the variable declaration. Thanks, Paolo
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c index a499e460594b..56b7074db1a2 100644 --- a/drivers/net/ethernet/microsoft/mana/mana_en.c +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c @@ -2345,9 +2345,13 @@ int mana_attach(struct net_device *ndev) static int mana_dealloc_queues(struct net_device *ndev) { struct mana_port_context *apc = netdev_priv(ndev); + unsigned long timeout = jiffies + 120 * HZ; struct gdma_dev *gd = apc->ac->gdma_dev; struct mana_txq *txq; + struct sk_buff *skb; + struct mana_cq *cq; int i, err; + u32 tsleep; if (apc->port_is_up) return -EINVAL; @@ -2363,15 +2367,33 @@ static int mana_dealloc_queues(struct net_device *ndev) * to false, but it doesn't matter since mana_start_xmit() drops any * new packets due to apc->port_is_up being false. * - * Drain all the in-flight TX packets + * Drain all the in-flight TX packets. + * A timeout of 120 seconds for all the queues is used. + * This will break the while loop when h/w is not responding. + * This value of 120 has been decided here considering max + * number of queues. */ + for (i = 0; i < apc->num_queues; i++) { txq = &apc->tx_qp[i].txq; - - while (atomic_read(&txq->pending_sends) > 0) - usleep_range(1000, 2000); + tsleep = 1000; + while (atomic_read(&txq->pending_sends) > 0 && + time_before(jiffies, timeout)) { + usleep_range(tsleep, tsleep << 1); + tsleep <<= 1; + } } + for (i = 0; i < apc->num_queues; i++) { + txq = &apc->tx_qp[i].txq; + cq = &apc->tx_qp[i].tx_cq; + while (atomic_read(&txq->pending_sends)) { + skb = skb_dequeue(&txq->pending_skbs); + mana_unmap_skb(skb, apc); + dev_consume_skb_any(skb); + atomic_sub(1, &txq->pending_sends); + } + } /* We're 100% sure the queues can no longer be woken up, because * we're sure now mana_poll_tx_cq() can't be running. */
When unloading the MANA driver, mana_dealloc_queues() waits for the MANA hardware to complete any inflight packets and set the pending send count to zero. But if the hardware has failed, mana_dealloc_queues() could wait forever. Fix this by adding a timeout to the wait. Set the timeout to 120 seconds, which is a somewhat arbitrary value that is more than long enough for functional hardware to complete any sends. Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)") --- V4 -> V5: * Added fixes tag * Changed the usleep_range from static to incremental value. * Initialized timeout in the begining. --- Signed-off-by: Souradeep Chakrabarti <schakrabarti@linux.microsoft.com> --- drivers/net/ethernet/microsoft/mana/mana_en.c | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-)