Re: [PATCH net-next 1/8] net: move netdev_upper to netdevice.h

[Veaceslav Falico <vfalico@redhat.com>]

On Mon, Aug 26, 2013 at 06:41:15PM +0200, Jiri Pirko wrote:
>Mon, Aug 26, 2013 at 06:28:46PM CEST, vfalico@redhat.com wrote:
...snip...
>>+struct netdev_upper {
>>+	struct net_device *dev;
>>+	bool master;
>>+	struct list_head list;
>>+	struct rcu_head rcu;
>>+	struct list_head search_list;
>>+};
>>+
>
>
>I like your patchset.  However I'm not entirely comfortable with exposing
>this struct. I would love to have it "under control" in net/core/dev.c

I've taken this approach first, however the change to non-bonding stuff
became a bit too big to justify the (only) bonding use.

bonding only reads from it, and there are already primitives in dev.c to
modify it, so if they will be used for it it's still the dev.c who controls
it (if someone writes directly to it - it's a bug, and can be NAKed).

>
>I'm thinking of some getter/iterator for this use. It can work by
>type as well so you would be able to remove the checks from bonding
>code.

There are 3 checks in bonding - looking for vlan devs, for a specific dev
and for a specific ip address. list_for_each_entry() fits here perfectly
for each case, otherwise the best way to do this would be to

while ((next_dev = netdev_upper_get_next_dev(dev, next_dev)))

or something like that, which adds quite a bit of overhead (looking for the
previous dev and then returning the next one on each iteration), and looks
ugly.

So, given that it's a plain list actually, and any modification to this
list can (and should be) done via functions from dev.c, while reading can
be done with standard list_for_each_entry(_rcu)(), I think it's better to
expose it this way.