Thread (35 messages) 35 messages, 5 authors, 2021-04-06

RE: [Patch bpf-next v8 11/16] udp: implement ->read_sock() for sockmap

From: John Fastabend <john.fastabend@gmail.com>
Date: 2021-04-01 06:01:44
Also in: netdev

Cong Wang wrote:
From: Cong Wang <redacted>

This is similar to tcp_read_sock(), except we do not need
to worry about connections, we just need to retrieve skb
from UDP receive queue.

Note, the return value of ->read_sock() is unused in
sk_psock_verdict_data_ready(), and UDP still does not
support splice() due to lack of ->splice_read(), so users
can not reach udp_read_sock() directly.

Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jakub Sitnicki <jakub@cloudflare.com>
Cc: Lorenz Bauer <redacted>
Signed-off-by: Cong Wang <redacted>
---
Thanks this is easier to read IMO. One nit below.

Acked-by: John Fastabend <john.fastabend@gmail.com>

[...]
 
+int udp_read_sock(struct sock *sk, read_descriptor_t *desc,
+		  sk_read_actor_t recv_actor)
+{
+	int copied = 0;
+
+	while (1) {
+		struct sk_buff *skb;
+		int err, used;
+
+		skb = skb_recv_udp(sk, 0, 1, &err);
+		if (!skb)
+			return err;
+		used = recv_actor(desc, skb, 0, skb->len);
+		if (used <= 0) {
+			if (!copied)
+				copied = used;
+			break;
+		} else if (used <= skb->len) {
+			copied += used;
+		}
This 'else if' is always true if above is false right? Would be
impler and clearer IMO as,

               if (used <= 0) {
		        if (!copied)
				copied = used;
			break;
               }
               copied += used;

I don't see anyway for used to be great than  skb->len.
+
+		if (!desc->count)
+			break;
+	}
+
+	return copied;
+}
+EXPORT_SYMBOL(udp_read_sock);
+
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help