On Mon, Feb 26, 2024 at 2:11 PM Mohammad Heib <[email protected]> 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 <[email protected]>
> ---
> v3:
>   Address Ales comments in v2 and rebase over main.
> ---
>  NEWS                  |  2 ++
>  controller/ip-mcast.c | 11 +++++++++--
>  controller/ip-mcast.h | 11 ++++++-----
>  controller/pinctrl.c  | 16 ++++++++++++++--
>  northd/ovn-northd.c   |  2 +-
>  ovn-sb.ovsschema      |  5 +++--
>  ovn-sb.xml            |  4 ++++
>  tests/ovn.at          |  3 +++
>  8 files changed, 42 insertions(+), 12 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index c0131ceee..784fedfa0 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -5,6 +5,8 @@ Post v24.03.0
>      cloned to all unknown ports connected to the same Logical Switch.
>    - Added a new logical switch port option "disable_arp_nd_rsp" to
>      disable adding the ARP responder flows if set to true.
> +  - IGMP_Group has new "protocol" column that displays the the group
> +    protocol version.
>
>  OVN v24.03.0 - xx xxx xxxx
>  --------------------------
> diff --git a/controller/ip-mcast.c b/controller/ip-mcast.c
> index b457c7e69..0969cc07f 100644
> --- a/controller/ip-mcast.c
> +++ b/controller/ip-mcast.c
> @@ -111,11 +111,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 bool igmp_support_protocol)
>      OVS_REQ_RDLOCK(ms->rwlock)
>  {
>      struct igmp_group_port *old_ports_storage =
> @@ -155,6 +156,12 @@ igmp_group_update_ports(const struct sbrec_igmp_group
> *g,
>          sbrec_igmp_group_update_ports_delvalue(g, igmp_port->port);
>      }
>
> +    /* set Group protocol */
> +    if (igmp_support_protocol) {
> +         sbrec_igmp_group_set_protocol(g,
> mcast_snooping_group_protocol_str(
> +                                       mc_group->protocol_version));
> +    }
> +
>      free(old_ports_storage);
>      hmap_destroy(&old_ports);
>  }
> diff --git a/controller/ip-mcast.h b/controller/ip-mcast.h
> index eebada968..026503139 100644
> --- a/controller/ip-mcast.h
> +++ b/controller/ip-mcast.h
> @@ -47,11 +47,12 @@ struct sbrec_igmp_group *igmp_mrouter_create(
>      const struct sbrec_chassis *chassis,
>      bool igmp_group_has_chassis_name);
>
> -void igmp_group_update_ports(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)
> +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 bool igmp_support_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 98b29de9f..66e390e80 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -181,6 +181,7 @@ struct pinctrl {
>      bool fdb_can_timestamp;
>      bool dns_supports_ovn_owned;
>      bool igmp_group_has_chassis_name;
> +    bool igmp_support_protocol;
>  };
>
>  static struct pinctrl pinctrl;
> @@ -3599,6 +3600,17 @@ pinctrl_update(const struct ovsdb_idl *idl, const
> char *br_int_name)
>          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);
>  }
>
> @@ -5409,9 +5421,9 @@ ip_mcast_sync(struct ovsdb_idl_txn *ovnsb_idl_txn,
>                      chassis, pinctrl.igmp_group_has_chassis_name);
>              }
>
> -            igmp_group_update_ports(sbrec_igmp,
> sbrec_datapath_binding_by_key,
> +            igmp_group_update(sbrec_igmp, sbrec_datapath_binding_by_key,
>                                      sbrec_port_binding_by_key, ip_ms->ms,
> -                                    mc_group);
> +                                    mc_group,
> pinctrl.igmp_support_protocol);
>          }
>
>          /* Mrouters. */
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index b1be73cb2..3a5544b0c 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[] =
>      {"chassis_name"};
>  static const char *rbac_igmp_group_update[] =
> -    {"address", "chassis", "datapath", "ports"};
> +    {"address", "protocol", "chassis", "datapath", "ports"};
>  static const char *rbac_bfd_auth[] =
>      {"chassis_name"};
>  static const char *rbac_bfd_update[] =
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index 84ae09515..b6c051ae6 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>      "name": "OVN_Southbound",
> -    "version": "20.33.0",
> -    "cksum": "4076371179 31328",
> +    "version": "20.34.0",
> +    "cksum": "2786607656 31376",
>      "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 ac4e585f2..4c26c6714 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 d26c95054..2c71370f1 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -22706,6 +22706,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])
> @@ -22783,6 +22784,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.
> @@ -23444,6 +23446,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
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Looks good to me, thanks.

Acked-by: Ales Musil <[email protected]>

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA <https://www.redhat.com>

[email protected]
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to