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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20130826165535.GG1992@redhat.com>
Date:	Mon, 26 Aug 2013 18:55:35 +0200
From:	Veaceslav Falico <vfalico@...hat.com>
To:	Jiri Pirko <jiri@...nulli.us>
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

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

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