2024-02-19, 12:07:03 -0800, Jakub Kicinski wrote:
On Thu, 15 Feb 2024 17:17:31 +0100 Sabrina Dubroca wrote:
@@ -1772,7 +1772,8 @@ static int process_rx_list(struct tls_sw_context_rx *ctx, u8 *control, size_t skip, size_t len,
bool is_peek)
bool is_peek,bool *more){ struct sk_buff *skb = skb_peek(&ctx->rx_list); struct tls_msg *tlm;
@@ -1844,6 +1845,10 @@ static int process_rx_list(struct tls_sw_context_rx *ctx, out: return copied ? : err; +more:
- if (more)
*more = true;- goto out;
Patches look correct, one small nit here -
I don't have great ideas how to avoid the 7th argument completely but
I hesitated between this patch and a variant combining is_peek and more into a single u8 *flags, but that felt a bit messy (or does that fall into what you describe as "not [having] great ideas"? :))
@@ -1772,9 +1777,10 @@ static int process_rx_list(struct tls_sw_context_rx *ctx, u8 *control, size_t skip, size_t len, - bool is_peek) + u8 *flags) { struct sk_buff *skb = skb_peek(&ctx->rx_list); + bool is_peek = *flags & RXLIST_PEEK; struct tls_msg *tlm; ssize_t copied = 0; int err; [...] @@ -1844,6 +1850,9 @@ static int process_rx_list(struct tls_sw_context_rx *ctx,
out: return copied ? : err; +more: + *flags |= RXLIST_MORE; + goto out; }
and then in tls_sw_recvmsg: u8 rxlist_flags = is_peek ? RXLIST_PEEK : 0; err = process_rx_list(ctx, msg, &control, 0, len, &rxlist_flags);
I think it'd be a little cleaner if we either:
- passed in err as an output argument (some datagram code does that IIRC), then function can always return copied directly, or
(yes, __skb_wait_for_more_packets, __skb_try_recv_datagram, and their variants)
- passed copied as an output argument, and then we can always return err?
Aren't those 2 options adding an 8th argument?
I tend to find ">= 0 on success, otherwise errno" more readable, probably because that's a very common pattern (either for recvmsg style of cases, or all the ERR_PTR type situations).
I like the former a little better because we won't have to special case NULL for the "after async decryption" call sites.
We could also pass &rx_more every time and not check for NULL.
What do you want to clean up more specifically? The number of arguments, the backwards goto, the NULL check before setting *more, something else/all of the above?