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]
Message-ID: <1A3363FD-16A1-4A4B-AB30-DD56AFA5FFB0@oracle.com>
Date:   Thu, 9 Feb 2023 16:34:58 +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 9, 2023, at 11:02 AM, Paolo Abeni <pabeni@...hat.com> wrote:
> 
> On Thu, 2023-02-09 at 15:43 +0000, Chuck Lever III wrote:
>>> On Feb 9, 2023, at 1:00 AM, Jakub Kicinski <kuba@...nel.org> wrote:
>>> 
>>> On Tue, 07 Feb 2023 16:41:13 -0500 Chuck Lever wrote:
>>>> diff --git a/tools/include/uapi/linux/netlink.h
>>>> b/tools/include/uapi/linux/netlink.h
>>>> index 0a4d73317759..a269d356f358 100644
>>>> --- a/tools/include/uapi/linux/netlink.h
>>>> +++ b/tools/include/uapi/linux/netlink.h
>>>> @@ -29,6 +29,7 @@
>>>> #define NETLINK_RDMA		20
>>>> #define NETLINK_CRYPTO		21	/* Crypto layer */
>>>> #define NETLINK_SMC		22	/* SMC monitoring */
>>>> +#define NETLINK_HANDSHAKE	23	/* transport layer sec
>>>> handshake requests */
>>> 
>>> The extra indirection of genetlink introduces some complications?
>> 
>> I don't think it does, necessarily. But neither does it seem
>> to add any value (for this use case). <shrug>
> 
> To me it introduces a good separation between the handshake mechanism
> itself and the current subject (sock).
> 
> IIRC the previous version allowed the user-space to create a socket of
> the HANDSHAKE family which in turn accept()ed tcp sockets. That kind of
> construct - assuming I interpreted it correctly - did not sound right
> to me.
> 
> Back to these patches, they looks sane to me, even if the whole
> architecture is a bit hard to follow, given the non trivial cross
> references between the patches - I can likely have missed some relevant
> point.

One of the original goals was to support other security protocols
besides TLS v1.3, which is why the code is split between two
patches. I know that is cumbersome for some review workflows.

Now is a good time to simplify, if we see a sensible opportunity
to do so.


> 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?

All that said, the single pending list can be replaced easily. It
would be straightforward to move it into struct net, for example.


--
Chuck Lever



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ