Hi Numan Sure, I have sent out a v3 with the updated commit message.
Thanks Mary From: Numan Siddique <[email protected]> Date: Wednesday, May 18, 2022 at 7:37 AM To: Mark Michelson <[email protected]> Cc: Mary Manohar <[email protected]>, Xavier Simonart <[email protected]>, [email protected] <[email protected]> Subject: Re: [ovs-dev] [PATCH ovn v2] Do not set 'up' attribute in the port-binding table if ovn-northd is at an older version. On Tue, May 17, 2022 at 9:55 AM Mark Michelson <[email protected]<mailto:[email protected]>> wrote: OK thanks for the explanations, Xavier and Mary. This is pretty important since we expect that ovn-controller should be upgraded before ovn-northd. I'm acking the patch because the content is correct. I suggest that when this is merged, the person that merges should expand on the commit message a bit to explain why the commit is necessary. If nothing else, make it clear that 20.09 is the cutoff between an "older" and "newer" version of ovn-northd in this case. Acked-by: Mark Michelson Thanks Mark for the review. @Mary - would you mind updating just the commit message here ? Thanks Numan On 5/12/22 11:59, Mary Manohar wrote: > Thanks Xavier for clarifying. > > Hi Mark, > > Xavier’s explanation is bang on. The ‘up’ field is introduced in the > Port Binding table post 20.09. > > So, when the southbound is at 20.09 version, the set operation fails in > ovn-controller. > > Thanks > > Mary > > *From: *Xavier Simonart <[email protected]<mailto:[email protected]>> > *Date: *Wednesday, May 11, 2022 at 8:10 AM > *To: *Mark Michelson <[email protected]<mailto:[email protected]>> > *Cc: *Mary Manohar > <[email protected]<mailto:[email protected]>>, > [email protected]<mailto:[email protected]> > <[email protected]<mailto:[email protected]>> > *Subject: *Re: [ovs-dev] [PATCH ovn v2] Do not set 'up' attribute in the > port-binding table if ovn-northd is at an older version. > > Hi Mark > > This is fixing a backward compatibility issue when using an older > ovn-northd. > > ovn-controller should only set Port_Binding.up field if the Southbound > DB is aware of this field (or transaction would fail). > > We already do this when setting pb up through notification (if-status > module). > > More explanations on commit 8b45fc9b2 from Dumitru. > > Thanks > > Xavier > > On Tue, May 10, 2022 at 8:12 PM Mark Michelson > <[email protected]<mailto:[email protected]> > <mailto:[email protected]<mailto:[email protected]>>> wrote: > > Hi Mary, > > I'm confused by this change. The summary mentions ovn-northd being > at an > older version, but there's no version check being performed in the > patch. Also, what constitutes an "older" version of ovn-northd? Why > shouldn't we set the port up in this case? What problem is being solved? > > On 5/9/22 15:28, [email protected]<mailto:[email protected]> > <mailto:[email protected]<mailto:[email protected]>> wrote: > > From: Mary Manohar > <[email protected]<mailto:[email protected]> > <mailto:[email protected]<mailto:[email protected]>>> > > > > Signed-off-by: Mary Manohar > <[email protected]<mailto:[email protected]> > <mailto:[email protected]<mailto:[email protected]>>> > > --- > > controller/binding.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/controller/binding.c b/controller/binding.c > > index e284704..e7dc537 100644 > > --- a/controller/binding.c > > +++ b/controller/binding.c > > @@ -908,7 +908,9 @@ claimed_lport_set_up(const struct > sbrec_port_binding *pb, > > if (!notify_up) { > > bool up = true; > > if (!parent_pb || (parent_pb->n_up && parent_pb->up[0])) { > > - sbrec_port_binding_set_up(pb, &up, 1); > > + if (pb->n_up) { > > + sbrec_port_binding_set_up(pb, &up, 1); > > + } > > } > > return; > > } > > > > _______________________________________________ > dev mailing list > [email protected]<mailto:[email protected]> > <mailto:[email protected]<mailto:[email protected]>> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > [mail.openvswitch.org]<https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwMFaQ&c=s883GpUCOChKOHiocYtGcg&r=PrJpP14VwAwIfuISqmhQn4UbXr0029-Ifb7y0YpWR_A&m=KmOoW5dfxKzbFrROo_PaGE36ZvSWHlCdMXuBsimXSH7E92h_RChtCJmRv5aCt_1G&s=HRZ3rnLuu5tvokmX59dIHek5OOjuxvG9NAoDbpkGTBM&e=> > [mail.openvswitch.org > [mail.openvswitch.org]<https://urldefense.proofpoint.com/v2/url?u=http-3A__mail.openvswitch.org&d=DwMFaQ&c=s883GpUCOChKOHiocYtGcg&r=PrJpP14VwAwIfuISqmhQn4UbXr0029-Ifb7y0YpWR_A&m=KmOoW5dfxKzbFrROo_PaGE36ZvSWHlCdMXuBsimXSH7E92h_RChtCJmRv5aCt_1G&s=qRZq7GsPn2LTWBePiH5z-Xd30QjTu_TNoZYZRuz9Wmw&e=>] > > <https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwMFaQ&c=s883GpUCOChKOHiocYtGcg&r=PrJpP14VwAwIfuISqmhQn4UbXr0029-Ifb7y0YpWR_A&m=4M1akZ2SPpiYOkwcXOUTtZYo6SeEU7e0T0dg9b9E3E1ypfMSyt_QPOvkYzq20Pkr&s=go8izusrxnEFuOBTOkGitblsby6P-0arnWEq0ZkNJ2w&e=> > _______________________________________________ dev mailing list [email protected]<mailto:[email protected]> https://mail.openvswitch.org/mailman/listinfo/ovs-dev [mail.openvswitch.org]<https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwMFaQ&c=s883GpUCOChKOHiocYtGcg&r=PrJpP14VwAwIfuISqmhQn4UbXr0029-Ifb7y0YpWR_A&m=KmOoW5dfxKzbFrROo_PaGE36ZvSWHlCdMXuBsimXSH7E92h_RChtCJmRv5aCt_1G&s=HRZ3rnLuu5tvokmX59dIHek5OOjuxvG9NAoDbpkGTBM&e=> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
