[remote] Checking for supported packets

[remote] Checking for supported packets

[Daniel Jacobowitz]

I've been working, for the last couple of months, on a wide variety
of projects that involve new remote protocol packets.  I'm planning
to submit each and every one of them; I've just been busy, and
also some of them aren't quite fully baked yet.

Before I start doing that, though, I'd like to trigger a little
conversation about a topic that's been bothering me.  When we
connect to a target, there's a number of packets that we need
to send right away, to find out if they're supported or to get
information about the target's current state.  Here's a current
sample:

Sending packet: $Hc-1#09...Ack
Packet received:
Sending packet: $qC#b4...Ack
Packet received:
Sending packet: $qOffsets#4b...Ack
Packet received:
Sending packet: $?#3f...Ack
Packet received: S00
Sending packet: $Hg0#df...Ack
Packet received:
Sending packet: $pf#d6...Ack
Packet received: 00000000
Packet p (fetch-register) is supported
Sending packet: $m0,4#fd...Ack
Packet received: 1000ffe7

qC and qOffsets are both probes.  [Hc/Hg we don't even notice that
the target has failed to support, so we keep sending them.  That's
unrelated.]

I've got at least two more packets on my list that would have
to be probed for at connect: one to read available features (the
XML bits on my branch, which are coming along well), and another
very useful probe that asks the target how big of a packet it
can handle.  That's a huge performance help on any link with latency
measurable in respect to its bandwidth; it's especially useful over
TCP, whether to a local system or to one far away.

Every new packet of this type has to be probed for on each new
target connection.  This takes a noticeable amount of time.

So, I'm wondering, can we cut that number down, and should we?
Something like this:

-> qPackets?
<- qPackets,qPacketSize+,qOffsets-,qPart:available+,qC-

"I support the qPacketSize and qPart:available queries, but don't
bother probing for qOffsets or qC, they won't work".

This isn't the most useful as it is.  There's a lot of possible
alternatives, for instance:

  1. If qPackets is not present, assume new packets are not supported;
     i.e. require the stub to respond to qPackets before trying
     qPart:available.

  2. If the qPackets response does not mention a packet, assume it
     is not supported.

  3. Or, if the qPackets response does not mention a packet, probe
     for it.

I'm guessing that the most useful interpretation would be something
like 1 && 2, but I mention 3 because implementing #2 could prove
to be a problem for some designs of remote stub (with separate
halves for the protocol bits and the controlling hardware bits).
Anyone think this is likely to be a problem in practice?  Other
possible approaches?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [remote] Checking for supported packets

[Daniel Jacobowitz]

On Mon, Mar 13, 2006 at 09:15:26PM -0500, Daniel Jacobowitz wrote:
> I've been working, for the last couple of months, on a wide variety
> of projects that involve new remote protocol packets.  I'm planning
> to submit each and every one of them; I've just been busy, and
> also some of them aren't quite fully baked yet.

...

> Something like this:
> 
> -> qPackets?
> <- qPackets,qPacketSize+,qOffsets-,qPart:available+,qC-
> 
> "I support the qPacketSize and qPart:available queries, but don't
> bother probing for qOffsets or qC, they won't work".

Don't suppose anyone had time to look at this?

I don't know if there's any active GDB maintainers, right now, who are
interested in the remote protocol.  Or e.g. stub developers who are
interested, and reading this list.  As I said, I have a whole bundle
of upcoming proposed additions to the remote protocol; I do my best
to design them intelligently and compatibly, and I will document them
prettily and post them for review, but the benefit's much lessened
if there's no one interested in reviewing them :-)

I intend to turn this particular example into a more thought-out
proposal this week.  And if I'm really lucky, I'll have the
target-defined features proposal ready this week also; but don't
hold your breath, it might be next week.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [remote] Checking for supported packets

[Paul Koning]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

 Daniel> On Mon, Mar 13, 2006 at 09:15:26PM -0500, Daniel Jacobowitz
 Daniel> wrote:
 >> I've been working, for the last couple of months, on a wide
 >> variety of projects that involve new remote protocol packets.  I'm
 >> planning to submit each and every one of them; I've just been
 >> busy, and also some of them aren't quite fully baked yet.

 Daniel> ...

 >> Something like this:
 >> 
 -> qPackets?
 >> <- qPackets,qPacketSize+,qOffsets-,qPart:available+,qC-
 >> 
 >> "I support the qPacketSize and qPart:available queries, but don't
 >> bother probing for qOffsets or qC, they won't work".

 Daniel> Don't suppose anyone had time to look at this?

Not to speak of, unfortunately...

 Daniel> I don't know if there's any active GDB maintainers, right
 Daniel> now, who are interested in the remote protocol.  Or e.g. stub
 Daniel> developers who are interested, and reading this list.  As I
 Daniel> said, I have a whole bundle of upcoming proposed additions to
 Daniel> the remote protocol; I do my best to design them
 Daniel> intelligently and compatibly, and I will document them
 Daniel> prettily and post them for review, but the benefit's much
 Daniel> lessened if there's no one interested in reviewing them :-)

We're every day users of the remote protocol, and I've done some minor
digging into it.  So I'm interested.

I've certainly noticed that the sensing of stub capabilities right now
is rather messy and chatty.  The specific example I ran into was the
support of hardware watchpoints and/or breakpoints.

	paul

[PROPOSAL] Checking for supported packets

[Daniel Jacobowitz]

On Tue, Mar 21, 2006 at 12:12:21AM -0500, Daniel Jacobowitz wrote:
> I intend to turn this particular example into a more thought-out
> proposal this week.  And if I'm really lucky, I'll have the
> target-defined features proposal ready this week also; but don't
> hold your breath, it might be next week.

That one escaped first, as it happens.  Here's the other.

Restating the problem:

Whenever we connect to a remote target, we set a number of packets
to auto-detect.  The first time they are needed, we send them, and
interpret an empty response as "unsupported".  Remote stubs are
taught to ignore unknown packets and generate the empty response we
expect.

The down side of this is performance.  Some of the probes are in
response to direct user commands, and those are only a small nuisance,
e.g. qGetTLSAddress will be generated in response to an attempt to
print a TLS variable.  But some commands generate more than one.
Particularly important is the initial connect (which probes for
qOffsets, various thread packets, and more to come in my future
proposals - which is why I'm tackling this first).  Also of interest
is the first continue or step, which probes for vCont and also an
assortment of breakpoint related packets (typically Z0 to set a
breakpoint, and if that fails X to do binary download, and only then
falling back to M).

Other problems I decided to fix at the same time:

We guess very conservatively at the maximum supported packet size
by the remote stub, in order to support simplistic stubs which don't
do decent bounds checking.  Many modern stubs support larger transfers,
and larger transfers are more efficient.  Over TCP this can make a huge
difference for "load".

We have no way to tell the stub about responses we can understand, but
earlier versions of GDB would have choked on.  For instance, I proposed
a month or two ago for qPart responses to use a trailing '=' as an
end-of-object marker, to save an extra packet round trip.  This could
generate an error if the client doesn't support it.  So when I
implement that, soon, I'll add a feature notification for it.

Here's the actual proposal, in texinfo.  I also have a tested
implementation of this, which I will not post right away; I'd
like feedback on the interface first, if anyone has comments.

(Is Texinfo or makeinfo's text output more useful for proposals?
I can do either.)

@@ -23241,6 +23255,59 @@ encoded 32 bit mode; @var{threadid} is a
 
 Reply: see @code{remote.c:remote_unpack_thread_info_response()}.
 
+@item qPacketInfo @r{[};@var{feature}@r{]}...
+@cindex support packets, remote query
+@cindex @samp{qPacketInfo} packet
+Tell the remote target about features supported by @value{GDBN}, and
+query it for features it supports.
+
+No values of @var{feature} are defined yet.  Targets should ignore any
+unknown values for @var{feature}.  Any @value{GDBN} which sends a
+@samp{qPacketInfo} packet supports receiving packets of unlimited
+length.  Values for @var{feature} may be defined in the future to let
+the stub take advantage of new features in @value{GDBN}, e.g.@:
+incompatible improvements in the remote protocol.
+
+The reply is one or more feature responses, or empty if this packet is
+not supported.  Multiple feature responses are separated by semicolons,
+and individual feature responses may not include semicolons.  @value{GDBN}
+will silently ignore unrecognized feature responses, as long as the
+unrecognized response has one of the standard forms.  The standard forms
+are:
+
+@table @samp
+@item @var{name}=@var{value}
+Set a remote communication parameter to the specified value.
+@item @var{name}+
+The remote protocol packet @var{name} is supported.
+@item @var{name}-
+The remote protocol packet @var{name} is not supported.
+@end table
+
+Currently only one communication parameter is supported:
+
+@table @samp
+@item PacketSize=@var{bytes}
+The remote target can accept packets up to at least @var{bytes} in
+length.
+@end table
+
+The name of a packet which can be marked as supported or unsupported
+is the text of the packet in this documentation, up to but not
+including the first punctuation character or variable.  For example, a
+target which supports hardware watchpoints but not hardware
+breakpoints might report @samp{Z0-;Z1-;Z2+;Z3+;Z4+}.  An exception is
+made for @samp{qPart:@var{object}} packets; the name of the packet
+includes the @var{object}, but not the @var{annex}.  Individual
+@samp{qPart} packet types must be reported separately.
+
+Currently, all remote packets which are not mentioned in the response
+will be probed individually, just as if the @samp{qPacketInfo} command
+was not supported.  In the future, some new packets may be added to
+the remote protocol which will be assumed to be unsupported unless
+@samp{qPacketInfo} is supported and the new packet is reported in the
+@samp{qPacketInfo} response.
+
 @item qPart:@var{object}:read:@var{annex}:@var{offset},@var{length}
 @cindex read special object, remote request
 @cindex @samp{qPart} packet


-- 
Daniel Jacobowitz
CodeSourcery

Re: [PROPOSAL] Checking for supported packets

[Eli Zaretskii]

> Date: Thu, 30 Mar 2006 16:52:47 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> Here's the actual proposal, in texinfo.  I also have a tested
> implementation of this, which I will not post right away; I'd
> like feedback on the interface first, if anyone has comments.

Should I restrain from commenting on the documentation itself for now?

> (Is Texinfo or makeinfo's text output more useful for proposals?
> I can do either.)

I prefer Texinfo, since it shows things that are not visible in the
text output (such as indexing and markup that only shows in the
printed version).

> +The reply is one or more feature responses, or empty if this packet is
> +not supported.

The notion of ``empty response'' is not described anywhere in the
manual, AFAICS.  (Yes, this is not directly related to the changes you
are proposing, but I'd like this to be fixed as a side effect.)

Re: [PROPOSAL] Checking for supported packets

[Daniel Jacobowitz]

On Fri, Mar 31, 2006 at 10:54:02AM +0300, Eli Zaretskii wrote:
> > Date: Thu, 30 Mar 2006 16:52:47 -0500
> > From: Daniel Jacobowitz <drow@false.org>
> > 
> > Here's the actual proposal, in texinfo.  I also have a tested
> > implementation of this, which I will not post right away; I'd
> > like feedback on the interface first, if anyone has comments.
> 
> Should I restrain from commenting on the documentation itself for now?

Up to you; I think such comments will be more useful later, but if you
offer them now, I'll fix them now :-)

> > +The reply is one or more feature responses, or empty if this packet is
> > +not supported.
> 
> The notion of ``empty response'' is not described anywhere in the
> manual, AFAICS.  (Yes, this is not directly related to the changes you
> are proposing, but I'd like this to be fixed as a side effect.)

     For any COMMAND not supported by the stub, an empty response
  (`$#00') should be returned.  That way it is possible to extend the
  protocol.  A newer GDB can tell if a packet is supported based on that
  response.

This is the first result for searching for empty response; it's in the
remote protocol Overview section.  Is that sufficient?  Everywhere else
it's just described as "empty" or "empty reply".

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PROPOSAL] Checking for supported packets

[Eli Zaretskii]

> Date: Fri, 31 Mar 2006 09:19:58 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Fri, Mar 31, 2006 at 05:09:27PM +0300, Eli Zaretskii wrote:
> > > This is the first result for searching for empty response; it's in the
> > > remote protocol Overview section.  Is that sufficient?  Everywhere else
> > > it's just described as "empty" or "empty reply".
> > 
> > Let's make a point of saying ``empty response'' everywhere, okay?
> 
> Should we settle on "reply" or "response"?  I've been using response,
> but most packet's have a section labelled "Reply:" with an entry for
> "empty", which suggests that it is also or instead called an empty reply.

I don't mind using ``empty reply'' as long as we do that consistently.

> Oops, did I write that?  It's supposed to be "supported packets, remote
> query".
> 
> > > +@cindex @samp{qPacketInfo} packet
> > > +Tell the remote target about features supported by @value{GDBN}, and
> > > +query it for features it supports.
> > 
> > I think we need here an index entry that mentions the word
> > ``features''.  Observe how extensively you use it here and afterwards,
> > it's certainly something readers will try to search for when they are
> > looking for this description.
> 
> How about "features of a remote target, query"?  Oh, blast, that's
> ambiguous with the other proposal I posted (for registers et cetera).
> Does features need to be at the start of the index entry?  If not, how
> about "protocol features, remote query"?

No, ``features'' does not need to be the first word, although if it
is, users will see that entry when they type "i features TAB" in Info,
so it is desirable if possible.  "protocol features, remote query" is
okay, although we could also have "features of remote protocol, query
for support" or even simply "features of remote protocol" (since the
other index entries don't use ``query'' at all).

> That is, all four failed queries are consolidated to a single
> what-do-you-support query.  Sending a couple of additional bytes in the
> qPacketInfo response is generally less costly than waiting for four
> round trips, and it will be even more beneficial when packets not
> mentioned in qPacketInfo are assumed to be unsupported; for instance,
> the qPart:features packet I recently proposed would not be tried if
> qPacketInfo failed or if qPacketInfo didn't mention it, only if there
> was a qPart:features+ response.

Thanks.  But in practice, qPacketInfo could result in an empty reply,
so for stubs that don't support qPacketInfo, this addition is a
burden.  Should we have a facility to tell GDB not to use qPacketInfo?

In any case, I think this explanation should be in the manual, because
it is not immediately obvious that several short packets take so much
less time than one longer packet.

As another aside, you wrote in your original message:

  "Remote stubs are taught to ignore unknown packets and generate the
  empty response we expect."

Is there anything else included in ``ignoring'' besides the empty
response?  I'm asking because, in everyday's speech, ``ignore'' means
``do nothing'', not ``send a packet''.

Re: [PROPOSAL] Checking for supported packets - revised

[Daniel Jacobowitz]

I haven't followed up on the qPacketInfo proposal in a while, but I
certainly hadn't forgotten about it either.  I have several
improvements stacked up in a row, and this one ought to get done first.

On Sat, Apr 01, 2006 at 01:21:59PM +0300, Eli Zaretskii wrote:
> Thanks.  But in practice, qPacketInfo could result in an empty reply,
> so for stubs that don't support qPacketInfo, this addition is a
> burden.  Should we have a facility to tell GDB not to use qPacketInfo?

Yes, probably; I've added one in my working copy.

> In any case, I think this explanation should be in the manual, because
> it is not immediately obvious that several short packets take so much
> less time than one longer packet.

Thanks, this is a good idea.

> As another aside, you wrote in your original message:
> 
>   "Remote stubs are taught to ignore unknown packets and generate the
>   empty response we expect."
> 
> Is there anything else included in ``ignoring'' besides the empty
> response?  I'm asking because, in everyday's speech, ``ignore'' means
> ``do nothing'', not ``send a packet''.

Nope, just the bit about "empty responses" described in the overview
section.

I've also revised the proposal in a couple of other ways:

  - Clarified documentation about the interaction with new packets.

  - A new "maybe" response, as in "maybe I support that packet, ask
    me when you need it".  This turns out to be quite useful if you
    have a remote protocol library separated from your target
    management library, which seems reasonably common.  For instance,
    a breakpoint-related target might not be supported on some
    particular target board lacking the necessary debug register.
    But the stub's internal interface may not facilitate checking
    for this.  Rather than forcing stubs to change, let's be flexible.

  - Packet renamed from qPacketInfo to qSupported following the recent
    discussion of qC/qL/qP prefixes.  This is where we actually
    discovered the problem; CodeSourcery's latest GDB release sends
    qPacketInfo to RedBoot, which responds with "huh?  that's not a
    valid qP packet - E01".

Here are the new commands currently in my patch, and the updated
protocol documentation.  If no one has any additional comments about
it, I'll submit the corresponding code soon.

+@item set remote load-offsets
+@kindex set remote load-offsets
+@cindex load offsets of remote targets
+This command enables or disables the use of the @samp{qOffsets}
+request packet.  This packet is used to find a relocated downloaded
+image.  @xref{General Query Packets, qOffsets}, for more details about
+this packet.  The default depends on the remote stub's support of this
+packet (@value{GDBN} queries the stub when this packet is first
+required).
+
+@item show remote load-offsets
+@kindex show remote load-offsets
+Show the current setting of @samp{qOffsets} packet usage.
+
+@item set remote supported-packets
+@kindex set remote supported-packets
+@cindex query supported packets of remote targets
+This command enables or disables the use of the @samp{qSupported}
+request packet.  @xref{General Query Packets, qSupported}, for more
+details about this packet.  The default is to use @samp{qSupported}.
+
+@item show remote supported-packets
+@kindex show remote supported-packets
+Show the current setting of @samp{qSupported} packet usage.



+@item qSupported @r{[}:@var{feature} @r{[};@var{feature}@r{]}... @r{]}
+@cindex supported packets, remote query
+@cindex features of the remote protocol
+@cindex @samp{qSupported} packet
+Tell the remote target about features supported by @value{GDBN}, and
+query it for features it supports.  This packet allows @value{GDBN}
+and the remote target to take advantage of each others' features.
+It also can eliminate excessive round trips when connecting to
+a target; on high-latency links, a single @samp{qSupported} packet
+is faster than a series of probe packets for unsupported packets.
+
+No values of @var{feature} are defined yet.  Targets should ignore any
+unknown values for @var{feature}.  Any @value{GDBN} which sends a
+@samp{qSupported} packet supports receiving packets of unlimited
+length (earlier versions of @value{GDBN} may reject overly long
+responses).  Values for @var{feature} may be defined in the future to let
+the stub take advantage of new features in @value{GDBN}, e.g.@:
+incompatible improvements in the remote protocol.
+
+The reply is one or more feature responses, or empty if this packet is
+not supported.  Multiple feature responses are separated by semicolons,
+and individual feature responses may not include semicolons.  @value{GDBN}
+will silently ignore unrecognized feature responses, as long as the
+unrecognized response has one of the standard forms.  The standard forms
+are:
+
+@table @samp
+@item @var{name}=@var{value}
+Set a remote communication parameter to the specified value (see below
+for a list of communication parameters).
+@item @var{name}+
+The remote protocol packet @var{name} is supported.
+@item @var{name}-
+The remote protocol packet @var{name} is not supported.
+@item @var{name}?
+The remote protocol packet @var{name} may be supported, and @value{GDBN}
+should attempt to detect the packet when it is needed.  This is useful
+when a stub's internal architecture does not allow the protocol layer
+to know some information about the underlying target in advance.
+@end table
+
+Currently only one communication parameter is supported:
+
+@table @samp
+@item PacketSize=@var{bytes}
+The remote target can accept packets up to at least @var{bytes} in
+length.  @value{GDBN} will send packets up to this size for bulk
+transfers, and will never send larger packets.  This is a limit on the
+data characters in the packet, including the frame and checksum.
+There is no trailing NUL byte in a remote protocol packet; if the stub
+stores packets in a NUL-terminated format, it should allow an extra
+byte in its buffer for the NUL.
+@end table
+
+The name of a packet which can be marked as supported or unsupported
+is the text of the packet in this documentation, up to but not
+including the first punctuation character or variable.  For example, a
+target which supports hardware watchpoints but not hardware
+breakpoints might report @samp{Z0-;Z1-;Z2+;Z3+;Z4+}.  An exception is
+made for @samp{qPart:@var{object}} packets; the name of the packet
+includes the @var{object}, but not the @var{annex}.  Individual
+@samp{qPart} objects types must be reported separately.
+
+Currently, all remote packets which are not mentioned in the response
+will be probed individually, just as if the @samp{qSupported} query
+was not supported.  In the future, some new packets may be added to
+the remote protocol which will be assumed to be unsupported unless
+@samp{qSupported} is supported and the new packet is reported in the
+@samp{qSupported} response.  Any such packets will be listed here,
+and their documentation will refer to @samp{qSupported}.
+


-- 
Daniel Jacobowitz
CodeSourcery

Re: [PROPOSAL] Checking for supported packets - revised

[Eli Zaretskii]

> Date: Tue, 9 May 2006 19:01:23 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> Here are the new commands currently in my patch, and the updated
> protocol documentation.  If no one has any additional comments about
> it, I'll submit the corresponding code soon.

The text is okay with me, but please take care of these minor gotchas:

> +@item qSupported @r{[}:@var{feature} @r{[};@var{feature}@r{]}... @r{]}
                                                                ^^^
This should use @dots{}, not literal dots.

> +@cindex supported packets, remote query
> +@cindex features of the remote protocol
> +@cindex @samp{qSupported} packet
> +Tell the remote target about features supported by @value{GDBN}, and
> +query it for features it supports.  This packet allows @value{GDBN}
> +and the remote target to take advantage of each others' features.
> +It also can eliminate excessive round trips when connecting to
> +a target; on high-latency links, a single @samp{qSupported} packet
> +is faster than a series of probe packets for unsupported packets.
> +
> +No values of @var{feature} are defined yet.

Is there any way to somehow mark this last sentence, so that we will
remove it as soon as at least one feature is defined?  I'm afraid we
will forget.

> +Currently, all remote packets which are not mentioned in the response
> +will be probed individually, just as if the @samp{qSupported} query
> +was not supported.  In the future, some new packets may be added to

Same here.

> +@item @var{name}?
> +The remote protocol packet @var{name} may be supported, and @value{GDBN}
> +should attempt to detect the packet when it is needed.

"attempt to detect the packet"?  Perhaps it's better to say "attempt
to detect whether the packet is supported".

> +The name of a packet which can be marked as supported or unsupported
> +is the text of the packet in this documentation, up to but not
> +including the first punctuation character or variable.  For example, a
> +target which supports hardware watchpoints but not hardware
> +breakpoints might report @samp{Z0-;Z1-;Z2+;Z3+;Z4+}.  An exception is
> +made for @samp{qPart:@var{object}} packets; the name of the packet
> +includes the @var{object}, but not the @var{annex}.  Individual
> +@samp{qPart} objects types must be reported separately.

Please add a cross-reference to the two places where the two example
packets are described, so that the reader could consult them in case
they don't remember the packets' formats by heart.

Re: [PROPOSAL] Checking for supported packets - revised

[Daniel Jacobowitz]

On Wed, May 10, 2006 at 09:26:47PM +0300, Eli Zaretskii wrote:
> > +@item qSupported @r{[}:@var{feature} @r{[};@var{feature}@r{]}... @r{]}
>                                                                 ^^^
> This should use @dots{}, not literal dots.

Thanks!  Fixed.

> > +No values of @var{feature} are defined yet.
> 
> Is there any way to somehow mark this last sentence, so that we will
> remove it as soon as at least one feature is defined?  I'm afraid we
> will forget.
> 
> > +Currently, all remote packets which are not mentioned in the response
> > +will be probed individually, just as if the @samp{qSupported} query
> > +was not supported.  In the future, some new packets may be added to
> 
> Same here.

Well, I am intending to add a packet of that sort shortly after this
patch goes in.  I couldn't think of any other way to write the
documentation to reflect the current state, in which there are no
examples.  A @c comment wouldn't help much; it's just as easily
forgotten.

If you have any ideas on a better way to mark it, I'll do that;
otherwise, I will simply flag this message, and make sure that
I revisit it soon.

> > +@item @var{name}?
> > +The remote protocol packet @var{name} may be supported, and @value{GDBN}
> > +should attempt to detect the packet when it is needed.
> 
> "attempt to detect the packet"?  Perhaps it's better to say "attempt
> to detect whether the packet is supported".

How about this?

The remote protocol packet @var{name} may be supported, and @value{GDBN}
should auto-detect support when it is needed.

> > +The name of a packet which can be marked as supported or unsupported
> > +is the text of the packet in this documentation, up to but not
> > +including the first punctuation character or variable.  For example, a
> > +target which supports hardware watchpoints but not hardware
> > +breakpoints might report @samp{Z0-;Z1-;Z2+;Z3+;Z4+}.  An exception is
> > +made for @samp{qPart:@var{object}} packets; the name of the packet
> > +includes the @var{object}, but not the @var{annex}.  Individual
> > +@samp{qPart} objects types must be reported separately.
> 
> Please add a cross-reference to the two places where the two example
> packets are described, so that the reader could consult them in case
> they don't remember the packets' formats by heart.

To Z0 and qPart, you mean?  I don't see how to do it.  They're not
nodes; they're @items in tables.  Would an xref to the entire packet
table, which is in the previous section, be helpful for Z0?  qPart is
in the same table as this paragraph.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PROPOSAL] Checking for supported packets - revised

[Eli Zaretskii]

> Date: Wed, 10 May 2006 14:44:34 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> > Is there any way to somehow mark this last sentence, so that we will
> > remove it as soon as at least one feature is defined?  I'm afraid we
> > will forget.
> > 
> > > +Currently, all remote packets which are not mentioned in the response
> > > +will be probed individually, just as if the @samp{qSupported} query
> > > +was not supported.  In the future, some new packets may be added to
> > 
> > Same here.
> 
> Well, I am intending to add a packet of that sort shortly after this
> patch goes in.  I couldn't think of any other way to write the
> documentation to reflect the current state, in which there are no
> examples.  A @c comment wouldn't help much; it's just as easily
> forgotten.
> 
> If you have any ideas on a better way to mark it, I'll do that;
> otherwise, I will simply flag this message, and make sure that
> I revisit it soon.

The best way is to add a comment that has some string for which you
will grep when you make the change that requires the text to be
updated.  If you can think about such a string, please add it to the
comment; otherwise I guess we will have to try to remember.

> > > +@item @var{name}?
> > > +The remote protocol packet @var{name} may be supported, and @value{GDBN}
> > > +should attempt to detect the packet when it is needed.
> > 
> > "attempt to detect the packet"?  Perhaps it's better to say "attempt
> > to detect whether the packet is supported".
> 
> How about this?
> 
> The remote protocol packet @var{name} may be supported, and @value{GDBN}
> should auto-detect support when it is needed.

That's fine.

> > > +The name of a packet which can be marked as supported or unsupported
> > > +is the text of the packet in this documentation, up to but not
> > > +including the first punctuation character or variable.  For example, a
> > > +target which supports hardware watchpoints but not hardware
> > > +breakpoints might report @samp{Z0-;Z1-;Z2+;Z3+;Z4+}.  An exception is
> > > +made for @samp{qPart:@var{object}} packets; the name of the packet
> > > +includes the @var{object}, but not the @var{annex}.  Individual
> > > +@samp{qPart} objects types must be reported separately.
> > 
> > Please add a cross-reference to the two places where the two example
> > packets are described, so that the reader could consult them in case
> > they don't remember the packets' formats by heart.
> 
> To Z0 and qPart, you mean?  I don't see how to do it.  They're not
> nodes; they're @items in tables.

Use @anchor.  It lets you specify a location other than a node to
which an @xref can refer.  You will find several examples in the
manual.

Re: [PROPOSAL] Checking for supported packets - revised

[Jim Blandy]

Daniel Jacobowitz <drow@false.org> writes:
> I haven't followed up on the qPacketInfo proposal in a while, but I
> certainly hadn't forgotten about it either.  I have several
> improvements stacked up in a row, and this one ought to get done first.
>
> On Sat, Apr 01, 2006 at 01:21:59PM +0300, Eli Zaretskii wrote:
>> Thanks.  But in practice, qPacketInfo could result in an empty reply,
>> so for stubs that don't support qPacketInfo, this addition is a
>> burden.  Should we have a facility to tell GDB not to use qPacketInfo?
>
> Yes, probably; I've added one in my working copy.
>
>> In any case, I think this explanation should be in the manual, because
>> it is not immediately obvious that several short packets take so much
>> less time than one longer packet.
>
> Thanks, this is a good idea.
>
>> As another aside, you wrote in your original message:
>> 
>>   "Remote stubs are taught to ignore unknown packets and generate the
>>   empty response we expect."
>> 
>> Is there anything else included in ``ignoring'' besides the empty
>> response?  I'm asking because, in everyday's speech, ``ignore'' means
>> ``do nothing'', not ``send a packet''.
>
> Nope, just the bit about "empty responses" described in the overview
> section.
>
> I've also revised the proposal in a couple of other ways:
>
>   - Clarified documentation about the interaction with new packets.
>
>   - A new "maybe" response, as in "maybe I support that packet, ask
>     me when you need it".  This turns out to be quite useful if you
>     have a remote protocol library separated from your target
>     management library, which seems reasonably common.  For instance,
>     a breakpoint-related target might not be supported on some
>     particular target board lacking the necessary debug register.
>     But the stub's internal interface may not facilitate checking
>     for this.  Rather than forcing stubs to change, let's be flexible.
>
>   - Packet renamed from qPacketInfo to qSupported following the recent
>     discussion of qC/qL/qP prefixes.  This is where we actually
>     discovered the problem; CodeSourcery's latest GDB release sends
>     qPacketInfo to RedBoot, which responds with "huh?  that's not a
>     valid qP packet - E01".
>
> Here are the new commands currently in my patch, and the updated
> protocol documentation.  If no one has any additional comments about
> it, I'll submit the corresponding code soon.
>
> +@item set remote load-offsets
> +@kindex set remote load-offsets
> +@cindex load offsets of remote targets
> +This command enables or disables the use of the @samp{qOffsets}
> +request packet.  This packet is used to find a relocated downloaded
> +image.  @xref{General Query Packets, qOffsets}, for more details about
> +this packet.  The default depends on the remote stub's support of this
> +packet (@value{GDBN} queries the stub when this packet is first
> +required).
> +
> +@item show remote load-offsets
> +@kindex show remote load-offsets
> +Show the current setting of @samp{qOffsets} packet usage.
> +
> +@item set remote supported-packets
> +@kindex set remote supported-packets
> +@cindex query supported packets of remote targets
> +This command enables or disables the use of the @samp{qSupported}
> +request packet.  @xref{General Query Packets, qSupported}, for more
> +details about this packet.  The default is to use @samp{qSupported}.
> +
> +@item show remote supported-packets
> +@kindex show remote supported-packets
> +Show the current setting of @samp{qSupported} packet usage.
>
>
>
> +@item qSupported @r{[}:@var{feature} @r{[};@var{feature}@r{]}... @r{]}
> +@cindex supported packets, remote query
> +@cindex features of the remote protocol
> +@cindex @samp{qSupported} packet
> +Tell the remote target about features supported by @value{GDBN}, and
> +query it for features it supports.  This packet allows @value{GDBN}
> +and the remote target to take advantage of each others' features.
> +It also can eliminate excessive round trips when connecting to
> +a target; on high-latency links, a single @samp{qSupported} packet
> +is faster than a series of probe packets for unsupported packets.
> +
> +No values of @var{feature} are defined yet.  Targets should ignore any
> +unknown values for @var{feature}.  Any @value{GDBN} which sends a
> +@samp{qSupported} packet supports receiving packets of unlimited
> +length (earlier versions of @value{GDBN} may reject overly long
> +responses).  Values for @var{feature} may be defined in the future to let
> +the stub take advantage of new features in @value{GDBN}, e.g.@:
> +incompatible improvements in the remote protocol.
> +
> +The reply is one or more feature responses, or empty if this packet is
> +not supported.  Multiple feature responses are separated by semicolons,
> +and individual feature responses may not include semicolons.

I think it's a bit clearer to say "must not" instead of "may not"; the
latter hits my ears as "might not".

> +will silently ignore unrecognized feature responses, as long as the
> +unrecognized response has one of the standard forms.  The standard forms
> +are:
> +
> +@table @samp
> +@item @var{name}=@var{value}
> +Set a remote communication parameter to the specified value (see below
> +for a list of communication parameters).
> +@item @var{name}+
> +The remote protocol packet @var{name} is supported.
> +@item @var{name}-
> +The remote protocol packet @var{name} is not supported.
> +@item @var{name}?
> +The remote protocol packet @var{name} may be supported, and @value{GDBN}
> +should attempt to detect the packet when it is needed.  This is useful
> +when a stub's internal architecture does not allow the protocol layer
> +to know some information about the underlying target in advance.
> +@end table
> +
> +Currently only one communication parameter is supported:
> +
> +@table @samp
> +@item PacketSize=@var{bytes}
> +The remote target can accept packets up to at least @var{bytes} in
> +length.  @value{GDBN} will send packets up to this size for bulk
> +transfers, and will never send larger packets.  This is a limit on the
> +data characters in the packet, including the frame and checksum.
> +There is no trailing NUL byte in a remote protocol packet; if the stub
> +stores packets in a NUL-terminated format, it should allow an extra
> +byte in its buffer for the NUL.
> +@end table
> +
> +The name of a packet which can be marked as supported or unsupported
> +is the text of the packet in this documentation, up to but not
> +including the first punctuation character or variable.  For example, a
> +target which supports hardware watchpoints but not hardware
> +breakpoints might report @samp{Z0-;Z1-;Z2+;Z3+;Z4+}.  An exception is

Doesn't that indicate that the target does not support software
breakpoints, as well?

> +made for @samp{qPart:@var{object}} packets; the name of the packet
> +includes the @var{object}, but not the @var{annex}.  Individual
> +@samp{qPart} objects types must be reported separately.
> +
> +Currently, all remote packets which are not mentioned in the response
> +will be probed individually, just as if the @samp{qSupported} query
> +was not supported.  In the future, some new packets may be added to
> +the remote protocol which will be assumed to be unsupported unless
> +@samp{qSupported} is supported and the new packet is reported in the
> +@samp{qSupported} response.  Any such packets will be listed here,
> +and their documentation will refer to @samp{qSupported}.
> +

It seems to me that GDB should assume that if a packet isn't
mentioned, it's not supported.  My thinking is:

- As GDB grows new packets over time, if GDB does not assume these new
  packets are unsupported if unmentioned, then we'll grow a new series
  of round trips with older stubs that do support the qSupported
  packet.  If we're going to go to all this trouble, let's get rid of
  these round trips once and for all.

- Under the rule I'm suggesting (call it "unmentioned means
  unimplemented"?), if a stub supports a new packet but doesn't
  mention it, GDB will never send it.  So stub implementors have an
  incentive and a reminder to keep their qSupported responses up to
  date.

Re: [PROPOSAL] Checking for supported packets - revised

[Daniel Jacobowitz]

On Wed, May 10, 2006 at 03:15:17PM -0700, Jim Blandy wrote:
> > +The reply is one or more feature responses, or empty if this packet is
> > +not supported.  Multiple feature responses are separated by semicolons,
> > +and individual feature responses may not include semicolons.
> 
> I think it's a bit clearer to say "must not" instead of "may not"; the
> latter hits my ears as "might not".

Fixed.

> > +The name of a packet which can be marked as supported or unsupported
> > +is the text of the packet in this documentation, up to but not
> > +including the first punctuation character or variable.  For example, a
> > +target which supports hardware watchpoints but not hardware
> > +breakpoints might report @samp{Z0-;Z1-;Z2+;Z3+;Z4+}.  An exception is
> 
> Doesn't that indicate that the target does not support software
> breakpoints, as well?

Fixed, thanks.

> > +made for @samp{qPart:@var{object}} packets; the name of the packet
> > +includes the @var{object}, but not the @var{annex}.  Individual
> > +@samp{qPart} objects types must be reported separately.
> > +
> > +Currently, all remote packets which are not mentioned in the response
> > +will be probed individually, just as if the @samp{qSupported} query
> > +was not supported.  In the future, some new packets may be added to
> > +the remote protocol which will be assumed to be unsupported unless
> > +@samp{qSupported} is supported and the new packet is reported in the
> > +@samp{qSupported} response.  Any such packets will be listed here,
> > +and their documentation will refer to @samp{qSupported}.
> > +
> 
> It seems to me that GDB should assume that if a packet isn't
> mentioned, it's not supported.  My thinking is:
> 
> - As GDB grows new packets over time, if GDB does not assume these new
>   packets are unsupported if unmentioned, then we'll grow a new series
>   of round trips with older stubs that do support the qSupported
>   packet.  If we're going to go to all this trouble, let's get rid of
>   these round trips once and for all.
> 
> - Under the rule I'm suggesting (call it "unmentioned means
>   unimplemented"?), if a stub supports a new packet but doesn't
>   mention it, GDB will never send it.  So stub implementors have an
>   incentive and a reminder to keep their qSupported responses up to
>   date.

I do not think that it is feasible to require an existing stub, when
adding qSupported support, to list the exact set of packets it
supports currently.  Also, it will make the reply much bigger, for
packets which might not be used in this session.  Do you think
it's worth it?  I could be persuaded, but I'm currently leaning
against; I'll explain my reasoning.

As a basis for comparison, gdbserver currently supports six packets
with long names, and roughly 28 packets with short names.  The reply
would be about two hundred bytes - not huge, but a bit unwieldy to
maintain.

I'm thinking of things like qGetTLSAddr here.  When GDB needs it, it
will try it.  If the program being debugged needs TLS, then the stub
supports an environment including TLS, and is likely to support
fetching the TLS addr.  On the other hand, if it doesn't, the user's
asked us to make a round trip to the stub and we've done so - it's
just that we came back with an error message.

For things like qOffsets, the story is a bit different.  That's
something we send unsolicited at connection, and it's not necessarily
fatal to what we're doing if it's missing.  So there's a clear
advantage in mentioning it - that's why I implemented qSupported
support for it as my example.

That's why I implemented an option to choose an intelligent default for
new packets.  If it's something that will show up in transcripts with
other stubs, and its loss is survivable, then requiring it to be
mentioned saves on roundtrips.

I intend to use the "only supported if mentioned" path ruthlessly for
new packets.  Let's see, queued up in various trees (and in various
states - don't bank on all these being submitted and useful):

  - A qOffsets replacement with a different reply format, designed for
    ELF segments rather than sections.  This is a startup packet,
    so the round trip is important.

  - The next generation of qPart:features.  Also a startup packet.

  - A "make the remote stub shut down" packet, useful in extended mode.
    There'd have to be a user command for this, so the round trip
    seems ignorable; if it fails, we'll let the user know.

  - Additional commands for running programs with arguments in extended
    mode and attaching to remote packets.  Again, only sent in response
    to user request, so if the stub doesn't support it, we can find out
    at that point.

The first two should definitely be assumed missing if they are not
supported.  The last two could be either.  We could set a policy that
says "we require use of qSupported for any new packets", though, and
that wouldn't hurt either.  Simply keep a list in the manual of which
are which, and save on "m+;M+;c+".

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PROPOSAL] Checking for supported packets - revised

[Jim Blandy]

Daniel Jacobowitz <drow@false.org> writes:
> On Wed, May 10, 2006 at 03:15:17PM -0700, Jim Blandy wrote:
>> It seems to me that GDB should assume that if a packet isn't
>> mentioned, it's not supported.  My thinking is:
>> 
>> - As GDB grows new packets over time, if GDB does not assume these new
>>   packets are unsupported if unmentioned, then we'll grow a new series
>>   of round trips with older stubs that do support the qSupported
>>   packet.  If we're going to go to all this trouble, let's get rid of
>>   these round trips once and for all.
>> 
>> - Under the rule I'm suggesting (call it "unmentioned means
>>   unimplemented"?), if a stub supports a new packet but doesn't
>>   mention it, GDB will never send it.  So stub implementors have an
>>   incentive and a reminder to keep their qSupported responses up to
>>   date.
>
> I do not think that it is feasible to require an existing stub, when
> adding qSupported support, to list the exact set of packets it
> supports currently.  Also, it will make the reply much bigger, for
> packets which might not be used in this session.  Do you think
> it's worth it?  I could be persuaded, but I'm currently leaning
> against; I'll explain my reasoning.
>
> As a basis for comparison, gdbserver currently supports six packets
> with long names, and roughly 28 packets with short names.  The reply
> would be about two hundred bytes - not huge, but a bit unwieldy to
> maintain.

Assuming packets are more often added than deleted, it doesn't seem
like a maintenance problem to me: any mistakes will get caught
quickly; you can't test the packet without fixing them.

> I'm thinking of things like qGetTLSAddr here.  When GDB needs it, it
> will try it.  If the program being debugged needs TLS, then the stub
> supports an environment including TLS, and is likely to support
> fetching the TLS addr.  On the other hand, if it doesn't, the user's
> asked us to make a round trip to the stub and we've done so - it's
> just that we came back with an error message.
>
> For things like qOffsets, the story is a bit different.  That's
> something we send unsolicited at connection, and it's not necessarily
> fatal to what we're doing if it's missing.  So there's a clear
> advantage in mentioning it - that's why I implemented qSupported
> support for it as my example.
>
> That's why I implemented an option to choose an intelligent default for
> new packets.  If it's something that will show up in transcripts with
> other stubs, and its loss is survivable, then requiring it to be
> mentioned saves on roundtrips.
>
> I intend to use the "only supported if mentioned" path ruthlessly for
> new packets.  Let's see, queued up in various trees (and in various
> states - don't bank on all these being submitted and useful):
>
>   - A qOffsets replacement with a different reply format, designed for
>     ELF segments rather than sections.  This is a startup packet,
>     so the round trip is important.
>
>   - The next generation of qPart:features.  Also a startup packet.
>
>   - A "make the remote stub shut down" packet, useful in extended mode.
>     There'd have to be a user command for this, so the round trip
>     seems ignorable; if it fails, we'll let the user know.
>
>   - Additional commands for running programs with arguments in extended
>     mode and attaching to remote packets.  Again, only sent in response
>     to user request, so if the stub doesn't support it, we can find out
>     at that point.
>
> The first two should definitely be assumed missing if they are not
> supported.  The last two could be either.  We could set a policy that
> says "we require use of qSupported for any new packets", though, and
> that wouldn't hurt either.  Simply keep a list in the manual of which
> are which, and save on "m+;M+;c+".

Okay --- I hadn't thought of making that distinction.  If we go that
way, then the protocol documentation should also explain how to decide
whether to make GDB treat a new packet as "only supported if
mentioned" or "try if unmentioned".  Having the rule in your head
alone is no good! :)

Re: [PROPOSAL] Checking for supported packets - revised

[Daniel Jacobowitz]

On Wed, May 10, 2006 at 05:19:28PM -0700, Jim Blandy wrote:
> Okay --- I hadn't thought of making that distinction.  If we go that
> way, then the protocol documentation should also explain how to decide
> whether to make GDB treat a new packet as "only supported if
> mentioned" or "try if unmentioned".  Having the rule in your head
> alone is no good! :)

I certainly meant to document this intention; if it was lacking, I'll
fix it.

I've been having second thoughts about the scheme.  When I showed this
to Sandra (one of our coworkers at CodeSourcery, for those reading
along) her primary reaction was that it was too complicated - too
many cases.  Here's a definition with the minimal number of cases
that I can manage:

- If your stub responds to qSupported, the response must include
  every packet it supports.
- If your stub does not respond to qSupported, every packet is
  probed.

I don't think that one is usable, because of the widespread existance
of stubs which don't know about packets we haven't invented yet -
they'd still have startup time grow with time.  The next step up the
complexity ladder is:

- If your stub responds to qSupported, the response must include
  every packet it supports.
- If your stub does not respond to qSupported, only a fixed list
  of packets will be used or probed for.  So 'qOffsets' will still
  be tried, but the new qFooBaz (added after qSupported) will be
  assumed missing.

The next step is what I posted:

- qSupported can report present or absent packets.
- Some packets must be mentioned in order to be supported.  Others
  will be probed for if not mentioned.
- If your stub does not respond to qSupported, only a fixed list
  of packets will be used or probed for.

So which of these is best?

The qSupported response for gdbserver would currently be on the order
of 200 bytes if all packets were mentioned.  It'll grow with time, too;
the more complex a stub, the larger a qSupported response it will have
to send.  I can easily envision one getting up to 400-500 bytes.
That's not too bad, even over an extremely slow link, but it's more
bytes than the current startup probing overhead by far.

If only some packets are required to be mentioned in qSupported rather
than all, along the lines of my previous message, then the size of the
qSupported response will still grow, but more slowly.  And the total
size of the packet will be much smaller as things like "g" and "Hc" are
omitted.

The tradeoff is in the complexity of the documentation; hopefully
a sufficiently clear manual will mitigate that, and some of the
clarifications that I added to this posting did in fact reassure
Sandra that it would be manageable for stub implementors.

I think a blurb up front of the remote protocol chapter referring to
this where it talks about packets which must be implemented would help.
(And what's there is quite bogus at the moment anyway.)

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PROPOSAL] Checking for supported packets - revised

[PAUL GILLIAM]

On Fri, 2006-05-12 at 09:24 -0400, Daniel Jacobowitz wrote:
> - If your stub responds to qSupported, the response must include
>   every packet it supports.
> - If your stub does not respond to qSupported, only a fixed list
>   of packets will be used or probed for.  So 'qOffsets' will still
>   be tried, but the new qFooBaz (added after qSupported) will be
>   assumed missing.

I would vote for this one.  In essence it says "we will cut some
slack to stubs too old to have 'qSupported', but we expect it to
be there for newer stubs".

This sound reasonable to me.

-=# Paul #=-

Data for: [PROPOSAL] Checking for supported packets - revised

[Daniel Jacobowitz]

On Fri, May 12, 2006 at 09:24:50AM -0400, Daniel Jacobowitz wrote:
> I've been having second thoughts about the scheme.  When I showed this
> to Sandra (one of our coworkers at CodeSourcery, for those reading
> along) her primary reaction was that it was too complicated - too
> many cases.

Coming to a conclusion on which one of these options was "best" is not
coming along well; Jim and I and some other coworkers talked about it
for a long while yesterday, and ended up waffling.  Mark suggested that
I put together some hard numbers, so that we could see what we were
really dealing with.

The raw data's down below, but here are some conclusions first.

One of the tests (compare testcases "features" and "smallpkt") shows
that negotiating a large packet size is very helpful: between 20% on a
fast, low latency link, and 300% on a high latency link.  So, that part
is worth including.

The currently unused feature to tell the stub "the GDB client supports
this new protocol feature" from my previous posting is still valuable,
because that wasn't performance related.

As far as performance, compare the "+two" cases with those directly
above them to see the effect of adding two new packet probes
to an existing startup sequence, either already chatty (gdbserver)
or bare metal (stub).  The higher the latency of the link, the higher
the performance cost, naturally.  But for low latency links (serial,
or ethernet LAN) the cost was quite small.

So, avoiding the probes is less important than I thought.  It's
still not completely ignorable.  Jim made some interesting points
about a loss of flexibility by relying on this feature widely.
I have a new proposal, which I will post in a second - this message is
too large already.


The table, with explanations further down:

           sirius   local   nevyn   jahdo    9600   115200

gdbserver  3.311    0.730   0.615   0.380   2.694    0.258
 +two      3.946    0.728   0.862   0.430   2.775    0.269
stub       2.458    0.474   0.518   0.210   0.310    0.042
 +two      2.459    0.384   0.601   0.260   0.382    0.047
features   5.890    0.439   1.256   0.711   9.521    0.851
smallpkt  15.358    2.913   3.628   1.912  11.254    1.053




My notes from testing:

Over TCP, there is definitely a high per-packet cost; if you know anything
about TCP and IP, this makes perfect sense.  Not only is there negotiation
and round trip time involved, but also a lot of extra bytes per packet for
the headers.  When I started testing, gdbreplay did single byte writes,
which translated into single byte TCP packets.  I changed it to write the
whole remote response into a single TCP packet.  Since in a real stub the
ack is usually generated immediately and the packet response much later, I
wrote my test files to send the ack in one write and the response in a
second one.

Testing methodology:

I ran two gdbreplay binaries on symmetric log files, talking to each other. 
Over TCP this involved a couple of netcat binaries to initiate the
connection, since that was simpler than teaching gdbreplay to call
connect().  For TCP, I timed the netcats; for serial, I timed the
gdbreplay session which started with a write (corresponding to the GDB
client).  Timing was done using the 'time' shell builtin; of course, I
was interested in wall clock time, not user or system time.

I also added serial support to gdbreplay (copied from gdbserver).

I ran each test about three times and took the middle time; just eyeballing
here, not being too statistically careful.  The long distance TCP link,
especially, was very noisy.

These were my tests:

1. gdbserver-handshake.txt

I took an x86_64-linux GDB and gdbserver, debugging /bin/cat, and logged
"target remote".  Current, HEAD versions.  The current case for a hosted
system.

2. gdbserver-twomore.txt

I took gdbserver-handshake.txt and manually added two new unsupported
packets that would get probed during connection.

3. stub-shake.txt

I took an arm-none-eabi GDB and connected it to the CodeSourcery RDI stub.
This actually generated the next trace, but I hand-edited it to what it
would have looked like if I'd used a HEAD GDB.  No binary was loaded.

4. stub-twomore.txt

I took stub-shake.txt and manually added two new unsupported
packets that would get probed during connection.

5.  features-shake.txt

I took CodeSourcery's 2006-Q1 arm-none-eabi-gdb and connected it to the
CodeSourcery RDI stub.  This uses qPacketInfo to set a packet size maximum
of 16K and suppress qOffsets, but adds qPart:features and transfers a
good-sized chunk of XML.

6.  features-smallpacket.txt

Same as the previous test, except instead of 16K packet max I used 256 bytes
for the maximum size.  This caused the XML files to be split into much
smaller packets (and many more of them).  And the additional request
packets took more total bytes.  About 10% more bytes and three times as many
packets.


DATA

The rows of these tables are the six tests above.  The columns are testing
configurations:

1. sirius: SSH tunnel from my local system to a machine across the country.
2. localhost: TCP over localhost, Linux
3. nevyn: TCP over 100MBit LAN, Linux
4. jahdo to nevyn: TCP over wireless 802.11g, Cygwin to Linux
5. caradoc to nevyn: Serial 9600 baud connection
6. caradoc to nevyn: Serial 115200 baud connection

           sirius   local   nevyn   jahdo    9600   115200

gdbserver  3.311    0.730   0.615   0.380   2.694    0.258
 +two      3.946    0.728   0.862   0.430   2.775    0.269
stub       2.458    0.474   0.518   0.210   0.310    0.042
 +two      2.459    0.384   0.601   0.260   0.382    0.047
features   5.890    0.439   1.256   0.711   9.521    0.851
smallpkt  15.358    2.913   3.628   1.912  11.254    1.053




-- 
Daniel Jacobowitz
CodeSourcery

Take three: [PROPOSAL] Checking for supported packets

[Daniel Jacobowitz]

On Tue, May 23, 2006 at 02:31:55PM -0400, Daniel Jacobowitz wrote:
> Coming to a conclusion on which one of these options was "best" is not
> coming along well; Jim and I and some other coworkers talked about it
> for a long while yesterday, and ended up waffling.  Mark suggested that
> I put together some hard numbers, so that we could see what we were
> really dealing with.

It's coming along much better now :-)  I'd like to thank my coworkers,
particularly Jim, Sandra, and Paul, for their feedback.  I am
completely happy with this version.  It's changed a bit: it
addresses a much more restrictive problem, rather than automatically
applying to all packets.  Also the documentation is much clearer.

This is a plaintext version of my current proposal; I'll post the
texinfo separately later - the nested tables make it very hard for
me to look over the text and get the structure.  qXfer:features
is just an example; I wanted to fill in the table properly, but
I haven't fully proposed the qXfer:features packet yet.  That
will be along shortly.

I'm planning to go ahead with this version, unless of course anyone has
concerns about it.  It's going to require a bit of a revamp of my code,
though, so I won't be doing that until next week.  As always, I am
entirely appreciative of feedback, even if you don't like my choices
:-)  This version is, IMO, much superior to the first version I posted
a month ago (although it's taken far too long for comfort).

`qSupported [:GDBFEATURE [;GDBFEATURE]... ]'
     Tell the remote stub about features supported by GDB, and query
     the stub for features it supports.  This packet allows GDB and the
     remote stub to take advantage of each others' features.
     `qSupported' also consolidates multiple feature probes at startup,
     to improve GDB performance--a single larger packet performs better
     than multiple smaller probe packets on high-latency links.  Some
     features may enable behavior which must not be on by default, e.g.
     because it would confuse older clients or stubs.  Other features
     may describe packets which could be automatically probed for, but
     are not.  These features must be reported before GDB will use
     them.  This "default unsupported" behavior is not appropriate for
     all packets, but it helps to keep the initial connection time
     under control with new versions of GDB which support increasing
     numbers of packets.

     Reply:
    `STUBFEATURE [;STUBFEATURE]...'
          The stub supports or does not support each returned
          STUBFEATURE, depending on the form of each STUBFEATURE (see
          below for the possible forms).

    `'
          An empty reply indicates that `qSupported' is not recognized,
          or that no features needed to be reported to GDB.

     The allowed forms for each feature (either a GDBFEATURE in the
     `qSupported' packet, or a STUBFEATURE in the response) are:

    `NAME=VALUE'
          The remote protocol feature NAME is supported, and associated
          with the specified VALUE.  The format of VALUE depends on the
          feature, but it must not include a semicolon.

    `NAME+'
          The remote protocol feature NAME is supported, and does not
          need an associated value.

    `NAME-'
          The remote protocol feature NAME is not supported.

    `NAME?'
          The remote protocol feature NAME may be supported, and GDB
          should auto-detect support in some other way when it is
          needed.  This form will not be used for GDBFEATURE
          notifications, but may be used for STUBFEATURE responses.

     Whenever the stub receives a `qSupported' request, the supplied
     set of GDB features should override any previous request.  This
     allows GDB to put the stub in a known state, even if the stub had
     previously been communicating with a different version of GDB.

     No values of GDBFEATURE (for the packet sent by GDB) are defined
     yet.  Stubs should ignore any unknown values for GDBFEATURE.  Any
     GDB which sends a `qSupported' packet supports receiving packets
     of unlimited length (earlier versions of GDB may reject overly
     long responses).  Values for GDBFEATURE may be defined in the
     future to let the stub take advantage of new features in GDB, e.g.
     incompatible improvements in the remote protocol--support for
     unlimited length responses would be a GDBFEATURE example, if it
     were not implied by the `qSupported' query.

     Similarly, GDB will silently ignore unrecognized stub feature
     responses, as long as each response uses one of the standard forms.

     Some features are flags.  A stub which supports a flag feature
     should respond with a `+' form response.  Other features require
     values, and the stub should respond with an `=' form response.

     Each feature has a default value, which GDB will use if
     `qSupported' is not available or if the feature is not mentioned
     in the `qSupported' response.  The default values are fixed; a
     stub is free to omit any feature responses that match the defaults.

     Not all features can be probed, but for those which can, the
     probing mechanism is useful: in some cases, a stub's internal
     architecture may not allow the protocol layer to know some
     information about the underlying target in advance.  This is
     especially common in stubs which may be configured for multiple
     targets.

     These are the currently defined stub features and their properties:

     Packet Name       Value         Default       Probe Allowed
                       Required                    
     `PacketSize'      Yes           `-'           No
     `qXfer:features'  No            `-'           Yes

     These are the currently defined stub features, in more detail:

    `PacketSize=BYTES'
          The remote stub can accept packets up to at least BYTES in
          length.  GDB will send packets up to this size for bulk
          transfers, and will never send larger packets.  This is a
          limit on the data characters in the packet, including the
          frame and checksum.  There is no trailing NUL byte in a
          remote protocol packet; if the stub stores packets in a
          NUL-terminated format, it should allow an extra byte in its
          buffer for the NUL.  If this stub feature is not supported,
          GDB guesses based on the size of the `g' packet response.

    `qXfer:features'
          The remote stub can respond to the the `qXfer:features' query
          (*note `qXfer:features': qXfer features.).


-- 
Daniel Jacobowitz
CodeSourcery