diff mbox series

[net,2/2] net: lantiq_xrx200: fix return value in ENOMEM case

Message ID 20220717194824.1017750-2-olek2@wp.pl (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net,1/2] net: lantiq_xrx200: confirm skb is allocated before using | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 30 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Aleksander Jan Bajkowski July 17, 2022, 7:48 p.m. UTC
The xrx200_hw_receive() function can return:
* XRX200_DMA_PACKET_IN_PROGRESS (the next descriptor contains the
further part of the packet),
* XRX200_DMA_PACKET_COMPLETE (a complete packet has been received),
* -ENOMEM (a memory allocation error occurred).

Currently, the third of these cases is incorrectly handled. The NAPI
poll function returns then a negative value (-ENOMEM). Correctly in
such a situation, the driver should try to receive next packet in
the hope that this time the memory allocation for the next descriptor
will succeed.

This patch replaces the XRX200_DMA_PACKET_IN_PROGRESS definition with
-EINPROGRESS to simplify the driver.

Fixes: c3e6b2c35b34 ("net: lantiq_xrx200: add ingress SG DMA support")
Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
---
 drivers/net/ethernet/lantiq_xrx200.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Comments

Paolo Abeni July 19, 2022, 11:31 a.m. UTC | #1
On Sun, 2022-07-17 at 21:48 +0200, Aleksander Jan Bajkowski wrote:
> The xrx200_hw_receive() function can return:
> * XRX200_DMA_PACKET_IN_PROGRESS (the next descriptor contains the
> further part of the packet),
> * XRX200_DMA_PACKET_COMPLETE (a complete packet has been received),
> * -ENOMEM (a memory allocation error occurred).
> 
> Currently, the third of these cases is incorrectly handled. The NAPI
> poll function returns then a negative value (-ENOMEM). Correctly in
> such a situation, the driver should try to receive next packet in
> the hope that this time the memory allocation for the next descriptor
> will succeed.

> This patch replaces the XRX200_DMA_PACKET_IN_PROGRESS definition with
> -EINPROGRESS to simplify the driver.
> 
> Fixes: c3e6b2c35b34 ("net: lantiq_xrx200: add ingress SG DMA support")
> Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
> ---
>  drivers/net/ethernet/lantiq_xrx200.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
> index 6a83a6c19484..2865d07f3fc8 100644
> --- a/drivers/net/ethernet/lantiq_xrx200.c
> +++ b/drivers/net/ethernet/lantiq_xrx200.c
> @@ -27,9 +27,6 @@
>  #define XRX200_DMA_TX		1
>  #define XRX200_DMA_BURST_LEN	8
>  
> -#define XRX200_DMA_PACKET_COMPLETE	0
> -#define XRX200_DMA_PACKET_IN_PROGRESS	1
> -
>  /* cpu port mac */
>  #define PMAC_RX_IPG		0x0024
>  #define PMAC_RX_IPG_MASK	0xf
> @@ -272,9 +269,8 @@ static int xrx200_hw_receive(struct xrx200_chan *ch)
>  		netif_receive_skb(ch->skb_head);
>  		ch->skb_head = NULL;
>  		ch->skb_tail = NULL;
> -		ret = XRX200_DMA_PACKET_COMPLETE;
>  	} else {
> -		ret = XRX200_DMA_PACKET_IN_PROGRESS;
> +		ret = -EINPROGRESS;
>  	}
>  
>  	return ret;
> @@ -292,10 +288,8 @@ static int xrx200_poll_rx(struct napi_struct *napi, int budget)
>  
>  		if ((desc->ctl & (LTQ_DMA_OWN | LTQ_DMA_C)) == LTQ_DMA_C) {
>  			ret = xrx200_hw_receive(ch);
> -			if (ret == XRX200_DMA_PACKET_IN_PROGRESS)
> +			if (ret)
>  				continue;
> -			if (ret != XRX200_DMA_PACKET_COMPLETE)
> -				return ret;

Note that in case of persistent pressure and with the device under
flood, 'rx' is not incremented, and this loop can run for an unbound
number of iteration.

If you keep running, you do need to increment 'rx' at every iteration,
even in case of error.

Note that 'rx' is more an estimate of the work done than a 'received
packet counter'.

Paolo
diff mbox series

Patch

diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
index 6a83a6c19484..2865d07f3fc8 100644
--- a/drivers/net/ethernet/lantiq_xrx200.c
+++ b/drivers/net/ethernet/lantiq_xrx200.c
@@ -27,9 +27,6 @@ 
 #define XRX200_DMA_TX		1
 #define XRX200_DMA_BURST_LEN	8
 
-#define XRX200_DMA_PACKET_COMPLETE	0
-#define XRX200_DMA_PACKET_IN_PROGRESS	1
-
 /* cpu port mac */
 #define PMAC_RX_IPG		0x0024
 #define PMAC_RX_IPG_MASK	0xf
@@ -272,9 +269,8 @@  static int xrx200_hw_receive(struct xrx200_chan *ch)
 		netif_receive_skb(ch->skb_head);
 		ch->skb_head = NULL;
 		ch->skb_tail = NULL;
-		ret = XRX200_DMA_PACKET_COMPLETE;
 	} else {
-		ret = XRX200_DMA_PACKET_IN_PROGRESS;
+		ret = -EINPROGRESS;
 	}
 
 	return ret;
@@ -292,10 +288,8 @@  static int xrx200_poll_rx(struct napi_struct *napi, int budget)
 
 		if ((desc->ctl & (LTQ_DMA_OWN | LTQ_DMA_C)) == LTQ_DMA_C) {
 			ret = xrx200_hw_receive(ch);
-			if (ret == XRX200_DMA_PACKET_IN_PROGRESS)
+			if (ret)
 				continue;
-			if (ret != XRX200_DMA_PACKET_COMPLETE)
-				return ret;
 			rx++;
 		} else {
 			break;