Hi John, I think this one should solve the problem. Pease try it when you are back from vacation, and give feedback.
BR ///jon > -----Original Message----- > From: Jon Maloy [mailto:jon.ma...@ericsson.com] > Sent: Friday, 30 September, 2016 18:07 > To: tipc-discussion@lists.sourceforge.net; Parthasarathy Bhuvaragan > <parthasarathy.bhuvara...@ericsson.com>; Ying Xue > <ying....@windriver.com>; ri...@highwind.se; Jon Maloy > <jon.ma...@ericsson.com> > Cc: ma...@donjonn.com; thompa....@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 issue. > > We have seen it happen that LINK_PROTOCOL/STATE packets with a valid > non-zero unicast acknowledge number may bypass BCAST_PROTOCOL > initialization, NAME_DISTRIBUTOR and other STATE packets with invalid > broadcast acknowledge numbers, leading to premature opening of the > broadcast link. When the bypassed packets finally arrive, they are > inadvertently accepted, and the already correctly initialized > acknowledge number in the broadcast receive link is overwritten by > the invalid (zero) value of the said packets. After this the broadcast > link goes stale. > > We now fix this by identifying packets where we know the acknowledge > value is or may be invalid, and then ignoring those values. > > For BCAST_PROTOCOL and NAME_DISTRIBUTOR packets this identification is > easy; their broadcast acknowledge value is always zero, while false > positives are far between and harmless. > > For LINK_PROTOCOL/STATE packets we cannot accept false positives, so > we have to claim an unused bit in the header to indicate that the value > is invalid. This minor protocol change is fully backwards compatible. > > Reported-by: John Thompson <thompa....@gmail.com> > Signed-off-by: Jon Maloy <jon.ma...@ericsson.com> > --- > net/tipc/bcast.c | 17 ++++++++++++----- > net/tipc/bcast.h | 3 ++- > net/tipc/msg.h | 10 ++++++++++ > net/tipc/node.c | 2 +- > 4 files changed, 25 insertions(+), 7 deletions(-) > > diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c > index 753f774..62d2dfd 100644 > --- a/net/tipc/bcast.c > +++ b/net/tipc/bcast.c > @@ -247,15 +247,22 @@ 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, > + struct tipc_msg *hdr) > { > struct sk_buff_head *inputq = &tipc_bc_base(net)->inputq; > + int usr = msg_user(hdr); > + u16 bc_ack = msg_bcast_ack(hdr); > struct sk_buff_head xmitq; > > + /* Ignore bc acks sent by peer before bcast synch point was received */ > + if (!bc_ack && ((usr == BCAST_PROTOCOL) || (usr == > NAME_DISTRIBUTOR))) > + return; > + > __skb_queue_head_init(&xmitq); > > tipc_bcast_lock(net); > - tipc_link_bc_ack_rcv(l, acked, &xmitq); > + tipc_link_bc_ack_rcv(l, bc_ack, &xmitq); > tipc_bcast_unlock(net); > > tipc_bcbase_xmit(net, &xmitq); > @@ -279,11 +286,11 @@ int tipc_bcast_sync_rcv(struct net *net, struct > tipc_link > *l, > __skb_queue_head_init(&xmitq); > > tipc_bcast_lock(net); > - if (msg_type(hdr) == STATE_MSG) { > + if (msg_type(hdr) != STATE_MSG) { > + tipc_link_bc_init_rcv(l, hdr); > + } else if (!msg_bc_ack_invalid(hdr)) { > tipc_link_bc_ack_rcv(l, msg_bcast_ack(hdr), &xmitq); > rc = tipc_link_bc_sync_rcv(l, hdr, &xmitq); > - } else { > - tipc_link_bc_init_rcv(l, hdr); > } > tipc_bcast_unlock(net); > > diff --git a/net/tipc/bcast.h b/net/tipc/bcast.h > index 5ffe344..855d53c 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, > + struct tipc_msg *hdr); > int 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/msg.h b/net/tipc/msg.h > index c3832cd..ef37e49 100644 > --- a/net/tipc/msg.h > +++ b/net/tipc/msg.h > @@ -714,6 +714,16 @@ static inline void msg_set_peer_stopping(struct tipc_msg > *m, u32 s) > msg_set_bits(m, 5, 13, 0x1, s); > } > > +static inline bool msg_bc_ack_invalid(struct tipc_msg *m) > +{ > + return msg_bits(m, 5, 14, 0x1); > +} > + > +static inline void msg_set_bc_ack_invalid(struct tipc_msg *m, bool invalid) > +{ > + msg_set_bits(m, 5, 14, 0x1, invalid); > +} > + > static inline char *msg_media_addr(struct tipc_msg *m) > { > return (char *)&m->hdr[TIPC_MEDIA_INFO_OFFSET]; > diff --git a/net/tipc/node.c b/net/tipc/node.c > index 7ef14e2..9d2f4c2 100644 > --- a/net/tipc/node.c > +++ b/net/tipc/node.c > @@ -1535,7 +1535,7 @@ void tipc_rcv(struct net *net, struct sk_buff *skb, > struct > tipc_bearer *b) > if (unlikely(usr == LINK_PROTOCOL)) > tipc_node_bc_sync_rcv(n, hdr, bearer_id, &xmitq); > 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, hdr); > > /* 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
