[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <48AAB7CB-8EEC-4E30-B821-3E0D84531AAA@oracle.com>
Date: Thu, 13 Aug 2009 14:15:41 -0400
From: Chuck Lever <chuck.lever@...cle.com>
To: Joe Perches <joe@...ches.com>
Cc: Brian Haley <brian.haley@...com>, Jens Rosenboom <jens@...one.net>,
Linux Network Developers <netdev@...r.kernel.org>
Subject: Re: [RFC] ipv6: Change %pI6 format to output compacted addresses?
On Aug 13, 2009, at 2:10 PM, Joe Perches wrote:
> On Thu, 2009-08-13 at 12:27 -0400, Brian Haley wrote:
>> Jens Rosenboom wrote:
>>> Here is a new version that also
>>> fixes
>>>
>>> - Leave %pi6 alone
>>> - Don't compress a single :0:
>>> - Do output 0
>>> The results and also the remaining issues can be seen with the
>>> attached
>>> test program, that also exposes a bug in glibc for v4-mapped
>>> addresses
>>> from 0/16.
>>> To fully conform to the cited draft, we would still have to
>>> implement
>>> v4-mapped and also check whether a second run of zeros would be
>>> longer
>>> than the first one, although the draft also suggests that operators
>>> should avoid using this kind of addresses, so maybe this second
>>> issue
>>> can be neglected.
>>
>> Yes, the "compress the most zeros" would be harder, and require two
>> passes over the address. I had to cut corners somewhere :)
>
> 2 things.
>
> First a question, then a compilable but untested patch.
>
> The patch allows "%p6ic" for compressed and "%p6ic4" for compressed
> with ipv4 last u32.
Why do these need to be separate?
> Can somebody tell me what I'm doing wrong when I link Jens' test?
>
> cc -o test test_ipv6.c lib/vsprintf.o lib/ctype.o
> lib/vsprintf.o: In function `global constructors keyed to
> 65535_0_simple_strtoul':
> /home/joe/linux/linux-2.6/lib/vsprintf.c:1972: undefined reference to
> `__gcov_init'
> lib/vsprintf.o:(.data+0x28): undefined reference to `__gcov_merge_add'
> collect2: ld returned 1 exit status
>
>
> Now for the patch. Perhaps something like this (compiled, untested)
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 756ccaf..dd02842 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -630,7 +630,7 @@ static char *resource_string(char *buf, char
> *end, struct resource *res,
> }
>
> static char *mac_address_string(char *buf, char *end, u8 *addr,
> - struct printf_spec spec)
> + struct printf_spec spec, const char *fmt)
> {
> char mac_addr[6 * 3]; /* (6 * 2 hex digits), 5 colons and trailing
> zero */
> char *p = mac_addr;
> @@ -638,36 +638,128 @@ static char *mac_address_string(char *buf,
> char *end, u8 *addr,
>
> for (i = 0; i < 6; i++) {
> p = pack_hex_byte(p, addr[i]);
> - if (!(spec.flags & SPECIAL) && i != 5)
> + if (!(fmt[0] == 'm') && i != 5)
> *p++ = ':';
> }
> *p = '\0';
> - spec.flags &= ~SPECIAL;
>
> return string(buf, end, mac_addr, spec);
> }
>
> +typedef enum { DC_START, DC_MIDDLE, DC_DONE } ip6_colon_t;
> +
> +static char *ip6_compress_u16(char *p, u16 addr16, u16 addr16_next,
> + ip6_colon_t *colon, bool *needcolon)
> +{
> + bool printhi;
> + u8 hi;
> + u8 lo;
> +
> + if (addr16 == 0 && addr16_next == 0 && *colon == DC_START) {
> + *colon = DC_MIDDLE;
> + return p;
> + }
> + if (*colon == DC_MIDDLE) {
> + if (addr16 == 0)
> + return p;
> + *colon = DC_DONE;
> + *p++ = ':';
> + *p++ = ':';
> + } else if (*needcolon)
> + *p++ = ':';
> +
> + printhi = false;
> + hi = addr16 >> 8;
> + lo = addr16 & 0xff;
> + if (hi) {
> + if (hi > 0x0f)
> + p = pack_hex_byte(p, hi);
> + else
> + *p++ = hex_asc_lo(hi);
> + printhi = true;
> + }
> + /*
> + * If we printed the high-order bits we must print the
> + * low-order ones, even if they're all zeros.
> + */
> + if (printhi || lo > 0x0f)
> + p = pack_hex_byte(p, lo);
> + else
> + *p++ = hex_asc_lo(lo);
> + *needcolon = true;
> +
> + return p;
> +}
> +
> +static char *ip6_compressed_string(char *buf, char *end, u8 *addr,
> + struct printf_spec spec, const char *fmt,
> + char *ip6_addr)
> +{
> + char *p = ip6_addr;
> + int i;
> + bool needcolon = false;
> + u16 *addr16 = (u16 *)addr;
> + ip6_colon_t colon = DC_START;
> +
> + if (fmt[3] == '4') { /* use :: and ipv4 */
> + for (i = 0; i < 6; i++) {
> + p = ip6_compress_u16(p, addr16[i], addr16[i+1],
> + &colon, &needcolon);
> + }
> + if (colon != DC_DONE) {
> + *p++ = ':';
> + *p++ = ':';
> + colon = DC_DONE;
> + }
> + addr += 6 * 2;
> + for (i = 0; i < 4; i++) {
> + p = put_dec_trunc(p, *addr++);
> + if (i != 3)
> + *p++ = '.';
> + }
> + } else {
> + for (i = 0; i < 7; i++) {
> + p = ip6_compress_u16(p, addr16[i], addr16[i+1],
> + &colon, &needcolon);
> + }
> + p = ip6_compress_u16(p, addr16[7], 0xff, &colon, &needcolon);
> + }
> +
> + if (colon == DC_MIDDLE) {
> + *p++ = ':';
> + *p++ = ':';
> + }
> +
> + *p = '\0';
> +
> + return string(buf, end, ip6_addr, spec);
> +}
> +
> static char *ip6_addr_string(char *buf, char *end, u8 *addr,
> - struct printf_spec spec)
> + struct printf_spec spec, const char *fmt)
> {
> - char ip6_addr[8 * 5]; /* (8 * 4 hex digits), 7 colons and trailing
> zero */
> + char ip6_addr[7 * 4 + 7 + 4 * 4]; /* (7 * 4 hex digits) + 7 colons +
> + * ipv4 address, and trailing zero */
> char *p = ip6_addr;
> int i;
>
> + if (fmt[0] == 'i' && fmt[2] == 'c')
> + return ip6_compressed_string(buf, end, addr, spec, fmt,
> + ip6_addr);
> +
> 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)
> + if (fmt[0] == 'i' && i != 7)
> *p++ = ':';
> }
> *p = '\0';
> - spec.flags &= ~SPECIAL;
>
> return string(buf, end, ip6_addr, spec);
> }
>
> static char *ip4_addr_string(char *buf, char *end, u8 *addr,
> - struct printf_spec spec)
> + struct printf_spec spec)
> {
> char ip4_addr[4 * 4]; /* (4 * 3 decimal digits), 3 dots and
> trailing zero */
> char temp[3]; /* hold each IP quad in reverse order */
> @@ -683,11 +775,18 @@ static char *ip4_addr_string(char *buf, char
> *end, u8 *addr,
> *p++ = '.';
> }
> *p = '\0';
> - spec.flags &= ~SPECIAL;
>
> return string(buf, end, ip4_addr, spec);
> }
>
> +static char *ip_addr_string(char *buf, char *end, u8 *addr,
> + struct printf_spec spec, const char *fmt)
> +{
> + if (fmt[1] == '6')
> + return ip6_addr_string(buf, end, addr, spec, fmt);
> + return ip4_addr_string(buf, end, addr, spec);
> +}
> +
> /*
> * Show a '%p' thing. A kernel extension is that the '%p' is followed
> * by an extra set of alphanumeric characters that are extended format
> @@ -726,20 +825,19 @@ static char *pointer(const char *fmt, char
> *buf, char *end, void *ptr,
> return symbol_string(buf, end, ptr, spec, *fmt);
> case 'R':
> return resource_string(buf, end, ptr, spec);
> - case 'm':
> - spec.flags |= SPECIAL;
> - /* Fallthrough */
> - case 'M':
> - return mac_address_string(buf, end, ptr, spec);
> - case 'i':
> - spec.flags |= SPECIAL;
> - /* Fallthrough */
> - case 'I':
> - if (fmt[1] == '6')
> - return ip6_addr_string(buf, end, ptr, spec);
> - if (fmt[1] == '4')
> - return ip4_addr_string(buf, end, ptr, spec);
> - spec.flags &= ~SPECIAL;
> + case 'm': /* Colon separated: 00:01:02:03:04:05 */
> + case 'M': /* Contiguous: 000102030405 */
> + return mac_address_string(buf, end, ptr, spec, fmt);
> + case 'i': /*
> + * Formatted IP supported
> + * 4: 001.002.003.004
> + * 6: 0001:0203:...:0708
> + * 6c: 1::708
> + * 6c4: 1::1.2.3.4
> + */
> + case 'I': /* Contiguous: */
> + if (fmt[1] == '4' || fmt[1] == '6')
> + ip_addr_string(buf, end, ptr, spec, fmt);
> break;
> }
> spec.flags |= SMALL;
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists