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