| Message ID | 20240130210810.548338-1-mmichels@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v2,1/3] rbac: Only allow relevant chassis to update service monitors. | 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, Jan 30, 2024 at 10:08 PM Mark Michelson <mmichels@redhat.com> wrote: > Service monitors already had the restriction that chassis could not > insert or delete records. However, there was nothing restricting chassis > from updating records for service monitors that are relevant to other > chassis. > > This change adds a new "chassis_name" column to the Service_Monitor > table. ovn-northd will set this column to the chassis on which the > relevant logical port is bound. This way, only that particular chassis > can update the status of the service monitor. > > Signed-off-by: Mark Michelson <mmichels@redhat.com> > --- > v1 -> v2: > * Rebased on top of currrent main > --- > northd/northd.c | 19 +++++++++++++++++-- > northd/ovn-northd.c | 2 +- > ovn-sb.ovsschema | 5 +++-- > ovn-sb.xml | 4 ++++ > 4 files changed, 25 insertions(+), 5 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index d2091d4bc..2a2fab231 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -3799,13 +3799,19 @@ static struct service_monitor_info * > create_or_get_service_mon(struct ovsdb_idl_txn *ovnsb_txn, > struct hmap *monitor_map, > const char *ip, const char *logical_port, > - uint16_t service_port, const char *protocol) > + uint16_t service_port, const char *protocol, > + const char *chassis_name) > { > struct service_monitor_info *mon_info = > get_service_mon(monitor_map, ip, logical_port, service_port, > protocol); > > if (mon_info) { > + if (chassis_name && strcmp(mon_info->sbrec_mon->chassis_name, > + chassis_name)) { > + sbrec_service_monitor_set_chassis_name(mon_info->sbrec_mon, > + chassis_name); > + } > return mon_info; > } > > @@ -3820,6 +3826,9 @@ create_or_get_service_mon(struct ovsdb_idl_txn > *ovnsb_txn, > sbrec_service_monitor_set_port(sbrec_mon, service_port); > sbrec_service_monitor_set_logical_port(sbrec_mon, logical_port); > sbrec_service_monitor_set_protocol(sbrec_mon, protocol); > + if (chassis_name) { > + sbrec_service_monitor_set_chassis_name(sbrec_mon, chassis_name); > + } > mon_info = xzalloc(sizeof *mon_info); > mon_info->sbrec_mon = sbrec_mon; > hmap_insert(monitor_map, &mon_info->hmap_node, hash); > @@ -3862,12 +3871,18 @@ ovn_lb_svc_create(struct ovsdb_idl_txn *ovnsb_txn, > protocol = "tcp"; > } > > + const char *chassis_name = NULL; > + if (op->sb && op->sb->chassis) { > + chassis_name = op->sb->chassis->name; > + } > + > struct service_monitor_info *mon_info = > create_or_get_service_mon(ovnsb_txn, monitor_map, > backend->ip_str, > backend_nb->logical_port, > backend->port, > - protocol); > + protocol, > + chassis_name); > ovs_assert(mon_info); > sbrec_service_monitor_set_options( > mon_info->sbrec_mon, > &lb_vip_nb->lb_health_check->options); > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index dadc1af38..c32a11cbd 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -114,7 +114,7 @@ static const char *rbac_mac_binding_update[] = > {"logical_port", "ip", "mac", "datapath", "timestamp"}; > > static const char *rbac_svc_monitor_auth[] = > - {""}; > + {"chassis_name"}; > static const char *rbac_svc_monitor_auth_update[] = > {"status"}; > static const char *rbac_igmp_group_auth[] = > diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema > index 72e230b75..1d2b3028d 100644 > --- a/ovn-sb.ovsschema > +++ b/ovn-sb.ovsschema > @@ -1,7 +1,7 @@ > { > "name": "OVN_Southbound", > - "version": "20.30.0", > - "cksum": "2972392849 31172", > + "version": "20.31.0", > + "cksum": "2473562445 31224", > "tables": { > "SB_Global": { > "columns": { > @@ -509,6 +509,7 @@ > "logical_port": {"type": "string"}, > "src_mac": {"type": "string"}, > "src_ip": {"type": "string"}, > + "chassis_name": {"type": "string"}, > "status": { > "type": {"key": {"type": "string", > "enum": ["set", ["online", "offline", > "error"]]}, > diff --git a/ovn-sb.xml b/ovn-sb.xml > index e393f92b3..1f3b318e0 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -4815,6 +4815,10 @@ tcp.flags = RST; > Source IPv4 address to use in the service monitor packet. > </column> > > + <column name="chassis_name"> > + The name of the chassis where the logical port is bound. > + </column> > + > <column name="options" key="interval" type='{"type": "integer"}'> > The interval, in seconds, between service monitor checks. > </column> > -- > 2.43.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Looks good to me, thanks. Acked-by: Ales Musil <amusil@redhat.com>
diff --git a/northd/northd.c b/northd/northd.c index d2091d4bc..2a2fab231 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -3799,13 +3799,19 @@ static struct service_monitor_info * create_or_get_service_mon(struct ovsdb_idl_txn *ovnsb_txn, struct hmap *monitor_map, const char *ip, const char *logical_port, - uint16_t service_port, const char *protocol) + uint16_t service_port, const char *protocol, + const char *chassis_name) { struct service_monitor_info *mon_info = get_service_mon(monitor_map, ip, logical_port, service_port, protocol); if (mon_info) { + if (chassis_name && strcmp(mon_info->sbrec_mon->chassis_name, + chassis_name)) { + sbrec_service_monitor_set_chassis_name(mon_info->sbrec_mon, + chassis_name); + } return mon_info; } @@ -3820,6 +3826,9 @@ create_or_get_service_mon(struct ovsdb_idl_txn *ovnsb_txn, sbrec_service_monitor_set_port(sbrec_mon, service_port); sbrec_service_monitor_set_logical_port(sbrec_mon, logical_port); sbrec_service_monitor_set_protocol(sbrec_mon, protocol); + if (chassis_name) { + sbrec_service_monitor_set_chassis_name(sbrec_mon, chassis_name); + } mon_info = xzalloc(sizeof *mon_info); mon_info->sbrec_mon = sbrec_mon; hmap_insert(monitor_map, &mon_info->hmap_node, hash); @@ -3862,12 +3871,18 @@ ovn_lb_svc_create(struct ovsdb_idl_txn *ovnsb_txn, protocol = "tcp"; } + const char *chassis_name = NULL; + if (op->sb && op->sb->chassis) { + chassis_name = op->sb->chassis->name; + } + struct service_monitor_info *mon_info = create_or_get_service_mon(ovnsb_txn, monitor_map, backend->ip_str, backend_nb->logical_port, backend->port, - protocol); + protocol, + chassis_name); ovs_assert(mon_info); sbrec_service_monitor_set_options( mon_info->sbrec_mon, &lb_vip_nb->lb_health_check->options); diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index dadc1af38..c32a11cbd 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -114,7 +114,7 @@ static const char *rbac_mac_binding_update[] = {"logical_port", "ip", "mac", "datapath", "timestamp"}; static const char *rbac_svc_monitor_auth[] = - {""}; + {"chassis_name"}; static const char *rbac_svc_monitor_auth_update[] = {"status"}; static const char *rbac_igmp_group_auth[] = diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema index 72e230b75..1d2b3028d 100644 --- a/ovn-sb.ovsschema +++ b/ovn-sb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Southbound", - "version": "20.30.0", - "cksum": "2972392849 31172", + "version": "20.31.0", + "cksum": "2473562445 31224", "tables": { "SB_Global": { "columns": { @@ -509,6 +509,7 @@ "logical_port": {"type": "string"}, "src_mac": {"type": "string"}, "src_ip": {"type": "string"}, + "chassis_name": {"type": "string"}, "status": { "type": {"key": {"type": "string", "enum": ["set", ["online", "offline", "error"]]}, diff --git a/ovn-sb.xml b/ovn-sb.xml index e393f92b3..1f3b318e0 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -4815,6 +4815,10 @@ tcp.flags = RST; Source IPv4 address to use in the service monitor packet. </column> + <column name="chassis_name"> + The name of the chassis where the logical port is bound. + </column> + <column name="options" key="interval" type='{"type": "integer"}'> The interval, in seconds, between service monitor checks. </column>
Service monitors already had the restriction that chassis could not insert or delete records. However, there was nothing restricting chassis from updating records for service monitors that are relevant to other chassis. This change adds a new "chassis_name" column to the Service_Monitor table. ovn-northd will set this column to the chassis on which the relevant logical port is bound. This way, only that particular chassis can update the status of the service monitor. Signed-off-by: Mark Michelson <mmichels@redhat.com> --- v1 -> v2: * Rebased on top of currrent main --- northd/northd.c | 19 +++++++++++++++++-- northd/ovn-northd.c | 2 +- ovn-sb.ovsschema | 5 +++-- ovn-sb.xml | 4 ++++ 4 files changed, 25 insertions(+), 5 deletions(-)