netdev - Re: combining sockmap + ktls

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 15 Nov 2019 18:12:42 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     John Fastabend <john.fastabend@...il.com>
Cc:     Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        bpf <bpf@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Daniel Borkmann <daniel@...earbox.net>
Subject: Re: combining sockmap + ktls

> > For this workload, more interesting is sk_msg directly to icept
> > egress, anyway. This works without ktls. Support for ktls is added in
> > commit d3b18ad31f93 ("tls: add bpf support to sk_msg handling"). The
> > relevant callback function tls_sw_sendpage_locked was not immediately
> > used and subsequently removed in commit cc1dbdfed023 ("Revert
> > "net/tls: remove unused function tls_sw_sendpage_locked""). It appears
> > to work once reverting that change, plus registering the function
>
> I don't fully understand this. Are you saying a BPF_SK_MSG_VERDICT
> program attach to a ktls socket is not being called? Or packets are
> being dropped or ...? Or that the program doesn't work even with
> just KTLS and no bpf involved.

Not the verdict program. The setup is as follows:

  client --> icept.1 -- (optionally ktls) --> icept.2 --> server

I'm trying to redirect on send from the client directly into the send
side of the ktls socket, to avoid waking up the icept.1 process
completely. The ktls enabled socket has no BPF programs associated.

>
> >
> >         @@ -859,6 +861,7 @@ static int __init tls_register(void)
> >
> >                 tls_sw_proto_ops = inet_stream_ops;
> >                 tls_sw_proto_ops.splice_read = tls_sw_splice_read;
> >         +       tls_sw_proto_ops.sendpage_locked   = tls_sw_sendpage_locked,
> >
> > and additionally allowing MSG_NO_SHARED_FRAGS:
> >
> >          int tls_sw_sendpage_locked(struct sock *sk, struct page *page,
> >                                     int offset, size_t size, int flags)
> >          {
> >                if (flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL |
> >         -                     MSG_SENDPAGE_NOTLAST | MSG_SENDPAGE_NOPOLICY))
> >         +                     MSG_SENDPAGE_NOTLAST |
> > MSG_SENDPAGE_NOPOLICY | MSG_NO_SHARED_FRAGS))
> >                          return -ENOTSUPP;
> >
>
> If you had added MSG_NO_SHARED_FRAGS to the existing tls_sw_sendpage
> would that have been sufficient?

No, the stack trace I observed is

  tcp_bpf_sendmsg_redir
    tcp_bpf_push_locked
      tcp_bpf_push
        kernel_sendpage_locked
          sock->ops->sendpage_locked

which never tries tls_sw_sendpage. Perhaps the relevant part is the
following in tcp_bpf_push?

                if (has_tx_ulp) {
                        flags |= MSG_SENDPAGE_NOPOLICY;
                        ret = kernel_sendpage_locked(sk,
                                                     page, off, size, flags);
                } else {
                        ret = do_tcp_sendpages(sk, page, off, size, flags);
                }

> > and not registering parser+verdict programs on the destination socket.
> > Note that without ktls this mode also works with such programs
> > attached.
>
> Right ingress + ktls is known to be broken at the moment. Also I have
> plans to cleanup ingress side at some point. The current model is a
> bit clumsy IMO. The workqueue adds latency spikes on the 99+
> percentiles. At this point it makes the ingress side similar to the
> egress side without a workqueue and with verdict+parser done in a
> single program.

Good to know thanks. Then I won't spend too much time on this.

> >
> > Lastly, sockmap splicing from icept ingress to egress (no sk_msg) also
> > stops working when I enable ktls on the egress socket. I'm taking a
> > look at that next. But this email is long enough already ;)
>
> Yes this is a known bug I've got a set of patches to address this.

Same thing. Good to know I'm not crazy :)

> I've
> been trying to get to it for awhile now and just resolved a few other
> things on my side so plan to do this Monday/Tuesday next week.

To be clear, I don't mean to pressure at all. Was just running through
a variety of points in the option space. The sk_msg  plus redirect to
egress of a ktls-enabled socket is the variant I'm most interested in.

Do let me know if there's anything I can help out with. Thanks for
your quick answer!