Hi Matt,
On Wed, Mar 13, 2024 at 07:46:41PM +0100, Matthieu Baerts wrote:
> Hi Geliang,
>
> On 11/03/2024 02:48, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> >
> > This is the last part for "add helpers and vars in mptcp_lib.sh" series,
> > add endpoint operation helpers into mptcp_lib.sh.
>
> Thank you for looking at that!
>
> I just did a review, see my different replies.
Thanks for your review. It's very useful. I just sent a v2, addressed all
your comments.
>
> In general, I think we should only add tests if they validate new code
> path on the kernel side, e.g. new feature. Having more tests means more
> code to maintain, a longer time to execute them, etc. Best to only add
> "valuable" ones.
I dropped them in v2.
>
> > Geliang Tang (10):
> > selftests: mptcp: export ip_mptcp to mptcp_lib
> > selftests: mptcp: get support for limits
> > selftests: mptcp: id support for show_endpoints
> > selftests: mptcp: addr support for change_endpoint
> > selftests: mptcp: netlink: fix positions of newline
> > selftests: mptcp: netlink: add outputs for ip_mptcp
> > selftests: mptcp: add endpoint_ops API helper
> > selftests: mptcp: use mptcp_lib_endpoint_ops
> > selftests: mptcp: add ip_mptcp option for more scripts
>
> I hope some people are going to validate that. I don't think our CI
> should always run these tests with and without this new '-i' option.
>
> Maybe once in a while, to validate the interface with 'ip mptcp' is
> still OK? Do you plan to do this check periodically? e.g. before a new
> version of 'iproute2'?
Sure, I can check them in the future.
Thanks,
-Geliang
>
>
> > selftests: mptcp: netlink: drop disable=SC2086
> >
> > .../testing/selftests/net/mptcp/mptcp_join.sh | 108 ++-----
> > .../testing/selftests/net/mptcp/mptcp_lib.sh | 166 ++++++++++
> > .../selftests/net/mptcp/mptcp_sockopt.sh | 34 ++-
> > .../testing/selftests/net/mptcp/pm_netlink.sh | 288 +++++++++++-------
> > .../selftests/net/mptcp/simult_flows.sh | 14 +-
> > 5 files changed, 401 insertions(+), 209 deletions(-)
> >
>
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.