[ovs-dev] RFC DEMO of using parallel processing in OVN

[Anton Ivanov]

On 15/07/2020 09:57, Dumitru Ceara wrote:
> On 7/14/20 6:06 PM, Anton Ivanov wrote:
>> On 14/07/2020 16:27, Dumitru Ceara wrote:
>>> On 7/10/20 10:39 AM, anton.ivanov at cambridgegreys.com wrote:
>>>> Hi all,
>>>>
>>>> This is a series of patches to demo the use in OVN of the parallel
>>>> processing of hashes patchset which I submitted to OVS.
>>>>
>>>> The OVS patch series required for this patch can be found here:
>>>>
>>>> https://patchwork.ozlabs.org/project/openvswitch/patch/20200706083650.29443-2-anton.ivanov@cambridgegreys.com/
>>>>
>>>> https://patchwork.ozlabs.org/project/openvswitch/patch/20200706083650.29443-3-anton.ivanov@cambridgegreys.com/
>>>>
>>>>
>>>> or here:
>>>>
>>>> https://github.com/kot-begemot-uk/ovs/tree/parallel-submit-final
>>>>
>>>> They improve the performance and stability of ovn in a scale test.
>>>> I have used 64+ fake nodes in my scale testing. Your mileage may
>>>> vary depending on the performance of the systems used to run the tests.
>>>>
>>>> The patchset covers most parts of ovn-northd which look amenable to
>>>> parallel processing. It may be possible to expand it to also cover
>>>> initial datapath hash generation and sb_only/nb_only lists, but that
>>>> does not look like it is worth it except for extremely large datasets.
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> dev mailing list
>>>> dev at openvswitch.org
>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>>
>>> Hi Anton,
>>>
>>> Would you mind rebasing and sending a v2 of this RFC? For some reason
>>> the emails for the first two patches don't show up in patchwork:
>> I think I know what it is. I will try to find it and fix it in the next
>> revision. There is something somewhere which is not ASCII clean and I
>> had UTF8 warnings for these.
>>
> Hi Anton,
>
> Ok. There might be more issues though. If I patch your first patch
> manually, the second patch doesn't apply cleanly. Patch 3 applies ok
> afterwards but #4 fails.
>
>>> https://patchwork.ozlabs.org/project/openvswitch/list/?series=188809
>>>
>>> Also, there were changes in OVN master in the meantime and trying to
>>> apply your changes manually also fails due to conflicts in quite a few
>>> places.
>> That unfortunately will be the case with the next version after a few
>> commits to master.
>>
>> As long as the code follows the current layout, any version which tries
>> to re-organize processing will be very painful to rebase.
>>
> I agree, unfortunately I don't know what we can do to avoid that right now.
>
>> It may be a good idea to apply something like patches 1 from this series
>> (the one that got lost). It splits out the sequential code in the
>> lswitch and lrouter lflow build into a sequence of procedures.
> Sounds good, let's try that. To make that happen it's better to send the
> series as non-rfc and mention explicitly in the cover letter that the
> first patch doesn't depend on any unapplied OVS patches. Like that, once
> acked, patch 1 could be applied on its own.
>
>> This makes any future changes much easier to merge.
>>
> To save a round of reviews, I have some comments on patch #1. It might
> be good to address them before sending a new version of the series.
>
> Also I think it might make review easier if patch #1 is split in
> multiple smaller patches (that would get applied together), one per
> function. Right now it's quite hard to compare the old version with the
> new one.
>
> Regarding patch #4, I think it should be split in (at least) two parts:
> - refactoring of lflow generation for logical switches: this should
> happen in the beginning of the series like for logical router flows.
> - parallelization of the lflow generation.

I am going to split the whole sequence in much smaller chunks and I will deliberately preserve the existing indent.

One of the reasons why git cannot figure out what is what and merge anything is that moving code to per-stage functions will change the indent one step to the left. This marks the whole block as a change for git.

If the original indent is kept temporarily "as is" (despite this being an obvious style violation), the diffs should be become possible to rebase.

So I apologize in advance for the next series being "ugly". If we decide to use this and it is merged we can always tidy up the indents at that point.

>
> Thanks,
> Dumitru
>
>
-- 
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/