svn commit: r323942 - head/sys/net

[Stephen Hurd]

Stephen Hurd wrote:
> Hans Petter Selasky wrote:
>> On 09/24/17 01:46, Stephen Hurd wrote:
>>> Basically, it changed from this:
>>>
>>> foreach (mbuf in rx) {
>>>    if (lro && tcp_lro_rx(mbuf) == 0)
>>>      continue;
>>>    if_input(mbuf)
>>> }
>>>
>>> To this:
>>>
>>> prev_mbuf = first_mbuf = NULL;
>>> foreach (mbuf in rx) {
>>>    if (lro && tcp_lro_rx(mbuf) == 0)
>>>      continue;
>>>    if (prev_mbuf) {
>>>      prev_mbuf->m_nextpkt = mbuf;
>>>      prev_mbuf = mbuf;
>>>    }
>>>    else {
>>>      first_mbuf = prev_mbuf = mbuf;
>>>    }
>>> }
>>>
>>> if (first_mbuf)
>>>    if_input(first_mbuf);
>>>
>>> So while before it called if_input() for each separate mbuf that was 
>>> not LROed, it now builds a chain of mbufs that were not LROed, and 
>>> makes a single call to if_input() with the whole chain.  For cases 
>>> like packet forwarding where no packets are LROed, performance is 
>>> better.
>>>
>>
>> Can such a similar logic be applied inside TCP LRO aswell?
>
> It looks like it would be more complex to do a similar thing in 
> tcp_lro.c, and I'm not certain it would help much except in cases with 
> a large number of streams that mostly end up not being coalesced.  
> Taking a quick look, tcp_lro_flush() would need to be modified to 
> return an mbuf head and tail, then the caller would need to be 
> responsible for combining them into a single mbuf chain and calling 
> if_input().
>
> Either that, or an mbuf tail could be passed into tcp_lro_flush(), the 
> tail modified in there, and an mbuf head returned... that way it would 
> work something like this:
>
> The caller would be something like this:
>
> m_head = m_tail = NULL;
> LIST_FOREACH(le, bucket, hash_next) {
>   head = tcp_lro_flush(lc, le, &m_tail);
>   if (m_head == NULL)
>     m_head = head;
> }
> if (m_head)
>   if_input(m_head);
>
> And tcp_lro_flush() would be something like this:
>
> struct mbuf *tcp_lro_flush(struct lro_ctrl *lc, struct lro_entry *le, 
> struct mbuf **tail)
> {
>   ...
>   if (*tail)
>     *tail->m_next = le->m_head;
>   *tail = le->m_tail;
>   ...
>   return le->m_head;
> }
>
> Hrm, maybe it wouldn't be all that difficult after all.  :-)
>
> I'll be driving across the country later this week, so I don't want to 
> start poking into LRO then disappear, so if nobody else tries it out 
> before then, I should take a look in a couple weeks.

I've done an initial pass here: https://reviews.freebsd.org/D12487

Feel free to test it out and report findings in the review.