> -----Original Message-----
> From: Doron Roberts-Kedes [mailto:doro...@fb.com]
> Sent: Tuesday, August 7, 2018 12:02 AM
> To: Vakul Garg <vakul.g...@nxp.com>
> Cc: David S . Miller <da...@davemloft.net>; Dave Watson
> <davejwat...@fb.com>; Boris Pismenny <bor...@mellanox.com>; Aviad
> Yehezkel <avia...@mellanox.com>; netdev@vger.kernel.org
> Subject: Re: [PATCH net-next] net/tls: Calculate nsg for zerocopy path
> without skb_cow_data.
>
> On Fri, Aug 03, 2018 at 11:49:02AM -0700, Doron Roberts-Kedes wrote:
> > On Fri, Aug 03, 2018 at 01:23:33AM +0000, Vakul Garg wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Doron Roberts-Kedes [mailto:doro...@fb.com]
> > > > Sent: Friday, August 3, 2018 6:00 AM
> > > > To: David S . Miller <da...@davemloft.net>
> > > > Cc: Dave Watson <davejwat...@fb.com>; Vakul Garg
> > > > <vakul.g...@nxp.com>; Boris Pismenny <bor...@mellanox.com>;
> Aviad
> > > > Yehezkel <avia...@mellanox.com>; netdev@vger.kernel.org; Doron
> > > > Roberts-Kedes <doro...@fb.com>
> > > > Subject: [PATCH net-next] net/tls: Calculate nsg for zerocopy path
> > > > without skb_cow_data.
> > > >
> > > > decrypt_skb fails if the number of sg elements required to map is
> > > > greater than MAX_SKB_FRAGS. As noted by Vakul Garg, nsg must
> > > > always be calculated, but skb_cow_data adds unnecessary memcpy's
> > > > for the zerocopy case.
> > > >
> > > > The new function skb_nsg calculates the number of scatterlist
> > > > elements required to map the skb without the extra overhead of
> > > > skb_cow_data. This function mimics the structure of skb_to_sgvec.
> > > >
> > > > Fixes: c46234ebb4d1 ("tls: RX path for ktls")
> > > > Signed-off-by: Doron Roberts-Kedes <doro...@fb.com>
> > > > ---
> > > > net/tls/tls_sw.c | 89
> > > > ++++++++++++++++++++++++++++++++++++++++++++++--
> > > > 1 file changed, 86 insertions(+), 3 deletions(-)
> > > >
> > > > memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
> > > > if (!sgout) {
> > > > - nsg = skb_cow_data(skb, 0, &unused) + 1;
> > > > + nsg = skb_cow_data(skb, 0, &unused);
> > > > + } else {
> > > > + nsg = skb_nsg(skb,
> > > > + rxm->offset + tls_ctx->rx.prepend_size,
> > > > + rxm->full_len - tls_ctx->rx.prepend_size);
> > > > + if (nsg <= 0)
> > > > + return nsg;
> > > Comparison should be (nsg < 1). TLS forbids '0' sized records.
> >
> > Yes true, v2 incoming
> >
>
> Glancing at this a second time, I actually don't believe this should be
> changed. nsg <= 0 is equivalent to nsg < 1.
Correct.
> Returning 0 if the record is
> 0 sized is the proper behavior here, since decrypting a zero-length skb is a
> no-
> op. It is true that zero sized TLS records are forbidden, but it is confusing
> to
> enforce that in this part of the code. I would be surpised to learn that
> tls_sw_recvmsg could be invoked with a len equal to 0, but if I'm wrong, and
> that case does need to be handled, then it should be in a different patch.
