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); +