Re: remote protocol patch

[Zoltan Filyo <zoltan.filyo@ericsson.com>]

On Wed, 2008-01-30 at 10:45 -0500, Daniel Jacobowitz wrote:
> On Wed, Jan 30, 2008 at 03:30:27PM +0100, Zoltan Filyo wrote:
> > The KGDB stub does not handle timeouts (fact). The stub go in a state,
> > and waiting until an end packet character or a new packet start
> > character. (Maybe this is a misbehaviour.)
> 
> This does not sound right.  Could you describe your original example
> as seen by the KGDB stub - what text did it lose, and what text did it
> receive and ignore?
> 
The code in the netbsd 4.0 is: /usr/src/sys/kern/kgdb_stub.c, line 239,
kgdb_recv(). After the kgdb_recv() read packet start step into the while
statement in line 253. It is waiting here until gets a "packet end char"
or the temp buffer run out of space.
It seems characters generally looses in set. Many times kgdb collects
"$6" or "$7" strings instead of "$g#67". The GDB before the patch gives
up after 3 trying, and sends "-" characters. But at this moment program
control had been leave the putpkt_binary() function.

Finally netbsd uses the com_common_getc() function to read characters
from the line (/usr/src/sys/dev/ic/com.c). I think this function should
be improved to handle "timeout" -- in that case too when the interrupts
are disabled. And a "timeout char" should be passed back as in GDB.


> > I could not find protocol definition (the requested behaviour, the state
> > diagrams for two sides etc.) of GDB. The exact solution will be a full,
> > closed protocol definition and conformance test suite for target and
> > remote side too. I have no enough spirit for this, sorry.
> 
> It won't help you, anyway.  The protocol is not robust against a noisy
> line, and modeling it more accurately will just make it clear how
> broken it is if the line is noisy.
> 
> > My patch causes a "better" behaviour only on the GDB side, but without
> > affect previous behaviour (if you want that). It takes differences
> > between "single send/response session"  and a "whole command resend"
> > case. Two parameters "max_ack_retry_count" and "max_packet_retry_count"
> > to be able to use regulate the GDB behaviour.
> 
> I don't understand the difference between these two cases.
> 
First case (state): the gdb waiting for an acknowledge from the peer
after packet send (sub state of the next). The second case (state): GDB
trying to send a packet to the peer.

In this particular debug session after the patch, gdb does not giving up
the packet resending, and kernel stub sooner or later get a "packet end
char" or its buffer run out of space. This causes that kgdb stub leaves
"collecting the packet" state and go into the "wait for packet start"
state.

Yes, this patch does not solve command duplication.