infiniband: use %p6 for printing message ids

PATCH Applied Harvey Harrison 5 participants 11 messages Oct 28, 2008 – Oct 29, 2008

Subsystems (4)

Networking Network drivers InfiniBand SCSI RDMA protocol (SRP) initiator

lore

Share & tools

lore

Harvey Harrison

author

2008-10-28 21:40 UTC #0 lore raw

PATCH 1/1 infiniband: use %p6 for printing message ids

Signed-off-by Harvey Harrison <[email protected]>

---
 drivers/infiniband/core/sysfs.c         |   10 +---------
 drivers/infiniband/hw/mthca/mthca_mcg.c |   23 ++---------------------
 drivers/infiniband/ulp/srp/ib_srp.c     |   31 ++++---------------------------
 drivers/net/mlx4/mcg.c                  |   23 ++---------------------
 4 files changed, 9 insertions(+), 78 deletions(-)

drivers/infiniband/core/sysfs.c +1 -9

index 4d10421..e985193 100644
@@ -262,15 +262,7 @@ static ssize_t show_port_gid(struct ib_port *p, struct port_attribute *attr,
 	if (ret)
 		return ret;
 
-	return sprintf(buf, "%04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x\n",
-		       be16_to_cpu(((__be16 *) gid.raw)[0]),
-		       be16_to_cpu(((__be16 *) gid.raw)[1]),
-		       be16_to_cpu(((__be16 *) gid.raw)[2]),
-		       be16_to_cpu(((__be16 *) gid.raw)[3]),
-		       be16_to_cpu(((__be16 *) gid.raw)[4]),
-		       be16_to_cpu(((__be16 *) gid.raw)[5]),
-		       be16_to_cpu(((__be16 *) gid.raw)[6]),
-		       be16_to_cpu(((__be16 *) gid.raw)[7]));
+	return sprintf(buf, "%p6\n", gid.raw);
 }
 
 static ssize_t show_port_pkey(struct ib_port *p, struct port_attribute *attr,

drivers/infiniband/hw/mthca/mthca_mcg.c +2 ~~-21~~

index 3f5f948..693bed0 100644
@@ -87,17 +87,7 @@ static int find_mgm(struct mthca_dev *dev,
 	}
 
 	if (0)
-		mthca_dbg(dev, "Hash for %04x:%04x:%04x:%04x:"
-			  "%04x:%04x:%04x:%04x is %04x\n",
-			  be16_to_cpu(((__be16 *) gid)[0]),
-			  be16_to_cpu(((__be16 *) gid)[1]),
-			  be16_to_cpu(((__be16 *) gid)[2]),
-			  be16_to_cpu(((__be16 *) gid)[3]),
-			  be16_to_cpu(((__be16 *) gid)[4]),
-			  be16_to_cpu(((__be16 *) gid)[5]),
-			  be16_to_cpu(((__be16 *) gid)[6]),
-			  be16_to_cpu(((__be16 *) gid)[7]),
-			  *hash);
+		mthca_dbg(dev, "Hash for %p6 is %04x\n", gid, *hash);
 
 	*index = *hash;
 	*prev  = -1;

@@ -264,16 +254,7 @@ int mthca_multicast_detach(struct ib_qp *ibqp, union ib_gid *gid, u16 lid)
 		goto out;
 
 	if (index == -1) {
-		mthca_err(dev, "MGID %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x "
-			  "not found\n",
-			  be16_to_cpu(((__be16 *) gid->raw)[0]),
-			  be16_to_cpu(((__be16 *) gid->raw)[1]),
-			  be16_to_cpu(((__be16 *) gid->raw)[2]),
-			  be16_to_cpu(((__be16 *) gid->raw)[3]),
-			  be16_to_cpu(((__be16 *) gid->raw)[4]),
-			  be16_to_cpu(((__be16 *) gid->raw)[5]),
-			  be16_to_cpu(((__be16 *) gid->raw)[6]),
-			  be16_to_cpu(((__be16 *) gid->raw)[7]));
+		mthca_err(dev, "MGID %p6 not found\n", gid->raw);
 		err = -EINVAL;
 		goto out;
 	}

drivers/infiniband/ulp/srp/ib_srp.c +4 ~~-27~~

index 5b8b533..bc82531 100644
@@ -1514,15 +1514,7 @@ static ssize_t show_dgid(struct device *dev, struct device_attribute *attr,
 	    target->state == SRP_TARGET_REMOVED)
 		return -ENODEV;
 
-	return sprintf(buf, "%04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x\n",
-		       be16_to_cpu(((__be16 *) target->path.dgid.raw)[0]),
-		       be16_to_cpu(((__be16 *) target->path.dgid.raw)[1]),
-		       be16_to_cpu(((__be16 *) target->path.dgid.raw)[2]),
-		       be16_to_cpu(((__be16 *) target->path.dgid.raw)[3]),
-		       be16_to_cpu(((__be16 *) target->path.dgid.raw)[4]),
-		       be16_to_cpu(((__be16 *) target->path.dgid.raw)[5]),
-		       be16_to_cpu(((__be16 *) target->path.dgid.raw)[6]),
-		       be16_to_cpu(((__be16 *) target->path.dgid.raw)[7]));
+	return sprintf(buf, "%p6\n", target->path.dgid.raw);
 }
 
 static ssize_t show_orig_dgid(struct device *dev,

@@ -1534,15 +1526,7 @@ static ssize_t show_orig_dgid(struct device *dev,
 	    target->state == SRP_TARGET_REMOVED)
 		return -ENODEV;
 
-	return sprintf(buf, "%04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x\n",
-		       be16_to_cpu(target->orig_dgid[0]),
-		       be16_to_cpu(target->orig_dgid[1]),
-		       be16_to_cpu(target->orig_dgid[2]),
-		       be16_to_cpu(target->orig_dgid[3]),
-		       be16_to_cpu(target->orig_dgid[4]),
-		       be16_to_cpu(target->orig_dgid[5]),
-		       be16_to_cpu(target->orig_dgid[6]),
-		       be16_to_cpu(target->orig_dgid[7]));
+	return sprintf(buf, "%p6\n", target->orig_dgid);
 }
 
 static ssize_t show_zero_req_lim(struct device *dev,

@@ -1883,19 +1867,12 @@ static ssize_t srp_create_target(struct device *dev,
 
 	shost_printk(KERN_DEBUG, target->scsi_host, PFX
 		     "new target: id_ext %016llx ioc_guid %016llx pkey %04x "
-		     "service_id %016llx dgid %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x\n",
+		     "service_id %016llx dgid %p6\n",
 	       (unsigned long long) be64_to_cpu(target->id_ext),
 	       (unsigned long long) be64_to_cpu(target->ioc_guid),
 	       be16_to_cpu(target->path.pkey),
 	       (unsigned long long) be64_to_cpu(target->service_id),
-	       (int) be16_to_cpu(*(__be16 *) &target->path.dgid.raw[0]),
-	       (int) be16_to_cpu(*(__be16 *) &target->path.dgid.raw[2]),
-	       (int) be16_to_cpu(*(__be16 *) &target->path.dgid.raw[4]),
-	       (int) be16_to_cpu(*(__be16 *) &target->path.dgid.raw[6]),
-	       (int) be16_to_cpu(*(__be16 *) &target->path.dgid.raw[8]),
-	       (int) be16_to_cpu(*(__be16 *) &target->path.dgid.raw[10]),
-	       (int) be16_to_cpu(*(__be16 *) &target->path.dgid.raw[12]),
-	       (int) be16_to_cpu(*(__be16 *) &target->path.dgid.raw[14]));
+	       target->path.dgid.raw);
 
 	ret = srp_create_target_ib(target);
 	if (ret)

drivers/net/mlx4/mcg.c +2 ~~-21~~

index 592c01a..6f79e84 100644
@@ -118,17 +118,7 @@ static int find_mgm(struct mlx4_dev *dev,
 		return err;
 
 	if (0)
-		mlx4_dbg(dev, "Hash for %04x:%04x:%04x:%04x:"
-			  "%04x:%04x:%04x:%04x is %04x\n",
-			  be16_to_cpu(((__be16 *) gid)[0]),
-			  be16_to_cpu(((__be16 *) gid)[1]),
-			  be16_to_cpu(((__be16 *) gid)[2]),
-			  be16_to_cpu(((__be16 *) gid)[3]),
-			  be16_to_cpu(((__be16 *) gid)[4]),
-			  be16_to_cpu(((__be16 *) gid)[5]),
-			  be16_to_cpu(((__be16 *) gid)[6]),
-			  be16_to_cpu(((__be16 *) gid)[7]),
-			  *hash);
+		mlx4_dbg(dev, "Hash for %p6 is %04x\n", gid, *hash);
 
 	*index = *hash;
 	*prev  = -1;

@@ -277,16 +267,7 @@ int mlx4_multicast_detach(struct mlx4_dev *dev, struct mlx4_qp *qp, u8 gid[16])
 		goto out;
 
 	if (index == -1) {
-		mlx4_err(dev, "MGID %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x "
-			  "not found\n",
-			  be16_to_cpu(((__be16 *) gid)[0]),
-			  be16_to_cpu(((__be16 *) gid)[1]),
-			  be16_to_cpu(((__be16 *) gid)[2]),
-			  be16_to_cpu(((__be16 *) gid)[3]),
-			  be16_to_cpu(((__be16 *) gid)[4]),
-			  be16_to_cpu(((__be16 *) gid)[5]),
-			  be16_to_cpu(((__be16 *) gid)[6]),
-			  be16_to_cpu(((__be16 *) gid)[7]));
+		mlx4_err(dev, "MGID %p6 not found\n", gid);
 		err = -EINVAL;
 		goto out;
 	}

Signature · 1 line

1.6.0.3.729.g6ea410

Looks fine (once the p6 support goes in of course). You could also look at IPOIB_GID_FMT in drivers/infiniband/ulp/ipoib for further things worth converting. And I didn't notice if you already did it, but drivers/net/mlx4 has lots of similar stuff to convert the same way as you did for mthca.

Thanks,
Roland

Joe Perches

2008-10-28 23:03 UTC #2 lore raw

In reply to Harvey Harrison

Quoted 8 lines · Harvey Harrison

drivers/infiniband/core/sysfs.c +0 -0

index 4d10421..e985193 100644
@@ -262,15 +262,7 @@ static ssize_t show_port_gid(struct ib_port *p, struct port_attribute *attr,
 	if (ret)
 		return ret;

drivers/infiniband/core/sysfs.c static ssize_t show_port_gid(struct ib_port *p, struct port_attribute *attr, · line 265

Quoted 10 lines

drivers/infiniband/core/sysfs.c +1 -9

-	return sprintf(buf, "%04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x\n",
-		       be16_to_cpu(((__be16 *) gid.raw)[0]),
-		       be16_to_cpu(((__be16 *) gid.raw)[1]),
-		       be16_to_cpu(((__be16 *) gid.raw)[2]),
-		       be16_to_cpu(((__be16 *) gid.raw)[3]),
-		       be16_to_cpu(((__be16 *) gid.raw)[4]),
-		       be16_to_cpu(((__be16 *) gid.raw)[5]),
-		       be16_to_cpu(((__be16 *) gid.raw)[6]),
-		       be16_to_cpu(((__be16 *) gid.raw)[7]));
+	return sprintf(buf, "%p6\n", gid.raw);

I'm not sure any of these should be converted like this. Infiniband gids are not IPv6 addresses.

I think a new %p<foo> type (maybe: IB) type should be defined for these.

Roland Dreier

2008-10-28 23:07 UTC #3 lore raw

Quoted 2 lines

I'm not sure any of these should be converted like this. Infiniband gids are not IPv6 addresses.

Actually, they are. The IB specification section 4.1 says:

"A GID is a valid 128-bit IPv6 address (per RFC 2373) with additional
properties / restrictions defined...."

and it makes sense to format an IB GID as an IPv6 address for display (that's what the IB spec does; eg it mentions the "Multicast GID FF02:0:0:0:0:0:0:1").

- R.

Joe Perches

2008-10-28 23:22 UTC #4 lore raw

Quoted 11 lines · Roland Dreier

Quoted 2 lines
I'm not sure any of these should be converted like this. Infiniband gids are not IPv6 addresses.
Actually, they are. The IB specification section 4.1 says:
"A GID is a valid 128-bit IPv6 address (per RFC 2373) with additional
properties / restrictions defined...."
and it makes sense to format an IB GID as an IPv6 address for display (that's what the IB spec does; eg it mentions the "Multicast GID FF02:0:0:0:0:0:0:1").

Perhaps the union in ib_verbs

union ib_gid {
	u8	raw[16];
	struct {
		__be64	subnet_prefix;
		__be64	interface_id;
	} global;
};

could/should include a struct in6_addr?

Joe Perches

2008-10-29 00:16 UTC #5 lore raw

[ofa-general] ***SPAM*** [PATCH] infiniband: Add struct in6_addr addr to union ib_gib

ib_gid's can be print'd using the new %p6 facility

Signed-off-by Joe Perches <[email protected]>

include/rdma/ib_verbs.h +2 -0

index 936e333..464ed9d 100644
@@ -47,6 +47,7 @@
 #include <linux/list.h>
 #include <linux/rwsem.h>
 #include <linux/scatterlist.h>
+#include <linux/in6.h>
 
 #include <asm/atomic.h>
 #include <asm/uaccess.h>

@@ -57,6 +58,7 @@ union ib_gid {
 		__be64	subnet_prefix;
 		__be64	interface_id;
 	} global;
+	struct in6_addr	addr;
 };
 
 enum rdma_node_type {

David Miller

maintainer

2008-10-29 05:39 UTC #6 lore raw

[ofa-general] Re: [PATCH] infiniband: Add struct in6_addr addr to union ib_gib

From: Joe Perches <[email protected]>
Date: Tue, 28 Oct 2008 17:16:14 -0700

Quoted 3 lines

ib_gid's can be print'd using the new %p6 facility
Signed-off-by Joe Perches <[email protected]>

Joe, please provide something relative to Harvey's patches so that this new union member gets passed into the %p6 uses.

Thanks!

Joe Perches

2008-10-29 06:58 UTC #7 lore raw

[ofa-general] Re: [PATCH] infiniband: Add struct in6_addr addr to union ib_gib

Quoted 7 lines · David Miller

From: Joe Perches <[email protected]>
Date: Tue, 28 Oct 2008 17:16:14 -0700
Quoted 2 lines
ib_gid's can be print'd using the new %p6 facility
Signed-off-by Joe Perches <[email protected]>
Joe, please provide something relative to Harvey's patches so that this new union member gets passed into the %p6 uses.

Sure. After Harvey's patches show up in
http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=summary
I'll make a patchset including this union ib_gid addition as well as changes to drivers/infiniband/

Roland, in a separate question, are the infiniband maintainers also the maintainers of include/rdma/? If "F: patterns" ever gets accepted into MAINTAINERS, should include/rdma/ be listed under infiniband?

You and Sean Hefty seem to be the primary authors.

$ git log include/rdma/ | grep Author: | \
  sort | uniq -c | sort -nr | head -5
     32 Author: Sean Hefty <[email protected]>
     23 Author: Roland Dreier <[email protected]>
      6 Author: Michael S. Tsirkin <[email protected]>
      5 Author: Or Gerlitz <[email protected]>
      4 Author: Jack Morgenstein <[email protected]>

David Miller

maintainer

2008-10-29 08:39 UTC #8 lore raw

[ofa-general] Re: [PATCH] infiniband: Add struct in6_addr addr to union ib_gib

From: Joe Perches <[email protected]>
Date: Tue, 28 Oct 2008 23:58:40 -0700

Quoted 11 lines · David Miller

Quoted 7 lines
From: Joe Perches <[email protected]>
Date: Tue, 28 Oct 2008 17:16:14 -0700
Quoted 2 lines
ib_gid's can be print'd using the new %p6 facility
Signed-off-by Joe Perches <[email protected]>
Joe, please provide something relative to Harvey's patches so that this new union member gets passed into the %p6 uses.
Sure. After Harvey's patches show up in
http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=summary

They should be there now.

Roland Dreier

2008-10-29 18:15 UTC #9 lore raw

Re: [ofa-general] Re: [PATCH] infiniband: Add struct in6_addr addr to union ib_gib

In reply to Joe Perches

Quoted 4 lines

Sure. After Harvey's patches show up in
http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=summary
I'll make a patchset including this union ib_gid addition as well as changes to drivers/infiniband/

OK I guess. I also don't have any objection to using a different format letter but the
same underlying code as the p6 implementation as an alternative for handling the type checking.

Quoted 4 lines

Roland, in a separate question, are the infiniband maintainers also the maintainers of include/rdma/? If "F: patterns" ever gets accepted into MAINTAINERS, should include/rdma/ be listed under infiniband?

Yes, drivers/infiniband and include/rdma are all part of the InfiniBand/RDMA stack.

David Miller

maintainer Applied

2008-10-29 05:37 UTC #10 lore raw

In reply to Harvey Harrison

From: Harvey Harrison <[email protected]>
Date: Tue, 28 Oct 2008 14:38:21 -0700

Quoted 1 line

Signed-off-by Harvey Harrison <[email protected]>

Applied, thanks!