Hi, On 9/8/2021 2:06 PM, Martin KaFai Lau wrote: > On Wed, Sep 01, 2021 at 04:53:44PM +0800, Hou Tao wrote: >> Currently if a function ptr in struct_ops has a return value, its >> caller will get a random return value from it, because the return >> value of related BPF_PROG_TYPE_STRUCT_OPS prog is just dropped. >> >> So adding a new flag BPF_TRAMP_F_RET_FENTRY_RET to tell bpf trampoline >> to save and return the return value of struct_ops prog if ret_size of >> the function ptr is greater than 0. Also restricting the flag to be >> used alone. > Thanks for the report and fix! Sorry for the late reply. > > This bug is missed because the tcp-cc func is not always called. > A better test needs to be created to force exercising these funcs > in bpf_test_run(), which can be a follow-up patch in the bpf-next. > Could you help to create this test as a follow up? Yes, will do. The first thought comes into my mind is implementing .get_info hook in a bpf tcp_congestion_ops and checking its return value in userspace by getsockopt(fd, TCP_CC_INFO). I also consider to add a new BPF struct_ops for testing purpose, but it may be a little overkill. > The patch lgtm. However, it does not apply cleanly on bpf, > so please rebase and repost. I applied it manually and > tested it by hard coding to call the ->ssthresh() and > observes the return value. I just check that it can be applied both on bpf and bpf-next, do you have other commits in your tree ? > .
