tests: also run Join tests with Checksum support #1
Conversation
entrypoint.sh
Outdated
| @@ -451,6 +452,10 @@ run_mptcp_connect_mmap() { | |||
| _run_selftest_one_tap "${mptcp_connect_mmap_tap}" ./mptcp_connect.sh -m mmap | |||
| } | |||
|
|
|||
| run_mptcp_join_csum() { | |||
| _run_selftest_one_tap "${mptcp_join_csum_tap}" ./mptcp_join.sh -C | |||
There was a problem hiding this comment.
| _run_selftest_one_tap "${mptcp_join_csum_tap}" ./mptcp_join.sh -C | |
| _run_selftest_one_tap "${mptcp_join_csum_tap}" ./mptcp_join.sh -fsabpkC |
This runs in about 1/4 the time on my system.
There was a problem hiding this comment.
@mjmartineau Thank you for the suggestion! (and sorry for the delay!)
That seems to add 3-4 minutes on a slow environment.
But what worry me is more to maintain a list of acceptable options. I mean: if we modify mptcp_join.sh and maintain this list of tests there, that seems to be a more valuable solution (or better: some tests are ran twice in a normal situation: with an without checksum).
Or maybe enough to run mptcp_connect.sh a second time with checksum support? That seems to cover quite a few scenarios. For the ones with multiple paths, we have mptcp_join.sh that already cover some tests with checksum enabled.
In other words: drop this commit and only take yours?
There was a problem hiding this comment.
I think it's important to get both the mptcp_connect.sh coverage in #2 and (at least) some of the multiple-subflow cases in mptcp_join.sh. Modifying mptcp_join.sh would be fine with me, that way the checksum code would get exercised more by individual developers and other CI systems like kbuild.
Much like multipath-tcp#1, run the connect tests an extra time with the -C flag in order to exercise the checksum code in more scenarios. Signed-off-by: Mat Martineau <[email protected]>
matttbe
force-pushed
the
mptcp_join_csum
branch
from
d022f0b to
ca3335b
Compare
To add more coverage. We already have some in mptcp_join.sh. Signed-off-by: Matthieu Baerts <[email protected]>
matttbe
force-pushed
the
mptcp_join_csum
branch
from
ca3335b to
768b427
Compare
|
@mjmartineau with some delay, I'm reviving this PR: do you think we still need more tests to run with csum enabled? Now that tests are running on 4 instances, we have a bit more time. Sadly, not enough to run all But do you think it makes sense to run |
This needs to be covered by the CI.
Only downside: it takes ~25 minutes with the public CI and a debug
kernel.
cc: @mjmartineau