On 12/14/22 16:28, Mark Michelson wrote:
> This looks good to me. Thanks Dumitru!
>
> Acked-by: Mark Michelson <[email protected]>
>
Thanks!
> Just to double-check, this is a candidate for main and branch-22.12
> only, right?
That's what I thought initially. That's because on branches < 22.12 we
didn't add new tables that need conditional monitoring.
Just to be on the safe side we could do a custom backport for other
branches. What do you think?
>
> On 12/14/22 08:17, Dumitru Ceara wrote:
>> Setting the monitor condition on a table that's not part of the SB
>> schema triggers an error from the server side, a downgrade to monitor v0
>> (from v2) and also causes the conditional monitoring sequence numbers to
>> go out of sync.
>>
>> Use the <DB>rec_server_has_<TABLE>_table() API to determine if a table
>> is present in the server schema and therefore usable in the monitor
>> condition.
>>
>> This commit also bumps the OVS submodule because the following fix is
>> needed:
>> a787fbbf9dd6 ("ovsdb-cs: Consider default conditions implicitly
>> acked.")
>>
>> We also pick up the following relevant OVS changes:
>> b8bf410a5c94 db-ctl-base: Use partial map/set updates for last
>> add/set commands.
>> 55b9507e6824 ovsdb-idl: Add the support to specify the uuid for row
>> insert.
>>
>> Reported-by: Numan Siddique <[email protected]>
>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2151063
>> Signed-off-by: Dumitru Ceara <[email protected]>
>> ---
>> Note: this should be backported to branch-22.12 where we introduced the
>> new Chassis_Template_Var table for which we need conditional monitoring.
>> ---
>> controller/ovn-controller.c | 30 +++++++++++++++++++-----------
>> ovs | 2 +-
>> utilities/ovn-dbctl.c | 2 +-
>> utilities/ovn-ic-nbctl.c | 7 ++++---
>> utilities/ovn-ic-sbctl.c | 7 ++++---
>> 5 files changed, 29 insertions(+), 19 deletions(-)
>>
>> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
>> index f0fd248204..1aca13e666 100644
>> --- a/controller/ovn-controller.c
>> +++ b/controller/ovn-controller.c
>> @@ -153,6 +153,14 @@ struct pending_pkt {
>> /* Registered ofctrl seqno type for nb_cfg propagation. */
>> static size_t ofctrl_seq_type_nb_cfg;
>> +/* Only set monitor conditions on tables that are available in the
>> + * server schema.
>> + */
>> +#define sb_table_set_monitor_condition(idl, table, cond) \
>> + (sbrec_server_has_##table##_table(idl) \
>> + ? sbrec_##table##_set_condition(idl, cond) \
>> + : 0)
>> +
>> static unsigned int
>> update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
>> const struct sbrec_chassis *chassis,
>> @@ -288,17 +296,17 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
>> out:;
>> unsigned int cond_seqnos[] = {
>> - sbrec_port_binding_set_condition(ovnsb_idl, &pb),
>> - sbrec_logical_flow_set_condition(ovnsb_idl, &lf),
>> - sbrec_logical_dp_group_set_condition(ovnsb_idl, &ldpg),
>> - sbrec_mac_binding_set_condition(ovnsb_idl, &mb),
>> - sbrec_multicast_group_set_condition(ovnsb_idl, &mg),
>> - sbrec_dns_set_condition(ovnsb_idl, &dns),
>> - sbrec_controller_event_set_condition(ovnsb_idl, &ce),
>> - sbrec_ip_multicast_set_condition(ovnsb_idl, &ip_mcast),
>> - sbrec_igmp_group_set_condition(ovnsb_idl, &igmp),
>> - sbrec_chassis_private_set_condition(ovnsb_idl, &chprv),
>> - sbrec_chassis_template_var_set_condition(ovnsb_idl, &tv),
>> + sb_table_set_monitor_condition(ovnsb_idl, port_binding, &pb),
>> + sb_table_set_monitor_condition(ovnsb_idl, logical_flow, &lf),
>> + sb_table_set_monitor_condition(ovnsb_idl, logical_dp_group,
>> &ldpg),
>> + sb_table_set_monitor_condition(ovnsb_idl, mac_binding, &mb),
>> + sb_table_set_monitor_condition(ovnsb_idl, multicast_group, &mg),
>> + sb_table_set_monitor_condition(ovnsb_idl, dns, &dns),
>> + sb_table_set_monitor_condition(ovnsb_idl, controller_event,
>> &ce),
>> + sb_table_set_monitor_condition(ovnsb_idl, ip_multicast,
>> &ip_mcast),
>> + sb_table_set_monitor_condition(ovnsb_idl, igmp_group, &igmp),
>> + sb_table_set_monitor_condition(ovnsb_idl, chassis_private,
>> &chprv),
>> + sb_table_set_monitor_condition(ovnsb_idl,
>> chassis_template_var, &tv),
>> };
>> unsigned int expected_cond_seqno = 0;
>> diff --git a/ovs b/ovs
>> index bb9fedb79a..a787fbbf9d 160000
>> --- a/ovs
>> +++ b/ovs
>> @@ -1 +1 @@
>> -Subproject commit bb9fedb79af8df5f14922ae588866314a0e31bf5
>> +Subproject commit a787fbbf9dd6a108a53053afb45fb59a0b58b514
>> diff --git a/utilities/ovn-dbctl.c b/utilities/ovn-dbctl.c
>> index 7ee0b92b43..a850c2f317 100644
>> --- a/utilities/ovn-dbctl.c
>> +++ b/utilities/ovn-dbctl.c
>> @@ -721,7 +721,7 @@ do_dbctl(const struct ovn_dbctl_options
>> *dbctl_options,
>> ctl_context_init(ctx, NULL, idl, txn, symtab, NULL);
>> for (size_t i = 0; i < n_commands; i++) {
>> struct ctl_command *c = &commands[i];
>> - ctl_context_init_command(ctx, c);
>> + ctl_context_init_command(ctx, c, c == &commands[n_commands -
>> 1]);
>> if (c->syntax->run) {
>> (c->syntax->run)(ctx);
>> }
>> diff --git a/utilities/ovn-ic-nbctl.c b/utilities/ovn-ic-nbctl.c
>> index be294a7be0..63550d492f 100644
>> --- a/utilities/ovn-ic-nbctl.c
>> +++ b/utilities/ovn-ic-nbctl.c
>> @@ -675,9 +675,9 @@ static const struct ctl_table_class
>> tables[ICNBREC_N_TABLES] = {
>>
>> static void
>> ic_nbctl_context_init_command(struct ic_nbctl_context *ic_nbctl_ctx,
>> - struct ctl_command *command)
>> + struct ctl_command *command, bool
>> last_command)
>> {
>> - ctl_context_init_command(&ic_nbctl_ctx->base, command);
>> + ctl_context_init_command(&ic_nbctl_ctx->base, command,
>> last_command);
>> }
>> static void
>> @@ -762,7 +762,8 @@ do_ic_nbctl(const char *args, struct ctl_command
>> *commands, size_t n_commands,
>> }
>> ic_nbctl_context_init(&ic_nbctl_ctx, NULL, idl, txn, symtab);
>> for (c = commands; c < &commands[n_commands]; c++) {
>> - ic_nbctl_context_init_command(&ic_nbctl_ctx, c);
>> + ic_nbctl_context_init_command(&ic_nbctl_ctx, c,
>> + c == &commands[n_commands - 1]);
>> if (c->syntax->run) {
>> (c->syntax->run)(&ic_nbctl_ctx.base);
>> }
>> diff --git a/utilities/ovn-ic-sbctl.c b/utilities/ovn-ic-sbctl.c
>> index 64d1d469e8..39fbb83057 100644
>> --- a/utilities/ovn-ic-sbctl.c
>> +++ b/utilities/ovn-ic-sbctl.c
>> @@ -747,9 +747,9 @@ static const struct ctl_table_class
>> tables[ICSBREC_N_TABLES] = {
>>
>> static void
>> ic_sbctl_context_init_command(struct ic_sbctl_context *ic_sbctl_ctx,
>> - struct ctl_command *command)
>> + struct ctl_command *command, bool
>> last_command)
>> {
>> - ctl_context_init_command(&ic_sbctl_ctx->base, command);
>> + ctl_context_init_command(&ic_sbctl_ctx->base, command,
>> last_command);
>> }
>> static void
>> @@ -833,7 +833,8 @@ do_ic_sbctl(const char *args, struct ctl_command
>> *commands, size_t n_commands,
>> }
>> ic_sbctl_context_init(&ic_sbctl_ctx, NULL, idl, txn, symtab);
>> for (c = commands; c < &commands[n_commands]; c++) {
>> - ic_sbctl_context_init_command(&ic_sbctl_ctx, c);
>> + ic_sbctl_context_init_command(&ic_sbctl_ctx, c,
>> + c == &commands[n_commands - 1]);
>> if (c->syntax->run) {
>> (c->syntax->run)(&ic_sbctl_ctx.base);
>> }
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
