On 15/07/2020 11:04, Dumitru Ceara wrote:
On 7/15/20 11:54 AM, Anton Ivanov wrote:
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.iva...@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.iva...@cambridgegreys.com/
https://patchwork.ozlabs.org/project/openvswitch/patch/20200706083650.29443-3-anton.iva...@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
d...@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.
I'll let other people comment on this too. From my perspective though,
if the commit is relatively small, e.g., introducing only one refactored
function at a time, it's desired to have the correct indentation from
the beginning.
The full patch sequence is on the list.
It should be significantly faster than the original one too. I will try to run
some performance tests on it overnight.
A.
Regards,
Dumitru
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
