diff mbox

net: fec: only clear a queue's work bit if the queue was emptied

Message ID 1462286333-23402-1-git-send-email-u.kleine-koenig@pengutronix.de
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Uwe Kleine-König May 3, 2016, 2:38 p.m. UTC
In the receive path a queue's work bit was cleared unconditionally even
if fec_enet_rx_queue only read out a part of the available packets from
the hardware. This resulted in not reading any packets in the next napi
turn and so packets were delayed or lost.

The obvious fix is to only clear a queue's bit when the queue was
emptied.

Fixes: 4d494cdc92b3 ("net: fec: change data structure to support multiqueue")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

I created this patch against net/master. If you think it's to late to
get it into 4.6 and it doesn't fit on net-next/master, just tell me and
I will rebase.

Best regards
Uwe


 drivers/net/ethernet/freescale/fec_main.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Lucas Stach May 3, 2016, 2:42 p.m. UTC | #1
Am Dienstag, den 03.05.2016, 16:38 +0200 schrieb Uwe Kleine-König:
> In the receive path a queue's work bit was cleared unconditionally even
> if fec_enet_rx_queue only read out a part of the available packets from
> the hardware. This resulted in not reading any packets in the next napi
> turn and so packets were delayed or lost.
> 
> The obvious fix is to only clear a queue's bit when the queue was
> emptied.
> 
> Fixes: 4d494cdc92b3 ("net: fec: change data structure to support multiqueue")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

> ---
> Hello,
> 
> I created this patch against net/master. If you think it's to late to
> get it into 4.6 and it doesn't fit on net-next/master, just tell me and
> I will rebase.
> 
> Best regards
> Uwe
> 
> 
>  drivers/net/ethernet/freescale/fec_main.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 08243c2ff4b4..2a03857cca18 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1521,9 +1521,15 @@ fec_enet_rx(struct net_device *ndev, int budget)
>  	struct fec_enet_private *fep = netdev_priv(ndev);
>  
>  	for_each_set_bit(queue_id, &fep->work_rx, FEC_ENET_MAX_RX_QS) {
> -		clear_bit(queue_id, &fep->work_rx);
> -		pkt_received += fec_enet_rx_queue(ndev,
> +		int ret;
> +
> +		ret = fec_enet_rx_queue(ndev,
>  					budget - pkt_received, queue_id);
> +
> +		if (ret < budget - pkt_received)
> +			clear_bit(queue_id, &fep->work_rx);
> +
> +		pkt_received += ret;
>  	}
>  	return pkt_received;
>  }
Andy Duan May 4, 2016, 2:12 a.m. UTC | #2
From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Sent: Tuesday, May 03, 2016 10:39 PM

> To: Fugang Duan <fugang.duan@nxp.com>; David S . Miller

> <davem@davemloft.net>

> Cc: kernel@pengutronix.de; netdev@vger.kernel.org

> Subject: [PATCH] net: fec: only clear a queue's work bit if the queue was

> emptied

> 

> In the receive path a queue's work bit was cleared unconditionally even if

> fec_enet_rx_queue only read out a part of the available packets from the

> hardware. This resulted in not reading any packets in the next napi turn and so

> packets were delayed or lost.

> 

> The obvious fix is to only clear a queue's bit when the queue was emptied.

> 

> Fixes: 4d494cdc92b3 ("net: fec: change data structure to support multiqueue")

> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

> ---

> Hello,

> 

> I created this patch against net/master. If you think it's to late to get it into 4.6

> and it doesn't fit on net-next/master, just tell me and I will rebase.

> 

> Best regards

> Uwe

> 

> 

>  drivers/net/ethernet/freescale/fec_main.c | 10 ++++++++--

>  1 file changed, 8 insertions(+), 2 deletions(-)

> 

> diff --git a/drivers/net/ethernet/freescale/fec_main.c

> b/drivers/net/ethernet/freescale/fec_main.c

> index 08243c2ff4b4..2a03857cca18 100644

> --- a/drivers/net/ethernet/freescale/fec_main.c

> +++ b/drivers/net/ethernet/freescale/fec_main.c

> @@ -1521,9 +1521,15 @@ fec_enet_rx(struct net_device *ndev, int budget)

>  	struct fec_enet_private *fep = netdev_priv(ndev);

> 

>  	for_each_set_bit(queue_id, &fep->work_rx, FEC_ENET_MAX_RX_QS) {

> -		clear_bit(queue_id, &fep->work_rx);

> -		pkt_received += fec_enet_rx_queue(ndev,

> +		int ret;

> +

> +		ret = fec_enet_rx_queue(ndev,

>  					budget - pkt_received, queue_id);

> +

> +		if (ret < budget - pkt_received)

> +			clear_bit(queue_id, &fep->work_rx);

> +

> +		pkt_received += ret;

>  	}

>  	return pkt_received;

>  }

> --

> 2.8.0.rc3


Tested-by: Fugang Duan <fugang.duan@nxp.com>

Acked-by: Fugang Duan <fugang.duan@nxp.com>
David Miller May 4, 2016, 6:09 p.m. UTC | #3
From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date: Tue,  3 May 2016 16:38:53 +0200

> In the receive path a queue's work bit was cleared unconditionally even
> if fec_enet_rx_queue only read out a part of the available packets from
> the hardware. This resulted in not reading any packets in the next napi
> turn and so packets were delayed or lost.
> 
> The obvious fix is to only clear a queue's bit when the queue was
> emptied.
> 
> Fixes: 4d494cdc92b3 ("net: fec: change data structure to support multiqueue")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Applied and queued up for -stable, thanks.
diff mbox

Patch

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 08243c2ff4b4..2a03857cca18 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1521,9 +1521,15 @@  fec_enet_rx(struct net_device *ndev, int budget)
 	struct fec_enet_private *fep = netdev_priv(ndev);
 
 	for_each_set_bit(queue_id, &fep->work_rx, FEC_ENET_MAX_RX_QS) {
-		clear_bit(queue_id, &fep->work_rx);
-		pkt_received += fec_enet_rx_queue(ndev,
+		int ret;
+
+		ret = fec_enet_rx_queue(ndev,
 					budget - pkt_received, queue_id);
+
+		if (ret < budget - pkt_received)
+			clear_bit(queue_id, &fep->work_rx);
+
+		pkt_received += ret;
 	}
 	return pkt_received;
 }