netdev - Re: net_rx_action/NAPI oops [PATCH]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 30 Nov 2007 09:13:00 -0800
From:	"Kok, Auke" <auke-jan.h.kok@...el.com>
To:	Robert Olsson <Robert.Olsson@...a.slu.se>
CC:	Stephen Hemminger <shemminger@...ux-foundation.org>,
	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: net_rx_action/NAPI oops [PATCH]

Robert Olsson wrote:
>  Hello!
> 
>  After further investigations. The bug was just in front of us...
> 
>  ifconfig down in combination with the test for || !netif_running() can 
>  return a full quota and netif_rx_complete() done which causes the oops 
>  in net_rx_action. Of course the load must be high enough to fill the 
>  quota.
> 
>  I've found a bunch of drivers having this bug.

OK, I think however that we want to get rid of the netif_running() tests. I think
they're an artifact of an old bug that was fixed a long time ago and we no longer
need to check for that at all. Your patch (minus the formatting mistakes) looks
otherwise OK and I'll have someone do some quick testing on it.

Thanks,

Auke
> 
>  Cheers.
> 					--ro
> 
>  
> Signed-off-by: Robert Olsson <robert.olsson@....uu.se>
> 
> 
> diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> index cf39473..f4137ad 100644
> --- a/drivers/net/e1000/e1000_main.c
> +++ b/drivers/net/e1000/e1000_main.c
> @@ -3947,6 +3947,10 @@ e1000_clean(struct napi_struct *napi, int budget)
>  quit_polling:
>  		if (likely(adapter->itr_setting & 3))
>  			e1000_set_itr(adapter);
> +
> +		if(work_done == budget)
> +			work_done--;
> +	
>  		netif_rx_complete(poll_dev, napi);
>  		e1000_irq_enable(adapter);
>  	}
> diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
> index 4fd2e23..e43b5ca 100644
> --- a/drivers/net/e1000e/netdev.c
> +++ b/drivers/net/e1000e/netdev.c
> @@ -1408,6 +1408,9 @@ static int e1000_clean(struct napi_struct *napi, int budget)
>  	if ((!tx_cleaned && (work_done < budget)) ||
>  	   !netif_running(poll_dev)) {
>  quit_polling:
> +		if(work_done == budget)
> +			work_done--;
> +
>  		if (adapter->itr_setting & 3)
>  			e1000_set_itr(adapter);
>  		netif_rx_complete(poll_dev, napi);
> diff --git a/drivers/net/ixgb/ixgb_main.c b/drivers/net/ixgb/ixgb_main.c
> index 3021234..e3064ef 100644
> --- a/drivers/net/ixgb/ixgb_main.c
> +++ b/drivers/net/ixgb/ixgb_main.c
> @@ -1783,6 +1783,10 @@ ixgb_clean(struct napi_struct *napi, int budget)
>  
>  	/* if no Tx and not enough Rx work done, exit the polling mode */
>  	if((!tx_cleaned && (work_done == 0)) || !netif_running(netdev)) {
> +
> +		if(work_done == budget)
> +			work_done--;
> +
>  		netif_rx_complete(netdev, napi);
>  		ixgb_irq_enable(adapter);
>  	}
> diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
> index 00bc525..204f5fa 100644
> --- a/drivers/net/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ixgbe/ixgbe_main.c
> @@ -579,6 +579,9 @@ static int ixgbe_clean_rxonly(struct napi_struct *napi, int budget)
>  	/* If no Tx and not enough Rx work done, exit the polling mode */
>  	if ((work_done < budget) || !netif_running(netdev)) {
>  quit_polling:
> +		if(work_done == budget)
> +			work_done--;
> +
>  		netif_rx_complete(netdev, napi);
>  		if (!test_bit(__IXGBE_DOWN, &adapter->state))
>  			IXGBE_WRITE_REG(&adapter->hw, IXGBE_EIMS,
> @@ -1483,6 +1486,9 @@ static int ixgbe_clean(struct napi_struct *napi, int budget)
>  	if ((!tx_cleaned && (work_done < budget)) ||
>  	    !netif_running(adapter->netdev)) {
>  quit_polling:
> +		if(work_done == budget)
> +			work_done--;
> +
>  		netif_rx_complete(netdev, napi);
>  		ixgbe_irq_enable(adapter);
>  	}
> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
> index 3dbaec6..c566491 100644
> --- a/drivers/net/e100.c
> +++ b/drivers/net/e100.c
> @@ -1998,6 +1998,10 @@ static int e100_poll(struct napi_struct *napi, int budget)
>  
>  	/* If no Rx and Tx cleanup work was done, exit polling mode. */
>  	if((!tx_cleaned && (work_done == 0)) || !netif_running(netdev)) {
> +
> +		if(work_done == budget)
> +			work_done--;
> +
>  		netif_rx_complete(netdev, napi);
>  		e100_enable_irq(nic);
>  	}
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html