[Click] socket.cc patch

[Eddie Kohler]

Hi Mark,

Holy patch bonanza!

- click-daemon.patch: Should Click print its *initial* error messages to 
stderr, then switch to the logfile after forking?  Is SIGHUP documented?  Why 
not use the new signal catching infrastructure provided by Master?  Not yet 
applied.

- click-icmppingrewriter.patch: Awesome!

- click-fromhost.patch: Applied, although I might prefer a configuration 
variable a bit more verbose than HAVE_PROPER.

- click-proper.patch: If you find "-lproper" in the absence of a --with-proper 
option, you assume it should be used.  This seems a bit aggressive to me; is 
-lproper ONLY installed on PlanetLab nodes?  Is it completely harmless 
elsewhere?  (RawSocket's optional inclusion of PROPER would seem to argue 
not.)  Instead I've applied a version where the absence of --with-proper is 
equivalent to --without-proper.  Having gone through all of this, it looks 
like the more consisntent thing would be to compile with proper support if 
it's found, but to add to all relevant elements a PROPER keyword arg that 
defaults to false.  Only FromHost didn't follow this pattern.  What do you think?

It also looks like you test for curl/curl.h, but do nothing with the 
information?  I applied other adjustments too.

- click-rawsocket.patch: Applied!  I simplified your keyword argument handling.

- click-socket.patch: Applied!

I hadn't noticed before the blocking behavior of Socket::push().  Not very 
Clicklike.

I notice that you rely on the contents of the "fds" fd_set in Socket::push() 
after an error; the man page says not to do this.  I've adjusted the code.

There's a memory leak too, in that if Socket::_active is false, Socket::push() 
will not free the packet it is passed.  Fixed.

Also changed some logic in Socket::run_task().  Check it out.

I love the ALLOW and DENY stuff.  Very clean.

- click-reframe/umlswitch.patch: Applied!

Thanks so very, very much.
Eddie



Mark Huang wrote:
> Eddie Kohler wrote:
>>> I have a couple of additional patches to Socket that I need to post, 
>>> that simplifies the polling loop in run_task() and gets rid of the 
>>> backoff timer, which I copied from ToDevice without really knowing 
>>> why it was necessary. All that the backoff timer did was reduce 
>>> performance in some of our PlanetLab experiments by 20-30%.
> 
> This change (among others) is encompassed in click-socket.patch. Along 
> with a minor change to click/lib/master.cc (click-gettimeofday.patch), 
> we were able to reduce syscall overhead significantly in many of our 
> experiments. I also made a couple of changes to enable the Socket code 
> to be reused by UMLSwitch (click-umlswitch.patch).
> 
> I was able to reproduce Beyer's hard lock, it looks like some sort of 
> massive memory leak in Socket(TCP); if I didn't have 3 GB of RAM on my 
> dev box, it probably would have swapped itself to death, too. I'll look 
> into this separately since my patch doesn't seem to fix it or make it 
> any worse.
> 
> Other random patches which you may or may not want to apply:
> 
> click-daemon.patch: enable userlevel Click to run as a background daemon
> 
> click-icmppingrewriter.patch: reset IP destination annotation
> 
> click-reframe.patch: pull encapsulated packets out of a TCP stream
> 
> click-fromhost.patch: add "Proper" support (PlanetLab specific). Support 
> pre-configured ("static") TUN/TAP interfaces by specifying a
> 0.0.0.0 destination IP address.
> 
> click-proper.patch: add "Proper" support (PlanetLab specific)
> 
> click-umlswitch.patch: add a UMLSwitch element for interfacing with 
> UML's uml_switch daemon.
> 
> click-rawsocket.patch: add "Proper" support (PlanetLab specific)
>