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

Powered by blists - more mailing lists