Thanks, Ilya,
Looks good to me.

Acked-by: Mohammad Heib <[email protected]>

Thanks,



On Fri, Jan 26, 2024 at 7:11 PM Ilya Maximets <[email protected]> wrote:

> Typedefs are confusing and the coding style generally advises to not
> use them.  Removing typedef until others start using it.
>
> This typedef already got me while testing an OVN update to use OVS 3.3
> as a submodule, since the variable was declared in a switch statement
> and it wasn't clearly visible that there is a variable definition in
> one of the cases and braces should be used.  Strangely some versions
> of compilers do not require braces in this case, so OVN change works
> locally, but not in CI.
>
> Fixes: 077d0bad0436 ("mcast-snooping: Store IGMP/MLD protocol version.")
> Signed-off-by: Ilya Maximets <[email protected]>
> ---
>  lib/mcast-snooping.c         |  6 +++---
>  lib/mcast-snooping.h         | 12 ++++++------
>  ofproto/ofproto-dpif-xlate.c |  2 +-
>  3 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
> index 60ef8381e..dc5164b41 100644
> --- a/lib/mcast-snooping.c
> +++ b/lib/mcast-snooping.c
> @@ -58,7 +58,7 @@ mcast_snooping_flood_unreg(const struct mcast_snooping
> *ms)
>  }
>
>  char *
> -mcast_snooping_group_protocol_str(mcast_group_proto grp_proto)
> +mcast_snooping_group_protocol_str(enum mcast_group_proto grp_proto)
>  {
>      switch (grp_proto) {
>      case MCAST_GROUP_IGMPV1:
> @@ -414,7 +414,7 @@ bool
>  mcast_snooping_add_group(struct mcast_snooping *ms,
>                           const struct in6_addr *addr,
>                           uint16_t vlan, void *port,
> -                         mcast_group_proto grp_proto)
> +                         enum mcast_group_proto grp_proto)
>      OVS_REQ_WRLOCK(ms->rwlock)
>  {
>      bool learned;
> @@ -460,7 +460,7 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
>  bool
>  mcast_snooping_add_group4(struct mcast_snooping *ms, ovs_be32 ip4,
>                           uint16_t vlan, void *port,
> -                         mcast_group_proto grp_proto)
> +                         enum mcast_group_proto grp_proto)
>      OVS_REQ_WRLOCK(ms->rwlock)
>  {
>      struct in6_addr addr = in6_addr_mapped_ipv4(ip4);
> diff --git a/lib/mcast-snooping.h b/lib/mcast-snooping.h
> index 76ab4e4f7..de42cf826 100644
> --- a/lib/mcast-snooping.h
> +++ b/lib/mcast-snooping.h
> @@ -40,13 +40,13 @@ struct mcast_snooping;
>  #define MCAST_MROUTER_PORT_IDLE_TIME 180
>
>  /* Multicast group protocol. */
> -typedef enum {
> +enum mcast_group_proto {
>      MCAST_GROUP_IGMPV1 = 0,
>      MCAST_GROUP_IGMPV2,
>      MCAST_GROUP_IGMPV3,
>      MCAST_GROUP_MLDV1,
>      MCAST_GROUP_MLDV2,
> -} mcast_group_proto;
> +};
>
>  /* Multicast group entry.
>   * Guarded by owning 'mcast_snooping''s rwlock. */
> @@ -61,7 +61,7 @@ struct mcast_group {
>      uint16_t vlan;
>
>      /* Multicast group IPv6/IPv4 Protocol version IGMPv1,2,3 or MLDv1,2 */
> -    mcast_group_proto protocol_version;
> +    enum mcast_group_proto protocol_version;
>
>      /* Node in parent struct mcast_snooping group_lru. */
>      struct ovs_list group_node OVS_GUARDED;
> @@ -198,11 +198,11 @@ mcast_snooping_lookup4(const struct mcast_snooping
> *ms, ovs_be32 ip4,
>  bool mcast_snooping_add_group(struct mcast_snooping *ms,
>                                const struct in6_addr *addr,
>                                uint16_t vlan, void *port,
> -                              mcast_group_proto grp_proto)
> +                              enum mcast_group_proto grp_proto)
>      OVS_REQ_WRLOCK(ms->rwlock);
>  bool mcast_snooping_add_group4(struct mcast_snooping *ms, ovs_be32 ip4,
>                                 uint16_t vlan, void *port,
> -                               mcast_group_proto grp_proto)
> +                               enum mcast_group_proto grp_proto)
>      OVS_REQ_WRLOCK(ms->rwlock);
>  int mcast_snooping_add_report(struct mcast_snooping *ms,
>                                const struct dp_packet *p,
> @@ -224,7 +224,7 @@ bool mcast_snooping_add_mrouter(struct mcast_snooping
> *ms, uint16_t vlan,
>      OVS_REQ_WRLOCK(ms->rwlock);
>  bool mcast_snooping_is_query(ovs_be16 igmp_type);
>  bool mcast_snooping_is_membership(ovs_be16 igmp_type);
> -char *mcast_snooping_group_protocol_str(mcast_group_proto grp_proto);
> +char *mcast_snooping_group_protocol_str(enum mcast_group_proto grp_proto);
>
>  /* Flush. */
>  void mcast_snooping_mdb_flush(struct mcast_snooping *ms);
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index f4d1d7194..1cf4d5f7c 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -2796,7 +2796,7 @@ update_mcast_snooping_table4__(const struct
> xlate_ctx *ctx,
>      OVS_REQ_WRLOCK(ms->rwlock)
>  {
>      const struct igmp_header *igmp;
> -    mcast_group_proto grp_proto;
> +    enum mcast_group_proto grp_proto;
>      int count;
>      size_t offset;
>      ovs_be32 ip4 = flow->igmp_group_ip4;
> --
> 2.43.0
>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to