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
| ||
|
Message-ID: <A2BAEFC30C8FD34388F02C9B3121859D1C2BED68@eusaamb103.ericsson.se> Date: Mon, 25 Aug 2014 21:40:04 +0000 From: Jon Maloy <jon.maloy@...csson.com> To: Jon Maloy <jon.maloy@...csson.com>, Erik Hugne <erik.hugne@...csson.com>, "ying.xue@...driver.com" <ying.xue@...driver.com>, Richard Alpe <richard.alpe@...csson.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org> CC: "tipc-discussion@...ts.sourceforge.net" <tipc-discussion@...ts.sourceforge.net> Subject: RE: [tipc-discussion] [PATCH net-next 2/2] tipc: add name distributor resiliency queue > -----Original Message----- > From: Jon Maloy [mailto:jon.maloy@...csson.com] > Sent: August-25-14 5:18 PM > To: Erik Hugne; ying.xue@...driver.com; Richard Alpe; > netdev@...r.kernel.org > Cc: tipc-discussion@...ts.sourceforge.net > Subject: Re: [tipc-discussion] [PATCH net-next 2/2] tipc: add name distributor > resiliency queue > > > > > -----Original Message----- > > From: Erik Hugne > > Sent: August-25-14 10:54 AM > > To: Jon Maloy; ying.xue@...driver.com; Richard Alpe; > > netdev@...r.kernel.org > > Cc: tipc-discussion@...ts.sourceforge.net; Erik Hugne > > Subject: [PATCH net-next 2/2] tipc: add name distributor resiliency > > queue > > > > From: Erik Hugne <erik.hugne@...csson.com> > > > > TIPC name table updates are distributed asynchronously in a cluster, > > entailing a risk of certain race conditions. E.g., if two nodes > > simultaneously issue conflicting (overlapping) publications, this may > > not be detected until both publications have reached a third node, in > > which case one of the publications will be silently dropped on that > > node. Hence, we end up with an inconsistent name table. > > > > In most cases this conflict is just a temporary race, e.g., one node > > is issuing a publication under the assumption that a previous, > > conflicting, publication has already been withdrawn by the other node. > > However, because of the (rtt related) distributed update delay, this > > may not yet hold true on all nodes. The symptom of this failure is a syslog > message: > > "tipc: Cannot publish {%u,%u,%u}, overlap error". > > > > In this commit we add a resiliency queue at the receiving end of the > > name table distributor. When insertion of an arriving publication > > fails, we retain it in this queue for a short amount of time, assuming > > that another update will arrive very soon and clear the conflict. If > > so happens, we insert the publication, otherwise we drop it. > > > > The (configurable) retention value defaults to 2000 ms. Knowing from > > experience that the situation described above is extremely rare, there > > is no risk that the queue will accumulate any large number of items. > > > > Signed-off-by: Erik Hugne <erik.hugne@...csson.com> > > Signed-off-by: Jon Maloy <jon.maloy@...csson.com> > > --- > > Documentation/sysctl/net.txt | 14 +++++++++ > > net/tipc/core.h | 1 + > > net/tipc/name_distr.c | 69 > > ++++++++++++++++++++++++++++++++++++++++++-- > > net/tipc/name_distr.h | 1 + > > net/tipc/name_table.c | 8 ++--- > > net/tipc/sysctl.c | 7 +++++ > > 6 files changed, 93 insertions(+), 7 deletions(-) > > > > diff --git a/Documentation/sysctl/net.txt > > b/Documentation/sysctl/net.txt index 9a0319a..89ce54e 100644 > > --- a/Documentation/sysctl/net.txt > > +++ b/Documentation/sysctl/net.txt > > @@ -241,6 +241,9 @@ address of the router (or Connected) for internal > > networks. > > 6. TIPC > > ------------------------------------------------------- > > > > +tipc_rmem > > +---------- > > + > > The TIPC protocol now has a tunable for the receive memory, similar > > to the tcp_rmem - i.e. a vector of 3 INTEGERs: (min, default, max) > > > > @@ -252,3 +255,14 @@ The max value is set to CONN_OVERLOAD_LIMIT, > and > > the default and min values are scaled (shifted) versions of that same > > value. Note that the min value is not at this point in time used in > > any meaningful way, but the triplet is preserved in order to be > > consistent with things like tcp_rmem. > > + > > +named_timeout > > +-------------- > > + > > +TIPC name table updates are distributed asynchronous > > asynchronously > > >+ in a cluster, > without any form of transaction handling. > > This means that different race scenarios are possible. One such is that a > name withdrawal sent out by one node and received by a another node may > arrive after a second, overlapping name publication already has been > accepted from a third node, although the conflicting updates originally may > have been issued in the correct sequential order. > > > +If named_timeout is nonzero, failed topology updates will be placed > > +on a defer queue until another event arrives that clears the error, > > +or until the timeout expires. Value is in milliseconds. > > diff --git a/net/tipc/core.h b/net/tipc/core.h index d2607a8..f773b14 > > 100644 > > --- a/net/tipc/core.h > > +++ b/net/tipc/core.h > > @@ -81,6 +81,7 @@ extern u32 tipc_own_addr __read_mostly; extern int > > tipc_max_ports __read_mostly; extern int tipc_net_id __read_mostly; > > extern int sysctl_tipc_rmem[3] __read_mostly; > > +extern int sysctl_tipc_named_timeout __read_mostly; > > > > /* > > * Other global variables > > diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c index > > 0591f33..0cbe5e1 100644 > > --- a/net/tipc/name_distr.c > > +++ b/net/tipc/name_distr.c > > @@ -1,7 +1,7 @@ > > /* > > * net/tipc/name_distr.c: TIPC name distribution code > > * > > - * Copyright (c) 2000-2006, Ericsson AB > > + * Copyright (c) 2000-2006, 2014, Ericsson AB > > * Copyright (c) 2005, 2010-2011, Wind River Systems > > * All rights reserved. > > * > > @@ -71,6 +71,21 @@ static struct publ_list *publ_lists[] = { }; > > > > > > +int sysctl_tipc_named_timeout __read_mostly = 2000; > > + > > +/** > > + * struct tipc_dist_queue - queue holding deferred name table updates > > +*/ static struct list_head tipc_dist_queue = > > +LIST_HEAD_INIT(tipc_dist_queue); > > + > > +struct distr_queue_item { > > + struct distr_item i; > > + u32 dtype; > > + u32 node; > > + u64 expiry; > > + struct list_head next; > > +}; > > + > > /** > > * publ_to_item - add publication info to a publication message > > */ > > @@ -299,6 +314,52 @@ struct publication *tipc_update_nametbl(struct > > distr_item *i, u32 node, } > > > > /** > > + * tipc_named_add_backlog - add a failed name table update to the > > +backlog > > + * > > + */ > > +static void tipc_named_add_backlog(struct distr_item *i, u32 type, > > +u32 > > +node) { > > + struct distr_queue_item *e; > > + u64 now = get_jiffies_64(); > > + > > + e = kzalloc(sizeof(*e), GFP_ATOMIC); > > + if (!e) > > + return; > > + e->dtype = type; > > + e->node = node; > > + e->expiry = now + msecs_to_jiffies(sysctl_tipc_named_timeout); > > + memcpy(e, i, sizeof(*i)); > > + list_add_tail(&e->next, &tipc_dist_queue); } > > + > > +/** > > + * tipc_named_process_backlog - try to process any pending name table > > +updates > > + * from the network. > > + */ > > +void tipc_named_process_backlog(void) { > > + struct distr_queue_item *e, *tmp; > > + char addr[16]; > > + u64 now = get_jiffies_64(); > > + > > + list_for_each_entry_safe(e, tmp, &tipc_dist_queue, next) { > > + if (e->expiry > now) { > > + if (!tipc_update_nametbl(&e->i, e->node, e- > > >dtype)) > > + continue; > > + } else { > > + tipc_addr_string_fill(addr, e->node); > > + pr_warn_ratelimited("Dropping name table update > > (%d) of {%u, %u, %u} from %s key=%u\n", > > + e->dtype, ntohl(e->i.type), > > + ntohl(e->i.lower), > > + ntohl(e->i.upper), > > > + > + tipc_addr_string_fill(addr), > > Otherwise you'll have a nice little crash here ;) Ahh, I see it is correct now. Sorry for the noice. ///jon > > Apart from this I am ok with it. You can add "Reviewed-by" from me. > > ///jon > > > > + ntohl(e->i.key)); > > + } > > + list_del(&e->next); > > + kfree(e); > > + } > > +} > > + > > +/** > > * tipc_named_rcv - process name table update message sent by another > > node > > */ > > void tipc_named_rcv(struct sk_buff *buf) @@ -306,13 +367,15 @@ void > > tipc_named_rcv(struct sk_buff *buf) > > struct tipc_msg *msg = buf_msg(buf); > > struct distr_item *item = (struct distr_item *)msg_data(msg); > > u32 count = msg_data_sz(msg) / ITEM_SIZE; > > + u32 node = msg_orignode(msg); > > > > write_lock_bh(&tipc_nametbl_lock); > > while (count--) { > > - tipc_update_nametbl(item, msg_orignode(msg), > > - msg_type(msg)); > > + if (!tipc_update_nametbl(item, node, msg_type(msg))) > > + tipc_named_add_backlog(item, msg_type(msg), > > node); > > item++; > > } > > + tipc_named_process_backlog(); > > write_unlock_bh(&tipc_nametbl_lock); > > kfree_skb(buf); > > } > > diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h index > > 8afe32b..b9e75fe 100644 > > --- a/net/tipc/name_distr.h > > +++ b/net/tipc/name_distr.h > > @@ -73,5 +73,6 @@ void named_cluster_distribute(struct sk_buff *buf); > > void tipc_named_node_up(u32 dnode); void tipc_named_rcv(struct > > sk_buff *buf); void tipc_named_reinit(void); > > +void tipc_named_process_backlog(void); > > > > #endif > > diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c index > > c058e30..3a6a0a7 100644 > > --- a/net/tipc/name_table.c > > +++ b/net/tipc/name_table.c > > @@ -261,8 +261,6 @@ static struct publication > > *tipc_nameseq_insert_publ(struct name_seq *nseq, > > > > /* Lower end overlaps existing entry => need an exact match > */ > > if ((sseq->lower != lower) || (sseq->upper != upper)) { > > - pr_warn("Cannot publish {%u,%u,%u}, overlap > > error\n", > > - type, lower, upper); > > return NULL; > > } > > > > @@ -284,8 +282,6 @@ static struct publication > > *tipc_nameseq_insert_publ(struct name_seq *nseq, > > /* Fail if upper end overlaps into an existing entry */ > > if ((inspos < nseq->first_free) && > > (upper >= nseq->sseqs[inspos].lower)) { > > - pr_warn("Cannot publish {%u,%u,%u}, overlap > > error\n", > > - type, lower, upper); > > return NULL; > > } > > > > @@ -677,6 +673,8 @@ struct publication *tipc_nametbl_publish(u32 type, > > u32 lower, u32 upper, > > if (likely(publ)) { > > table.local_publ_count++; > > buf = tipc_named_publish(publ); > > + /* Any pending external events? */ > > + tipc_named_process_backlog(); > > } > > write_unlock_bh(&tipc_nametbl_lock); > > > > @@ -698,6 +696,8 @@ int tipc_nametbl_withdraw(u32 type, u32 lower, > u32 > > ref, u32 key) > > if (likely(publ)) { > > table.local_publ_count--; > > buf = tipc_named_withdraw(publ); > > + /* Any pending external events? */ > > + tipc_named_process_backlog(); > > write_unlock_bh(&tipc_nametbl_lock); > > list_del_init(&publ->pport_list); > > kfree(publ); > > diff --git a/net/tipc/sysctl.c b/net/tipc/sysctl.c index > > f3fef93..1a779b1 100644 > > --- a/net/tipc/sysctl.c > > +++ b/net/tipc/sysctl.c > > @@ -47,6 +47,13 @@ static struct ctl_table tipc_table[] = { > > .mode = 0644, > > .proc_handler = proc_dointvec, > > }, > > + { > > + .procname = "named_timeout", > > + .data = &sysctl_tipc_named_timeout, > > + .maxlen = > > sizeof(sysctl_tipc_named_timeout), > > + .mode = 0644, > > + .proc_handler = proc_dointvec, > > + }, > > {} > > }; > > > > -- > > 1.8.3.2 > > > ------------------------------------------------------------------------------ > Slashdot TV. > Video for Nerds. Stuff that matters. > http://tv.slashdot.org/ > _______________________________________________ > tipc-discussion mailing list > tipc-discussion@...ts.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/tipc-discussion -- 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