Cong Wang wrote:
> From: Cong Wang <cong.wang@xxxxxxxxxxxxx>
>
> 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@xxxxxxxxx>
> Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
> Cc: Jakub Sitnicki <jakub@xxxxxxxxxxxxxx>
> Cc: Lorenz Bauer <lmb@xxxxxxxxxxxxxx>
> Signed-off-by: Cong Wang <cong.wang@xxxxxxxxxxxxx>
> ---
Thanks this is easier to read IMO. One nit below.
Acked-by: John Fastabend <john.fastabend@xxxxxxxxx>
[...]
>
> +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);
> +
