| Message ID | 20240131121335.1625993-1-mheib@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v2] OVN-SB: Exposes igmp group protocol version through IGMP table. | 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 Wed, Jan 31, 2024 at 1:14 PM Mohammad Heib <mheib@redhat.com> wrote: > Expose the igmp/mld group protocol version through the > IGMP_GROUP table in SBDB. > > This patch can be used by ovn consumer for debuggability purposes, user > now can match between the protocol version used in the OVN logical > switches and the uplink ports. > > Rreported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2160476 > Signed-off-by > <https://bugzilla.redhat.com/show_bug.cgi?id=2160476Signed-off-by>: > Mohammad Heib <mheib@redhat.com> > --- > Hi Mohammad. thank you for the patch, I have two small comments down below. > NEWS | 2 ++ > controller/ip-mcast.c | 10 ++++++++-- > controller/ip-mcast.h | 5 +++-- > controller/pinctrl.c | 28 ++++++++++++++++++++++++---- > northd/ovn-northd.c | 2 +- > ovn-sb.ovsschema | 5 +++-- > ovn-sb.xml | 4 ++++ > tests/ovn.at | 3 +++ > 8 files changed, 48 insertions(+), 11 deletions(-) > > diff --git a/NEWS b/NEWS > index 6553bd078..6505ef22b 100644 > --- a/NEWS > +++ b/NEWS > @@ -18,6 +18,8 @@ Post v23.09.0 > - Support selecting encapsulation IP based on the source/destination > VIF's > settting. See ovn-controller(8) 'external_ids:ovn-encap-ip' for more > details. > + - IGMP_Group has new "protocol" column that displays the the group > + protocol version. > > OVN v23.09.0 - 15 Sep 2023 > -------------------------- > diff --git a/controller/ip-mcast.c b/controller/ip-mcast.c > index a870fb29e..18a3e1fc2 100644 > --- a/controller/ip-mcast.c > +++ b/controller/ip-mcast.c > @@ -107,11 +107,12 @@ igmp_mrouter_create(struct ovsdb_idl_txn *idl_txn, > } > > void > -igmp_group_update_ports(const struct sbrec_igmp_group *g, > +igmp_group_update(const struct sbrec_igmp_group *g, > struct ovsdb_idl_index *datapaths, > struct ovsdb_idl_index *port_bindings, > const struct mcast_snooping *ms OVS_UNUSED, > - const struct mcast_group *mc_group) > + const struct mcast_group *mc_group, > + const char *protocol) > OVS_REQ_RDLOCK(ms->rwlock) > { > struct igmp_group_port *old_ports_storage = > @@ -151,6 +152,11 @@ igmp_group_update_ports(const struct sbrec_igmp_group > *g, > sbrec_igmp_group_update_ports_delvalue(g, igmp_port->port); > } > > + /* set Group protocol */ > + if (protocol) { > + sbrec_igmp_group_set_protocol(g, protocol); > + } > + > free(old_ports_storage); > hmap_destroy(&old_ports); > } > diff --git a/controller/ip-mcast.h b/controller/ip-mcast.h > index 326f39db1..2a8921976 100644 > --- a/controller/ip-mcast.h > +++ b/controller/ip-mcast.h > @@ -45,11 +45,12 @@ struct sbrec_igmp_group *igmp_mrouter_create( > const struct sbrec_datapath_binding *datapath, > const struct sbrec_chassis *chassis); > > -void igmp_group_update_ports(const struct sbrec_igmp_group *g, > +void igmp_group_update(const struct sbrec_igmp_group *g, > struct ovsdb_idl_index *datapaths, > struct ovsdb_idl_index *port_bindings, > const struct mcast_snooping *ms, > - const struct mcast_group *mc_group) > + const struct mcast_group *mc_group, > + const char *protocol) > OVS_REQ_RDLOCK(ms->rwlock); > void > igmp_mrouter_update_ports(const struct sbrec_igmp_group *g, > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index bd3bd3d81..f3597b489 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -180,6 +180,7 @@ struct pinctrl { > bool mac_binding_can_timestamp; > bool fdb_can_timestamp; > bool dns_supports_ovn_owned; > + bool igmp_support_protocol; > }; > > static struct pinctrl pinctrl; > @@ -3586,11 +3587,21 @@ pinctrl_update(const struct ovsdb_idl *idl, const > char *br_int_name) > if (dns_supports_ovn_owned != pinctrl.dns_supports_ovn_owned) { > pinctrl.dns_supports_ovn_owned = dns_supports_ovn_owned; > > - /* Notify pinctrl_handler that fdb timestamp column > + /* Notify pinctrl_handler that dns ovn_owned column > * availability has changed. */ > This change is nice, but not really related to the commit itself. I should be separate. > notify_pinctrl_handler(); > } > > + bool igmp_support_proto = > + sbrec_server_has_igmp_group_table_col_protocol(idl); > + if (igmp_support_proto != pinctrl.igmp_support_protocol) { > + pinctrl.igmp_support_protocol = igmp_support_proto; > + > + /* Notify pinctrl_handler that igmp protocol column > + * availability has changed. */ > + notify_pinctrl_handler(); > + } > + > ovs_mutex_unlock(&pinctrl_mutex); > } > > @@ -5400,9 +5411,18 @@ ip_mcast_sync(struct ovsdb_idl_txn *ovnsb_idl_txn, > local_dp->datapath, > chassis); > } > > - igmp_group_update_ports(sbrec_igmp, > sbrec_datapath_binding_by_key, > - sbrec_port_binding_by_key, ip_ms->ms, > - mc_group); > + /* Set Group protocol if supported */ > + if (pinctrl.igmp_support_protocol) { > + igmp_group_update(sbrec_igmp, > sbrec_datapath_binding_by_key, > + sbrec_port_binding_by_key, ip_ms->ms, > + mc_group, > + mcast_snooping_group_protocol_str( > + mc_group->protocol_version)); > + } else { > + igmp_group_update(sbrec_igmp, > sbrec_datapath_binding_by_key, > + sbrec_port_binding_by_key, ip_ms->ms, > + mc_group, NULL); > + } > nit: We can avoid duplication if we pass pinctrl.igmp_support_protocol and extract the value there WDYT? > } > > /* Mrouters. */ > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index dadc1af38..5e593069f 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -120,7 +120,7 @@ static const char *rbac_svc_monitor_auth_update[] = > static const char *rbac_igmp_group_auth[] = > {""}; > static const char *rbac_igmp_group_update[] = > - {"address", "chassis", "datapath", "ports"}; > + {"address", "protocol", "chassis", "datapath", "ports"}; > static const char *rbac_bfd_auth[] = > {""}; > static const char *rbac_bfd_update[] = > diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema > index 72e230b75..240d65f69 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": "2643271413 31220", > "tables": { > "SB_Global": { > "columns": { > @@ -480,6 +480,7 @@ > "IGMP_Group": { > "columns": { > "address": {"type": "string"}, > + "protocol": {"type": "string"}, > "datapath": {"type": {"key": {"type": "uuid", > "refTable": > "Datapath_Binding", > "refType": "weak"}, > diff --git a/ovn-sb.xml b/ovn-sb.xml > index e393f92b3..56e26b674 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -4756,6 +4756,10 @@ tcp.flags = RST; > Destination IPv4 address for the IGMP group. > </column> > > + <column name="protocol"> > + Group protocol version either IGMPv1,v2,v3 or MLDv1,v2. > + </column> > + > <column name="datapath"> > Datapath to which this IGMP group belongs. > </column> > diff --git a/tests/ovn.at b/tests/ovn.at > index 28c6b6c34..c4e67e4f5 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -22549,6 +22549,7 @@ send_igmp_v3_report hv2-vif1 hv2 000000000002 > $(ip_to_hex 10 0 0 2) f9f9 \ > > # Check that the IGMP Group is learned on both hv. > wait_row_count IGMP_Group 2 address=239.0.1.68 > +wait_row_count IGMP_Group 2 protocol=IGMPv3 > check ovn-nbctl --wait=hv sync > > AT_CAPTURE_FILE([sbflows3]) > @@ -22626,6 +22627,7 @@ send_igmp_v3_report hv1-vif1 hv1 \ > > # Check that the IGMP Group is learned. > wait_row_count IGMP_Group 1 address=224.0.0.42 > +wait_row_count IGMP_Group 1 protocol=IGMPv3 > > AS_BOX([IGMP traffic test 3]) > # Send traffic and make sure it gets flooded to all ports. > @@ -23287,6 +23289,7 @@ send_mld_v2_report hv2-vif1 hv2 \ > > # Check that the IP multicast group is learned on both hv. > wait_row_count IGMP_Group 2 address='"ff0a:dead:beef::1"' > +wait_row_count IGMP_Group 2 protocol=MLDv2 > > # This gives the ovn-controller nodes a chance to see the new IGMP_Group. > check ovn-nbctl --wait=hv sync > -- > 2.34.3 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thanks, Ales
diff --git a/NEWS b/NEWS index 6553bd078..6505ef22b 100644 --- a/NEWS +++ b/NEWS @@ -18,6 +18,8 @@ Post v23.09.0 - Support selecting encapsulation IP based on the source/destination VIF's settting. See ovn-controller(8) 'external_ids:ovn-encap-ip' for more details. + - IGMP_Group has new "protocol" column that displays the the group + protocol version. OVN v23.09.0 - 15 Sep 2023 -------------------------- diff --git a/controller/ip-mcast.c b/controller/ip-mcast.c index a870fb29e..18a3e1fc2 100644 --- a/controller/ip-mcast.c +++ b/controller/ip-mcast.c @@ -107,11 +107,12 @@ igmp_mrouter_create(struct ovsdb_idl_txn *idl_txn, } void -igmp_group_update_ports(const struct sbrec_igmp_group *g, +igmp_group_update(const struct sbrec_igmp_group *g, struct ovsdb_idl_index *datapaths, struct ovsdb_idl_index *port_bindings, const struct mcast_snooping *ms OVS_UNUSED, - const struct mcast_group *mc_group) + const struct mcast_group *mc_group, + const char *protocol) OVS_REQ_RDLOCK(ms->rwlock) { struct igmp_group_port *old_ports_storage = @@ -151,6 +152,11 @@ igmp_group_update_ports(const struct sbrec_igmp_group *g, sbrec_igmp_group_update_ports_delvalue(g, igmp_port->port); } + /* set Group protocol */ + if (protocol) { + sbrec_igmp_group_set_protocol(g, protocol); + } + free(old_ports_storage); hmap_destroy(&old_ports); } diff --git a/controller/ip-mcast.h b/controller/ip-mcast.h index 326f39db1..2a8921976 100644 --- a/controller/ip-mcast.h +++ b/controller/ip-mcast.h @@ -45,11 +45,12 @@ struct sbrec_igmp_group *igmp_mrouter_create( const struct sbrec_datapath_binding *datapath, const struct sbrec_chassis *chassis); -void igmp_group_update_ports(const struct sbrec_igmp_group *g, +void igmp_group_update(const struct sbrec_igmp_group *g, struct ovsdb_idl_index *datapaths, struct ovsdb_idl_index *port_bindings, const struct mcast_snooping *ms, - const struct mcast_group *mc_group) + const struct mcast_group *mc_group, + const char *protocol) OVS_REQ_RDLOCK(ms->rwlock); void igmp_mrouter_update_ports(const struct sbrec_igmp_group *g, diff --git a/controller/pinctrl.c b/controller/pinctrl.c index bd3bd3d81..f3597b489 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -180,6 +180,7 @@ struct pinctrl { bool mac_binding_can_timestamp; bool fdb_can_timestamp; bool dns_supports_ovn_owned; + bool igmp_support_protocol; }; static struct pinctrl pinctrl; @@ -3586,11 +3587,21 @@ pinctrl_update(const struct ovsdb_idl *idl, const char *br_int_name) if (dns_supports_ovn_owned != pinctrl.dns_supports_ovn_owned) { pinctrl.dns_supports_ovn_owned = dns_supports_ovn_owned; - /* Notify pinctrl_handler that fdb timestamp column + /* Notify pinctrl_handler that dns ovn_owned column * availability has changed. */ notify_pinctrl_handler(); } + bool igmp_support_proto = + sbrec_server_has_igmp_group_table_col_protocol(idl); + if (igmp_support_proto != pinctrl.igmp_support_protocol) { + pinctrl.igmp_support_protocol = igmp_support_proto; + + /* Notify pinctrl_handler that igmp protocol column + * availability has changed. */ + notify_pinctrl_handler(); + } + ovs_mutex_unlock(&pinctrl_mutex); } @@ -5400,9 +5411,18 @@ ip_mcast_sync(struct ovsdb_idl_txn *ovnsb_idl_txn, local_dp->datapath, chassis); } - igmp_group_update_ports(sbrec_igmp, sbrec_datapath_binding_by_key, - sbrec_port_binding_by_key, ip_ms->ms, - mc_group); + /* Set Group protocol if supported */ + if (pinctrl.igmp_support_protocol) { + igmp_group_update(sbrec_igmp, sbrec_datapath_binding_by_key, + sbrec_port_binding_by_key, ip_ms->ms, + mc_group, + mcast_snooping_group_protocol_str( + mc_group->protocol_version)); + } else { + igmp_group_update(sbrec_igmp, sbrec_datapath_binding_by_key, + sbrec_port_binding_by_key, ip_ms->ms, + mc_group, NULL); + } } /* Mrouters. */ diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index dadc1af38..5e593069f 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -120,7 +120,7 @@ static const char *rbac_svc_monitor_auth_update[] = static const char *rbac_igmp_group_auth[] = {""}; static const char *rbac_igmp_group_update[] = - {"address", "chassis", "datapath", "ports"}; + {"address", "protocol", "chassis", "datapath", "ports"}; static const char *rbac_bfd_auth[] = {""}; static const char *rbac_bfd_update[] = diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema index 72e230b75..240d65f69 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": "2643271413 31220", "tables": { "SB_Global": { "columns": { @@ -480,6 +480,7 @@ "IGMP_Group": { "columns": { "address": {"type": "string"}, + "protocol": {"type": "string"}, "datapath": {"type": {"key": {"type": "uuid", "refTable": "Datapath_Binding", "refType": "weak"}, diff --git a/ovn-sb.xml b/ovn-sb.xml index e393f92b3..56e26b674 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -4756,6 +4756,10 @@ tcp.flags = RST; Destination IPv4 address for the IGMP group. </column> + <column name="protocol"> + Group protocol version either IGMPv1,v2,v3 or MLDv1,v2. + </column> + <column name="datapath"> Datapath to which this IGMP group belongs. </column> diff --git a/tests/ovn.at b/tests/ovn.at index 28c6b6c34..c4e67e4f5 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -22549,6 +22549,7 @@ send_igmp_v3_report hv2-vif1 hv2 000000000002 $(ip_to_hex 10 0 0 2) f9f9 \ # Check that the IGMP Group is learned on both hv. wait_row_count IGMP_Group 2 address=239.0.1.68 +wait_row_count IGMP_Group 2 protocol=IGMPv3 check ovn-nbctl --wait=hv sync AT_CAPTURE_FILE([sbflows3]) @@ -22626,6 +22627,7 @@ send_igmp_v3_report hv1-vif1 hv1 \ # Check that the IGMP Group is learned. wait_row_count IGMP_Group 1 address=224.0.0.42 +wait_row_count IGMP_Group 1 protocol=IGMPv3 AS_BOX([IGMP traffic test 3]) # Send traffic and make sure it gets flooded to all ports. @@ -23287,6 +23289,7 @@ send_mld_v2_report hv2-vif1 hv2 \ # Check that the IP multicast group is learned on both hv. wait_row_count IGMP_Group 2 address='"ff0a:dead:beef::1"' +wait_row_count IGMP_Group 2 protocol=MLDv2 # This gives the ovn-controller nodes a chance to see the new IGMP_Group. check ovn-nbctl --wait=hv sync
Expose the igmp/mld group protocol version through the IGMP_GROUP table in SBDB. This patch can be used by ovn consumer for debuggability purposes, user now can match between the protocol version used in the OVN logical switches and the uplink ports. Rreported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2160476 Signed-off-by: Mohammad Heib <mheib@redhat.com> --- NEWS | 2 ++ controller/ip-mcast.c | 10 ++++++++-- controller/ip-mcast.h | 5 +++-- controller/pinctrl.c | 28 ++++++++++++++++++++++++---- northd/ovn-northd.c | 2 +- ovn-sb.ovsschema | 5 +++-- ovn-sb.xml | 4 ++++ tests/ovn.at | 3 +++ 8 files changed, 48 insertions(+), 11 deletions(-)