Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] netlink: Deal with ESRCH error in nlmsg_notify()
@ 2021-07-19  5:18 Yajun Deng
  2021-07-19 14:47 ` Yonghong Song
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Yajun Deng @ 2021-07-19  5:18 UTC (permalink / raw)
  To: davem, kuba, ast, andrii, kafai, songliubraving, yhs,
	john.fastabend, kpsingh
  Cc: netdev, linux-kernel, Yajun Deng

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(-)

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;
 	}
 
-- 
2.32.0


^ 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
                   ` (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

end of thread, other threads:[~2021-07-20 10:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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

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).