netdev - Re: [PATCH] netpoll: fix race on poll_list resulting in garbage entry

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20081211181528.GB10558@hmsendeavour.rdu.redhat.com>
Date:	Thu, 11 Dec 2008 13:15:28 -0500
From:	Neil Horman <nhorman@...driver.com>
To:	Stephen Hemminger <shemminger@...tta.com>
Cc:	Jarek Poplawski <jarkao2@...il.com>, netdev@...r.kernel.org,
	davem@...emloft.net
Subject: Re: [PATCH] netpoll: fix race on poll_list resulting in garbage entry

On Thu, Dec 11, 2008 at 09:01:04AM -0800, Stephen Hemminger wrote:
> On Thu, 11 Dec 2008 13:07:28 +0000
> Jarek Poplawski <jarkao2@...il.com> wrote:
> 
> > On 09-12-2008 22:06, Neil Horman wrote:
> > ...
> > > When executing napi->poll from the netpoll_path, this bit will
> > > be set. When a driver calls netif_rx_complete, if that bit is set, it will not
> > > remove the napi_struct from the poll_list.  That work will be saved for the next
> > > iteration of net_rx_action.
> > 
> > This could be not enough: some drivers, e.g. sky2, call napi_complete()
> > directly.
> > 
> 
> There is good reason for this. Although most drivers only have one NAPI
> instance per device, and multiqueue drivers have several NAPI structures
> per device, a few devices like sky2 need to support multiple devices
> running off one NAPI receive. The Marvell hardware has a common receive
> interrupt for both ports on a dual port card.
> 
> This kind of hardware limits usage of netpoll. Only one port can be
> used with netpoll because netpoll makes assumptions about NAPI
> association.
> 

There was previously good cause to use __netif_rx_complete instead of
netif_rx_complete some time ago when multiqueue rx was implemented using a set
of dummy netdevices.  But with the separation of the napi code, there is no
longer any reason for this to be done.

I just took a quick look, and it appears that sky2 is the last remaining driver
to use the underlying napi routines.

This patch maintains exactly the same functionality that it previously had, but
allows for the netpoll patch to be safe with respect to the per-cpu poll_lists
used by net_rx_action.

Regards
Neil


Signed-off-by: Neil Horman <nhorman@...driver.com>


 sky2.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 3813d15..84bdc3c 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -2694,7 +2694,7 @@ static int sky2_poll(struct napi_struct *napi, int work_limit)
 		sky2_write8(hw, STAT_TX_TIMER_CTRL, TIM_STOP);
 		sky2_write8(hw, STAT_TX_TIMER_CTRL, TIM_START);
 	}
-	napi_complete(napi);
+	netif_rx_complete(napi->dev, napi);
 	sky2_read32(hw, B0_Y2_SP_LISR);
 done:
 
-- 
/***************************************************
 *Neil Horman
 *nhorman@...driver.com
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/
--
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