This patch is linked to the previous commit ("mptcp: only send RM_ADDR in
nl_cmd_remove").
To align with what is done by the in-kernel PM, update userspace pm addr
selftests, by sending a remove_subflows command together after the
remove_addrs command.
Fixes: d9a4594edabf ("mptcp: netlink: Add MPTCP_PM_CMD_REMOVE")
Fixes: 97040cf9806e ("selftests: mptcp: userspace pm address tests")
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
tools/testing/selftests/net/mptcp/mptcp_join.sh | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 26310c17b4c6..67d5d724266a 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -850,6 +850,14 @@ do_transfer()
ip netns exec ${listener_ns} ./pm_nl_ctl ann $addr token $tk id $id
sleep 1
ip netns exec ${listener_ns} ./pm_nl_ctl rem token $tk id $id
+ sp=$(grep "type:10" "$evts_ns1" |
+ sed -n 's/.*\(sport:\)\([[:digit:]]*\).*$/\2/p;q')
+ da=$(grep "type:10" "$evts_ns1" |
+ sed -n 's/.*\(daddr6:\)\([0-9a-f:.]*\).*$/\2/p;q')
+ dp=$(grep "type:10" "$evts_ns1" |
+ sed -n 's/.*\(dport:\)\([[:digit:]]*\).*$/\2/p;q')
+ ip netns exec ${listener_ns} ./pm_nl_ctl dsf lip "::ffff:$addr" \
+ lport $sp rip $da rport $dp token $tk
fi
counter=$((counter + 1))
--
2.35.3Hi Geliang,
On 26/04/2023 10:56, Geliang Tang wrote:
> This patch is linked to the previous commit ("mptcp: only send RM_ADDR in
> nl_cmd_remove").
>
> To align with what is done by the in-kernel PM, update userspace pm addr
> selftests, by sending a remove_subflows command together after the
> remove_addrs command.
>
> Fixes: d9a4594edabf ("mptcp: netlink: Add MPTCP_PM_CMD_REMOVE")
> Fixes: 97040cf9806e ("selftests: mptcp: userspace pm address tests")
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> tools/testing/selftests/net/mptcp/mptcp_join.sh | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 26310c17b4c6..67d5d724266a 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -850,6 +850,14 @@ do_transfer()
> ip netns exec ${listener_ns} ./pm_nl_ctl ann $addr token $tk id $id
> sleep 1
> ip netns exec ${listener_ns} ./pm_nl_ctl rem token $tk id $id
Out of curiosity, why did you have to move this command before the
destruction of the subflow (just below)? Is it because since v10, we
cannot remove the subflow if the subflow has been destroyed?
For the moment, I guess it is OK because if I'm not mistaken, we don't
check that the subflow destruction has been done, right?
If at some points we do verify the following command is doing what we
expect to do, my fear is that the next command might not work if we are
unlucky: when the RM_ADDR from the previous step will be received, the
other peer will destroy the associated subflows, no? If yes and if it is
done quicker than launching the following command, the subflow could
already be destroyed and the following command might fail, no?
Maybe that's not an issue because we don't check for errors in pm_nl_ctl
(which is maybe not a good thing but that's another problem). So fine to
keep this like that, right?
> + sp=$(grep "type:10" "$evts_ns1" |
> + sed -n 's/.*\(sport:\)\([[:digit:]]*\).*$/\2/p;q')
> + da=$(grep "type:10" "$evts_ns1" |
> + sed -n 's/.*\(daddr6:\)\([0-9a-f:.]*\).*$/\2/p;q')
> + dp=$(grep "type:10" "$evts_ns1" |
> + sed -n 's/.*\(dport:\)\([[:digit:]]*\).*$/\2/p;q')
> + ip netns exec ${listener_ns} ./pm_nl_ctl dsf lip "::ffff:$addr" \
> + lport $sp rip $da rport $dp token $tk
> fi
>
> counter=$((counter + 1))
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
Hi Geliang,
On 03/05/2023 19:33, Matthieu Baerts wrote:
> Hi Geliang,
>
> On 26/04/2023 10:56, Geliang Tang wrote:
>> This patch is linked to the previous commit ("mptcp: only send RM_ADDR in
>> nl_cmd_remove").
>>
>> To align with what is done by the in-kernel PM, update userspace pm addr
>> selftests, by sending a remove_subflows command together after the
>> remove_addrs command.
>>
>> Fixes: d9a4594edabf ("mptcp: netlink: Add MPTCP_PM_CMD_REMOVE")
>> Fixes: 97040cf9806e ("selftests: mptcp: userspace pm address tests")
>> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
>> ---
>> tools/testing/selftests/net/mptcp/mptcp_join.sh | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> index 26310c17b4c6..67d5d724266a 100755
>> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> @@ -850,6 +850,14 @@ do_transfer()
>> ip netns exec ${listener_ns} ./pm_nl_ctl ann $addr token $tk id $id
>> sleep 1
>> ip netns exec ${listener_ns} ./pm_nl_ctl rem token $tk id $id
>
> Out of curiosity, why did you have to move this command before the
> destruction of the subflow (just below)? Is it because since v10, we
> cannot remove the subflow if the subflow has been destroyed?
>
> For the moment, I guess it is OK because if I'm not mistaken, we don't
> check that the subflow destruction has been done, right?
>
> If at some points we do verify the following command is doing what we
> expect to do, my fear is that the next command might not work if we are
> unlucky: when the RM_ADDR from the previous step will be received, the
> other peer will destroy the associated subflows, no? If yes and if it is
> done quicker than launching the following command, the subflow could
> already be destroyed and the following command might fail, no?
>
> Maybe that's not an issue because we don't check for errors in pm_nl_ctl
> (which is maybe not a good thing but that's another problem). So fine to
> keep this like that, right?
Does what I saying above correct: the next command could fail but that's
fine because we ignore errors?
>> + sp=$(grep "type:10" "$evts_ns1" |
>> + sed -n 's/.*\(sport:\)\([[:digit:]]*\).*$/\2/p;q')
>> + da=$(grep "type:10" "$evts_ns1" |
>> + sed -n 's/.*\(daddr6:\)\([0-9a-f:.]*\).*$/\2/p;q')
>> + dp=$(grep "type:10" "$evts_ns1" |
>> + sed -n 's/.*\(dport:\)\([[:digit:]]*\).*$/\2/p;q')
>> + ip netns exec ${listener_ns} ./pm_nl_ctl dsf lip "::ffff:$addr" \
>> + lport $sp rip $da rport $dp token $tk
>> fi
>>
>> counter=$((counter + 1))
>
> Cheers,
> Matt
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
Hi Matt,
Matthieu Baerts <matthieu.baerts@tessares.net> 于2023年5月4日周四 21:07写道：
>
> Hi Geliang,
>
> On 03/05/2023 19:33, Matthieu Baerts wrote:
> > Hi Geliang,
> >
> > On 26/04/2023 10:56, Geliang Tang wrote:
> >> This patch is linked to the previous commit ("mptcp: only send RM_ADDR in
> >> nl_cmd_remove").
> >>
> >> To align with what is done by the in-kernel PM, update userspace pm addr
> >> selftests, by sending a remove_subflows command together after the
> >> remove_addrs command.
> >>
> >> Fixes: d9a4594edabf ("mptcp: netlink: Add MPTCP_PM_CMD_REMOVE")
> >> Fixes: 97040cf9806e ("selftests: mptcp: userspace pm address tests")
> >> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> >> ---
> >> tools/testing/selftests/net/mptcp/mptcp_join.sh | 8 ++++++++
> >> 1 file changed, 8 insertions(+)
> >>
> >> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> >> index 26310c17b4c6..67d5d724266a 100755
> >> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> >> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> >> @@ -850,6 +850,14 @@ do_transfer()
> >> ip netns exec ${listener_ns} ./pm_nl_ctl ann $addr token $tk id $id
> >> sleep 1
> >> ip netns exec ${listener_ns} ./pm_nl_ctl rem token $tk id $id
> >
> > Out of curiosity, why did you have to move this command before the
> > destruction of the subflow (just below)? Is it because since v10, we
> > cannot remove the subflow if the subflow has been destroyed?
> >
> > For the moment, I guess it is OK because if I'm not mistaken, we don't
> > check that the subflow destruction has been done, right?
> >
> > If at some points we do verify the following command is doing what we
> > expect to do, my fear is that the next command might not work if we are
> > unlucky: when the RM_ADDR from the previous step will be received, the
> > other peer will destroy the associated subflows, no? If yes and if it is
> > done quicker than launching the following command, the subflow could
> > already be destroyed and the following command might fail, no?
> >
> > Maybe that's not an issue because we don't check for errors in pm_nl_ctl
> > (which is maybe not a good thing but that's another problem). So fine to
> > keep this like that, right?
>
> Does what I saying above correct: the next command could fail but that's
> fine because we ignore errors?
Now I moved the RM_ADDR command after the destruction of the subflow in v13.
Thank,
-Geliang
>
> >> + sp=$(grep "type:10" "$evts_ns1" |
> >> + sed -n 's/.*\(sport:\)\([[:digit:]]*\).*$/\2/p;q')
> >> + da=$(grep "type:10" "$evts_ns1" |
> >> + sed -n 's/.*\(daddr6:\)\([0-9a-f:.]*\).*$/\2/p;q')
> >> + dp=$(grep "type:10" "$evts_ns1" |
> >> + sed -n 's/.*\(dport:\)\([[:digit:]]*\).*$/\2/p;q')
> >> + ip netns exec ${listener_ns} ./pm_nl_ctl dsf lip "::ffff:$addr" \
> >> + lport $sp rip $da rport $dp token $tk
> >> fi
> >>
> >> counter=$((counter + 1))
> >
> > Cheers,
> > Matt
>
> Cheers,
> Matt
> --
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net
>
© 2016 - 2025 Red Hat, Inc.