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
| ||
|
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