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
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]>
*Date: *Wednesday, May 11, 2022 at 8:10 AM
*To: *Mark Michelson <[email protected]>
*Cc: *Mary Manohar <[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.
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]>> 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]> wrote:
> From: Mary Manohar <[email protected]
<mailto:[email protected]>>
>
> Signed-off-by: Mary Manohar <[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]>
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=4M1akZ2SPpiYOkwcXOUTtZYo6SeEU7e0T0dg9b9E3E1ypfMSyt_QPOvkYzq20Pkr&s=go8izusrxnEFuOBTOkGitblsby6P-0arnWEq0ZkNJ2w&e=>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
