Skip to contentSkip to conversation

add %pM printf format specifier

PATCH In review Johannes Berg 8 participants 33 messages Oct 24, 2008 – Oct 28, 2008

Subsystems (4)
Networking Network drivers Library code Vsprintf
lore
Share & tools
lore

Johannes Berg

author
#0 lore raw

PATCH 1/1 add %pM printf format specifier

This adds a new printf format specifier for the kernel, %pM, to be used to print out MAC addresses. This has advantages over the current print_mac scheme:
* no need for DECLARE_MAC_BUF
* can be used safely in statements that might be compiled out without the print_mac call staying.

Signed-off-by Johannes Berg <[email protected]>
---
 lib/vsprintf.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
lib/vsprintf.c +20 -0
@@ -581,6 +581,22 @@ static char *resource_string(char *buf, 
 	return string(buf, end, sym, field_width, precision, flags);
 }
 
+static char *mac_address(char *buf, char *end, u8 *addr, int field_width, int precision, int flags)
+{
+	/* room for 6 * two hex digits, 5 colons and trailing zero */
+	char mac[18];
+	char *p = mac, *pend = mac + sizeof(mac);
+	int i;
+
+	for (i=0; i < 6; i++) {
+		p = number(p, pend, addr[i], 16, 2, -1, SMALL | ZEROPAD);
+		*p++ = ':';
+	}
+	mac[17] = '\0';
+
+	return string(buf, end, mac, field_width, precision, flags);
+}
+
 /*
  * Show a '%p' thing.  A kernel extension is that the '%p' is followed
  * by an extra set of alphanumeric characters that are extended format

@@ -592,6 +608,8 @@ static char *resource_string(char *buf, 
  * - 'S' For symbolic direct pointers
  * - 'R' For a struct resource pointer, it prints the range of
  *       addresses (not the name nor the flags)
+ * - 'M' For a 6-byte MAC address, it prints the address in the
+ *       usual colon-separated hex notation
  *
  * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
  * function pointers are really function descriptors, which contain a

@@ -607,6 +625,8 @@ static char *pointer(const char *fmt, ch
 		return symbol_string(buf, end, ptr, field_width, precision, flags);
 	case 'R':
 		return resource_string(buf, end, ptr, field_width, precision, flags);
+	case 'M':
+		return mac_address(buf, end, ptr, field_width, precision, flags);
 	}
 	flags |= SMALL;
 	if (field_width == -1) {
#1 lore raw
Quoted 6 lines · Johannes Berg

This adds a new printf format specifier for the kernel, %pM, to be used to print out MAC addresses. This has advantages over the current print_mac scheme:
* no need for DECLARE_MAC_BUF
* can be used safely in statements that might be compiled out without the print_mac call staying.

Good things.

Quoted 7 lines
lib/vsprintf.c +1 -0
@@ -581,6 +581,22 @@ static char *resource_string(char *buf, 
 	return string(buf, end, sym, field_width, precision, flags);
 }
 
+static char *mac_address(char *buf, char *end, u8 *addr, int field_width, int precision, int flags)

lib/vsprintf.c static char *resource_string(char *buf, · line 594

Quoted 10 lines
lib/vsprintf.c +10 -0
+{
+	/* room for 6 * two hex digits, 5 colons and trailing zero */
+	char mac[18];
+	char *p = mac, *pend = mac + sizeof(mac);
+	int i;
+
+	for (i=0; i < 6; i++) {
+		p = number(p, pend, addr[i], 16, 2, -1, SMALL | ZEROPAD);
+		*p++ = ':';
+	}

For consistency, shouldn't the function name be mac_address_string?

I (and perhaps Harvey H) would be happier with:

	char mac[18];
	char *p = mac;
	int i;

	for (i = 0; i < 5; i++) {
		p = pack_hex_byte(p, addr[i]);
		*p++ = ':';
	}
	p = pack_hex_byte(p, addr[5]);
	*p = '\0';

Smaller, faster, etc...

Also, as the number of %p(foo) types increases, perhaps something like sparse could validate the printf argument types?

cheers, Joe

author
#2 lore raw
Quoted 27 lines · Joe Perches

lib/vsprintf.c static char *resource_string(char *buf, · line 594

Quoted 11 lines
lib/vsprintf.c +11 -0
+static char *mac_address(char *buf, char *end, u8 *addr, int field_width, int precision, int flags)
+{
+	/* room for 6 * two hex digits, 5 colons and trailing zero */
+	char mac[18];
+	char *p = mac, *pend = mac + sizeof(mac);
+	int i;
+
+	for (i=0; i < 6; i++) {
+		p = number(p, pend, addr[i], 16, 2, -1, SMALL | ZEROPAD);
+		*p++ = ':';
+	}

For consistency, shouldn't the function name be mac_address_string?

I (and perhaps Harvey H) would be happier with:

	char mac[18];
	char *p = mac;
	int i;

	for (i = 0; i < 5; i++) {
		p = pack_hex_byte(p, addr[i]);
		*p++ = ':';
	}
	p = pack_hex_byte(p, addr[5]);
	*p = '\0';
Quoted 1 line

Smaller, faster, etc...

Heh. I wasn't aware of pack_hex_byte. Not sure it really is smaller though with the inlining, and kernel messages ought not to be performance sensitive. OTOH, this is _way_ simpler in terms of code, I'll send v2.

Quoted 3 lines

Also, as the number of %p(foo) types increases, perhaps something like sparse could validate the printf argument types?

Hard to do really, what guarantees that a MAC address is 6 bytes, which is the relevant thing? :)

johannes

#3 lore raw
In reply to Johannes Berg
Quoted 6 lines · Johannes Berg

This adds a new printf format specifier for the kernel, %pM, to be used to print out MAC addresses. This has advantages over the current print_mac scheme:
* no need for DECLARE_MAC_BUF
* can be used safely in statements that might be compiled out without the print_mac call staying.

Nice, have you though of something similar for IPv4 and IPv6 adresses?

Regards
Sven

Signature · 9 lines
 Sven Anders <[email protected]>                 () Ascii Ribbon Campaign
                                                 /\ Support plain text e-mail
 ANDURAS service solutions AG
 Innstraße 71 - 94036 Passau - Germany
 Web: www.anduras.de - Tel: +49 (0)851-4 90 50-0 - Fax: +49 (0)851-4 90 50-55

Rechtsform: Aktiengesellschaft - Sitz: Passau - Amtsgericht Passau HRB 6032
Mitglieder des Vorstands: Sven Anders, Marcus Junker
Vorsitzender des Aufsichtsrats: Mark Peters

begin:vcard
fn:Sven Anders
n:Anders;Sven
org:ANDURAS AG;Research and Development adr;quoted-printable:;;Innstra=C3=9Fe 71;Passau;Bavaria;94036;Germany email;internet:[email protected]
title:Dipl. Inf.
tel;work:++49 (0)851 / 490 50 -0
tel;fax:++49 (0)851 / 590 50 - 55
x-mozilla-html:FALSE
url:http://www.anduras.de
version:2.1
end:vcard

author
#4 lore raw
Quoted 10 lines · Sven Anders
Quoted 6 lines · Johannes Berg

This adds a new printf format specifier for the kernel, %pM, to be used to print out MAC addresses. This has advantages over the current print_mac scheme:
* no need for DECLARE_MAC_BUF
* can be used safely in statements that might be compiled out without the print_mac call staying.

Nice, have you though of something similar for IPv4 and IPv6 adresses?

Not really, are they printed often? I haven't had to touch much code that printed IP addresses. Nor have I seen them in logs a lot.

johannes

maintainer
#5 lore raw

From: Johannes Berg <[email protected]>
Date: Sun, 26 Oct 2008 09:01:36 +0100

Quoted 9 lines · Sven Anders
Quoted 8 lines
Quoted 6 lines · Johannes Berg

This adds a new printf format specifier for the kernel, %pM, to be used to print out MAC addresses. This has advantages over the current print_mac scheme:
* no need for DECLARE_MAC_BUF
* can be used safely in statements that might be compiled out without the print_mac call staying.

Quoted 5 lines
Quoted 2 lines

Nice, have you though of something similar for IPv4 and IPv6 adresses?

Not really, are they printed often? I haven't had to touch much code that printed IP addresses. Nor have I seen them in logs a lot.

It would be a nice future project for someone. But don't worry about it for now.

author
#6 lore raw
Quoted 8 lines · David Miller
Quoted 5 lines
Quoted 2 lines

Nice, have you though of something similar for IPv4 and IPv6 adresses?

Not really, are they printed often? I haven't had to touch much code that printed IP addresses. Nor have I seen them in logs a lot.

It would be a nice future project for someone. But don't worry about it for now.

Of course, actually adding, say, %p4 and %p6 would be trivial, but making lots of code use it so it's useful would be another mass conversion I'm not too keen to do again right away :)

johannes

author
#7 lore raw
Quoted 3 lines · Johannes Berg

Of course, actually adding, say, %p4 and %p6 would be trivial, but making lots of code use it so it's useful would be another mass conversion I'm not too keen to do again right away :)

Actually... At least for ipv6 that isn't true, just do something like this:

@@ include/linux/kernel.h
-#define NIP6(addr) \
-...
+#define NIP6(addr) &addr

-#define NIP6_FMT "%04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x" -#define NIP6_SEQFMT "%04x%04x%04x%04x%04x%04x%04x%04x" +#define NIP6_FMT "%p6"
+#define NIP6_SEQFMT "%#p6"

and then add the correct stuff to pointer() in lib/vsprintf.c, something like

static char *ip6_string(char *buf, char *end, be16 *addr, int field_width, int precision, int flags)

{
	char ip6[8*5];
	char *p = ip6;
	int i;
	u16 tmp;

	for (i=0; i<7; i++) {
		tmp = cpu_to_le16(addr[i]);
		p = pack_hex_byte(p, tmp >> 8);
		p = pack_hex_byte(p, tmp & 0xFF);
		if (!(flags & SPECIAL))
			*p++ = ':';
	}
	tmp = cpu_to_le16(addr[7]);
	p = pack_hex_byte(p, tmp >> 8);
	p = pack_hex_byte(p, tmp & 0xFF);
	*p = '\0';

	return string(buf, end, ip6, field_width, precision, flags & ~SPECIAL);
}
#8 lore raw
Quoted 9 lines · Johannes Berg

static char *ip6_string(char *buf, char *end, be16 *addr, int field_width, int precision, int flags)

{
	char ip6[8*5];
	char *p = ip6;
	int i;
	u16 tmp;

	for (i=0; i<7; i++) {
		tmp = cpu_to_le16(addr[i]);
		tmp = be16_to_cpup(addr + i);
Quoted 6 lines
		p = pack_hex_byte(p, tmp >> 8);
		p = pack_hex_byte(p, tmp & 0xFF);
		if (!(flags & SPECIAL))
			*p++ = ':';
	}
	tmp = cpu_to_le16(addr[7]);
	tmp = be16_to_cpup(addr + 7);

Or maybe just cast addr to char * and avoid the byteswapping altogether.

Just a thought,

Harvey

author
#9 lore raw
Quoted 20 lines · Harvey Harrison
Quoted 9 lines · Johannes Berg

static char *ip6_string(char *buf, char *end, be16 *addr, int field_width, int precision, int flags)

{
	char ip6[8*5];
	char *p = ip6;
	int i;
	u16 tmp;

	for (i=0; i<7; i++) {
		tmp = cpu_to_le16(addr[i]);
		tmp = be16_to_cpup(addr + i);
Quoted 6 lines
		p = pack_hex_byte(p, tmp >> 8);
		p = pack_hex_byte(p, tmp & 0xFF);
		if (!(flags & SPECIAL))
			*p++ = ':';
	}
	tmp = cpu_to_le16(addr[7]);
Quoted 3 lines
	tmp = be16_to_cpup(addr + 7);

Or maybe just cast addr to char * and avoid the byteswapping altogether.

Yeah, this was obviously not tested. Want to build it? I don't have access to a fast build machine right now.

johannes

#10 lore raw
Quoted 21 lines · Johannes Berg
Quoted 20 lines · Harvey Harrison
Quoted 9 lines · Johannes Berg

static char *ip6_string(char *buf, char *end, be16 *addr, int field_width, int precision, int flags)

{
	char ip6[8*5];
	char *p = ip6;
	int i;
	u16 tmp;

	for (i=0; i<7; i++) {
		tmp = cpu_to_le16(addr[i]);
		tmp = be16_to_cpup(addr + i);
Quoted 6 lines
		p = pack_hex_byte(p, tmp >> 8);
		p = pack_hex_byte(p, tmp & 0xFF);
		if (!(flags & SPECIAL))
			*p++ = ':';
	}
	tmp = cpu_to_le16(addr[7]);
Quoted 7 lines
Quoted 3 lines
	tmp = be16_to_cpup(addr + 7);

Or maybe just cast addr to char * and avoid the byteswapping altogether.

Yeah, this was obviously not tested. Want to build it? I don't have access to a fast build machine right now.

Could you send me whatever you've got currently and I'll have a go at it.

Cheers,

Harvey

author
#11 lore raw
Quoted 24 lines · Harvey Harrison
Quoted 23 lines · Johannes Berg
Quoted 22 lines · Harvey Harrison
Quoted 9 lines · Johannes Berg

static char *ip6_string(char *buf, char *end, be16 *addr, int field_width, int precision, int flags)

{
	char ip6[8*5];
	char *p = ip6;
	int i;
	u16 tmp;

	for (i=0; i<7; i++) {
		tmp = cpu_to_le16(addr[i]);
		tmp = be16_to_cpup(addr + i);
Quoted 6 lines
		p = pack_hex_byte(p, tmp >> 8);
		p = pack_hex_byte(p, tmp & 0xFF);
		if (!(flags & SPECIAL))
			*p++ = ':';
	}
	tmp = cpu_to_le16(addr[7]);
	tmp = be16_to_cpup(addr + 7);
Quoted 8 lines
Quoted 5 lines
Quoted 1 line

Or maybe just cast addr to char * and avoid the byteswapping altogether.

Yeah, this was obviously not tested. Want to build it? I don't have access to a fast build machine right now.

Could you send me whatever you've got currently and I'll have a go at it.

I don't have anything. I typed this into the email directly.

johannes

#12 lore raw

RFC PATCH 1/1 printk: add the %pM, %p4, %p6 format specifiers

Add format specifiers for printing out colon-separated bytes:

MAC addresses (%pM):
xx:xx:xx:xx:xx:xx

IPv4 addresses (%p4):
xx:xx:xx:xx

IPv6 addresses (%p6):
xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx

%#pM, %#p4, %#p6 are also supported and print without the colon separators.

[Based on Johannes Berg's initial patch]
Signed-off-by Harvey Harrison <[email protected]>
---
 lib/vsprintf.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 64 insertions(+), 0 deletions(-)
lib/vsprintf.c +64 -0
index a013bbc..eb19471 100644
@@ -581,6 +581,58 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
 	return string(buf, end, sym, field_width, precision, flags);
 }
 
+static char *ip4_address(char *buf, char *end, u8 *addr, int field_width,
+			 int precision, int flags)
+{
+	char ip4_addr[4 * 3]; /* (4 * 2 hex digits), 3 colons and trailing zero */
+	char *p = ip4_addr;
+	int i;
+
+	for (i = 0; i < 4; i++) {
+		p = pack_hex_byte(p, addr[i]);
+		if (!(flags & SPECIAL) && i != 3)
+			*p++ = ':';
+	}
+	*p = '\0';
+
+	return string(buf, end, ip4_addr, field_width, precision, flags & ~SPECIAL);
+}
+
+static char *ip6_address(char *buf, char *end, u8 *addr, int field_width,
+			 int precision, int flags)
+{
+	char ip6_addr[8 * 5]; /* (8 * 4 hex digits), 7 colons and trailing zero */
+	char *p = ip6_addr;
+	int i;
+
+	for (i = 0; i < 7; i++) {
+		p = pack_hex_byte(p, addr[2 * i]);
+		p = pack_hex_byte(p, addr[2 * i + 1]);
+		if (!(flags & SPECIAL) && i != 6)
+			*p++ = ':';
+	}
+	*p = '\0';
+
+	return string(buf, end, ip6_addr, field_width, precision, flags & ~SPECIAL);
+}
+
+static char *mac_address(char *buf, char *end, u8 *addr, int field_width,
+			 int precision, int flags)
+{
+	char mac_addr[6 * 3]; /* (6 * 2 hex digits), 5 colons and trailing zero */
+	char *p = mac_addr;
+	int i;
+
+	for (i = 0; i < 5; i++) {
+		p = pack_hex_byte(p, addr[i]);
+		if (!(flags & SPECIAL) && i != 4)
+			*p++ = ':';
+	}
+	*p = '\0';
+
+	return string(buf, end, mac_addr, field_width, precision, flags & ~SPECIAL);
+}
+
 /*
  * Show a '%p' thing.  A kernel extension is that the '%p' is followed
  * by an extra set of alphanumeric characters that are extended format

@@ -592,6 +644,12 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
  * - 'S' For symbolic direct pointers
  * - 'R' For a struct resource pointer, it prints the range of
  *       addresses (not the name nor the flags)
+ * - 'M' For a 6-byte MAC address, it prints the address in the
+ *       usual colon-separated hex notation
+ * - '4' For a 4-byte IPv4 address, it prints the address in the
+ *       usual colon-separated hex notation
+ * - 'M' For a 16-byte IPv6 address, it prints the address in colon separated
+ *       big-endian 16 bit hex notation
  *
  * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
  * function pointers are really function descriptors, which contain a

@@ -607,6 +665,12 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
 		return symbol_string(buf, end, ptr, field_width, precision, flags);
 	case 'R':
 		return resource_string(buf, end, ptr, field_width, precision, flags);
+	case 'M':
+		return mac_address(buf, end, ptr, field_width, precision, flags);
+	case '4':
+		return ip4_address(buf, end, ptr, field_width, precision, flags);
+	case '6':
+		return ip6_address(buf, end, ptr, field_width, precision, flags);
 	}
 	flags |= SMALL;
 	if (field_width == -1) {
Signature · 1 line
1.6.0.3.729.g6ea410
#13 lore raw

RFC PATCH v2 1/1 printk: add the %pM, %p4, %p6 format specifiers

Add format specifiers for printing out colon-separated bytes:

MAC addresses (%pM):
xx:xx:xx:xx:xx:xx

IPv4 addresses (%p4):
xx:xx:xx:xx

IPv6 addresses (%p6):
xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx

%#pM, %#p4, %#p6 are also supported and print without the colon separators.

[Based on Johannes Berg's initial patch]
Signed-off-by Harvey Harrison <[email protected]>

---
This one without the embarrassing index typos in ip6_address and mac_address.

 lib/vsprintf.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 64 insertions(+), 0 deletions(-)
lib/vsprintf.c +64 -0
index a013bbc..eec3879 100644
@@ -581,6 +581,58 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
 	return string(buf, end, sym, field_width, precision, flags);
 }
 
+static char *ip4_address(char *buf, char *end, u8 *addr, int field_width,
+			 int precision, int flags)
+{
+	char ip4_addr[4 * 3]; /* (4 * 2 hex digits), 3 colons and trailing zero */
+	char *p = ip4_addr;
+	int i;
+
+	for (i = 0; i < 4; i++) {
+		p = pack_hex_byte(p, addr[i]);
+		if (!(flags & SPECIAL) && i != 3)
+			*p++ = ':';
+	}
+	*p = '\0';
+
+	return string(buf, end, ip4_addr, field_width, precision, flags & ~SPECIAL);
+}
+
+static char *ip6_address(char *buf, char *end, u8 *addr, int field_width,
+			 int precision, int flags)
+{
+	char ip6_addr[8 * 5]; /* (8 * 4 hex digits), 7 colons and trailing zero */
+	char *p = ip6_addr;
+	int i;
+
+	for (i = 0; i < 8; i++) {
+		p = pack_hex_byte(p, addr[2 * i]);
+		p = pack_hex_byte(p, addr[2 * i + 1]);
+		if (!(flags & SPECIAL) && i != 7)
+			*p++ = ':';
+	}
+	*p = '\0';
+
+	return string(buf, end, ip6_addr, field_width, precision, flags & ~SPECIAL);
+}
+
+static char *mac_address(char *buf, char *end, u8 *addr, int field_width,
+			 int precision, int flags)
+{
+	char mac_addr[6 * 3]; /* (6 * 2 hex digits), 5 colons and trailing zero */
+	char *p = mac_addr;
+	int i;
+
+	for (i = 0; i < 6; i++) {
+		p = pack_hex_byte(p, addr[i]);
+		if (!(flags & SPECIAL) && i != 5)
+			*p++ = ':';
+	}
+	*p = '\0';
+
+	return string(buf, end, mac_addr, field_width, precision, flags & ~SPECIAL);
+}
+
 /*
  * Show a '%p' thing.  A kernel extension is that the '%p' is followed
  * by an extra set of alphanumeric characters that are extended format

@@ -592,6 +644,12 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
  * - 'S' For symbolic direct pointers
  * - 'R' For a struct resource pointer, it prints the range of
  *       addresses (not the name nor the flags)
+ * - 'M' For a 6-byte MAC address, it prints the address in the
+ *       usual colon-separated hex notation
+ * - '4' For a 4-byte IPv4 address, it prints the address in the
+ *       usual colon-separated hex notation
+ * - 'M' For a 16-byte IPv6 address, it prints the address in colon separated
+ *       big-endian 16 bit hex notation
  *
  * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
  * function pointers are really function descriptors, which contain a

@@ -607,6 +665,12 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
 		return symbol_string(buf, end, ptr, field_width, precision, flags);
 	case 'R':
 		return resource_string(buf, end, ptr, field_width, precision, flags);
+	case 'M':
+		return mac_address(buf, end, ptr, field_width, precision, flags);
+	case '4':
+		return ip4_address(buf, end, ptr, field_width, precision, flags);
+	case '6':
+		return ip6_address(buf, end, ptr, field_width, precision, flags);
 	}
 	flags |= SMALL;
 	if (field_width == -1) {
Signature · 1 line
1.6.0.3.729.g6ea410
#14 lore raw

Re: [RFC PATCHv2] printk: add the %pM, %p4, %p6 format specifiers

Quoted 7 lines · Harvey Harrison
lib/vsprintf.c +0 -0
index a013bbc..eec3879 100644
@@ -592,6 +644,12 @@ static char *resource_string(char *buf, char *end,

struct resource *res, int fie
* - 'S' For symbolic direct pointers

Quoted 8 lines

* - 'R' For a struct resource pointer, it prints the range of
* addresses (not the name nor the flags)
+ * - 'M' For a 6-byte MAC address, it prints the address in the
+ * usual colon-separated hex notation
+ * - '4' For a 4-byte IPv4 address, it prints the address in the
+ * usual colon-separated hex notation
+ * - 'M' For a 16-byte IPv6 address, it prints the address in colon separated
+ * big-endian 16 bit hex notation

Shouldn't that last one be '6' instead of 'M'?

#15 lore raw

Re: [RFC PATCHv2] printk: add the %pM, %p4, %p6 format specifiers

Quoted 2 lines · Frans Pop
Quoted 2 lines

+ * - '4' For a 4-byte IPv4 address, it prints the address in the
+ * usual colon-separated hex notation

Since when are IPv4 addresses "usually" printed in colon-separated hex?

-hpa

author
#16 lore raw

Re: [RFC PATCHv2] printk: add the %pM, %p4, %p6 format specifiers

In reply to Harvey Harrison
Quoted 7 lines · Harvey Harrison

Add format specifiers for printing out colon-separated bytes:

MAC addresses (%pM):
xx:xx:xx:xx:xx:xx

IPv4 addresses (%p4):
xx:xx:xx:xx

As hpa points out, the usual notation would be %d.%d.%d.%d :)

Quoted 2 lines

%#pM, %#p4, %#p6 are also supported and print without the colon separators.

The # for ipv4 we might use for cpu-endian rather than network-endian maybe?

Also can you keep mac/ip address patches split up so we can use my version of the mac address patch (it's already well-tested)?

johannes

#17 lore raw

Re: [RFC PATCHv2] printk: add the %pM, %p4, %p6 format specifiers

Quoted 3 lines · Johannes Berg

Also can you keep mac/ip address patches split up so we can use my version of the mac address patch (it's already well-tested)?

Sure. I'll build on top.

Harvey

maintainer
#18 lore raw

Re: [RFC PATCHv2] printk: add the %pM, %p4, %p6 format specifiers

From: Harvey Harrison <[email protected]>
Date: Mon, 27 Oct 2008 09:28:24 -0700

Quoted 7 lines · Johannes Berg
Quoted 4 lines

Also can you keep mac/ip address patches split up so we can use my version of the mac address patch (it's already well-tested)?

Sure. I'll build on top.

So I'll wait for Harvey's "v3" of this patch, and once I have that in the net-next-2.6 tree I'll start adding Johannes's conversions on top.

author
#19 lore raw

Re: [RFC PATCHv2] printk: add the %pM, %p4, %p6 format specifiers

Quoted 5 lines · David Miller

From: Harvey Harrison <[email protected]>
Date: Mon, 27 Oct 2008 09:28:24 -0700

Quoted 2 lines · Johannes Berg
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

Also can you keep mac/ip address patches split up so we can use my version of the mac address patch (it's already well-tested)?

Sure. I'll build on top.

So I'll wait for Harvey's "v3" of this patch, and once I have that in the net-next-2.6 tree I'll start adding Johannes's conversions on top.

Ok, fine with me too. Harvey, can you sort out the ip4 issue and then send with %pM included?

johannes

#20 lore raw

RFC PATCH v3 1/1 printk: add %pM format specifier for MAC addresses

In reply to David Miller

Add format specifiers for printing out six colon-separated bytes:

MAC addresses (%pM):
xx:xx:xx:xx:xx:xx

%#pM is also supported and omits the colon separators.

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

---
Dave, this passes testing here, but I was wondering if perhaps it would be better to allow a length to be specified as well, which would allow:

%pM6 for mac addresses, etc as there seem a lot of places in kernel that print out a list of colon separated bytes of various lengths.

But if that was added, it may be more natural to call it %pB (bytes)
%pW (words)

Then mac addresses would be %pB6
IPv6 addresses would be %pW8 (8 words)

It would be trivial to add, but maybe I'm overthinking this. In any event, this patch only adds %pM for mac addresses.

 lib/vsprintf.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)
lib/vsprintf.c +21 -0
index a013bbc..2025305 100644
@@ -581,6 +581,23 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
 	return string(buf, end, sym, field_width, precision, flags);
 }
 
+static char *mac_address(char *buf, char *end, u8 *addr, int field_width,
+			 int precision, int flags)
+{
+	char mac_addr[6 * 3]; /* (6 * 2 hex digits), 5 colons and trailing zero */
+	char *p = mac_addr;
+	int i;
+
+	for (i = 0; i < 6; i++) {
+		p = pack_hex_byte(p, addr[i]);
+		if (!(flags & SPECIAL) && i != 5)
+			*p++ = ':';
+	}
+	*p = '\0';
+
+	return string(buf, end, mac_addr, field_width, precision, flags & ~SPECIAL);
+}
+
 /*
  * Show a '%p' thing.  A kernel extension is that the '%p' is followed
  * by an extra set of alphanumeric characters that are extended format

@@ -592,6 +609,8 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
  * - 'S' For symbolic direct pointers
  * - 'R' For a struct resource pointer, it prints the range of
  *       addresses (not the name nor the flags)
+ * - 'M' For a 6-byte MAC address, it prints the address in the
+ *       usual colon-separated hex notation
  *
  * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
  * function pointers are really function descriptors, which contain a

@@ -607,6 +626,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
 		return symbol_string(buf, end, ptr, field_width, precision, flags);
 	case 'R':
 		return resource_string(buf, end, ptr, field_width, precision, flags);
+	case 'M':
+		return mac_address(buf, end, ptr, field_width, precision, flags);
 	}
 	flags |= SMALL;
 	if (field_width == -1) {
Signature · 1 line
1.6.0.3.729.g6ea410
#21 lore raw

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

On Mon, 2008-10-27 at 12:59 -0700, Harvey Harrison wrote:

The changes to use %p4 aren't too bad. I did that last year using a similar style to print_mac. 80 or so files.
http://repo.or.cz/w/linux-2.6/trivial-mods.git?a=shortlog;h=refs/heads/print_ipv4
ipv6 was 30 or so files
http://repo.or.cz/w/linux-2.6/trivial-mods.git?a=shortlog;h=refs/heads/print_ipv6

Quoted 2 lines

I was wondering if perhaps it would be
better to allow a length to be specified as well, which would allow:

[]

Quoted 3 lines

But if that was added, it may be more natural to call it %pB (bytes)
%pW (words)

I think length would not be good addition. print_dump_hex already does a fine job. There are also BE/LE expectation problems with pW.

The %p6 pointer should be in6_addr *
The %p4 pointer should be __be32 *. Printing an ipv4 address should use %u.%u.%u.%u

I've been doodling with sparse to check calls with __attribute__(format(printf parsing the format strings for %p<FOO> and match the argument types.

I'll post it if anything comes of it.

lib/vsprintf.c static char *resource_string(char *buf, char *end, struct resource *res, int fie · line 586

Quoted 3 lines
lib/vsprintf.c +3 -0
+static char *mac_address(char *buf, char *end, u8 *addr, int field_width,
+			 int precision, int flags)
+{

mac_address_string please.

maintainer
#22 lore raw

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

From: Joe Perches <[email protected]>
Date: Mon, 27 Oct 2008 14:55:15 -0700

Quoted 7 lines · Harvey Harrison

lib/vsprintf.c static char *resource_string(char *buf, char *end, struct resource *res, int fie · line 586

Quoted 4 lines
lib/vsprintf.c +3 -0
+static char *mac_address(char *buf, char *end, u8 *addr, int field_width,
+			 int precision, int flags)
+{

mac_address_string please.

I'll make this change when I apply Harvey's patch.

#24 lore raw

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

Quoted 5 lines · David Miller

From: Harvey Harrison <[email protected]>
Date: Mon, 27 Oct 2008 12:59:02 -0700

Quoted 2 lines

Add format specifiers for printing out six colon-separated bytes:

Quoted 8 lines
Quoted 6 lines

MAC addresses (%pM):
xx:xx:xx:xx:xx:xx

%#pM is also supported and omits the colon separators.

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

Applied, thanks.

Did you happen to have a preference with regard to the specifier for IPv6 addresses:

I was thinking of using %pI6 to replace NIP6() and NIP6_FMT and using %#pI6 for NIP6_SEQFMT.

On the IPv4 side, maybe use %pI4 for network endian NIPQUAD() and NIPQUAD_FMT and then %#pI4 for host-endian HIQUAD(), as displaying the IPv4 address without the periods isn't useful?

What do you think of that?

Harvey

#25 lore raw

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

Quoted 18 lines · Harvey Harrison
Quoted 5 lines · David Miller

From: Harvey Harrison <[email protected]>
Date: Mon, 27 Oct 2008 12:59:02 -0700

Quoted 2 lines

Add format specifiers for printing out six colon-separated bytes:

Quoted 8 lines
Quoted 6 lines

MAC addresses (%pM):
xx:xx:xx:xx:xx:xx

%#pM is also supported and omits the colon separators.

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

Applied, thanks.

Did you happen to have a preference with regard to the specifier for IPv6 addresses:

Quoted 6 lines

I was thinking of using %pI6 to replace NIP6() and NIP6_FMT and using %#pI6 for NIP6_SEQFMT.

On the IPv4 side, maybe use %pI4 for network endian NIPQUAD() and NIPQUAD_FMT and then %#pI4 for host-endian HIQUAD(), as displaying the IPv4 address without the periods isn't useful?

%#pI4 is horrible and %p4 and %p6 were cool.

#26 lore raw

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

In reply to Harvey Harrison
Quoted 6 lines · Harvey Harrison

I was thinking of using %pI6 to replace NIP6() and NIP6_FMT and using %#pI6 for NIP6_SEQFMT.

On the IPv4 side, maybe use %pI4 for network endian NIPQUAD() and NIPQUAD_FMT and then %#pI4 for host-endian HIQUAD(), as displaying the IPv4 address without the periods isn't useful?

HIPQUAD is horrible and should disappear.
Just use htonl first.

I think just %p4 %p6 %#p6 for the NIP6_SEQFMT uses.

author
#27 lore raw

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

Quoted 10 lines · Joe Perches
Quoted 6 lines · Harvey Harrison

I was thinking of using %pI6 to replace NIP6() and NIP6_FMT and using %#pI6 for NIP6_SEQFMT.

On the IPv4 side, maybe use %pI4 for network endian NIPQUAD() and NIPQUAD_FMT and then %#pI4 for host-endian HIQUAD(), as displaying the IPv4 address without the periods isn't useful?

HIPQUAD is horrible and should disappear.
Just use htonl first.

But you can't really say

printk("... %p4 ...", &htonl(host_ip4));

can you? I suppose the compiler would actually create a variable for that?

johannes

maintainer
#28 lore raw

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

In reply to Harvey Harrison

From: Harvey Harrison <[email protected]>
Date: Mon, 27 Oct 2008 16:14:14 -0700

Quoted 11 lines

Did you happen to have a preference with regard to the specifier for IPv6 addresses:

I was thinking of using %pI6 to replace NIP6() and NIP6_FMT and using %#pI6 for NIP6_SEQFMT.

On the IPv4 side, maybe use %pI4 for network endian NIPQUAD() and NIPQUAD_FMT and then %#pI4 for host-endian HIQUAD(), as displaying the IPv4 address without the periods isn't useful?

What do you think of that?

I'm ok with the '#' modifier but remind me again what's wrong with using plain %p4 and %p6?

#29 lore raw

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

Quoted 9 lines · David Miller

From: Harvey Harrison <[email protected]>
Date: Mon, 27 Oct 2008 16:14:14 -0700

Quoted 6 lines

Did you happen to have a preference with regard to the specifier for IPv6 addresses:

I was thinking of using %pI6 to replace NIP6() and NIP6_FMT and using %#pI6 for NIP6_SEQFMT.

Quoted 8 lines
Quoted 5 lines

On the IPv4 side, maybe use %pI4 for network endian NIPQUAD() and NIPQUAD_FMT and then %#pI4 for host-endian HIQUAD(), as displaying the IPv4 address without the periods isn't useful?

What do you think of that?

I'm ok with the '#' modifier but remind me again what's wrong with using plain %p4 and %p6?

Nothing at all, just wanted to know what color bikeshed you wanted.

I'll go with %p4 %p6.

Harvey

#30 lore raw

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

Quoted 5 lines · Harvey Harrison
Quoted 2 lines · David Miller

I'm ok with the '#' modifier but remind me again what's wrong with using plain %p4 and %p6?

Nothing at all, just wanted to know what color bikeshed you wanted. I'll go with %p4 %p6.

Just an FYI for v6 addresses:

There are a few uses of ipv6 address as "%08X%08X%08X%08X" Pavel Emelyanov tried to isolate them with a suggested patch:

http://www.mail-archive.com/[email protected]/msg51655.html

These addresses are not shown in network order, but are printed backwards in host order. Oops. Apparently it's too late to fix them.

author
#31 lore raw

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

Quoted 9 lines · Joe Perches
Quoted 5 lines · Harvey Harrison
Quoted 2 lines · David Miller

I'm ok with the '#' modifier but remind me again what's wrong with using plain %p4 and %p6?

Nothing at all, just wanted to know what color bikeshed you wanted. I'll go with %p4 %p6.

Just an FYI for v6 addresses:

Quoted 8 lines

There are a few uses of ipv6 address as "%08X%08X%08X%08X" Pavel Emelyanov tried to isolate them with a suggested patch:

http://www.mail-archive.com/[email protected]/msg51655.html

These addresses are not shown in network order, but are printed backwards in host order. Oops. Apparently it's too late to fix them.

So that code we don't touch here at all.

johannes

author
#32 lore raw
In reply to Johannes Berg
Quoted 6 lines · Johannes Berg
Quoted 2 lines

It would be a nice future project for someone. But don't worry about it for now.

Of course, actually adding, say, %p4 and %p6 would be trivial, but making lots of code use it so it's useful would be another mass conversion I'm not too keen to do again right away :)

And for IPv4 you can do it in a similar way, except for HIPQUAD which you'd have to actually replace in the code, but there are less than 20 instances of it. Maybe use SPECIAL to indicate host byte order or something, I'll leave that to the interested, unless I get really bored next week (I'm kinda bored right now but don't have access to a fast build machine ;) )

johannes