diff mbox

[net-next] net/mlx4_en: Advertise HIGH DMA support only if dma mask is 64 bit

Message ID 1376296230-12276-1-git-send-email-amirv@mellanox.com
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Amir Vadai Aug. 12, 2013, 8:30 a.m. UTC
Need to check that the pci device DMA mask was set to 64 bit support
(done by mlx4_core) before advertising HIGH DMA support in higher drivers.
Otherwise, may run into an issue that skb/frag/data is in high memory
and can't dma map it, thus packets are dropped.


Signed-off-by: Alexander Guller <alexg@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Ben Hutchings Aug. 12, 2013, 4:32 p.m. UTC | #1
On Mon, 2013-08-12 at 11:30 +0300, Amir Vadai wrote:
> Need to check that the pci device DMA mask was set to 64 bit support
> (done by mlx4_core) before advertising HIGH DMA support in higher drivers.
> Otherwise, may run into an issue that skb/frag/data is in high memory
> and can't dma map it, thus packets are dropped.

NETIF_F_HIGHDMA means that the driver can transmit skbs with page
fragments in high memory, i.e. it doesn't require them to have an
existing virtual mapping for the kernel.  If I'm not mistaken, this
should be cleared only for drivers that don't use DMA or kmap functions.

On a 64-bit machine there is no highmem and without NETIF_F_HIGHDMA you
can still be given pages at any physical address.

On a 32-bit machine highmem usually starts at 1GB and I doubt you have
any chips that are that limited in DMA range. :-)

I really doubt this is the fix you're looking for.

Ben.

> Signed-off-by: Alexander Guller <alexg@mellanox.com>
> Signed-off-by: Amir Vadai <amirv@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index fa37b7a..9568b77 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -2317,9 +2317,13 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
>  	dev->vlan_features = dev->hw_features;
>  
>  	dev->hw_features |= NETIF_F_RXCSUM | NETIF_F_RXHASH;
> -	dev->features = dev->hw_features | NETIF_F_HIGHDMA |
> +	dev->features = dev->hw_features |
>  			NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX |
>  			NETIF_F_HW_VLAN_CTAG_FILTER;
> +
> +	if (mdev->dev->pdev->dma_mask == DMA_BIT_MASK(64))
> +		dev->features |= NETIF_F_HIGHDMA;
> +
>  	dev->hw_features |= NETIF_F_LOOPBACK;
>  
>  	if (mdev->dev->caps.steering_mode ==
Amir Vadai Aug. 13, 2013, 11:19 a.m. UTC | #2
On 12/08/2013 19:32, Ben Hutchings wrote:
> On Mon, 2013-08-12 at 11:30 +0300, Amir Vadai wrote:
>> Need to check that the pci device DMA mask was set to 64 bit support
>> (done by mlx4_core) before advertising HIGH DMA support in higher drivers.
>> Otherwise, may run into an issue that skb/frag/data is in high memory
>> and can't dma map it, thus packets are dropped.
> 
> NETIF_F_HIGHDMA means that the driver can transmit skbs with page
> fragments in high memory, i.e. it doesn't require them to have an
> existing virtual mapping for the kernel.  If I'm not mistaken, this
> should be cleared only for drivers that don't use DMA or kmap functions.
> 
> On a 64-bit machine there is no highmem and without NETIF_F_HIGHDMA you
> can still be given pages at any physical address.
> 
> On a 32-bit machine highmem usually starts at 1GB and I doubt you have
> any chips that are that limited in DMA range. :-)
> 
> I really doubt this is the fix you're looking for.
> 
You are right. We were mixing highmem with addresses greater than 2^32.
Thanks for reviewing :)

Dave, please abandon this patch.

Amir.

> Ben.
> 
>> Signed-off-by: Alexander Guller <alexg@mellanox.com>
>> Signed-off-by: Amir Vadai <amirv@mellanox.com>
>> ---
>>  drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> index fa37b7a..9568b77 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> @@ -2317,9 +2317,13 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
>>  	dev->vlan_features = dev->hw_features;
>>  
>>  	dev->hw_features |= NETIF_F_RXCSUM | NETIF_F_RXHASH;
>> -	dev->features = dev->hw_features | NETIF_F_HIGHDMA |
>> +	dev->features = dev->hw_features |
>>  			NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX |
>>  			NETIF_F_HW_VLAN_CTAG_FILTER;
>> +
>> +	if (mdev->dev->pdev->dma_mask == DMA_BIT_MASK(64))
>> +		dev->features |= NETIF_F_HIGHDMA;
>> +
>>  	dev->hw_features |= NETIF_F_LOOPBACK;
>>  
>>  	if (mdev->dev->caps.steering_mode ==
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index fa37b7a..9568b77 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2317,9 +2317,13 @@  int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 	dev->vlan_features = dev->hw_features;
 
 	dev->hw_features |= NETIF_F_RXCSUM | NETIF_F_RXHASH;
-	dev->features = dev->hw_features | NETIF_F_HIGHDMA |
+	dev->features = dev->hw_features |
 			NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX |
 			NETIF_F_HW_VLAN_CTAG_FILTER;
+
+	if (mdev->dev->pdev->dma_mask == DMA_BIT_MASK(64))
+		dev->features |= NETIF_F_HIGHDMA;
+
 	dev->hw_features |= NETIF_F_LOOPBACK;
 
 	if (mdev->dev->caps.steering_mode ==