| Message ID | 20240726-mptcp-pm-avail-v5-9-fb1117ddeef6@kernel.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | 19c879bdde12d5903e89dcc9a75b83583fe35cb4 |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | mptcp: fix endpoints with 'signal' and 'subflow' flags | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 7 lines checked |
| matttbe/shellcheck | success | No ShellCheck issues |
| matttbe/build | success | Build and static analysis OK |
| matttbe/KVM_Validation__normal | success | Success! ✅ |
| matttbe/KVM_Validation__debug | success | Success! ✅ |
| matttbe/KVM_Validation__btf__only_bpftest_all_ | success | Success! ✅ |
On Fri, 2024-07-26 at 16:28 +0200, Matthieu Baerts (NGI0) wrote: > This case was not covered, and the wrong ID was set before the > previous > commit. > > The rest is not modified, it is just that it will increase the code > coverage. > > The right address ID can be verified by looking at the packet traces. > We > could automate that using Netfilter with some cBPF code for example, > but > that's always a bit cryptic. Packetdrill seems better fitted for > that. > > Fixes: 4f49d63352da ("selftests: mptcp: add fullmesh testcases") > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > --- > tools/testing/selftests/net/mptcp/mptcp_join.sh | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh > b/tools/testing/selftests/net/mptcp/mptcp_join.sh > index f609c02c6123..e4ac275366ce 100755 > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > @@ -3058,6 +3058,7 @@ fullmesh_tests() > if reset "fullmesh test 1x1"; then > pm_nl_set_limits $ns1 1 3 > pm_nl_set_limits $ns2 1 3 > + pm_nl_add_endpoint $ns2 10.0.1.2 flags > subflow,fullmesh nit: add this line behind "pm_nl_add_endpoint $ns1 10.0.2.1 flags signal". Usually, set n1 first and then n2. > pm_nl_add_endpoint $ns1 10.0.2.1 flags signal > fullmesh=1 speed=slow \ > run_tests $ns1 $ns2 10.0.1.1 >
Hi Geliang, On 29/07/2024 03:19, Geliang Tang wrote: > On Fri, 2024-07-26 at 16:28 +0200, Matthieu Baerts (NGI0) wrote: >> This case was not covered, and the wrong ID was set before the >> previous >> commit. >> >> The rest is not modified, it is just that it will increase the code >> coverage. >> >> The right address ID can be verified by looking at the packet traces. >> We >> could automate that using Netfilter with some cBPF code for example, >> but >> that's always a bit cryptic. Packetdrill seems better fitted for >> that. >> >> Fixes: 4f49d63352da ("selftests: mptcp: add fullmesh testcases") >> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> >> --- >> tools/testing/selftests/net/mptcp/mptcp_join.sh | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh >> b/tools/testing/selftests/net/mptcp/mptcp_join.sh >> index f609c02c6123..e4ac275366ce 100755 >> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh >> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh >> @@ -3058,6 +3058,7 @@ fullmesh_tests() >> if reset "fullmesh test 1x1"; then >> pm_nl_set_limits $ns1 1 3 >> pm_nl_set_limits $ns2 1 3 >> + pm_nl_add_endpoint $ns2 10.0.1.2 flags >> subflow,fullmesh > > nit: add this line behind "pm_nl_add_endpoint $ns1 10.0.2.1 flags > signal". Usually, set n1 first and then n2. It looks like we have a mix, but yes, most of the time we set the endpoints for the ns1, then the ns2. I can fix that when applying the patches if there are no other big changes needed here. >> pm_nl_add_endpoint $ns1 10.0.2.1 flags signal >> fullmesh=1 speed=slow \ >> run_tests $ns1 $ns2 10.0.1.1 >> > > Cheers, Matt
On Mon, 2024-07-29 at 11:35 +0200, Matthieu Baerts wrote: > Hi Geliang, > > On 29/07/2024 03:19, Geliang Tang wrote: > > On Fri, 2024-07-26 at 16:28 +0200, Matthieu Baerts (NGI0) wrote: > > > This case was not covered, and the wrong ID was set before the > > > previous > > > commit. > > > > > > The rest is not modified, it is just that it will increase the > > > code > > > coverage. > > > > > > The right address ID can be verified by looking at the packet > > > traces. > > > We > > > could automate that using Netfilter with some cBPF code for > > > example, > > > but > > > that's always a bit cryptic. Packetdrill seems better fitted for > > > that. > > > > > > Fixes: 4f49d63352da ("selftests: mptcp: add fullmesh testcases") > > > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > > > --- > > > tools/testing/selftests/net/mptcp/mptcp_join.sh | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh > > > b/tools/testing/selftests/net/mptcp/mptcp_join.sh > > > index f609c02c6123..e4ac275366ce 100755 > > > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > > > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > > > @@ -3058,6 +3058,7 @@ fullmesh_tests() > > > if reset "fullmesh test 1x1"; then > > > pm_nl_set_limits $ns1 1 3 > > > pm_nl_set_limits $ns2 1 3 > > > + pm_nl_add_endpoint $ns2 10.0.1.2 flags > > > subflow,fullmesh > > > > nit: add this line behind "pm_nl_add_endpoint $ns1 10.0.2.1 flags > > signal". Usually, set n1 first and then n2. > > It looks like we have a mix, but yes, most of the time we set the > endpoints for the ns1, then the ns2. I can fix that when applying the > patches if there are no other big changes needed here. Sure, please update this when merging it. > > > > pm_nl_add_endpoint $ns1 10.0.2.1 flags signal > > > fullmesh=1 speed=slow \ > > > run_tests $ns1 $ns2 10.0.1.1 > > > > > > > > > Cheers, > Matt
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index f609c02c6123..e4ac275366ce 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -3058,6 +3058,7 @@ fullmesh_tests() if reset "fullmesh test 1x1"; then pm_nl_set_limits $ns1 1 3 pm_nl_set_limits $ns2 1 3 + pm_nl_add_endpoint $ns2 10.0.1.2 flags subflow,fullmesh pm_nl_add_endpoint $ns1 10.0.2.1 flags signal fullmesh=1 speed=slow \ run_tests $ns1 $ns2 10.0.1.1
This case was not covered, and the wrong ID was set before the previous commit. The rest is not modified, it is just that it will increase the code coverage. The right address ID can be verified by looking at the packet traces. We could automate that using Netfilter with some cBPF code for example, but that's always a bit cryptic. Packetdrill seems better fitted for that. Fixes: 4f49d63352da ("selftests: mptcp: add fullmesh testcases") Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 1 + 1 file changed, 1 insertion(+)