2009-12-30 18:28:42

by Hartley Sweeten

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

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 <[email protected]>
Cc: David S. Miller <[email protected]>

---

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


2010-01-04 18:49:55

by Maciej W. Rozycki

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

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 <[email protected]>
> Cc: David S. Miller <[email protected]>

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

2010-01-04 19:03:29

by David Miller

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

From: "Maciej W. Rozycki" <[email protected]>
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.

2010-01-04 23:43:26

by Maciej W. Rozycki

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

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

2010-01-05 01:05:50

by Joe Perches

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

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 <[email protected]>

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