[ofa-general] [PATCH] ipoib - high dma support

Roland Dreier rdreier at cisco.com
Tue Aug 14 21:05:20 PDT 2007


Overall this series looks like interesting stuff.

However a few general comments:

 - There seems to be a fair bit of whitespace damage and other
   formatting glitches.  Running scripts/checkpatch.pl on all your
   patches would be worthwhile.

 - The changes need to be split up in a better way.  For each feature
   (eg checksum offload), I would have four patches: "add core
   support for X," which just has the core changes to ib_verbs.h etc;
   "use X in IPoIB," which uses the feature in IPoIB; and "implement X
   in mthca" and "implement X in mlx4," which add low-level driver
   support for the feature.

Also more specific comments below:

 > +	pdev = to_pci_dev(hca->dma_device);

this doesn't make sense... not all IB devices are PCI devices (eg ehca
is not a PCI device).

 > +	if (pdev->dma_mask & DMA_64BIT_MASK)
 > +		priv->dev->features |= NETIF_F_HIGHDMA;

I don't think we need this test.  The assumption that IB devices can
do 64-bit DMA is pretty safe; I think we can enable NETIF_F_HIGHDMA
unconditionally.

 - R.



More information about the general mailing list