Hi Jon, I will give this patch a test and get back in touch. I am on vacation for a few days and so will be in touch next week.
Thanks, JT On Thu, Sep 8, 2016 at 12:28 AM, Jon Maloy <jon.ma...@ericsson.com> wrote: > Hi John, > I am unable to reproduce the scenario you are describing, but from your > debug data it looks clear enough: a STATE packet with ack value !=0 has > bypassed the first in-sequence packets, which still have an invalid > pre-synch ack value. > > If that diagnosis is right, this patch should solve the problem. Please > test this and give feedback. > > BR > ///jon > > > > -----Original Message----- > > From: Jon Maloy [mailto:jon.ma...@ericsson.com] > > Sent: Wednesday, 07 September, 2016 08:18 > > To: tipc-discussion@lists.sourceforge.net; Parthasarathy Bhuvaragan > > <parthasarathy.bhuvara...@ericsson.com>; Ying Xue > > <ying....@windriver.com>; Richard Alpe <richard.a...@ericsson.com>; Jon > > Maloy <jon.ma...@ericsson.com> > > Cc: ma...@donjonn.com; gbala...@gmail.com > > Subject: [PATCH net 1/1] tipc: fix broadcast link synchronization problem > > > > In commit 2d18ac4ba745 ("tipc: extend broadcast link initialization > > criteria") we tried to fix a problem with the initial synchronization > > of broadcast link acknowledge values. Unfortunately that solution is > > not sufficient to solve the problem. > > > > We have seen it happen that a LINK_PROTOCOL/STATE message with a valid > > non-zero unicast acknowledge number may bypass BCAST_PROTOCOL > > initialization and NAME_DISRIBUTOR packets with invalid broadcast > > acknowledge numbers, leading to premature opening of the broadcast link. > > When this happens, the already correctly initialized acknowledge number > > of the broadcast receive link will be overwritten by the invalid (zero) > > value of the said packets, and the broadcast link risks going stale. > > > > We now fix this by ignoring acknowledges that we know potentially may > > be invalid. Identifying such acknowledges is easy, since their broadcast > > acknowledge value always is zero, and since this can only happen to > > BCAST_PROTOCOL and NAME_DISTRIBUTOR messages. > > > > Reported-by: John Thompson <thompa....@gmail.com> > > Signed-off-by: Jon Maloy <jon.ma...@ericsson.com> > > --- > > net/tipc/bcast.c | 7 ++++++- > > net/tipc/bcast.h | 3 ++- > > net/tipc/node.c | 2 +- > > 3 files changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c > > index ae469b3..27ffa37 100644 > > --- a/net/tipc/bcast.c > > +++ b/net/tipc/bcast.c > > @@ -247,11 +247,16 @@ int tipc_bcast_rcv(struct net *net, struct > tipc_link *l, > > struct sk_buff *skb) > > * > > * RCU is locked, no other locks set > > */ > > -void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l, u32 acked) > > +void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l, > > + int usr, u16 acked) > > { > > struct sk_buff_head *inputq = &tipc_bc_base(net)->inputq; > > struct sk_buff_head xmitq; > > > > + /* Ignore acks potentially sent before bcast synch point was > received */ > > + if (!acked && ((usr == BCAST_PROTOCOL) || (usr == > > NAME_DISTRIBUTOR))) > > + return; > > + > > __skb_queue_head_init(&xmitq); > > > > tipc_bcast_lock(net); > > diff --git a/net/tipc/bcast.h b/net/tipc/bcast.h > > index d5e79b3..0529cb5 100644 > > --- a/net/tipc/bcast.h > > +++ b/net/tipc/bcast.h > > @@ -55,7 +55,8 @@ void tipc_bcast_dec_bearer_dst_cnt(struct net *net, > int > > bearer_id); > > int tipc_bcast_get_mtu(struct net *net); > > int tipc_bcast_xmit(struct net *net, struct sk_buff_head *list); > > int tipc_bcast_rcv(struct net *net, struct tipc_link *l, struct sk_buff > *skb); > > -void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l, u32 > acked); > > +void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l, > > + int usr, u16 acked); > > void tipc_bcast_sync_rcv(struct net *net, struct tipc_link *l, > > struct tipc_msg *hdr); > > int tipc_nl_add_bc_link(struct net *net, struct tipc_nl_msg *msg); > > diff --git a/net/tipc/node.c b/net/tipc/node.c > > index 2197419..42f98af 100644 > > --- a/net/tipc/node.c > > +++ b/net/tipc/node.c > > @@ -1507,7 +1507,7 @@ void tipc_rcv(struct net *net, struct sk_buff > *skb, struct > > tipc_bearer *b) > > if (unlikely(usr == LINK_PROTOCOL)) > > tipc_bcast_sync_rcv(net, n->bc_entry.link, hdr); > > else if (unlikely(tipc_link_acked(n->bc_entry.link) != bc_ack)) > > - tipc_bcast_ack_rcv(net, n->bc_entry.link, bc_ack); > > + tipc_bcast_ack_rcv(net, n->bc_entry.link, usr, bc_ack); > > > > /* Receive packet directly if conditions permit */ > > tipc_node_read_lock(n); > > -- > > 2.7.4 > > > ------------------------------------------------------------ > ------------------ > _______________________________________________ > tipc-discussion mailing list > tipc-discussion@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/tipc-discussion > ------------------------------------------------------------------------------ _______________________________________________ tipc-discussion mailing list tipc-discussion@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tipc-discussion
