[PATCH v3] Print ip and port associated with descriptor with -yy

[Zubin Mithra]

Hi Masatake !

Thank you for reviewing the patch!

>
> It is nice if you prepare a test case, too. See code/tests/.
>

Sure, I'll check out how tests are done and add relevant tests.


> To make adding more families a bit easier how do you think use switch/case
> instead of if/else? Like:
>
>         switch (diag_msg->idiag_family) {
>                case AF_INET:
>                     ...
>                case AF_INET6:
>                     ...
>         }
>

Indeed, I'll make this improvement.

> On Thu,  7 Aug 2014 18:17:12 +0530, zubin.mithra at gmail.com wrote:
>> +/* Given an inode number of a socket, print out the details
>> + * of the remote ip address and remote port */
>> +int
>> +printsockdetails(int inodenr)
>> +{
>> +     int sockfd;
>> +     int i, j;
>> +     int protocols[] = {IPPROTO_TCP, IPPROTO_UDP};
>> +     int families[] = {AF_INET, AF_INET6};
>> +
>> +     //Create the monitoring socket
>> +     if((sockfd = socket(AF_NETLINK, SOCK_DGRAM, NETLINK_INET_DIAG)) == -1)
>> +             return -1;
>
> Do we have to create a netlink socket each time printsockdetails is inovked?
> How do you think cache the response from the netlink socket?

This is interesting -- perhaps I could work on adding this in as part
of a second patch? I could also add in the option to resolve ports and
ip addresses along with that patch. That way it would be easier to
review(and improve upon the patch).

Caching sounds useful too, especially in cases where there are lots of
connections. In effect, I'd need a fast lookup from inode number to an
ip address and port(I dont have to keep all ips/ports in memory, just
the ones corresponding to the inode number I've looked up, I guess).

>
> System calls that adding and removing a socket from fd table of process are
> enumerable. So you can cache the response from the netlink socket safely
> if the target program is not multi-threaded.
>
> * adding
>
>   + socket
>   + accept
>   + passing
>
> * removing
>
>   + close
>   + shutdown?
>
>
> Could the technique I used in commit 1d78d22058da04eac7bf726c059d5c3fb193da08
> help you?

I'll check this commit out and reply asap.

>
>> +
>> +     for (i = 0; i < 2; i++) {
>
> "2" is hardcoded here. sizeof(families)/sizeof(families[0]) may be better
> because it makes adding more families a bit easier.

Thanks, I'll fix this.

>
> On Thu,  7 Aug 2014 18:17:12 +0530, zubin.mithra at gmail.com wrote:
>>  void
>>  printfd(struct tcb *tcp, int fd)
>>  {
>>       char path[PATH_MAX + 1];
>>
>> -     if (show_fd_path && getfdpath(tcp, fd, path, sizeof(path)) >= 0)
>> +     if (show_fd_path == 1 && getfdpath(tcp, fd, path, sizeof(path)) >= 0)
>>               tprintf("%d<%s>", fd, path);
>> +     else if (show_fd_path > 1 && getfdpath(tcp, fd, path, sizeof(path)) >= 0) {
>> +             char *ptr = NULL;
>> +             int inodenr;
>> +             ptr = strstr(path, "socket:[");
>> +             if (ptr != path) {
>> +                     tprintf("%d<%s>", fd, path);
>> +             }
>> +             else {
>> +                     int retval;
>> +                     ptr = path + 8;
>> +                     path[strlen(path)-1] = '\0';
>> +                     inodenr = strtol(ptr, NULL, 10);
>> +                     tprintf("%d<", fd);
>> +                     retval = printsockdetails(inodenr);
>> +                     if (retval == -1) tprintf("socket:[%d]",inodenr);
>> +                     tprints(">");
>> +             }
>> +     }
>>       else
>>               tprintf("%d", fd);
>
> I cannot show even pseudo code but if possible, could you consider
> adding code to handle other fd type like pipe, device?

Sounds interesting, sure, I could consider doing this too.

Thanks!
-- zm