[Click] socket.cc patch

Mark Huang mlhuang at CS.Princeton.EDU
Wed Jun 21 18:12:40 EDT 2006


Eddie Kohler wrote:
> - 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.

Probably; no; I didn't know about/understand the signal catching stuff. I'll try 
again.

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

Yes, I think that's definitely appropriate.

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

The Proper libraries require libcurl. The reason I added the check was that the 
Proper header files require curl/curl.h (whether they need to or not). I'll take 
another look and see if I can just fix Proper instead.

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

It does improve throughput in some cases, though. Yun Mao asked that Socket have 
a pull input to avoid blocking and send buffer overflows; Andy Bavier asked that 
it be changed back for performance. I figure that making it agnostic is a 
compromise; for more Clicklike packet scheduling, just use Socket in a pull context.

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

Ah. Yeah, I'm not sure EINTR counts as an error, but it wouldn't hurt to move 
the FD_SET() inside the loop.

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

Yup, doh. Fixed in the second patch I sent after Beyer's message. Also note that 
in this second patch, I added a missing break statement, for when connections 
are terminated.

> - click-reframe/umlswitch.patch: Applied!

UML + Click = poor man's Click-in-2.6.


More information about the click mailing list