netdev - Re: [PATCH net-next 1/8] net: move netdev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130826173844.GA1437@minipsycho.orion>
Date:	Mon, 26 Aug 2013 19:38:44 +0200
From:	Jiri Pirko <jiri@...nulli.us>
To:	Veaceslav Falico <vfalico@...hat.com>
Cc:	netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Alexander Duyck <alexander.h.duyck@...el.com>,
	Cong Wang <amwang@...hat.com>
Subject: Re: [PATCH net-next 1/8] net: move netdev_upper to netdevice.h

Mon, Aug 26, 2013 at 06:55:35PM CEST, vfalico@...hat.com wrote:
>On Mon, Aug 26, 2013 at 06:41:15PM +0200, Jiri Pirko wrote:
>>Mon, Aug 26, 2013 at 06:28:46PM CEST, vfalico@...hat.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)))

I was imagine something like:

struct list_head *iter;
struct net_device *dev, *upper;

netdev_for_each_upper_dev(dev, upper, iter) {

}

This macro can be easily implented using netdev_upper_get_next_dev()
from dev.c

Not much of added overhead other than netdev_upper_get_next_dev calls
(without any search when using list_head iter).
	
>
>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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html