[PATCH] drivers/net/defxx.c: use %pM to show MAC address

[PATCH] drivers/net/defxx.c: use %pM to show MAC address

[H Hartley Sweeten]

Use the %pM kernel extension to display the MAC address.

The only difference in the output is that the MAC address is
shown in the usual colon-separated hex notation.

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: David S. Miller <davem@davemloft.net>

---

diff --git a/drivers/net/defxx.c b/drivers/net/defxx.c
index 6a6ea03..d27b716 100644
--- a/drivers/net/defxx.c
+++ b/drivers/net/defxx.c
@@ -1052,12 +1052,9 @@ static int __devinit dfx_driver_init(struct net_device *dev,
 		board_name = "DEFEA";
 	if (dfx_bus_pci)
 		board_name = "DEFPA";
-	pr_info("%s: %s at %saddr = 0x%llx, IRQ = %d, "
-		"Hardware addr = %02X-%02X-%02X-%02X-%02X-%02X\n",
+	pr_info("%s: %s at %saddr = 0x%llx, IRQ = %d, Hardware addr = %pM\n",
 		print_name, board_name, dfx_use_mmio ? "" : "I/O ",
-		(long long)bar_start, dev->irq,
-		dev->dev_addr[0], dev->dev_addr[1], dev->dev_addr[2],
-		dev->dev_addr[3], dev->dev_addr[4], dev->dev_addr[5]);
+		(long long)bar_start, dev->irq, dev->dev_addr);
 
 	/*
 	 * Get memory for descriptor block, consumer block, and other buffers 

Re: [PATCH] drivers/net/defxx.c: use %pM to show MAC address

[Maciej W. Rozycki]

On Wed, 30 Dec 2009, H Hartley Sweeten wrote:

> Use the %pM kernel extension to display the MAC address.
> 
> The only difference in the output is that the MAC address is
> shown in the usual colon-separated hex notation.
> 
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: David S. Miller <davem@davemloft.net>

 Nack.  While I appreciate your effort, the FDDI standard defines opposite 
bit-ordering for MAC addresses written in the dash-separated and the 
colon-separated hexadecimal notation, which makes your change invalid, 
sorry (you'd have to bit-swap them too and that would make addresses 
counter-intuitive).  Please note that `ifconfig' consistently uses the 
dash-separated notation for FDDI interfaces too (I haven't checked other 
tools like those in the "iproute" package; they may be buggy).

 [I'd appreciate to be CCed on patch submissions to code I claim 
maintenance of too.]

  Maciej

Re: [PATCH] drivers/net/defxx.c: use %pM to show MAC address

[David Miller]

From: "Maciej W. Rozycki" <macro@linux-mips.org>
Date: Mon, 4 Jan 2010 18:49:48 +0000 (GMT)

>  Nack.  While I appreciate your effort, the FDDI standard defines opposite 
> bit-ordering for MAC addresses written in the dash-separated and the 
> colon-separated hexadecimal notation, which makes your change invalid, 
> sorry (you'd have to bit-swap them too and that would make addresses 
> counter-intuitive).  Please note that `ifconfig' consistently uses the 
> dash-separated notation for FDDI interfaces too (I haven't checked other 
> tools like those in the "iproute" package; they may be buggy).

Well:

1) It's a bootup kernel message, we can use whatever format
   we want.

2) As for consistency's sake, the only other FDDI driver using
   alloc_fddidev() and friends, skfp, uses yet another format
   for printing FDDI addresses to the log:

	pr_debug(KERN_INFO "HW-Addr: %02x %02x %02x %02x %02x %02x\n",

   So striving for "consistency" is a bit of a stretch.

   Against a userspace tool's output format?  Even more so...

And I doubt anybody is going to suddenly faint and become
confused if they see the MAC address in ethernet format.

In fact I'd rather see all of our FDDI drivers use the standard
ethernet MAC format in visible kernel log messages and save all
of that object code space that gets taken up by these by-hand
printk's in these obscure drivers.

Re: [PATCH] drivers/net/defxx.c: use %pM to show MAC address

[Maciej W. Rozycki]

On Mon, 4 Jan 2010, David Miller wrote:

> >  Nack.  While I appreciate your effort, the FDDI standard defines opposite 
> > bit-ordering for MAC addresses written in the dash-separated and the 
> > colon-separated hexadecimal notation, which makes your change invalid, 
> > sorry (you'd have to bit-swap them too and that would make addresses 
> > counter-intuitive).  Please note that `ifconfig' consistently uses the 
> > dash-separated notation for FDDI interfaces too (I haven't checked other 
> > tools like those in the "iproute" package; they may be buggy).
> 
> Well:
> 
> 1) It's a bootup kernel message, we can use whatever format
>    we want.
> 
> 2) As for consistency's sake, the only other FDDI driver using
>    alloc_fddidev() and friends, skfp, uses yet another format
>    for printing FDDI addresses to the log:
> 
> 	pr_debug(KERN_INFO "HW-Addr: %02x %02x %02x %02x %02x %02x\n",
> 
>    So striving for "consistency" is a bit of a stretch.
> 
>    Against a userspace tool's output format?  Even more so...
> 
> And I doubt anybody is going to suddenly faint and become
> confused if they see the MAC address in ethernet format.
> 
> In fact I'd rather see all of our FDDI drivers use the standard
> ethernet MAC format in visible kernel log messages and save all
> of that object code space that gets taken up by these by-hand
> printk's in these obscure drivers.

 I agree with you and I like things to be sweet and consistent as long as 
they don't cause confusion.

 Unfortunately FDDI defines both formats and one obvious for people that 
have only ever worked with Ethernet (i.e. in the colon-separated notation) 
is the opposite to how people familiar with FDDI would interpret it.  The 
other driver should use the same notation as this one does and I guess the 
author just couldn't decide or something.

 Here's a quotation from the relevant FDDI standard document -- that's 
"FDDI STATION MANAGEMENT (SMT)", my draft copy is marked as X3T9/92-067, 
and it looks it's been later designated as ANSI X3.229-1994 and ISO 9314-6 
too.  This is from chapter 4.1.2.3 "Notation for Addresses":

 "The ANSI/IEEE 802.1 standard has defined the canonical form for 
representing 48-bit addresses as octet strings.  The canonical form is a 
6-octet string.  The first octet contains the first 8 bits of the address, 
with the I/G bit as the least significant bit, the U/L bit as the next 
more significant bit, and so on.  When displayed for use by humans, the 
canonical form is shown as a hexadecimal string, written from left to 
right, with hyphens separating the individual octets.

 This SMT document uses the Most Significant Bit (MSB) form for 
representing 48 bit addresses as 6-octet strings.  The first octet 
contains the first 8 bits of the address, with the I/G bit as the most 
significant bit, the U/L bit as the next less significant bit, and so on.  
When displayed for use by humans, the MSB form is shown as a hexadecimal 
string, written from left to right, with colons separating the individual 
octets.

 The example below shows an address, and the sequence of bits or symbols 
that would be transmitted when the address is used in the Source Address 
or Destination Address fields on the MAC header.  The transmission line 
shows the address bits in the order transmitted, from left to right.  For 
IEEE 802 LANs these correspond to actual bits on the medium.  The FDDI 
symbols line shows how the FDDI PHY sends the address bits as encoded 
symbols.

        MSB:            35:7B:12:00:00:01
        Canonical:      AC-DE-48-00-00-80
        Transmission:   00110101 01111011 00010010 00000000 00000000 00000001
        FDDI Symbols:   35 7B 12 00 00 01"

Please note that this address has its group bit clear.

 This notation is also defined in the "FDDI MEDIA ACCESS CONTROL-2 
(MAC-2)" (X3T9/92-120) document although that book does not have a need 
to use the MSB form and it's skipped.

 As you can see the documents make it pretty clear how FDDI addresses are 
to be shown to humans and everyone at least a bit familiar with FDDI will 
expect them to look so and be confused otherwise -- whether it's a Linux 
kernel bootup message or anything else (and yes, I did have to fix things 
coming from this confusion at least once before, so please let's not go 
through this again).

 And yes, Ethernet and FDDI can be bridged between at the MAC layer (I 
have such a device), so it can be shown that it's the canonical form that 
corresponds to that used for Ethernet.

  Maciej

[RFC PATCH] lib/vsprintf.c: Add %pMF to for FDDI bit reversed dashed output

[Joe Perches]

On Mon, 2010-01-04 at 23:43 +0000, Maciej W. Rozycki wrote:
> The example below shows an address, and the sequence of bits or symbols 
> that would be transmitted when the address is used in the Source Address 
> or Destination Address fields on the MAC header.  The transmission line 
> shows the address bits in the order transmitted, from left to right.  For 
> IEEE 802 LANs these correspond to actual bits on the medium.  The FDDI 
> symbols line shows how the FDDI PHY sends the address bits as encoded 
> symbols.
> 
>         MSB:            35:7B:12:00:00:01
>         Canonical:      AC-DE-48-00-00-80
>         Transmission:   00110101 01111011 00010010 00000000 00000000 00000001
>         FDDI Symbols:   35 7B 12 00 00 01"
> 
> Please note that this address has its group bit clear.
> 
>  This notation is also defined in the "FDDI MEDIA ACCESS CONTROL-2 
> (MAC-2)" (X3T9/92-120) document although that book does not have a need 
> to use the MSB form and it's skipped.

Here's a possible patch to vsprintf to add %pMF to print
FDDI bit-reversed dash separated addresses.

vsprintf.o increases by 90 bytes.
Perhaps it's not worth that.

Signed-off-by: Joe Perches <joe@perches.com>

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index d4996cf..96f1987 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -25,6 +25,7 @@
 #include <linux/kallsyms.h>
 #include <linux/uaccess.h>
 #include <linux/ioport.h>
+#include <linux/bitrev.h>
 #include <net/addrconf.h>
 
 #include <asm/page.h>		/* for PAGE_SIZE */
@@ -682,10 +683,18 @@ static char *mac_address_string(char *buf, char *end, u8 *addr,
 	char *p = mac_addr;
 	int i;
 
-	for (i = 0; i < 6; i++) {
-		p = pack_hex_byte(p, addr[i]);
-		if (fmt[0] == 'M' && i != 5)
-			*p++ = ':';
+	if (fmt[1] == 'F') {		/* FDDI canonical format */
+		for (i = 0; i < 6; i++) {
+			p = pack_hex_byte(p, bitrev8(addr[i]));
+			if (fmt[0] == 'M' && i != 5)
+				*p++ = '-';
+		}
+	} else {
+		for (i = 0; i < 6; i++) {
+			p = pack_hex_byte(p, addr[i]);
+			if (fmt[0] == 'M' && i != 5)
+				*p++ = ':';
+		}
 	}
 	*p = '\0';
 
@@ -896,6 +905,10 @@ static char *uuid_string(char *buf, char *end, const u8 *addr,
  * - 'M' For a 6-byte MAC address, it prints the address in the
  *       usual colon-separated hex notation
  * - 'm' For a 6-byte MAC address, it prints the hex address without colons
+ * - 'MF' For a 6-byte MAC FDDI address, it prints the address
+ *       with a dash-separated hex notation with bit reversed bytes
+ * - 'mF' For a 6-byte MAC FDDI address, it prints the address
+ *       in hex notation without separators with bit reversed bytes
  * - 'I' [46] for IPv4/IPv6 addresses printed in the usual way
  *       IPv4 uses dot-separated decimal without leading 0's (1.2.3.4)
  *       IPv6 uses colon separated network-order 16 bit hex with leading 0's
@@ -939,6 +952,7 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 		return resource_string(buf, end, ptr, spec, fmt);
 	case 'M':			/* Colon separated: 00:01:02:03:04:05 */
 	case 'm':			/* Contiguous: 000102030405 */
+					/* [mM]F (FDDI, bit reversed) */
 		return mac_address_string(buf, end, ptr, spec, fmt);
 	case 'I':			/* Formatted IP supported
 					 * 4:	1.2.3.4