* [PATCH net 0/2] selftests: pmtu: fix and increase coverage @ 2019-02-22 16:06 Paolo Abeni 2019-02-22 16:06 ` [PATCH net 1/2] selftests: pmtu: disable dad in all namespaces Paolo Abeni 2019-02-22 16:06 ` [PATCH net 2/2] selftests: pmtu: add explicit tests for pmtu exceptions cleanup Paolo Abeni 0 siblings, 2 replies; 7+ messages in thread From: Paolo Abeni @ 2019-02-22 16:06 UTC (permalink / raw) To: netdev; +Cc: David S. Miller, David Ahern, Stefano Brivio This series includes a fixup for the pmtu test script, related to ipv6 address management, and adds coverage for the recently reported and fixed pmtu exception issue Paolo Abeni (2): selftests: pmtu: disable dad in all namespaces selftests: pmtu: add explicit tests for pmtu exceptions cleanup tools/testing/selftests/net/pmtu.sh | 80 +++++++++++++++++++++++------ 1 file changed, 64 insertions(+), 16 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net 1/2] selftests: pmtu: disable dad in all namespaces 2019-02-22 16:06 [PATCH net 0/2] selftests: pmtu: fix and increase coverage Paolo Abeni @ 2019-02-22 16:06 ` Paolo Abeni 2019-02-22 17:49 ` Stefano Brivio 2019-02-22 22:05 ` David Ahern 2019-02-22 16:06 ` [PATCH net 2/2] selftests: pmtu: add explicit tests for pmtu exceptions cleanup Paolo Abeni 1 sibling, 2 replies; 7+ messages in thread From: Paolo Abeni @ 2019-02-22 16:06 UTC (permalink / raw) To: netdev; +Cc: David S. Miller, David Ahern, Stefano Brivio Otherwise, the configured IPv6 address could be still "tentative" at test time, possibly causing tests failures. We can also drop some sleep along the code and decrease the timeout for most commands so that the test runtime decreases. Fixes: d1f1b9cbf34c ("selftests: net: Introduce first PMTU test") Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- tools/testing/selftests/net/pmtu.sh | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh index e2c94e47707c..634e91e8fe25 100755 --- a/tools/testing/selftests/net/pmtu.sh +++ b/tools/testing/selftests/net/pmtu.sh @@ -263,8 +263,6 @@ setup_fou_or_gue() { ${ns_a} ip link set ${encap}_a up ${ns_b} ip link set ${encap}_b up - - sleep 1 } setup_fou44() { @@ -302,6 +300,10 @@ setup_gue66() { setup_namespaces() { for n in ${NS_A} ${NS_B} ${NS_R1} ${NS_R2}; do ip netns add ${n} || return 1 + + # disable dad, so that we don't have to wait to use the + # configured IPv6 addresses + ip netns exec ${n} sysctl -q net/ipv6/conf/default/accept_dad=0 done } @@ -337,8 +339,6 @@ setup_vti() { ${ns_a} ip link set vti${proto}_a up ${ns_b} ip link set vti${proto}_b up - - sleep 1 } setup_vti4() { @@ -375,8 +375,6 @@ setup_vxlan_or_geneve() { ${ns_a} ip link set ${type}_a up ${ns_b} ip link set ${type}_b up - - sleep 1 } setup_geneve4() { @@ -588,8 +586,8 @@ test_pmtu_ipvX() { mtu "${ns_b}" veth_B-R2 1500 # Create route exceptions - ${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s 1800 ${dst1} > /dev/null - ${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s 1800 ${dst2} > /dev/null + ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1800 ${dst1} > /dev/null + ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1800 ${dst2} > /dev/null # Check that exceptions have been created with the correct PMTU pmtu_1="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst1})" @@ -621,7 +619,7 @@ test_pmtu_ipvX() { # Decrease remote MTU on path via R2, get new exception mtu "${ns_r2}" veth_R2-B 400 mtu "${ns_b}" veth_B-R2 400 - ${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s 1400 ${dst2} > /dev/null + ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1400 ${dst2} > /dev/null pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})" check_pmtu_value "lock 552" "${pmtu_2}" "exceeding MTU, with MTU < min_pmtu" || return 1 @@ -638,7 +636,7 @@ test_pmtu_ipvX() { check_pmtu_value "1500" "${pmtu_2}" "increasing local MTU" || return 1 # Get new exception - ${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s 1400 ${dst2} > /dev/null + ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1400 ${dst2} > /dev/null pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})" check_pmtu_value "lock 552" "${pmtu_2}" "exceeding MTU, with MTU < min_pmtu" || return 1 } @@ -687,7 +685,7 @@ test_pmtu_ipvX_over_vxlanY_or_geneveY_exception() { mtu "${ns_a}" ${type}_a $((${ll_mtu} + 1000)) mtu "${ns_b}" ${type}_b $((${ll_mtu} + 1000)) - ${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s $((${ll_mtu} + 500)) ${dst} > /dev/null + ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${dst} > /dev/null # Check that exception was created pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst})" @@ -767,7 +765,7 @@ test_pmtu_ipvX_over_fouY_or_gueY() { mtu "${ns_a}" ${encap}_a $((${ll_mtu} + 1000)) mtu "${ns_b}" ${encap}_b $((${ll_mtu} + 1000)) - ${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s $((${ll_mtu} + 500)) ${dst} > /dev/null + ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${dst} > /dev/null # Check that exception was created pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst})" @@ -825,13 +823,13 @@ test_pmtu_vti4_exception() { # Send DF packet without exceeding link layer MTU, check that no # exception is created - ${ns_a} ping -q -M want -i 0.1 -w 2 -s ${ping_payload} ${tunnel4_b_addr} > /dev/null + ${ns_a} ping -q -M want -i 0.1 -w 1 -s ${ping_payload} ${tunnel4_b_addr} > /dev/null pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel4_b_addr})" check_pmtu_value "" "${pmtu}" "sending packet smaller than PMTU (IP payload length ${esp_payload_rfc4106})" || return 1 # Now exceed link layer MTU by one byte, check that exception is created # with the right PMTU value - ${ns_a} ping -q -M want -i 0.1 -w 2 -s $((ping_payload + 1)) ${tunnel4_b_addr} > /dev/null + ${ns_a} ping -q -M want -i 0.1 -w 1 -s $((ping_payload + 1)) ${tunnel4_b_addr} > /dev/null pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel4_b_addr})" check_pmtu_value "${esp_payload_rfc4106}" "${pmtu}" "exceeding PMTU (IP payload length $((esp_payload_rfc4106 + 1)))" } @@ -847,7 +845,7 @@ test_pmtu_vti6_exception() { mtu "${ns_b}" veth_b 4000 mtu "${ns_a}" vti6_a 5000 mtu "${ns_b}" vti6_b 5000 - ${ns_a} ${ping6} -q -i 0.1 -w 2 -s 60000 ${tunnel6_b_addr} > /dev/null + ${ns_a} ${ping6} -q -i 0.1 -w 1 -s 60000 ${tunnel6_b_addr} > /dev/null # Check that exception was created pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel6_b_addr})" -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net 1/2] selftests: pmtu: disable dad in all namespaces 2019-02-22 16:06 ` [PATCH net 1/2] selftests: pmtu: disable dad in all namespaces Paolo Abeni @ 2019-02-22 17:49 ` Stefano Brivio 2019-02-22 22:05 ` David Ahern 1 sibling, 0 replies; 7+ messages in thread From: Stefano Brivio @ 2019-02-22 17:49 UTC (permalink / raw) To: Paolo Abeni; +Cc: netdev, David S. Miller, David Ahern On Fri, 22 Feb 2019 17:06:32 +0100 Paolo Abeni <pabeni@redhat.com> wrote: > Otherwise, the configured IPv6 address could be still "tentative" > at test time, possibly causing tests failures. > We can also drop some sleep along the code and decrease the > timeout for most commands so that the test runtime decreases. Thanks for fixing this! I tried this back then and it didn't work, perhaps some DAD changes intervened meanwhile (or I simply tried it wrong). Just one comment: > Fixes: d1f1b9cbf34c ("selftests: net: Introduce first PMTU test") > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > tools/testing/selftests/net/pmtu.sh | 28 +++++++++++++--------------- > 1 file changed, 13 insertions(+), 15 deletions(-) > > diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh > index e2c94e47707c..634e91e8fe25 100755 > --- a/tools/testing/selftests/net/pmtu.sh > +++ b/tools/testing/selftests/net/pmtu.sh > @@ -263,8 +263,6 @@ setup_fou_or_gue() { > > ${ns_a} ip link set ${encap}_a up > ${ns_b} ip link set ${encap}_b up > - > - sleep 1 > } > > setup_fou44() { > @@ -302,6 +300,10 @@ setup_gue66() { > setup_namespaces() { > for n in ${NS_A} ${NS_B} ${NS_R1} ${NS_R2}; do > ip netns add ${n} || return 1 > + > + # disable dad, so that we don't have to wait to use the > + # configured IPv6 addresses For consistency: "Disable DAD ..." -- Stefano ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net 1/2] selftests: pmtu: disable dad in all namespaces 2019-02-22 16:06 ` [PATCH net 1/2] selftests: pmtu: disable dad in all namespaces Paolo Abeni 2019-02-22 17:49 ` Stefano Brivio @ 2019-02-22 22:05 ` David Ahern 1 sibling, 0 replies; 7+ messages in thread From: David Ahern @ 2019-02-22 22:05 UTC (permalink / raw) To: Paolo Abeni, netdev; +Cc: David S. Miller, Stefano Brivio On 2/22/19 11:06 AM, Paolo Abeni wrote: > Otherwise, the configured IPv6 address could be still "tentative" > at test time, possibly causing tests failures. > We can also drop some sleep along the code and decrease the > timeout for most commands so that the test runtime decreases. > > Fixes: d1f1b9cbf34c ("selftests: net: Introduce first PMTU test") > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > tools/testing/selftests/net/pmtu.sh | 28 +++++++++++++--------------- > 1 file changed, 13 insertions(+), 15 deletions(-) > Reviewed-by: David Ahern <dsahern@gmail.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net 2/2] selftests: pmtu: add explicit tests for pmtu exceptions cleanup 2019-02-22 16:06 [PATCH net 0/2] selftests: pmtu: fix and increase coverage Paolo Abeni 2019-02-22 16:06 ` [PATCH net 1/2] selftests: pmtu: disable dad in all namespaces Paolo Abeni @ 2019-02-22 16:06 ` Paolo Abeni 2019-02-22 17:50 ` Stefano Brivio 2019-02-22 22:22 ` Stefano Brivio 1 sibling, 2 replies; 7+ messages in thread From: Paolo Abeni @ 2019-02-22 16:06 UTC (permalink / raw) To: netdev; +Cc: David S. Miller, David Ahern, Stefano Brivio Add a couple of new tests, explicitly checking that the kernel timely releases pmtu exceptions on related device removal. This is mostly a regression test vs the issue fixed by: https://patchwork.ozlabs.org/patch/1045488/ Only 2 new test cases have been added, instead of extending all the existing ones, because the reproducer requires executing several commands and would slow down too much the tests otherwise. Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- tools/testing/selftests/net/pmtu.sh | 52 ++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh index 634e91e8fe25..4bc72bc26899 100755 --- a/tools/testing/selftests/net/pmtu.sh +++ b/tools/testing/selftests/net/pmtu.sh @@ -135,7 +135,9 @@ tests=" pmtu_vti6_default_mtu vti6: default MTU assignment pmtu_vti4_link_add_mtu vti4: MTU setting on link creation pmtu_vti6_link_add_mtu vti6: MTU setting on link creation - pmtu_vti6_link_change_mtu vti6: MTU changes on link changes" + pmtu_vti6_link_change_mtu vti6: MTU changes on link changes + pmtu_ipv4_exception_cleanup ipv4: cleanup of cached exceptions + pmtu_ipv6_exception_cleanup ipv6: cleanup of cached exceptions" NS_A="ns-$(mktemp -u XXXXXX)" NS_B="ns-$(mktemp -u XXXXXX)" @@ -1006,6 +1008,54 @@ test_pmtu_vti6_link_change_mtu() { return ${fail} } +test_cleanup_vxlanY_or_geneveY_exception() { + outer="${1}" + encap="${2}" + ll_mtu=4000 + + which taskset 2>/dev/null | return 2 + which timeout 2>/dev/null | return 2 + cpu_list=`grep processor /proc/cpuinfo | cut -d ' ' -f 2` + + setup namespaces routing ${encap}${outer} || return 2 + trace "${ns_a}" ${encap}_a "${ns_b}" ${encap}_b \ + "${ns_a}" veth_A-R1 "${ns_r1}" veth_R1-A \ + "${ns_b}" veth_B-R1 "${ns_r1}" veth_R1-B + + # Create route exception by exceeding link layer MTU + mtu "${ns_a}" veth_A-R1 $((${ll_mtu} + 1000)) + mtu "${ns_r1}" veth_R1-A $((${ll_mtu} + 1000)) + mtu "${ns_b}" veth_B-R1 ${ll_mtu} + mtu "${ns_r1}" veth_R1-B ${ll_mtu} + + mtu "${ns_a}" ${encap}_a $((${ll_mtu} + 1000)) + mtu "${ns_b}" ${encap}_b $((${ll_mtu} + 1000)) + + # fill exception cache for multiple cpus [2] + # we can always use inner ipv4 for that + ncpus=0 + for cpu in $cpu_list; do + taskset --cpu-list ${cpu} ${ns_a} ping -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${tunnel4_b_addr} > /dev/null + ncpus=$((ncpus + 1)) + [ ${ncpus} -gt 1 ] && break + done + + fail=0 + if ! timeout 1 ${ns_a} ip link del dev veth_A-R1; then + err " can't delete veth device in a timely manner, pmtu dst likely leaked" + fail=1 + fi + return ${fail} +} + +test_pmtu_ipv6_exception_cleanup() { + test_cleanup_vxlanY_or_geneveY_exception 6 vxlan +} + +test_pmtu_ipv4_exception_cleanup() { + test_cleanup_vxlanY_or_geneveY_exception 4 vxlan +} + usage() { echo echo "$0 [OPTIONS] [TEST]..." -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net 2/2] selftests: pmtu: add explicit tests for pmtu exceptions cleanup 2019-02-22 16:06 ` [PATCH net 2/2] selftests: pmtu: add explicit tests for pmtu exceptions cleanup Paolo Abeni @ 2019-02-22 17:50 ` Stefano Brivio 2019-02-22 22:22 ` Stefano Brivio 1 sibling, 0 replies; 7+ messages in thread From: Stefano Brivio @ 2019-02-22 17:50 UTC (permalink / raw) To: Paolo Abeni; +Cc: netdev, David S. Miller, David Ahern On Fri, 22 Feb 2019 17:06:33 +0100 Paolo Abeni <pabeni@redhat.com> wrote: > Add a couple of new tests, explicitly checking that the kernel > timely releases pmtu exceptions on related device removal. > This is mostly a regression test vs the issue fixed by: > https://patchwork.ozlabs.org/patch/1045488/ > > Only 2 new test cases have been added, instead of extending all > the existing ones, because the reproducer requires executing > several commands and would slow down too much the tests otherwise. > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > tools/testing/selftests/net/pmtu.sh | 52 ++++++++++++++++++++++++++++- > 1 file changed, 51 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh > index 634e91e8fe25..4bc72bc26899 100755 > --- a/tools/testing/selftests/net/pmtu.sh > +++ b/tools/testing/selftests/net/pmtu.sh > @@ -135,7 +135,9 @@ tests=" > pmtu_vti6_default_mtu vti6: default MTU assignment > pmtu_vti4_link_add_mtu vti4: MTU setting on link creation > pmtu_vti6_link_add_mtu vti6: MTU setting on link creation > - pmtu_vti6_link_change_mtu vti6: MTU changes on link changes" > + pmtu_vti6_link_change_mtu vti6: MTU changes on link changes > + pmtu_ipv4_exception_cleanup ipv4: cleanup of cached exceptions > + pmtu_ipv6_exception_cleanup ipv6: cleanup of cached exceptions" > > NS_A="ns-$(mktemp -u XXXXXX)" > NS_B="ns-$(mktemp -u XXXXXX)" > @@ -1006,6 +1008,54 @@ test_pmtu_vti6_link_change_mtu() { > return ${fail} > } > > +test_cleanup_vxlanY_or_geneveY_exception() { > + outer="${1}" > + encap="${2}" As you don't implement GENEVE tests, this function can be kept simpler. > + ll_mtu=4000 > + > + which taskset 2>/dev/null | return 2 > + which timeout 2>/dev/null | return 2 This test will not be skipped if taskset or timeout are not available, I guess you meant: || return 2. Besides, 'which' can output to both stdout and stderr. See how it's used anywhere else in this script, e.g.: which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping) You might also want to skip this test on non-SMP. You should also indicate why the test is skipped, see how it's done in other places, e.g.: ${ns_a} ip link add dummy0 mtu 1500 type dummy [ $? -ne 0 ] && err " dummy not supported" && return 2 > + cpu_list=`grep processor /proc/cpuinfo | cut -d ' ' -f 2` For consistency, please use $( ... ) If you grep -m2, then you can skip the ncpus thing below. > + > + setup namespaces routing ${encap}${outer} || return 2 > + trace "${ns_a}" ${encap}_a "${ns_b}" ${encap}_b \ > + "${ns_a}" veth_A-R1 "${ns_r1}" veth_R1-A \ > + "${ns_b}" veth_B-R1 "${ns_r1}" veth_R1-B > + > + # Create route exception by exceeding link layer MTU > + mtu "${ns_a}" veth_A-R1 $((${ll_mtu} + 1000)) > + mtu "${ns_r1}" veth_R1-A $((${ll_mtu} + 1000)) > + mtu "${ns_b}" veth_B-R1 ${ll_mtu} > + mtu "${ns_r1}" veth_R1-B ${ll_mtu} > + > + mtu "${ns_a}" ${encap}_a $((${ll_mtu} + 1000)) > + mtu "${ns_b}" ${encap}_b $((${ll_mtu} + 1000)) > + > + # fill exception cache for multiple cpus [2] > + # we can always use inner ipv4 for that Please keep comments consistent with the rest, "Fill ... CPUs (2) ... IPv4". > + ncpus=0 > + for cpu in $cpu_list; do Nit: ${cpu_list} > + taskset --cpu-list ${cpu} ${ns_a} ping -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${tunnel4_b_addr} > /dev/null > + ncpus=$((ncpus + 1)) > + [ ${ncpus} -gt 1 ] && break > + done > + > + fail=0 No need for fail=0... > + if ! timeout 1 ${ns_a} ip link del dev veth_A-R1; then > + err " can't delete veth device in a timely manner, pmtu dst likely leaked" (For consistency: "PMTU") > + fail=1 ...just return 1 here and > + fi > + return ${fail} ...skip the explicit return. The return code comes from the last command executed. > +} > + > +test_pmtu_ipv6_exception_cleanup() { > + test_cleanup_vxlanY_or_geneveY_exception 6 vxlan > +} > + > +test_pmtu_ipv4_exception_cleanup() { > + test_cleanup_vxlanY_or_geneveY_exception 4 vxlan > +} > + > usage() { > echo > echo "$0 [OPTIONS] [TEST]..." -- Stefano ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net 2/2] selftests: pmtu: add explicit tests for pmtu exceptions cleanup 2019-02-22 16:06 ` [PATCH net 2/2] selftests: pmtu: add explicit tests for pmtu exceptions cleanup Paolo Abeni 2019-02-22 17:50 ` Stefano Brivio @ 2019-02-22 22:22 ` Stefano Brivio 1 sibling, 0 replies; 7+ messages in thread From: Stefano Brivio @ 2019-02-22 22:22 UTC (permalink / raw) To: Paolo Abeni; +Cc: netdev, David S. Miller, David Ahern On Fri, 22 Feb 2019 17:06:33 +0100 Paolo Abeni <pabeni@redhat.com> wrote: > @@ -135,7 +135,9 @@ tests=" > pmtu_vti6_default_mtu vti6: default MTU assignment > pmtu_vti4_link_add_mtu vti4: MTU setting on link creation > pmtu_vti6_link_add_mtu vti6: MTU setting on link creation > - pmtu_vti6_link_change_mtu vti6: MTU changes on link changes" > + pmtu_vti6_link_change_mtu vti6: MTU changes on link changes > + pmtu_ipv4_exception_cleanup ipv4: cleanup of cached exceptions > + pmtu_ipv6_exception_cleanup ipv6: cleanup of cached exceptions" I forgot: while at it, could you please also add test descriptions above as it was done for the other tests? Thank you. -- Stefano ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-02-22 22:22 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-02-22 16:06 [PATCH net 0/2] selftests: pmtu: fix and increase coverage Paolo Abeni 2019-02-22 16:06 ` [PATCH net 1/2] selftests: pmtu: disable dad in all namespaces Paolo Abeni 2019-02-22 17:49 ` Stefano Brivio 2019-02-22 22:05 ` David Ahern 2019-02-22 16:06 ` [PATCH net 2/2] selftests: pmtu: add explicit tests for pmtu exceptions cleanup Paolo Abeni 2019-02-22 17:50 ` Stefano Brivio 2019-02-22 22:22 ` Stefano Brivio
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).