Some NIPQUAD_FMT -> %pI4 conversions are broken

I just noticed (by getting a messed up print in my kernel log) that most of the changes to drivers/infiniband/hw/nes in commit 63779436 ("drivers: replace NIPQUAD()") were wrong: the idea was to change printk statements like

	printk("..." NIPQUAD_FMT "...", NIPQUAD(foo));

but most of the changes in nes were to code like

	printk("..." NIPQUAD_FMT "...", HIPQUAD(foo));

ie *H* IPQUAD not *N* IPQUAD, indicating the foo is in host endian order, and hence the new code

	printk("...%pI4...", &foo);

prints the IP in reverse order now.

I don't see a good way to fix this without introducing a temporary variable to hold a swapped IP address... but is there a better way?

- R.

My apologies indeed, the best choice I think is to add a temp var for now and I'll work with you to eliminate the byteswapping further up the callchain so a be32 gets passed down, but that might be too involved for 2.6.29.

Sound OK?

Harvey

From: Harvey Harrison <[email protected]>
Date: Fri, 09 Jan 2009 22:07:25 -0800

Alternatively we could provide a %p* variant that took cpu endian addresses.

I don't know how useful that would be, generally.

I don't think we want that, it would be better to just start work on the nes drivers to work directly on be32s throughout, and eject all of the byteswapping throughout the driver.

Having such a %p helper for host-endian just encourages more drivers to be written the same way.

Harvey

commit 63779436ab4ad0867bcea53bf853b0004d7b895di (drivers: replace NIPQUAD()) accidentally replaced two HIPQUAD()s causing IP addresses to be printed in reverse order, add two temporary local vars.

---
I'll start working on a patch working directly with _be32s, but this is the safest for now.

 drivers/infiniband/hw/nes/nes_cm.c    |    3 ++-
 drivers/infiniband/hw/nes/nes_utils.c |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

index 6ba57e9..2395b20 100644
@@ -778,12 +778,13 @@ static struct nes_cm_node *find_node(struct nes_cm_core *cm_core,
 	unsigned long flags;
 	struct list_head *hte;
 	struct nes_cm_node *cm_node;
+	__be32 tmp_addr = cpu_to_be32(loc_addr);
 
 	/* get a handle on the hte */
 	hte = &cm_core->connected_nodes;
 
 	nes_debug(NES_DBG_CM, "Searching for an owner node: %pI4:%x from core %p->%p\n",
-		  &loc_addr, loc_port, cm_core, hte);
+		  &tmp_addr, loc_port, cm_core, hte);
 
 	/* walk list and find cm_node associated with this session ID */
 	spin_lock_irqsave(&cm_core->ht_lock, flags);

index aa9b734..feb7401 100644
@@ -655,6 +655,7 @@ int nes_arp_table(struct nes_device *nesdev, u32 ip_addr, u8 *mac_addr, u32 acti
 	struct nes_adapter *nesadapter = nesdev->nesadapter;
 	int arp_index;
 	int err = 0;
+	__be32 tmp_addr = cpu_to_be32(ip_addr);
 
 	for (arp_index = 0; (u32) arp_index < nesadapter->arp_table_size; arp_index++) {
 		if (nesadapter->arp_table[arp_index].ip_addr == ip_addr)

@@ -683,7 +684,7 @@ int nes_arp_table(struct nes_device *nesdev, u32 ip_addr, u8 *mac_addr, u32 acti
 	/* DELETE or RESOLVE */
 	if (arp_index == nesadapter->arp_table_size) {
 		nes_debug(NES_DBG_NETDEV, "MAC for %pI4 not in ARP table - cannot %s\n",
-			  &ip_addr, action == NES_ARP_RESOLVE ? "resolve" : "delete");
+			  &tmp_addr, action == NES_ARP_RESOLVE ? "resolve" : "delete");
 		return -1;
 	}
 

1.6.1.94.g9388

Sorry missed a git add after catching the other hunks, will get this right in the morning.

Harvey