Re: [PATCH net-next v4 5/5] selftests: net: bridge: add MRC and QQIC field encoding tests

[Ujjal Roy]

On Mon, Apr 13, 2026 at 2:18 PM Ido Schimmel <idosch@xxxxxxxxxx> wrote:
>
> See some comments below, but note that net-next is closed:
>
> https://lore.kernel.org/netdev/20260412142250.131bf997@xxxxxxxxxx/
>
> So you can either wait with v5 until it is open again or post it as RFC
> so that we can at least review (but not merge) it while net-next is
> closed.

Let me clear the changes asked here inline, so that I will be prepared
with v5 until net-next is open. You can ask me to send it as RFC v5,
if you have doubts about inline answers.

>
> On Sun, Apr 12, 2026 at 11:10:47AM +0000, Ujjal Roy wrote:
> > Enhance vlmc_query_intvl_test and vlmc_query_response_intvl_test in
> > bridge_vlan_mcast.sh to validate IGMPv3/MLDv2 protocol compliance for
> > MRC and QQIC field encoding across both linear and exponential ranges.
> >
> > TEST: Vlan multicast snooping enable                                [ OK ]
> > TEST: Vlan mcast_query_interval global option default value         [ OK ]
> > INFO: Vlan 10 mcast_query_interval (QQIC) test cases:
> > TEST: Number of tagged IGMPv2 general query                         [ OK ]
> > TEST: IGMPv3 QQIC linear value 60                                   [ OK ]
> > TEST: MLDv2 QQIC linear value 60                                    [ OK ]
> > TEST: IGMPv3 QQIC non linear value 160                              [ OK ]
> > TEST: MLDv2 QQIC non linear value 160                               [ OK ]
> > TEST: Vlan mcast_query_response_interval global option default value   [ OK ]
> > INFO: Vlan 10 mcast_query_response_interval (MRC) test cases:
> > TEST: IGMPv3 MRC linear value 60                                    [ OK ]
> > TEST: IGMPv3 MRC non linear value 160                               [ OK ]
> > TEST: MLDv2 MRC linear value 30000                                  [ OK ]
> > TEST: MLDv2 MRC non linear value 60000                              [ OK ]
> >
> > Signed-off-by: Ujjal Roy <royujjal@xxxxxxxxx>
> > ---
> >  .../net/forwarding/bridge_vlan_mcast.sh       | 150 +++++++++++++++++-
> >  1 file changed, 142 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/testing/selftests/net/forwarding/bridge_vlan_mcast.sh b/tools/testing/selftests/net/forwarding/bridge_vlan_mcast.sh
> > index e8031f68200a..9f9f33d58286 100755
> > --- a/tools/testing/selftests/net/forwarding/bridge_vlan_mcast.sh
> > +++ b/tools/testing/selftests/net/forwarding/bridge_vlan_mcast.sh
> > @@ -162,14 +162,27 @@ vlmc_query_cnt_setup()
> >  {
> >       local type=$1
> >       local dev=$2
> > +     local match=$3
> >
> >       if [[ $type == "igmp" ]]; then
> > -             tc filter add dev $dev egress pref 10 prot 802.1Q \
> > +             # This matches: IP Protocol 2 (IGMP)
> > +             tc filter add dev "$dev" egress pref 10 prot 802.1Q \
> >                       flower vlan_id 10 vlan_ethtype ipv4 dst_ip 224.0.0.1 ip_proto 2 \
> > +                     action continue
> > +             # AND Type 0x11 (Query) at offset 24 after IP
> > +             # IP (20 byte IP + 4 bytes Option)
>
> Let's make it clearer: 20 bytes IPv4 header + 4 bytes Router Alert option
             #    20 bytes IPv4 header + 4 bytes Router Alert option +
IGMP[offset 0] Query

>
> > +             match=(match u8 0x11 0xff at 24 $match)
> > +             tc filter add dev "$dev" egress pref 20 prot 802.1Q u32 "${match[@]}" \
> >                       action pass
> >       else
> > -             tc filter add dev $dev egress pref 10 prot 802.1Q \
> > +             # This matches: ICMPv6
> > +             tc filter add dev "$dev" egress pref 10 prot 802.1Q \
> >                       flower vlan_id 10 vlan_ethtype ipv6 dst_ip ff02::1 ip_proto icmpv6 \
> > +                     action continue
> > +             # AND Type 0x82 (Query) at offset 48 after IPv6
> > +             # IPv6 (40 bytes IPv6 + 2 bytes next HDR + 4 bytes Option + 2 byte pad)
>
> Same: 40 bytes IPv6 header + 8 bytes Hop-by-hop option
             #    40 bytes IPv6 header + 8 bytes Hop-by-hop option +
MLD[offset 0] Query

>
> > +             match=(match u8 0x82 0xff at 48 $match)
> > +             tc filter add dev "$dev" egress pref 20 prot 802.1Q u32 "${match[@]}" \
> >                       action pass
> >       fi
>
> Sashiko has a relevant comment:
>
> "
> Does this configuration evaluate all packets against the pref 20 filter,
> regardless of the pref 10 result?
>
> In tc, if a packet does not match a filter, classification automatically falls
> through to the next priority filter.  By using "action continue" on pref 10,
> matching packets are also instructed to continue evaluation at the next filter.
>
> Because both matching and non-matching packets proceed to pref 20, pref 10
> seems to act as a no-op gate.  Could this cause the u32 rules in pref 20 to
> inadvertently match unrelated background traffic on the interface?
>
> To implement a logical AND across different classifiers, should pref 10 use
> "action goto chain 1" with pref 20 placed inside chain 1?
> "
Answer: No, it should evaluate IGMP only by pref 10 filter AND IGMPv3
Query by pref 20 filter. Query filter may include additional match for
QQIC/MRC.

Here is my new filter:
             tc filter add dev "$dev" egress pref 10 prot 802.1Q \
                     flower vlan_id 10 vlan_ethtype ipv4 dst_ip
224.0.0.1 ip_proto 2 \
                     action goto chain 1

>
> >
> > @@ -181,7 +194,53 @@ vlmc_query_cnt_cleanup()
> >       local dev=$1
> >
> >       ip link set dev br0 type bridge mcast_stats_enabled 0
> > -     tc filter del dev $dev egress pref 10
> > +     tc filter del dev "$dev" egress pref 20
> > +     tc filter del dev "$dev" egress pref 10
> > +}
> > +
> > +vlmc_query_get_intvl_match()
> > +{
> > +     local type=$1
> > +     local version=$2
> > +     local test=$3
> > +     local interval=$4
> > +
> > +     if [ "$test" = "qqic" ]; then
> > +             # QQIC is 8-bit floating point encoding for IGMPv3 and MLDv2
> > +             if [ "${type}v${version}" = "igmpv3" ]; then
> > +                     # IP 20 bytes + 4 bytes Option + IGMPv3[9]
> > +                     if [[ $interval -lt 128 ]]; then
> > +                             echo "match u8 0x3c 0xff at 33"
>
> Please pass the expected value as an argument instead of hard coding
> "0x3c" here. Same in other places in the function.
Will pass the expected code as an argument. Also will update the comments here.
                     # 20 bytes IPv4 header + 4 bytes Router Alert
option + IGMPv3[offset 9] QQIC

>
> > +                     else
> > +                             echo "match u8 0x84 0xff at 33"
> > +                     fi
> > +             elif [ "${type}v${version}" = "mldv2" ]; then
> > +                     # IPv6 40 + 2 next HDR + 4 Option + 2 pad + MLDv2[25]
> > +                     if [[ $interval -lt 128 ]]; then
> > +                             echo "match u8 0x3c 0xff at 73"
> > +                     else
> > +                             echo "match u8 0x84 0xff at 73"
> > +                     fi
> > +             fi
> > +     elif [ "$test" = "mrc" ]; then
> > +             if [ "${type}v${version}" = "igmpv3" ]; then
> > +                     # MRC is 8-bit floating point encoding for IGMPv3
> > +                     # IP 20 bytes + 4 bytes Option + IGMPv3[1]
> > +                     if [[ $interval -lt 128 ]]; then
> > +                             echo "match u8 0x3c 0xff at 25"
> > +                     else
> > +                             echo "match u8 0x84 0xff at 25"
> > +                     fi
> > +             elif [ "${type}v${version}" = "mldv2" ]; then
> > +                     # MRC is 16-bit floating point encoding for MLDv2
> > +                     # IPv6 40 + 2 next HDR + 4 Option + 2 pad + MLDv2[4]
> > +                     if [[ $interval -lt 32768 ]]; then
> > +                             echo "match u16 0x7530 0xffff at 52"
> > +                     else
> > +                             echo "match u16 0x8d4c 0xffff at 52"
> > +                     fi
> > +             fi
> > +     fi
> >  }
> >
> >  vlmc_check_query()
> > @@ -191,9 +250,13 @@ vlmc_check_query()
> >       local dev=$3
> >       local expect=$4
> >       local time=$5
> > +     local test=$6
> > +     local interval=$7
> > +     local intvl_match=""
> >       local ret=0
> >
> > -     vlmc_query_cnt_setup $type $dev
> > +     intvl_match="$(vlmc_query_get_intvl_match "$type" "$version" "$test" "$interval")"
> > +     vlmc_query_cnt_setup "$type" "$dev" "$intvl_match"
> >
> >       local pre_tx_xstats=$(vlmc_query_cnt_xstats $type $version $dev)
> >       bridge vlan global set vid 10 dev br0 mcast_snooping 1 mcast_querier 1
> > @@ -201,7 +264,7 @@ vlmc_check_query()
> >       if [[ $ret -eq 0 ]]; then
> >               sleep $time
> >
> > -             local tcstats=$(tc_rule_stats_get $dev 10 egress)
> > +             local tcstats=$(tc_rule_stats_get "$dev" 20 egress)
> >               local post_tx_xstats=$(vlmc_query_cnt_xstats $type $version $dev)
> >
> >               if [[ $tcstats != $expect || \
> > @@ -441,6 +504,7 @@ vlmc_query_intvl_test()
> >       check_err $? "Wrong default mcast_query_interval global vlan option value"
> >       log_test "Vlan mcast_query_interval global option default value"
> >
> > +     log_info "Vlan 10 mcast_query_interval (QQIC) test cases:"
>
> Let's remove this as it makes the output confusing:
Sure, I will remove this line.

>
> INFO: Vlan 10 mcast_query_response_interval (MRC) test cases:
> TEST: IGMPv3 MRC linear value 60                                    [ OK ]
> [...]
> TEST: Flood unknown vlan multicast packets to router port only      [ OK ]
> TEST: Disable multicast vlan snooping when vlan filtering is disabled   [ OK ]
>
> >       RET=0
> >       bridge vlan global set vid 10 dev br0 mcast_snooping 1 mcast_startup_query_count 0
> >       bridge vlan global set vid 10 dev br0 mcast_snooping 1 mcast_query_interval 200
> > @@ -448,8 +512,42 @@ vlmc_query_intvl_test()
> >       # 1 is sent immediately, then 2 more in the next 5 seconds
> >       vlmc_check_query igmp 2 $swp1 3 5
> >       check_err $? "Wrong number of tagged IGMPv2 general queries sent"
> > -     log_test "Vlan 10 mcast_query_interval option changed to 200"
> > +     log_test "Number of tagged IGMPv2 general query"
> >
> > +     RET=0
> > +     bridge vlan global set vid 10 dev br0 mcast_snooping 1 mcast_igmp_version 3
> > +     check_err $? "Could not set mcast_igmp_version in vlan 10"
> > +     bridge vlan global set vid 10 dev br0 mcast_snooping 1 mcast_mld_version 2
> > +     check_err $? "Could not set mcast_mld_version in vlan 10"
> > +     bridge vlan global set vid 10 dev br0 mcast_snooping 1 mcast_query_interval 6000
> > +     check_err $? "Could not set mcast_query_interval in vlan 10"
> > +     # 1 is sent immediately, IGMPv3 QQIC should match with linear value 60s
> > +     vlmc_check_query igmp 3 $swp1 1 1 qqic 60
> > +     check_err $? "Wrong QQIC in generated IGMPv3 general queries"
> > +     log_test "IGMPv3 QQIC linear value 60"
> > +
> > +     RET=0
> > +     # 1 is sent immediately, MLDv2 QQIC should match with linear value 60s
> > +     vlmc_check_query mld 2 $swp1 1 1 qqic 60
> > +     check_err $? "Wrong QQIC in generated MLDv2 general queries"
> > +     log_test "MLDv2 QQIC linear value 60"
> > +
> > +     RET=0
> > +     bridge vlan global set vid 10 dev br0 mcast_snooping 1 mcast_query_interval 16000
> > +     check_err $? "Could not set mcast_query_interval in vlan 10"
> > +     # 1 is sent immediately, IGMPv3 QQIC should match with non linear value 160s
> > +     vlmc_check_query igmp 3 $swp1 1 1 qqic 160
> > +     check_err $? "Wrong QQIC in generated IGMPv3 general queries"
> > +     log_test "IGMPv3 QQIC non linear value 160"
> > +
> > +     RET=0
> > +     # 1 is sent immediately, MLDv2 QQIC should match with non linear value 160s
> > +     vlmc_check_query mld 2 $swp1 1 1 qqic 160
> > +     check_err $? "Wrong QQIC in generated MLDv2 general queries"
> > +     log_test "MLDv2 QQIC non linear value 160"
> > +
> > +     bridge vlan global set vid 10 dev br0 mcast_snooping 1 mcast_igmp_version 2
> > +     bridge vlan global set vid 10 dev br0 mcast_snooping 1 mcast_mld_version 1
> >       bridge vlan global set vid 10 dev br0 mcast_snooping 1 mcast_startup_query_count 2
> >       bridge vlan global set vid 10 dev br0 mcast_snooping 1 mcast_query_interval 12500
> >  }
> > @@ -468,11 +566,47 @@ vlmc_query_response_intvl_test()
> >       check_err $? "Wrong default mcast_query_response_interval global vlan option value"
> >       log_test "Vlan mcast_query_response_interval global option default value"
> >
> > +     log_info "Vlan 10 mcast_query_response_interval (MRC) test cases:"
>
> Same
I will remove this line also.

[...]