Multi-Interface Patch

[Brian Aker]

Hi!

On Feb 3, 2008, at 6:01 AM, Tomash Brechko wrote:

> The original interface was designed to return one fd.  While you may
> extend it to return an array of those, the code might become cleaner
> if you redesign the whole thing instead.  It seems that the array of
> fds isn't really needed: fds are created, then passed to libevent.

I took this route first, but it meant a lot more changed code then  
what  did. I also did not want to just remove the pre_gdb() calls  
without more understanding as to why they are done.

> OTOH, as the person doing the whole work, you are free to choose any
> way ;).

I am a refactorer. This was my second shot at doing this. I tend to  
aim for
1) Get stuff in small chunks so it can be understood and committed.
2) Work out a plan for long term changes.

> the array at all, or set it to -1, as zero is a valid file descriptor.

Good point, I'll modify this.

> Unfortunately the rest is also incorrect.  Why do you multiply by
> MIN_NUMBER_OF_SOCKETS?  What's the meaning of this constant (== 2)?

That should have been add, not multiply. We need one more then the  
number of addresses so that the array count does not need to be kept.  
I added on more to this number so that in the next patch I can make it  
so that the unix socket code will be streamlined into the same loop  
(which also means you can have unix socket support with network at the  
same time).

> is where you would use the single socket array.  However, wouldn't it
> be simpler not to do any cleanup?  memcached would exit soon anyway.

The other option is to put a jump into an error block of code.

> Drepper explains that at least EADDRINUSE should be ignored.  But I
> think you may ignore _any_ bind error, just skip over such sockets.
> If even with AI_ADDRCONFIG the system returns the address that you
> can't bind to, there may be some reasons for that (or it may be a
> system bug).  Continuing would also solve that issue on OS X 10.5, as
> eventually you'd come to IPv4 address down the list for UDP.

I'll look into this. I'm not that comfortable with ignoring bind()  
errors out of hand.

> u_socket hols the array returned by server_socket().  Thus
> semantically it's better to write not *u_socket, but u_socket[0].
> Because this will quickly bring us to the question: why only the  
> first?

Yep, but UDP has other issues as well. I'm going to tackle it as a  
different problem. I'd like to see it enabled by default, but before  
we do that we need to  fix it so that a simple port scan does not  
crash memcached.

>> +/* Minimum number of sockets to allocate for listeners */
>> +#define MIN_NUMBER_OF_SOCKETS 2
>
> The comment is wrong.  Instead, it's some kind of coefficient you
> multiply the number of sockets by.  I can't figure the purpose of it.


The comment was right, the multiplier was wrong :)

Now that I am reflecting on it, I think the answer might be to just  
merge the unix socket function into the TCP one.

Cheers,
	-Brian
--
_______________________________________________________
Brian "Krow" Aker, brian at tangent.org
Seattle, Washington
http://krow.net/                     <-- Me
http://tangent.org/                <-- Software
http://exploitseattle.com/    <-- Fun
_______________________________________________________
You can't grep a dead tree.