[Xorp-hackers] Using results from one XRL to respond to another

[Steven Simpson]

On 07/02/11 15:10, Steven Simpson wrote:
>    3. Redesign the code generator so that the Xrl*TargetBase class
>       contains methods to be provided by the implementation that receive
>       the in-args plus a call-back for the out-args.

I've just sent 3 patches to the list regarding this redesign, to allow
asynchronous method implementations.  Please excuse any unconventional
use of git, as I'm not used to it.  I'll summarise the changes...

Here's what's now generated in "finder_client_base.hh" for
"common/0.1/get_target_name":

    virtual XrlCmdError common_0_1_get_target_name(
	// Output values,
	string&	name) = 0;

    typedef
    XorpCallback2<void, const XrlCmdError &,
	const string&>::RefPtr
    Common01GetTargetNameCB;
    virtual void async_common_0_1_get_target_name
       (
	Common01GetTargetNameCB);

The first declaration is unchanged from before.  Even for an
asynchronous implementation, this method will still have to be provided
- but if it's not meant to be called, then it should probably be set up
to assert(false) or something equivalent.

The second declaration is new, and defines a callback type for passing
the error and out-arguments.

The third declaration is also new, and declares a function which the
server should override as an asynchronous implementation.  It's virtual,
but not pure - here's the definition:

void
XrlFinderclientTargetBase::async_common_0_1_get_target_name(
	Common01GetTargetNameCB c_b)
{

    /* Return value declarations */
    string rarg_name;
    XrlCmdError e = common_0_1_get_target_name(
        rarg_name);
    c_b->dispatch(e,
        rarg_name);
}

So it just makes the asynchronous call into a synchronous one by making
a call to the synchronous implementation and immediately calling the
supplied callback.  This way, existing methods can still be implemented
synchronously; however, by overriding this function, you can provide a
fully asynchronous implementation - no need to call c_b straight-away.

All the changes stem from ones in "xorp/libxipc/xrl_cmd_map.hh", which
now defines a callback type:

typedef
XorpCallback2<void, const XrlCmdError &, const XrlArgs *>::RefPtr
XrlRespCallback;

This change propagates throughout the code, basically changing any
function which takes XrlArgs*out and returns XrlCmdError into one that
returns void and takes an XrlRespCallback.

In order to reach a compilable state from these changes, the
asynchronous mode is used in anger in FinderClientXrlTarget in
xorp/libxipc/finder_client_xrl_target.(cc|hh).  The method
"finder_client/0.2/dispatch_tunneled_xrl" was previously implemented
synchronously with:

XrlCmdError
FinderClientXrlTarget::finder_client_0_2_dispatch_tunneled_xrl(
						const string& xrl,
						uint32_t&     xrl_errno,
						string&	      xrl_errtxt
						)
{
    XrlCmdError e = _client->dispatch_tunneled_xrl(xrl);
    xrl_errno  = e.error_code();
    xrl_errtxt = e.note();
    return XrlCmdError::OKAY();
}

Now it is implemented with two functions:

void
FinderClientXrlTarget::async_finder_client_0_2_dispatch_tunneled_xrl
(const string&	xrl,
 FinderClient02DispatchTunneledXrlCB cb)
{
    _client->dispatch_tunneled_xrl
	(xrl,
	 callback(this,
		  &FinderClientXrlTarget::dispatch_tunneled_xrl_cb,
		  cb));
}

void
FinderClientXrlTarget::dispatch_tunneled_xrl_cb
(const XrlCmdError &e,
 const XrlArgs *out,
 FinderClient02DispatchTunneledXrlCB cb) const
{
    UNUSED(out);
    cb->dispatch(XrlCmdError::OKAY(), e.error_code(), e.note());
}



Now for a couple of caveats.  First,
STCPRequestHandler::dispatch_request in xorp/libxipc/xrl_pf_stcp.cc
packs out-arguments into some sort of buffering.  There's a sequence
number involved, so I'd be hopeful that that's the only information
needed to ensure that the response gets back to the correct entity in
the client - however, I haven't investigated any deeper.

Second, FinderMessengerBase::dispatch_xrl in
xorp/libxipc/finder_messenger.cc invokes
FinderMessengerManager::messenger_(in)active_event in pairs.  Initially,
the steps were:

   1. Mark the messenger as active.
   2. Dispatch the call with the in-args, and wait for the out-args and
      error code.
   3. Call 'reply' with the out-args/error code.
   4. Mark the messenger as inactive.

To make it asynchronous, I split the steps up into (1,2) for
dispatch_xrl and (3,4) for its callback.  However, this fails an
assertion in finder_client.cc, possibly because you can no longer
guarantee that each 'inactive' event will occur before the next 'active'
event this way.  The FinderMessengerManager that the Finder implements
can only cope with one FinderMessenger at a time, which is why the
guarantee is required.  The finder seems to use these events to
determine who is invoking it, so that a caller can not overwrite an XRL
registration for some other caller's target.

Instead, dispatch_xrl performs (1,2,4), while the callback just does
(3).  This restores the guarantee, but it means that the messenger is
not necessarily considered active during the 'reply' call.  It will be
for synchronously implemented methods, as they cause the callback to
occur before returning.  I don't know if this change will have an impact
on 'reply' for asynchronously implemented methods.



Anyway, for testing, I'm not clear on what I should be doing.  I just
tried the following:

configure
set firewall rule4 4 source network 255.255.255.255/32
set firewall rule4 4 action drop
commit

On the synchronous version, I got this among the trace:

Handling method for fea_firewall/0.1/commit_transaction failed: 
XrlCmdError 102 Command failed No firewall plugin to set the entries

I dare say, there's more I'd need to do to make that do something
proper, but I figured it was enough to compare against.  From the async
version after patch#1, I got the assertion failure in
FinderClient::messenger_active_event.  After patch#2, I managed to get
the same trace as for the sync version, as it fixes the (in)active
status of the messenger.



I hope that this turns out to be useful (and working), and invite your
comments.

Cheers,

Steven

--