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]
Date:	Sun, 23 Aug 2009 17:30:37 +0200
From:	Krzysztof Halasa <khc@...waw.pl>
To:	David Miller <davem@...emloft.net>,
	Walt Holman <walt@...mansrus.com>
Cc:	netdev@...r.kernel.org, jeffrey.t.kirsher@...el.com,
	jesse.brandeburg@...el.com, bruce.w.allan@...el.com,
	peter.p.waskiewicz.jr@...el.com, john.ronciak@...el.com
Subject: Re: E100 RX ring buffers continued...

David Miller <davem@...emloft.net> writes:

> I think going down the road of trying to use the flexible mode is a
> dead end.  I doubt any other OS driver is using it, and that means
> that there are likely many other errata hiding in the bushes which you
> will unearth by trying to use this new descriptor mode.  And it won't
> show up when you test it, it will show up when some random person in
> some remote data center somewhere updates their kernel, and they won't
> send us a bug report, they'll replace their card or downgrade their
> kernel instead.

Well, I'm afraid it's a possible scenario. I won't touch the flexible
mode unless Intel folks tell me it's safe.
OTOH it seems it was used by less common software, at least in the
82557-9 times. Not sure about Windows driver.
(It seems the simplified mode was meant for Linux-alike "1 buffer per
packet" approach while the flexible mode was to support systems using
mbuf-like structures).

> Just make the driver use consistent buffers for RX, and when packets
> arrive an SKB is allocated and the packet data is copied into the SKB
>>>From the consistent buffer.

That would be a performance hit, wouldn't it? Especially in older
machines, where e100 was typically installed. I think it should be the
last resort.

> And for 2.6.31-rcX we probably have to simply revert your change.

Unfortunately it seems that reverting will not fix operation completely
on a system with swiotlb (checking the descriptor status isn't the only
racy operation which depends on the cache behaviour, though it's the
most frequent). And it will break all non-coherent archs again, they
will either need to use the patch or still stick to (already removed)
eepro100.c

I looked at the eepro100.c sources. It uses BIDIR mapping the same way
as e100.c does, but then syncs using FROM_DEVICE/TO_DEVICE instead of
e100's always-BIDIR. I think the same in e100.c would work on all
platforms (though still violating the DMA API a bit). Perhaps we should
do that instead?

Walt, can you check if 2.6.30.5 with the following patch applied still
breaks e100 with the 6 GB of RAM, please? TIA.
(This patch makes swiotlb aware that the CPU didn't alter the buffer,
though I don't know if swiotlb will use this info).

diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index 014dfb6..53e8252 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -1764,7 +1764,7 @@ static int e100_rx_indicate(struct nic *nic, struct rx *rx,
 				nic->ru_running = RU_SUSPENDED;
 		pci_dma_sync_single_for_device(nic->pdev, rx->dma_addr,
 					       sizeof(struct rfd),
-					       PCI_DMA_BIDIRECTIONAL);
+					       PCI_DMA_FROMDEVICE);
 		return -ENODATA;
 	}
 
-- 
Krzysztof Halasa
--
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

Powered by blists - more mailing lists