The mvneta controller provides a 8-bit register to update the pending Tx descriptor counter. Then, a maximum of 255 Tx descriptors can be added at once. In the current code the mvneta_txq_pend_desc_add function assumes the caller takes care of this limit. But it is not the case. In some situations (xmit_more flag), more than 255 descriptors are added. When this happens, the Tx descriptor counter register is updated with a wrong value, which breaks the whole Tx queue management.
This patch fixes the issue by allowing the mvneta_txq_pend_desc_add function to process more than 255 Tx descriptors.
Fixes: 2a90f7e1d5d0 ("net: mvneta: add xmit_more support") Cc: stable@vger.kernel.org # 4.11+ Signed-off-by: Simon Guinot simon.guinot@sequanux.org --- drivers/net/ethernet/marvell/mvneta.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c index 64a04975bcf8..027c08ce4e5d 100644 --- a/drivers/net/ethernet/marvell/mvneta.c +++ b/drivers/net/ethernet/marvell/mvneta.c @@ -816,11 +816,14 @@ static void mvneta_txq_pend_desc_add(struct mvneta_port *pp, { u32 val;
- /* Only 255 descriptors can be added at once ; Assume caller - * process TX desriptors in quanta less than 256 - */ - val = pend_desc + txq->pending; - mvreg_write(pp, MVNETA_TXQ_UPDATE_REG(txq->id), val); + pend_desc += txq->pending; + + /* Only 255 Tx descriptors can be added at once */ + while (pend_desc > 0) { + val = min(pend_desc, 255); + mvreg_write(pp, MVNETA_TXQ_UPDATE_REG(txq->id), val); + pend_desc -= val; + } txq->pending = 0; }
@@ -2413,8 +2416,7 @@ static int mvneta_tx(struct sk_buff *skb, struct net_device *dev) if (txq->count >= txq->tx_stop_threshold) netif_tx_stop_queue(nq);
- if (!skb->xmit_more || netif_xmit_stopped(nq) || - txq->pending + frags > MVNETA_TXQ_DEC_SENT_MASK) + if (!skb->xmit_more || netif_xmit_stopped(nq)) mvneta_txq_pend_desc_add(pp, txq, frags); else txq->pending += frags;
From: Simon Guinot
Sent: 08 November 2017 16:59
The mvneta controller provides a 8-bit register to update the pending Tx descriptor counter. Then, a maximum of 255 Tx descriptors can be added at once. In the current code the mvneta_txq_pend_desc_add function assumes the caller takes care of this limit. But it is not the case. In some situations (xmit_more flag), more than 255 descriptors are added. When this happens, the Tx descriptor counter register is updated with a wrong value, which breaks the whole Tx queue management.
Even with xmit_more it is usually best to limit the number (in order to reduce tx latency).
OTOH there are probably paths where it 'just happens'.
This patch fixes the issue by allowing the mvneta_txq_pend_desc_add function to process more than 255 Tx descriptors.
Fixes: 2a90f7e1d5d0 ("net: mvneta: add xmit_more support") Cc: stable@vger.kernel.org # 4.11+ Signed-off-by: Simon Guinot simon.guinot@sequanux.org
drivers/net/ethernet/marvell/mvneta.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c index 64a04975bcf8..027c08ce4e5d 100644 --- a/drivers/net/ethernet/marvell/mvneta.c +++ b/drivers/net/ethernet/marvell/mvneta.c @@ -816,11 +816,14 @@ static void mvneta_txq_pend_desc_add(struct mvneta_port *pp, { u32 val;
- /* Only 255 descriptors can be added at once ; Assume caller
* process TX desriptors in quanta less than 256
*/
- val = pend_desc + txq->pending;
- mvreg_write(pp, MVNETA_TXQ_UPDATE_REG(txq->id), val);
- pend_desc += txq->pending;
- /* Only 255 Tx descriptors can be added at once */
- while (pend_desc > 0) {
val = min(pend_desc, 255);
mvreg_write(pp, MVNETA_TXQ_UPDATE_REG(txq->id), val);
pend_desc -= val;
- }
do { ... } while () ??
David
From: Simon Guinot simon.guinot@sequanux.org Date: Wed, 8 Nov 2017 17:58:35 +0100
@@ -2413,8 +2416,7 @@ static int mvneta_tx(struct sk_buff *skb, struct net_device *dev) if (txq->count >= txq->tx_stop_threshold) netif_tx_stop_queue(nq);
if (!skb->xmit_more || netif_xmit_stopped(nq) ||
txq->pending + frags > MVNETA_TXQ_DEC_SENT_MASK)
else txq->pending += frags;if (!skb->xmit_more || netif_xmit_stopped(nq)) mvneta_txq_pend_desc_add(pp, txq, frags);
As David Laight said, you should not allow unlimited amounts of ->xmit_more frames to be queued without a TX doorbell update.
Therefore, please keep some kind of limit here otherwise latency will spike in some circumstances.
The mvneta controller provides a 8-bit register to update the pending Tx descriptor counter. Then, a maximum of 255 Tx descriptors can be added at once. In the current code the mvneta_txq_pend_desc_add function assumes the caller takes care of this limit. But it is not the case. In some situations (xmit_more flag), more than 255 descriptors are added. When this happens, the Tx descriptor counter register is updated with a wrong value, which breaks the whole Tx queue management.
This patch fixes the issue by allowing the mvneta_txq_pend_desc_add function to process more than 255 Tx descriptors.
Fixes: 2a90f7e1d5d0 ("net: mvneta: add xmit_more support") Cc: stable@vger.kernel.org # 4.11+ Signed-off-by: Simon Guinot simon.guinot@sequanux.org --- drivers/net/ethernet/marvell/mvneta.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
Changes for v2:
- Use do {} while instead of while {}. - Keep "txq->pending + frags > MVNETA_TXQ_DEC_SENT_MASK" as a flushing condition for the Tx queue.
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c index 64a04975bcf8..bc93b69cfd1e 100644 --- a/drivers/net/ethernet/marvell/mvneta.c +++ b/drivers/net/ethernet/marvell/mvneta.c @@ -816,11 +816,14 @@ static void mvneta_txq_pend_desc_add(struct mvneta_port *pp, { u32 val;
- /* Only 255 descriptors can be added at once ; Assume caller - * process TX desriptors in quanta less than 256 - */ - val = pend_desc + txq->pending; - mvreg_write(pp, MVNETA_TXQ_UPDATE_REG(txq->id), val); + pend_desc += txq->pending; + + /* Only 255 Tx descriptors can be added at once */ + do { + val = min(pend_desc, 255); + mvreg_write(pp, MVNETA_TXQ_UPDATE_REG(txq->id), val); + pend_desc -= val; + } while (pend_desc > 0); txq->pending = 0; }
From: Simon Guinot simon.guinot@sequanux.org Date: Mon, 13 Nov 2017 16:27:02 +0100
The mvneta controller provides a 8-bit register to update the pending Tx descriptor counter. Then, a maximum of 255 Tx descriptors can be added at once. In the current code the mvneta_txq_pend_desc_add function assumes the caller takes care of this limit. But it is not the case. In some situations (xmit_more flag), more than 255 descriptors are added. When this happens, the Tx descriptor counter register is updated with a wrong value, which breaks the whole Tx queue management.
This patch fixes the issue by allowing the mvneta_txq_pend_desc_add function to process more than 255 Tx descriptors.
Fixes: 2a90f7e1d5d0 ("net: mvneta: add xmit_more support") Cc: stable@vger.kernel.org # 4.11+ Signed-off-by: Simon Guinot simon.guinot@sequanux.org
Applied.
linux-stable-mirror@lists.linaro.org