[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [Nbd] [PATCHv5 0/7] Introduce TLS support on nbdserver & nbdclient



On Wed, May 04, 2016 at 10:11:48AM +0100, Alex Bligh wrote:
> Wouter,
> 
> > I'd rather see that the server doesn't spawn a child to do TLS at all,
> > as I've indicated before. My long-term plan is to go to a situation
> > where the server, at most, spawns a thread for each client (if even
> > that). Currently we have a process per client, which take resources and
> > is a suboptimal situation.
> 
> Sure, I think we'd all rather that.
> 
> >   It also means the maxthreads option is to be
> > multiplied per client, rather than be a single configuration for the
> > whole server.
> 
> Actually I would have thought 'per client' would be more useful.
> 
> > I'm not saying that fixing that up is a prerequisite for implementing
> > TLS,
> 
> That's where we got to last time
> 
> > but "not making the situation worse" is.
> 
> Don't quite understand that.

Obviously :-)

> It doesn't make the situation worse as it is, for non-TLS clients;
> they work exactly as currently. It makes the situation infinitely better
> for TLS clients!

What I mean is, I think the current way of fork-per-child is bad design,
and I want to (eventually) work towards its elimination. You don't do
that by adding in more places where you do fork().

[...]
> The reason why I fixed it 'properly' was precisely so if you threaded
> the whole thing (including the TLS client), we'd not introduce another
> problem (I'm assuming the threads would be stored in the hash structure).

The only reason we do the hash table in the first place is so we can
proxy signals (e.g., SIGTERM or SIGINT) that the parent received to its
clients.

As a side effect, we can use it to count currently-active clients for
the maxconnections option, but that's not the reason for us to do that
(and not the most efficient way to do that, either). If we ever manage
to move away from the fork-per-client approach, we won't need the
hashtable anymore (and can move to a simple refcount for the
maxconnections stuff).

> We could thread proxy rather than run it as a child - that would be
> easy enough I guess?

Not a bad idea, tbh. You still have two sets of user/kernel copies for
every read and write (which isn't ideal), but at least you no longer
need separate processes.

> Of course the right solution would be to rewrite every network
> read / write to go via the proxy layer, but that is a big undertaking.

Sure. If it's in a separate thread (rather than a separate process), we
can make this change gradually.

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12



Reply to: