svn commit: r357051 - head/sys/dev/bge

[Jeff Roberson]

On Sun, 26 Jan 2020, John Baldwin wrote:

> On 1/23/20 7:32 PM, Gleb Smirnoff wrote:
>> On Thu, Jan 23, 2020 at 05:09:14PM -1000, Jeff Roberson wrote:
>> J> While we don't have a policy strictly requiring reviews it is the norm to
>> J> have substantial changes socialized and reviewed.  I appreciate the work
>> J> that you are doing but it likely should've been discussed somewhere
>> J> more publicly.  I apologized if I missed it but I don't see reference to
>> J> anything.
>>
>> That was https://reviews.freebsd.org/D23242
>
> A review alone isn't sufficient for large, sweeping changes in my mind.
> For major changes, a thread on arch@ or net@ or the like is probably more
> appropriate.  You can include a link to a review or git branch, etc. in
> that e-mail, but phabricator aren't as well suited to higher-level
> design-review type discussion, more for implementation-review.
>> J> Architecturally I am more concerned with the coarseness of net_epoch and
>> J> the duration of hold becoming a resource utilization problem in high
>> J> turn-over workloads.  Like short connection tcp.  Has anyone done
>> J> substantial testing here?  epoch as it is today will hold every free
>> J> callback for a minimum of several clock ticks and a maximum of 2x the
>> J> duration of the longest epoch section time.  With preemption, etc. this
>> J> could be 100s of ms of PCBs held.
>>
>> We also are concerned about that theoretically. Haven't yet seen effect
>> in practice, but our sessions are mostly longer living. First we have the
>> tunable to limit batching. Second, there are some ideas on how to improve
>> the garbage collector performance if it becomes an issue.
>
> There are other workloads than Netflix. ;)  Verisign has incredibly short-lived
> connections with very high turnover.  I think though that they have already
> abandoned the in-tree network stack for a userland stack built on netmap.  Still,
> I think that there are probably other FreeBSD users that are probably somewhere
> in the middle that shouldn't be ignored.
>
> Packet batching would not be impossible by simply using m_nextpkt chains in
> mbufs passed up to ether_input and having ether_input pass them in a loop to
> the next higher loop (as a first step).  That would reduce unlock/lock operations
> in drivers (for those still using locks on receive) as well as permitting
> ether_input to process batches under a single epoch invocation.

This is actually the approach that I took for nokia.  You could prefetch 
m->m_nextpkt at the top of the loop iteration.  It was very effective 
there.

Seeing how many drivers and unexpected entry points had to have the 
NET_EPOCH added I would want to review again once it's stable and see if 
there is a way to simplify through API changes.  It seems like more than 
expected slipped through the cracks and I worry about long-term 
maintenance.

Thanks,
Jeff

>
> -- 
> John Baldwin
>