On 27 Dec 2023, at 12:15, Mohammad Heib wrote:
> Expose the mcast group protocol via the mdb/show
> command output.
>
> Signed-off-by: Mohammad Heib <[email protected]>
Thanks Mohammad, for the updated patch series. I have three small comments, see
below diffs.
Cheers,
Eelco
On the same note. If you ever get some time to do a follow-up patch to properly
align the age column, that would be appreciated ;)
> ---
> lib/mcast-snooping.c | 24 ++++++++
> lib/mcast-snooping.h | 1 +
> ofproto/ofproto-dpif.c | 9 ++-
> tests/mcast-snooping.at | 130 ++++++++++++++++++++++++++++++++--------
> tests/stp.at | 6 +-
> 5 files changed, 139 insertions(+), 31 deletions(-)
>
> diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
> index 995216a4b..60ef8381e 100644
> --- a/lib/mcast-snooping.c
> +++ b/lib/mcast-snooping.c
> @@ -57,6 +57,30 @@ mcast_snooping_flood_unreg(const struct mcast_snooping *ms)
> return ms->flood_unreg;
> }
>
> +char *
> +mcast_snooping_group_protocol_str(mcast_group_proto grp_proto)
> +{
> + switch (grp_proto) {
> + case MCAST_GROUP_IGMPV1:
> + return "IGMPv1";
> + break;
> + case MCAST_GROUP_IGMPV2:
> + return "IGMPv2";
> + break;
> + case MCAST_GROUP_IGMPV3:
> + return "IGMPv3";
> + break;
> + case MCAST_GROUP_MLDV1:
> + return "MLDv1";
> + break;
> + case MCAST_GROUP_MLDV2:
> + return "MLDv2";
> + break;
> + default:
> + return "UNKNOWN";
> + }
> +}
> +
> bool
> mcast_snooping_is_query(ovs_be16 igmp_type)
> {
> diff --git a/lib/mcast-snooping.h b/lib/mcast-snooping.h
> index 8cc8fb0fb..acd9fdb68 100644
> --- a/lib/mcast-snooping.h
> +++ b/lib/mcast-snooping.h
> @@ -224,6 +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);
The space after the * should be removed:
diff --git a/lib/mcast-snooping.h b/lib/mcast-snooping.h
index acd9fdb68..76ab4e4f7 100644
--- a/lib/mcast-snooping.h
+++ b/lib/mcast-snooping.h
@@ -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(mcast_group_proto grp_proto);
/* Flush. */
void mcast_snooping_mdb_flush(struct mcast_snooping *ms);
>
> /* Flush. */
> void mcast_snooping_mdb_flush(struct mcast_snooping *ms);
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 6e62ed1f9..434ea1670 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -6172,7 +6172,7 @@ ofproto_unixctl_mcast_snooping_show(struct unixctl_conn
> *conn,
> return;
> }
>
> - ds_put_cstr(&ds, " port VLAN GROUP Age\n");
> + ds_put_cstr(&ds, " port VLAN protocol GROUP Age\n");
> ovs_rwlock_rdlock(&ofproto->ms->rwlock);
> LIST_FOR_EACH (grp, group_node, &ofproto->ms->group_lru) {
> LIST_FOR_EACH(b, bundle_node, &grp->bundle_lru) {
> @@ -6181,7 +6181,9 @@ ofproto_unixctl_mcast_snooping_show(struct unixctl_conn
> *conn,
> bundle = b->port;
> ofputil_port_to_string(ofbundle_get_a_port(bundle)->up.ofp_port,
> NULL, name, sizeof name);
> - ds_put_format(&ds, "%5s %4d ", name, grp->vlan);
> + ds_put_format(&ds, "%5s %4d %8s ", name, grp->vlan,
> + mcast_snooping_group_protocol_str(
> + grp->protocol_version));
Need indentation:
+ ds_put_format(&ds, "%5s %4d %8s ", name, grp->vlan,
+ mcast_snooping_group_protocol_str(
+ grp->protocol_version));
> ipv6_format_mapped(&grp->addr, &ds);
> ds_put_format(&ds, " %3d\n",
> mcast_bundle_age(ofproto->ms, b));
> @@ -6195,8 +6197,9 @@ ofproto_unixctl_mcast_snooping_show(struct unixctl_conn
> *conn,
> bundle = mrouter->port;
> ofputil_port_to_string(ofbundle_get_a_port(bundle)->up.ofp_port,
> NULL, name, sizeof name);
> - ds_put_format(&ds, "%5s %4d querier %3d\n",
> + ds_put_format(&ds, "%5s %4d %8s querier %3d\n",
> name, mrouter->vlan,
> + mcast_snooping_group_protocol_str(-1),
> mcast_mrouter_age(ofproto->ms, mrouter));
> }
I think the protocol string should be left aligned. Here is a diff:
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 434ea1670..a20c85ad0 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -6181,7 +6181,7 @@ ofproto_unixctl_mcast_snooping_show(struct unixctl_conn
*conn,
bundle = b->port;
ofputil_port_to_string(ofbundle_get_a_port(bundle)->up.ofp_port,
NULL, name, sizeof name);
- ds_put_format(&ds, "%5s %4d %8s ", name, grp->vlan,
+ ds_put_format(&ds, "%5s %4d %-8s ", name, grp->vlan,
mcast_snooping_group_protocol_str(
grp->protocol_version));
ipv6_format_mapped(&grp->addr, &ds);
@@ -6197,7 +6197,7 @@ ofproto_unixctl_mcast_snooping_show(struct unixctl_conn
*conn,
bundle = mrouter->port;
ofputil_port_to_string(ofbundle_get_a_port(bundle)->up.ofp_port,
NULL, name, sizeof name);
- ds_put_format(&ds, "%5s %4d %8s querier %3d\n",
+ ds_put_format(&ds, "%5s %4d %-8s querier %3d\n",
name, mrouter->vlan,
mcast_snooping_group_protocol_str(-1),
mcast_mrouter_age(ofproto->ms, mrouter));
diff --git a/tests/mcast-snooping.at b/tests/mcast-snooping.at
index 800a77504..a91b3e13a 100644
--- a/tests/mcast-snooping.at
+++ b/tests/mcast-snooping.at
@@ -45,8 +45,8 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p2 \
AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
port VLAN protocol GROUP Age
- 2 1725 UNKNOWN querier 0
- 2 1728 UNKNOWN querier 0
+ 2 1725 UNKNOWN querier 0
+ 2 1728 UNKNOWN querier 0
])
AT_CHECK([ovs-vsctl set Interface p2 options:tx_pcap=p2.pcap])
@@ -88,7 +88,7 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p2 \
AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
port VLAN protocol GROUP Age
- 2 0 MLDv1 ff02::1:ff0e:4c67 0
+ 2 0 MLDv1 ff02::1:ff0e:4c67 0
])
AT_CHECK([ovs-appctl mdb/flush br0], [0], [dnl
@@ -155,7 +155,7 @@ AT_CHECK([
], [0])
AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
port VLAN protocol GROUP Age
- 1 0 IGMPv1 224.1.1.1 0
+ 1 0 IGMPv1 224.1.1.1 0
])
AT_CHECK([ovs-appctl ofproto/trace
"in_port(3),eth(src=aa:55:aa:55:00:ff,dst=01:00:5e:01:01:01),eth_type(0x0800),ipv4(src=10.0.0.1,dst=224.1.1.1,proto=17,tos=0,ttl=64,frag=no),udp(src=0,dst=8000)"],
[0], [dnl
@@ -468,18 +468,18 @@ AT_CHECK([
AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
port VLAN protocol GROUP Age
- 1 1 IGMPv1 224.1.1.1 0
- 1 2 IGMPv1 224.1.1.1 0
- 3 1 UNKNOWN querier 0
- 3 2 UNKNOWN querier 0
+ 1 1 IGMPv1 224.1.1.1 0
+ 1 2 IGMPv1 224.1.1.1 0
+ 3 1 UNKNOWN querier 0
+ 3 2 UNKNOWN querier 0
])
AT_CHECK([ovs-vsctl set port p3 tag=2], [0])
AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
port VLAN protocol GROUP Age
- 1 1 IGMPv1 224.1.1.1 0
- 1 2 IGMPv1 224.1.1.1 0
+ 1 1 IGMPv1 224.1.1.1 0
+ 1 2 IGMPv1 224.1.1.1 0
])
AT_CLEANUP
@@ -523,18 +523,18 @@ AT_CHECK([
AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
port VLAN protocol GROUP Age
- 1 1 IGMPv1 224.1.1.1 0
- 1 2 IGMPv1 224.1.1.1 0
- 2 1 UNKNOWN querier 0
- 2 2 UNKNOWN querier 0
+ 1 1 IGMPv1 224.1.1.1 0
+ 1 2 IGMPv1 224.1.1.1 0
+ 2 1 UNKNOWN querier 0
+ 2 2 UNKNOWN querier 0
])
AT_CHECK([ovs-vsctl del-port br0 p2], [0])
AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
port VLAN protocol GROUP Age
- 1 1 IGMPv1 224.1.1.1 0
- 1 2 IGMPv1 224.1.1.1 0
+ 1 1 IGMPv1 224.1.1.1 0
+ 1 2 IGMPv1 224.1.1.1 0
])
AT_CLEANUP
@@ -644,9 +644,9 @@ AT_CHECK([
# the mdb with the appropriate protocol.
AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
port VLAN protocol GROUP Age
- 1 0 IGMPv1 224.1.1.1 0
- 1 0 IGMPv2 224.1.1.2 0
- 1 0 IGMPv3 233.54.12.230 0
+ 1 0 IGMPv1 224.1.1.1 0
+ 1 0 IGMPv2 224.1.1.2 0
+ 1 0 IGMPv3 233.54.12.230 0
])
# Send IGMPv1 report packet to address 224.1.1.2
@@ -659,9 +659,9 @@ AT_CHECK([
AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
port VLAN protocol GROUP Age
- 1 0 IGMPv1 224.1.1.1 0
- 1 0 IGMPv3 233.54.12.230 0
- 1 0 IGMPv1 224.1.1.2 0
+ 1 0 IGMPv1 224.1.1.1 0
+ 1 0 IGMPv3 233.54.12.230 0
+ 1 0 IGMPv1 224.1.1.2 0
])
# Flush the mdb.
@@ -681,7 +681,7 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p1 \
# the mdb with the appropriate protocol.
AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
port VLAN protocol GROUP Age
- 1 0 MLDv2 ff02::1:ff52:f3e1 0
- 1 0 MLDv1 ff02::1:ff0e:4c67 0
+ 1 0 MLDv2 ff02::1:ff52:f3e1 0
+ 1 0 MLDv1 ff02::1:ff0e:4c67 0
])
AT_CLEANUP
> ovs_rwlock_unlock(&ofproto->ms->rwlock);
> diff --git a/tests/mcast-snooping.at b/tests/mcast-snooping.at
> index 890e6aca0..800a77504 100644
> --- a/tests/mcast-snooping.at
> +++ b/tests/mcast-snooping.at
> @@ -44,9 +44,9 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p2 \
>
> '01005e0000015c8a38552552810006c0080046c000240000000001027c00ac111c01e0000001940400001164ec1e00000000027d000000000000000000000000'])
>
> AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> - port VLAN GROUP Age
> - 2 1725 querier 0
> - 2 1728 querier 0
> + port VLAN protocol GROUP Age
> + 2 1725 UNKNOWN querier 0
> + 2 1728 UNKNOWN querier 0
> ])
>
> AT_CHECK([ovs-vsctl set Interface p2 options:tx_pcap=p2.pcap])
> @@ -75,7 +75,7 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p2 \
>
> '01005e0000015c8a38552552810006bd080046c000240000000001027f00ac111901e0000001940400001164ec1000000000027d000000000000000000000000'])
>
> AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> - port VLAN GROUP Age
> + port VLAN protocol GROUP Age
> ])
>
>
> @@ -87,8 +87,8 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p2 \
>
> '3333ff0e4c67000c290e4c6786dd600000000020000100000000000000000000000000000000ff0200000000000000000001ff0e4c673a000502000001008300e7b800000000ff0200000000000000000001ff0e4c67'])
>
> AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> - port VLAN GROUP Age
> - 2 0 ff02::1:ff0e:4c67 0
> + port VLAN protocol GROUP Age
> + 2 0 MLDv1 ff02::1:ff0e:4c67 0
> ])
>
> AT_CHECK([ovs-appctl mdb/flush br0], [0], [dnl
> @@ -99,7 +99,7 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p2 \
>
> '3333ff0e4c67000c290e4c6786dd600000000020000100000000000000000000000000000000ff0200000000000000000001ff0e4c673a000502000001008300e7b000000000ff0200000000000000000001ff0e4c67'])
>
> AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> - port VLAN GROUP Age
> + port VLAN protocol GROUP Age
> ])
>
> OVS_VSWITCHD_STOP
> @@ -154,8 +154,8 @@ AT_CHECK([
>
> '01005E010101000C29A027A108004500001C000100004002CBAEAC10221EE001010112140CE9E0010101'
> ], [0])
> AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> - port VLAN GROUP Age
> - 1 0 224.1.1.1 0
> + port VLAN protocol GROUP Age
> + 1 0 IGMPv1 224.1.1.1 0
> ])
>
> AT_CHECK([ovs-appctl ofproto/trace
> "in_port(3),eth(src=aa:55:aa:55:00:ff,dst=01:00:5e:01:01:01),eth_type(0x0800),ipv4(src=10.0.0.1,dst=224.1.1.1,proto=17,tos=0,ttl=64,frag=no),udp(src=0,dst=8000)"],
> [0], [dnl
> @@ -467,19 +467,19 @@ AT_CHECK([
> ], [0])
>
> AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> - port VLAN GROUP Age
> - 1 1 224.1.1.1 0
> - 1 2 224.1.1.1 0
> - 3 1 querier 0
> - 3 2 querier 0
> + port VLAN protocol GROUP Age
> + 1 1 IGMPv1 224.1.1.1 0
> + 1 2 IGMPv1 224.1.1.1 0
> + 3 1 UNKNOWN querier 0
> + 3 2 UNKNOWN querier 0
> ])
>
> AT_CHECK([ovs-vsctl set port p3 tag=2], [0])
>
> AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> - port VLAN GROUP Age
> - 1 1 224.1.1.1 0
> - 1 2 224.1.1.1 0
> + port VLAN protocol GROUP Age
> + 1 1 IGMPv1 224.1.1.1 0
> + 1 2 IGMPv1 224.1.1.1 0
> ])
>
> AT_CLEANUP
> @@ -522,19 +522,19 @@ AT_CHECK([
> ], [0])
>
> AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> - port VLAN GROUP Age
> - 1 1 224.1.1.1 0
> - 1 2 224.1.1.1 0
> - 2 1 querier 0
> - 2 2 querier 0
> + port VLAN protocol GROUP Age
> + 1 1 IGMPv1 224.1.1.1 0
> + 1 2 IGMPv1 224.1.1.1 0
> + 2 1 UNKNOWN querier 0
> + 2 2 UNKNOWN querier 0
> ])
>
> AT_CHECK([ovs-vsctl del-port br0 p2], [0])
>
> AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> - port VLAN GROUP Age
> - 1 1 224.1.1.1 0
> - 1 2 224.1.1.1 0
> + port VLAN protocol GROUP Age
> + 1 1 IGMPv1 224.1.1.1 0
> + 1 2 IGMPv1 224.1.1.1 0
> ])
>
> AT_CLEANUP
> @@ -605,3 +605,83 @@
> recirc_id(<recirc>),in_port(1),ct_state(+new-inv+trk),eth_type(0x0800),ipv4(prot
> ])
>
> AT_CLEANUP
> +
> +AT_SETUP([mcast - mcast_group protocol updated in mdb])
> +OVS_VSWITCHD_START([])
> +
> +AT_CHECK([
> + ovs-vsctl set bridge br0 \
> + datapath_type=dummy \
> + mcast_snooping_enable=true \
> +], [0])
> +
> +AT_CHECK([ovs-ofctl add-flow br0 action=normal])
> +
> +AT_CHECK([
> + ovs-vsctl add-port br0 p1 -- set Interface p1 type=dummy \
> + other-config:hwaddr=aa:55:aa:55:00:01 ofport_request=1 \
> +], [0])
> +
> +# Send IGMPv1 report packet.
> +AT_CHECK([
> + ovs-appctl netdev-dummy/receive p1 \
> +
> '01005E010101000C29A027A18100000008004500001C000100004002CBAEAC10221EE001010112140CE9E0010101'
> +], [0])
> +
> +# Send IGMPv2 report packet.
> +AT_CHECK([
> + ovs-appctl netdev-dummy/receive p1 \
> +
> '01005e010102505400000103080046c00020000040000102f8110a000103e001010294040000160008fce0010102'
> +], [0])
> +
> +# Send IGMPv3 report packet.
> +AT_CHECK([
> + ovs-appctl netdev-dummy/receive p1 \
> +
> '01005e000016505400000003080046c00028000040000102f9f60a000003e0000016940400002200e3e10000000104000000e9360ce6'
> +], [0])
> +
> +# Check that all the ipv4 mcast groups were updated in
> +# the mdb with the appropriate protocol.
> +AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> + port VLAN protocol GROUP Age
> + 1 0 IGMPv1 224.1.1.1 0
> + 1 0 IGMPv2 224.1.1.2 0
> + 1 0 IGMPv3 233.54.12.230 0
> +])
> +
> +# Send IGMPv1 report packet to address 224.1.1.2
> +# and make sure that the protocol will be updated to
> +# IGMPV1.
> +AT_CHECK([
> + ovs-appctl netdev-dummy/receive p1 \
> +
> '01005e010102505400000103080046c00020000040000102f8110a000103e00101029404000012000cfce0010102'
> +], [0])
> +
> +AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> + port VLAN protocol GROUP Age
> + 1 0 IGMPv1 224.1.1.1 0
> + 1 0 IGMPv3 233.54.12.230 0
> + 1 0 IGMPv1 224.1.1.2 0
> +])
> +
> +# Flush the mdb.
> +AT_CHECK([ovs-appctl mdb/flush br0], [0], [dnl
> +table successfully flushed
> +])
> +
> +# Send MLDV2 packet.
> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 \
> +'333300000016d0509956ddf986dd60000000001c3a01fe80000000000000712065589886fa88ff0200000000000000000000000000168f00134d0000000104000000ff0200000000000000000001ff52f3e1'])
> +
> +# Send MLDV1 packet.
> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 \
> +'3333ff0e4c67000c290e4c6786dd600000000020000100000000000000000000000000000000ff0200000000000000000001ff0e4c673a000502000001008300e7b800000000ff0200000000000000000001ff0e4c67'])
> +
> +# Check that all the ipv6 mcast groups were updated in
> +# the mdb with the appropriate protocol.
> +AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> + port VLAN protocol GROUP Age
> + 1 0 MLDv2 ff02::1:ff52:f3e1 0
> + 1 0 MLDv1 ff02::1:ff0e:4c67 0
> +])
> +AT_CLEANUP
> diff --git a/tests/stp.at b/tests/stp.at
> index a6b6465d1..e7bf3958a 100644
> --- a/tests/stp.at
> +++ b/tests/stp.at
> @@ -583,13 +583,13 @@ AT_CHECK([ovs-appctl fdb/show br2], [0], [dnl
> ])
>
> AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> - port VLAN GROUP Age
> + port VLAN protocol GROUP Age
> ])
> AT_CHECK([ovs-appctl mdb/show br1], [0], [dnl
> - port VLAN GROUP Age
> + port VLAN protocol GROUP Age
> ])
> AT_CHECK([ovs-appctl mdb/show br2], [0], [dnl
> - port VLAN GROUP Age
> + port VLAN protocol GROUP Age
> ])
>
> AT_CLEANUP
> --
> 2.34.3
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
