Re: Patch to support NACK packet from stub

[jtc@redback.com (J.T. Conklin)]

>> I discovered that the putpkt() routine does not handle NACK ('-')
>> packets from remote stub, instead the NACK is junked and putpkt()
>> waits until the subsequent read times out.  This is a big penalty
>> (the default value of remote_timeout is 20 seconds) that can be
>> easily avoided.

Andrew> I think it would be best if we leave this one out of 4.18 and
Andrew> have a hard think about it for devo.  If you get rid of the
Andrew> timeout there is a chance that other nastier things can
Andrew> happen.

Andrew> Remembering that the remote GDB protocol is only robust
Andrew> against data overrun what can happen is a sequence like:

Andrew> -> <start><scrambled-data-looking-like-eop>
Andrew> <- NACK
Andrew> -> <more crambled data looking like another packet>
Andrew> <- NACK

Andrew> It is probably safer to just drain the target like the code is
Andrew> currently doing.

No hurry.  I've checked it into in my local sources, and will be able 
to report how well it works after it's been in production.

I also consider this yet another nail in the coffin of the existing
remote protocol.  My current thinking is:

  * separate link layer framing from debug protocol.

    Instead of calling getDebugChar and putDebugChar(), the sample
    stubs would call getDebugPacket() and putDebugPacket().  These
    user-supplied functions would en/decapsulate the packet as
    required for the mediatype used for the debug channel.  I'm 
    currently experimenting with using the same framing used in
    SL/IP for serial connections.  Other possible framings would 
    be within UDP datagrams or even raw ethernet packets.  

    As possibilities are endless, so it would be best if support for
    different "back ends" for each media/framing could be added with-
    out extensive changes.  It would be even better if such backends
    could be dynamically loaded, but I'd settle for ease of
    integration.

  * There needs to be some re-definition of what remote_timeout is and
    is used for.  Currently it's used for the timeout for each
    character read, when it probably needs to be the timeout for the
    entire packet.  If a media type has need for an intercharacter
    timeout, it should be specific to that framing type.

  * Since the least common denominator would be a lossy datagram
    transport, the high level debug protocol must be responsible
    for handling lost and duplicate packets.

  * I'm currently thinking the that high level debug protocol might
    look something like this:

    <type><seq>?length?[<data>]<chksum>

    Instead of having explicit "ACK" packets, the each response to a
    command would contain the ACK.  This would improve performance on
    high latency links.

    I've ?'d length, since the received packet length might not be
    necessary with an ASCII protocol, perhaps not even with a binary
    protocol.  I haven't got this far yet...


	--jtc

-- 
J.T. Conklin
RedBack Networks