Message ID | 20210915124340.1765-1-anton.ivanov@cambridgegreys.com |
---|---|
State | Accepted |
Headers | show |
Series | [ovs-dev,v8,1/3] northd: Disable parallel processing for logical_dp_groups | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
Hi Anton, For the series: Acked-by: Mark Michelson <mmichels@redhat.com> On 9/15/21 8:43 AM, anton.ivanov@cambridgegreys.com wrote: > From: Anton Ivanov <anton.ivanov@cambridgegreys.com> > > Work on improving processing with dp_groups enabled has > discovered that the locking mechanism presently in use > is not reliable. Disabling parallel processing if dp_groups > are enabled until the root cause is determined and fixed. > > Signed-off-by: Anton Ivanov <anton.ivanov@cambridgegreys.com> > --- > northd/ovn-northd.c | 2 +- > ovs | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index baaddb73e..3113fafc7 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -12974,7 +12974,7 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > } > } > > - if (use_parallel_build) { > + if (use_parallel_build && (!use_logical_dp_groups)) { > struct hmap *lflow_segs; > struct lswitch_flow_build_info *lsiv; > int index; > diff --git a/ovs b/ovs > index 748010ff3..50e5523b9 160000 > --- a/ovs > +++ b/ovs > @@ -1 +1 @@ > -Subproject commit 748010ff304b7cd2c43f4eb98a554433f0df07f9 > +Subproject commit 50e5523b9b2b154e5fafc5acdcdec85e9cc5a330 >
On 9/15/21 14:43, anton.ivanov@cambridgegreys.com wrote: > From: Anton Ivanov <anton.ivanov@cambridgegreys.com> > > Work on improving processing with dp_groups enabled has > discovered that the locking mechanism presently in use > is not reliable. Disabling parallel processing if dp_groups > are enabled until the root cause is determined and fixed. > > Signed-off-by: Anton Ivanov <anton.ivanov@cambridgegreys.com> > --- > northd/ovn-northd.c | 2 +- > ovs | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index baaddb73e..3113fafc7 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -12974,7 +12974,7 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > } > } > > - if (use_parallel_build) { > + if (use_parallel_build && (!use_logical_dp_groups)) { > struct hmap *lflow_segs; > struct lswitch_flow_build_info *lsiv; > int index; > diff --git a/ovs b/ovs > index 748010ff3..50e5523b9 160000 > --- a/ovs > +++ b/ovs > @@ -1 +1 @@ > -Subproject commit 748010ff304b7cd2c43f4eb98a554433f0df07f9 > +Subproject commit 50e5523b9b2b154e5fafc5acdcdec85e9cc5a330 > This change breaks the build due to submodule downgrade. Best regards, Ilya Maximets.
On 17/09/2021 18:23, Mark Michelson wrote: > Hi Anton, > > For the series: > > Acked-by: Mark Michelson <mmichels@redhat.com> Thanks. > > On 9/15/21 8:43 AM, anton.ivanov@cambridgegreys.com wrote: >> From: Anton Ivanov <anton.ivanov@cambridgegreys.com> >> >> Work on improving processing with dp_groups enabled has >> discovered that the locking mechanism presently in use >> is not reliable. Disabling parallel processing if dp_groups >> are enabled until the root cause is determined and fixed. >> >> Signed-off-by: Anton Ivanov <anton.ivanov@cambridgegreys.com> >> --- >> northd/ovn-northd.c | 2 +- >> ovs | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c >> index baaddb73e..3113fafc7 100644 >> --- a/northd/ovn-northd.c >> +++ b/northd/ovn-northd.c >> @@ -12974,7 +12974,7 @@ build_lswitch_and_lrouter_flows(struct hmap >> *datapaths, struct hmap *ports, >> } >> } >> - if (use_parallel_build) { >> + if (use_parallel_build && (!use_logical_dp_groups)) { >> struct hmap *lflow_segs; >> struct lswitch_flow_build_info *lsiv; >> int index; >> diff --git a/ovs b/ovs >> index 748010ff3..50e5523b9 160000 As noted by Ilia I had a submodule change from ovs filter through, so I will resend the series. >> --- a/ovs >> +++ b/ovs >> @@ -1 +1 @@ >> -Subproject commit 748010ff304b7cd2c43f4eb98a554433f0df07f9 >> +Subproject commit 50e5523b9b2b154e5fafc5acdcdec85e9cc5a330 >> > >
On 9/17/21 1:38 PM, Ilya Maximets wrote: > On 9/15/21 14:43, anton.ivanov@cambridgegreys.com wrote: >> From: Anton Ivanov <anton.ivanov@cambridgegreys.com> >> >> Work on improving processing with dp_groups enabled has >> discovered that the locking mechanism presently in use >> is not reliable. Disabling parallel processing if dp_groups >> are enabled until the root cause is determined and fixed. >> >> Signed-off-by: Anton Ivanov <anton.ivanov@cambridgegreys.com> >> --- >> northd/ovn-northd.c | 2 +- >> ovs | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c >> index baaddb73e..3113fafc7 100644 >> --- a/northd/ovn-northd.c >> +++ b/northd/ovn-northd.c >> @@ -12974,7 +12974,7 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports, >> } >> } >> >> - if (use_parallel_build) { >> + if (use_parallel_build && (!use_logical_dp_groups)) { >> struct hmap *lflow_segs; >> struct lswitch_flow_build_info *lsiv; >> int index; >> diff --git a/ovs b/ovs >> index 748010ff3..50e5523b9 160000 >> --- a/ovs >> +++ b/ovs >> @@ -1 +1 @@ >> -Subproject commit 748010ff304b7cd2c43f4eb98a554433f0df07f9 >> +Subproject commit 50e5523b9b2b154e5fafc5acdcdec85e9cc5a330 >> > > This change breaks the build due to submodule downgrade. > > Best regards, Ilya Maximets. > Thanks for the note, Ilya. I did a manual rebase (which was non-trivial due to the northd split) and pushed to my fork. Seems to be OK with the submodule downgrade fixed: https://github.com/putnopvut/ovn/runs/3635759940?check_suite_focus=true
Based on the successful GHA run I had in my fork, I fixed the submodule downgrade (and fixed a couple of grammar errors in comments). I've pushed this to master. On 9/17/21 3:49 PM, Mark Michelson wrote: > On 9/17/21 1:38 PM, Ilya Maximets wrote: >> On 9/15/21 14:43, anton.ivanov@cambridgegreys.com wrote: >>> From: Anton Ivanov <anton.ivanov@cambridgegreys.com> >>> >>> Work on improving processing with dp_groups enabled has >>> discovered that the locking mechanism presently in use >>> is not reliable. Disabling parallel processing if dp_groups >>> are enabled until the root cause is determined and fixed. >>> >>> Signed-off-by: Anton Ivanov <anton.ivanov@cambridgegreys.com> >>> --- >>> northd/ovn-northd.c | 2 +- >>> ovs | 2 +- >>> 2 files changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c >>> index baaddb73e..3113fafc7 100644 >>> --- a/northd/ovn-northd.c >>> +++ b/northd/ovn-northd.c >>> @@ -12974,7 +12974,7 @@ build_lswitch_and_lrouter_flows(struct hmap >>> *datapaths, struct hmap *ports, >>> } >>> } >>> - if (use_parallel_build) { >>> + if (use_parallel_build && (!use_logical_dp_groups)) { >>> struct hmap *lflow_segs; >>> struct lswitch_flow_build_info *lsiv; >>> int index; >>> diff --git a/ovs b/ovs >>> index 748010ff3..50e5523b9 160000 >>> --- a/ovs >>> +++ b/ovs >>> @@ -1 +1 @@ >>> -Subproject commit 748010ff304b7cd2c43f4eb98a554433f0df07f9 >>> +Subproject commit 50e5523b9b2b154e5fafc5acdcdec85e9cc5a330 >>> >> >> This change breaks the build due to submodule downgrade. >> >> Best regards, Ilya Maximets. >> > > Thanks for the note, Ilya. I did a manual rebase (which was non-trivial > due to the northd split) and pushed to my fork. Seems to be OK with the > submodule downgrade fixed: > https://github.com/putnopvut/ovn/runs/3635759940?check_suite_focus=true
On 17/09/2021 21:44, Mark Michelson wrote: > Based on the successful GHA run I had in my fork, I fixed the > submodule downgrade (and fixed a couple of grammar errors in > comments). I've pushed this to master. Thanks, I will rebase the thread API improvements on top of that. A. > > On 9/17/21 3:49 PM, Mark Michelson wrote: >> On 9/17/21 1:38 PM, Ilya Maximets wrote: >>> On 9/15/21 14:43, anton.ivanov@cambridgegreys.com wrote: >>>> From: Anton Ivanov <anton.ivanov@cambridgegreys.com> >>>> >>>> Work on improving processing with dp_groups enabled has >>>> discovered that the locking mechanism presently in use >>>> is not reliable. Disabling parallel processing if dp_groups >>>> are enabled until the root cause is determined and fixed. >>>> >>>> Signed-off-by: Anton Ivanov <anton.ivanov@cambridgegreys.com> >>>> --- >>>> northd/ovn-northd.c | 2 +- >>>> ovs | 2 +- >>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c >>>> index baaddb73e..3113fafc7 100644 >>>> --- a/northd/ovn-northd.c >>>> +++ b/northd/ovn-northd.c >>>> @@ -12974,7 +12974,7 @@ build_lswitch_and_lrouter_flows(struct hmap >>>> *datapaths, struct hmap *ports, >>>> } >>>> } >>>> - if (use_parallel_build) { >>>> + if (use_parallel_build && (!use_logical_dp_groups)) { >>>> struct hmap *lflow_segs; >>>> struct lswitch_flow_build_info *lsiv; >>>> int index; >>>> diff --git a/ovs b/ovs >>>> index 748010ff3..50e5523b9 160000 >>>> --- a/ovs >>>> +++ b/ovs >>>> @@ -1 +1 @@ >>>> -Subproject commit 748010ff304b7cd2c43f4eb98a554433f0df07f9 >>>> +Subproject commit 50e5523b9b2b154e5fafc5acdcdec85e9cc5a330 >>>> >>> >>> This change breaks the build due to submodule downgrade. >>> >>> Best regards, Ilya Maximets. >>> >> >> Thanks for the note, Ilya. I did a manual rebase (which was >> non-trivial due to the northd split) and pushed to my fork. Seems to >> be OK with the submodule downgrade fixed: >> https://github.com/putnopvut/ovn/runs/3635759940?check_suite_focus=true > >
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index baaddb73e..3113fafc7 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -12974,7 +12974,7 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports, } } - if (use_parallel_build) { + if (use_parallel_build && (!use_logical_dp_groups)) { struct hmap *lflow_segs; struct lswitch_flow_build_info *lsiv; int index; diff --git a/ovs b/ovs index 748010ff3..50e5523b9 160000 --- a/ovs +++ b/ovs @@ -1 +1 @@ -Subproject commit 748010ff304b7cd2c43f4eb98a554433f0df07f9 +Subproject commit 50e5523b9b2b154e5fafc5acdcdec85e9cc5a330