[PATCH 1/1] altera tse: Fix network-delays and -retransmissions after high throughput.

[PATCH 1/1] altera tse: Fix network-delays and -retransmissions after high throughput.

[Andreas Oetken]

From: Andreas Oetken <andreas@oetken.name>

Fix bug which occurs when more than <limit> packets are available during napi-poll,
leading to "delays" and retransmissions on the network.

Check for (count < limit) before checking the get_rx_status in tse_rx-function. get_rx_status is reading from the response-fifo.
If there is currently a response in the fifo, reading the last byte of the response pops the value from the fifo.
If the limit is check as second condition and the limit is reached the fifo is popped but the packet is not processed.

Signed-off-by: Andreas Oetken <ennoerlangen@gmail.com>
---
 drivers/net/ethernet/altera/altera_tse_main.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index 6725dc0..b927b8b 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -376,8 +376,12 @@ static int tse_rx(struct altera_tse_private *priv, int limit)
 	u16 pktlength;
 	u16 pktstatus;
 
-	while (((rxstatus = priv->dmaops->get_rx_status(priv)) != 0) &&
-	       (count < limit))  {
+	/* Check for limit first. get_rx_status is reading the from the
+	* response-fifo. If there is a currently a response in the fifo,
+	* reading the last byte of the response pops the value from the fifo.
+	*/
+	while ((count < limit) &&
+		((rxstatus = priv->dmaops->get_rx_status(priv)) != 0)) {
 		pktstatus = rxstatus >> 16;
 		pktlength = rxstatus & 0xffff;
 
-- 
2.1.4

Re: [PATCH 1/1] altera tse: Fix network-delays and -retransmissions after high throughput.

[David Miller]

From: Andreas Oetken <ennoerlangen@googlemail.com>
Date: Tue, 14 Apr 2015 22:25:26 +0200

> From: Andreas Oetken <andreas@oetken.name>
> 
> Fix bug which occurs when more than <limit> packets are available during napi-poll,
> leading to "delays" and retransmissions on the network.
> 
> Check for (count < limit) before checking the get_rx_status in tse_rx-function. get_rx_status is reading from the response-fifo.
> If there is currently a response in the fifo, reading the last byte of the response pops the value from the fifo.
> If the limit is check as second condition and the limit is reached the fifo is popped but the packet is not processed.
> 
> Signed-off-by: Andreas Oetken <ennoerlangen@gmail.com>

Your commit message lines are very long, format them to 80 columns
or less.

> -	while (((rxstatus = priv->dmaops->get_rx_status(priv)) != 0) &&
> -	       (count < limit))  {
> +	/* Check for limit first. get_rx_status is reading the from the
> +	* response-fifo. If there is a currently a response in the fifo,
> +	* reading the last byte of the response pops the value from the fifo.
> +	*/
> +	while ((count < limit) &&
> +		((rxstatus = priv->dmaops->get_rx_status(priv)) != 0)) {

You're corrupted the indentation.

The second line of this while statement should start precisely at
the first column after the openning parenthesis of the first line.
You must use the appropriate number of TAB then SPACE characters
necessary to achieve this.

If you are purely using TAB characters, as you have done here, for
indentation then it is very likely that you have indented things
incorrectly.

[PATCH 1/1] altera tse: Fix network-delays and -retransmissions after high throughput.

[Andreas Oetken]

From: Andreas Oetken <andreas@oetken.name>

Fix bug which occurs when more than <limit> packets are available during
napi-poll, leading to "delays" and retransmissions on the network.

Check for (count < limit) before checking the get_rx_status in tse_rx-function.
Function get_rx_status is reading from the response-fifo.
If there is currently a response in the fifo,
reading the last byte of the response pops the value from the fifo.
If the limit is checked as second condition
and the limit is reached the fifo is popped but the packet is not processed.

Signed-off-by: Andreas Oetken <ennoerlangen@gmail.com>
---
 drivers/net/ethernet/altera/altera_tse_main.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index 6725dc0..dbbbd34 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -376,8 +376,13 @@ static int tse_rx(struct altera_tse_private *priv, int limit)
 	u16 pktlength;
 	u16 pktstatus;
 
-	while (((rxstatus = priv->dmaops->get_rx_status(priv)) != 0) &&
-	       (count < limit))  {
+	/* Check for count < limit first as get_rx_status is changing
+	* the response-fifo so we must process the next packet
+	* after calling get_rx_status if a response is pending.
+	* (reading the last byte of the response pops the value from the fifo.)
+	*/
+	while ((count < limit) &&
+	       ((rxstatus = priv->dmaops->get_rx_status(priv)) != 0)) {
 		pktstatus = rxstatus >> 16;
 		pktlength = rxstatus & 0xffff;
 
-- 
2.1.4

Re: [PATCH 1/1] altera tse: Fix network-delays and -retransmissions after high throughput.

[David Miller]

From: Andreas Oetken <ennoerlangen@googlemail.com>
Date: Thu, 16 Apr 2015 23:48:08 +0200

> From: Andreas Oetken <andreas@oetken.name>
> 
> Fix bug which occurs when more than <limit> packets are available during
> napi-poll, leading to "delays" and retransmissions on the network.
> 
> Check for (count < limit) before checking the get_rx_status in tse_rx-function.
> Function get_rx_status is reading from the response-fifo.
> If there is currently a response in the fifo,
> reading the last byte of the response pops the value from the fifo.
> If the limit is checked as second condition
> and the limit is reached the fifo is popped but the packet is not processed.
> 
> Signed-off-by: Andreas Oetken <ennoerlangen@gmail.com>

Applied, thanks.