Patch 1 makes a function static because it is only used in one file.
Patch 2 adds info about the git trees we use to help occasional devs.
Patch 3 removes an unused variable.
Patch 4 removes duplicated entries from the help menu of a tool used in
MPTCP selftests.
Patch 5 removes some ShellCheck warnings in mptcp_join.sh selftest.
Only very minor improvements then.
Signed-off-by: Matthieu Baerts <[email protected]>
---
Geliang Tang (1):
mptcp: make userspace_pm_append_new_local_addr static
Matthieu Baerts (4):
MAINTAINERS: add git trees for MPTCP
mptcp: remove unused 'remaining' variable
selftests: mptcp: remove duplicated entries in usage
selftests: mptcp: join: fix ShellCheck warnings
MAINTAINERS | 2 ++
net/mptcp/options.c | 7 ++-----
net/mptcp/pm_userspace.c | 4 ++--
net/mptcp/protocol.h | 2 --
tools/testing/selftests/net/mptcp/mptcp_connect.c | 8 ++++----
tools/testing/selftests/net/mptcp/mptcp_join.sh | 10 ++++++++--
6 files changed, 18 insertions(+), 15 deletions(-)
---
base-commit: c11d2e718c792468e67389b506451eddf26c2dac
change-id: 20230414-upstream-net-next-20230414-mptcp-small-cleanups-1cba986990b1
Best regards,
--
Matthieu Baerts <[email protected]>
In some functions, 'remaining' variable was given in argument and/or set
but never read.
net/mptcp/options.c:779:3: warning: Value stored to 'remaining' is never
read [clang-analyzer-deadcode.DeadStores].
net/mptcp/options.c:547:3: warning: Value stored to 'remaining' is never
read [clang-analyzer-deadcode.DeadStores].
The issue has been reported internally by Alibaba CI.
Reported-by: Abaci Robot <[email protected]>
Suggested-by: Mat Martineau <[email protected]>
Co-developed-by: Jiapeng Chong <[email protected]>
Signed-off-by: Jiapeng Chong <[email protected]>
Acked-by: Paolo Abeni <[email protected]>
Signed-off-by: Matthieu Baerts <[email protected]>
---
net/mptcp/options.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 355f798d575a..a9801cfe7d15 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -442,7 +442,6 @@ static void clear_3rdack_retransmission(struct sock *sk)
static bool mptcp_established_options_mp(struct sock *sk, struct sk_buff *skb,
bool snd_data_fin_enable,
unsigned int *size,
- unsigned int remaining,
struct mptcp_out_options *opts)
{
struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
@@ -556,7 +555,6 @@ static void mptcp_write_data_fin(struct mptcp_subflow_context *subflow,
static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
bool snd_data_fin_enable,
unsigned int *size,
- unsigned int remaining,
struct mptcp_out_options *opts)
{
struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
@@ -580,7 +578,6 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
opts->ext_copy = *mpext;
}
- remaining -= map_size;
dss_size = map_size;
if (skb && snd_data_fin_enable)
mptcp_write_data_fin(subflow, skb, &opts->ext_copy);
@@ -851,9 +848,9 @@ bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
}
snd_data_fin = mptcp_data_fin_enabled(msk);
- if (mptcp_established_options_mp(sk, skb, snd_data_fin, &opt_size, remaining, opts))
+ if (mptcp_established_options_mp(sk, skb, snd_data_fin, &opt_size, opts))
ret = true;
- else if (mptcp_established_options_dss(sk, skb, snd_data_fin, &opt_size, remaining, opts)) {
+ else if (mptcp_established_options_dss(sk, skb, snd_data_fin, &opt_size, opts)) {
unsigned int mp_fail_size;
ret = true;
--
2.39.2
mptcp_connect tool was printing some duplicated entries when showing how
to use it: -j -l -r
While at it, I also:
- moved the very few entries that were not sorted,
- added -R that was missing since
commit 8a4b910d005d ("mptcp: selftests: add rcvbuf set option"),
- removed the -u parameter that has been removed in
commit f730b65c9d85 ("selftests: mptcp: try to set mptcp ulp mode in different sk states").
No need to backport this, it is just an internal tool used by our
selftests. The help menu is mainly useful for MPTCP kernel devs.
Acked-by: Paolo Abeni <[email protected]>
Signed-off-by: Matthieu Baerts <[email protected]>
---
tools/testing/selftests/net/mptcp/mptcp_connect.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index b25a31445ded..c7f9ebeebc2c 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -106,8 +106,8 @@ static struct cfg_sockopt_types cfg_sockopt_types;
static void die_usage(void)
{
fprintf(stderr, "Usage: mptcp_connect [-6] [-c cmsg] [-f offset] [-i file] [-I num] [-j] [-l] "
- "[-m mode] [-M mark] [-o option] [-p port] [-P mode] [-j] [-l] [-r num] "
- "[-s MPTCP|TCP] [-S num] [-r num] [-t num] [-T num] [-u] [-w sec] connect_address\n");
+ "[-m mode] [-M mark] [-o option] [-p port] [-P mode] [-r num] [-R num] "
+ "[-s MPTCP|TCP] [-S num] [-t num] [-T num] [-w sec] connect_address\n");
fprintf(stderr, "\t-6 use ipv6\n");
fprintf(stderr, "\t-c cmsg -- test cmsg type <cmsg>\n");
fprintf(stderr, "\t-f offset -- stop the I/O after receiving and sending the specified amount "
@@ -126,13 +126,13 @@ static void die_usage(void)
fprintf(stderr, "\t-p num -- use port num\n");
fprintf(stderr,
"\t-P [saveWithPeek|saveAfterPeek] -- save data with/after MSG_PEEK form tcp socket\n");
- fprintf(stderr, "\t-t num -- set poll timeout to num\n");
- fprintf(stderr, "\t-T num -- set expected runtime to num ms\n");
fprintf(stderr, "\t-r num -- enable slow mode, limiting each write to num bytes "
"-- for remove addr tests\n");
fprintf(stderr, "\t-R num -- set SO_RCVBUF to num\n");
fprintf(stderr, "\t-s [MPTCP|TCP] -- use mptcp(default) or tcp sockets\n");
fprintf(stderr, "\t-S num -- set SO_SNDBUF to num\n");
+ fprintf(stderr, "\t-t num -- set poll timeout to num\n");
+ fprintf(stderr, "\t-T num -- set expected runtime to num ms\n");
fprintf(stderr, "\t-w num -- wait num sec before closing the socket\n");
exit(1);
}
--
2.39.2
Most of the code had an issue according to ShellCheck.
That's mainly due to the fact it incorrectly believes most of the code
was unreachable because it's invoked by variable name, see how the
"tests" array is used.
Once SC2317 has been ignored, three small warnings were still visible:
- SC2155: Declare and assign separately to avoid masking return values.
- SC2046: Quote this to prevent word splitting: can be ignored because
"ip netns pids" can display more than one pid.
- SC2166: Prefer [ p ] || [ q ] as [ p -o q ] is not well defined.
This probably didn't fix any actual issues but it might help spotting
new interesting warnings reported by ShellCheck as just before,
ShellCheck was reporting issues for most lines making it a bit useless.
Signed-off-by: Matthieu Baerts <[email protected]>
---
tools/testing/selftests/net/mptcp/mptcp_join.sh | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index fafd19ec7e1f..26310c17b4c6 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -6,6 +6,10 @@
# address all other issues detected by shellcheck.
#shellcheck disable=SC2086
+# ShellCheck incorrectly believes that most of the code here is unreachable
+# because it's invoked by variable name, see how the "tests" array is used
+#shellcheck disable=SC2317
+
ret=0
sin=""
sinfail=""
@@ -371,8 +375,9 @@ check_transfer()
local line
if [ -n "$bytes" ]; then
+ local out_size
# when truncating we must check the size explicitly
- local out_size=$(wc -c $out | awk '{print $1}')
+ out_size=$(wc -c $out | awk '{print $1}')
if [ $out_size -ne $bytes ]; then
echo "[ FAIL ] $what output file has wrong size ($out_size, $bytes)"
fail_test
@@ -500,6 +505,7 @@ kill_events_pids()
kill_tests_wait()
{
+ #shellcheck disable=SC2046
kill -SIGUSR1 $(ip netns pids $ns2) $(ip netns pids $ns1)
wait
}
@@ -1703,7 +1709,7 @@ chk_subflow_nr()
cnt1=$(ss -N $ns1 -tOni | grep -c token)
cnt2=$(ss -N $ns2 -tOni | grep -c token)
- if [ "$cnt1" != "$subflow_nr" -o "$cnt2" != "$subflow_nr" ]; then
+ if [ "$cnt1" != "$subflow_nr" ] || [ "$cnt2" != "$subflow_nr" ]; then
echo "[fail] got $cnt1:$cnt2 subflows expected $subflow_nr"
fail_test
dump_stats=1
--
2.39.2
This will help occasional developers to find our git repo without having
to look at our wiki.
Acked-by: Paolo Abeni <[email protected]>
Signed-off-by: Matthieu Baerts <[email protected]>
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index b8b275e27cdb..1c09473685b1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14617,6 +14617,8 @@ L: [email protected]
S: Maintained
W: https://github.com/multipath-tcp/mptcp_net-next/wiki
B: https://github.com/multipath-tcp/mptcp_net-next/issues
+T: git https://github.com/multipath-tcp/mptcp_net-next.git export-net
+T: git https://github.com/multipath-tcp/mptcp_net-next.git export
F: Documentation/networking/mptcp-sysctl.rst
F: include/net/mptcp.h
F: include/trace/events/mptcp.h
--
2.39.2
From: Geliang Tang <[email protected]>
mptcp_userspace_pm_append_new_local_addr() has always exclusively been
used in pm_userspace.c since its introduction in
commit 4638de5aefe5 ("mptcp: handle local addrs announced by userspace PMs").
So make it static.
Signed-off-by: Geliang Tang <[email protected]>
Reviewed-by: Matthieu Baerts <[email protected]>
Signed-off-by: Matthieu Baerts <[email protected]>
---
net/mptcp/pm_userspace.c | 4 ++--
net/mptcp/protocol.h | 2 --
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index a02d3cbf2a1b..27a275805c06 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -25,8 +25,8 @@ void mptcp_free_local_addr_list(struct mptcp_sock *msk)
}
}
-int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
- struct mptcp_pm_addr_entry *entry)
+static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
+ struct mptcp_pm_addr_entry *entry)
{
DECLARE_BITMAP(id_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
struct mptcp_pm_addr_entry *match = NULL;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index e1310bc113be..c8dd24feabef 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -830,8 +830,6 @@ int mptcp_pm_remove_subflow(struct mptcp_sock *msk, const struct mptcp_rm_list *
void mptcp_pm_remove_addrs_and_subflows(struct mptcp_sock *msk,
struct list_head *rm_list);
-int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
- struct mptcp_pm_addr_entry *entry);
void mptcp_free_local_addr_list(struct mptcp_sock *msk);
int mptcp_nl_cmd_announce(struct sk_buff *skb, struct genl_info *info);
int mptcp_nl_cmd_remove(struct sk_buff *skb, struct genl_info *info);
--
2.39.2
Hello:
This series was applied to netdev/net-next.git (main)
by David S. Miller <[email protected]>:
On Fri, 14 Apr 2023 17:47:05 +0200 you wrote:
> Patch 1 makes a function static because it is only used in one file.
>
> Patch 2 adds info about the git trees we use to help occasional devs.
>
> Patch 3 removes an unused variable.
>
> Patch 4 removes duplicated entries from the help menu of a tool used in
> MPTCP selftests.
>
> [...]
Here is the summary with links:
- [net-next,1/5] mptcp: make userspace_pm_append_new_local_addr static
https://git.kernel.org/netdev/net-next/c/aa5887dca2d2
- [net-next,2/5] MAINTAINERS: add git trees for MPTCP
https://git.kernel.org/netdev/net-next/c/c3d713409b53
- [net-next,3/5] mptcp: remove unused 'remaining' variable
https://git.kernel.org/netdev/net-next/c/ce395d0e3ad5
- [net-next,4/5] selftests: mptcp: remove duplicated entries in usage
https://git.kernel.org/netdev/net-next/c/0a85264e48b6
- [net-next,5/5] selftests: mptcp: join: fix ShellCheck warnings
https://git.kernel.org/netdev/net-next/c/0fcd72df8847
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html