netdev - Re: [PATCH 3/4] [Net] Support Xen accelerated network plugin modules

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: <1179818108.4145.18.camel@moonstone.uk.level5networks.com>
Date:	Tue, 22 May 2007 08:15:08 +0100
From:	Kieran Mansley <kmansley@...arflare.com>
To:	Stephen Hemminger <shemminger@...ux-foundation.org>
Cc:	xen-devel@...ts.xensource.com, netdev@...r.kernel.org,
	muli@...ibm.com, herbert@...dor.apana.org.au
Subject: Re: [PATCH 3/4] [Net] Support Xen accelerated network plugin
	modules

On Mon, 2007-05-21 at 10:52 -0700, Stephen Hemminger wrote:
> On Fri, 18 May 2007 14:16:48 +0100
> Kieran Mansley <kmansley@...arflare.com> wrote:
> 
> > Add support to Xen netfront for accelerated plugin module
> > 
> >
> > 
> > +/*
> > + * List of all netfront accelerator plugin modules available.  Each
> > + * list entry is of type struct netfront_accelerator.
> > + */ 
> > +static struct list_head accelerators_list;
> > +/*
> > + * Lock to protect access to accelerators_list, and also used to
> > + * protect the hooks_usecount field in struct netfront_accelerator
> > + * against concurrent access 
> > + */
> > +static spinlock_t accelerators_lock;
> > +
> 
> 
> Your locking model is more complex than it needs to be.
> If you just used RCU for access, and depended on the existing RT netlink
> mutex (a.k.a big network lock), for setup; then you wouldn't need to
> do any of your own locking.

The complexity arises from the requirement to not take additional locks
on the data path, and not hold a lock during the calls from netfront
into the accelerated plugin.  The lock and ref count aren't really
providing mutual exclusion, but ensuring the function pointers persist
and are consistent while they're in use.  RCU on its own wouldn't
prevent the accelerated plugin being unloaded while netfront was using
one of the hooks.  Wrapping each one of these calls with the big network
lock would be OK, but then we'd be holding locks while calling into the
hooks.

I could remove the lock and ref count as you suggest if I increased the
plugin module's use count to prevent it being unloaded, but then we'd
need some mechanism to signal that a network interface should no longer
be accelerated so that the hooks can be removed, module use count
decreased, and module safely unloaded.

This is something I'm happy to change if necessary, but I thought I
should explain why it's complex before going ahead.  If you still think
it needs changing, then let me know.

Kieran

-
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