On Tue, Dec 6, 2022 at 10:21 AM Ales Musil <[email protected]> wrote:
>
> In order to keep backward compatibility with northd we need
> to check if MAC binding table actually has the timestamp column.
>
> Reported-at: https://bugzilla.redhat.com/2151066
> Signed-off-by: Ales Musil <[email protected]>

Acked-by: Numan Siddique <[email protected]>

Numan

> ---
> v2: Address comments from Dumitru.
> v3: Address comments from Dumitru.
> ---
>  controller/ovn-controller.c |  1 +
>  controller/pinctrl.c        | 38 ++++++++++++++++++++++++++++++-------
>  controller/pinctrl.h        |  4 +++-
>  3 files changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index d6251afb8..f0fd24820 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -4615,6 +4615,7 @@ main(int argc, char *argv[])
>                          }
>                          stopwatch_start(PINCTRL_RUN_STOPWATCH_NAME,
>                                          time_msec());
> +                        pinctrl_update(ovnsb_idl_loop.idl, br_int->name);
>                          pinctrl_run(ovnsb_idl_txn,
>                                      sbrec_datapath_binding_by_key,
>                                      sbrec_port_binding_by_datapath,
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index f44775c7e..82da6ae73 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -173,6 +173,7 @@ struct pinctrl {
>      pthread_t pinctrl_thread;
>      /* Latch to destroy the 'pinctrl_thread' */
>      struct latch pinctrl_thread_exit;
> +    bool mac_binding_can_timestamp;
>  };
>
>  static struct pinctrl pinctrl;
> @@ -544,6 +545,7 @@ pinctrl_init(void)
>      bfd_monitor_init();
>      init_fdb_entries();
>      pinctrl.br_int_name = NULL;
> +    pinctrl.mac_binding_can_timestamp = false;
>      pinctrl_handler_seq = seq_create();
>      pinctrl_main_seq = seq_create();
>
> @@ -3519,7 +3521,7 @@ pinctrl_handler(void *arg_)
>  }
>
>  static void
> -pinctrl_set_br_int_name_(char *br_int_name)
> +pinctrl_set_br_int_name_(const char *br_int_name)
>      OVS_REQUIRES(pinctrl_mutex)
>  {
>      if (br_int_name && (!pinctrl.br_int_name || strcmp(pinctrl.br_int_name,
> @@ -3533,13 +3535,31 @@ pinctrl_set_br_int_name_(char *br_int_name)
>  }
>
>  void
> -pinctrl_set_br_int_name(char *br_int_name)
> +pinctrl_set_br_int_name(const char *br_int_name)
>  {
>      ovs_mutex_lock(&pinctrl_mutex);
>      pinctrl_set_br_int_name_(br_int_name);
>      ovs_mutex_unlock(&pinctrl_mutex);
>  }
>
> +void
> +pinctrl_update(const struct ovsdb_idl *idl, const char *br_int_name)
> +{
> +    ovs_mutex_lock(&pinctrl_mutex);
> +    pinctrl_set_br_int_name_(br_int_name);
> +
> +    bool can_timestamp = 
> sbrec_server_has_mac_binding_table_col_timestamp(idl);
> +    if (can_timestamp != pinctrl.mac_binding_can_timestamp) {
> +        pinctrl.mac_binding_can_timestamp = can_timestamp;
> +
> +        /* Notify pinctrl_handler that mac binding timestamp column
> +         * availability has changed. */
> +        notify_pinctrl_handler();
> +    }
> +
> +    ovs_mutex_unlock(&pinctrl_mutex);
> +}
> +
>  /* Called by ovn-controller. */
>  void
>  pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> @@ -3563,7 +3583,6 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
>              const struct shash *local_active_ports_ras)
>  {
>      ovs_mutex_lock(&pinctrl_mutex);
> -    pinctrl_set_br_int_name_(br_int->name);
>      run_put_mac_bindings(ovnsb_idl_txn, sbrec_datapath_binding_by_key,
>                           sbrec_port_binding_by_key,
>                           sbrec_mac_binding_by_lport_ip);
> @@ -4245,12 +4264,17 @@ mac_binding_add_to_sb(struct ovsdb_idl_txn 
> *ovnsb_idl_txn,
>          b = sbrec_mac_binding_insert(ovnsb_idl_txn);
>          sbrec_mac_binding_set_logical_port(b, logical_port);
>          sbrec_mac_binding_set_ip(b, ip);
> -        sbrec_mac_binding_set_mac(b, mac_string);
>          sbrec_mac_binding_set_datapath(b, dp);
> -        sbrec_mac_binding_set_timestamp(b, time_wall_msec());
> -    } else if (strcmp(b->mac, mac_string)) {
> +    }
> +
> +    if (strcmp(b->mac, mac_string)) {
>          sbrec_mac_binding_set_mac(b, mac_string);
> -        sbrec_mac_binding_set_timestamp(b, time_wall_msec());
> +
> +        /* For backward compatibility check if timestamp column is available
> +         * in SB DB. */
> +        if (pinctrl.mac_binding_can_timestamp) {
> +            sbrec_mac_binding_set_timestamp(b, time_wall_msec());
> +        }
>      }
>  }
>
> diff --git a/controller/pinctrl.h b/controller/pinctrl.h
> index d4f52e94d..cfece04da 100644
> --- a/controller/pinctrl.h
> +++ b/controller/pinctrl.h
> @@ -26,6 +26,7 @@
>  struct hmap;
>  struct shash;
>  struct lport_index;
> +struct ovsdb_idl;
>  struct ovsdb_idl_index;
>  struct ovsdb_idl_txn;
>  struct ovsrec_bridge;
> @@ -57,7 +58,8 @@ void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
>                   const struct shash *local_active_ports_ras);
>  void pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn);
>  void pinctrl_destroy(void);
> -void pinctrl_set_br_int_name(char *br_int_name);
> +void pinctrl_set_br_int_name(const char *br_int_name);
> +void pinctrl_update(const struct ovsdb_idl *idl, const char *br_int_name);
>
>  struct activated_port {
>      uint32_t dp_key;
> --
> 2.38.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to