Message-ID: <B990C26A-DCEF-4FFF-B35B-F311A097D02A@oracle.com>
Date:   Fri, 10 Feb 2023 15:38:37 +0000
From:   Chuck Lever III <chuck.lever@...cle.com>
To:     Paolo Abeni <pabeni@...hat.com>
CC:     Jakub Kicinski <kuba@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        "open list:NETWORKING [GENERAL]" <netdev@...r.kernel.org>,
        "hare@...e.com" <hare@...e.com>,
        David Howells <dhowells@...hat.com>,
        Benjamin Coddington <bcodding@...hat.com>,
        Olga Kornievskaia <kolga@...app.com>,
        "jmeneghi@...hat.com" <jmeneghi@...hat.com>
Subject: Re: [PATCH v3 1/2] net/handshake: Create a NETLINK service for
 handling handshake requests



> On Feb 10, 2023, at 10:21 AM, Paolo Abeni <pabeni@...hat.com> wrote:
> 
> On Fri, 2023-02-10 at 14:31 +0000, Chuck Lever III wrote:
>> In previous generations of this series, there was an addition
>> to Documentation/ that explained how kernel TLS consumers use
>> the tls_ handshake API. I can add that back now that things
>> are settling down.
> 
> That would be useful, thank!
> 
>> But maybe you are thinking of some other topics. I'm happy to
>> write down whatever is needed, but I'd like suggestions about
>> what particular areas would be most helpful.
> 
> A reference user-space implementation would be very interesting, too.

We've got one of those, specifically for TLSv1.3:

   https://github.com/oracle/ktls-utils

netlink support is added on the "netlink" branch. The user space
handshake agent for TLS is under src/tlshd. The netlink stuff is
pretty fresh, so there's clean-up to be done.


> Even a completely "dummy" one for self-tests purpose only could be
> useful. 
> 
> Speaking of that, at some point we will need some self-tests ;)

Jakub mentioned that during the first round of review last year.
I've got some Kunit chops, so I can construct tests. But I'm
coming up empty on exactly what would need to be tested. Right,
maybe Kunit is the wrong tool for this job...


>>>>> I'm wondering if this approach scales well enough with the number of
>>>>> concurrent handshakes: the single list looks like a potential bottle-
>>>>> neck.
>>>> 
>>>> It's not clear how much scaling is needed. I don't have a strong
>>>> sense of how frequently a busy storage server will need a handshake,
>>>> for instance, but it seems like it would be relatively less frequent
>>>> than, say, I/O. Network storage connections are typically long-lived,
>>>> unlike http.
>>>> 
>>>> In terms of scalability, I am a little more concerned about the
>>>> handshake_mutex. Maybe that isn't needed since the pending list is
>>>> spinlock protected?
>>> 
>>> Good point. Indeed it looks like that is not needed.
>> 
>> I will remove the handshake_mutex in v4.
>> 
>> 
>>>> All that said, the single pending list can be replaced easily. It
>>>> would be straightforward to move it into struct net, for example.
>>> 
>>> In the end I don't see a operations needing a full list traversal.
>>> handshake_nl_msg_accept walk that, but it stops at netns/proto matching
>>> which should be ~always /~very soon in the typical use-case. And as you
>>> said it should be easy to avoid even that.
>>> 
>>> I think it could be useful limiting the number of pending handshake to
>>> some maximum, to avoid problems in pathological/malicious scenarios.
>> 
>> Defending against DoS is sensible. Maybe having a per-net
>> maximum of 5 or 10 pending handshakes? handshake_request() can
>> return an error code if a handshake is requested while we're
>> over that maximum.
> 
> I'm wondering if we could use an {r,w}mem based limits, so that the
> user-space could eventually tune it as/if needed without any additional
> knob.


--
Chuck Lever

Powered by blists - more mailing lists