On Tue, 19 May 2020 13:21:56 -0400 Pooja Trivedi wrote:
> On Mon, May 18, 2020 at 6:50 PM Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
> > On Sun, 17 May 2020 16:26:36 +0000 Pooja Trivedi wrote:
> > > In pure sw ktls(AES-NI), -EAGAIN from tcp layer (do_tcp_sendpages for
> > > encrypted record) gets treated as error, subtracts the offset, and
> > > returns to application. Because of this, application sends data from
> > > subtracted offset, which leads to data integrity issue. Since record is
> > > already encrypted, ktls module marks it as partially sent and pushes the
> > > packet to tcp layer in the following iterations (either from bottom half
> > > or when pushing next chunk). So returning success in case of EAGAIN
> > > will fix the issue.
> > >
> > > Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption")
> > > Signed-off-by: Pooja Trivedi <pooja.trivedi@xxxxxxxxxxxxx>
> > > Reviewed-by: Mallesham Jatharkonda <mallesham.jatharkonda@xxxxxxxxxxxxxxxxxx>
> > > Reviewed-by: Josh Tway <josh.tway@xxxxxxxxxxxxx>
> >
> > This looks reasonable, I think. Next time user space calls if no new
> > buffer space was made available it will get a -EAGAIN, right?
>
> Yes, this fix should only affect encrypted record. Plain text calls from
> user space should be unaffected.
AFAICS if TCP layer is full next call from user space should hit
sk_stream_wait_memory() immediately and if it has MSG_DONTWAIT set
exit with EAGAIN. Which I believe to be correct behavior.
> > Two questions - is there any particular application or use case that
> > runs into this?
>
> We are running into this case when we hit our kTLS-enabled homegrown
> webserver with a 'pipeline' test tool, also homegrown. The issue basically
> happens whenever the send buffer on the server gets full and TCP layer
> returns EAGAIN when attempting to TX the encrypted record. In fact, we
> are also able to reproduce the issue by using a simple wget with a large
> file, if/when sndbuf fills up.
I see just a coincidence, then, no worries.
> > Seems a bit surprising to see a patch from Vadim and
> > you guys come at the same time.
>
> Not familiar with Vadim or her/his patch. Could you please point me to it?
http://patchwork.ozlabs.org/project/netdev/patch/20200517014451.954F05026DE@xxxxxxxx/
> > Could you also add test for this bug?
> > In tools/testing/selftests/net/tls.c
> >
>
> Sure, yes. Let me look into this.
Thanks!
