| Message ID | 20221206152136.6531-1-amusil@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v3] controller: Check if MAC binding table has timestamp column | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Tue, Dec 6, 2022 at 10:21 AM Ales Musil <amusil@redhat.com> 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 <amusil@redhat.com> Acked-by: Numan Siddique <numans@ovn.org> 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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 12/7/22 18:29, Numan Siddique wrote: > On Tue, Dec 6, 2022 at 10:21 AM Ales Musil <amusil@redhat.com> 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 <amusil@redhat.com> > > Acked-by: Numan Siddique <numans@ovn.org> > > Numan > Thanks Ales and Numan! I pushed this to the main branch and backported it 22.12 and 22.09. Regards, Dumitru
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;
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 <amusil@redhat.com> --- 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(-)