[Xorp-hackers] Crash due to stale cached xrl sender pointer.
Bruce Simpson
bms at incunabulum.net
Mon Nov 9 10:57:10 PST 2009
Ben Greear wrote:
>
> With regard to larger issues of the IPC, I dislike the current callback
> logic. I would like to get rid of all the auto-generated template code
> and use real callback objects. Each xrl call will have a pointer to a
> callback
> object and will call that at appropriate times.
>
> If/when you get your Thrift stuff in, I might attempt to work on
> the callback logic.
Can you expand on what a 'real callback object' is?
The callback() method in XORP is a template method which returns a real
C++ object.
FWIW, Boost.Function is actually implemented in a pretty similar way to
XORP's callback library:
http://www.boost.org/doc/libs/1_40_0/doc/html/function.html
It is actually more generic than XORP's, which assumes that a callback()
is never going to be invoked right away (like a functor).
The code already maintains a callback pointer for XRL invocation, see
XrlSender::send(). Because Xrl is an intermediate representation, and is
sometimes used synchronously, the callback isn't maintained there.
>
> I don't think we should worry about any async message support in the
> transport
> level. As long as we have useful callbacks, then only the
> client/servers need
> to worry about async. For instance, client says 'do-big-work()'.
> Server immediately responds 'working' (RPC call is now done).
> Later, server can send more messages to client as the big work completes.
>
> XRL doesn't need to know anything about this.
We already do. Asynchrony is just a fact of life in libxipc.
To dispel any confusion: I'm not referring to asynchrony at the level of
a socket or a UNIX file descriptor. When discussing asynchrony in my
posts, I am writing about mechanisms already present in the code.
It's been clear from past history, and private exchanges with other
developers, that scalability has been a concern, particularly with BGP.
Scalability is something I consider on the worktop now, and to be
considered, but not necessarily taking action on it right now.
I am documenting what I'm seeing, as I'm reading further into it; the
way we do XRL, does influence our scalability. We may not always use
streams for the transport, for such reasons.
The big problem with Thrift has been that there is no asynchrony story
in Thrift as it's currently shipped. As such, I'm 'retro-fitting' the
protocol into XORP, along a principle of least invasive change.
I don't plan to change callback behaviour, nor other aspects of the
existing programming model.
>
> If/when it's actually needed, we can do that work then. (For threads,
> likely this would require a re-write of the entire application in
> question, as well as hacking on the various libraries.)
My current opinion is that making the tree thread-safe is feasible, but
most likely means getting rid of some of the non-reentrant code which
exists in libxorp.
Thread safety is very unlikely to involve a ground-up rewrite of the
whole tree, if appropriate constraints are in place.
Boost could be leveraged to deliver some of that thread safety, but
that's out-of-scope for the current effort.
>
> For instance, in my case, just adding that netlink filtering took
> system load
> from 300+ to 20, effectively making an O(N^2) to O(N). This sort of
> thing is way more important than saving a few cycles on encode/decode
> of messages we send once per second.
In the case of XRL, pack/unpack is probably not the bottleneck;
allocations probably are.
I appreciate what you're doing with Netlink; I often wish myself that
the BSD developers would just implement that API, as it would solve a
number of problems they're now having with API stability.
>
> Most of the slowdowns I've seen are stupid things like sleeping for
> a timeout instead of doing work, probably to work around some race
> no one felt like debugging.
Were these timeouts actually blocking execution of other pending tasks?
If so, that's something which needs dealing with.
thanks,
BMS
More information about the Xorp-hackers
mailing list