On Tue, Mar 7, 2023 at 3:14 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@nec.com> wrote:
On 2023/03/07 13:46, lijiang wrote:

>>     > +     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

I'm not sure why the kernel one is 48 (for a multiple of 8?), but
the INET6_ADDRSTRLEN in glibc header should mean the longest result
of inet_ntop() in glibc, and the latest glibc also has the same 46.

So I don't think we need to consider the kernel one.

>>
>>
>> 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.

Yes, please.  I've not seen the above style (do_list and minus OFFSET)
in crash before.  Although we may go with the above, I think generally
a new style tends to cause an unexpected result.  so I'd like to use
the common style if possible.


Thank you for the comment, Kazu.
I will post it later with the above suggestions.

Thanks.
Lianbo

 
Thanks,
Kazu