[bind10-dev] Socket creator and low-level code

[Shane Kerr]

Michal & Jinmei, 

No dictate, I think we can discuss this a little bit more still. Sorry!

On Thu, 2010-10-21 at 18:28 +0200, Michal 'vorner' Vaner wrote:
> 
> Thanks for the note about MSG_WAITALL, I didn't know that one. It seems it
> wouldn't work on signal, but the process shouldn't get signals anyway.

Yeah, that's cool. I will be using MSG_WAITALL for all of my socket
reading purposes from now on! :)

> > Having said that, I would actually propose one different type of
> > approach, not actually related to the pointer vs higher-level API
> > discussion.
> > 
> > I wonder whether we can pass addresses and ports as a string, which
> > will be represented in the form of length + C-string.  Then we can use
> > getaddrinfo(3) to support both IPv4 and IPv6 in unified code, without
> > worrying about portability of sa_len (btw your current code has this
> > portability bug).  The main drawback of this approach is exchanging
> > variable-length data may be considered riskier, but, I would accept
> > that risk if we use safer API.
> 
> Well, I read about sa_len and I do not think this is a bug, because I pass the
> value there. If the bind() has different integral type, it would get
> auto-converted on the function call. The problem arises when passing a pointer
> to it to some function returning the address, because it can have a different
> length.
> 
> But that would require a bigger rewrite, actually. I can either change the
> internal parts as I said (current tests test the external behaviour, so it would
> need only small changes) and get it into some defined point. Or there will be
> major change of the protocol, but then all the tests would need to change as
> well and so, so I would leave it to someone else, as this does not really belong
> to R-team.
> 
> So, what do you think? Does it make sense to change the use of the read/write to
> a template functions, eliminate type casts and clean it up of pointer as much as
> possible for me?

When I was thinking about this earlier, I had two basic ideas on how to
approach the problem:

     1. Fixed-sized binary protocol.
     2. Line-based text protocol.

My approach to the binary protocol was something like:

        'T'/'U'          -> TCP or UDP, 1 byte
        '4'/'6'          -> IPv4 or IPv6, 1 byte
        AAAAAAAAAAAAAAAA -> address (0-padded for IPv4), 16 bytes
        PP               -> port, 2 bytes

So we always send exactly 20 bytes. My thinking is that this would
simplify both sending and receiving. We can guarantee that the client &
server stay in sync by always sending full packets.

For the line-based text protocol, it was actually not much different,
but rather in the form:

        tcp 172.29.1.193:53\n
        udp 0.0.0.0:53\n
        udp [2001:610:719:1:221:5dff:fe1e:113a]:53\n
        
I suppose to use getaddrinfo() we could use spaces instead:

        tcp 172.29.1.193 53\n
        udp 0.0.0.0 53\n
        udp 2001:610:719:1:221:5dff:fe1e:113a 53\n        
        
We can re-sync the client & server by scanning until the next newline
and picking things up from there. Thinking a bit more, maybe we can use
multi-line input, to avoid most parsing:

   tcp\n
   172.29.1.193\n
   53\n

In this case we can use istream::getline() and read these in. Still
annoying since we have to check for buffer limits and suchlike, but not
too bad. In this case re-sync would occur by looking for the next 
"tcp\n" or "udp\n" string.


In both cases someone who hacks the client process can allocate
arbitrary ports and cause mischief that way, but I don't think we can
avoid that.


I have no strong preference, but the binary way (including Michal's
binary way) seems fundamentally simpler. OTOH, with a proper design
perhaps not?

--
Shane