> -Original Message-
> From: Dave Watson
> Sent: Saturday, August 18, 2018 3:43 AM
> To: Vakul Garg
> Cc: netdev@vger.kernel.org; bor...@mellanox.com;
> avia...@mellanox.com; da...@davemloft.net
> Subject: Re: [PATCH net-next v1] net/tls: Add support for async decryption of
> tls records
>
> On 08/16/18 08:49 PM, Vakul Garg wrote:
> > Changes since RFC version:
> > 1) Improved commit message.
> > 2) Fixed dequeued record offset handling because of which few of
> >tls selftests 'recv_partial, recv_peek, recv_peek_multiple' were
> failing.
>
> Thanks! Commit message much more clear, tests work great for me also,
> only minor comments on clarity
>
> > - if (tls_sw_advance_skb(sk, skb, chunk)) {
> > + if (async) {
> > + /* Finished with current record, pick up next
> */
> > + ctx->recv_pkt = NULL;
> > + __strp_unpause(>strp);
> > + goto mark_eor_chk_ctrl;
>
> Control flow is a little hard to follow here, maybe just pass an async flag to
> tls_sw_advance_skb? It already does strp_unpause and recv_pkt = NULL.
>
I improved it but in a slightly different way. Please see in v2.
As net-next is closed right now, I would send the patch to you privately &
later post it on list when David gives a green signal.
Is it ok?
> > + } else if (tls_sw_advance_skb(sk, skb, chunk)) {
> > /* Return full control message to
> > * userspace before trying to parse
> > * another message type
> > */
> > +mark_eor_chk_ctrl:
> > msg->msg_flags |= MSG_EOR;
> > if (control != TLS_RECORD_TYPE_DATA)
> > goto recv_end;
> > + } else {
> > + break;
>
> I don't see the need for the else { break; }, isn't this already covered by
> while(len); below as before?
When tls_sw_advance_skb() returns false, it is certain that we cannot
continue in the loop. So putting a break here avoids having to execute
'if' checks and while (len) checks down below.
