netdev - Re: E100 RX ring buffers continued...

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

Walt Holman <walt@...mansrus.com> writes:

> I've tested this under 2.6.31-rc7 (which was also broken to begin
> with) and this appears to have fixed it.  It's a fairly limited test
> at this point, as I've just been downloading a large file for the past
> 15 mins. or so, but this was normally enough to have multiple
> stoppages.  Do you still need 2.6.30.5 tested as well?

No, these tests and the drivers in both versions are equivalent. Thanks
a lot.

David, I think it's safe to apply this. Probably to "stable" series,
too.

I think at least this should theoretically be changed as well:

e100_rx_indicate()
        ...
        /* Need to sync before taking a peek at cb_complete bit */
        pci_dma_sync_single_for_cpu(nic->pdev, rx->dma_addr,
                sizeof(struct rfd), PCI_DMA_BIDIRECTIONAL);
        rfd_status = le16_to_cpu(rfd->status);

but since it appears to work I wouldn't touch it ATM. I guess the
swiotlb code is smart enough to not write to device-owner memory on
for_cpu() call.

...

E100: fix interaction with swiotlb on X86.

E100 places it's RX packet descriptors inside skb->data and uses them
with bidirectional streaming DMA mapping. Data in descriptors is
accessed simultaneously by the chip (writing status and size when
a packet is received) and CPU (reading to check if the packet was
received). This isn't a valid usage of PCI DMA API, which requires use
of the coherent (consistent) memory for such purpose. Unfortunately e100
chips working in "simplified" RX mode have to store received data
directly after the descriptor. Fixing the driver to conform to the API
would require using unsupported "flexible" RX mode or receiving data
into a coherent memory and using CPU to copy it to network buffers.

This patch, while not yet making the driver conform to the PCI DMA API,
allows it to work correctly on X86 with swiotlb (while not breaking
other architectures).

Signed-off-by: Krzysztof Hałasa <khc@...waw.pl>

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