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

[Hoang H. Le <hoa...@de...>]

> -----Original Message-----
> From: Jon Maloy <jm...@re...>
> Sent: Thursday, September 9, 2021 5:42 AM
> To: Hoang Huu Le <hoa...@de...>; tip...@li...; Tung Quang Nguyen
> <tun...@de...>; Xin Long <luc...@gm...>; Ying Xue <yin...@wi...>
> Cc: Huy Xuan Nhat Hoang <huy...@de...>
> 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 <jm...@re...>
> >> Sent: Wednesday, September 1, 2021 7:39 AM
> >> To: Hoang Huu Le <hoa...@de...>; tip...@li...; Tung Quang Nguyen
> >> <tun...@de...>; Xin Long <luc...@gm...>; Ying Xue <yin...@wi...>
> >> Cc: Huy Xuan Nhat Hoang <huy...@de...>
> >> 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 <jm...@re...>
> >>>> Sent: Wednesday, July 28, 2021 6:20 AM
> >>>> To: tip...@li...; Tung Quang Nguyen <tun...@de...>; Hoang Huu Le
> >>>> <hoa...@de...>; Xin Long <luc...@gm...>; Ying Xue <yin...@wi...>
> >>>> 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
> >>>>
> >>>
> >