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
| ||
|
Date: Fri, 15 Oct 2010 06:48:58 -0400 From: Neil Horman <nhorman@...driver.com> To: Paul Gortmaker <paul.gortmaker@...driver.com> Cc: davem@...emloft.net, netdev@...r.kernel.org, allan.stephens@...driver.com Subject: Re: [PATCH net-next 2/5] tipc: Simplify bearer shutdown logic On Thu, Oct 14, 2010 at 07:58:26PM -0400, Paul Gortmaker wrote: > [Re: [PATCH net-next 2/5] tipc: Simplify bearer shutdown logic] On 13/10/2010 (Wed 10:39) Neil Horman wrote: > > > On Tue, Oct 12, 2010 at 08:25:55PM -0400, Paul Gortmaker wrote: > > > From: Allan Stephens <allan.stephens@...driver.com> > > > > > > Disable all active bearers when TIPC is shut down without having to do > > > a name-based search to locate each bearer object. > > > > > It seems like you're doing a good deal more in this patch than just disabling > > all active bearers without doing a name search. The description is implemented > > in the for loop of tipc_bearer_stop. Whats the rest of it for? > > It seems the original needlessly bloated out the patch size by > swapping the order of tipc_bearer_find_interface & bearer_find > in the file (now fixed) - and you are right, the locking change > wasn't properly covered in the commit log. The extra test you'd > suggested tossing out is also now gone. > > This change doesn't explicitly depend on any other changes, > so if it is now OK, the option is there for it to be applied > independently of the others that haven't been reworked yet. > > Thanks, > Paul. > > > From 1771ad642cb076dbeb71e3533a25cb2f07df9cd8 Mon Sep 17 00:00:00 2001 > From: Allan Stephens <allan.stephens@...driver.com> > Date: Sat, 4 Sep 2010 09:29:04 -0400 > Subject: [PATCH] tipc: Simplify bearer shutdown logic > > Optimize processing in TIPC's bearer shutdown code, including: > > 1. Remove an unnecessary check to see if TIPC bearer's can exist. > 2. Don't release spinlocks before calling a media-specific disabling > routine, since the routine can't sleep. > 3. Make bearer_disable() operate directly on a struct bearer, instead > of needlessly taking a name and then mapping that to the struct. > > Signed-off-by: Allan Stephens <allan.stephens@...driver.com> > Signed-off-by: Paul Gortmaker <paul.gortmaker@...driver.com> > --- > net/tipc/bearer.c | 38 +++++++++++--------------------------- > 1 files changed, 11 insertions(+), 27 deletions(-) > > diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c > index 9c10c6b..fd9c06c 100644 > --- a/net/tipc/bearer.c > +++ b/net/tipc/bearer.c > @@ -288,9 +288,6 @@ static struct bearer *bearer_find(const char *name) > struct bearer *b_ptr; > u32 i; > > - if (tipc_mode != TIPC_NET_MODE) > - return NULL; > - > for (i = 0, b_ptr = tipc_bearers; i < MAX_BEARERS; i++, b_ptr++) { > if (b_ptr->active && (!strcmp(b_ptr->publ.name, name))) > return b_ptr; > @@ -630,30 +627,17 @@ int tipc_block_bearer(const char *name) > * Note: This routine assumes caller holds tipc_net_lock. > */ > > -static int bearer_disable(const char *name) > +static int bearer_disable(struct bearer *b_ptr) > { > - struct bearer *b_ptr; > struct link *l_ptr; > struct link *temp_l_ptr; > > - b_ptr = bearer_find(name); > - if (!b_ptr) { > - warn("Attempt to disable unknown bearer <%s>\n", name); > - return -EINVAL; > - } > - > - info("Disabling bearer <%s>\n", name); > + info("Disabling bearer <%s>\n", b_ptr->publ.name); > tipc_disc_stop_link_req(b_ptr->link_req); > spin_lock_bh(&b_ptr->publ.lock); > b_ptr->link_req = NULL; > b_ptr->publ.blocked = 1; > - if (b_ptr->media->disable_bearer) { > - spin_unlock_bh(&b_ptr->publ.lock); > - write_unlock_bh(&tipc_net_lock); > - b_ptr->media->disable_bearer(&b_ptr->publ); > - write_lock_bh(&tipc_net_lock); > - spin_lock_bh(&b_ptr->publ.lock); > - } > + b_ptr->media->disable_bearer(&b_ptr->publ); > list_for_each_entry_safe(l_ptr, temp_l_ptr, &b_ptr->links, link_list) { > tipc_link_delete(l_ptr); > } > @@ -664,10 +648,16 @@ static int bearer_disable(const char *name) > > int tipc_disable_bearer(const char *name) > { > + struct bearer *b_ptr; > int res; > > write_lock_bh(&tipc_net_lock); > - res = bearer_disable(name); > + b_ptr = bearer_find(name); > + if (b_ptr == NULL) { > + warn("Attempt to disable unknown bearer <%s>\n", name); > + res = -EINVAL; > + } else > + res = bearer_disable(b_ptr); > write_unlock_bh(&tipc_net_lock); > return res; > } > @@ -680,13 +670,7 @@ void tipc_bearer_stop(void) > > for (i = 0; i < MAX_BEARERS; i++) { > if (tipc_bearers[i].active) > - tipc_bearers[i].publ.blocked = 1; > - } > - for (i = 0; i < MAX_BEARERS; i++) { > - if (tipc_bearers[i].active) > - bearer_disable(tipc_bearers[i].publ.name); > + bearer_disable(&tipc_bearers[i]); > } > media_count = 0; > } > - > - > -- > 1.7.2.1 > > Yes, this looks much better, thank you. Reviewed-by: Neil Horman <nhorman@...driver.com> -- 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