Re: [tipc-discussion] Strange behavior in socket.c::tipc_sk_enqueue()

2021-09-08 Thread Hoang Huu Le



> -Original Message-
> From: Jon Maloy 
> Sent: Thursday, September 9, 2021 5:42 AM
> To: Hoang Huu Le ; 
> tipc-discussion@lists.sourceforge.net; Tung Quang Nguyen
> ; Xin Long ; Ying Xue 
> 
> Cc: Huy Xuan Nhat Hoang 
> Subject: Re: Strange behavior in socket.c::tipc_sk_enqueue()
> 
> 
> 
> On 06/09/2021 05:02, Hoang Huu Le wrote:
> > Hi Jon,  all,
> >
> > I did a test by setting two variables condition in range:
> > - time limit:  2 msecs ... unlimited
> > - search depth limit (sock's skbs): 2 skbs ... unlimited
> >
> > With above range settings, a maximum sock's skbs can be enqueued around 12 
> > skbs regardless of time and search depth limit.
> > I also combine the test with iperf TCP traffic generated and the result 
> > looks the same.
> >
> > So, I don't think we need to apply the search depth limit condition and/or 
> > longer timer in this function, just 2msecs is enough.
> > I guess this result depends on kernel schedule. What are your views?
> 
> I assume your test was done with many, e.g. 100 connections?

Yes, I did the test from 1 to 150 connections and combine with/out other 
traffic generate (i.e TCP).

> 
> ///jon
> 
> >
> > Regards,
> > Hoang
> >> -Original Message-
> >> From: Jon Maloy 
> >> Sent: Wednesday, September 1, 2021 7:39 AM
> >> To: Hoang Huu Le ; 
> >> tipc-discussion@lists.sourceforge.net; Tung Quang Nguyen
> >> ; Xin Long ; Ying Xue 
> >> 
> >> Cc: Huy Xuan Nhat Hoang 
> >> Subject: Re: Strange behavior in socket.c::tipc_sk_enqueue()
> >>
> >> Guys,
> >> After our discussion this morning regarding this problem I gave it some
> >> more thought.
> >>
> >> What if we simply limit the search depth in the receive queue to some
> >> fix number, 10, 20, 50 or something and return NULL if nothing is found
> >> within this range. This would be a simple stack counter inside
> >> tipc_skb_dequeue(), and would cost almost nothing.
> >>
> >> If you experiment with this, of course in combination with a max limit
> >> of some milliseconds as we also discussed, we might obtain acceptable
> >> results.
> >>
> >> What do you think?
> >>
> >> ///jon
> >>
> >>
> >> On 28/07/2021 04:04, Hoang Huu Le wrote:
> >>> Hi Jon,
> >>>
> >>> Let's enjoy your vacation.
> >>> Our new team member (CCed) will take a look at it.
> >>>
> >>> Regards,
> >>> Hoang
>  -Original Message-
>  From: Jon Maloy 
>  Sent: Wednesday, July 28, 2021 6:20 AM
>  To: tipc-discussion@lists.sourceforge.net; Tung Quang Nguyen 
>  ; Hoang Huu Le
>  ; Xin Long ; Ying Xue 
>  
>  Subject: Strange behavior in socket.c::tipc_sk_enqueue()
> 
>  I did by accident discover a strange behavior in the function
>  tipc_sk_enqueue:
> 
> 
>  static void tipc_sk_enqueue(struct sk_buff_head *inputq,
> struct sock *sk, u32 dport,
> struct sk_buff_head *xmitq)
>  {
> struct tipc_sock *tsk = tipc_sk(sk);
> unsigned long time_limit = jiffies + 2;
> struct sk_buff *skb;
> unsigned int lim;
> atomic_t *dcnt;
> u32 onode;
> 
> while (skb_queue_len(inputq)) {
> if (unlikely(time_after_eq(jiffies, time_limit)))
>   return;
> [...]
> }
>  }
> 
>  At the moment we call time_after_eq() the two jiffies often
>  have already passed, and the skb is not dequeued.
>  I noticed that tipc_sk_rcv() may call tipc_sk_enqueue()
>  with the same skb dozens of times before the buffer can
>  be delivered further upwards in the stack.
> 
>  Needless to say that this cannot be good for performance.
> 
>  I believe the value of 2 jiffies was hard coded at a time
>  when machines were slower, and a jiffie represented a much
>  longer time interval.
> 
>  Now it is clearly too short, and should be replaced with something
>  longer and more consisten, e.g. msec_to_jiffies(2).
> 
>  Can anybody look into this?
> 
>  Also, I will be on vacation the next two weeks, which means we
>  should cancel the bi-weekly meeting next Tuesday.
> 
>  ///jon
> 
> >>>
> >


___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion


Re: [tipc-discussion] Strange behavior in socket.c::tipc_sk_enqueue()

2021-09-08 Thread Jon Maloy




On 06/09/2021 05:02, Hoang Huu Le wrote:

Hi Jon,  all,

I did a test by setting two variables condition in range:
- time limit:  2 msecs ... unlimited
- search depth limit (sock's skbs): 2 skbs ... unlimited

With above range settings, a maximum sock's skbs can be enqueued around 12 skbs 
regardless of time and search depth limit.
I also combine the test with iperf TCP traffic generated and the result looks 
the same.

So, I don't think we need to apply the search depth limit condition and/or 
longer timer in this function, just 2msecs is enough.
I guess this result depends on kernel schedule. What are your views?


I assume your test was done with many, e.g. 100 connections?

///jon



Regards,
Hoang

-Original Message-
From: Jon Maloy 
Sent: Wednesday, September 1, 2021 7:39 AM
To: Hoang Huu Le ; 
tipc-discussion@lists.sourceforge.net; Tung Quang Nguyen
; Xin Long ; Ying Xue 

Cc: Huy Xuan Nhat Hoang 
Subject: Re: Strange behavior in socket.c::tipc_sk_enqueue()

Guys,
After our discussion this morning regarding this problem I gave it some
more thought.

What if we simply limit the search depth in the receive queue to some
fix number, 10, 20, 50 or something and return NULL if nothing is found
within this range. This would be a simple stack counter inside
tipc_skb_dequeue(), and would cost almost nothing.

If you experiment with this, of course in combination with a max limit
of some milliseconds as we also discussed, we might obtain acceptable
results.

What do you think?

///jon


On 28/07/2021 04:04, Hoang Huu Le wrote:

Hi Jon,

Let's enjoy your vacation.
Our new team member (CCed) will take a look at it.

Regards,
Hoang

-Original Message-
From: Jon Maloy 
Sent: Wednesday, July 28, 2021 6:20 AM
To: tipc-discussion@lists.sourceforge.net; Tung Quang Nguyen 
; Hoang Huu Le
; Xin Long ; Ying Xue 

Subject: Strange behavior in socket.c::tipc_sk_enqueue()

I did by accident discover a strange behavior in the function
tipc_sk_enqueue:


static void tipc_sk_enqueue(struct sk_buff_head *inputq,
   struct sock *sk, u32 dport,
   struct sk_buff_head *xmitq)
{
   struct tipc_sock *tsk = tipc_sk(sk);
   unsigned long time_limit = jiffies + 2;
   struct sk_buff *skb;
   unsigned int lim;
   atomic_t *dcnt;
   u32 onode;

   while (skb_queue_len(inputq)) {
   if (unlikely(time_after_eq(jiffies, time_limit)))
 return;
   [...]
   }
}

At the moment we call time_after_eq() the two jiffies often
have already passed, and the skb is not dequeued.
I noticed that tipc_sk_rcv() may call tipc_sk_enqueue()
with the same skb dozens of times before the buffer can
be delivered further upwards in the stack.

Needless to say that this cannot be good for performance.

I believe the value of 2 jiffies was hard coded at a time
when machines were slower, and a jiffie represented a much
longer time interval.

Now it is clearly too short, and should be replaced with something
longer and more consisten, e.g. msec_to_jiffies(2).

Can anybody look into this?

Also, I will be on vacation the next two weeks, which means we
should cancel the bi-weekly meeting next Tuesday.

///jon









___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion