Re: [dpdk-dev] [dpdk-stable] [PATCH v4] net/bonding: per-slave intermediate rx ring

[Matan Azrad <matan@mellanox.com>]

From: Chas Williams
>On Mon, Aug 27, 2018 at 11:30 AM Matan Azrad <mailto:matan@mellanox.com> wrote:
<snip>
>>>Because rings are generally quite efficient.
>>
>>But you are using a ring in addition to regular array management, it must hurt performance of the bonding PMD
>>(means the bonding itself - not the slaves PMDs which are called from the bonding)
>
>It adds latency.

And by that hurts the application performance because it takes more CPU time in the bonding PMD.

>It increases performance because we spend less CPU time reading from the PMDs.

So, it's hack in the bonding PMD to improve some slaves code performance but hurt the bonding code performance,
Over all the performance we gain for those slaves improves the application performance only when working with those slaves. 
But may hurt the application performance when working with other slaves.

>  This means we have more CPU to use for
>post processing (i.e. routing).



>>>Bonding is in a middle ground between application and PMD.
>>Yes.
>>>What bonding is doing, may not improve all applications.
>>Yes, but it can be solved using some bonding modes.
>>> If using a ring to buffer the vectorized receive routines, improves your particular application,
>>>that's great. 
>>It may be not great and even bad for some other PMDs which are not vectororized.
>>
>>> However, I don't think I can say that it would help all
>>>applications.  As you point out, there is overhead associated with
>>>a ring.
>>Yes.
>>>Bonding's receive burst isn't especially efficient (in mode 4).
>>
>>Why?
>>
>>It makes a copy of the slaves, has a fair bit of stack usage, 
>>needs to check the slave status, and needs to examine each
>>packet to see if it is a slow protocol packet.  So each
>>packet is essentially read twice.  The fast queue code for mode 4
>>avoids some of this (and probably ignores checking collecting
>>incorrectly).  If you find a slow protocol packet, you need to
>>chop it out of the array with memmove due to overlap.
>
>Agree.
>So to improve the bonding performance you need to optimize the aboves problems.
>There is no connection to the ring.
>
>And as I have described numerous times, these problems
>can't be easily fixed and preserve the existing API.

Sometimes we need to work harder to see a gain for all.
We should not apply a patch because it is easy and show a gain for specific scenarios.

>>> Bonding benefits from being able to read as much as possible (within limits of
>>>course, large reads would blow out caches) from each slave.
>>
>>The slaves PMDs can benefits in the same way.
>>
>>>It can't return all that data though because applications tend to use the 
>>>burst size that would be efficient for a typical PMD.
>>
>>What is the preferred burst size of the bonding? Maybe the application should use it when they are using bonding.
>>
>>The preferred burst size for bonding would be the sum of all the
>>slaves ideal read size.  However, that's not likely to be simple
>>since most applications decide early the size for the read/write
>>burst operations.
>> 
>>>An alternative might be to ask bonding applications to simply issue larger reads for
>>>certain modes.  That's probably not as easy as it sounds given the
>>>way that the burst length effects multiplexing.
>>
>>Can you explain it more?
>>
>>A longer burst size on one PMD will tend to favor that PMD
>>over others.  It will fill your internal queues with more 
>>of its packets.
>
>Agree, it's about fairness.
> 
>>
>>>Another solution might be just alternatively poll the individual
>>>slaves on each rx burst.  But that means you need to poll at a
>>>faster rate.  Depending on your application, you might not be
>>>able to do that.
>
>>Again, can you be more precise in the above explanation?
>>
>>If the application knows that there are two slaves backing
>>a bonding interface, the application could just read twice
>>from the bonding interface, knowing that the bonding 
>>interface is going to alternate between the slaves.  But
>>this requires the application to know things about the bonding
>>PMD, like the number of slaves.
>
>Why should the application poll twice?
>Poll slave 0, than process it's packets, poll slave 1 than process its packets...
>What is the problem?
>
>Because let's say that each slave is 10G and you are using
>
>link aggregation with two slaves.  If you have tuned your
>
>application on the assumption that a PMD is approximately
>10G, then you are going to be under polling the bonding PMD.
>For the above to work, you need to ensure that the application
>is polling sufficiently to keep up.

But each poll will be shorter, no slaves loops, only one slave call.

>>>  We can avoid this scheduling overhead by just
>>>doing the extra reads in bonding and buffering in a ring.
>>>
>>>Since bonding is going to be buffering everything in a ring,
>>
>>? I don't familiar with it. For now I don't think we need a ring.
>>
>>We have some empirical testing that shows performance improvements.
>>How do you explain this?  
>
>You are using a hack in bonding which hurt the bonding but improves the vectorized PMD you use.
>
>>Rings aren't inefficient.
>
>Only when there is a need to use it.
>
>I believe we have identified a need.
> 
>
>>There's significant overhead of calling the rx burst routine on any PMD.
>>You need to get the PMD data structures into local memory.
>
>Yes.
>
>>Reading as much as possible makes sense.
>
>Agree.
>
>> Could you generally do this for all PMDs?  Yes, but the ring adds latency.  Latency
>>that isn't an issue for the bonding PMD because of all the
>>other inefficiencies (for mode 4).
>
>Enlarging the bonding latency by that way makes some vectorized slave PMDs happy and makes the bonding worst 
>for other slaves PMDs - this is not a good idea to put it upstream.
>
>Improving the other problems in the bonding(reduce the bonding latency) will do the job for you and for others.
>
>Latency is not the issue with bonding.  The issue with bonding
>is the overhead associated with making an rx burst call.  We
>add latency (via rings) to make part of the bonding driver more
>efficient.
>
>Again, I suspect it even helps the non-vectorized PMDs.
 >Calling a PMD's rx burst routine is costly and we are switching between
>PMD's inside the bonding driver.  Bonding is halfway between an
>application and a PMD.  What we are doing in the bonding PMD is
>what an application would typically do.  But instead of forcing
>all the bonding users to do this, we are doing it for them.

Not agree.
The overhead in bonding may be critical for some other PMDs\scenario.

To summarize:
We are not agree.

Bottom line in my side,
It is not a good Idea to add overhead in the bonding PMD which is a generic PMD just to get gain for some specific scenarios in some specific PMDs while for other scenarios\PMDs it is bad.

Matan.