| Message ID | 20240122141443.2074919-2-mheib@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Dumitru Ceara |
| Headers | show |
| Series | [ovs-dev,1/2] ovs: Bump submodule to include igmp protocol version. | expand |
On 1/22/24 15:14, Mohammad Heib 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: Mohammad Heib <mheib@redhat.com> > --- Hi Mohammad, I have a few (minor) comments, please see below. > NEWS | 2 ++ > controller/ip-mcast.c | 8 ++++++++ > controller/ip-mcast.h | 3 +++ > controller/pinctrl.c | 19 ++++++++++++++++++- > northd/ovn-northd.c | 2 +- > ovn-sb.ovsschema | 5 +++-- > ovn-sb.xml | 4 ++++ > tests/ovn.at | 3 +++ > 8 files changed, 42 insertions(+), 4 deletions(-) > > diff --git a/NEWS b/NEWS > index 5f267b4c6..9075e7d80 100644 > --- a/NEWS > +++ b/NEWS > @@ -14,6 +14,8 @@ Post v23.09.0 > - ovn-northd-ddlog has been removed. > - A new LSP option "enable_router_port_acl" has been added to enable > conntrack for the router port whose peer is l3dgw_port if set it true. > + - IGMP_Group have a new "protocol" column that displays the the group Nit: s/have/has > + protocol version. > > OVN v23.09.0 - 15 Sep 2023 > -------------------------- > diff --git a/controller/ip-mcast.c b/controller/ip-mcast.c > index a870fb29e..83e41c81f 100644 > --- a/controller/ip-mcast.c > +++ b/controller/ip-mcast.c > @@ -226,6 +226,14 @@ igmp_group_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, > return true; > } > > + > +void igmp_group_set_protocol(const struct sbrec_igmp_group *group, > + mcast_group_proto protocol) > +{ > + sbrec_igmp_group_set_protocol(group, > + mcast_snooping_group_protocol_str(protocol)); > +} > + Does it make more sense to add the protocol as argument to igmp_group_update_ports() and rename that function to igmp_group_update()? > static const struct sbrec_igmp_group * > igmp_group_lookup_(struct ovsdb_idl_index *igmp_groups, > const char *addr_str, > diff --git a/controller/ip-mcast.h b/controller/ip-mcast.h > index 326f39db1..f0c34343f 100644 > --- a/controller/ip-mcast.h > +++ b/controller/ip-mcast.h > @@ -63,4 +63,7 @@ void igmp_group_delete(const struct sbrec_igmp_group *g); > bool igmp_group_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, > struct ovsdb_idl_index *igmp_groups); > > +void igmp_group_set_protocol(const struct sbrec_igmp_group *group, > + mcast_group_proto protocol); > + > #endif /* controller/ip-mcast.h */ > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index 77bf67e58..6379b7afb 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; We only use this in the main thread, when updating the SB, why can't we just directly check the column support there instead? > + > + /* Notify pinctrl_handler that igmp protocol column > + * availability has changed. */ > + notify_pinctrl_handler(); > + } > + > ovs_mutex_unlock(&pinctrl_mutex); > } > > @@ -5400,6 +5411,12 @@ ip_mcast_sync(struct ovsdb_idl_txn *ovnsb_idl_txn, > local_dp->datapath, chassis); > } > > + /* Set Group protocol*/ > + if (pinctrl.igmp_support_protocol) { > + igmp_group_set_protocol(sbrec_igmp, > + mc_group->protocol_version); > + } > + > igmp_group_update_ports(sbrec_igmp, sbrec_datapath_binding_by_key, > sbrec_port_binding_by_key, ip_ms->ms, > mc_group); > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index f3868068d..700c9cf6e 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 8cc4c311c..180b9bfdd 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 Regards, Dumitru
On Mon, Jan 22, 2024 at 5:26 PM Dumitru Ceara <dceara@redhat.com> wrote: > On 1/22/24 15:14, Mohammad Heib 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: Mohammad Heib <mheib@redhat.com> > > --- > Hi Dumitru, Thank you for your review :), i have small question please see below. > Hi Mohammad, > > I have a few (minor) comments, please see below. > > > NEWS | 2 ++ > > controller/ip-mcast.c | 8 ++++++++ > > controller/ip-mcast.h | 3 +++ > > controller/pinctrl.c | 19 ++++++++++++++++++- > > northd/ovn-northd.c | 2 +- > > ovn-sb.ovsschema | 5 +++-- > > ovn-sb.xml | 4 ++++ > > tests/ovn.at | 3 +++ > > 8 files changed, 42 insertions(+), 4 deletions(-) > > > > diff --git a/NEWS b/NEWS > > index 5f267b4c6..9075e7d80 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -14,6 +14,8 @@ Post v23.09.0 > > - ovn-northd-ddlog has been removed. > > - A new LSP option "enable_router_port_acl" has been added to enable > > conntrack for the router port whose peer is l3dgw_port if set it > true. > > + - IGMP_Group have a new "protocol" column that displays the the group > > Nit: s/have/has > > > + protocol version. > > > > OVN v23.09.0 - 15 Sep 2023 > > -------------------------- > > diff --git a/controller/ip-mcast.c b/controller/ip-mcast.c > > index a870fb29e..83e41c81f 100644 > > --- a/controller/ip-mcast.c > > +++ b/controller/ip-mcast.c > > @@ -226,6 +226,14 @@ igmp_group_cleanup(struct ovsdb_idl_txn > *ovnsb_idl_txn, > > return true; > > } > > > > + > > +void igmp_group_set_protocol(const struct sbrec_igmp_group *group, > > + mcast_group_proto protocol) > > +{ > > + sbrec_igmp_group_set_protocol(group, > > + > mcast_snooping_group_protocol_str(protocol)); > > +} > > + > > Does it make more sense to add the protocol as argument to > igmp_group_update_ports() and rename that function to igmp_group_update()? > yes that make sense and will make the code more cleaner. > > > static const struct sbrec_igmp_group * > > igmp_group_lookup_(struct ovsdb_idl_index *igmp_groups, > > const char *addr_str, > > diff --git a/controller/ip-mcast.h b/controller/ip-mcast.h > > index 326f39db1..f0c34343f 100644 > > --- a/controller/ip-mcast.h > > +++ b/controller/ip-mcast.h > > @@ -63,4 +63,7 @@ void igmp_group_delete(const struct sbrec_igmp_group > *g); > > bool igmp_group_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, > > struct ovsdb_idl_index *igmp_groups); > > > > +void igmp_group_set_protocol(const struct sbrec_igmp_group *group, > > + mcast_group_proto protocol); > > + > > #endif /* controller/ip-mcast.h */ > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > > index 77bf67e58..6379b7afb 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; > > We only use this in the main thread, when updating the SB, why can't we > just directly check the column support there instead? > *like you mean to call sbrec_server_has_igmp_group_table_col_protocol(idl); inside the **ip_mcast_sync function?* *something like this:* * /* Set Group protocol*/ if (sbrec_server_has_igmp_group_table_col_protocol(idl)) { igmp_group_set_protocol(sbrec_igmp, mc_group->protocol_version); }* > > > + > > + /* Notify pinctrl_handler that igmp protocol column > > + * availability has changed. */ > > + notify_pinctrl_handler(); > > + } > > + > > ovs_mutex_unlock(&pinctrl_mutex); > > } > > > > @@ -5400,6 +5411,12 @@ ip_mcast_sync(struct ovsdb_idl_txn *ovnsb_idl_txn, > > local_dp->datapath, > chassis); > > } > > > > + /* Set Group protocol*/ > > + if (pinctrl.igmp_support_protocol) { > > + igmp_group_set_protocol(sbrec_igmp, > > + mc_group->protocol_version); > > + } > > + > > igmp_group_update_ports(sbrec_igmp, > sbrec_datapath_binding_by_key, > > sbrec_port_binding_by_key, > ip_ms->ms, > > mc_group); > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index f3868068d..700c9cf6e 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 8cc4c311c..180b9bfdd 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 > > Regards, > Dumitru > > Thanks,
On 1/23/24 14:58, Mohammad Heib wrote: >>> 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; >> We only use this in the main thread, when updating the SB, why can't we >> just directly check the column support there instead? >> > *like you mean to call > sbrec_server_has_igmp_group_table_col_protocol(idl); inside the **ip_mcast_sync > function?* > *something like this:* > > > > > * /* Set Group protocol*/ if > (sbrec_server_has_igmp_group_table_col_protocol(idl)) { > igmp_group_set_protocol(sbrec_igmp, > mc_group->protocol_version); }* Yes, I think that would be better. Or even inside igmp_group_update_ports() but then we should also pass the IDL pointer; the latter seems like a better option to me. Please let me know what you think. Regards, Dumitru
On Tue, Jan 23, 2024 at 4:10 PM Dumitru Ceara <dceara@redhat.com> wrote: > On 1/23/24 14:58, Mohammad Heib wrote: > >>> 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; > >> We only use this in the main thread, when updating the SB, why can't we > >> just directly check the column support there instead? > >> > > *like you mean to call > > sbrec_server_has_igmp_group_table_col_protocol(idl); inside the > **ip_mcast_sync > > function?* > > *something like this:* > > > > > > > > > > * /* Set Group protocol*/ if > > (sbrec_server_has_igmp_group_table_col_protocol(idl)) { > > igmp_group_set_protocol(sbrec_igmp, > > mc_group->protocol_version); }* > > Yes, I think that would be better. Or even inside > igmp_group_update_ports() but then we should also pass the IDL pointer; > the latter seems like a better option to me. > > Please let me know what you think. > Sound good to me :), i will send v2 addressing your comments when I can bump the ovs to the last stable. Thank you so much :) > > Regards, > Dumitru > >
diff --git a/NEWS b/NEWS index 5f267b4c6..9075e7d80 100644 --- a/NEWS +++ b/NEWS @@ -14,6 +14,8 @@ Post v23.09.0 - ovn-northd-ddlog has been removed. - A new LSP option "enable_router_port_acl" has been added to enable conntrack for the router port whose peer is l3dgw_port if set it true. + - IGMP_Group have a 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..83e41c81f 100644 --- a/controller/ip-mcast.c +++ b/controller/ip-mcast.c @@ -226,6 +226,14 @@ igmp_group_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, return true; } + +void igmp_group_set_protocol(const struct sbrec_igmp_group *group, + mcast_group_proto protocol) +{ + sbrec_igmp_group_set_protocol(group, + mcast_snooping_group_protocol_str(protocol)); +} + static const struct sbrec_igmp_group * igmp_group_lookup_(struct ovsdb_idl_index *igmp_groups, const char *addr_str, diff --git a/controller/ip-mcast.h b/controller/ip-mcast.h index 326f39db1..f0c34343f 100644 --- a/controller/ip-mcast.h +++ b/controller/ip-mcast.h @@ -63,4 +63,7 @@ void igmp_group_delete(const struct sbrec_igmp_group *g); bool igmp_group_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, struct ovsdb_idl_index *igmp_groups); +void igmp_group_set_protocol(const struct sbrec_igmp_group *group, + mcast_group_proto protocol); + #endif /* controller/ip-mcast.h */ diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 77bf67e58..6379b7afb 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,6 +5411,12 @@ ip_mcast_sync(struct ovsdb_idl_txn *ovnsb_idl_txn, local_dp->datapath, chassis); } + /* Set Group protocol*/ + if (pinctrl.igmp_support_protocol) { + igmp_group_set_protocol(sbrec_igmp, + mc_group->protocol_version); + } + igmp_group_update_ports(sbrec_igmp, sbrec_datapath_binding_by_key, sbrec_port_binding_by_key, ip_ms->ms, mc_group); diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index f3868068d..700c9cf6e 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 8cc4c311c..180b9bfdd 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 | 8 ++++++++ controller/ip-mcast.h | 3 +++ controller/pinctrl.c | 19 ++++++++++++++++++- northd/ovn-northd.c | 2 +- ovn-sb.ovsschema | 5 +++-- ovn-sb.xml | 4 ++++ tests/ovn.at | 3 +++ 8 files changed, 42 insertions(+), 4 deletions(-)