| Message ID | TY2PR01MB212429263FC238F44A2C1D7D89C79@TY2PR01MB2124.jpnprd01.prod.outlook.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | network/mpls: sleep 1 after setup in mpls02 | expand |
On 26.08.2021 08:26, suy.fnst@fujitsu.com wrote: > While running mpls02.sh -6 in our slow test box with kernel > version > v5.11, we found the test hangs at ping6 occasionally: > > + grep -q 'invalid option' > + for size in ${msg_sizes:-"56"} > + EXPECT_PASS ping6 -I fd00:23::2 -c 500 -s 10 -f fd00:23::1 > '>/dev/null' > + _tst_expect_pass tst_res ping6 -I fd00:23::2 -c 500 -s 10 -f > fd00:23::1 '>/dev/null' > + local fnc=tst_res > + shift > + tst_rod ping6 -I fd00:23::2 -c 500 -s 10 -f fd00:23::1 '>/dev/null' > > The weird part is that manual ping6 works in the meantime. > > Then we found that adding `sleep 1` at end of mpls02.sh/setup() make > the test pass in 100%. Dig depper in the problem, ftrace catched during > the 1 second shows (The test started at [ 3979.485488]): > > 3981.210701 | 2) kworker-24742 | | addrconf_dad_work() { > 3981.210712 | 2) kworker-24742 | | addrconf_dad_completed() { > 3981.210712 | 2) kworker-24742 | 0.417 us | addrconf_del_dad_work(); > 3981.210713 | 2) kworker-24742 | | __ipv6_ifa_notify() { > 3981.210716 | 2) kworker-24742 | 1.097 us | inet6_fill_ifaddr(); > 3981.210730 | 2) kworker-24742 | | ip6_ins_rt() { > 3981.210731 | 2) kworker-24742 | | fib6_add() { > > In kernel, addrconf_dad_work() is delegated by inet6_addr_add() to do > route related jobs in a delayed workqueue . Hence, there is tiny period > we need to wait for it. Adding the `sleep 1` seems suffcient for now. What is happening in the ping6, it is OK for it to hang on this? Adding nodad to loopback might only change the flags (remove tentative if ifa_flags==IFA_F_NODAD), the delayed dad work should be completed in addrconf_dad_begin() for it anyway... but will it make any difference for ping6 to change the test as below? ip addr add $ip_loc/$mask dev lo nodad tst_rhost_run -s -c "ip addr add $ip_rmt/$mask dev lo nodad" > Signed-off-by: Su Yue <suy.fnst@fujitsu.com> > --- > testcases/network/mpls/mpls02.sh | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/testcases/network/mpls/mpls02.sh b/testcases/network/mpls/mpls02.sh > index 2fd3ec5bf4ba..75f5fca32f9a 100755 > --- a/testcases/network/mpls/mpls02.sh > +++ b/testcases/network/mpls/mpls02.sh > @@ -35,6 +35,8 @@ setup() > tst_rhost_run -s -c "ip addr add $ip_rmt/$mask dev lo" > tst_rhost_run -s -c "ip route add $ip_loc/$mask encap mpls 60 via inet$TST_IPV6 $(tst_ipaddr)" > tst_rhost_run -s -c "ip -f mpls route add 50 dev lo" > + > + sleep 1 > } > > do_test() >
Hi, Alexey Sorry for late reply and the bad email format. And thanks a lot for your suggestion! It inspire me to find the cause. I'll explain it in V2. -- Su
Hi,
I found that it's indeed related to ipv6 DAD as you said.
Calling
`ip netns exec ltp_ns sysctl -n net.ipv6.conf.ltp_ns_veth1.accept_dad=0`
or tst_wait_ipv6_dad() at end of the setup both solves the problem.
However there is one super strange part that the tentative address is
the local link adress of the ltp_ns_veth1:
5: ltp_ns_veth1@if4: <BROADCAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
link/ether f2:8f:24:d4:ba:26 brd ff:ff:ff:ff:ff:ff link-netnsid 0
inet 10.0.0.1/24 scope global ltp_ns_veth1
valid_lft forever preferred_lft forever
inet6 fd00:1:1:1::1/64 scope global nodad
valid_lft forever preferred_lft forever
inet6 fe80::f08f:24ff:fed4:ba26/64 scope link tentative <-------------------
valid_lft forever preferred_lft forever
However, there is no place using the address in mpls02 test.
It makes me wonder how could it be possible to trigger the issue..
Thanks!
--
Su
Hi Su, Alexey, On 2021-08-30 11:26, suy.fnst@fujitsu.com wrote: > Hi, > I found that it's indeed related to ipv6 DAD as you said. > Calling > `ip netns exec ltp_ns sysctl -n > net.ipv6.conf.ltp_ns_veth1.accept_dad=0` > or tst_wait_ipv6_dad() at end of the setup both solves the problem. > However there is one super strange part that the tentative address is > the local link adress of the ltp_ns_veth1: > > 5: ltp_ns_veth1@if4: <BROADCAST,UP,LOWER_UP> mtu 1500 qdisc noqueue > state UP group default qlen 1000 > link/ether f2:8f:24:d4:ba:26 brd ff:ff:ff:ff:ff:ff link-netnsid 0 > inet 10.0.0.1/24 scope global ltp_ns_veth1 > valid_lft forever preferred_lft forever > inet6 fd00:1:1:1::1/64 scope global nodad > valid_lft forever preferred_lft forever > inet6 fe80::f08f:24ff:fed4:ba26/64 scope link tentative > <------------------- > valid_lft forever preferred_lft forever > > However, there is no place using the address in mpls02 test. > It makes me wonder how could it be possible to trigger the issue.. I wonder if test still needs be fixed to work on all setups. Petr > > Thanks! > -- > Su
On 09.09.2021 18:53, pvorel wrote: > Hi Su, Alexey, > > On 2021-08-30 11:26, suy.fnst@fujitsu.com wrote: >> Hi, >> I found that it's indeed related to ipv6 DAD as you said. >> Calling >> `ip netns exec ltp_ns sysctl -n net.ipv6.conf.ltp_ns_veth1.accept_dad=0` >> or tst_wait_ipv6_dad() at end of the setup both solves the problem. >> However there is one super strange part that the tentative address is >> the local link adress of the ltp_ns_veth1: >> >> 5: ltp_ns_veth1@if4: <BROADCAST,UP,LOWER_UP> mtu 1500 qdisc noqueue >> state UP group default qlen 1000 >> link/ether f2:8f:24:d4:ba:26 brd ff:ff:ff:ff:ff:ff link-netnsid 0 >> inet 10.0.0.1/24 scope global ltp_ns_veth1 >> valid_lft forever preferred_lft forever >> inet6 fd00:1:1:1::1/64 scope global nodad >> valid_lft forever preferred_lft forever >> inet6 fe80::f08f:24ff:fed4:ba26/64 scope link tentative >> <------------------- >> valid_lft forever preferred_lft forever >> >> However, there is no place using the address in mpls02 test.>> It makes me wonder how could it be possible to trigger the issue.. Looks like the problem here in the neighbor discovery of fd00:1:1:1::2 using link-local address, and vice verse for the other side. mpls is using the following route with the address: fd00:23::2 encap mpls 60 via fd00:1:1:1::2 dev ltp_ns_veth1 metric 1024 pref medium so the address there should be in a reachable state... Adding it manually in setup might fix the test as well: ROD ip neigh replace $(tst_ipaddr rhost) lladdr $(tst_hwaddr rhost) dev $(tst_iface) nud reachable tst_rhost_run -s -c "ip neigh replace $(tst_ipaddr) lladdr $(tst_hwaddr) dev $(tst_iface rhost) nud reachable" > > I wonder if test still needs be fixed to work on all setups. > I think we could set accept_dad to 0 in generic setup of the test interfaces, in tst_net.sh/tst_init_iface().
> On 09.09.2021 18:53, pvorel wrote: > > Hi Su, Alexey, > > On 2021-08-30 11:26, suy.fnst@fujitsu.com wrote: > >> Hi, > >> I found that it's indeed related to ipv6 DAD as you said. > >> Calling > >> `ip netns exec ltp_ns sysctl -n net.ipv6.conf.ltp_ns_veth1.accept_dad=0` > >> or tst_wait_ipv6_dad() at end of the setup both solves the problem. > >> However there is one super strange part that the tentative address is > >> the local link adress of the ltp_ns_veth1: > >> 5: ltp_ns_veth1@if4: <BROADCAST,UP,LOWER_UP> mtu 1500 qdisc noqueue > >> state UP group default qlen 1000 > >> link/ether f2:8f:24:d4:ba:26 brd ff:ff:ff:ff:ff:ff link-netnsid 0 > >> inet 10.0.0.1/24 scope global ltp_ns_veth1 > >> valid_lft forever preferred_lft forever > >> inet6 fd00:1:1:1::1/64 scope global nodad > >> valid_lft forever preferred_lft forever > >> inet6 fe80::f08f:24ff:fed4:ba26/64 scope link tentative > >> <------------------- > >> valid_lft forever preferred_lft forever > >> However, there is no place using the address in mpls02 test.>> It makes me wonder how could it be possible to trigger the issue.. > Looks like the problem here in the neighbor discovery of fd00:1:1:1::2 > using link-local address, and vice verse for the other side. mpls is > using the following route with the address: > fd00:23::2 encap mpls 60 via fd00:1:1:1::2 dev ltp_ns_veth1 metric 1024 pref medium > so the address there should be in a reachable state... Thanks for info! I wonder if it's a bug in mpls or elsewhere. WDYT? > Adding it manually in setup might fix the test as well: > ROD ip neigh replace $(tst_ipaddr rhost) lladdr $(tst_hwaddr rhost) dev $(tst_iface) nud reachable > tst_rhost_run -s -c "ip neigh replace $(tst_ipaddr) lladdr $(tst_hwaddr) dev $(tst_iface rhost) nud reachable" > > I wonder if test still needs be fixed to work on all setups. > I think we could set accept_dad to 0 in generic setup of the > test interfaces, in tst_net.sh/tst_init_iface(). Unless it's a bug we'd hide by setting it, I'd be for this general solution. It'd be nice to get it fixed before release. Kind regards, Petr
Hi, Alexey
Thanks! It works! Could you send a patch for the fix?
You can add my
Tested-by: Su Yue <suy.fnst@fujitsu.com>
--
Su
Hi Petr, On 10.09.2021 12:36, Petr Vorel wrote: >> On 09.09.2021 18:53, pvorel wrote: >>> Hi Su, Alexey, > >>> On 2021-08-30 11:26, suy.fnst@fujitsu.com wrote: >>>> Hi, >>>> I found that it's indeed related to ipv6 DAD as you said. >>>> Calling >>>> `ip netns exec ltp_ns sysctl -n net.ipv6.conf.ltp_ns_veth1.accept_dad=0` >>>> or tst_wait_ipv6_dad() at end of the setup both solves the problem. >>>> However there is one super strange part that the tentative address is >>>> the local link adress of the ltp_ns_veth1: > >>>> 5: ltp_ns_veth1@if4: <BROADCAST,UP,LOWER_UP> mtu 1500 qdisc noqueue >>>> state UP group default qlen 1000 >>>> link/ether f2:8f:24:d4:ba:26 brd ff:ff:ff:ff:ff:ff link-netnsid 0 >>>> inet 10.0.0.1/24 scope global ltp_ns_veth1 >>>> valid_lft forever preferred_lft forever >>>> inet6 fd00:1:1:1::1/64 scope global nodad >>>> valid_lft forever preferred_lft forever >>>> inet6 fe80::f08f:24ff:fed4:ba26/64 scope link tentative >>>> <------------------- >>>> valid_lft forever preferred_lft forever > >>>> However, there is no place using the address in mpls02 test.>> It makes me wonder how could it be possible to trigger the issue.. > >> Looks like the problem here in the neighbor discovery of fd00:1:1:1::2 >> using link-local address, and vice verse for the other side. mpls is >> using the following route with the address: > >> fd00:23::2 encap mpls 60 via fd00:1:1:1::2 dev ltp_ns_veth1 metric 1024 pref medium >> so the address there should be in a reachable state... > > Thanks for info! I wonder if it's a bug in mpls or elsewhere. WDYT? https://github.com/iputils/iputils/issues/300 So we should be careful with the flood option in ping, especially if it is the first test to run after initial test interfaces setup. For example, for mpls02, it is "icmp tcp udp". > >> Adding it manually in setup might fix the test as well: > >> ROD ip neigh replace $(tst_ipaddr rhost) lladdr $(tst_hwaddr rhost) dev $(tst_iface) nud reachable >> tst_rhost_run -s -c "ip neigh replace $(tst_ipaddr) lladdr $(tst_hwaddr) dev $(tst_iface rhost) nud reachable" > >>> I wonder if test still needs be fixed to work on all setups. > > >> I think we could set accept_dad to 0 in generic setup of the >> test interfaces, in tst_net.sh/tst_init_iface(). > Unless it's a bug we'd hide by setting it, I'd be for this general solution. > > It'd be nice to get it fixed before release. OK
> Hi Petr, > On 10.09.2021 12:36, Petr Vorel wrote: > >> On 09.09.2021 18:53, pvorel wrote: > >>> Hi Su, Alexey, > >>> On 2021-08-30 11:26, suy.fnst@fujitsu.com wrote: > >>>> Hi, > >>>> I found that it's indeed related to ipv6 DAD as you said. > >>>> Calling > >>>> `ip netns exec ltp_ns sysctl -n net.ipv6.conf.ltp_ns_veth1.accept_dad=0` > >>>> or tst_wait_ipv6_dad() at end of the setup both solves the problem. > >>>> However there is one super strange part that the tentative address is > >>>> the local link adress of the ltp_ns_veth1: > >>>> 5: ltp_ns_veth1@if4: <BROADCAST,UP,LOWER_UP> mtu 1500 qdisc noqueue > >>>> state UP group default qlen 1000 > >>>> link/ether f2:8f:24:d4:ba:26 brd ff:ff:ff:ff:ff:ff link-netnsid 0 > >>>> inet 10.0.0.1/24 scope global ltp_ns_veth1 > >>>> valid_lft forever preferred_lft forever > >>>> inet6 fd00:1:1:1::1/64 scope global nodad > >>>> valid_lft forever preferred_lft forever > >>>> inet6 fe80::f08f:24ff:fed4:ba26/64 scope link tentative > >>>> <------------------- > >>>> valid_lft forever preferred_lft forever > >>>> However, there is no place using the address in mpls02 test.>> It makes me wonder how could it be possible to trigger the issue.. > >> Looks like the problem here in the neighbor discovery of fd00:1:1:1::2 > >> using link-local address, and vice verse for the other side. mpls is > >> using the following route with the address: > >> fd00:23::2 encap mpls 60 via fd00:1:1:1::2 dev ltp_ns_veth1 metric 1024 pref medium > >> so the address there should be in a reachable state... > > Thanks for info! I wonder if it's a bug in mpls or elsewhere. WDYT? > https://github.com/iputils/iputils/issues/300 Ah, thanks for pointing this. > So we should be careful with the flood option in ping, especially > if it is the first test to run after initial test interfaces setup. > For example, for mpls02, it is "icmp tcp udp". Flooding is done with -f or -i 0. IMHO we don't do that in tst_ping(), what am I missing? The bug is about flooding (-i 0) with zero packet size (-s 0, but maybe our use -s 10 would trigger the bug as well). Kind regards, Petr > >> Adding it manually in setup might fix the test as well: > >> ROD ip neigh replace $(tst_ipaddr rhost) lladdr $(tst_hwaddr rhost) dev $(tst_iface) nud reachable > >> tst_rhost_run -s -c "ip neigh replace $(tst_ipaddr) lladdr $(tst_hwaddr) dev $(tst_iface rhost) nud reachable" > >>> I wonder if test still needs be fixed to work on all setups. > >> I think we could set accept_dad to 0 in generic setup of the > >> test interfaces, in tst_net.sh/tst_init_iface(). > > Unless it's a bug we'd hide by setting it, I'd be for this general solution. > > It'd be nice to get it fixed before release. > OK
On 13.09.2021 15:28, Petr Vorel wrote: >> Hi Petr, >> On 10.09.2021 12:36, Petr Vorel wrote: >>>> On 09.09.2021 18:53, pvorel wrote: >>>>> Hi Su, Alexey, > >>>>> On 2021-08-30 11:26, suy.fnst@fujitsu.com wrote: >>>>>> Hi, >>>>>> I found that it's indeed related to ipv6 DAD as you said. >>>>>> Calling >>>>>> `ip netns exec ltp_ns sysctl -n net.ipv6.conf.ltp_ns_veth1.accept_dad=0` >>>>>> or tst_wait_ipv6_dad() at end of the setup both solves the problem. >>>>>> However there is one super strange part that the tentative address is >>>>>> the local link adress of the ltp_ns_veth1: > >>>>>> 5: ltp_ns_veth1@if4: <BROADCAST,UP,LOWER_UP> mtu 1500 qdisc noqueue >>>>>> state UP group default qlen 1000 >>>>>> link/ether f2:8f:24:d4:ba:26 brd ff:ff:ff:ff:ff:ff link-netnsid 0 >>>>>> inet 10.0.0.1/24 scope global ltp_ns_veth1 >>>>>> valid_lft forever preferred_lft forever >>>>>> inet6 fd00:1:1:1::1/64 scope global nodad >>>>>> valid_lft forever preferred_lft forever >>>>>> inet6 fe80::f08f:24ff:fed4:ba26/64 scope link tentative >>>>>> <------------------- >>>>>> valid_lft forever preferred_lft forever > >>>>>> However, there is no place using the address in mpls02 test.>> It makes me wonder how could it be possible to trigger the issue.. > >>>> Looks like the problem here in the neighbor discovery of fd00:1:1:1::2 >>>> using link-local address, and vice verse for the other side. mpls is >>>> using the following route with the address: > >>>> fd00:23::2 encap mpls 60 via fd00:1:1:1::2 dev ltp_ns_veth1 metric 1024 pref medium >>>> so the address there should be in a reachable state... > >>> Thanks for info! I wonder if it's a bug in mpls or elsewhere. WDYT? > >> https://github.com/iputils/iputils/issues/300 > Ah, thanks for pointing this. > >> So we should be careful with the flood option in ping, especially >> if it is the first test to run after initial test interfaces setup. >> For example, for mpls02, it is "icmp tcp udp". > Flooding is done with -f or -i 0. IMHO we don't do that in tst_ping(), > what am I missing? The bug is about flooding (-i 0) with zero packet size (-s 0, > but maybe our use -s 10 would trigger the bug as well). Actually, we do have -f option in tst_ping(), in $flood_opt var.
> On 13.09.2021 15:28, Petr Vorel wrote: > >> Hi Petr, > >> On 10.09.2021 12:36, Petr Vorel wrote: > >>>> On 09.09.2021 18:53, pvorel wrote: > >>>>> Hi Su, Alexey, > >>>>> On 2021-08-30 11:26, suy.fnst@fujitsu.com wrote: > >>>>>> Hi, > >>>>>> I found that it's indeed related to ipv6 DAD as you said. > >>>>>> Calling > >>>>>> `ip netns exec ltp_ns sysctl -n net.ipv6.conf.ltp_ns_veth1.accept_dad=0` > >>>>>> or tst_wait_ipv6_dad() at end of the setup both solves the problem. > >>>>>> However there is one super strange part that the tentative address is > >>>>>> the local link adress of the ltp_ns_veth1: > >>>>>> 5: ltp_ns_veth1@if4: <BROADCAST,UP,LOWER_UP> mtu 1500 qdisc noqueue > >>>>>> state UP group default qlen 1000 > >>>>>> link/ether f2:8f:24:d4:ba:26 brd ff:ff:ff:ff:ff:ff link-netnsid 0 > >>>>>> inet 10.0.0.1/24 scope global ltp_ns_veth1 > >>>>>> valid_lft forever preferred_lft forever > >>>>>> inet6 fd00:1:1:1::1/64 scope global nodad > >>>>>> valid_lft forever preferred_lft forever > >>>>>> inet6 fe80::f08f:24ff:fed4:ba26/64 scope link tentative > >>>>>> <------------------- > >>>>>> valid_lft forever preferred_lft forever > >>>>>> However, there is no place using the address in mpls02 test.>> It makes me wonder how could it be possible to trigger the issue.. > >>>> Looks like the problem here in the neighbor discovery of fd00:1:1:1::2 > >>>> using link-local address, and vice verse for the other side. mpls is > >>>> using the following route with the address: > >>>> fd00:23::2 encap mpls 60 via fd00:1:1:1::2 dev ltp_ns_veth1 metric 1024 pref medium > >>>> so the address there should be in a reachable state... > >>> Thanks for info! I wonder if it's a bug in mpls or elsewhere. WDYT? > >> https://github.com/iputils/iputils/issues/300 > > Ah, thanks for pointing this. > >> So we should be careful with the flood option in ping, especially > >> if it is the first test to run after initial test interfaces setup. > >> For example, for mpls02, it is "icmp tcp udp". > > Flooding is done with -f or -i 0. IMHO we don't do that in tst_ping(), > > what am I missing? The bug is about flooding (-i 0) with zero packet size (-s 0, > > but maybe our use -s 10 would trigger the bug as well). > Actually, we do have -f option in tst_ping(), in $flood_opt var. Ah, I'm blind today, thanks :). I wonder if it's really enough for mpls to just move icmp in TST_TEST_DATA to the end (or just not being first). If it's working, feel free to add ack from my to the commit message. Kind regards, Petr
diff --git a/testcases/network/mpls/mpls02.sh b/testcases/network/mpls/mpls02.sh index 2fd3ec5bf4ba..75f5fca32f9a 100755 --- a/testcases/network/mpls/mpls02.sh +++ b/testcases/network/mpls/mpls02.sh @@ -35,6 +35,8 @@ setup() tst_rhost_run -s -c "ip addr add $ip_rmt/$mask dev lo" tst_rhost_run -s -c "ip route add $ip_loc/$mask encap mpls 60 via inet$TST_IPV6 $(tst_ipaddr)" tst_rhost_run -s -c "ip -f mpls route add 50 dev lo" + + sleep 1 } do_test()
While running mpls02.sh -6 in our slow test box with kernel version > v5.11, we found the test hangs at ping6 occasionally: + grep -q 'invalid option' + for size in ${msg_sizes:-"56"} + EXPECT_PASS ping6 -I fd00:23::2 -c 500 -s 10 -f fd00:23::1 '>/dev/null' + _tst_expect_pass tst_res ping6 -I fd00:23::2 -c 500 -s 10 -f fd00:23::1 '>/dev/null' + local fnc=tst_res + shift + tst_rod ping6 -I fd00:23::2 -c 500 -s 10 -f fd00:23::1 '>/dev/null' The weird part is that manual ping6 works in the meantime. Then we found that adding `sleep 1` at end of mpls02.sh/setup() make the test pass in 100%. Dig depper in the problem, ftrace catched during the 1 second shows (The test started at [ 3979.485488]): 3981.210701 | 2) kworker-24742 | | addrconf_dad_work() { 3981.210712 | 2) kworker-24742 | | addrconf_dad_completed() { 3981.210712 | 2) kworker-24742 | 0.417 us | addrconf_del_dad_work(); 3981.210713 | 2) kworker-24742 | | __ipv6_ifa_notify() { 3981.210716 | 2) kworker-24742 | 1.097 us | inet6_fill_ifaddr(); 3981.210730 | 2) kworker-24742 | | ip6_ins_rt() { 3981.210731 | 2) kworker-24742 | | fib6_add() { In kernel, addrconf_dad_work() is delegated by inet6_addr_add() to do route related jobs in a delayed workqueue . Hence, there is tiny period we need to wait for it. Adding the `sleep 1` seems suffcient for now. Signed-off-by: Su Yue <suy.fnst@fujitsu.com> --- testcases/network/mpls/mpls02.sh | 2 ++ 1 file changed, 2 insertions(+)