* [net-next v4 3/3] selftests/sysctl: add sysctl macro test
From: xiangxia.m.yue @ 2022-04-22 7:01 UTC (permalink / raw)
To: netdev, linux-fsdevel
Cc: Tonghao Zhang, Luis Chamberlain, Kees Cook, Iurii Zaikin,
David S. Miller, Jakub Kicinski, Paolo Abeni, Hideaki YOSHIFUJI,
David Ahern, Simon Horman, Julian Anastasov, Pablo Neira Ayuso,
Jozsef Kadlecsik, Florian Westphal, Shuah Khan, Andrew Morton,
Alexei Starovoitov, Eric Dumazet, Lorenz Bauer, Akhmat Karakotov
In-Reply-To: <20220422070141.39397-1-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Iurii Zaikin <yzaikin@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: David Ahern <dsahern@kernel.org>
Cc: Simon Horman <horms@verge.net.au>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Lorenz Bauer <lmb@cloudflare.com>
Cc: Akhmat Karakotov <hmukos@yandex-team.ru>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
lib/test_sysctl.c | 21 ++++++++++++
tools/testing/selftests/sysctl/sysctl.sh | 43 ++++++++++++++++++++++++
2 files changed, 64 insertions(+)
diff --git a/lib/test_sysctl.c b/lib/test_sysctl.c
index a5a3d6c27e1f..43b8d502f4c7 100644
--- a/lib/test_sysctl.c
+++ b/lib/test_sysctl.c
@@ -43,6 +43,7 @@ struct test_sysctl_data {
int int_0001;
int int_0002;
int int_0003[4];
+ int match_int[12];
int boot_int;
@@ -95,6 +96,13 @@ static struct ctl_table test_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec,
},
+ {
+ .procname = "match_int",
+ .data = &test_data.match_int,
+ .maxlen = sizeof(test_data.match_int),
+ .mode = 0444,
+ .proc_handler = proc_dointvec,
+ },
{
.procname = "boot_int",
.data = &test_data.boot_int,
@@ -132,6 +140,19 @@ static struct ctl_table_header *test_sysctl_header;
static int __init test_sysctl_init(void)
{
+ test_data.match_int[0] = *(int *)SYSCTL_ZERO,
+ test_data.match_int[1] = *(int *)SYSCTL_ONE,
+ test_data.match_int[2] = *(int *)SYSCTL_TWO,
+ test_data.match_int[3] = *(int *)SYSCTL_THREE,
+ test_data.match_int[4] = *(int *)SYSCTL_FOUR,
+ test_data.match_int[5] = *(int *)SYSCTL_ONE_HUNDRED,
+ test_data.match_int[6] = *(int *)SYSCTL_TWO_HUNDRED,
+ test_data.match_int[7] = *(int *)SYSCTL_ONE_THOUSAND,
+ test_data.match_int[8] = *(int *)SYSCTL_THREE_THOUSAND,
+ test_data.match_int[9] = *(int *)SYSCTL_INT_MAX,
+ test_data.match_int[10] = *(int *)SYSCTL_MAXOLDUID,
+ test_data.match_int[11] = *(int *)SYSCTL_NEG_ONE,
+
test_data.bitmap_0001 = kzalloc(SYSCTL_TEST_BITMAP_SIZE/8, GFP_KERNEL);
if (!test_data.bitmap_0001)
return -ENOMEM;
diff --git a/tools/testing/selftests/sysctl/sysctl.sh b/tools/testing/selftests/sysctl/sysctl.sh
index 19515dcb7d04..cd74f4749748 100755
--- a/tools/testing/selftests/sysctl/sysctl.sh
+++ b/tools/testing/selftests/sysctl/sysctl.sh
@@ -40,6 +40,7 @@ ALL_TESTS="$ALL_TESTS 0004:1:1:uint_0001"
ALL_TESTS="$ALL_TESTS 0005:3:1:int_0003"
ALL_TESTS="$ALL_TESTS 0006:50:1:bitmap_0001"
ALL_TESTS="$ALL_TESTS 0007:1:1:boot_int"
+ALL_TESTS="$ALL_TESTS 0008:1:1:match_int"
function allow_user_defaults()
{
@@ -785,6 +786,47 @@ sysctl_test_0007()
return $ksft_skip
}
+sysctl_test_0008()
+{
+ TARGET="${SYSCTL}/match_int"
+ if [ ! -f $TARGET ]; then
+ echo "Skipping test for $TARGET as it is not present ..."
+ return $ksft_skip
+ fi
+
+ echo -n "Testing if $TARGET is matched with kernel ..."
+ ORIG_VALUES=$(cat "${TARGET}")
+
+ # SYSCTL_ZERO 0
+ # SYSCTL_ONE 1
+ # SYSCTL_TWO 2
+ # SYSCTL_THREE 3
+ # SYSCTL_FOUR 4
+ # SYSCTL_ONE_HUNDRED 100
+ # SYSCTL_TWO_HUNDRED 200
+ # SYSCTL_ONE_THOUSAND 1000
+ # SYSCTL_THREE_THOUSAND 3000
+ # SYSCTL_INT_MAX INT_MAX
+ # SYSCTL_MAXOLDUID 65535
+ # SYSCTL_NEG_ONE -1
+ local VALUES=(0 1 2 3 4 100 200 1000 3000 $INT_MAX 65535 -1)
+ idx=0
+
+ for ori in $ORIG_VALUES; do
+ val=${VALUES[$idx]}
+ if [ $ori -ne $val ]; then
+ echo "Expected $val, got $ori, TEST FAILED"
+ rc=1
+ test_rc
+ fi
+
+ let idx=$idx+1
+ done
+
+ echo "ok"
+ return 0
+}
+
list_tests()
{
echo "Test ID list:"
@@ -800,6 +842,7 @@ list_tests()
echo "0005 x $(get_test_count 0005) - tests proc_douintvec() array"
echo "0006 x $(get_test_count 0006) - tests proc_do_large_bitmap()"
echo "0007 x $(get_test_count 0007) - tests setting sysctl from kernel boot param"
+ echo "0008 x $(get_test_count 0008) - tests sysctl macro values match"
}
usage()
--
2.27.0
^ permalink raw reply related
* [net-next v4 2/3] net: sysctl: introduce sysctl SYSCTL_THREE
From: xiangxia.m.yue @ 2022-04-22 7:01 UTC (permalink / raw)
To: netdev, linux-fsdevel
Cc: Tonghao Zhang, Luis Chamberlain, Kees Cook, Iurii Zaikin,
David S. Miller, Jakub Kicinski, Paolo Abeni, Hideaki YOSHIFUJI,
David Ahern, Simon Horman, Julian Anastasov, Pablo Neira Ayuso,
Jozsef Kadlecsik, Florian Westphal, Shuah Khan, Andrew Morton,
Alexei Starovoitov, Eric Dumazet, Lorenz Bauer, Akhmat Karakotov
In-Reply-To: <20220422070141.39397-1-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
This patch introdues the SYSCTL_THREE.
KUnit:
[00:10:14] ================ sysctl_test (10 subtests) =================
[00:10:14] [PASSED] sysctl_test_api_dointvec_null_tbl_data
[00:10:14] [PASSED] sysctl_test_api_dointvec_table_maxlen_unset
[00:10:14] [PASSED] sysctl_test_api_dointvec_table_len_is_zero
[00:10:14] [PASSED] sysctl_test_api_dointvec_table_read_but_position_set
[00:10:14] [PASSED] sysctl_test_dointvec_read_happy_single_positive
[00:10:14] [PASSED] sysctl_test_dointvec_read_happy_single_negative
[00:10:14] [PASSED] sysctl_test_dointvec_write_happy_single_positive
[00:10:14] [PASSED] sysctl_test_dointvec_write_happy_single_negative
[00:10:14] [PASSED] sysctl_test_api_dointvec_write_single_less_int_min
[00:10:14] [PASSED] sysctl_test_api_dointvec_write_single_greater_int_max
[00:10:14] =================== [PASSED] sysctl_test ===================
./run_kselftest.sh -c sysctl
...
ok 1 selftests: sysctl: sysctl.sh
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Iurii Zaikin <yzaikin@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: David Ahern <dsahern@kernel.org>
Cc: Simon Horman <horms@verge.net.au>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Lorenz Bauer <lmb@cloudflare.com>
Cc: Akhmat Karakotov <hmukos@yandex-team.ru>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Reviewed-by: Simon Horman <horms@verge.net.au>
---
fs/proc/proc_sysctl.c | 2 +-
include/linux/sysctl.h | 9 +++++----
net/core/sysctl_net_core.c | 3 +--
net/ipv4/sysctl_net_ipv4.c | 3 +--
net/ipv6/sysctl_net_ipv6.c | 3 +--
net/netfilter/ipvs/ip_vs_ctl.c | 4 +---
6 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 7d9cfc730bd4..5851c2a92c0d 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -26,7 +26,7 @@ static const struct file_operations proc_sys_dir_file_operations;
static const struct inode_operations proc_sys_dir_operations;
/* shared constants to be used in various sysctls */
-const int sysctl_vals[] = { -1, 0, 1, 2, 4, 100, 200, 1000, 3000, INT_MAX, 65535 };
+const int sysctl_vals[] = { 0, 1, 2, 3, 4, 100, 200, 1000, 3000, INT_MAX, 65535, -1 };
EXPORT_SYMBOL(sysctl_vals);
const unsigned long sysctl_long_vals[] = { 0, 1, LONG_MAX };
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 6353d6db69b2..80263f7cdb77 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -38,10 +38,10 @@ struct ctl_table_header;
struct ctl_dir;
/* Keep the same order as in fs/proc/proc_sysctl.c */
-#define SYSCTL_NEG_ONE ((void *)&sysctl_vals[0])
-#define SYSCTL_ZERO ((void *)&sysctl_vals[1])
-#define SYSCTL_ONE ((void *)&sysctl_vals[2])
-#define SYSCTL_TWO ((void *)&sysctl_vals[3])
+#define SYSCTL_ZERO ((void *)&sysctl_vals[0])
+#define SYSCTL_ONE ((void *)&sysctl_vals[1])
+#define SYSCTL_TWO ((void *)&sysctl_vals[2])
+#define SYSCTL_THREE ((void *)&sysctl_vals[3])
#define SYSCTL_FOUR ((void *)&sysctl_vals[4])
#define SYSCTL_ONE_HUNDRED ((void *)&sysctl_vals[5])
#define SYSCTL_TWO_HUNDRED ((void *)&sysctl_vals[6])
@@ -51,6 +51,7 @@ struct ctl_dir;
/* this is needed for the proc_dointvec_minmax for [fs_]overflow UID and GID */
#define SYSCTL_MAXOLDUID ((void *)&sysctl_vals[10])
+#define SYSCTL_NEG_ONE ((void *)&sysctl_vals[11])
extern const int sysctl_vals[];
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 3a0ce309ffcd..195ca5c28771 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -25,7 +25,6 @@
#include "dev.h"
-static int three = 3;
static int int_3600 = 3600;
static int min_sndbuf = SOCK_MIN_SNDBUF;
static int min_rcvbuf = SOCK_MIN_RCVBUF;
@@ -553,7 +552,7 @@ static struct ctl_table net_core_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec_minmax,
.extra1 = SYSCTL_ZERO,
- .extra2 = &three,
+ .extra2 = SYSCTL_THREE,
},
{
.procname = "high_order_alloc_disable",
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 9ff60a389cd0..cd448cdd3b38 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -20,7 +20,6 @@
#include <net/protocol.h>
#include <net/netevent.h>
-static int three __maybe_unused = 3;
static int tcp_retr1_max = 255;
static int ip_local_port_range_min[] = { 1, 1 };
static int ip_local_port_range_max[] = { 65535, 65535 };
@@ -1056,7 +1055,7 @@ static struct ctl_table ipv4_net_table[] = {
.mode = 0644,
.proc_handler = proc_fib_multipath_hash_policy,
.extra1 = SYSCTL_ZERO,
- .extra2 = &three,
+ .extra2 = SYSCTL_THREE,
},
{
.procname = "fib_multipath_hash_fields",
diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
index 560c48d0ddb7..94a0a294c6a1 100644
--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -23,7 +23,6 @@
#endif
#include <linux/ioam6.h>
-static int three = 3;
static int flowlabel_reflect_max = 0x7;
static int auto_flowlabels_max = IP6_AUTO_FLOW_LABEL_MAX;
static u32 rt6_multipath_hash_fields_all_mask =
@@ -171,7 +170,7 @@ static struct ctl_table ipv6_table_template[] = {
.mode = 0644,
.proc_handler = proc_rt6_multipath_hash_policy,
.extra1 = SYSCTL_ZERO,
- .extra2 = &three,
+ .extra2 = SYSCTL_THREE,
},
{
.procname = "fib_multipath_hash_fields",
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 7f645328b47f..efab2b06d373 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1767,8 +1767,6 @@ static int ip_vs_zero_all(struct netns_ipvs *ipvs)
#ifdef CONFIG_SYSCTL
-static int three = 3;
-
static int
proc_do_defense_mode(struct ctl_table *table, int write,
void *buffer, size_t *lenp, loff_t *ppos)
@@ -1977,7 +1975,7 @@ static struct ctl_table vs_vars[] = {
.mode = 0644,
.proc_handler = proc_dointvec_minmax,
.extra1 = SYSCTL_ZERO,
- .extra2 = &three,
+ .extra2 = SYSCTL_THREE,
},
{
.procname = "nat_icmp_send",
--
2.27.0
^ permalink raw reply related
* [net-next v4 1/3] net: sysctl: use shared sysctl macro
From: xiangxia.m.yue @ 2022-04-22 7:01 UTC (permalink / raw)
To: netdev, linux-fsdevel
Cc: Tonghao Zhang, Luis Chamberlain, Kees Cook, Iurii Zaikin,
David S. Miller, Jakub Kicinski, Paolo Abeni, Hideaki YOSHIFUJI,
David Ahern, Simon Horman, Julian Anastasov, Pablo Neira Ayuso,
Jozsef Kadlecsik, Florian Westphal, Shuah Khan, Andrew Morton,
Alexei Starovoitov, Eric Dumazet, Lorenz Bauer, Akhmat Karakotov
In-Reply-To: <20220422070141.39397-1-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
This patch replace two, four and long_one to SYSCTL_XXX.
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Iurii Zaikin <yzaikin@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: David Ahern <dsahern@kernel.org>
Cc: Simon Horman <horms@verge.net.au>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Lorenz Bauer <lmb@cloudflare.com>
Cc: Akhmat Karakotov <hmukos@yandex-team.ru>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
net/core/sysctl_net_core.c | 10 ++++------
net/ipv4/sysctl_net_ipv4.c | 13 +++++--------
net/ipv6/sysctl_net_ipv6.c | 3 +--
3 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 8295e5877eb3..3a0ce309ffcd 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -25,13 +25,11 @@
#include "dev.h"
-static int two = 2;
static int three = 3;
static int int_3600 = 3600;
static int min_sndbuf = SOCK_MIN_SNDBUF;
static int min_rcvbuf = SOCK_MIN_RCVBUF;
static int max_skb_frags = MAX_SKB_FRAGS;
-static long long_one __maybe_unused = 1;
static long long_max __maybe_unused = LONG_MAX;
static int net_msg_warn; /* Unused, but still a sysctl */
@@ -390,7 +388,7 @@ static struct ctl_table net_core_table[] = {
.extra2 = SYSCTL_ONE,
# else
.extra1 = SYSCTL_ZERO,
- .extra2 = &two,
+ .extra2 = SYSCTL_TWO,
# endif
},
# ifdef CONFIG_HAVE_EBPF_JIT
@@ -401,7 +399,7 @@ static struct ctl_table net_core_table[] = {
.mode = 0600,
.proc_handler = proc_dointvec_minmax_bpf_restricted,
.extra1 = SYSCTL_ZERO,
- .extra2 = &two,
+ .extra2 = SYSCTL_TWO,
},
{
.procname = "bpf_jit_kallsyms",
@@ -419,7 +417,7 @@ static struct ctl_table net_core_table[] = {
.maxlen = sizeof(long),
.mode = 0600,
.proc_handler = proc_dolongvec_minmax_bpf_restricted,
- .extra1 = &long_one,
+ .extra1 = SYSCTL_LONG_ONE,
.extra2 = &bpf_jit_limit_max,
},
#endif
@@ -546,7 +544,7 @@ static struct ctl_table net_core_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec_minmax,
.extra1 = SYSCTL_ZERO,
- .extra2 = &two,
+ .extra2 = SYSCTL_TWO,
},
{
.procname = "devconf_inherit_init_net",
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index ad80d180b60b..9ff60a389cd0 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -20,10 +20,7 @@
#include <net/protocol.h>
#include <net/netevent.h>
-static int two = 2;
static int three __maybe_unused = 3;
-static int four = 4;
-static int thousand = 1000;
static int tcp_retr1_max = 255;
static int ip_local_port_range_min[] = { 1, 1 };
static int ip_local_port_range_max[] = { 65535, 65535 };
@@ -1006,7 +1003,7 @@ static struct ctl_table ipv4_net_table[] = {
.mode = 0644,
.proc_handler = proc_dou8vec_minmax,
.extra1 = SYSCTL_ZERO,
- .extra2 = &two,
+ .extra2 = SYSCTL_TWO,
},
{
.procname = "tcp_max_syn_backlog",
@@ -1117,7 +1114,7 @@ static struct ctl_table ipv4_net_table[] = {
.mode = 0644,
.proc_handler = proc_dou8vec_minmax,
.extra1 = SYSCTL_ZERO,
- .extra2 = &four,
+ .extra2 = SYSCTL_FOUR,
},
{
.procname = "tcp_recovery",
@@ -1310,7 +1307,7 @@ static struct ctl_table ipv4_net_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec_minmax,
.extra1 = SYSCTL_ZERO,
- .extra2 = &thousand,
+ .extra2 = SYSCTL_ONE_THOUSAND,
},
{
.procname = "tcp_pacing_ca_ratio",
@@ -1319,7 +1316,7 @@ static struct ctl_table ipv4_net_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec_minmax,
.extra1 = SYSCTL_ZERO,
- .extra2 = &thousand,
+ .extra2 = SYSCTL_ONE_THOUSAND,
},
{
.procname = "tcp_wmem",
@@ -1391,7 +1388,7 @@ static struct ctl_table ipv4_net_table[] = {
.mode = 0644,
.proc_handler = proc_dou8vec_minmax,
.extra1 = SYSCTL_ZERO,
- .extra2 = &two,
+ .extra2 = SYSCTL_TWO,
},
{ }
};
diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
index d53dd142bf87..560c48d0ddb7 100644
--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -23,7 +23,6 @@
#endif
#include <linux/ioam6.h>
-static int two = 2;
static int three = 3;
static int flowlabel_reflect_max = 0x7;
static int auto_flowlabels_max = IP6_AUTO_FLOW_LABEL_MAX;
@@ -197,7 +196,7 @@ static struct ctl_table ipv6_table_template[] = {
.mode = 0644,
.proc_handler = proc_dou8vec_minmax,
.extra1 = SYSCTL_ZERO,
- .extra2 = &two,
+ .extra2 = SYSCTL_TWO,
},
{
.procname = "ioam6_id",
--
2.27.0
^ permalink raw reply related
* [net-next v4 0/3] use standard sysctl macro
From: xiangxia.m.yue @ 2022-04-22 7:01 UTC (permalink / raw)
To: netdev, linux-fsdevel
Cc: Tonghao Zhang, Luis Chamberlain, Kees Cook, Iurii Zaikin,
David S. Miller, Jakub Kicinski, Paolo Abeni, Hideaki YOSHIFUJI,
David Ahern, Simon Horman, Julian Anastasov, Pablo Neira Ayuso,
Jozsef Kadlecsik, Florian Westphal, Shuah Khan, Andrew Morton,
Alexei Starovoitov, Eric Dumazet, Lorenz Bauer, Akhmat Karakotov
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
This patchset introduce sysctl macro or replace var
with macro.
Tonghao Zhang (3):
net: sysctl: use shared sysctl macro
net: sysctl: introduce sysctl SYSCTL_THREE
selftests/sysctl: add sysctl macro test
fs/proc/proc_sysctl.c | 2 +-
include/linux/sysctl.h | 9 ++---
lib/test_sysctl.c | 21 ++++++++++++
net/core/sysctl_net_core.c | 13 +++----
net/ipv4/sysctl_net_ipv4.c | 16 ++++-----
net/ipv6/sysctl_net_ipv6.c | 6 ++--
net/netfilter/ipvs/ip_vs_ctl.c | 4 +--
tools/testing/selftests/sysctl/sysctl.sh | 43 ++++++++++++++++++++++++
8 files changed, 84 insertions(+), 30 deletions(-)
--
v4: add selftests/sysctl patch
v3: split patch to two.
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Iurii Zaikin <yzaikin@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: David Ahern <dsahern@kernel.org>
Cc: Simon Horman <horms@verge.net.au>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Lorenz Bauer <lmb@cloudflare.com>
Cc: Akhmat Karakotov <hmukos@yandex-team.ru>
--
2.27.0
^ permalink raw reply
* Re: [PATCHv2 bpf-next 1/4] kallsyms: Add kallsyms_lookup_names function
From: Jiri Olsa @ 2022-04-22 6:47 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
netdev, bpf, lkml, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh
In-Reply-To: <Yl5yHVOJpCYr+T3r@krava>
On Tue, Apr 19, 2022 at 10:26:05AM +0200, Jiri Olsa wrote:
SNIP
> > > +static int kallsyms_callback(void *data, const char *name,
> > > + struct module *mod, unsigned long addr)
> > > +{
> > > + struct kallsyms_data *args = data;
> > > +
> > > + if (!bsearch(&name, args->syms, args->cnt, sizeof(*args->syms), symbols_cmp))
> > > + return 0;
> > > +
> > > + addr = ftrace_location(addr);
> > > + if (!addr)
> > > + return 0;
> >
> > Ooops, wait. Did you do this last version? I missed this point.
> > This changes the meanings of the kernel function.
>
> yes, it was there before ;-) and you're right.. so some archs can
> return different address, I did not realize that
>
> >
> > > +
> > > + args->addrs[args->found++] = addr;
> > > + return args->found == args->cnt ? 1 : 0;
> > > +}
> > > +
> > > +/**
> > > + * kallsyms_lookup_names - Lookup addresses for array of symbols
> >
> > More correctly "Lookup 'ftraced' addresses for array of sorted symbols", right?
> >
> > I'm not sure, we can call it as a 'kallsyms' API, since this is using
> > kallsyms but doesn't return symbol address, but ftrace address.
> > I think this name misleads user to expect returning symbol address.
> >
> > > + *
> > > + * @syms: array of symbols pointers symbols to resolve, must be
> > > + * alphabetically sorted
> > > + * @cnt: number of symbols/addresses in @syms/@addrs arrays
> > > + * @addrs: array for storing resulting addresses
> > > + *
> > > + * This function looks up addresses for array of symbols provided in
> > > + * @syms array (must be alphabetically sorted) and stores them in
> > > + * @addrs array, which needs to be big enough to store at least @cnt
> > > + * addresses.
> >
> > Hmm, sorry I changed my mind. I rather like to expose kallsyms_on_each_symbol()
> > and provide this API from fprobe or ftrace, because this returns ftrace address
> > and thus this is only used from fprobe.
>
> ok, so how about:
>
> int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *addrs);
quick question.. is it ok if it stays in kalsyms.c object?
so we don't need to expose kallsyms_on_each_symbol,
and it stays in 'kalsyms' place
jirka
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index ce1bd2fbf23e..177e0b13c8c5 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -72,6 +72,7 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
#ifdef CONFIG_KALLSYMS
/* Lookup the address for a symbol. Returns 0 if not found. */
unsigned long kallsyms_lookup_name(const char *name);
+int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *addrs);
extern int kallsyms_lookup_size_offset(unsigned long addr,
unsigned long *symbolsize,
@@ -103,6 +104,11 @@ static inline unsigned long kallsyms_lookup_name(const char *name)
return 0;
}
+static inline int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *addrs);
+{
+ return -ERANGE;
+}
+
static inline int kallsyms_lookup_size_offset(unsigned long addr,
unsigned long *symbolsize,
unsigned long *offset)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 79f2eb617a62..1e7136a765a9 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -29,6 +29,7 @@
#include <linux/compiler.h>
#include <linux/module.h>
#include <linux/kernel.h>
+#include <linux/bsearch.h>
/*
* These will be re-linked against their real values
@@ -228,7 +229,7 @@ unsigned long kallsyms_lookup_name(const char *name)
return module_kallsyms_lookup_name(name);
}
-#ifdef CONFIG_LIVEPATCH
+#if defined(CONFIG_LIVEPATCH) || defined(CONFIG_FPROBE)
/*
* Iterate over all symbols in vmlinux. For symbols from modules use
* module_kallsyms_on_each_symbol instead.
@@ -572,6 +573,73 @@ int sprint_backtrace_build_id(char *buffer, unsigned long address)
return __sprint_symbol(buffer, address, -1, 1, 1);
}
+#ifdef CONFIG_FPROBE
+static int symbols_cmp(const void *a, const void *b)
+{
+ const char **str_a = (const char **) a;
+ const char **str_b = (const char **) b;
+
+ return strcmp(*str_a, *str_b);
+}
+
+struct kallsyms_data {
+ unsigned long *addrs;
+ const char **syms;
+ size_t cnt;
+ size_t found;
+};
+
+static int kallsyms_callback(void *data, const char *name,
+ struct module *mod, unsigned long addr)
+{
+ struct kallsyms_data *args = data;
+
+ if (!bsearch(&name, args->syms, args->cnt, sizeof(*args->syms), symbols_cmp))
+ return 0;
+
+ addr = ftrace_location(addr);
+ if (!addr)
+ return 0;
+
+ args->addrs[args->found++] = addr;
+ return args->found == args->cnt ? 1 : 0;
+}
+
+/**
+ * ftrace_lookup_symbols - Lookup addresses for array of symbols
+ *
+ * @sorted_syms: array of symbols pointers symbols to resolve,
+ * must be alphabetically sorted
+ * @cnt: number of symbols/addresses in @syms/@addrs arrays
+ * @addrs: array for storing resulting addresses
+ *
+ * This function looks up addresses for array of symbols provided in
+ * @syms array (must be alphabetically sorted) and stores them in
+ * @addrs array, which needs to be big enough to store at least @cnt
+ * addresses.
+ *
+ * This function returns 0 if all provided symbols are found,
+ * -ESRCH otherwise.
+ */
+int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *addrs)
+{
+ struct kallsyms_data args;
+
+ args.addrs = addrs;
+ args.syms = sorted_syms;
+ args.cnt = cnt;
+ args.found = 0;
+ kallsyms_on_each_symbol(kallsyms_callback, &args);
+
+ return args.found == args.cnt ? 0 : -ESRCH;
+}
+#else
+int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *addrs)
+{
+ return -ERANGE;
+}
+#endif /* CONFIG_FPROBE */
+
/* To avoid using get_symbol_offset for every symbol, we carry prefix along. */
struct kallsym_iter {
loff_t pos;
^ permalink raw reply related
* Re: [PATCH v2] wwan_hwsim: Avoid flush_scheduled_work() usage
From: Loic Poulain @ 2022-04-22 6:38 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Sergey Ryazanov, Johannes Berg, David S. Miller, Jakub Kicinski,
Paolo Abeni, Network Development
In-Reply-To: <7390d51f-60e2-3cee-5277-b819a55ceabe@I-love.SAKURA.ne.jp>
On Fri, 22 Apr 2022 at 05:01, Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> Flushing system-wide workqueues is dangerous and will be forbidden.
> Replace system_wq with local wwan_wq.
>
> While we are at it, make err_clean_devs: label of wwan_hwsim_init()
> behave like wwan_hwsim_exit(), for it is theoretically possible to call
> wwan_hwsim_debugfs_devcreate_write()/wwan_hwsim_debugfs_devdestroy_write()
> by the moment wwan_hwsim_init_devs() returns.
>
> Link: https://lkml.kernel.org/r/49925af7-78a8-a3dd-bce6-cfc02e1a9236@I-love.SAKURA.ne.jp
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Reviewed-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
Reviewed-by: Loic Poulain <loic.poulain@linaro.org>
> ---
> Changes in v2:
> Keep flush_workqueue(wwan_wq) explicit in order to match the comment.
> Make error path of wwan_hwsim_init() identical to wwan_hwsim_exit().
>
> drivers/net/wwan/wwan_hwsim.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wwan/wwan_hwsim.c b/drivers/net/wwan/wwan_hwsim.c
> index 5b62cf3b3c42..fad642f9ffd8 100644
> --- a/drivers/net/wwan/wwan_hwsim.c
> +++ b/drivers/net/wwan/wwan_hwsim.c
> @@ -33,6 +33,7 @@ static struct dentry *wwan_hwsim_debugfs_devcreate;
> static DEFINE_SPINLOCK(wwan_hwsim_devs_lock);
> static LIST_HEAD(wwan_hwsim_devs);
> static unsigned int wwan_hwsim_dev_idx;
> +static struct workqueue_struct *wwan_wq;
>
> struct wwan_hwsim_dev {
> struct list_head list;
> @@ -371,7 +372,7 @@ static ssize_t wwan_hwsim_debugfs_portdestroy_write(struct file *file,
> * waiting this callback to finish in the debugfs_remove() call. So,
> * use workqueue.
> */
> - schedule_work(&port->del_work);
> + queue_work(wwan_wq, &port->del_work);
>
> return count;
> }
> @@ -416,7 +417,7 @@ static ssize_t wwan_hwsim_debugfs_devdestroy_write(struct file *file,
> * waiting this callback to finish in the debugfs_remove() call. So,
> * use workqueue.
> */
> - schedule_work(&dev->del_work);
> + queue_work(wwan_wq, &dev->del_work);
>
> return count;
> }
> @@ -506,9 +507,15 @@ static int __init wwan_hwsim_init(void)
> if (wwan_hwsim_devsnum < 0 || wwan_hwsim_devsnum > 128)
> return -EINVAL;
>
> + wwan_wq = alloc_workqueue("wwan_wq", 0, 0);
> + if (!wwan_wq)
> + return -ENOMEM;
> +
> wwan_hwsim_class = class_create(THIS_MODULE, "wwan_hwsim");
> - if (IS_ERR(wwan_hwsim_class))
> - return PTR_ERR(wwan_hwsim_class);
> + if (IS_ERR(wwan_hwsim_class)) {
> + err = PTR_ERR(wwan_hwsim_class);
> + goto err_wq_destroy;
> + }
>
> wwan_hwsim_debugfs_topdir = debugfs_create_dir("wwan_hwsim", NULL);
> wwan_hwsim_debugfs_devcreate =
> @@ -523,9 +530,13 @@ static int __init wwan_hwsim_init(void)
> return 0;
>
> err_clean_devs:
> + debugfs_remove(wwan_hwsim_debugfs_devcreate); /* Avoid new devs */
> wwan_hwsim_free_devs();
> + flush_workqueue(wwan_wq); /* Wait deletion works completion */
> debugfs_remove(wwan_hwsim_debugfs_topdir);
> class_destroy(wwan_hwsim_class);
> +err_wq_destroy:
> + destroy_workqueue(wwan_wq);
>
> return err;
> }
> @@ -534,9 +545,10 @@ static void __exit wwan_hwsim_exit(void)
> {
> debugfs_remove(wwan_hwsim_debugfs_devcreate); /* Avoid new devs */
> wwan_hwsim_free_devs();
> - flush_scheduled_work(); /* Wait deletion works completion */
> + flush_workqueue(wwan_wq); /* Wait deletion works completion */
> debugfs_remove(wwan_hwsim_debugfs_topdir);
> class_destroy(wwan_hwsim_class);
> + destroy_workqueue(wwan_wq);
> }
>
> module_init(wwan_hwsim_init);
> --
> 2.32.0
>
^ permalink raw reply
* Re:Re: [PATCH] net/wireless: add debugfs exit function
From: z @ 2022-04-22 6:37 UTC (permalink / raw)
To: Kalle Valo
Cc: Jakub Kicinski, David S. Miller, Paolo Abeni, Matthias Brugger,
linux-wireless, netdev, linux-arm-kernel, linux-mediatek,
linux-kernel, bernard
In-Reply-To: <877d7hoe2i.fsf@kernel.org>
Hi Kalle Valo:
>-----邮件原件-----
>发件人: Kalle Valo <kvalo@kernel.org>
>发送时间: 2022年4月22日 13:57
>收件人: Bernard Zhao <zhaojunkui2008@126.com>
>抄送: Jakub Kicinski <kubakici@wp.pl>; David S. Miller <davem@davemloft.net>; Paolo Abeni <pabeni@redhat.com>; Matthias Brugger <matthias.bgg@gmail.com>; linux-wireless@vger.kernel.org; netdev@vger.kernel.org; >linux-arm-kernel@lists.infradead.org; linux-mediatek@lists.infradead.org; linux-kernel@vger.kernel.org; 赵军奎 <bernard@vivo.com>
>主题: Re: [PATCH] net/wireless: add debugfs exit function
>Bernard Zhao <zhaojunkui2008@126.com> writes:
>> This patch add exit debugfs function to mt7601u.
>> Debugfs need to be cleanup when module is unloaded or load fail.
>"load fail"? Please be more specific, are you saying that the second module load fails or what?
For this part, there are two cases:
First when mt7601u is loaded, in function mt7601u_probe, if function mt7601u_probe run into error lable err_hw, mt7601u_cleanup didn`t cleanup the debugfs node.
Second when the module disconnect, in function mt7601u_disconnect, mt7601u_cleanup didn`t cleanup the debugfs node.
I think these are the mt7601u unloaded or load fail cases, but both with no debugfs cleanup work.
>> drivers/net/wireless/mediatek/mt7601u/debugfs.c | 9 +++++++--
>> drivers/net/wireless/mediatek/mt7601u/init.c | 1 +
>> drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 1 +
>The title should be:
>mt7601u: add debugfs exit function
Got it, thanks！
>> --- a/drivers/net/wireless/mediatek/mt7601u/debugfs.c
>> +++ b/drivers/net/wireless/mediatek/mt7601u/debugfs.c
>> @@ -9,6 +9,8 @@
>> #include "mt7601u.h"
>> #include "eeprom.h"
>>
>> +static struct dentry *dir;
>How will this work when there are multiple mt7601u devices? Because of that, avoid using non-const static variables.
Sorry for missing this part, I understand that the better way is to manage it in the struct of the matched device, I would fix this in the next patch.
Thank you very much!
BR//Bernard
>--
>https://patchwork.kernel.org/project/linux-wireless/list/
>https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* [PATCH net-next] arp: fix unused variable warnning when CONFIG_PROC_FS=n
From: Yajun Deng @ 2022-04-22 6:14 UTC (permalink / raw)
To: davem, yoshfuji, dsahern, kuba, pabeni
Cc: netdev, linux-kernel, Yajun Deng, kernel test robot
net/ipv4/arp.c:1412:36: warning: unused variable 'arp_seq_ops' [-Wunused-const-variable]
Add #ifdef CONFIG_PROC_FS for 'arp_seq_ops'.
Fixes: e968b1b3e9b8 ("arp: Remove #ifdef CONFIG_PROC_FS")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
---
net/ipv4/arp.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 2d0c05ca9c6f..ab4a5601c82a 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -1304,9 +1304,9 @@ static struct packet_type arp_packet_type __read_mostly = {
.func = arp_rcv,
};
+#ifdef CONFIG_PROC_FS
#if IS_ENABLED(CONFIG_AX25)
-/* ------------------------------------------------------------------------ */
/*
* ax25 -> ASCII conversion
*/
@@ -1412,16 +1412,13 @@ static void *arp_seq_start(struct seq_file *seq, loff_t *pos)
return neigh_seq_start(seq, pos, &arp_tbl, NEIGH_SEQ_SKIP_NOARP);
}
-/* ------------------------------------------------------------------------ */
-
static const struct seq_operations arp_seq_ops = {
.start = arp_seq_start,
.next = neigh_seq_next,
.stop = neigh_seq_stop,
.show = arp_seq_show,
};
-
-/* ------------------------------------------------------------------------ */
+#endif /* CONFIG_PROC_FS */
static int __net_init arp_net_init(struct net *net)
{
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] mt76：mt7603： move spin_lock_bh() to spin_lock()
From: Kalle Valo @ 2022-04-22 6:12 UTC (permalink / raw)
To: Yunbo Yu
Cc: nbd, lorenzo, ryder.lee, shayne.chen, sean.wang, davem, kuba,
pabeni, matthias.bgg, linux-wireless, netdev, linux-arm-kernel,
linux-mediatek, linux-kernel
In-Reply-To: <20220422060723.424862-1-yuyunbo519@gmail.com>
Yunbo Yu <yuyunbo519@gmail.com> writes:
> It is unnecessary to call spin_lock_bh(), for you are already in a tasklet.
>
> Signed-off-by: Yunbo Yu <yuyunbo519@gmail.com>
Why do you use unicode character "=EF=BC=9A"[1] as colon in the title?
After all recent news about left-to-right trickery all special
characters make me suspicious.
https://www.fileformat.info/info/unicode/char/ff1a/index.htm
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* [PATCH] net/mlx5: Remove useless kfree
From: Haowen Bai @ 2022-04-22 6:08 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky, David S. Miller, Jakub Kicinski,
Paolo Abeni
Cc: Haowen Bai, netdev, linux-rdma, linux-kernel
After alloc fail, we do not need to kfree.
Signed-off-by: Haowen Bai <baihaowen@meizu.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
index e49f51124c74..89aa0208b5d2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
@@ -1812,7 +1812,6 @@ __mlx5_tc_ct_flow_offload(struct mlx5_tc_ct_priv *ct_priv,
ct_flow = kzalloc(sizeof(*ct_flow), GFP_KERNEL);
if (!ct_flow) {
- kfree(ct_flow);
return ERR_PTR(-ENOMEM);
}
--
2.7.4
^ permalink raw reply related
* [PATCH] mt76：mt7603： move spin_lock_bh() to spin_lock()
From: Yunbo Yu @ 2022-04-22 6:07 UTC (permalink / raw)
To: nbd, lorenzo, ryder.lee, shayne.chen, sean.wang, kvalo, davem,
kuba, pabeni, matthias.bgg, yuyunbo519
Cc: linux-wireless, netdev, linux-arm-kernel, linux-mediatek,
linux-kernel
It is unnecessary to call spin_lock_bh(), for you are already in a tasklet.
Signed-off-by: Yunbo Yu <yuyunbo519@gmail.com>
---
drivers/net/wireless/mediatek/mt76/mt7603/beacon.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/beacon.c b/drivers/net/wireless/mediatek/mt76/mt7603/beacon.c
index 5d4522f440b7..b5e8308e0cc7 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7603/beacon.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7603/beacon.c
@@ -82,12 +82,12 @@ void mt7603_pre_tbtt_tasklet(struct tasklet_struct *t)
__skb_queue_head_init(&data.q);
q = dev->mphy.q_tx[MT_TXQ_BEACON];
- spin_lock_bh(&q->lock);
+ spin_lock(&q->lock);
ieee80211_iterate_active_interfaces_atomic(mt76_hw(dev),
IEEE80211_IFACE_ITER_RESUME_ALL,
mt7603_update_beacon_iter, dev);
mt76_queue_kick(dev, q);
- spin_unlock_bh(&q->lock);
+ spin_unlock(&q->lock);
/* Flush all previous CAB queue packets */
mt76_wr(dev, MT_WF_ARB_CAB_FLUSH, GENMASK(30, 16) | BIT(0));
@@ -117,7 +117,7 @@ void mt7603_pre_tbtt_tasklet(struct tasklet_struct *t)
mt76_skb_set_moredata(data.tail[i], false);
}
- spin_lock_bh(&q->lock);
+ spin_lock(&q->lock);
while ((skb = __skb_dequeue(&data.q)) != NULL) {
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
struct ieee80211_vif *vif = info->control.vif;
@@ -126,7 +126,7 @@ void mt7603_pre_tbtt_tasklet(struct tasklet_struct *t)
mt76_tx_queue_skb(dev, q, skb, &mvif->sta.wcid, NULL);
}
mt76_queue_kick(dev, q);
- spin_unlock_bh(&q->lock);
+ spin_unlock(&q->lock);
for (i = 0; i < ARRAY_SIZE(data.count); i++)
mt76_wr(dev, MT_WF_ARB_CAB_COUNT_B0_REG(i),
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2] brcmfmac: of: introduce new property to allow disable PNO
From: Kalle Valo @ 2022-04-22 5:59 UTC (permalink / raw)
To: Hermes Zhang
Cc: Arend van Spriel, Franky Lin, Hante Meuleman, David S. Miller,
Jakub Kicinski, Paolo Abeni, kernel, Hermes Zhang, linux-wireless,
brcm80211-dev-list.pdl, SHA-cyfmac-dev-list, netdev, linux-kernel
In-Reply-To: <20220422044419.3415842-1-chenhui.zhang@axis.com>
Hermes Zhang <chenhui.zhang@axis.com> writes:
> From: Hermes Zhang <chenhuiz@axis.com>
>
> The PNO feature need to be disable for some scenario in different
> product. This commit introduce a new property to allow the
> product-specific toggling of this feature.
"some scenario"? That's not really helpful.
> Signed-off-by: Hermes Zhang <chenhuiz@axis.com>
> ---
>
> Notes:
> Change property name to brcm,pno-disable
>
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> index 8623bde5eb70..121a195e4054 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> @@ -11,6 +11,7 @@
> #include "core.h"
> #include "common.h"
> #include "of.h"
> +#include "feature.h"
>
> static int brcmf_of_get_country_codes(struct device *dev,
> struct brcmf_mp_device *settings)
> @@ -102,6 +103,9 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
> if (bus_type != BRCMF_BUSTYPE_SDIO)
> return;
>
> + if (of_find_property(np, "brcm,pno-disable", NULL))
> + settings->feature_disable |= BIT(BRCMF_FEAT_PNO);
Is this DT property documented and acked by the Device Tree maintainers?
AFAIK DT is not supposed to be used as a software configuration
database.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] net/wireless: add debugfs exit function
From: Kalle Valo @ 2022-04-22 5:57 UTC (permalink / raw)
To: Bernard Zhao
Cc: Jakub Kicinski, David S. Miller, Paolo Abeni, Matthias Brugger,
linux-wireless, netdev, linux-arm-kernel, linux-mediatek,
linux-kernel, bernard
In-Reply-To: <20220422012830.342993-1-zhaojunkui2008@126.com>
Bernard Zhao <zhaojunkui2008@126.com> writes:
> This patch add exit debugfs function to mt7601u.
> Debugfs need to be cleanup when module is unloaded or load fail.
"load fail"? Please be more specific, are you saying that the second
module load fails or what?
> drivers/net/wireless/mediatek/mt7601u/debugfs.c | 9 +++++++--
> drivers/net/wireless/mediatek/mt7601u/init.c | 1 +
> drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 1 +
The title should be:
mt7601u: add debugfs exit function
> --- a/drivers/net/wireless/mediatek/mt7601u/debugfs.c
> +++ b/drivers/net/wireless/mediatek/mt7601u/debugfs.c
> @@ -9,6 +9,8 @@
> #include "mt7601u.h"
> #include "eeprom.h"
>
> +static struct dentry *dir;
How will this work when there are multiple mt7601u devices? Because of
that, avoid using non-const static variables.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] batman-adv: remove unnecessary type castings
From: kernel test robot @ 2022-04-22 5:07 UTC (permalink / raw)
To: Yu Zhe, mareklindner, sw, a, sven, davem, kuba, pabeni
Cc: kbuild-all, b.a.t.m.a.n, netdev, linux-kernel, liqiong,
kernel-janitors, Yu Zhe
In-Reply-To: <20220421154829.9775-1-yuzhe@nfschina.com>
Hi Yu,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.18-rc3 next-20220421]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Yu-Zhe/batman-adv-remove-unnecessary-type-castings/20220421-235254
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git b253435746d9a4a701b5f09211b9c14d3370d0da
config: parisc-randconfig-s031-20220421 (https://download.01.org/0day-ci/archive/20220422/202204221227.5z0xsJa9-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 11.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/intel-lab-lkp/linux/commit/2474b41c585e849d3546e0aba8f3c862735a04ff
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Yu-Zhe/batman-adv-remove-unnecessary-type-castings/20220421-235254
git checkout 2474b41c585e849d3546e0aba8f3c862735a04ff
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=parisc SHELL=/bin/bash net/batman-adv/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> net/batman-adv/bridge_loop_avoidance.c:68:42: sparse: sparse: incorrect type in initializer (different modifiers) @@ expected struct batadv_bla_claim *claim @@ got void const *data @@
net/batman-adv/bridge_loop_avoidance.c:68:42: sparse: expected struct batadv_bla_claim *claim
net/batman-adv/bridge_loop_avoidance.c:68:42: sparse: got void const *data
>> net/batman-adv/bridge_loop_avoidance.c:68:42: sparse: sparse: incorrect type in initializer (different modifiers) @@ expected struct batadv_bla_claim *claim @@ got void const *data @@
net/batman-adv/bridge_loop_avoidance.c:68:42: sparse: expected struct batadv_bla_claim *claim
net/batman-adv/bridge_loop_avoidance.c:68:42: sparse: got void const *data
--
>> net/batman-adv/translation-table.c:109:12: sparse: sparse: incorrect type in assignment (different modifiers) @@ expected struct batadv_tt_common_entry *tt @@ got void const *data @@
net/batman-adv/translation-table.c:109:12: sparse: expected struct batadv_tt_common_entry *tt
net/batman-adv/translation-table.c:109:12: sparse: got void const *data
>> net/batman-adv/translation-table.c:109:12: sparse: sparse: incorrect type in assignment (different modifiers) @@ expected struct batadv_tt_common_entry *tt @@ got void const *data @@
net/batman-adv/translation-table.c:109:12: sparse: expected struct batadv_tt_common_entry *tt
net/batman-adv/translation-table.c:109:12: sparse: got void const *data
vim +68 net/batman-adv/bridge_loop_avoidance.c
53
54 static void batadv_bla_periodic_work(struct work_struct *work);
55 static void
56 batadv_bla_send_announce(struct batadv_priv *bat_priv,
57 struct batadv_bla_backbone_gw *backbone_gw);
58
59 /**
60 * batadv_choose_claim() - choose the right bucket for a claim.
61 * @data: data to hash
62 * @size: size of the hash table
63 *
64 * Return: the hash index of the claim
65 */
66 static inline u32 batadv_choose_claim(const void *data, u32 size)
67 {
> 68 struct batadv_bla_claim *claim = data;
69 u32 hash = 0;
70
71 hash = jhash(&claim->addr, sizeof(claim->addr), hash);
72 hash = jhash(&claim->vid, sizeof(claim->vid), hash);
73
74 return hash % size;
75 }
76
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply
* [PATCH v2] brcmfmac: of: introduce new property to allow disable PNO
From: Hermes Zhang @ 2022-04-22 4:44 UTC (permalink / raw)
To: Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo,
David S. Miller, Jakub Kicinski, Paolo Abeni
Cc: kernel, Hermes Zhang, linux-wireless, brcm80211-dev-list.pdl,
SHA-cyfmac-dev-list, netdev, linux-kernel
From: Hermes Zhang <chenhuiz@axis.com>
The PNO feature need to be disable for some scenario in different
product. This commit introduce a new property to allow the
product-specific toggling of this feature.
Signed-off-by: Hermes Zhang <chenhuiz@axis.com>
---
Notes:
Change property name to brcm,pno-disable
drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
index 8623bde5eb70..121a195e4054 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
@@ -11,6 +11,7 @@
#include "core.h"
#include "common.h"
#include "of.h"
+#include "feature.h"
static int brcmf_of_get_country_codes(struct device *dev,
struct brcmf_mp_device *settings)
@@ -102,6 +103,9 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
if (bus_type != BRCMF_BUSTYPE_SDIO)
return;
+ if (of_find_property(np, "brcm,pno-disable", NULL))
+ settings->feature_disable |= BIT(BRCMF_FEAT_PNO);
+
if (of_property_read_u32(np, "brcm,drive-strength", &val) == 0)
sdio->drive_strength = val;
--
2.30.2
^ permalink raw reply related
* Re: [PATCH v3 net-next 4/4] rtnetlink: return EINVAL when request cannot succeed
From: Eric Dumazet @ 2022-04-22 4:16 UTC (permalink / raw)
To: Saeed Mahameed; +Cc: Florent Fourcot, netdev, Cong Wang, Brian Baboch
In-Reply-To: <20220422035049.lkdmvfhznfbwk7jw@sx1>
On Thu, Apr 21, 2022 at 8:50 PM Saeed Mahameed <saeed@kernel.org> wrote:
>
> On 05 Apr 15:42, Florent Fourcot wrote:
> >A request without interface name/interface index/interface group cannot
> >work. We should return EINVAL
> >
> >Signed-off-by: Florent Fourcot <florent.fourcot@wifirst.fr>
> >Signed-off-by: Brian Baboch <brian.baboch@wifirst.fr>
> >---
> > net/core/rtnetlink.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> >index e93f4058cf08..690324479cf5 100644
> >--- a/net/core/rtnetlink.c
> >+++ b/net/core/rtnetlink.c
> >@@ -3420,7 +3420,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
> > return rtnl_group_changelink(skb, net,
> > nla_get_u32(tb[IFLA_GROUP]),
> > ifm, extack, tb);
> >- return -ENODEV;
> >+ return -EINVAL;
> > }
>
> This introduced a regression iproute2->iplink_have_newlink() checks this
> return value to determine if newlink is supported by kernel, if the
> returned value is -EINVAL iproute2 falls back to ioctl mode, any value
> other than -EINVAL or -EOPNOTSUPP should be ok here to not break compatibility
> with iproute2.
>
Yep, but apparently network maintainers are MIA
https://patchwork.kernel.org/project/netdevbpf/patch/20220419125151.15589-1-florent.fourcot@wifirst.fr/
^ permalink raw reply
* Re: [PATCH v3 net-next 4/4] rtnetlink: return EINVAL when request cannot succeed
From: Saeed Mahameed @ 2022-04-22 3:50 UTC (permalink / raw)
To: Florent Fourcot; +Cc: netdev, cong.wang, edumazet, Brian Baboch
In-Reply-To: <20220405134237.16533-4-florent.fourcot@wifirst.fr>
On 05 Apr 15:42, Florent Fourcot wrote:
>A request without interface name/interface index/interface group cannot
>work. We should return EINVAL
>
>Signed-off-by: Florent Fourcot <florent.fourcot@wifirst.fr>
>Signed-off-by: Brian Baboch <brian.baboch@wifirst.fr>
>---
> net/core/rtnetlink.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>index e93f4058cf08..690324479cf5 100644
>--- a/net/core/rtnetlink.c
>+++ b/net/core/rtnetlink.c
>@@ -3420,7 +3420,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
> return rtnl_group_changelink(skb, net,
> nla_get_u32(tb[IFLA_GROUP]),
> ifm, extack, tb);
>- return -ENODEV;
>+ return -EINVAL;
> }
This introduced a regression iproute2->iplink_have_newlink() checks this
return value to determine if newlink is supported by kernel, if the
returned value is -EINVAL iproute2 falls back to ioctl mode, any value
other than -EINVAL or -EOPNOTSUPP should be ok here to not break compatibility
with iproute2.
^ permalink raw reply
* Re: [PATCH 4/5] net: phy: marvell: Add LED accessors for Marvell 88E1510
From: Kai-Heng Feng @ 2022-04-22 3:49 UTC (permalink / raw)
To: Andrew Lunn
Cc: hkallweit1, linux, peppe.cavallaro, alexandre.torgue, joabreu,
davem, kuba, pabeni, netdev, linux-kernel
In-Reply-To: <YmFUwXLDIW5ouDCd@lunn.ch>
On Thu, Apr 21, 2022 at 8:57 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Thu, Apr 21, 2022 at 08:24:00PM +0800, Kai-Heng Feng wrote:
> > On Thu, Apr 21, 2022 at 7:51 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > > This is not feasible.
> > > > If BIOS can define a method and restore the LED by itself, it can put
> > > > the method inside its S3 method and I don't have to work on this at
> > > > the first place.
> > >
> > > So maybe just declare the BIOS as FUBAR and move on to the next issue
> > > assigned to you.
> > >
> > > Do we really want the maintenance burden of this code for one machines
> > > BIOS?
> >
> > Wasn't this the "set precedence" we discussed earlier for? Someone has
> > to be the first, and more users will leverage the new property we
> > added.
>
> I both agree and disagree. I'm trying to make this feature generic,
> unlike you who seem to be doing the minimal, only saving one of three
> LED configuration registers. But on the other hand, i'm not sure there
> will be more users. Do you have a list of machines where the BIOS is
> FUBAR? Is it one machine? A range of machines from one vendor, or
> multiple vendors with multiple machines. I would feel better about the
> maintenance burden if i knew that this was going to be used a lot.
Right now it's only one machine. But someone has to be the first :)
>
> > > Maybe the better solution is to push back on the vendor and its
> > > BIOS, tell them how they should of done this, if the BIOS wants to be
> > > in control of the LEDs it needs to offer the methods to control the
> > > LEDs. And then hopefully the next machine the vendor produces will
> > > have working BIOS.
> >
> > The BIOS doesn't want to control the LED. It just provides a default
> > LED setting suitable for this platform, so the driver can use this
> > value over the hardcoded one in marvell phy driver.
>
> Exactly, it wants to control the LED, and tell the OS not to touch it
> ever.
That doesn't mean it wants to control the LED, it's still the phy
driver controls it.
>
> > So this really has nothing to do with with any ACPI method.
> > I believe the new property can be useful for DT world too.
>
> DT generally never trusts the bootloader to do anything. So i doubt
> such a DT property would ever be used. Also, DT is about describing
> the hardware, not how to configure the hardware. So you could list
> there is a PHY LED, what colour it is, etc. But in general, you would
> not describe how it is configured, that something else is configuring
> it and it should be left alone.
What if let the property list to the raw value of the LED should be?
So it can fall under "describing hardware" like 'clock-frequency' property.
>
> > > Your other option is to take part in the effort to add control of the
> > > LEDs via the standard Linux LED subsystem. The Marvel PHY driver is
> > > likely to be one of the first to gain support this for. So you can
> > > then totally take control of the LED from the BIOS and put it in the
> > > users hands. And such a solution will be applicable to many machines,
> > > not just one.
> >
> > This series just wants to use the default value platform firmware provides.
> > Create a sysfs to let user meddle with LED value doesn't really help
> > the case here.
>
> I would disagree. You can add a systemd service to configure it at
> boot however you want. It opens up the possibility to implement
> ethtool --identify in a generic way, etc. It is a much more powerful
> and useful feature than saying 'don't touch', and also it justify the
> maintenance burden.
That just pushed the maintenance burden to another subsystem and I
doubt it will bring more users than current approach.
Kai-Heng
>
> Andrew
^ permalink raw reply
* [PATCH] brcmfmac: of: introduce new property to allow disable PNO
From: Hermes Zhang @ 2022-04-22 3:24 UTC (permalink / raw)
To: Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo,
David S. Miller, Jakub Kicinski, Paolo Abeni
Cc: kernel, Hermes Zhang, linux-wireless, brcm80211-dev-list.pdl,
SHA-cyfmac-dev-list, netdev, linux-kernel
From: Hermes Zhang <chenhuiz@axis.com>
The PNO feature need to be disable for some scenario in different
product. This commit introduce a new property to allow the
product-specific toggling of this feature.
Signed-off-by: Hermes Zhang <chenhuiz@axis.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
index 8623bde5eb70..15f190368a8b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
@@ -11,6 +11,7 @@
#include "core.h"
#include "common.h"
#include "of.h"
+#include "feature.h"
static int brcmf_of_get_country_codes(struct device *dev,
struct brcmf_mp_device *settings)
@@ -102,6 +103,9 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
if (bus_type != BRCMF_BUSTYPE_SDIO)
return;
+ if (of_find_property(np, "pno-disable", NULL))
+ settings->feature_disable |= BIT(BRCMF_FEAT_PNO);
+
if (of_property_read_u32(np, "brcm,drive-strength", &val) == 0)
sdio->drive_strength = val;
--
2.30.2
^ permalink raw reply related
* Re: [PATCH net-next 0/3] ipv4: First steps toward removing RTO_ONLINK
From: David Ahern @ 2022-04-22 3:10 UTC (permalink / raw)
To: Guillaume Nault, David Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Hideaki YOSHIFUJI, dccp
In-Reply-To: <cover.1650470610.git.gnault@redhat.com>
On 4/20/22 5:21 PM, Guillaume Nault wrote:
> RTO_ONLINK is a flag that allows to reduce the scope of route lookups.
> It's stored in a normally unused bit of the ->flowi4_tos field, in
> struct flowi4. However it has several problems:
>
> * This bit is also used by ECN. Although ECN bits are supposed to be
> cleared before doing a route lookup, it happened that some code
> paths didn't properly sanitise their ->flowi4_tos. So this mechanism
> is fragile and we had bugs in the past where ECN bits slipped in and
> could end up being erroneously interpreted as RTO_ONLINK.
>
> * A dscp_t type was recently introduced to ensure ECN bits are cleared
> during route lookups. ->flowi4_tos is the most important structure
> field to convert, but RTO_ONLINK prevents such conversion, as dscp_t
> mandates that ECN bits (where RTO_ONLINK is stored) be zero.
>
> Therefore we need to stop using RTO_ONLINK altogether. Fortunately
> RTO_ONLINK isn't a necessity. Instead of passing a flag in ->flowi4_tos
> to tell the route lookup function to restrict the scope, we can simply
> initialise the scope correctly.
>
I believe the set looks ok. I think the fib test coverage in selftests
could use more tests to cover tos.
^ permalink raw reply
* Re: [PATCH net-next 3/3] ipv4: Initialise ->flowi4_scope properly in ICMP handlers.
From: David Ahern @ 2022-04-22 3:08 UTC (permalink / raw)
To: Guillaume Nault, David Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Hideaki YOSHIFUJI
In-Reply-To: <3aabf0b36042c11bc97343f899563cef2b9288e5.1650470610.git.gnault@redhat.com>
On 4/20/22 5:21 PM, Guillaume Nault wrote:
> All the *_redirect() and *_update_pmtu() functions initialise their
> struct flowi4 variable with either __build_flow_key() or
> build_sk_flow_key(). When sk is provided, these functions use
> RT_CONN_FLAGS() to set ->flowi4_tos and always use RT_SCOPE_UNIVERSE
> for ->flowi4_scope. Then they rely on ip_rt_fix_tos() to adjust the
> scope based on the RTO_ONLINK bit and to mask the tos with
> IPTOS_RT_MASK.
>
> This patch modifies __build_flow_key() and build_sk_flow_key() to
> properly initialise ->flowi4_tos and ->flowi4_scope, so that the
> ICMP redirects and PMTU handlers don't need an extra call to
> ip_rt_fix_tos() before doing a fib lookup. That is, we:
>
> * Drop RT_CONN_FLAGS(): use ip_sock_rt_tos() and ip_sock_rt_scope()
> instead, so that we don't have to rely on ip_rt_fix_tos() to adjust
> the scope anymore.
>
> * Apply IPTOS_RT_MASK to the tos, so that we don't need
> ip_rt_fix_tos() to do it for us.
>
> * Drop the ip_rt_fix_tos() calls that now become useless.
>
> The only remaining ip_rt_fix_tos() caller is ip_route_output_key_hash()
> which needs it as long as external callers still use the RTO_ONLINK
> flag.
>
> Note:
> This patch also drops some useless RT_TOS() calls as IPTOS_RT_MASK is
> a stronger mask.
>
> Signed-off-by: Guillaume Nault <gnault@redhat.com>
> ---
> net/ipv4/route.c | 37 +++++++++++++++++--------------------
> 1 file changed, 17 insertions(+), 20 deletions(-)
>
Reviewed-by: David Ahern <dsahern@kernel.org>
^ permalink raw reply
* Re: [PATCH] wwan_hwsim: Avoid flush_scheduled_work() usage
From: Tetsuo Handa @ 2022-04-22 3:02 UTC (permalink / raw)
To: Sergey Ryazanov, Loic Poulain
Cc: Johannes Berg, David S. Miller, Jakub Kicinski, Paolo Abeni,
Network Development
In-Reply-To: <CAHNKnsTkBiS8EKHXiF1MxoRfmGrv_Zrtgc2gaciCmZQREQULMQ@mail.gmail.com>
On 2022/04/22 1:14, Sergey Ryazanov wrote:
>> Do you want
>>
>> debugfs_remove(wwan_hwsim_debugfs_devcreate);
>>
>> here (as a separate patch)?
>
> Nope. But I will not be against such a patch. I remove the "devcreate"
> file in wwwan_hwsim_exit() to prevent new emulated device creation
> while the workqueue flushing, which can take a sufficient time. Here
> we cleanup the leftovers of the attempt to automatically create
> emulated devices. Here is no workqueue flushing, so the race window is
> very tight.
>
> In other words, the preparatory debugfs file removal is practically
> not required here, but it will not hurt anyone. And possibly will make
> the code less questionable.
OK. Since manual creation of emulated device via debugfs followed by
manual device deletion of emulated device via debugfs is possible before
automatic creation of emulated device via wwan_hwsim_init_devs() fails,
"/* Avoid new devs */" comment is applicable to this error path; I will
include debugfs_remove(wwan_hwsim_debugfs_devcreate) call.
On 2022/04/22 1:35, Sergey Ryazanov wrote:
>> @@ -506,9 +507,15 @@ static int __init wwan_hwsim_init(void)
>> if (wwan_hwsim_devsnum < 0 || wwan_hwsim_devsnum > 128)
>> return -EINVAL;
>>
>> + wwan_wq = alloc_workqueue("wwan_wq", 0, 0);
>> + if (!wwan_wq)
>> + return -ENOMEM;
>> +
>> wwan_hwsim_class = class_create(THIS_MODULE, "wwan_hwsim");
>> - if (IS_ERR(wwan_hwsim_class))
>> + if (IS_ERR(wwan_hwsim_class)) {
>> + destroy_workqueue(wwan_wq);
>
> How about jumping to some label from here and do the workqueue
> destroying there? E.g.
OK.
>> @@ -524,6 +531,7 @@ static int __init wwan_hwsim_init(void)
>>
>> err_clean_devs:
>> wwan_hwsim_free_devs();
>> + destroy_workqueue(wwan_wq);
>> debugfs_remove(wwan_hwsim_debugfs_topdir);
>> class_destroy(wwan_hwsim_class);
>
> As you can see there are no need to wait the workqueue flushing, since
> it was not used. So the queue destroying call can be moved below the
> class destroying to keep cleanup symmetrical to the init sequence.
I will add
debugfs_remove(wwan_hwsim_debugfs_devcreate); /* Avoid new devs */
here, for "it was not used" part is theoretically not always true.
>> @@ -534,7 +542,7 @@ static void __exit wwan_hwsim_exit(void)
>> {
>> debugfs_remove(wwan_hwsim_debugfs_devcreate); /* Avoid new devs */
>> wwan_hwsim_free_devs();
>> - flush_scheduled_work(); /* Wait deletion works completion */
>> + destroy_workqueue(wwan_wq); /* Wait deletion works completion */
>> debugfs_remove(wwan_hwsim_debugfs_topdir);
>> class_destroy(wwan_hwsim_class);
>> }
>
> I do not care too much, but can we explicitly call the queue flushing
> to make the exit handler as clear as possible?
OK.
On 2022/04/22 1:54, Sergey Ryazanov wrote:
> From what I understand, an inaccurate flushing of the system work
> queue can potentially cause a system freeze. That is why
> flush_scheduled_work() is planned to be removed. The hwsim module is
> just a random function user without any known issues. So, a 'fixes'
> tag is not required here, and there is no need to bother the stable
> team with a change backport.
Right, 'Fixes:' tag is not needed for this patch.
Flushing the system-wide workqueue is problematic under e.g. GFP_NOFS/GFP_NOIO context.
Removing flush_scheduled_work() is for proactively avoiding new problems like
https://lkml.kernel.org/r/385ce718-f965-4005-56b6-34922c4533b8@I-love.SAKURA.ne.jp
and https://lkml.kernel.org/r/20220225112405.355599-10-Jerome.Pouiller@silabs.com .
>
> Anyway, Tetsuo, you missed a target tree in the subject. If this is
> not a fix, then you probably should target your changes to the
> 'net-next' tree.
>
OK. I posted v2 patch at
https://lkml.kernel.org/r/7390d51f-60e2-3cee-5277-b819a55ceabe@I-love.SAKURA.ne.jp .
Thank you for responding.
^ permalink raw reply
* [PATCH v2] wwan_hwsim: Avoid flush_scheduled_work() usage
From: Tetsuo Handa @ 2022-04-22 3:01 UTC (permalink / raw)
To: Loic Poulain, Sergey Ryazanov, Johannes Berg, David S. Miller,
Jakub Kicinski, Paolo Abeni
Cc: Network Development
Flushing system-wide workqueues is dangerous and will be forbidden.
Replace system_wq with local wwan_wq.
While we are at it, make err_clean_devs: label of wwan_hwsim_init()
behave like wwan_hwsim_exit(), for it is theoretically possible to call
wwan_hwsim_debugfs_devcreate_write()/wwan_hwsim_debugfs_devdestroy_write()
by the moment wwan_hwsim_init_devs() returns.
Link: https://lkml.kernel.org/r/49925af7-78a8-a3dd-bce6-cfc02e1a9236@I-love.SAKURA.ne.jp
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reviewed-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---
Changes in v2:
Keep flush_workqueue(wwan_wq) explicit in order to match the comment.
Make error path of wwan_hwsim_init() identical to wwan_hwsim_exit().
drivers/net/wwan/wwan_hwsim.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wwan/wwan_hwsim.c b/drivers/net/wwan/wwan_hwsim.c
index 5b62cf3b3c42..fad642f9ffd8 100644
--- a/drivers/net/wwan/wwan_hwsim.c
+++ b/drivers/net/wwan/wwan_hwsim.c
@@ -33,6 +33,7 @@ static struct dentry *wwan_hwsim_debugfs_devcreate;
static DEFINE_SPINLOCK(wwan_hwsim_devs_lock);
static LIST_HEAD(wwan_hwsim_devs);
static unsigned int wwan_hwsim_dev_idx;
+static struct workqueue_struct *wwan_wq;
struct wwan_hwsim_dev {
struct list_head list;
@@ -371,7 +372,7 @@ static ssize_t wwan_hwsim_debugfs_portdestroy_write(struct file *file,
* waiting this callback to finish in the debugfs_remove() call. So,
* use workqueue.
*/
- schedule_work(&port->del_work);
+ queue_work(wwan_wq, &port->del_work);
return count;
}
@@ -416,7 +417,7 @@ static ssize_t wwan_hwsim_debugfs_devdestroy_write(struct file *file,
* waiting this callback to finish in the debugfs_remove() call. So,
* use workqueue.
*/
- schedule_work(&dev->del_work);
+ queue_work(wwan_wq, &dev->del_work);
return count;
}
@@ -506,9 +507,15 @@ static int __init wwan_hwsim_init(void)
if (wwan_hwsim_devsnum < 0 || wwan_hwsim_devsnum > 128)
return -EINVAL;
+ wwan_wq = alloc_workqueue("wwan_wq", 0, 0);
+ if (!wwan_wq)
+ return -ENOMEM;
+
wwan_hwsim_class = class_create(THIS_MODULE, "wwan_hwsim");
- if (IS_ERR(wwan_hwsim_class))
- return PTR_ERR(wwan_hwsim_class);
+ if (IS_ERR(wwan_hwsim_class)) {
+ err = PTR_ERR(wwan_hwsim_class);
+ goto err_wq_destroy;
+ }
wwan_hwsim_debugfs_topdir = debugfs_create_dir("wwan_hwsim", NULL);
wwan_hwsim_debugfs_devcreate =
@@ -523,9 +530,13 @@ static int __init wwan_hwsim_init(void)
return 0;
err_clean_devs:
+ debugfs_remove(wwan_hwsim_debugfs_devcreate); /* Avoid new devs */
wwan_hwsim_free_devs();
+ flush_workqueue(wwan_wq); /* Wait deletion works completion */
debugfs_remove(wwan_hwsim_debugfs_topdir);
class_destroy(wwan_hwsim_class);
+err_wq_destroy:
+ destroy_workqueue(wwan_wq);
return err;
}
@@ -534,9 +545,10 @@ static void __exit wwan_hwsim_exit(void)
{
debugfs_remove(wwan_hwsim_debugfs_devcreate); /* Avoid new devs */
wwan_hwsim_free_devs();
- flush_scheduled_work(); /* Wait deletion works completion */
+ flush_workqueue(wwan_wq); /* Wait deletion works completion */
debugfs_remove(wwan_hwsim_debugfs_topdir);
class_destroy(wwan_hwsim_class);
+ destroy_workqueue(wwan_wq);
}
module_init(wwan_hwsim_init);
--
2.32.0
^ permalink raw reply related
* Re: [PATCH -next] libbpf: Add additional null-pointer checking in make_parent_dir
From: cuigaosheng @ 2022-04-22 2:55 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh, Networking,
bpf, open list, gongruiqi1, wangweiyang2
In-Reply-To: <CAEf4Bza3inoAHsS0w=nKXNgxyFqzPXJVyDHq03Foody6Vgp7=Q@mail.gmail.com>
This email adjusts the code format.
I don't understand why we don't check path for NULL, bpf_link__pin is an
external
interface, It will be called by external functions and provide input
parameters,
for example in samples/bpf/hbm.c:
> 201 link = bpf_program__attach_cgroup(bpf_prog, cg1);
> 202 if (libbpf_get_error(link)) {
> 203 fprintf(stderr, "ERROR: bpf_program__attach_cgroup
> failed\n");
> 204 goto err;
> 205 }
> 206
> 207 sprintf(cg_pin_path, "/sys/fs/bpf/hbm%d", cg_id);
> 208 rc = bpf_link__pin(link, cg_pin_path);
> 209 if (rc < 0) {
> 210 printf("ERROR: bpf_link__pin failed: %d\n", rc);
> 211 goto err;
> 212 }
if cg_pin_path is NULL, strdup(NULL) will trigger a segmentation fault in
make_parent_dir, I think we should avoid this and add null-pointer checking
for path, just like check_path:
> 7673 static int check_path(const char *path)
> 7674 {
> 7675 char *cp, errmsg[STRERR_BUFSIZE];
> 7676 struct statfs st_fs;
> 7677 char *dname, *dir;
> 7678 int err = 0;
> 7679
> 7680 if (path == NULL)
> 7681 return -EINVAL;
> 7682
> 7683 dname = strdup(path);
> 7684 if (dname == NULL)
> 7685 return -ENOMEM;
> 7686
> 7687 dir = dirname(dname);
> 7688 if (statfs(dir, &st_fs)) {
> 7689 cp = libbpf_strerror_r(errno, errmsg,
> sizeof(errmsg));
> 7690 pr_warn("failed to statfs %s: %s\n", dir, cp);
> 7691 err = -errno;
> 7692 }
> 7693 free(dname);
> 7694
> 7695 if (!err && st_fs.f_type != BPF_FS_MAGIC) {
> 7696 pr_warn("specified path %s is not on BPF FS\n",
> path);
> 7697 err = -EINVAL;
> 7698 }
> 7699
> 7700 return err;
> 7701 }
Thanks.
在 2022/4/22 0:55, Andrii Nakryiko 写道:
> On Thu, Apr 21, 2022 at 6:01 AM Gaosheng Cui <cuigaosheng1@huawei.com> wrote:
>> The make_parent_dir is called without null-pointer checking for path,
>> such as bpf_link__pin. To ensure there is no null-pointer dereference
>> in make_parent_dir, so make_parent_dir requires additional null-pointer
>> checking for path.
>>
>> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
>> ---
>> tools/lib/bpf/libbpf.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index b53e51884f9e..5786e6184bf5 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -7634,6 +7634,9 @@ static int make_parent_dir(const char *path)
>> char *dname, *dir;
>> int err = 0;
>>
>> + if (path == NULL)
>> + return -EINVAL;
>> +
> API contract is that path shouldn't be NULL. Just like we don't check
> link, obj, prog for NULL in every single API, I don't think we need to
> do it here, unless I'm missing something?
>
>> dname = strdup(path);
>> if (dname == NULL)
>> return -ENOMEM;
>> --
>> 2.25.1
>>
> .
^ permalink raw reply
* Re: [PATCH] batman-adv: remove unnecessary type castings
From: kernel test robot @ 2022-04-22 2:51 UTC (permalink / raw)
To: Yu Zhe, mareklindner, sw, a, sven, davem, kuba, pabeni
Cc: llvm, kbuild-all, b.a.t.m.a.n, netdev, linux-kernel, liqiong,
kernel-janitors, Yu Zhe
In-Reply-To: <20220421154829.9775-1-yuzhe@nfschina.com>
Hi Yu,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.18-rc3 next-20220421]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Yu-Zhe/batman-adv-remove-unnecessary-type-castings/20220421-235254
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git b253435746d9a4a701b5f09211b9c14d3370d0da
config: i386-randconfig-a013 (https://download.01.org/0day-ci/archive/20220422/202204221034.hfPA4RPW-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 5bd87350a5ae429baf8f373cb226a57b62f87280)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/2474b41c585e849d3546e0aba8f3c862735a04ff
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Yu-Zhe/batman-adv-remove-unnecessary-type-castings/20220421-235254
git checkout 2474b41c585e849d3546e0aba8f3c862735a04ff
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash net/batman-adv/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> net/batman-adv/bridge_loop_avoidance.c:68:27: error: initializing 'struct batadv_bla_claim *' with an expression of type 'const void *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
struct batadv_bla_claim *claim = data;
^ ~~~~
1 error generated.
--
>> net/batman-adv/translation-table.c:109:5: error: assigning to 'struct batadv_tt_common_entry *' from 'const void *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
tt = data;
^ ~~~~
1 error generated.
vim +68 net/batman-adv/bridge_loop_avoidance.c
53
54 static void batadv_bla_periodic_work(struct work_struct *work);
55 static void
56 batadv_bla_send_announce(struct batadv_priv *bat_priv,
57 struct batadv_bla_backbone_gw *backbone_gw);
58
59 /**
60 * batadv_choose_claim() - choose the right bucket for a claim.
61 * @data: data to hash
62 * @size: size of the hash table
63 *
64 * Return: the hash index of the claim
65 */
66 static inline u32 batadv_choose_claim(const void *data, u32 size)
67 {
> 68 struct batadv_bla_claim *claim = data;
69 u32 hash = 0;
70
71 hash = jhash(&claim->addr, sizeof(claim->addr), hash);
72 hash = jhash(&claim->vid, sizeof(claim->vid), hash);
73
74 return hash % size;
75 }
76
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox