On 7/19/22 15:33, Mohammad Heib wrote:
> ovn-northd re-create sbrec row for lport of type
> virtual when the type explicitly updated in NBDB.
> 
> This approach was applied to handle container lport
> type updates and avoid handling issues in the ovn-controller,
> this patch expanded that approach to handle lport of type
> virtual in the same way.
> 
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2099288
> Fixes: cd3b685043fa (northd: handle container lport type update)
> Signed-off-by: Mohammad Heib <[email protected]>
> ---

Hi Mohammad,

Just a small comment below; with that addressed feel free to add my ack
to v2:

Acked-by: Dumitru Ceara <[email protected]>

>  northd/northd.c | 31 ++++++++++++++++++-------------
>  1 file changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/northd/northd.c b/northd/northd.c
> index d31cb1688..d587bc98d 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -1856,9 +1856,9 @@ localnet_can_learn_mac(const struct 
> nbrec_logical_switch_port *nbsp)
>  static bool
>  lsp_is_type_changed(const struct sbrec_port_binding *sb,
>                  const struct nbrec_logical_switch_port *nbsp,
> -                bool *is_old_container_lport)
> +                bool *update_sbrec)
>  {
> -    *is_old_container_lport = false;
> +    *update_sbrec = false;
>      if (!sb || !nbsp) {
>          return false;
>      }
> @@ -1870,13 +1870,19 @@ lsp_is_type_changed(const struct sbrec_port_binding 
> *sb,
>           */
>          if ((!sb->parent_port && nbsp->parent_name) ||
>                          (sb->parent_port && !nbsp->parent_name)) {
> -            *is_old_container_lport = true;
> +            *update_sbrec = true;
>              return true;
>          } else {
>              return false;
>          }
>      }
>  
> +    /* Cover cases where port changed to/from virtual port */
> +    if ((sb->type[0] && !strcmp(sb->type, "virtual"))||
> +            (nbsp->type[0] && !strcmp(nbsp->type, "virtual"))) {

There's no need to check for sb->type[0] and nbsp->type[0].

Also, there's a missing space before "||".

Thanks,
Dumitru

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to