Re: [ovs-dev] [PATCH ovn v2 1/3] rbac: Only allow relevant chassis to update service monitors.
On Tue, Jan 30, 2024 at 10:08 PM Mark Michelson 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
> ---
> 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, _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,
> _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.
>
>
> +
> +The
[ovs-dev] [PATCH ovn v2 1/3] rbac: Only allow relevant chassis to update service monitors.
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
---
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, _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, _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.
+
+The name of the chassis where the logical port is bound.
+
+
The interval, in seconds, between service monitor checks.
--
2.43.0
___
dev mailing list
d...@openvswitch.org
