NETIF_F_HIGHDMA misuse in networking drivers?

Was just part of a discussion in another thread talking about 64-bit DMA support issues where NETIF_F_HIGHDMA came up. I was originally under the impression that this flag indicated the device supported 64-bit DMA addressing. However, from looking at the code that checks for it (and, well, the actual comment for the flag) it really means "can access highmem" which has nothing to do with 64-bit at all. And if there's no highmem (like on x86_64) it has no effect at all.

From looking at some drivers, however, it seems a lot of others had similar confusion, as they are doing things like setting NETIF_F_HIGHDMA conditionally on whether setting 64-bit DMA mask succeeds:

drivers/net/8139cp.c:

        if (pci_using_dac)
                dev->features |= NETIF_F_HIGHDMA;

drivers/net/jme.c:

        if (jme->dev->features & NETIF_F_HIGHDMA)
                rxdesc->desc1.flags = RXFLAG_64BIT;

drivers/net/forcedeth.c:

                if (dma_64bit) {
                        if (pci_set_dma_mask(pci_dev, DMA_BIT_MASK(39)))
                                dev_printk(KERN_INFO, &pci_dev->dev,
                                        "64-bit DMA failed, using 
32-bit addressing\n");
                        else
                                dev->features |= NETIF_F_HIGHDMA;

(the last one is extra funny because it keeps saying 64-bit when it's really only 39-bit..)

The net result is that a lot of drivers aren't setting the HIGHDMA flag where they should, and thus on architectures where highmem exists, resulting in a bunch of unnecessary copying.

Assuming my interpretation is correct, I could try and fix up some of this mess.. any comments?

From: Robert Hancock <[email protected]>
Date: Sat, 20 Feb 2010 12:29:14 -0600

I think it's trying to do two things, and doing only one of them well :-)

It's trying to keep drivers from receiving highmem pages if they are not able to access them properly. F.e. imagine a driver that copies data out of the packet assuming that page_address() is always valid on the SKB pages.

It's also trying to use this to prevent physical addresses beyond 4GB from reaching the driver, which as you note is not implemented precisely here.

Without knowing something about how the driver "DMAs" packet data we can't really do an accurate test here. Maybe the driver directly DMAs using physical addressing (an ASIC inside of a CPU, for example, drivers/net/niu.c has a case of this).

Maybe the platform uses an IOMMU with 32-bit virtual addressing, so any physical address is fine even if the card only supports 32-bit addressing (basically any PCI coard on sparc64 is an example of this).

And finally maybe the card supports 64-bit DMA addressing so anything works on any platform.

How to test all of these possible cases precisely that at this spot in the transmit path is the question at hand. To be honest I consider the current hack "good enough" :-)

Yeah, it appears this is the only reason not to set the HIGHDMA flag. Of course, given that relatively few drivers should need to do this (I would think only emulated/virtual adapters, or really old/crappy hardware with no bus-master support) ideally it would be better if only those drivers needed to do something special (i.e. a NOHIGHDMA flag or something) rather than making all the normal ones do it..

If by "precisely" you mean "at all, on many architectures".. on a non-highmem arch, setting or not setting HIGHDMA has no effect on anything, including whether >4GB addresses will be received by the driver or not. Only the device DMA mask (presumably) does. Therefore, conditionally setting HIGHDMA based on what DMA mask the driver has been able to set is almost certainly a bug.

The DMA mask facility already should address all of these cases AFAICS, so there should be no need for any extra handling here..

Well, as I see it there are two big problems with the current state:

-As there are a lot of bad examples in the tree of this flag being set based on irrelevant criteria, such mistakes will propagate to new drivers in cargo-cult fashion. (Looking at some of the existing examples, it seems like has already occurred for a lot of them.)

-Failing to set the HIGHDMA flag when it should be is likely to impact performance on highmem architectures by causing unnecessary packet copying, and potentially using up more precious low memory.