Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140721190956.7d70902c@vostro>
Date: Mon, 21 Jul 2014 19:09:56 +0300
From: Timo Teras <timo.teras@....fi>
To: Rich Felker <dalias@...c.org>
Cc: musl@...ts.openwall.com
Subject: Re: if_nameindex, etc. [Re: Progress towards 1.1.4]

On Mon, 21 Jul 2014 10:59:35 -0400
Rich Felker <dalias@...c.org> wrote:

> On Mon, Jul 21, 2014 at 04:31:09PM +0300, Timo Teras wrote:
> > If considering my netlink patch, the biggest job is likely auditing
> > the code. But I will be happy to do any changes requested.
> 
> I admit it's been low priority on my list of things to look at, so I
> haven't looked at it in enough detail, but my first impression was
> that it looked larger and more complex than needed (but of course much
> smalled and less ugly than other netlink code I've seen, so maybe
> that's just netlink...).

Much of the code is just #define's copied from kernel. Basically the
whole header file is that. The header can be simplified a bit by
deleting the unused flags, etc.

> Some useful questions to get answers to:
> 
> - How does size(1) vary before/after the patch?

Example numbers from my x86-64 system (git snapshot with few additional
patches applied).

No netlink patch:

text     data    bss     dec    hex  filename
544538   1920  11576  558034  883d2  lib/libc.so

With netlink patch:

text     data    bss     dec    hex  filename
544846   1920  11576  558342  88506  lib/libc.so

So +300 bytes in text, but in return there is added functionality such
as returning the link-level info in PF_PACKET dump (including
statistics). I also expect the code to be a lot faster:

- less syscalls; info from multiple interfaces are returned in single
  recv() call, instead of one syscall per interface

- no text parsing, it is all binary structures that are fast to
  interpret

And for static builds, there is less dependencies: sscanf machinery
is not used.

The only thing I would like to fix in if_nameindex() is the current
realloc() per interface, it should be made exponential or similar.
Possibly fixing the list generation to happen properly right away thus
avoiding the final conversion that is done now.

Also, auditing for cancellation correctness is pending.

> - Would it be easy to make if_nameindex support pulling interface
>   names from 3 sources (ioctl, /proc/net/dev, and netlink), using the
>   index numbers obtained during that process where available (netlink
>   only?), and using ioctl to get index numbers for the rest? I think
>   that would provide maximum compatibility but it may also be a lot
>   more bloat than we want...

It would be doable. But I'm not sure if it feasible, sounds like just
adding bloat.

> - Would netlink code get any smaller/simpler if it were just flattened
>   in the two places it's used rather than having callbacks and general
>   framework?

I don't think so. The code would be pretty much duplicated in both
places.

> I'm not expecting you to have answers to all of these but maybe it's
> something others could look into and/or experiment with too.

- Timo

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.