mlx4: properly mask MGM entry members count

The members_count field size is 24 bits so mask it properly when reading it.

Signed-off-by Eli Cohen <eli-VPRAkNaXOzVS1MOuV/[email protected]>

---
 drivers/net/mlx4/mcg.c |    2 +-

 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/net/mlx4/mcg.c +1 -2

index c4f88b7..fff20fc 100644
@@ -187,7 +187,7 @@ int mlx4_multicast_attach(struct mlx4_dev *dev, struct mlx4_qp *qp, u8 gid[16],
                memcpy(mgm->gid, gid, 16);
        }
 
-       members_count = be32_to_cpu(mgm->members_count);
+       members_count = be32_to_cpu(mgm->members_count) & 0xffffff;
        if (members_count == MLX4_QP_PER_MGM) {
                mlx4_err(dev, "MGM at index %x is full.\n", index);
                err = -ENOMEM;
--

1.6.0.2

On Mon, Nov 15, 2010 at 4:08 PM, Eli Cohen <[email protected]> wrote:

Don't you need to do the same when detaching?

The members_count field size is 24 bits so mask it properly when reading it.

Quoted 1 line

The members_count field size is 24 bits so mask it properly when reading it.

what's the impact of this? Does this actually fix something seen in practice or is it just found by inspection?

We need this patch as the MS byte is used now to encode the protocol of the multicast address. I need to resend since the patch is missing handling in the dettach function.

Where is the change in handling in the members_count field? In the firmware?

Quoted 2 lines

Where is the change in handling in the members_count field? In the firmware?

The firmware now distinguishes between differnet multicast addresses according to a protocol field located in the high MS byte which previously was reserved. The patches that Alekseys will will probably send later make use of this.

Quoted 6 lines

Quoted 2 lines

Where is the change in handling in the members_count field? In the firmware?

The firmware now distinguishes between differnet multicast addresses according to a protocol field located in the high MS byte which previously was reserved. The patches that Alekseys will will probably send later make use of this.

OK, so what you seem to be saying is that this patch is not fixing anything until we add Alekseys patches?

Quoted 7 lines · Roland Dreier

 > > Where is the change in handling in the members_count field?  In the firmware? > >
 > The firmware now distinguishes between differnet multicast addresses > according to a protocol field located in the high MS byte which
 > previously was reserved. The patches that Alekseys will will probably > send later make use of this.

Quoted 3 lines

OK, so what you seem to be saying is that this patch is not fixing anything until we add Alekseys patches?

It only keeps the code and the PRM in sync.
--

RAW_ETH patches cannot be applied without it, so we really need this masking.

Quoted 1 line · Aleksey Senin

RAW_ETH patches cannot be applied without it, so we really need this masking.

So Maybe it would be easier to review and merge if you add this patch to the raw qp patch set you mentioned

Or.

Quoted 6 lines · Or Gerlitz

Quoted 1 line · Aleksey Senin

RAW_ETH patches cannot be applied without it, so we really need this masking.

So Maybe it would be easier to review and merge if you add this patch to the raw qp patch set you mentioned

Quoted 1 line

Or.

I don't think that RAW_ETH patches should contain fixes for Mellanox driver implementation and make it working according to Mellanox PRM. In any case, Eli sent patch that easy to review and merge. In addition, even if I include this change to my series, there is a chance that from the whole set, will be accepted only this one, and the rest will be accepted later ;-)

I afraid that you overloaded with dozen of e-mails and missed this question, so I'd ask you again. Is there any reason why this members_count patch cannot be applied?

 > I afraid that you  overloaded with dozen of e-mails and missed this > question, so I'd ask you again. Is there any reason why this > members_count patch cannot be applied?

No, it is fine.  I will merge it for 2.6.38.  However this long thread should (I hope) show how the changelog could be improved -- if the original patch had said why the patch was needed then we could have avoided all this long discussion.

 - R.