From: Peilin Ye <[email protected]>
Delete the unnecessary while loop in udp_read_skb() for readability.
Additionally, since recv_actor() cannot return a value greater than
skb->len (see sk_psock_verdict_recv()), remove the redundant check.
Suggested-by: Cong Wang <[email protected]>
Signed-off-by: Peilin Ye <[email protected]>
---
Depends on:
"[PATCH net v2] net: Use WARN_ON_ONCE() in {tcp,udp}_read_skb()"
https://lore.kernel.org/all/[email protected]/
Thanks,
Peilin Ye
net/ipv4/udp.c | 46 +++++++++++++++++-----------------------------
1 file changed, 17 insertions(+), 29 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 560d9eadeaa5..d63118ce5900 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1801,41 +1801,29 @@ EXPORT_SYMBOL(__skb_recv_udp);
int udp_read_skb(struct sock *sk, skb_read_actor_t recv_actor)
{
- int copied = 0;
-
- while (1) {
- struct sk_buff *skb;
- int err, used;
-
- skb = skb_recv_udp(sk, MSG_DONTWAIT, &err);
- if (!skb)
- return err;
+ struct sk_buff *skb;
+ int err, copied;
- if (udp_lib_checksum_complete(skb)) {
- __UDP_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS,
- IS_UDPLITE(sk));
- __UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS,
- IS_UDPLITE(sk));
- atomic_inc(&sk->sk_drops);
- kfree_skb(skb);
- continue;
- }
+try_again:
+ skb = skb_recv_udp(sk, MSG_DONTWAIT, &err);
+ if (!skb)
+ return err;
- WARN_ON_ONCE(!skb_set_owner_sk_safe(skb, sk));
- used = recv_actor(sk, skb);
- if (used <= 0) {
- if (!copied)
- copied = used;
- kfree_skb(skb);
- break;
- } else if (used <= skb->len) {
- copied += used;
- }
+ if (udp_lib_checksum_complete(skb)) {
+ int is_udplite = IS_UDPLITE(sk);
+ struct net *net = sock_net(sk);
+ __UDP_INC_STATS(net, UDP_MIB_CSUMERRORS, is_udplite);
+ __UDP_INC_STATS(net, UDP_MIB_INERRORS, is_udplite);
+ atomic_inc(&sk->sk_drops);
kfree_skb(skb);
- break;
+ goto try_again;
}
+ WARN_ON_ONCE(!skb_set_owner_sk_safe(skb, sk));
+ copied = recv_actor(sk, skb);
+ kfree_skb(skb);
+
return copied;
}
EXPORT_SYMBOL(udp_read_skb);
--
2.20.1
From: Peilin Ye <[email protected]>
Similar to udp_read_skb(), delete the unnecessary while loop in
unix_read_skb() for readability. Since recv_actor() cannot return a
value greater than skb->len (see sk_psock_verdict_recv()), remove the
redundant check.
Suggested-by: Cong Wang <[email protected]>
Signed-off-by: Peilin Ye <[email protected]>
---
net/unix/af_unix.c | 34 ++++++++++------------------------
1 file changed, 10 insertions(+), 24 deletions(-)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index dea2972c8178..c955c7253d4b 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2536,32 +2536,18 @@ static int unix_dgram_recvmsg(struct socket *sock, struct msghdr *msg, size_t si
static int unix_read_skb(struct sock *sk, skb_read_actor_t recv_actor)
{
- int copied = 0;
-
- while (1) {
- struct unix_sock *u = unix_sk(sk);
- struct sk_buff *skb;
- int used, err;
-
- mutex_lock(&u->iolock);
- skb = skb_recv_datagram(sk, MSG_DONTWAIT, &err);
- mutex_unlock(&u->iolock);
- if (!skb)
- return err;
+ struct unix_sock *u = unix_sk(sk);
+ struct sk_buff *skb;
+ int err, copied;
- used = recv_actor(sk, skb);
- if (used <= 0) {
- if (!copied)
- copied = used;
- kfree_skb(skb);
- break;
- } else if (used <= skb->len) {
- copied += used;
- }
+ mutex_lock(&u->iolock);
+ skb = skb_recv_datagram(sk, MSG_DONTWAIT, &err);
+ mutex_unlock(&u->iolock);
+ if (!skb)
+ return err;
- kfree_skb(skb);
- break;
- }
+ copied = recv_actor(sk, skb);
+ kfree_skb(skb);
return copied;
}
--
2.20.1
On Wed, 14 Sep 2022 01:15:30 -0700 Peilin Ye wrote:
> Delete the unnecessary while loop in udp_read_skb() for readability.
> Additionally, since recv_actor() cannot return a value greater than
> skb->len (see sk_psock_verdict_recv()), remove the redundant check.
These don't apply cleanly, please rebase?
On Tue, 20 Sep 2022 17:38:59 -0700 Jakub Kicinski wrote:
> On Wed, 14 Sep 2022 01:15:30 -0700 Peilin Ye wrote:
> > Delete the unnecessary while loop in udp_read_skb() for readability.
> > Additionally, since recv_actor() cannot return a value greater than
> > skb->len (see sk_psock_verdict_recv()), remove the redundant check.
>
> These don't apply cleanly, please rebase?
Ah, it's the WARN_ON_ONCE() change. In that case please resend after
net is merged into net-next (Thu evening).
On Tue, Sep 20, 2022 at 05:40:52PM -0700, Jakub Kicinski wrote:
> On Tue, 20 Sep 2022 17:38:59 -0700 Jakub Kicinski wrote:
> > On Wed, 14 Sep 2022 01:15:30 -0700 Peilin Ye wrote:
> > > Delete the unnecessary while loop in udp_read_skb() for readability.
> > > Additionally, since recv_actor() cannot return a value greater than
> > > skb->len (see sk_psock_verdict_recv()), remove the redundant check.
> >
> > These don't apply cleanly, please rebase?
>
> Ah, it's the WARN_ON_ONCE() change. In that case please resend after
> net is merged into net-next (Thu evening).
Sure, but only the TCP part was merged [1] into net. I just sent the
UDP part again [2], and will resend this patchset after both [1] and [2]
are merged into net-next. Thanks!
[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=96628951869c0dedf0377adca01c8675172d8639
[2] https://lore.kernel.org/netdev/[email protected]/T/#u
Peilin Ye
From: Peilin Ye <[email protected]>
Delete the unnecessary while loop in udp_read_skb() for readability.
Additionally, since recv_actor() cannot return a value greater than
skb->len (see sk_psock_verdict_recv()), remove the redundant check.
Suggested-by: Cong Wang <[email protected]>
Signed-off-by: Peilin Ye <[email protected]>
---
net/ipv4/udp.c | 46 +++++++++++++++++-----------------------------
1 file changed, 17 insertions(+), 29 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 560d9eadeaa5..d63118ce5900 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1801,41 +1801,29 @@ EXPORT_SYMBOL(__skb_recv_udp);
int udp_read_skb(struct sock *sk, skb_read_actor_t recv_actor)
{
- int copied = 0;
-
- while (1) {
- struct sk_buff *skb;
- int err, used;
-
- skb = skb_recv_udp(sk, MSG_DONTWAIT, &err);
- if (!skb)
- return err;
+ struct sk_buff *skb;
+ int err, copied;
- if (udp_lib_checksum_complete(skb)) {
- __UDP_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS,
- IS_UDPLITE(sk));
- __UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS,
- IS_UDPLITE(sk));
- atomic_inc(&sk->sk_drops);
- kfree_skb(skb);
- continue;
- }
+try_again:
+ skb = skb_recv_udp(sk, MSG_DONTWAIT, &err);
+ if (!skb)
+ return err;
- WARN_ON_ONCE(!skb_set_owner_sk_safe(skb, sk));
- used = recv_actor(sk, skb);
- if (used <= 0) {
- if (!copied)
- copied = used;
- kfree_skb(skb);
- break;
- } else if (used <= skb->len) {
- copied += used;
- }
+ if (udp_lib_checksum_complete(skb)) {
+ int is_udplite = IS_UDPLITE(sk);
+ struct net *net = sock_net(sk);
+ __UDP_INC_STATS(net, UDP_MIB_CSUMERRORS, is_udplite);
+ __UDP_INC_STATS(net, UDP_MIB_INERRORS, is_udplite);
+ atomic_inc(&sk->sk_drops);
kfree_skb(skb);
- break;
+ goto try_again;
}
+ WARN_ON_ONCE(!skb_set_owner_sk_safe(skb, sk));
+ copied = recv_actor(sk, skb);
+ kfree_skb(skb);
+
return copied;
}
EXPORT_SYMBOL(udp_read_skb);
--
2.20.1
Hello:
This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <[email protected]>:
On Thu, 22 Sep 2022 21:59:13 -0700 you wrote:
> From: Peilin Ye <[email protected]>
>
> Delete the unnecessary while loop in udp_read_skb() for readability.
> Additionally, since recv_actor() cannot return a value greater than
> skb->len (see sk_psock_verdict_recv()), remove the redundant check.
>
> Suggested-by: Cong Wang <[email protected]>
> Signed-off-by: Peilin Ye <[email protected]>
>
> [...]
Here is the summary with links:
- [net-next,RESEND,1/2] udp: Refactor udp_read_skb()
https://git.kernel.org/netdev/net-next/c/31f1fbcb346c
- [net-next,RESEND,2/2] af_unix: Refactor unix_read_skb()
https://git.kernel.org/netdev/net-next/c/d6e3b27cbd2d
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html