diff mbox

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

Message ID 1225137542.5396.10.camel@brick
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Harvey Harrison Oct. 27, 2008, 7:59 p.m. UTC
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 <harvey.harrison@gmail.com>
---
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(-)

Comments

Joe Perches Oct. 27, 2008, 9:55 p.m. UTC | #1
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

> I was wondering if perhaps it would be
> better to allow a length to be specified as well, which would allow:
[]
> 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.

> +static char *mac_address(char *buf, char *end, u8 *addr, int field_width,
> +			 int precision, int flags)
> +{

mac_address_string please.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Oct. 27, 2008, 10:46 p.m. UTC | #2
From: Joe Perches <joe@perches.com>
Date: Mon, 27 Oct 2008 14:55:15 -0700

> On Mon, 2008-10-27 at 12:59 -0700, Harvey Harrison wrote:
> 
> > +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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Oct. 27, 2008, 10:47 p.m. UTC | #3
From: Harvey Harrison <harvey.harrison@gmail.com>
Date: Mon, 27 Oct 2008 12:59:02 -0700

> 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 <harvey.harrison@gmail.com>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Harvey Harrison Oct. 27, 2008, 11:14 p.m. UTC | #4
On Mon, 2008-10-27 at 15:47 -0700, David Miller wrote:
> From: Harvey Harrison <harvey.harrison@gmail.com>
> Date: Mon, 27 Oct 2008 12:59:02 -0700
> 
> > 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 <harvey.harrison@gmail.com>
> 
> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Alexey Dobriyan Oct. 27, 2008, 11:31 p.m. UTC | #5
On Mon, Oct 27, 2008 at 04:14:14PM -0700, Harvey Harrison wrote:
> On Mon, 2008-10-27 at 15:47 -0700, David Miller wrote:
> > From: Harvey Harrison <harvey.harrison@gmail.com>
> > Date: Mon, 27 Oct 2008 12:59:02 -0700
> > 
> > > 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 <harvey.harrison@gmail.com>
> > 
> > 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?

%#pI4 is horrible and %p4 and %p6 were cool.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joe Perches Oct. 27, 2008, 11:48 p.m. UTC | #6
On Mon, 2008-10-27 at 16:14 -0700, Harvey Harrison wrote:
> 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.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Oct. 27, 2008, 11:55 p.m. UTC | #7
From: Harvey Harrison <harvey.harrison@gmail.com>
Date: Mon, 27 Oct 2008 16:14:14 -0700

> 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?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Harvey Harrison Oct. 28, 2008, 12:05 a.m. UTC | #8
On Mon, 2008-10-27 at 16:55 -0700, David Miller wrote:
> From: Harvey Harrison <harvey.harrison@gmail.com>
> Date: Mon, 27 Oct 2008 16:14:14 -0700
> 
> > 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?

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

I'll go with %p4 %p6.

Harvey

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joe Perches Oct. 28, 2008, 12:22 a.m. UTC | #9
On Mon, 2008-10-27 at 17:05 -0700, Harvey Harrison wrote:
> On Mon, 2008-10-27 at 16:55 -0700, David Miller wrote:
> > 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/netdev@vger.kernel.org/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.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johannes Berg Oct. 28, 2008, 8:04 a.m. UTC | #10
On Mon, 2008-10-27 at 16:48 -0700, Joe Perches wrote:
> On Mon, 2008-10-27 at 16:14 -0700, Harvey Harrison wrote:
> > 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
Johannes Berg Oct. 28, 2008, 8:06 a.m. UTC | #11
On Mon, 2008-10-27 at 17:22 -0700, Joe Perches wrote:
> On Mon, 2008-10-27 at 17:05 -0700, Harvey Harrison wrote:
> > On Mon, 2008-10-27 at 16:55 -0700, David Miller wrote:
> > > 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/netdev@vger.kernel.org/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
diff mbox

Patch

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index a013bbc..2025305 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -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) {