* [PATCH bpf-next 0/3] A couple of small refactorings of BPF program call sites
@ 2022-08-18 16:59 Toke Høiland-Jørgensen
2022-08-18 16:59 ` [PATCH bpf-next 1/3] dev: Move received_rps counter next to RPS members in softnet data Toke Høiland-Jørgensen
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Toke Høiland-Jørgensen @ 2022-08-18 16:59 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jesper Dangaard Brouer,
Björn Töpel, Magnus Karlsson, Maciej Fijalkowski,
Jonathan Lemon
Cc: Toke Høiland-Jørgensen, bpf, netdev
Stanislav suggested[0] that these small refactorings could be split out from the
XDP queueing RFC series and merged separately. The first change is a small
repacking of struct softnet_data, the others change the BPF call sites to
support full 64-bit values as arguments to bpf_redirect_map() and as the return
value of a BPF program, relying on the fact that BPF registers are always 64-bit
wide to maintain backwards compatibility.
Please see the individual patches for details.
[0] https://lore.kernel.org/r/CAKH8qBtdnku7StcQ-SamadvAF==DRuLLZO94yOR1WJ9Bg=uX1w@mail.gmail.com
Kumar Kartikeya Dwivedi (1):
bpf: Use 64-bit return value for bpf_prog_run
Toke Høiland-Jørgensen (2):
dev: Move received_rps counter next to RPS members in softnet data
bpf: Expand map key argument of bpf_redirect_map to u64
include/linux/bpf-cgroup.h | 12 +++++-----
include/linux/bpf.h | 16 ++++++-------
include/linux/filter.h | 46 +++++++++++++++++++-------------------
include/linux/netdevice.h | 2 +-
include/uapi/linux/bpf.h | 2 +-
kernel/bpf/cgroup.c | 12 +++++-----
kernel/bpf/core.c | 14 ++++++------
kernel/bpf/cpumap.c | 4 ++--
kernel/bpf/devmap.c | 4 ++--
kernel/bpf/offload.c | 4 ++--
kernel/bpf/verifier.c | 2 +-
net/bpf/test_run.c | 21 +++++++++--------
net/core/filter.c | 4 ++--
net/packet/af_packet.c | 7 ++++--
net/xdp/xskmap.c | 4 ++--
15 files changed, 80 insertions(+), 74 deletions(-)
--
2.37.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH bpf-next 1/3] dev: Move received_rps counter next to RPS members in softnet data
2022-08-18 16:59 [PATCH bpf-next 0/3] A couple of small refactorings of BPF program call sites Toke Høiland-Jørgensen
@ 2022-08-18 16:59 ` Toke Høiland-Jørgensen
2022-08-19 3:01 ` Jakub Kicinski
2022-08-18 16:59 ` [PATCH bpf-next 2/3] bpf: Expand map key argument of bpf_redirect_map to u64 Toke Høiland-Jørgensen
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Toke Høiland-Jørgensen @ 2022-08-18 16:59 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Toke Høiland-Jørgensen, netdev, bpf
Move the received_rps counter value next to the other RPS-related members
in softnet_data. This closes two four-byte holes in the structure, making
room for another pointer in the first two cache lines without bumping the
xmit struct to its own line.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
include/linux/netdevice.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 1a3cb93c3dcc..fe9aeca2fce9 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3100,7 +3100,6 @@ struct softnet_data {
/* stats */
unsigned int processed;
unsigned int time_squeeze;
- unsigned int received_rps;
#ifdef CONFIG_RPS
struct softnet_data *rps_ipi_list;
#endif
@@ -3133,6 +3132,7 @@ struct softnet_data {
unsigned int cpu;
unsigned int input_queue_tail;
#endif
+ unsigned int received_rps;
unsigned int dropped;
struct sk_buff_head input_pkt_queue;
struct napi_struct backlog;
--
2.37.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next 2/3] bpf: Expand map key argument of bpf_redirect_map to u64
2022-08-18 16:59 [PATCH bpf-next 0/3] A couple of small refactorings of BPF program call sites Toke Høiland-Jørgensen
2022-08-18 16:59 ` [PATCH bpf-next 1/3] dev: Move received_rps counter next to RPS members in softnet data Toke Høiland-Jørgensen
@ 2022-08-18 16:59 ` Toke Høiland-Jørgensen
2022-08-18 16:59 ` [PATCH bpf-next 3/3] bpf: Use 64-bit return value for bpf_prog_run Toke Høiland-Jørgensen
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Toke Høiland-Jørgensen @ 2022-08-18 16:59 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, David S. Miller,
Jakub Kicinski, Jesper Dangaard Brouer, Björn Töpel,
Magnus Karlsson, Maciej Fijalkowski, Jonathan Lemon
Cc: Toke Høiland-Jørgensen, Eric Dumazet, Paolo Abeni, bpf,
netdev
For queueing packets in XDP we want to add a new redirect map type with
support for 64-bit indexes. To prepare fore this, expand the width of the
'key' argument to the bpf_redirect_map() helper. Since BPF registers are
always 64-bit, this should be safe to do after the fact.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
include/linux/bpf.h | 2 +-
include/linux/filter.h | 12 ++++++------
include/uapi/linux/bpf.h | 2 +-
kernel/bpf/cpumap.c | 4 ++--
kernel/bpf/devmap.c | 4 ++--
kernel/bpf/verifier.c | 2 +-
net/core/filter.c | 4 ++--
net/xdp/xskmap.c | 4 ++--
8 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index a627a02cf8ab..9868dacce9c8 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -133,7 +133,7 @@ struct bpf_map_ops {
struct bpf_local_storage __rcu ** (*map_owner_storage_ptr)(void *owner);
/* Misc helpers.*/
- int (*map_redirect)(struct bpf_map *map, u32 ifindex, u64 flags);
+ int (*map_redirect)(struct bpf_map *map, u64 key, u64 flags);
/* map_meta_equal must be implemented for maps that can be
* used as an inner map. It is a runtime check to ensure
diff --git a/include/linux/filter.h b/include/linux/filter.h
index a5f21dc3c432..bd47a3ed681a 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -637,13 +637,13 @@ struct bpf_nh_params {
};
struct bpf_redirect_info {
- u32 flags;
- u32 tgt_index;
+ u64 tgt_index;
void *tgt_value;
struct bpf_map *map;
+ u32 flags;
+ u32 kern_flags;
u32 map_id;
enum bpf_map_type map_type;
- u32 kern_flags;
struct bpf_nh_params nh;
};
@@ -1494,7 +1494,7 @@ static inline bool bpf_sk_lookup_run_v6(struct net *net, int protocol,
}
#endif /* IS_ENABLED(CONFIG_IPV6) */
-static __always_inline int __bpf_xdp_redirect_map(struct bpf_map *map, u32 ifindex,
+static __always_inline int __bpf_xdp_redirect_map(struct bpf_map *map, u64 index,
u64 flags, const u64 flag_mask,
void *lookup_elem(struct bpf_map *map, u32 key))
{
@@ -1505,7 +1505,7 @@ static __always_inline int __bpf_xdp_redirect_map(struct bpf_map *map, u32 ifind
if (unlikely(flags & ~(action_mask | flag_mask)))
return XDP_ABORTED;
- ri->tgt_value = lookup_elem(map, ifindex);
+ ri->tgt_value = lookup_elem(map, index);
if (unlikely(!ri->tgt_value) && !(flags & BPF_F_BROADCAST)) {
/* If the lookup fails we want to clear out the state in the
* redirect_info struct completely, so that if an eBPF program
@@ -1517,7 +1517,7 @@ static __always_inline int __bpf_xdp_redirect_map(struct bpf_map *map, u32 ifind
return flags & action_mask;
}
- ri->tgt_index = ifindex;
+ ri->tgt_index = index;
ri->map_id = map->id;
ri->map_type = map->map_type;
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 934a2a8beb87..525b7b07830b 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2610,7 +2610,7 @@ union bpf_attr {
* Return
* 0 on success, or a negative error in case of failure.
*
- * long bpf_redirect_map(struct bpf_map *map, u32 key, u64 flags)
+ * long bpf_redirect_map(struct bpf_map *map, u64 key, u64 flags)
* Description
* Redirect the packet to the endpoint referenced by *map* at
* index *key*. Depending on its type, this *map* can contain
diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
index b5ba34ddd4b6..39ed08a2bb52 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -668,9 +668,9 @@ static int cpu_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
return 0;
}
-static int cpu_map_redirect(struct bpf_map *map, u32 ifindex, u64 flags)
+static int cpu_map_redirect(struct bpf_map *map, u64 index, u64 flags)
{
- return __bpf_xdp_redirect_map(map, ifindex, flags, 0,
+ return __bpf_xdp_redirect_map(map, index, flags, 0,
__cpu_map_lookup_elem);
}
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index f9a87dcc5535..d01e4c55b376 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -992,14 +992,14 @@ static int dev_map_hash_update_elem(struct bpf_map *map, void *key, void *value,
map, key, value, map_flags);
}
-static int dev_map_redirect(struct bpf_map *map, u32 ifindex, u64 flags)
+static int dev_map_redirect(struct bpf_map *map, u64 ifindex, u64 flags)
{
return __bpf_xdp_redirect_map(map, ifindex, flags,
BPF_F_BROADCAST | BPF_F_EXCLUDE_INGRESS,
__dev_map_lookup_elem);
}
-static int dev_hash_map_redirect(struct bpf_map *map, u32 ifindex, u64 flags)
+static int dev_hash_map_redirect(struct bpf_map *map, u64 ifindex, u64 flags)
{
return __bpf_xdp_redirect_map(map, ifindex, flags,
BPF_F_BROADCAST | BPF_F_EXCLUDE_INGRESS,
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 2c1f8069f7b7..db9938fc8a3b 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -14197,7 +14197,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
BUILD_BUG_ON(!__same_type(ops->map_peek_elem,
(int (*)(struct bpf_map *map, void *value))NULL));
BUILD_BUG_ON(!__same_type(ops->map_redirect,
- (int (*)(struct bpf_map *map, u32 ifindex, u64 flags))NULL));
+ (int (*)(struct bpf_map *map, u64 index, u64 flags))NULL));
BUILD_BUG_ON(!__same_type(ops->map_for_each_callback,
(int (*)(struct bpf_map *map,
bpf_callback_t callback_fn,
diff --git a/net/core/filter.c b/net/core/filter.c
index 5669248aff25..d68df8dc0c8a 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4408,10 +4408,10 @@ static const struct bpf_func_proto bpf_xdp_redirect_proto = {
.arg2_type = ARG_ANYTHING,
};
-BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex,
+BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u64, key,
u64, flags)
{
- return map->ops->map_redirect(map, ifindex, flags);
+ return map->ops->map_redirect(map, key, flags);
}
static const struct bpf_func_proto bpf_xdp_redirect_map_proto = {
diff --git a/net/xdp/xskmap.c b/net/xdp/xskmap.c
index acc8e52a4f5f..771d0fa90ef5 100644
--- a/net/xdp/xskmap.c
+++ b/net/xdp/xskmap.c
@@ -231,9 +231,9 @@ static int xsk_map_delete_elem(struct bpf_map *map, void *key)
return 0;
}
-static int xsk_map_redirect(struct bpf_map *map, u32 ifindex, u64 flags)
+static int xsk_map_redirect(struct bpf_map *map, u64 index, u64 flags)
{
- return __bpf_xdp_redirect_map(map, ifindex, flags, 0,
+ return __bpf_xdp_redirect_map(map, index, flags, 0,
__xsk_map_lookup_elem);
}
--
2.37.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next 3/3] bpf: Use 64-bit return value for bpf_prog_run
2022-08-18 16:59 [PATCH bpf-next 0/3] A couple of small refactorings of BPF program call sites Toke Høiland-Jørgensen
2022-08-18 16:59 ` [PATCH bpf-next 1/3] dev: Move received_rps counter next to RPS members in softnet data Toke Høiland-Jørgensen
2022-08-18 16:59 ` [PATCH bpf-next 2/3] bpf: Expand map key argument of bpf_redirect_map to u64 Toke Høiland-Jørgensen
@ 2022-08-18 16:59 ` Toke Høiland-Jørgensen
2022-08-18 22:26 ` [PATCH bpf-next 0/3] A couple of small refactorings of BPF program call sites sdf
2022-08-23 22:29 ` Daniel Borkmann
4 siblings, 0 replies; 12+ messages in thread
From: Toke Høiland-Jørgensen @ 2022-08-18 16:59 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, David S. Miller,
Jakub Kicinski, Jesper Dangaard Brouer
Cc: Kumar Kartikeya Dwivedi, Toke Høiland-Jørgensen,
Eric Dumazet, Paolo Abeni, bpf, netdev
From: Kumar Kartikeya Dwivedi <memxor@gmail.com>
BPF ABI always uses 64-bit return value, but so far __bpf_prog_run and
higher level wrappers always truncated the return value to 32-bit. We want
to be able to introduce a new BPF program type that returns a PTR_TO_BTF_ID
or NULL from the BPF program to the caller context in the kernel. To be
able to use this returned pointer value, the bpf_prog_run invocation needs
to be able to return a 64-bit value, so update the definitions to allow
this.
To avoid code churn in the whole kernel, we let the compiler handle
truncation normally, and allow new call sites to utilize the 64-bit
return value, by receiving the return value as a u64.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
include/linux/bpf-cgroup.h | 12 ++++++------
include/linux/bpf.h | 14 +++++++-------
include/linux/filter.h | 34 +++++++++++++++++-----------------
kernel/bpf/cgroup.c | 12 ++++++------
kernel/bpf/core.c | 14 +++++++-------
kernel/bpf/offload.c | 4 ++--
net/bpf/test_run.c | 21 ++++++++++++---------
net/packet/af_packet.c | 7 +++++--
8 files changed, 62 insertions(+), 56 deletions(-)
diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index 2bd1b5f8de9b..e975f89c491b 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -23,12 +23,12 @@ struct ctl_table;
struct ctl_table_header;
struct task_struct;
-unsigned int __cgroup_bpf_run_lsm_sock(const void *ctx,
- const struct bpf_insn *insn);
-unsigned int __cgroup_bpf_run_lsm_socket(const void *ctx,
- const struct bpf_insn *insn);
-unsigned int __cgroup_bpf_run_lsm_current(const void *ctx,
- const struct bpf_insn *insn);
+u64 __cgroup_bpf_run_lsm_sock(const void *ctx,
+ const struct bpf_insn *insn);
+u64 __cgroup_bpf_run_lsm_socket(const void *ctx,
+ const struct bpf_insn *insn);
+u64 __cgroup_bpf_run_lsm_current(const void *ctx,
+ const struct bpf_insn *insn);
#ifdef CONFIG_CGROUP_BPF
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 9868dacce9c8..28bf9f1bd961 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -57,8 +57,8 @@ typedef u64 (*bpf_callback_t)(u64, u64, u64, u64, u64);
typedef int (*bpf_iter_init_seq_priv_t)(void *private_data,
struct bpf_iter_aux_info *aux);
typedef void (*bpf_iter_fini_seq_priv_t)(void *private_data);
-typedef unsigned int (*bpf_func_t)(const void *,
- const struct bpf_insn *);
+typedef u64 (*bpf_func_t)(const void *,
+ const struct bpf_insn *);
struct bpf_iter_seq_info {
const struct seq_operations *seq_ops;
bpf_iter_init_seq_priv_t init_seq_private;
@@ -895,7 +895,7 @@ struct bpf_dispatcher {
struct bpf_ksym ksym;
};
-static __always_inline __nocfi unsigned int bpf_dispatcher_nop_func(
+static __always_inline __nocfi u64 bpf_dispatcher_nop_func(
const void *ctx,
const struct bpf_insn *insnsi,
bpf_func_t bpf_func)
@@ -924,7 +924,7 @@ int arch_prepare_bpf_dispatcher(void *image, s64 *funcs, int num_funcs);
}
#define DEFINE_BPF_DISPATCHER(name) \
- noinline __nocfi unsigned int bpf_dispatcher_##name##_func( \
+ noinline __nocfi u64 bpf_dispatcher_##name##_func( \
const void *ctx, \
const struct bpf_insn *insnsi, \
bpf_func_t bpf_func) \
@@ -935,7 +935,7 @@ int arch_prepare_bpf_dispatcher(void *image, s64 *funcs, int num_funcs);
struct bpf_dispatcher bpf_dispatcher_##name = \
BPF_DISPATCHER_INIT(bpf_dispatcher_##name);
#define DECLARE_BPF_DISPATCHER(name) \
- unsigned int bpf_dispatcher_##name##_func( \
+ u64 bpf_dispatcher_##name##_func( \
const void *ctx, \
const struct bpf_insn *insnsi, \
bpf_func_t bpf_func); \
@@ -1139,7 +1139,7 @@ struct bpf_prog {
u8 tag[BPF_TAG_SIZE];
struct bpf_prog_stats __percpu *stats;
int __percpu *active;
- unsigned int (*bpf_func)(const void *ctx,
+ u64 (*bpf_func)(const void *ctx,
const struct bpf_insn *insn);
struct bpf_prog_aux *aux; /* Auxiliary fields */
struct sock_fprog_kern *orig_prog; /* Original BPF program */
@@ -1488,7 +1488,7 @@ static inline void bpf_reset_run_ctx(struct bpf_run_ctx *old_ctx)
/* BPF program asks to set CN on the packet. */
#define BPF_RET_SET_CN (1 << 0)
-typedef u32 (*bpf_prog_run_fn)(const struct bpf_prog *prog, const void *ctx);
+typedef u64 (*bpf_prog_run_fn)(const struct bpf_prog *prog, const void *ctx);
static __always_inline u32
bpf_prog_run_array(const struct bpf_prog_array *array,
diff --git a/include/linux/filter.h b/include/linux/filter.h
index bd47a3ed681a..1c2ba627e82c 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -567,16 +567,16 @@ struct sk_filter {
DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
-typedef unsigned int (*bpf_dispatcher_fn)(const void *ctx,
- const struct bpf_insn *insnsi,
- unsigned int (*bpf_func)(const void *,
- const struct bpf_insn *));
+typedef u64 (*bpf_dispatcher_fn)(const void *ctx,
+ const struct bpf_insn *insnsi,
+ u64 (*bpf_func)(const void *,
+ const struct bpf_insn *));
-static __always_inline u32 __bpf_prog_run(const struct bpf_prog *prog,
+static __always_inline u64 __bpf_prog_run(const struct bpf_prog *prog,
const void *ctx,
bpf_dispatcher_fn dfunc)
{
- u32 ret;
+ u64 ret;
cant_migrate();
if (static_branch_unlikely(&bpf_stats_enabled_key)) {
@@ -596,7 +596,7 @@ static __always_inline u32 __bpf_prog_run(const struct bpf_prog *prog,
return ret;
}
-static __always_inline u32 bpf_prog_run(const struct bpf_prog *prog, const void *ctx)
+static __always_inline u64 bpf_prog_run(const struct bpf_prog *prog, const void *ctx)
{
return __bpf_prog_run(prog, ctx, bpf_dispatcher_nop_func);
}
@@ -609,10 +609,10 @@ static __always_inline u32 bpf_prog_run(const struct bpf_prog *prog, const void
* invocation of a BPF program does not require reentrancy protection
* against a BPF program which is invoked from a preempting task.
*/
-static inline u32 bpf_prog_run_pin_on_cpu(const struct bpf_prog *prog,
+static inline u64 bpf_prog_run_pin_on_cpu(const struct bpf_prog *prog,
const void *ctx)
{
- u32 ret;
+ u64 ret;
migrate_disable();
ret = bpf_prog_run(prog, ctx);
@@ -708,13 +708,13 @@ static inline u8 *bpf_skb_cb(const struct sk_buff *skb)
}
/* Must be invoked with migration disabled */
-static inline u32 __bpf_prog_run_save_cb(const struct bpf_prog *prog,
+static inline u64 __bpf_prog_run_save_cb(const struct bpf_prog *prog,
const void *ctx)
{
const struct sk_buff *skb = ctx;
u8 *cb_data = bpf_skb_cb(skb);
u8 cb_saved[BPF_SKB_CB_LEN];
- u32 res;
+ u64 res;
if (unlikely(prog->cb_access)) {
memcpy(cb_saved, cb_data, sizeof(cb_saved));
@@ -729,10 +729,10 @@ static inline u32 __bpf_prog_run_save_cb(const struct bpf_prog *prog,
return res;
}
-static inline u32 bpf_prog_run_save_cb(const struct bpf_prog *prog,
+static inline u64 bpf_prog_run_save_cb(const struct bpf_prog *prog,
struct sk_buff *skb)
{
- u32 res;
+ u64 res;
migrate_disable();
res = __bpf_prog_run_save_cb(prog, skb);
@@ -740,11 +740,11 @@ static inline u32 bpf_prog_run_save_cb(const struct bpf_prog *prog,
return res;
}
-static inline u32 bpf_prog_run_clear_cb(const struct bpf_prog *prog,
+static inline u64 bpf_prog_run_clear_cb(const struct bpf_prog *prog,
struct sk_buff *skb)
{
u8 *cb_data = bpf_skb_cb(skb);
- u32 res;
+ u64 res;
if (unlikely(prog->cb_access))
memset(cb_data, 0, BPF_SKB_CB_LEN);
@@ -759,14 +759,14 @@ DECLARE_STATIC_KEY_FALSE(bpf_master_redirect_enabled_key);
u32 xdp_master_redirect(struct xdp_buff *xdp);
-static __always_inline u32 bpf_prog_run_xdp(const struct bpf_prog *prog,
+static __always_inline u64 bpf_prog_run_xdp(const struct bpf_prog *prog,
struct xdp_buff *xdp)
{
/* Driver XDP hooks are invoked within a single NAPI poll cycle and thus
* under local_bh_disable(), which provides the needed RCU protection
* for accessing map entries.
*/
- u32 act = __bpf_prog_run(prog, xdp, BPF_DISPATCHER_FUNC(xdp));
+ u64 act = __bpf_prog_run(prog, xdp, BPF_DISPATCHER_FUNC(xdp));
if (static_branch_unlikely(&bpf_master_redirect_enabled_key)) {
if (act == XDP_TX && netif_is_bond_slave(xdp->rxq->dev))
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 59b7eb60d5b4..1721b09d0838 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -63,8 +63,8 @@ bpf_prog_run_array_cg(const struct cgroup_bpf *cgrp,
return run_ctx.retval;
}
-unsigned int __cgroup_bpf_run_lsm_sock(const void *ctx,
- const struct bpf_insn *insn)
+u64 __cgroup_bpf_run_lsm_sock(const void *ctx,
+ const struct bpf_insn *insn)
{
const struct bpf_prog *shim_prog;
struct sock *sk;
@@ -85,8 +85,8 @@ unsigned int __cgroup_bpf_run_lsm_sock(const void *ctx,
return ret;
}
-unsigned int __cgroup_bpf_run_lsm_socket(const void *ctx,
- const struct bpf_insn *insn)
+u64 __cgroup_bpf_run_lsm_socket(const void *ctx,
+ const struct bpf_insn *insn)
{
const struct bpf_prog *shim_prog;
struct socket *sock;
@@ -107,8 +107,8 @@ unsigned int __cgroup_bpf_run_lsm_socket(const void *ctx,
return ret;
}
-unsigned int __cgroup_bpf_run_lsm_current(const void *ctx,
- const struct bpf_insn *insn)
+u64 __cgroup_bpf_run_lsm_current(const void *ctx,
+ const struct bpf_insn *insn)
{
const struct bpf_prog *shim_prog;
struct cgroup *cgrp;
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 639437f36928..7549d765f7b6 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1999,7 +1999,7 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
#define PROG_NAME(stack_size) __bpf_prog_run##stack_size
#define DEFINE_BPF_PROG_RUN(stack_size) \
-static unsigned int PROG_NAME(stack_size)(const void *ctx, const struct bpf_insn *insn) \
+static u64 PROG_NAME(stack_size)(const void *ctx, const struct bpf_insn *insn) \
{ \
u64 stack[stack_size / sizeof(u64)]; \
u64 regs[MAX_BPF_EXT_REG]; \
@@ -2043,8 +2043,8 @@ EVAL4(DEFINE_BPF_PROG_RUN_ARGS, 416, 448, 480, 512);
#define PROG_NAME_LIST(stack_size) PROG_NAME(stack_size),
-static unsigned int (*interpreters[])(const void *ctx,
- const struct bpf_insn *insn) = {
+static u64 (*interpreters[])(const void *ctx,
+ const struct bpf_insn *insn) = {
EVAL6(PROG_NAME_LIST, 32, 64, 96, 128, 160, 192)
EVAL6(PROG_NAME_LIST, 224, 256, 288, 320, 352, 384)
EVAL4(PROG_NAME_LIST, 416, 448, 480, 512)
@@ -2069,8 +2069,8 @@ void bpf_patch_call_args(struct bpf_insn *insn, u32 stack_depth)
}
#else
-static unsigned int __bpf_prog_ret0_warn(const void *ctx,
- const struct bpf_insn *insn)
+static u64 __bpf_prog_ret0_warn(const void *ctx,
+ const struct bpf_insn *insn)
{
/* If this handler ever gets executed, then BPF_JIT_ALWAYS_ON
* is not working properly, so warn about it!
@@ -2205,8 +2205,8 @@ struct bpf_prog *bpf_prog_select_runtime(struct bpf_prog *fp, int *err)
}
EXPORT_SYMBOL_GPL(bpf_prog_select_runtime);
-static unsigned int __bpf_prog_ret1(const void *ctx,
- const struct bpf_insn *insn)
+static u64 __bpf_prog_ret1(const void *ctx,
+ const struct bpf_insn *insn)
{
return 1;
}
diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index 13e4efc971e6..d6a37ab87511 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -246,8 +246,8 @@ static int bpf_prog_offload_translate(struct bpf_prog *prog)
return ret;
}
-static unsigned int bpf_prog_warn_on_exec(const void *ctx,
- const struct bpf_insn *insn)
+static u64 bpf_prog_warn_on_exec(const void *ctx,
+ const struct bpf_insn *insn)
{
WARN(1, "attempt to execute device eBPF program on the host!");
return 0;
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index afa7125252f6..784ca06121f0 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -370,7 +370,7 @@ static int bpf_test_run_xdp_live(struct bpf_prog *prog, struct xdp_buff *ctx,
}
static int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat,
- u32 *retval, u32 *time, bool xdp)
+ u64 *retval, u32 *time, bool xdp)
{
struct bpf_prog_array_item item = {.prog = prog};
struct bpf_run_ctx *old_ctx;
@@ -757,7 +757,7 @@ int bpf_prog_test_run_tracing(struct bpf_prog *prog,
struct bpf_fentry_test_t arg = {};
u16 side_effect = 0, ret = 0;
int b = 2, err = -EFAULT;
- u32 retval = 0;
+ u64 retval = 0;
if (kattr->test.flags || kattr->test.cpu || kattr->test.batch_size)
return -EINVAL;
@@ -797,7 +797,7 @@ int bpf_prog_test_run_tracing(struct bpf_prog *prog,
struct bpf_raw_tp_test_run_info {
struct bpf_prog *prog;
void *ctx;
- u32 retval;
+ u64 retval;
};
static void
@@ -1045,15 +1045,15 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
union bpf_attr __user *uattr)
{
bool is_l2 = false, is_direct_pkt_access = false;
+ u32 size = kattr->test.data_size_in, duration;
struct net *net = current->nsproxy->net_ns;
struct net_device *dev = net->loopback_dev;
- u32 size = kattr->test.data_size_in;
u32 repeat = kattr->test.repeat;
struct __sk_buff *ctx = NULL;
- u32 retval, duration;
int hh_len = ETH_HLEN;
struct sk_buff *skb;
struct sock *sk;
+ u64 retval;
void *data;
int ret;
@@ -1241,15 +1241,16 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
bool do_live = (kattr->test.flags & BPF_F_TEST_XDP_LIVE_FRAMES);
u32 tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
u32 batch_size = kattr->test.batch_size;
- u32 retval = 0, duration, max_data_sz;
u32 size = kattr->test.data_size_in;
u32 headroom = XDP_PACKET_HEADROOM;
u32 repeat = kattr->test.repeat;
struct netdev_rx_queue *rxqueue;
struct skb_shared_info *sinfo;
+ u32 duration, max_data_sz;
struct xdp_buff xdp = {};
int i, ret = -EINVAL;
struct xdp_md *ctx;
+ u64 retval = 0;
void *data;
if (prog->expected_attach_type == BPF_XDP_DEVMAP ||
@@ -1407,7 +1408,8 @@ int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
struct bpf_flow_keys flow_keys;
const struct ethhdr *eth;
unsigned int flags = 0;
- u32 retval, duration;
+ u32 duration;
+ u64 retval;
void *data;
int ret;
@@ -1472,8 +1474,9 @@ int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog, const union bpf_attr *kat
struct bpf_sk_lookup_kern ctx = {};
u32 repeat = kattr->test.repeat;
struct bpf_sk_lookup *user_ctx;
- u32 retval, duration;
int ret = -EINVAL;
+ u32 duration;
+ u64 retval;
if (kattr->test.flags || kattr->test.cpu || kattr->test.batch_size)
return -EINVAL;
@@ -1571,8 +1574,8 @@ int bpf_prog_test_run_syscall(struct bpf_prog *prog,
void __user *ctx_in = u64_to_user_ptr(kattr->test.ctx_in);
__u32 ctx_size_in = kattr->test.ctx_size_in;
void *ctx = NULL;
- u32 retval;
int err = 0;
+ u64 retval;
/* doesn't support data_in/out, ctx_out, duration, or repeat or flags */
if (kattr->test.data_in || kattr->test.data_out ||
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 5cbe07116e04..bc4d9ff6f91c 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1444,8 +1444,11 @@ static unsigned int fanout_demux_bpf(struct packet_fanout *f,
rcu_read_lock();
prog = rcu_dereference(f->bpf_prog);
- if (prog)
- ret = bpf_prog_run_clear_cb(prog, skb) % num;
+ if (prog) {
+ ret = bpf_prog_run_clear_cb(prog, skb);
+ /* For some architectures, we need to do modulus in 32-bit width */
+ ret %= num;
+ }
rcu_read_unlock();
return ret;
--
2.37.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 0/3] A couple of small refactorings of BPF program call sites
2022-08-18 16:59 [PATCH bpf-next 0/3] A couple of small refactorings of BPF program call sites Toke Høiland-Jørgensen
` (2 preceding siblings ...)
2022-08-18 16:59 ` [PATCH bpf-next 3/3] bpf: Use 64-bit return value for bpf_prog_run Toke Høiland-Jørgensen
@ 2022-08-18 22:26 ` sdf
2022-08-19 5:24 ` Kumar Kartikeya Dwivedi
2022-08-23 22:29 ` Daniel Borkmann
4 siblings, 1 reply; 12+ messages in thread
From: sdf @ 2022-08-18 22:26 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Hao Luo, Jiri Olsa, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jesper Dangaard Brouer,
Björn Töpel, Magnus Karlsson, Maciej Fijalkowski,
Jonathan Lemon, bpf, netdev
On 08/18, Toke H�iland-J�rgensen wrote:
> Stanislav suggested[0] that these small refactorings could be split out
> from the
> XDP queueing RFC series and merged separately. The first change is a small
> repacking of struct softnet_data, the others change the BPF call sites to
> support full 64-bit values as arguments to bpf_redirect_map() and as the
> return
> value of a BPF program, relying on the fact that BPF registers are always
> 64-bit
> wide to maintain backwards compatibility.
> Please see the individual patches for details.
> [0]
> https://lore.kernel.org/r/CAKH8qBtdnku7StcQ-SamadvAF==DRuLLZO94yOR1WJ9Bg=uX1w@mail.gmail.com
Looks like a nice cleanup to me:
Reviewed-by: Stanislav Fomichev <sdf@google.com>
Can you share more on this comment?
/* For some architectures, we need to do modulus in 32-bit width */
Some - which ones? And why do they need it to be 32-bit?
> Kumar Kartikeya Dwivedi (1):
> bpf: Use 64-bit return value for bpf_prog_run
> Toke H�iland-J�rgensen (2):
> dev: Move received_rps counter next to RPS members in softnet data
> bpf: Expand map key argument of bpf_redirect_map to u64
> include/linux/bpf-cgroup.h | 12 +++++-----
> include/linux/bpf.h | 16 ++++++-------
> include/linux/filter.h | 46 +++++++++++++++++++-------------------
> include/linux/netdevice.h | 2 +-
> include/uapi/linux/bpf.h | 2 +-
> kernel/bpf/cgroup.c | 12 +++++-----
> kernel/bpf/core.c | 14 ++++++------
> kernel/bpf/cpumap.c | 4 ++--
> kernel/bpf/devmap.c | 4 ++--
> kernel/bpf/offload.c | 4 ++--
> kernel/bpf/verifier.c | 2 +-
> net/bpf/test_run.c | 21 +++++++++--------
> net/core/filter.c | 4 ++--
> net/packet/af_packet.c | 7 ++++--
> net/xdp/xskmap.c | 4 ++--
> 15 files changed, 80 insertions(+), 74 deletions(-)
> --
> 2.37.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 1/3] dev: Move received_rps counter next to RPS members in softnet data
2022-08-18 16:59 ` [PATCH bpf-next 1/3] dev: Move received_rps counter next to RPS members in softnet data Toke Høiland-Jørgensen
@ 2022-08-19 3:01 ` Jakub Kicinski
2022-08-19 12:38 ` Toke Høiland-Jørgensen
0 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2022-08-19 3:01 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: David S. Miller, Eric Dumazet, Paolo Abeni, netdev, bpf
On Thu, 18 Aug 2022 18:59:03 +0200 Toke Høiland-Jørgensen wrote:
> Move the received_rps counter value next to the other RPS-related members
> in softnet_data. This closes two four-byte holes in the structure, making
> room for another pointer in the first two cache lines without bumping the
> xmit struct to its own line.
What's the pointer you're making space for (which I hope will explain
why this patch is part of this otherwise bpf series)?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 0/3] A couple of small refactorings of BPF program call sites
2022-08-18 22:26 ` [PATCH bpf-next 0/3] A couple of small refactorings of BPF program call sites sdf
@ 2022-08-19 5:24 ` Kumar Kartikeya Dwivedi
0 siblings, 0 replies; 12+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-08-19 5:24 UTC (permalink / raw)
To: sdf
Cc: Toke Høiland-Jørgensen, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Hao Luo, Jiri Olsa,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Jesper Dangaard Brouer, Björn Töpel, Magnus Karlsson,
Maciej Fijalkowski, Jonathan Lemon, bpf, netdev
On Fri, 19 Aug 2022 at 00:29, <sdf@google.com> wrote:
>
> On 08/18, Toke H�iland-J�rgensen wrote:
> > Stanislav suggested[0] that these small refactorings could be split out
> > from the
> > XDP queueing RFC series and merged separately. The first change is a small
> > repacking of struct softnet_data, the others change the BPF call sites to
> > support full 64-bit values as arguments to bpf_redirect_map() and as the
> > return
> > value of a BPF program, relying on the fact that BPF registers are always
> > 64-bit
> > wide to maintain backwards compatibility.
>
> > Please see the individual patches for details.
>
> > [0]
> > https://lore.kernel.org/r/CAKH8qBtdnku7StcQ-SamadvAF==DRuLLZO94yOR1WJ9Bg=uX1w@mail.gmail.com
>
> Looks like a nice cleanup to me:
> Reviewed-by: Stanislav Fomichev <sdf@google.com>
>
> Can you share more on this comment?
>
> /* For some architectures, we need to do modulus in 32-bit width */
>
> Some - which ones? And why do they need it to be 32-bit?
It was a fix for i386, I saw a kernel test robot error when it was
sitting in Toke's tree.
See https://lore.kernel.org/all/202203140800.8pr81INh-lkp@intel.com.
Once bpf_prog_run_clear_cb starts returning a 64-bit value, we now
need to save it in 32-bit before doing the modulus.
>
> > Kumar Kartikeya Dwivedi (1):
> > bpf: Use 64-bit return value for bpf_prog_run
>
> > Toke H�iland-J�rgensen (2):
> > dev: Move received_rps counter next to RPS members in softnet data
> > bpf: Expand map key argument of bpf_redirect_map to u64
>
> > include/linux/bpf-cgroup.h | 12 +++++-----
> > include/linux/bpf.h | 16 ++++++-------
> > include/linux/filter.h | 46 +++++++++++++++++++-------------------
> > include/linux/netdevice.h | 2 +-
> > include/uapi/linux/bpf.h | 2 +-
> > kernel/bpf/cgroup.c | 12 +++++-----
> > kernel/bpf/core.c | 14 ++++++------
> > kernel/bpf/cpumap.c | 4 ++--
> > kernel/bpf/devmap.c | 4 ++--
> > kernel/bpf/offload.c | 4 ++--
> > kernel/bpf/verifier.c | 2 +-
> > net/bpf/test_run.c | 21 +++++++++--------
> > net/core/filter.c | 4 ++--
> > net/packet/af_packet.c | 7 ++++--
> > net/xdp/xskmap.c | 4 ++--
> > 15 files changed, 80 insertions(+), 74 deletions(-)
>
> > --
> > 2.37.2
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 1/3] dev: Move received_rps counter next to RPS members in softnet data
2022-08-19 3:01 ` Jakub Kicinski
@ 2022-08-19 12:38 ` Toke Høiland-Jørgensen
2022-08-19 22:54 ` Jakub Kicinski
0 siblings, 1 reply; 12+ messages in thread
From: Toke Høiland-Jørgensen @ 2022-08-19 12:38 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: David S. Miller, Eric Dumazet, Paolo Abeni, netdev, bpf
Jakub Kicinski <kuba@kernel.org> writes:
> On Thu, 18 Aug 2022 18:59:03 +0200 Toke Høiland-Jørgensen wrote:
>> Move the received_rps counter value next to the other RPS-related members
>> in softnet_data. This closes two four-byte holes in the structure, making
>> room for another pointer in the first two cache lines without bumping the
>> xmit struct to its own line.
>
> What's the pointer you're making space for (which I hope will explain
> why this patch is part of this otherwise bpf series)?
The XDP queueing series adds a pointer to keep track of which interfaces
were scheduled for transmission using the XDP dequeue hook (similar to
how the qdisc wake code works):
https://lore.kernel.org/r/20220713111430.134810-12-toke@redhat.com
Note that it's still up in the air if this ends up being the way this
will be implemented, so I'm OK with dropping this patch for now if you'd
rather wait until it's really needed. OTOH it also seemed like a benign
change on its own, so I figured I might as well include this patch when
sending these out. WDYT?
-Toke
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 1/3] dev: Move received_rps counter next to RPS members in softnet data
2022-08-19 12:38 ` Toke Høiland-Jørgensen
@ 2022-08-19 22:54 ` Jakub Kicinski
2022-08-22 10:17 ` Toke Høiland-Jørgensen
0 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2022-08-19 22:54 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: David S. Miller, Eric Dumazet, Paolo Abeni, netdev, bpf
On Fri, 19 Aug 2022 14:38:14 +0200 Toke Høiland-Jørgensen wrote:
> Jakub Kicinski <kuba@kernel.org> writes:
> > On Thu, 18 Aug 2022 18:59:03 +0200 Toke Høiland-Jørgensen wrote:
> >> Move the received_rps counter value next to the other RPS-related members
> >> in softnet_data. This closes two four-byte holes in the structure, making
> >> room for another pointer in the first two cache lines without bumping the
> >> xmit struct to its own line.
> >
> > What's the pointer you're making space for (which I hope will explain
> > why this patch is part of this otherwise bpf series)?
>
> The XDP queueing series adds a pointer to keep track of which interfaces
> were scheduled for transmission using the XDP dequeue hook (similar to
> how the qdisc wake code works):
>
> https://lore.kernel.org/r/20220713111430.134810-12-toke@redhat.com
I see, it makes more sense now :)
> Note that it's still up in the air if this ends up being the way this
> will be implemented, so I'm OK with dropping this patch for now if you'd
> rather wait until it's really needed. OTOH it also seemed like a benign
> change on its own, so I figured I might as well include this patch when
> sending these out. WDYT?
Whatever is easiest :)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 1/3] dev: Move received_rps counter next to RPS members in softnet data
2022-08-19 22:54 ` Jakub Kicinski
@ 2022-08-22 10:17 ` Toke Høiland-Jørgensen
0 siblings, 0 replies; 12+ messages in thread
From: Toke Høiland-Jørgensen @ 2022-08-22 10:17 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: David S. Miller, Eric Dumazet, Paolo Abeni, netdev, bpf
Jakub Kicinski <kuba@kernel.org> writes:
> On Fri, 19 Aug 2022 14:38:14 +0200 Toke Høiland-Jørgensen wrote:
>> Jakub Kicinski <kuba@kernel.org> writes:
>> > On Thu, 18 Aug 2022 18:59:03 +0200 Toke Høiland-Jørgensen wrote:
>> >> Move the received_rps counter value next to the other RPS-related members
>> >> in softnet_data. This closes two four-byte holes in the structure, making
>> >> room for another pointer in the first two cache lines without bumping the
>> >> xmit struct to its own line.
>> >
>> > What's the pointer you're making space for (which I hope will explain
>> > why this patch is part of this otherwise bpf series)?
>>
>> The XDP queueing series adds a pointer to keep track of which interfaces
>> were scheduled for transmission using the XDP dequeue hook (similar to
>> how the qdisc wake code works):
>>
>> https://lore.kernel.org/r/20220713111430.134810-12-toke@redhat.com
>
> I see, it makes more sense now :)
>
>> Note that it's still up in the air if this ends up being the way this
>> will be implemented, so I'm OK with dropping this patch for now if you'd
>> rather wait until it's really needed. OTOH it also seemed like a benign
>> change on its own, so I figured I might as well include this patch when
>> sending these out. WDYT?
>
> Whatever is easiest :)
Alright, I'm OK with either so let's leave it up to the (BPF)
maintainers if they want to drop this patch or just merge the whole
series? :)
-Toke
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 0/3] A couple of small refactorings of BPF program call sites
2022-08-18 16:59 [PATCH bpf-next 0/3] A couple of small refactorings of BPF program call sites Toke Høiland-Jørgensen
` (3 preceding siblings ...)
2022-08-18 22:26 ` [PATCH bpf-next 0/3] A couple of small refactorings of BPF program call sites sdf
@ 2022-08-23 22:29 ` Daniel Borkmann
2022-08-25 13:15 ` Kumar Kartikeya Dwivedi
4 siblings, 1 reply; 12+ messages in thread
From: Daniel Borkmann @ 2022-08-23 22:29 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, Alexei Starovoitov,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Jesper Dangaard Brouer, Björn Töpel, Magnus Karlsson,
Maciej Fijalkowski, Jonathan Lemon
Cc: bpf, netdev
On 8/18/22 6:59 PM, Toke Høiland-Jørgensen wrote:
> Stanislav suggested[0] that these small refactorings could be split out from the
> XDP queueing RFC series and merged separately. The first change is a small
> repacking of struct softnet_data, the others change the BPF call sites to
> support full 64-bit values as arguments to bpf_redirect_map() and as the return
> value of a BPF program, relying on the fact that BPF registers are always 64-bit
> wide to maintain backwards compatibility.
>
> Please see the individual patches for details.
>
> [0] https://lore.kernel.org/r/CAKH8qBtdnku7StcQ-SamadvAF==DRuLLZO94yOR1WJ9Bg=uX1w@mail.gmail.com
>
> Kumar Kartikeya Dwivedi (1):
> bpf: Use 64-bit return value for bpf_prog_run
>
> Toke Høiland-Jørgensen (2):
> dev: Move received_rps counter next to RPS members in softnet data
> bpf: Expand map key argument of bpf_redirect_map to u64
Looks like this series throws NULL pointer derefs in the CI. I just reran it and
same result whereas various other bpf-next targeted patches CI seems green and w/o
below panic ... perhaps an issue in last patch; please investigate.
https://github.com/kernel-patches/bpf/runs/7982907380?check_suite_focus=true
[...]
#231 verif_scale_strobemeta:OK
#232 verif_scale_strobemeta_bpf_loop:OK
#233 verif_scale_strobemeta_nounroll1:OK
#234 verif_scale_strobemeta_nounroll2:OK
#235 verif_scale_strobemeta_subprogs:OK
#236 verif_scale_sysctl_loop1:OK
#237 verif_scale_sysctl_loop2:OK
#238 verif_scale_xdp_loop:OK
#239 verif_stats:OK
#240 verif_twfw:OK
[ 828.755223] BUG: kernel NULL pointer dereference, address: 0000000000000000
[ 828.755223] #PF: supervisor instruction fetch in kernel mode
[ 828.755223] #PF: error_code(0x0010) - not-present page
[ 828.755223] PGD 0 P4D 0
[ 828.755223] Oops: 0010 [#1] PREEMPT SMP NOPTI
[ 828.755223] CPU: 4 PID: 0 Comm: swapper/4 Tainted: G OE 5.19.0-g3141b1878b85 #1
[ 828.755223] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
[ 828.755223] RIP: 0010:0x0
[ 828.755223] Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
[ 828.755223] RSP: 0018:ffffbccd400b3ea8 EFLAGS: 00000046
[ 828.755223] RAX: 0000000000000002 RBX: ffff9e79f9d1efc0 RCX: 000000000000000a
[ 828.755223] RDX: 0000000000000000 RSI: 000000d2fe34b800 RDI: ffff9e79f9d1efc0
[ 828.755223] RBP: 000000d2fe34b800 R08: 0000000000000000 R09: 0000000000000000
[ 828.755223] R10: 00000000000f4240 R11: ffffffffb5062510 R12: 0000000000000015
[ 828.755223] R13: 7fffffffffffffff R14: 0000000000000004 R15: 000000d2fe34b800
[ 828.767338] FS: 0000000000000000(0000) GS:ffff9e79f9d00000(0000) knlGS:0000000000000000
[ 828.767338] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 828.767338] CR2: ffffffffffffffd6 CR3: 0000000100970000 CR4: 00000000000006e0
[ 828.767338] Call Trace:
[ 828.767338] <TASK>
[ 828.767338] tick_nohz_idle_stop_tick+0x1da/0x380
[ 828.767338] do_idle+0xe6/0x280
[ 828.767338] cpu_startup_entry+0x19/0x20
[ 828.767338] start_secondary+0x8f/0x90
[ 828.767338] secondary_startup_64_no_verify+0xe1/0xeb
[ 828.767338] </TASK>
[ 828.767338] Modules linked in: bpf_testmod(OE) [last unloaded: bpf_testmod(OE)]
[ 828.767338] CR2: 0000000000000000
[ 828.767338] ---[ end trace 0000000000000000 ]---
[ 828.758172] BUG: kernel NULL pointer dereference, address: 0000000000000000
[ 828.758172] #PF: supervisor instruction fetch in kernel mode
[ 828.758172] #PF: error_code(0x0010) - not-present page
[ 828.758172] PGD 0 P4D 0
[ 828.767338] RIP: 0010:0x0
[ 828.758172] Oops: 0010 [#2] PREEMPT SMP NOPTI
[ 828.758172] CPU: 7 PID: 0 Comm: swapper/7 Tainted: G D OE 5.19.0-g3141b1878b85 #1
[ 828.758172] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
[ 828.767338] Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
[ 828.767338] RSP: 0018:ffffbccd400b3ea8 EFLAGS: 00000046
[ 828.758172] RIP: 0010:0x0
[ 828.767338]
[ 828.758172] Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
[ 828.767338] RAX: 0000000000000002 RBX: ffff9e79f9d1efc0 RCX: 000000000000000a
[ 828.767338] RDX: 0000000000000000 RSI: 000000d2fe34b800 RDI: ffff9e79f9d1efc0
[ 828.767338] RBP: 000000d2fe34b800 R08: 0000000000000000 R09: 0000000000000000
[ 828.758172] RSP: 0018:ffffbccd400cbea0 EFLAGS: 00000046
[ 828.767338] R10: 00000000000f4240 R11: ffffffffb5062510 R12: 0000000000000015
[ 828.767338] R13: 7fffffffffffffff R14: 0000000000000004 R15: 000000d2fe34b800
[ 828.758172]
[ 828.758172] RAX: 0000000000000005 RBX: ffff9e79f9ddefc0 RCX: 000000000000000a
[ 828.758172] RDX: 0000000000000000 RSI: 000000c0f6c7a580 RDI: ffff9e79f9ddefc0
[ 828.767338] FS: 0000000000000000(0000) GS:ffff9e79f9d00000(0000) knlGS:0000000000000000
[ 828.758172] RBP: 0000000000000013 R08: 7fffffffffffffff R09: 000000c0f6b86340
[ 828.767338] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 828.758172] R10: 00000000000f4240 R11: ffffffffb5062510 R12: 0000000000000000
[ 828.758172] R13: 0000000000000000 R14: 000000c0f6bc6d28 R15: 0000000000000000
[ 828.758172] FS: 0000000000000000(0000) GS:ffff9e79f9dc0000(0000) knlGS:0000000000000000
[ 828.767338] CR2: ffffffffffffffd6 CR3: 0000000100970000 CR4: 00000000000006e0
[ 828.758172] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 828.767338] Kernel panic - not syncing: Fatal exception
[ 828.758172] CR2: ffffffffffffffd6 CR3: 000000009a836000 CR4: 00000000000006e0
[ 828.758172] Call Trace:
[ 828.758172] <TASK>
[ 828.758172] tick_nohz_restart_sched_tick+0x6b/0x90
[ 828.758172] tick_nohz_idle_exit+0xfc/0x150
[ 828.758172] do_idle+0x23c/0x280
[ 828.758172] cpu_startup_entry+0x19/0x20
[ 828.758172] start_secondary+0x8f/0x90
[ 828.758172] secondary_startup_64_no_verify+0xe1/0xeb
[ 828.758172] </TASK>
[...]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 0/3] A couple of small refactorings of BPF program call sites
2022-08-23 22:29 ` Daniel Borkmann
@ 2022-08-25 13:15 ` Kumar Kartikeya Dwivedi
0 siblings, 0 replies; 12+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-08-25 13:15 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Toke Høiland-Jørgensen, Alexei Starovoitov,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Jesper Dangaard Brouer, Björn Töpel, Magnus Karlsson,
Maciej Fijalkowski, Jonathan Lemon, bpf, netdev
On Wed, 24 Aug 2022 at 00:42, Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 8/18/22 6:59 PM, Toke Høiland-Jørgensen wrote:
> > Stanislav suggested[0] that these small refactorings could be split out from the
> > XDP queueing RFC series and merged separately. The first change is a small
> > repacking of struct softnet_data, the others change the BPF call sites to
> > support full 64-bit values as arguments to bpf_redirect_map() and as the return
> > value of a BPF program, relying on the fact that BPF registers are always 64-bit
> > wide to maintain backwards compatibility.
> >
> > Please see the individual patches for details.
> >
> > [0] https://lore.kernel.org/r/CAKH8qBtdnku7StcQ-SamadvAF==DRuLLZO94yOR1WJ9Bg=uX1w@mail.gmail.com
> >
> > Kumar Kartikeya Dwivedi (1):
> > bpf: Use 64-bit return value for bpf_prog_run
> >
> > Toke Høiland-Jørgensen (2):
> > dev: Move received_rps counter next to RPS members in softnet data
> > bpf: Expand map key argument of bpf_redirect_map to u64
>
> Looks like this series throws NULL pointer derefs in the CI. I just reran it and
> same result whereas various other bpf-next targeted patches CI seems green and w/o
> below panic ... perhaps an issue in last patch; please investigate.
Was it only occurring with LLVM before, or with GCC too?
>
> https://github.com/kernel-patches/bpf/runs/7982907380?check_suite_focus=true
>
I've been trying to reproduce this for a day with no luck. First I did
it with GCC, then I noticed that the CI is only red for LLVM, so I
also tried with LLVM 16.
I'll keep trying, but just wanted to update the thread. Also, would
there be a way to look at logs of the past runs (that you saw and then
triggered this failing run again)? Maybe their splat has some
difference which might provide more clues.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-08-25 13:15 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-18 16:59 [PATCH bpf-next 0/3] A couple of small refactorings of BPF program call sites Toke Høiland-Jørgensen
2022-08-18 16:59 ` [PATCH bpf-next 1/3] dev: Move received_rps counter next to RPS members in softnet data Toke Høiland-Jørgensen
2022-08-19 3:01 ` Jakub Kicinski
2022-08-19 12:38 ` Toke Høiland-Jørgensen
2022-08-19 22:54 ` Jakub Kicinski
2022-08-22 10:17 ` Toke Høiland-Jørgensen
2022-08-18 16:59 ` [PATCH bpf-next 2/3] bpf: Expand map key argument of bpf_redirect_map to u64 Toke Høiland-Jørgensen
2022-08-18 16:59 ` [PATCH bpf-next 3/3] bpf: Use 64-bit return value for bpf_prog_run Toke Høiland-Jørgensen
2022-08-18 22:26 ` [PATCH bpf-next 0/3] A couple of small refactorings of BPF program call sites sdf
2022-08-19 5:24 ` Kumar Kartikeya Dwivedi
2022-08-23 22:29 ` Daniel Borkmann
2022-08-25 13:15 ` Kumar Kartikeya Dwivedi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).