| Message ID | 4A836D6D.1040400@hp.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, 2009-08-12 at 21:33 -0400, Brian Haley wrote: > Jens Rosenboom wrote: > > Currently the output looks like 2001:0db8:0000:0000:0000:0000:0000:0001 > > which might be compacted to 2001:db8::1. The code to do this could be > > adapted from inet_ntop in glibc, which would add about 80 lines to > > lib/vsprintf.c. How do you guys value the tradeoff between more readable > > logging and increased kernel size? > > > > This was already mentioned in > > http://kerneltrap.org/mailarchive/linux-netdev/2008/11/25/4231684 but > > noone seems to have taken up on it. > > Anyways, can you try this patch, it's less than 40 new lines :) > It might be good enough, but could probably use some help. You'll need to invent a new %p qualifier type to allow compressed representation. Your patch will change current uses with seq_<foo> output in net, which could break userspace. -- 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
On Thu, 2009-08-13 at 03:39 -0700, Joe Perches wrote: > On Wed, 2009-08-12 at 21:33 -0400, Brian Haley wrote: > > Jens Rosenboom wrote: > > > Currently the output looks like 2001:0db8:0000:0000:0000:0000:0000:0001 > > > which might be compacted to 2001:db8::1. The code to do this could be > > > adapted from inet_ntop in glibc, which would add about 80 lines to > > > lib/vsprintf.c. How do you guys value the tradeoff between more readable > > > logging and increased kernel size? > > > > > > This was already mentioned in > > > http://kerneltrap.org/mailarchive/linux-netdev/2008/11/25/4231684 but > > > noone seems to have taken up on it. > > > > Anyways, can you try this patch, it's less than 40 new lines :) > > It might be good enough, but could probably use some help. > > You'll need to invent a new %p qualifier type to allow > compressed representation. Your patch will change current uses > with seq_<foo> output in net, which could break userspace. Would it be possible to transform this to using %pi6, as most of teh seq_* stuff already does? It doesn't make sense to shorten the un-colon-ed version anyway, I'll send an updated version of Brian's patch soon. -- 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
On Thu, 2009-08-13 at 15:52 +0200, Jens Rosenboom wrote: > On Thu, 2009-08-13 at 03:39 -0700, Joe Perches wrote: > > You'll need to invent a new %p qualifier type to allow > > compressed representation. Your patch will change current uses > > with seq_<foo> output in net, which could break userspace. > > Would it be possible to transform this to using %pi6, as most of teh > seq_* stuff already does? No. There are applications that depend on the current output representation. I think it should be done with something like "%pi6c" rather than using another %p character because there are a limited number of single characters available. -- 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
diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 756ccaf..58602ba 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -652,13 +652,46 @@ static char *ip6_addr_string(char *buf, char *end, u8 *addr, { char ip6_addr[8 * 5]; /* (8 * 4 hex digits), 7 colons and trailing zero */ char *p = ip6_addr; - int i; + int i, needcolon = 0, printhi; + u16 *addr16 = (u16 *)addr; + enum { DC_START, DC_MIDDLE, DC_DONE } dcolon = DC_START; + + /* omit leading zeros and shorten using "::" */ for (i = 0; i < 8; i++) { - p = pack_hex_byte(p, addr[2 * i]); - p = pack_hex_byte(p, addr[2 * i + 1]); - if (!(spec.flags & SPECIAL) && i != 7) - *p++ = ':'; + if (!(spec.flags & SPECIAL)) { + if (addr16[i] == 0 && colon < DC_DONE) { + colon = DC_MIDDLE; + continue; + } + if (colon == DC_MIDDLE) { + colon = DC_DONE; + *p++ = ':'; + *p++ = ':'; + } else if (needcolon) + *p++ = ':'; + } + printhi = 0; + if (addr[2 * i]) { + if (addr[2 * i] > 0x0f) + p = pack_hex_byte(p, addr[2 * i]); + else + *p++ = hex_asc_lo(addr[2 * i]); + printhi++; + } + /* + * If we printed the high-order bits we must print the + * low-order ones, even if they're all zeros. + */ + if (printhi || addr[2 * i + 1] > 0x0f) + p = pack_hex_byte(p, addr[2 * i + 1]); + else if (addr[2 * i + 1]) + *p++ = hex_asc_lo(addr[2 * i + 1]); + needcolon++; + } + if (colon == DC_MIDDLE) { + *p++ = ':'; + *p++ = ':'; } *p = '\0'; spec.flags &= ~SPECIAL;