On 7/26/24 8:39 AM, Leon Hwang wrote:
The commit f7866c3587337731 ("bpf: Fix null pointer dereference in
resolve_prog_type() for BPF_PROG_TYPE_EXT") fixed a NULL pointer
dereference panic, but didn't fix the issue that fails to update attached
freplace prog to prog_array map.
Since commit 1c123c567fb138eb ("bpf: Resolve fext program type when
checking map compatibility"), freplace prog and its target prog are able
to tail call each other.
And the commit 3aac1ead5eb6b76f ("bpf: Move prog->aux->linked_prog and
trampoline into bpf_link on attach") sets prog->aux->dst_prog as NULL
after attaching freplace prog to its target prog.
Similar to my previous comment in the cover letter, please use 12 alphanum
for the commit and the commit subject should be in the same line.
Then, as for following example:
tailcall_freplace.c:
// SPDX-License-Identifier: GPL-2.0
\#include <linux/bpf.h>
\#include <bpf/bpf_helpers.h>
struct {
__uint(type, BPF_MAP_TYPE_PROG_ARRAY);
__uint(max_entries, 1);
__uint(key_size, sizeof(__u32));
__uint(value_size, sizeof(__u32));
} jmp_table SEC(".maps");
int count = 0;
SEC("freplace")
int entry_freplace(struct __sk_buff *skb)
{
count++;
Empty line is not necessary.
bpf_tail_call_static(skb, &jmp_table, 0);
Empty line is not necessary.
return count;
}
char __license[] SEC("license") = "GPL";
tc_bpf2bpf.c:
// SPDX-License-Identifier: GPL-2.0
\#include <linux/bpf.h>
\#include <bpf/bpf_helpers.h>
__noinline
int subprog(struct __sk_buff *skb)
{
volatile int ret = 1;
asm volatile (""::"r+"(ret));
Let us replace the above asm volatile to __sink(ret). Also, I think 'volatile' is not needed.
Also remove the empty line.
return ret;
}
SEC("tc")
int entry_tc(struct __sk_buff *skb)
{
return subprog(skb);
}
char __license[] SEC("license") = "GPL";
And entry_freplace's target is the entry_tc's subprog.
After loading entry_freplace, the jmp_table's owner type is
BPF_PROG_TYPE_SCHED_CLS.
Next, after attaching entry_freplace to entry_tc's subprog, its prog->aux->
dst_prog is NULL.
Next, while updating entry_freplace to jmp_table, bpf_prog_map_compatible()
returns false because resolve_prog_type() returns BPF_PROG_TYPE_EXT instead
of BPF_PROG_TYPE_SCHED_CLS.
With this patch, resolve_prog_type() returns BPF_PROG_TYPE_SCHED_CLS to
support updating the attached entry_freplace to jmp_table.
Fixes: f7866c358733 ("bpf: Fix null pointer dereference in resolve_prog_type() for BPF_PROG_TYPE_EXT")
Cc: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
Cc: Martin KaFai Lau <martin.lau@xxxxxxxxxx>
Signed-off-by: Leon Hwang <leon.hwang@xxxxxxxxx>
Other than the above, LGTM.
Acked-by: Yonghong Song <yonghong.song@xxxxxxxxx>
