On Wed, Feb 15, 2023 at 9:47 AM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@nec.com> wrote:
On 2023/02/14 18:20, lijiang wrote:
>>> On Tue, Feb 14, 2023 at 3:46 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@nec.com <mailto:k-hagio-ab@nec.com>> wrote:
>>>
>>>     On 2023/02/09 21:15, Lianbo Jiang wrote:
>>>     > Currently, the "net -s" command fails to show an IPv6 address. For
>>>     > example:
>>>     >
>>>     >    crash> net -s
>>>     >    PID: 305524   TASK: ffff9bc449895580  CPU: 6    COMMAND: "sshd"
>>>     >    FD      SOCKET            SOCK       FAMILY:TYPE SOURCE-PORT DESTINATION-PORT
>>>     >     3 ffff9bc446e9a680 ffff9bc4455b5940 UNIX:DGRAM
>>>     >     4 ffff9bc446e9c600 ffff9bc3b2b24e00 INET6:STREAM
>>>     >
>>>     > With the patch:
>>>     >    crash> net -s
>>>     >    PID: 305524   TASK: ffff9bc449895580  CPU: 6    COMMAND: "sshd"
>>>     >    FD      SOCKET            SOCK       FAMILY:TYPE SOURCE-PORT DESTINATION-PORT
>>>     >     3 ffff9bc446e9a680 ffff9bc4455b5940 UNIX:DGRAM
>>>     >     4 ffff9bc446e9c600 ffff9bc3b2b24e00 INET6:STREAM xxxx:xx:x:xxxx:xxxx:xxxx:xxxx:xxxx-22 yyyy:yy:y:yyyy:yyyy:yyyy:yyyy:yyyy-44870
>>>     >
>>>     > Let's support displaying the IPv6 address and port in the SOURCE-PORT
>>>     > and DESTINATION-PORT colums.
>>>     >
>>>     > Reported-by: Buland Kumar Singh <bsingh@redhat.com <mailto:bsingh@redhat.com>>
>>>     > Signed-off-by: Lianbo Jiang <lijiang@redhat.com <mailto:lijiang@redhat.com>>
>>>
>>>     Good catch, I didn't know "net -s" could print IPv6 addresses...
>>>
>>>     Is this the corresponding kernel patch?
>>>
>>> Yes. Actually there are two related kernel commits:
>>> [1] 1da177e4c3f4 ("Linux-2.6.12-rc2")

This looks the initial patch in the repository?


Yes.
 
>>> [2] efe4208f47f9 ("ipv6: make lookups simpler and faster") v3.13-rc1
>>>
>>> Can you help to add this to the patch log? I forgot to add them.

Yes, added this.

 
Ok, Thanks.
 
>>>
>>>     commit efe4208f47f907b86f528788da711e8ab9dea44d
>>>     Author: Eric Dumazet <edumazet@google.com <mailto:edumazet@google.com>>
>>>     Date:   Thu Oct 3 15:42:29 2013 -0700
>>>
>>>          ipv6: make lookups simpler and faster
>>>
>>>     > ---
>>>     >   defs.h    |  3 +++
>>>     >   net.c     | 40 ++++++++++++++++++++++++++++------------
>>>     >   symbols.c |  6 ++++++
>>>     >   3 files changed, 37 insertions(+), 12 deletions(-)
>>>     >
>>>     > diff --git a/defs.h b/defs.h
>>>     > index 33a823b7b67c..db20056dc3ed 100644
>>>     > --- a/defs.h
>>>     > +++ b/defs.h
>>>     > @@ -2204,6 +2204,9 @@ struct offset_table {                    /* stash of commonly-used offsets */
>>>     >       long maple_range_64_slot;
>>>     >       long maple_metadata_end;
>>>     >       long maple_metadata_gap;
>>>     > +     long sock_sk_common;
>>>     > +     long sock_common_skc_v6_daddr;
>>>     > +     long sock_common_skc_v6_rcv_saddr;
>>>     >   };
>>>     >
>>>     >   struct size_table {         /* stash of commonly-used sizes */
>>>     > diff --git a/net.c b/net.c
>>>     > index 7c9c8bd9c98d..7af299d0774d 100644
>>>     > --- a/net.c
>>>     > +++ b/net.c
>>>     > @@ -198,6 +198,9 @@ net_init(void)
>>>     >                        */
>>>     >                       MEMBER_OFFSET_INIT(sock_common_skc_family,
>>>     >                               "sock_common", "skc_family");
>>>     > +                     MEMBER_OFFSET_INIT(sock_sk_common, "sock", "__sk_common");
>>>     > +                     MEMBER_OFFSET_INIT(sock_common_skc_v6_daddr, "sock_common", "skc_v6_daddr");
>>>     > +                     MEMBER_OFFSET_INIT(sock_common_skc_v6_rcv_saddr, "sock_common", "skc_v6_rcv_saddr");
>>>     >                       MEMBER_OFFSET_INIT(sock_sk_type, "sock", "sk_type");
>>>     >                       /*
>>>     >                        *  struct inet_sock {
>>>     > @@ -1104,19 +1107,32 @@ get_sock_info(ulong sock, char *buf)
>>>     >               break;
>>>     >
>>>     >       case SOCK_V2:
>>>     > -             if (INVALID_MEMBER(ipv6_pinfo_rcv_saddr) ||
>>>     > -                 INVALID_MEMBER(ipv6_pinfo_daddr))
>>>     > -                     break;
>>>     > -
>>>     > -             ipv6_rcv_saddr = ipv6_pinfo + OFFSET(ipv6_pinfo_rcv_saddr);
>>>     > -             ipv6_daddr = ipv6_pinfo + OFFSET(ipv6_pinfo_daddr);
>>>     > -
>>>     > -             if (!readmem(ipv6_rcv_saddr, KVADDR, u6_addr16_src, SIZE(in6_addr),
>>>     > -                    "ipv6_rcv_saddr buffer", QUIET|RETURN_ON_ERROR))
>>>     > -                     break;
>>>     > -                if (!readmem(ipv6_daddr, KVADDR, u6_addr16_dest, SIZE(in6_addr),
>>>     > -                    "ipv6_daddr buffer", QUIET|RETURN_ON_ERROR))
>>>     > +             if (VALID_MEMBER(ipv6_pinfo_rcv_saddr) &&
>>>     > +                     VALID_MEMBER(ipv6_pinfo_daddr)) {
>>>     > +                     ipv6_rcv_saddr = ipv6_pinfo + OFFSET(ipv6_pinfo_rcv_saddr);
>>>     > +                     ipv6_daddr = ipv6_pinfo + OFFSET(ipv6_pinfo_daddr);
>>>     > +
>>>     > +                     if (!readmem(ipv6_rcv_saddr, KVADDR, u6_addr16_src, SIZE(in6_addr),
>>>     > +                             "ipv6_rcv_saddr buffer", QUIET|RETURN_ON_ERROR))
>>>     > +                             break;
>>>     > +                     if (!readmem(ipv6_daddr, KVADDR, u6_addr16_dest, SIZE(in6_addr),
>>>     > +                             "ipv6_daddr buffer", QUIET|RETURN_ON_ERROR))
>>>     > +                             break;

I moved these two readmem()s and the following two readmem()s
out of the if block, because they are the same.


Looks good.
 
>>>     > +             } else if (VALID_MEMBER(sock_sk_common) && VALID_MEMBER(sock_common_skc_v6_daddr) &&
>>>     > +                             VALID_MEMBER(sock_common_skc_v6_rcv_saddr)) {
>>>     > +                     ipv6_rcv_saddr = sock + OFFSET(sock_sk_common) + OFFSET(sock_common_skc_v6_rcv_saddr);
>>>     > +                     ipv6_daddr = sock + OFFSET(sock_sk_common) + OFFSET(sock_common_skc_v6_daddr);
>>>     > +
>>>     > +                     if (!readmem(ipv6_rcv_saddr, KVADDR, u6_addr16_src, SIZE(in6_addr),
>>>     > +                             "ipv6_rcv_saddr buffer", QUIET|RETURN_ON_ERROR))
>>>     > +                             break;
>>>     > +                     if (!readmem(ipv6_daddr, KVADDR, u6_addr16_dest, SIZE(in6_addr),
>>>     > +                             "ipv6_daddr buffer", QUIET|RETURN_ON_ERROR))
>>>     > +                             break;
>>>     > +             } else {
>>>     > +                     error(INFO, "Invalid member...\n");
>>>
>>>     This message is unusual and vague, and the output looks not good:
>>>     (I forced to print this.)
>>>
>>>     crash> net -s 1
>>>     PID: 1        TASK: ffff9c6b8159c8c0  CPU: 11   COMMAND: "systemd"
>>>     FD      SOCKET            SOCK       FAMILY:TYPE SOURCE-PORT DESTINATION-PORT
>>>     ...
>>>     41 ffff9c79974b70c0 ffff9c79afcfb700 INET:DGRAM   0.0.0.0-111 0.0.0.0-0
>>>     42 ffff9c79974b1e40 ffff9c7a6f4347c0 net: Invalid member...
>>>     INET6:STREAM
>>>     43 ffff9c79974b1340 ffff9c7a6f43e780 net: Invalid member...
>>>     INET6:DGRAM
>>>     44 ffff9c79974b3180 ffff9c767da66780 UNIX:SEQPACKET
>>>     ...
>>>
>>>     How about this?
>>>
>>>
>>> Looks good.
>>>
>>>                      } else {
>>>     -                       error(INFO, "Invalid member...\n");
>>>     +                       sprintf(&buf[strlen(buf)], "%s", "(cannot get IPv6 addresses)");
>>>                              break;
>>>
>>>     41 ffff9c79974b70c0 ffff9c79afcfb700 INET:DGRAM   0.0.0.0-111 0.0.0.0-0
>>>     42 ffff9c79974b1e40 ffff9c7a6f4347c0 INET6:STREAM (cannot get IPv6 addresses)
>>>     43 ffff9c79974b1340 ffff9c7a6f43e780 INET6:DGRAM  (cannot get IPv6 addresses)
>>>     44 ffff9c79974b3180 ffff9c767da66780 UNIX:SEQPACKET
>>>
>>>     Otherwise, looks good, although I would like to tweak some indents
>>>     a bit.
>>>
>>>
>>> The Ipv6 address is a very long string, I tried adjusting the indent, but it still doesn't look good.

sorry, I meant "code indents", changed some.

No problem.
 
Thanks.
Lianbo

>>> Thanks.
>>> Lianbo
>>>
>>>
>>>     Thanks,
>>>     Kazu
>>>
>>>     >                       break;
>>>     > +             }
>>>     >
>>>     >               sprintf(&buf[strlen(buf)], "%*s ", BITS32() ? 22 : 12,
>>>     >                       dump_in6_addr_port(u6_addr16_src, sport, buf2, &len));
>>>     > diff --git a/symbols.c b/symbols.c
>>>     > index e38df8aad0f5..d622de722ebd 100644
>>>     > --- a/symbols.c
>>>     > +++ b/symbols.c
>>>     > @@ -9820,6 +9820,12 @@ dump_offset_table(char *spec, ulong makestruct)
>>>     >               OFFSET(sock_sk_type));
>>>     >           fprintf(fp, "        sock_common_skc_family: %ld\n",
>>>     >               OFFSET(sock_common_skc_family));
>>>     > +     fprintf(fp, "                sock_sk_common: %ld\n",
>>>     > +             OFFSET(sock_sk_common));
>>>     > +     fprintf(fp, "      sock_common_skc_v6_daddr: %ld\n",
>>>     > +             OFFSET(sock_common_skc_v6_daddr));
>>>     > +     fprintf(fp, "  sock_common_skc_v6_rcv_saddr: %ld\n",
>>>     > +             OFFSET(sock_common_skc_v6_rcv_saddr));

Changed the order, because it's better to gather the same struct
members together. 
Applied with the above changes.
https://github.com/crash-utility/crash/commit/c64a827e0bcab15e86f8fbacec141c2bf4b776ea

Thanks!
Kazu