| Message ID | 20210719051816.11762-1-yajun.deng@linux.dev (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | fef773fc8110d8124c73a5e6610f89e52814637d |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | netlink: Deal with ESRCH error in nlmsg_notify() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | warning | 9 maintainers not CCed: marcelo.leitner@gmail.com bpf@vger.kernel.org daniel@iogearbox.net johannes.berg@intel.com 0x7f454c46@gmail.com rdunlap@infradead.org fw@strlen.de aahringo@redhat.com yangyingliang@huawei.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 4 this patch: 4 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 16 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 4 this patch: 4 |
| netdev/header_inline | success | Link |
On 7/18/21 10:18 PM, Yajun Deng wrote: > Yonghong Song report: > The bpf selftest tc_bpf failed with latest bpf-next. > The following is the command to run and the result: > $ ./test_progs -n 132 > [ 40.947571] bpf_testmod: loading out-of-tree module taints kernel. > test_tc_bpf:PASS:test_tc_bpf__open_and_load 0 nsec > test_tc_bpf:PASS:bpf_tc_hook_create(BPF_TC_INGRESS) 0 nsec > test_tc_bpf:PASS:bpf_tc_hook_create invalid hook.attach_point 0 nsec > test_tc_bpf_basic:PASS:bpf_obj_get_info_by_fd 0 nsec > test_tc_bpf_basic:PASS:bpf_tc_attach 0 nsec > test_tc_bpf_basic:PASS:handle set 0 nsec > test_tc_bpf_basic:PASS:priority set 0 nsec > test_tc_bpf_basic:PASS:prog_id set 0 nsec > test_tc_bpf_basic:PASS:bpf_tc_attach replace mode 0 nsec > test_tc_bpf_basic:PASS:bpf_tc_query 0 nsec > test_tc_bpf_basic:PASS:handle set 0 nsec > test_tc_bpf_basic:PASS:priority set 0 nsec > test_tc_bpf_basic:PASS:prog_id set 0 nsec > libbpf: Kernel error message: Failed to send filter delete notification > test_tc_bpf_basic:FAIL:bpf_tc_detach unexpected error: -3 (errno 3) > test_tc_bpf:FAIL:test_tc_internal ingress unexpected error: -3 (errno 3) > > The failure seems due to the commit > cfdf0d9ae75b ("rtnetlink: use nlmsg_notify() in rtnetlink_send()") > > Deal with ESRCH error in nlmsg_notify() even the report variable is zero. > > Reported-by: Yonghong Song <yhs@fb.com> > Signed-off-by: Yajun Deng <yajun.deng@linux.dev> Thanks for quick fix. This does fix the bpf selftest issu. But does this change have negative impacts on other nlmsg_notify() callers, below 1-3 items? 0 net/core/rtnetlink.c rtnetlink_send 714 return nlmsg_notify(rtnl, skb, pid, group, echo, GFP_KERNEL); 1 net/core/rtnetlink.c rtnl_notify 734 nlmsg_notify(rtnl, skb, pid, group, report, flags); 2 net/netfilter/nfnetlink.c nfnetlink_send 176 return nlmsg_notify(nfnlnet->nfnl, skb, portid, group, echo, flags); 3 net/netlink/genetlink.c genl_notify 1506 nlmsg_notify(sk, skb, info->snd_portid, group, report, flags); > --- > net/netlink/af_netlink.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c > index 380f95aacdec..24b7cf447bc5 100644 > --- a/net/netlink/af_netlink.c > +++ b/net/netlink/af_netlink.c > @@ -2545,13 +2545,15 @@ int nlmsg_notify(struct sock *sk, struct sk_buff *skb, u32 portid, > /* errors reported via destination sk->sk_err, but propagate > * delivery errors if NETLINK_BROADCAST_ERROR flag is set */ > err = nlmsg_multicast(sk, skb, exclude_portid, group, flags); > + if (err == -ESRCH) > + err = 0; > } > > if (report) { > int err2; > > err2 = nlmsg_unicast(sk, skb, portid); > - if (!err || err == -ESRCH) > + if (!err) > err = err2; > } > >
July 19, 2021 10:47 PM, "Yonghong Song" <yhs@fb.com> wrote: > On 7/18/21 10:18 PM, Yajun Deng wrote: > >> Yonghong Song report: >> The bpf selftest tc_bpf failed with latest bpf-next. >> The following is the command to run and the result: >> $ ./test_progs -n 132 >> [ 40.947571] bpf_testmod: loading out-of-tree module taints kernel. >> test_tc_bpf:PASS:test_tc_bpf__open_and_load 0 nsec >> test_tc_bpf:PASS:bpf_tc_hook_create(BPF_TC_INGRESS) 0 nsec >> test_tc_bpf:PASS:bpf_tc_hook_create invalid hook.attach_point 0 nsec >> test_tc_bpf_basic:PASS:bpf_obj_get_info_by_fd 0 nsec >> test_tc_bpf_basic:PASS:bpf_tc_attach 0 nsec >> test_tc_bpf_basic:PASS:handle set 0 nsec >> test_tc_bpf_basic:PASS:priority set 0 nsec >> test_tc_bpf_basic:PASS:prog_id set 0 nsec >> test_tc_bpf_basic:PASS:bpf_tc_attach replace mode 0 nsec >> test_tc_bpf_basic:PASS:bpf_tc_query 0 nsec >> test_tc_bpf_basic:PASS:handle set 0 nsec >> test_tc_bpf_basic:PASS:priority set 0 nsec >> test_tc_bpf_basic:PASS:prog_id set 0 nsec >> libbpf: Kernel error message: Failed to send filter delete notification >> test_tc_bpf_basic:FAIL:bpf_tc_detach unexpected error: -3 (errno 3) >> test_tc_bpf:FAIL:test_tc_internal ingress unexpected error: -3 (errno 3) >> The failure seems due to the commit >> cfdf0d9ae75b ("rtnetlink: use nlmsg_notify() in rtnetlink_send()") >> Deal with ESRCH error in nlmsg_notify() even the report variable is zero. >> Reported-by: Yonghong Song <yhs@fb.com> >> Signed-off-by: Yajun Deng <yajun.deng@linux.dev> > > Thanks for quick fix. This does fix the bpf selftest issu. > But does this change have negative impacts on other > nlmsg_notify() callers, below 1-3 items? > > 0 net/core/rtnetlink.c rtnetlink_send 714 return nlmsg_notify(rtnl, skb, pid, group, echo, > GFP_KERNEL); This is exactly what we need. > > 1 net/core/rtnetlink.c rtnl_notify 734 nlmsg_notify(rtnl, skb, pid, group, report, flags); > It doesn't matter because there is no return value. > 2 net/netfilter/nfnetlink.c nfnetlink_send 176 return nlmsg_notify(nfnlnet->nfnl, skb, portid, > group, echo, flags); > It only ctnetlink_conntrack_event() use the return value when call nfnetlink_send() in net/netfilter/nf_conntrack_netlink.c, but it doesn't matter when the return value is ESRCH or zero. > 3 net/netlink/genetlink.c genl_notify 1506 nlmsg_notify(sk, skb, info->snd_portid, group, report, > flags); > It doesn't matter because there is no return value. I think the caller for nlmsg_notify() doesn't need deal with the ESRCH. It also deal with ESRCH when report variable is not zero. >> --- >> net/netlink/af_netlink.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c >> index 380f95aacdec..24b7cf447bc5 100644 >> --- a/net/netlink/af_netlink.c >> +++ b/net/netlink/af_netlink.c >> @@ -2545,13 +2545,15 @@ int nlmsg_notify(struct sock *sk, struct sk_buff *skb, u32 portid, >> /* errors reported via destination sk->sk_err, but propagate >> * delivery errors if NETLINK_BROADCAST_ERROR flag is set */ >> err = nlmsg_multicast(sk, skb, exclude_portid, group, flags); >> + if (err == -ESRCH) >> + err = 0; >> } >>> if (report) { >> int err2; >>> err2 = nlmsg_unicast(sk, skb, portid); >> - if (!err || err == -ESRCH) >> + if (!err) >> err = err2; >> } >>>
On Sun, Jul 18, 2021 at 10:19 PM Yajun Deng <yajun.deng@linux.dev> wrote: > The failure seems due to the commit > cfdf0d9ae75b ("rtnetlink: use nlmsg_notify() in rtnetlink_send()") > > Deal with ESRCH error in nlmsg_notify() even the report variable is zero. Looks like the tc-testing failure I saw is also due to this... Why not just revert the above commit which does not have much value? It at most saves some instructions. Thanks.
Hello: This patch was applied to netdev/net-next.git (refs/heads/master): On Mon, 19 Jul 2021 13:18:16 +0800 you wrote: > Yonghong Song report: > The bpf selftest tc_bpf failed with latest bpf-next. > The following is the command to run and the result: > $ ./test_progs -n 132 > [ 40.947571] bpf_testmod: loading out-of-tree module taints kernel. > test_tc_bpf:PASS:test_tc_bpf__open_and_load 0 nsec > test_tc_bpf:PASS:bpf_tc_hook_create(BPF_TC_INGRESS) 0 nsec > test_tc_bpf:PASS:bpf_tc_hook_create invalid hook.attach_point 0 nsec > test_tc_bpf_basic:PASS:bpf_obj_get_info_by_fd 0 nsec > test_tc_bpf_basic:PASS:bpf_tc_attach 0 nsec > test_tc_bpf_basic:PASS:handle set 0 nsec > test_tc_bpf_basic:PASS:priority set 0 nsec > test_tc_bpf_basic:PASS:prog_id set 0 nsec > test_tc_bpf_basic:PASS:bpf_tc_attach replace mode 0 nsec > test_tc_bpf_basic:PASS:bpf_tc_query 0 nsec > test_tc_bpf_basic:PASS:handle set 0 nsec > test_tc_bpf_basic:PASS:priority set 0 nsec > test_tc_bpf_basic:PASS:prog_id set 0 nsec > libbpf: Kernel error message: Failed to send filter delete notification > test_tc_bpf_basic:FAIL:bpf_tc_detach unexpected error: -3 (errno 3) > test_tc_bpf:FAIL:test_tc_internal ingress unexpected error: -3 (errno 3) > > [...] Here is the summary with links: - netlink: Deal with ESRCH error in nlmsg_notify() https://git.kernel.org/netdev/net-next/c/fef773fc8110 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 380f95aacdec..24b7cf447bc5 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -2545,13 +2545,15 @@ int nlmsg_notify(struct sock *sk, struct sk_buff *skb, u32 portid, /* errors reported via destination sk->sk_err, but propagate * delivery errors if NETLINK_BROADCAST_ERROR flag is set */ err = nlmsg_multicast(sk, skb, exclude_portid, group, flags); + if (err == -ESRCH) + err = 0; } if (report) { int err2; err2 = nlmsg_unicast(sk, skb, portid); - if (!err || err == -ESRCH) + if (!err) err = err2; }
Yonghong Song report: The bpf selftest tc_bpf failed with latest bpf-next. The following is the command to run and the result: $ ./test_progs -n 132 [ 40.947571] bpf_testmod: loading out-of-tree module taints kernel. test_tc_bpf:PASS:test_tc_bpf__open_and_load 0 nsec test_tc_bpf:PASS:bpf_tc_hook_create(BPF_TC_INGRESS) 0 nsec test_tc_bpf:PASS:bpf_tc_hook_create invalid hook.attach_point 0 nsec test_tc_bpf_basic:PASS:bpf_obj_get_info_by_fd 0 nsec test_tc_bpf_basic:PASS:bpf_tc_attach 0 nsec test_tc_bpf_basic:PASS:handle set 0 nsec test_tc_bpf_basic:PASS:priority set 0 nsec test_tc_bpf_basic:PASS:prog_id set 0 nsec test_tc_bpf_basic:PASS:bpf_tc_attach replace mode 0 nsec test_tc_bpf_basic:PASS:bpf_tc_query 0 nsec test_tc_bpf_basic:PASS:handle set 0 nsec test_tc_bpf_basic:PASS:priority set 0 nsec test_tc_bpf_basic:PASS:prog_id set 0 nsec libbpf: Kernel error message: Failed to send filter delete notification test_tc_bpf_basic:FAIL:bpf_tc_detach unexpected error: -3 (errno 3) test_tc_bpf:FAIL:test_tc_internal ingress unexpected error: -3 (errno 3) The failure seems due to the commit cfdf0d9ae75b ("rtnetlink: use nlmsg_notify() in rtnetlink_send()") Deal with ESRCH error in nlmsg_notify() even the report variable is zero. Reported-by: Yonghong Song <yhs@fb.com> Signed-off-by: Yajun Deng <yajun.deng@linux.dev> --- net/netlink/af_netlink.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)