Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* Re: Patch to support NACK packet from stub
  1999-04-01  0:00 ` Patch to support NACK packet from stub Andrew Cagney
@ 1999-03-17 14:05   ` Andrew Cagney
  1999-03-17 16:54   ` J.T. Conklin
  1 sibling, 0 replies; 6+ messages in thread
From: Andrew Cagney @ 1999-03-17 14:05 UTC (permalink / raw)
  To: gdb-patches

"J.T. Conklin" wrote:
> 
> 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.
> 
> However, I had the '-' case fall through to SERIAL_TIMEOUT so the
> number of attempts a packet is sent is the same as timeouts.  If we
> wanted to add a remote_debug for the SERIAL_TIMEOUT case, that code
> needs to duplicated.

(Thanks for comments from Fernando Nasser and Michael Snyder)

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

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

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

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

	Andrew


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Patch to support NACK packet from stub
  1999-04-01  0:00 ` Patch to support NACK packet from stub Andrew Cagney
  1999-03-17 14:05   ` Andrew Cagney
@ 1999-03-17 16:54   ` J.T. Conklin
  1999-04-01  0:00     ` J.T. Conklin
  1 sibling, 1 reply; 6+ messages in thread
From: J.T. Conklin @ 1999-03-17 16:54 UTC (permalink / raw)
  To: gdb-patches

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Patch to support NACK packet from stub
       [not found] <5mu2vqw2co.fsf.cygnus.patches.gdb@jtc.redbacknetworks.com>
@ 1999-04-01  0:00 ` Andrew Cagney
  1999-03-17 14:05   ` Andrew Cagney
  1999-03-17 16:54   ` J.T. Conklin
  0 siblings, 2 replies; 6+ messages in thread
From: Andrew Cagney @ 1999-04-01  0:00 UTC (permalink / raw)
  To: gdb-patches

"J.T. Conklin" wrote:
> 
> 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.
> 
> However, I had the '-' case fall through to SERIAL_TIMEOUT so the
> number of attempts a packet is sent is the same as timeouts.  If we
> wanted to add a remote_debug for the SERIAL_TIMEOUT case, that code
> needs to duplicated.

(Thanks for comments from Fernando Nasser and Michael Snyder)

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

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

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

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

	Andrew


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Patch to support NACK packet from stub
  1999-03-17 16:54   ` J.T. Conklin
@ 1999-04-01  0:00     ` J.T. Conklin
  0 siblings, 0 replies; 6+ messages in thread
From: J.T. Conklin @ 1999-04-01  0:00 UTC (permalink / raw)
  To: gdb-patches

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Patch to support NACK packet from stub
@ 1999-04-01  0:00 J.T. Conklin
  1999-03-12 18:28 ` J.T. Conklin
  0 siblings, 1 reply; 6+ messages in thread
From: J.T. Conklin @ 1999-04-01  0:00 UTC (permalink / raw)
  To: gdb-patches

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.

However, I had the '-' case fall through to SERIAL_TIMEOUT so the
number of attempts a packet is sent is the same as timeouts.  If we
wanted to add a remote_debug for the SERIAL_TIMEOUT case, that code
needs to duplicated.

	--jtc

1999-03-12  J.T. Conklin  <jtc@redbacknetworks.com>

	* remote.c (putpkt): Handle NACK ('-') packet from stub.

Index: remote.c
===================================================================
RCS file: /usr/rback/release/tools-src/gdb/gdb/remote.c,v
retrieving revision 1.5
diff -c -r1.5 remote.c
*** remote.c	1999/02/15 21:33:55	1.5
--- remote.c	1999/03/13 02:20:20
***************
*** 2537,2542 ****
--- 2537,2543 ----
  	      switch (ch)
  		{
  		case '+':
+ 		case '-':
  		case SERIAL_TIMEOUT:
  		case '$':
  		  if (started_error_output)
***************
*** 2553,2558 ****
--- 2554,2563 ----
  	      if (remote_debug)
  		printf_unfiltered ("Ack\n");
  	      return 1;
+ 	    case '-':
+ 	      if (remote_debug)
+ 		printf_unfiltered ("Nack\n");
+ 	      /* FALLTHROUGH */
  	    case SERIAL_TIMEOUT:
  	      tcount ++;
  	      if (tcount > 3)




-- 
J.T. Conklin
RedBack Networks


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Patch to support NACK packet from stub
  1999-04-01  0:00 J.T. Conklin
@ 1999-03-12 18:28 ` J.T. Conklin
  0 siblings, 0 replies; 6+ messages in thread
From: J.T. Conklin @ 1999-03-12 18:28 UTC (permalink / raw)
  To: gdb-patches

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.

However, I had the '-' case fall through to SERIAL_TIMEOUT so the
number of attempts a packet is sent is the same as timeouts.  If we
wanted to add a remote_debug for the SERIAL_TIMEOUT case, that code
needs to duplicated.

	--jtc

1999-03-12  J.T. Conklin  <jtc@redbacknetworks.com>

	* remote.c (putpkt): Handle NACK ('-') packet from stub.

Index: remote.c
===================================================================
RCS file: /usr/rback/release/tools-src/gdb/gdb/remote.c,v
retrieving revision 1.5
diff -c -r1.5 remote.c
*** remote.c	1999/02/15 21:33:55	1.5
--- remote.c	1999/03/13 02:20:20
***************
*** 2537,2542 ****
--- 2537,2543 ----
  	      switch (ch)
  		{
  		case '+':
+ 		case '-':
  		case SERIAL_TIMEOUT:
  		case '$':
  		  if (started_error_output)
***************
*** 2553,2558 ****
--- 2554,2563 ----
  	      if (remote_debug)
  		printf_unfiltered ("Ack\n");
  	      return 1;
+ 	    case '-':
+ 	      if (remote_debug)
+ 		printf_unfiltered ("Nack\n");
+ 	      /* FALLTHROUGH */
  	    case SERIAL_TIMEOUT:
  	      tcount ++;
  	      if (tcount > 3)




-- 
J.T. Conklin
RedBack Networks


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~1999-04-01  0:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <5mu2vqw2co.fsf.cygnus.patches.gdb@jtc.redbacknetworks.com>
1999-04-01  0:00 ` Patch to support NACK packet from stub Andrew Cagney
1999-03-17 14:05   ` Andrew Cagney
1999-03-17 16:54   ` J.T. Conklin
1999-04-01  0:00     ` J.T. Conklin
1999-04-01  0:00 J.T. Conklin
1999-03-12 18:28 ` J.T. Conklin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox