On 30 Nov 2023, at 16:38, Mohammad Heib wrote:
> Expose the mcast group protocol via the mdb/show
> command output.
Thanks for adding this additional patch to help debugging.
I have some comments regarding the output format, see below.
Cheers.
Eelco
> Signed-off-by: Mohammad Heib <mh...@redhat.com>
> ---
> lib/mcast-snooping.c | 24 ++++++++++
> lib/mcast-snooping.h | 1 +
> ofproto/ofproto-dpif.c | 6 ++-
> tests/mcast-snooping.at | 103 ++++++++++++++++++++++++++++++++--------
> tests/stp.at | 6 +--
> 5 files changed, 116 insertions(+), 24 deletions(-)
>
> diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
> index 5046e35d2..d72cac0a4 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_proto_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 "";
Not sure, but would it be good to show ‘-‘ or ‘UNKNOWN’ if the protocol is
unavailable? For the later, the display with need to be adjusted (the header
could say PROTOCOL instead of PROTO).
> + }
> +}
> +
> bool
> mcast_snooping_is_query(ovs_be16 igmp_type)
> {
> diff --git a/lib/mcast-snooping.h b/lib/mcast-snooping.h
> index f54007740..4ab83b049 100644
> --- a/lib/mcast-snooping.h
> +++ b/lib/mcast-snooping.h
> @@ -225,6 +225,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_proto_str(mcast_group_proto grp_proto);
>
> /* Flush. */
> void mcast_snooping_mdb_flush(struct mcast_snooping *ms);
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 54e057d43..b116d84f9 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -6122,7 +6122,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 PROTO 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) {
> @@ -6131,7 +6131,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 %5s ", name, grp->vlan,
Max protocol length is 6, so should be %6s + header adjust.
> + mcast_snooping_group_proto_str(
> + grp->protocol_version));
> ipv6_format_mapped(&grp->addr, &ds);
> ds_put_format(&ds, " %3d\n",
> mcast_bundle_age(ofproto->ms, b));
> diff --git a/tests/mcast-snooping.at b/tests/mcast-snooping.at
> index 890e6aca0..02be196e4 100644
> --- a/tests/mcast-snooping.at
> +++ b/tests/mcast-snooping.at
> @@ -44,7 +44,7 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p2 \
>
> '01005e0000015c8a38552552810006c0080046c000240000000001027c00ac111c01e0000001940400001164ec1e00000000027d000000000000000000000000'])
>
> AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> - port VLAN GROUP Age
> + port VLAN PROTO GROUP Age
> 2 1725 querier 0
> 2 1728 querier 0
Here you see that the entries are not correctly aligned. You need to add the
PROTO space also to the querier loop below in
ofproto_unixctl_mcast_snooping_show().
> ])
> @@ -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 PROTO 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 PROTO 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 PROTO 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 PROTO GROUP Age
> + 1 0 IGMPv1 224.1.1.1 0
Here you see that the IP is not aligned to GROUP, this is related to the
comment on using %6s above.
> ])
>
> 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,9 +467,9 @@ 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
> + port VLAN PROTO GROUP Age
> + 1 1 IGMPv1 224.1.1.1 0
> + 1 2 IGMPv1 224.1.1.1 0
> 3 1 querier 0
> 3 2 querier 0
> ])
> @@ -477,9 +477,9 @@ AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> 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 PROTO GROUP Age
> + 1 1 IGMPv1 224.1.1.1 0
> + 1 2 IGMPv1 224.1.1.1 0
> ])
>
> AT_CLEANUP
> @@ -522,9 +522,9 @@ 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
> + port VLAN PROTO GROUP Age
> + 1 1 IGMPv1 224.1.1.1 0
> + 1 2 IGMPv1 224.1.1.1 0
> 2 1 querier 0
> 2 2 querier 0
> ])
> @@ -532,9 +532,9 @@ AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> 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 PROTO GROUP Age
> + 1 1 IGMPv1 224.1.1.1 0
> + 1 2 IGMPv1 224.1.1.1 0
> ])
>
> AT_CLEANUP
> @@ -605,3 +605,68 @@
> 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
Start with a capital and end with a . for all comments.
> +AT_CHECK([
> + ovs-appctl netdev-dummy/receive p1 \
> +
> '01005E010101000C29A027A18100000108004500001C000100004002CBAEAC10221EE001010112140CE9E0010101'
> +], [0])
> +
> +# send IGMPv2 report packet
> +AT_CHECK([
> + ovs-appctl netdev-dummy/receive p1 \
> +
> '01005E010101000C29A027A18100000108004500001C000100004002CBAEAC10221EE0010101161408E8E0010102'
> +], [0])
> +
> +# send IGMPv3 report packet
> +AT_CHECK([
> + ovs-appctl netdev-dummy/receive p1 \
> +
> '01005e000016505400000003080046c00028000040000102f9f60a000003e0000016940400002200e3e10000000104000000e9360ce6'
> +], [0])
> +
> +# Check that all the ipv4 mcast groups was updated in
... were updated in
> +# the mdb with the the appropriate protocol.
Double ‘the’
Same two comments in the comment below on ipv6.
> +AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> + port VLAN PROTO GROUP Age
> + 1 1 IGMPv1 224.1.1.1 0
> + 1 1 IGMPv2 224.1.1.2 0
> + 1 0 IGMPv3 233.54.12.230 0
> +])
Do we want a tests case where we have an IGMPv1 entry and later a v3 was sent
so the dbase gets updated? I’m assuming that we store the latest received
protocol.
> +
> +# 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 was updated in
> +# the mdb with the the appropriate protocol.
> +AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
> + port VLAN PROTO 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..de137d5d3 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 PROTO GROUP Age
> ])
> AT_CHECK([ovs-appctl mdb/show br1], [0], [dnl
> - port VLAN GROUP Age
> + port VLAN PROTO GROUP Age
> ])
> AT_CHECK([ovs-appctl mdb/show br2], [0], [dnl
> - port VLAN GROUP Age
> + port VLAN PROTO GROUP Age
> ])
>
> AT_CLEANUP
> --
> 2.34.3
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
