Hi Jon,
I have had some help testing this but unfortunately the testing has not
been able to be done yet. It is to be done tonight a long run overnight
test to see if we can reproduce the problem.
Sorry about the delay in getting back to you but the people assisting with
the testing have been on other things.
I will be in touch tomorrow.
JT
On Tue, Oct 25, 2016 at 2:15 PM, Jon Maloy <ma...@donjonn.com> wrote:
> Hi John,
>
> Any news about this? I would like to post this patch if you can confirm
> that it is working.
>
> ///jon
>
> On 10/12/2016 06:55 PM, John Thompson wrote:
>
> Hi,
>
> My initial testing has looked good with this patch. I am going to be
> performing more
> in depth testing over the weekend and will be in touch next week about how
> it has
> held up.
>
> Thanks,
> JT
>
>
> On Sat, Oct 1, 2016 at 10:23 PM, Jon Maloy <jon.ma...@ericsson.com> wrote:
>
>> 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 marking the packets where we know the acknowledge
>> value is or may be invalid, and then ignoring the acks from those.
>>
>> To this purpose, we claim an unused bit in the header to indicate that
>> the value is invalid. We set the bit to 1 in the initial BCAST_PROTOCOL
>> synchronization packet and all initial ("bulk") NAME_DISTRIBUTOR
>> packets, plus those LINK_PROTOCOL packets sent out before the broadcast
>> links are fully synchronized.
>>
>> This minor protocol update is fully backwards compatible.
>>
>> Reported-by: John Thompson <thompa....@gmail.com>
>> Signed-off-by: Jon Maloy <jon.ma...@ericsson.com>
>> ---
>> net/tipc/bcast.c | 14 ++++++++++----
>> net/tipc/bcast.h | 3 ++-
>> net/tipc/link.c | 2 ++
>> net/tipc/msg.h | 17 +++++++++++++++++
>> net/tipc/name_distr.c | 1 +
>> net/tipc/node.c | 2 +-
>> 6 files changed, 33 insertions(+), 6 deletions(-)
>>
>> diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
>> index 753f774..aa1babb 100644
>> --- a/net/tipc/bcast.c
>> +++ b/net/tipc/bcast.c
>> @@ -247,11 +247,17 @@ 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;
>> + u16 acked = msg_bcast_ack(hdr);
>> struct sk_buff_head xmitq;
>>
>> + /* Ignore bc acks sent by peer before bcast synch point was
>> received */
>> + if (msg_bc_ack_invalid(hdr))
>> + return;
>> +
>> __skb_queue_head_init(&xmitq);
>>
>> tipc_bcast_lock(net);
>> @@ -279,11 +285,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/link.c b/net/tipc/link.c
>> index b36e16c..1055164 100644
>> --- a/net/tipc/link.c
>> +++ b/net/tipc/link.c
>> @@ -1312,6 +1312,7 @@ static void tipc_link_build_proto_msg(struct
>> tipc_link *l, int mtyp, bool probe,
>> msg_set_next_sent(hdr, l->snd_nxt);
>> msg_set_ack(hdr, l->rcv_nxt - 1);
>> msg_set_bcast_ack(hdr, bcl->rcv_nxt - 1);
>> + msg_set_bc_ack_invalid(hdr, !node_up);
>> msg_set_last_bcast(hdr, l->bc_sndlink->snd_nxt - 1);
>> msg_set_link_tolerance(hdr, tolerance);
>> msg_set_linkprio(hdr, priority);
>> @@ -1574,6 +1575,7 @@ static void tipc_link_build_bc_init_msg(struct
>> tipc_link *l,
>> __skb_queue_head_init(&list);
>> if (!tipc_link_build_bc_proto_msg(l->bc_rcvlink, false, 0,
>> &list))
>> return;
>> + msg_set_bc_ack_invalid(buf_msg(skb_peek(&list)), true);
>> tipc_link_xmit(l, &list, xmitq);
>> }
>>
>> diff --git a/net/tipc/msg.h b/net/tipc/msg.h
>> index c3832cd..50a7398 100644
>> --- a/net/tipc/msg.h
>> +++ b/net/tipc/msg.h
>> @@ -714,6 +714,23 @@ 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)
>> +{
>> + switch (msg_user(m)) {
>> + case BCAST_PROTOCOL:
>> + case NAME_DISTRIBUTOR:
>> + case LINK_PROTOCOL:
>> + return msg_bits(m, 5, 14, 0x1);
>> + default:
>> + return false;
>> + }
>> +}
>> +
>> +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/name_distr.c b/net/tipc/name_distr.c
>> index a04fe9b..c1cfd92 100644
>> --- a/net/tipc/name_distr.c
>> +++ b/net/tipc/name_distr.c
>> @@ -156,6 +156,7 @@ static void named_distribute(struct net *net, struct
>> sk_buff_head *list,
>> pr_warn("Bulk publication failure\n");
>> return;
>> }
>> + msg_set_bc_ack_invalid(buf_msg(skb), true);
>> item = (struct distr_item
>> *)msg_data(buf_msg(skb));
>> }
>>
>> 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
>>
>>
>
>
------------------------------------------------------------------------------
The Command Line: Reinvented for Modern Developers
Did the resurgence of CLI tooling catch you by surprise?
Reconnect with the command line and become more productive.
Learn the new .NET and ASP.NET CLI. Get your free copy!
http://sdm.link/telerik
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
