diff mbox

[net] net/mlx4_core: Limit count field to 24 bits in qp_alloc_res

Message ID 1416909271-28840-1-git-send-email-ogerlitz@mellanox.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Or Gerlitz Nov. 25, 2014, 9:54 a.m. UTC
From: Jack Morgenstein <jackm@dev.mellanox.co.il>

Some VF drivers use the upper byte of "param1" (the qp count field)
in mlx4_qp_reserve_range() to pass flags which are used to optimize
the range allocation.

Under the current code, if any of these flags are set, the 32-bit
count field yields a count greater than 2^24, which is out of range,
and this VF fails.

As these flags represent a "best-effort" allocation hint anyway, they may
safely be ignored. Therefore, the PF driver may simply mask out the bits.

Fixes: c82e9aa0a8 "mlx4_core: resource tracking for HCA resources used by guests"
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---

Hi Dave, 

This fix goes way backi down to kernel 3.3... I think we will be good 
if it pushed to -stable of >= 3.10

 .../net/ethernet/mellanox/mlx4/resource_tracker.c  |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

David Miller Nov. 25, 2014, 7:16 p.m. UTC | #1
From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Tue, 25 Nov 2014 11:54:31 +0200

>  	case RES_OP_RESERVE:
> -		count = get_param_l(&in_param);
> +		count = get_param_l(&in_param) & 0xffffff;

I think if these high bits are set, you should be using the maximum
value rather then truncating.
--
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
Or Gerlitz Nov. 26, 2014, 2:20 p.m. UTC | #2
On 11/25/2014 9:16 PM, David Miller wrote:
> From: Or Gerlitz <ogerlitz@mellanox.com>
> Date: Tue, 25 Nov 2014 11:54:31 +0200
>
>>   	case RES_OP_RESERVE:
>> -		count = get_param_l(&in_param);
>> +		count = get_param_l(&in_param) & 0xffffff;
> I think if these high bits are set, you should be using the maximum
> value rather then truncating.

Dave,

To make it clear (maybe the change-log wasn't explaining it well 
enough), the 32 bits in_param are
divided to 24 bits of count and 8 bits for optimized allocation scheme 
which isn't yet supported
by the Linux PF driver.

Currently, the upstream PF driver is wrongly NOT masking out these eight 
bits and as such, some
VF drivers which are already setting them for optimized allocation are 
failing to allocate QPs as
the count seen by the PF becomes way too large.  The optimization is 
best effort anyway, and hence
we can safely ignore their request.

If we follow your suggestion and allocate to these VFs per the maximum 
number, we will
hit their quota after they open one QP and their over-all bring up will 
fail, so this will not help.

Or.
--
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
David Miller Nov. 26, 2014, 5:05 p.m. UTC | #3
From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Wed, 26 Nov 2014 16:20:13 +0200

> Currently, the upstream PF driver is wrongly NOT masking out these
> eight bits and as such, some VF drivers which are already setting
> them for optimized allocation are failing to allocate QPs as the
> count seen by the PF becomes way too large.  The optimization is
> best effort anyway, and hence we can safely ignore their request.

Ok, thanks for explaining.

Applied and queued up for -stable, thanks.
--
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/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
index 5d2498d..cd5cf6d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
+++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
@@ -1546,7 +1546,7 @@  static int qp_alloc_res(struct mlx4_dev *dev, int slave, int op, int cmd,
 
 	switch (op) {
 	case RES_OP_RESERVE:
-		count = get_param_l(&in_param);
+		count = get_param_l(&in_param) & 0xffffff;
 		align = get_param_h(&in_param);
 		err = mlx4_grant_resource(dev, slave, RES_QP, count, 0);
 		if (err)