On Tue, Mar 7, 2023 at 8:50 AM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com>
wrote:
 Hi Lianbo,
 thank you for the update.
 For a few details..
 On 2023/03/03 16:23, Lianbo Jiang wrote:
 > Currently, the "net" command displays only the IPv4 address of a network
 > interface, it doesn't support outputting IPv6 address yet. For example:
 >
 > Without the patch:
 >    crash> net
 >       NET_DEVICE     NAME   IP ADDRESS(ES)
 >    ffff8d01b1205000  lo     127.0.0.1
 >    ffff8d0087e40000  eno1   192.168.122.2
 >
 > With the patch:
 >    crash> net
 >       NET_DEVICE     NAME       IP ADDRESS(ES)
 >    ffff8d01b1205000  lo         127.0.0.1, ::1
 >    ffff8d0087e40000  eno1       192.168.122.2,
 xxxx:xx:x:xxxx:xxxx:xxx:xxxx:xxxx, yyyy::yyyy:yyy:yyyy:yyyy
 >
 > Related kernel commit:
 > 502a2ffd7376 ("ipv6: convert idev_list to list macros")
 >
 > Reported-by: Buland Kumar Singh <bsingh(a)redhat.com>
 > Signed-off-by: Lianbo Jiang <lijiang(a)redhat.com>
 > ---
 >   defs.h    |   6 +++
 >   net.c     | 109 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 >   symbols.c |   6 +++
 >   3 files changed, 112 insertions(+), 9 deletions(-)
 >
 > diff --git a/defs.h b/defs.h
 > index e76af3c78b69..1f2cf6e0ce01 100644
 > --- a/defs.h
 > +++ b/defs.h
 > @@ -2208,6 +2208,12 @@ struct offset_table {                    /* stash
 of commonly-used offsets */
 >       long sock_common_skc_v6_daddr;
 >       long sock_common_skc_v6_rcv_saddr;
 >       long inactive_task_frame_bp;
 > +     long net_device_ip6_ptr;
 > +     long inet6_dev_addr_list;
 > +     long inet6_ifaddr_addr;
 > +     long inet6_ifaddr_if_list;
 > +     long inet6_ifaddr_if_next;
 > +     long in6_addr_in6_u;
 >   };
 >
 >   struct size_table {         /* stash of commonly-used sizes */
 > diff --git a/net.c b/net.c
 > index aa445ab7ee13..6bca2a5d0098 100644
 > --- a/net.c
 > +++ b/net.c
 > @@ -71,6 +71,7 @@ static void print_neighbour_q(ulong, int);
 >   static void get_netdev_info(ulong, struct devinfo *);
 >   static void get_device_name(ulong, char *);
 >   static long get_device_address(ulong, char **, long);
 > +static void get_device_ip6_address(ulong, char **, long);
 >   static void get_sock_info(ulong, char *);
 >   static void dump_arp(void);
 >   static void arp_state_to_flags(unsigned char);
 > @@ -114,6 +115,13 @@ net_init(void)
 >               net->dev_ip_ptr = MEMBER_OFFSET_INIT(net_device_ip_ptr,
 >                       "net_device", "ip_ptr");
 >               MEMBER_OFFSET_INIT(net_device_dev_list, "net_device",
 "dev_list");
 > +             MEMBER_OFFSET_INIT(net_device_ip6_ptr, "net_device",
 "ip6_ptr");
 > +             MEMBER_OFFSET_INIT(inet6_dev_addr_list, "inet6_dev",
 "addr_list");
 > +             MEMBER_OFFSET_INIT(inet6_ifaddr_addr, "inet6_ifaddr",
 "addr");
 > +             MEMBER_OFFSET_INIT(inet6_ifaddr_if_list, "inet6_ifaddr",
 "if_list");
 > +             MEMBER_OFFSET_INIT(inet6_ifaddr_if_next, "inet6_ifaddr",
 "if_next");
 > +             MEMBER_OFFSET_INIT(in6_addr_in6_u, "in6_addr",
"in6_u");
 > +
 >               MEMBER_OFFSET_INIT(net_dev_base_head, "net",
 "dev_base_head");
 >               ARRAY_LENGTH_INIT(net->net_device_name_index,
 >                       net_device_name, "net_device.name", NULL,
 sizeof(char));
 > @@ -466,7 +474,7 @@ show_net_devices(ulong task)
 >       buf = GETBUF(buflen);
 >       flen = MAX(VADDR_PRLEN, strlen(net->netdevice));
 >
 > -     fprintf(fp, "%s  NAME   IP ADDRESS(ES)\n",
 > +     fprintf(fp, "%s  NAME       IP ADDRESS(ES)\n",
 >               mkstring(upper_case(net->netdevice, buf),
 >                       flen, CENTER|LJUST, NULL));
 >
 > @@ -475,9 +483,10 @@ show_net_devices(ulong task)
 >                       mkstring(buf, flen, CENTER|RJUST|LONG_HEX,
 MKSTR(next)));
 >
 >               get_device_name(next, buf);
 > -             fprintf(fp, "%-6s ", buf);
 > +             fprintf(fp, "%-10s ", buf);
 >
 > -             buflen = get_device_address(next, &buf, buflen);
 > +             get_device_address(next, &buf, buflen);
 > +             get_device_ip6_address(next, &buf, buflen);
 >               fprintf(fp, "%s\n", buf);
 >
 >               readmem(next+net->dev_next, KVADDR, &next,
 > @@ -503,7 +512,7 @@ show_net_devices_v2(ulong task)
 >       buf = GETBUF(buflen);
 >       flen = MAX(VADDR_PRLEN, strlen(net->netdevice));
 >
 > -     fprintf(fp, "%s  NAME   IP ADDRESS(ES)\n",
 > +     fprintf(fp, "%s  NAME       IP ADDRESS(ES)\n",
 >               mkstring(upper_case(net->netdevice, buf),
 >                       flen, CENTER|LJUST, NULL));
 >
 > @@ -528,9 +537,10 @@ show_net_devices_v2(ulong task)
 >                       MKSTR(ld->list_ptr[i])));
 >
 >               get_device_name(ld->list_ptr[i], buf);
 > -             fprintf(fp, "%-6s ", buf);
 > +             fprintf(fp, "%-10s ", buf);
 >
 > -             buflen = get_device_address(ld->list_ptr[i], &buf, buflen);
 > +             get_device_address(ld->list_ptr[i], &buf, buflen);
 > +             get_device_ip6_address(ld->list_ptr[i], &buf, buflen);
 >               fprintf(fp, "%s\n", buf);
 >       }
 >
 > @@ -556,7 +566,7 @@ show_net_devices_v3(ulong task)
 >       buf = GETBUF(buflen);
 >       flen = MAX(VADDR_PRLEN, strlen(net->netdevice));
 >
 > -     fprintf(fp, "%s  NAME   IP ADDRESS(ES)\n",
 > +     fprintf(fp, "%s  NAME       IP ADDRESS(ES)\n",
 >               mkstring(upper_case(net->netdevice, buf),
 >                       flen, CENTER|LJUST, NULL));
 >
 > @@ -591,9 +601,10 @@ show_net_devices_v3(ulong task)
 >                       MKSTR(ld->list_ptr[i])));
 >
 >               get_device_name(ld->list_ptr[i], buf);
 > -             fprintf(fp, "%-6s ", buf);
 > +             fprintf(fp, "%-10s ", buf);
 >
 > -             buflen = get_device_address(ld->list_ptr[i], &buf, buflen);
 > +             get_device_address(ld->list_ptr[i], &buf, buflen);
 > +             get_device_ip6_address(ld->list_ptr[i], &buf, buflen);
 >               fprintf(fp, "%s\n", buf);
 >       }
 >
 > @@ -925,6 +936,86 @@ get_device_address(ulong devaddr, char **bufp, long
 buflen)
 >       return buflen;
 >   }
 >
 > +static void
 > +get_device_ip6_address(ulong devaddr, char **bufp, long buflen)
 > +{
 > +     ulong ip6_ptr = 0, pos = 0, bufsize = buflen, addr = 0;
 > +     struct in6_addr ip6_addr;
 > +     char *buf;
 > +     char str[INET6_ADDRSTRLEN + 1] = {0};
 > +     char buffer[INET6_ADDRSTRLEN + 4] = {0};
 What are the +1 and +4 for?
 
I noticed that the size of INET6_ADDRSTRLEN is 48 in kernel code as below:
--- a/include/linux/inet.h
+++ b/include/linux/inet.h
+/*
+ * These mimic similar macros defined in user-space for inet_ntop(3).
+ * See /usr/include/netinet/in.h .
+ */
+#define INET_ADDRSTRLEN                (16)
+#define INET6_ADDRSTRLEN       (48)
And, the size of INET6_ADDRSTRLEN is 46 in my machine as below:
# cat /usr/include/netinet/in.h |grep INET6_ADDRSTRLEN
#define INET6_ADDRSTRLEN 46
The rfc2460 said that the IPv6 increases the IP address size from 32 bits
to 128 bits.
crash> struct in6_addr
struct in6_addr {
    union {
        __u8 u6_addr8[16];
        __be16 u6_addr16[8];
        __be32 u6_addr32[4];
    } in6_u;
}
SIZE: 16
Given that, maybe they should be defined like this?
+     char str[INET6_ADDRSTRLEN + 2] = {0};
+     char buffer[INET6_ADDRSTRLEN + 2 + 2] = {0};
 Not sure what's the best way for this case.
Looking at the example in the man page of inet_pton(3), INET6_ADDRSTRLEN
 seems enough for the str and contains a null char.  The buffer can
have
 a comma and a space (", ") so +2 is enough?  i.e.
    char str[INET6_ADDRSTRLEN] = {0};
    char buffer[INET6_ADDRSTRLEN + 2] = {0};
 > +     uint len = 0;
 > +
 > +     buf = *bufp;
 > +     pos = strlen(buf);
 Ah nicely done :)
 > +
 > +     readmem(devaddr + OFFSET(net_device_ip6_ptr), KVADDR,
 > +             &ip6_ptr, sizeof(ulong), "ip6_ptr", FAULT_ON_ERROR);
 > +
 > +     if (!ip6_ptr)
 > +             return;
 > +
 > +     if (VALID_MEMBER(inet6_ifaddr_if_list)) {
 > +             struct list_data list_data, *ld;
 > +             ulong cnt = 0, i;
 > +
 > +             ld = &list_data;
 > +             BZERO(ld, sizeof(struct list_data));
 > +             ld->flags |= LIST_ALLOCATE;
 > +             ld->start = ip6_ptr + OFFSET(inet6_dev_addr_list);
 > +             cnt = do_list(ld);
 > +
 > +             for (i = 1; i < cnt; i++) {
 > +
 > +                     addr = ld->list_ptr[i] + OFFSET(inet6_ifaddr_addr);
 > +                     addr -= OFFSET(inet6_ifaddr_if_list);
 
The above code is easy to understand, because it actually imitates the
container_of().
But if you would like to have the same style as show_net_devices_v2()
and show_net_devices_v3(), that's also fine to me.
This line can be removed with ld->list_head_offset parameter?
 i.e.
      ld->start = ip6_ptr + OFFSET(inet6_dev_addr_list);
 +   ld->list_head_offset = OFFSET(inet6_ifaddr_if_list);
      cnt = do_list(ld);
 > +                     readmem(addr + OFFSET(in6_addr_in6_u), KVADDR,
 &ip6_addr,
 > +                             sizeof(struct in6_addr), "in6_addr.in6_u",
 FAULT_ON_ERROR);
 > +
 > +                     inet_ntop(AF_INET6, (void*)&ip6_addr, str,
 INET6_ADDRSTRLEN);
 > +                     sprintf(buffer, "%s%s", pos ? ", " :
"", str);
 > +                     len = strlen(buffer);
 > +                     if (pos + len >= bufsize) {
 > +                             RESIZEBUF(*bufp, bufsize, bufsize +
 buflen);
 > +                             buf = *bufp;
 > +                             BZERO(buf + bufsize, buflen);
 > +                             bufsize += buflen;
 > +                     }
 > +                     BCOPY(buffer, &buf[pos], len);
 > +                     pos += len;
 > +             }
 > +
 > +             FREEBUF(ld->list_ptr);
 > +             return;
 > +     }
 > +
 > +     /*
 > +      * 502a2ffd7376 ("ipv6: convert idev_list to list macros")
 > +      * v2.6.35-rc1~473^2~733
 > +      */
 This should be moved before the if block above?
 
Yes, you are right. I forgot to move it there.
Thanks.
Lianbo
 > +     readmem(ip6_ptr + OFFSET(inet6_dev_addr_list), KVADDR,
 > +             &addr, sizeof(void *), "inet6_dev.addr_list",
 FAULT_ON_ERROR);
 > +
 > +     while (addr) {
 > +             readmem(addr + OFFSET(in6_addr_in6_u), KVADDR, &ip6_addr,
 > +                     sizeof(struct in6_addr), "in6_addr.in6_u",
 FAULT_ON_ERROR);
 > +             inet_ntop(AF_INET6, (void*)&ip6_addr, str,
 INET6_ADDRSTRLEN);
 > +             sprintf(buffer, "%s%s", pos ? ", " : "",
str);
 > +             len = strlen(buffer);
 > +
 > +             if (pos + len >= bufsize) {
 > +                     RESIZEBUF(*bufp, bufsize, bufsize + buflen);
 > +                     buf = *bufp;
 > +                     BZERO(buf + bufsize, buflen);
 > +                     bufsize += buflen;
 > +             }
 > +             BCOPY(buffer, &buf[pos], len);
 > +             pos += len;
 > +             readmem(addr + OFFSET(inet6_ifaddr_if_next), KVADDR, &addr,
 > +                     sizeof(void *), "inet6_ifaddr.if_next",
 FAULT_ON_ERROR);
 > +     }
 > +}
 > +
 >   /*
 >    *  Get the family, type, local and destination address/port pairs.
 >    */
 > diff --git a/symbols.c b/symbols.c
 > index a974fc9141a0..3efcdddfcadb 100644
 > --- a/symbols.c
 > +++ b/symbols.c
 > @@ -9787,6 +9787,7 @@ dump_offset_table(char *spec, ulong makestruct)
 >               OFFSET(net_device_addr_len));
 >       fprintf(fp, "             net_device_ip_ptr: %ld\n",
 >               OFFSET(net_device_ip_ptr));
 > +     fprintf(fp, "            net_device_ip6_ptr: %ld\n",
 OFFSET(net_device_ip6_ptr));
 >       fprintf(fp, "           net_device_dev_list: %ld\n",
 >               OFFSET(net_device_dev_list));
 >       fprintf(fp, "             net_dev_base_head: %ld\n",
 > @@ -9839,6 +9840,11 @@ dump_offset_table(char *spec, ulong makestruct)
 >           fprintf(fp, "                  inet_opt_num: %ld\n",
 >               OFFSET(inet_opt_num));
 >
 > +        fprintf(fp, "           inet6_dev_addr_list: %ld\n",
 OFFSET(inet6_dev_addr_list));
 > +        fprintf(fp, "             inet6_ifaddr_addr: %ld\n",
 OFFSET(inet6_ifaddr_addr));
 > +        fprintf(fp, "          inet6_ifaddr_if_list: %ld\n",
 OFFSET(inet6_ifaddr_if_list));
 > +        fprintf(fp, "          inet6_ifaddr_if_next: %ld\n",
 OFFSET(inet6_ifaddr_if_next));
 > +        fprintf(fp, "                in6_addr_in6_u: %ld\n",
 OFFSET(in6_addr_in6_u));
 >           fprintf(fp, "          ipv6_pinfo_rcv_saddr: %ld\n",
 >               OFFSET(ipv6_pinfo_rcv_saddr));
 >           fprintf(fp, "              ipv6_pinfo_daddr: %ld\n",
 Thanks, good!
 Kazu