* Re: [PATCH] netlink: Deal with ESRCH error in nlmsg_notify()
2021-07-19 5:18 [PATCH] netlink: Deal with ESRCH error in nlmsg_notify() Yajun Deng
@ 2021-07-19 14:47 ` Yonghong Song
2021-07-20 2:47 ` yajun.deng
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Yonghong Song @ 2021-07-19 14:47 UTC (permalink / raw)
To: Yajun Deng, davem, kuba, ast, andrii, kafai, songliubraving,
john.fastabend, kpsingh
Cc: netdev, linux-kernel
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;
> }
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] netlink: Deal with ESRCH error in nlmsg_notify()
2021-07-19 5:18 [PATCH] netlink: Deal with ESRCH error in nlmsg_notify() Yajun Deng
2021-07-19 14:47 ` Yonghong Song
@ 2021-07-20 2:47 ` yajun.deng
2021-07-20 4:29 ` Cong Wang
2021-07-20 10:00 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 5+ messages in thread
From: yajun.deng @ 2021-07-20 2:47 UTC (permalink / raw)
To: Yonghong Song, davem, kuba, ast, andrii, kafai, songliubraving,
john.fastabend, kpsingh
Cc: netdev, linux-kernel
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;
>> }
>>>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] netlink: Deal with ESRCH error in nlmsg_notify()
2021-07-19 5:18 [PATCH] netlink: Deal with ESRCH error in nlmsg_notify() Yajun Deng
2021-07-19 14:47 ` Yonghong Song
2021-07-20 2:47 ` yajun.deng
@ 2021-07-20 4:29 ` Cong Wang
2021-07-20 10:00 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 5+ messages in thread
From: Cong Wang @ 2021-07-20 4:29 UTC (permalink / raw)
To: Yajun Deng
Cc: David Miller, Jakub Kicinski, Alexei Starovoitov,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Linux Kernel Network Developers, LKML
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.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] netlink: Deal with ESRCH error in nlmsg_notify()
2021-07-19 5:18 [PATCH] netlink: Deal with ESRCH error in nlmsg_notify() Yajun Deng
` (2 preceding siblings ...)
2021-07-20 4:29 ` Cong Wang
@ 2021-07-20 10:00 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-07-20 10:00 UTC (permalink / raw)
To: Yajun Deng
Cc: davem, kuba, ast, andrii, kafai, songliubraving, yhs,
john.fastabend, kpsingh, netdev, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread