Re: [tipc-discussion] [PATCH net v1 2/3] tipc: return early for non-blocking sockets at link congestion

[Jon Maloy]

> -Original Message-
> From: Parthasarathy Bhuvaragan
> Sent: Wednesday, 05 October, 2016 04:18
> To: Jon Maloy ; tipc-discussion@lists.sourceforge.net;
> ma...@donjonn.com; Ying Xue 
> Subject: Re: [PATCH net v1 2/3] tipc: return early for non-blocking sockets 
> at link
> congestion
> 
> On 10/04/2016 09:26 PM, Jon Maloy wrote:
> >
> >
> >> -Original Message-
> >> From: Parthasarathy Bhuvaragan
> >> Sent: Tuesday, 04 October, 2016 08:29
> >> To: tipc-discussion@lists.sourceforge.net; Jon Maloy
> ;
> >> ma...@donjonn.com; Ying Xue 
> >> Subject: [PATCH net v1 2/3] tipc: return early for non-blocking sockets at 
> >> link
> >> congestion
> >>
> >> Until now, in stream/mcast send() we pass the message to the link
> >> layer even when the link is congested and add the socket to the
> >> link's wakeup queue. This is unnecessary for non-blocking sockets.
> >> If a socket is set to non-blocking and sends multicast with zero
> >> back off time while receiving EAGAIN, we exhaust the memory.
> >>
> >> In this commit, we return immediately at stream/mcast send() for
> >> non-blocking sockets.
> >>
> >> Signed-off-by: Parthasarathy Bhuvaragan
> >> 
> >> ---
> >>  net/tipc/socket.c | 6 ++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> >> index f9f5f3c3dab5..adf3e6ecf61e 100644
> >> --- a/net/tipc/socket.c
> >> +++ b/net/tipc/socket.c
> >> @@ -697,6 +697,9 @@ static int tipc_sendmcast(struct  socket *sock, struct
> >> tipc_name_seq *seq,
> >>   uint mtu;
> >>   int rc;
> >>
> >> + if (!timeo && tsk->link_cong)
> >> + return -ELINKCONG;
> >
> > Why is the !timeo test needed ?
> [partha] When a socket with a non-zero sndtimeo experiences congestion,
> the socket is put to sleep for the specified duration and is woken if
> congestion ceases and we attempt to transmit the buffer. If we had
> experienced congestion for the entire duration, then we return
> ELINKLCONG to the user. Hence I can return immediately only when timeout
> == 0.

Are you saying that we can have non-blocking sockets with a non-zero timeout?
I guess my point was that if the socket is blocking, i.e., the timeout is != 0, 
you will never get into this second call in the first place, since the socket 
already is blocked if sk->link_cong is true. Hence no need for testing on the 
timer value. Unsure if this is valid with multiple threads though.

> >
> > ///jon
> >
> >> +
> >>   msg_set_type(mhdr, TIPC_MCAST_MSG);
> >>   msg_set_lookup_scope(mhdr, TIPC_CLUSTER_SCOPE);
> >>   msg_set_destport(mhdr, 0);
> >> @@ -1072,6 +1075,9 @@ static int __tipc_send_stream(struct socket *sock,
> >> struct msghdr *m, size_t dsz)
> >>   }
> >>
> >>   timeo = sock_sndtimeo(sk, m->msg_flags & MSG_DONTWAIT);
> >> + if (!timeo && tsk->link_cong)
> >> + return -ELINKCONG;
> >> +
> >>   dnode = tsk_peer_node(tsk);
> >>   skb_queue_head_init();
> >>
> >> --
> >> 2.1.4
> >

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Re: [tipc-discussion] [PATCH net v1 1/3] tipc: fix socket wakeup when the transmit queue is empty

[Parthasarathy Bhuvaragan]

On 10/04/2016 09:24 PM, Jon Maloy wrote:
>
>
>> -Original Message-
>> From: Parthasarathy Bhuvaragan
>> Sent: Tuesday, 04 October, 2016 08:29
>> To: tipc-discussion@lists.sourceforge.net; Jon Maloy 
>> ;
>> ma...@donjonn.com; Ying Xue 
>> Subject: [PATCH net v1 1/3] tipc: fix socket wakeup when the transmit queue 
>> is
>> empty
>>
>> Until now, in tipc_link_rcv() the sockets are woken only when we were
>> able to release a frame from the link's transmit queue.
>> However link_prepare_wakeup(), wakes only as many users as permitted
>> by the current link window and the rest of the users are woken the
>> next time a frame is released. In case of the released frame being
>> the last frame in the transmit queue i.e the queue becomes empty, we
>> miss to wake the users and they experience permanent congestion.
>>
>> In this commit, we wakeup the users if link's transmit queue is empty.
>>
>> Signed-off-by: Parthasarathy Bhuvaragan
>> 
>> ---
>>  net/tipc/link.c | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/net/tipc/link.c b/net/tipc/link.c
>> index b36e16cdc945..28f9376ce6d7 100644
>> --- a/net/tipc/link.c
>> +++ b/net/tipc/link.c
>> @@ -1251,6 +1251,10 @@ int tipc_link_rcv(struct tipc_link *l, struct sk_buff 
>> *skb,
>>   link_prepare_wakeup(l);
>>   }
>>
>> + if (skb_queue_empty(>transmq) &&
>> + !skb_queue_empty(>wakeupq))
>> + link_prepare_wakeup(l);
>
> I don't think this one is needed. If you look at the last lines of 
> tipc_link_proto_rcv() the wakeups are attempted unconditionally on an idle 
> link, and it is anyway at STATE messages that this property is important.
Thanks for spotting it. Yes, I see that now. I drop this patch.
/Partha
>
> ///jon
>
>> +
>>   /* Defer delivery if sequence gap */
>>   if (unlikely(seqno != rcv_nxt)) {
>>   __tipc_skb_queue_sorted(defq, seqno, skb);
>> --
>> 2.1.4
>

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Re: [tipc-discussion] [PATCH net v1 2/3] tipc: return early for non-blocking sockets at link congestion

[Parthasarathy Bhuvaragan]

On 10/04/2016 09:26 PM, Jon Maloy wrote:
>
>
>> -Original Message-
>> From: Parthasarathy Bhuvaragan
>> Sent: Tuesday, 04 October, 2016 08:29
>> To: tipc-discussion@lists.sourceforge.net; Jon Maloy 
>> ;
>> ma...@donjonn.com; Ying Xue 
>> Subject: [PATCH net v1 2/3] tipc: return early for non-blocking sockets at 
>> link
>> congestion
>>
>> Until now, in stream/mcast send() we pass the message to the link
>> layer even when the link is congested and add the socket to the
>> link's wakeup queue. This is unnecessary for non-blocking sockets.
>> If a socket is set to non-blocking and sends multicast with zero
>> back off time while receiving EAGAIN, we exhaust the memory.
>>
>> In this commit, we return immediately at stream/mcast send() for
>> non-blocking sockets.
>>
>> Signed-off-by: Parthasarathy Bhuvaragan
>> 
>> ---
>>  net/tipc/socket.c | 6 ++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
>> index f9f5f3c3dab5..adf3e6ecf61e 100644
>> --- a/net/tipc/socket.c
>> +++ b/net/tipc/socket.c
>> @@ -697,6 +697,9 @@ static int tipc_sendmcast(struct  socket *sock, struct
>> tipc_name_seq *seq,
>>   uint mtu;
>>   int rc;
>>
>> + if (!timeo && tsk->link_cong)
>> + return -ELINKCONG;
>
> Why is the !timeo test needed ?
[partha] When a socket with a non-zero sndtimeo experiences congestion, 
the socket is put to sleep for the specified duration and is woken if 
congestion ceases and we attempt to transmit the buffer. If we had 
experienced congestion for the entire duration, then we return 
ELINKLCONG to the user. Hence I can return immediately only when timeout 
== 0.
>
> ///jon
>
>> +
>>   msg_set_type(mhdr, TIPC_MCAST_MSG);
>>   msg_set_lookup_scope(mhdr, TIPC_CLUSTER_SCOPE);
>>   msg_set_destport(mhdr, 0);
>> @@ -1072,6 +1075,9 @@ static int __tipc_send_stream(struct socket *sock,
>> struct msghdr *m, size_t dsz)
>>   }
>>
>>   timeo = sock_sndtimeo(sk, m->msg_flags & MSG_DONTWAIT);
>> + if (!timeo && tsk->link_cong)
>> + return -ELINKCONG;
>> +
>>   dnode = tsk_peer_node(tsk);
>>   skb_queue_head_init();
>>
>> --
>> 2.1.4
>

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion