diff mbox series

[ovs-dev] mcast-snooping: Remove typedef from mcast_group_proto.

Message ID 20240126170758.114302-1-i.maximets@ovn.org
State Accepted
Commit 7b838a24fcf55c7f289b801998294c8dcff09e32
Headers show
Series [ovs-dev] mcast-snooping: Remove typedef from mcast_group_proto. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Ilya Maximets Jan. 26, 2024, 5:07 p.m. UTC 
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 <i.maximets@ovn.org>
---
 lib/mcast-snooping.c         |  6 +++---
 lib/mcast-snooping.h         | 12 ++++++------
 ofproto/ofproto-dpif-xlate.c |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

Comments

Mohammad Heib Jan. 28, 2024, 10:20 a.m. UTC | #1 
Thanks, Ilya,
Looks good to me.

Acked-by: Mohammad Heib <mheib@redhat.com>

Thanks,



On Fri, Jan 26, 2024 at 7:11 PM Ilya Maximets <i.maximets@ovn.org> 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 <i.maximets@ovn.org>
> ---
>  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
>
>
Ilya Maximets Jan. 29, 2024, 3:23 p.m. UTC | #2 
On 1/28/24 11:20, Mohammad Heib wrote:
> Thanks, Ilya,
> Looks good to me.
> 
> Acked-by: Mohammad Heib <mheib@redhat.com <mailto:mheib@redhat.com>>
> 

Thanks!  I applied the change and backported to branch-3.3.

Best regards, Ilya Maximets.

> Thanks,
> 
> 
> 
> On Fri, Jan 26, 2024 at 7:11 PM Ilya Maximets <i.maximets@ovn.org <mailto:i.maximets@ovn.org>> 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 <i.maximets@ovn.org <mailto:i.maximets@ovn.org>>
>     ---
>      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
>
diff mbox series

Patch

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;