* [remote] Checking for supported packets @ 2006-03-14 2:15 Daniel Jacobowitz 2006-03-21 14:28 ` Daniel Jacobowitz 0 siblings, 1 reply; 18+ messages in thread From: Daniel Jacobowitz @ 2006-03-14 2:15 UTC (permalink / raw) To: gdb 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [remote] Checking for supported packets 2006-03-14 2:15 [remote] Checking for supported packets Daniel Jacobowitz @ 2006-03-21 14:28 ` Daniel Jacobowitz 2006-03-22 16:39 ` Paul Koning 2006-03-31 6:38 ` [PROPOSAL] " Daniel Jacobowitz 0 siblings, 2 replies; 18+ messages in thread From: Daniel Jacobowitz @ 2006-03-21 14:28 UTC (permalink / raw) To: gdb 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [remote] Checking for supported packets 2006-03-21 14:28 ` Daniel Jacobowitz @ 2006-03-22 16:39 ` Paul Koning 2006-03-31 6:38 ` [PROPOSAL] " Daniel Jacobowitz 1 sibling, 0 replies; 18+ messages in thread From: Paul Koning @ 2006-03-22 16:39 UTC (permalink / raw) To: drow; +Cc: gdb >>>>> "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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PROPOSAL] Checking for supported packets 2006-03-21 14:28 ` Daniel Jacobowitz 2006-03-22 16:39 ` Paul Koning @ 2006-03-31 6:38 ` Daniel Jacobowitz 2006-03-31 9:51 ` Eli Zaretskii 1 sibling, 1 reply; 18+ messages in thread From: Daniel Jacobowitz @ 2006-03-31 6:38 UTC (permalink / raw) To: gdb 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PROPOSAL] Checking for supported packets 2006-03-31 6:38 ` [PROPOSAL] " Daniel Jacobowitz @ 2006-03-31 9:51 ` Eli Zaretskii 2006-03-31 14:09 ` Daniel Jacobowitz 0 siblings, 1 reply; 18+ messages in thread From: Eli Zaretskii @ 2006-03-31 9:51 UTC (permalink / raw) To: gdb > 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.) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PROPOSAL] Checking for supported packets 2006-03-31 9:51 ` Eli Zaretskii @ 2006-03-31 14:09 ` Daniel Jacobowitz [not found] ` <uvetuaep4.fsf@gnu.org> 0 siblings, 1 reply; 18+ messages in thread From: Daniel Jacobowitz @ 2006-03-31 14:09 UTC (permalink / raw) To: gdb 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <uvetuaep4.fsf@gnu.org>]
[parent not found: <20060331141958.GA28073@nevyn.them.org>]
* Re: [PROPOSAL] Checking for supported packets [not found] ` <20060331141958.GA28073@nevyn.them.org> @ 2006-04-01 10:22 ` Eli Zaretskii 2006-05-10 7:21 ` [PROPOSAL] Checking for supported packets - revised Daniel Jacobowitz 0 siblings, 1 reply; 18+ messages in thread From: Eli Zaretskii @ 2006-04-01 10:22 UTC (permalink / raw) To: gdb > 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''. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PROPOSAL] Checking for supported packets - revised 2006-04-01 10:22 ` Eli Zaretskii @ 2006-05-10 7:21 ` Daniel Jacobowitz 2006-05-10 18:44 ` Eli Zaretskii 2006-05-11 0:19 ` Jim Blandy 0 siblings, 2 replies; 18+ messages in thread From: Daniel Jacobowitz @ 2006-05-10 7:21 UTC (permalink / raw) To: gdb 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PROPOSAL] Checking for supported packets - revised 2006-05-10 7:21 ` [PROPOSAL] Checking for supported packets - revised Daniel Jacobowitz @ 2006-05-10 18:44 ` Eli Zaretskii 2006-05-10 21:49 ` Daniel Jacobowitz 2006-05-11 0:19 ` Jim Blandy 1 sibling, 1 reply; 18+ messages in thread From: Eli Zaretskii @ 2006-05-10 18:44 UTC (permalink / raw) To: gdb > 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PROPOSAL] Checking for supported packets - revised 2006-05-10 18:44 ` Eli Zaretskii @ 2006-05-10 21:49 ` Daniel Jacobowitz 2006-05-11 6:02 ` Eli Zaretskii 0 siblings, 1 reply; 18+ messages in thread From: Daniel Jacobowitz @ 2006-05-10 21:49 UTC (permalink / raw) To: gdb 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PROPOSAL] Checking for supported packets - revised 2006-05-10 21:49 ` Daniel Jacobowitz @ 2006-05-11 6:02 ` Eli Zaretskii 0 siblings, 0 replies; 18+ messages in thread From: Eli Zaretskii @ 2006-05-11 6:02 UTC (permalink / raw) To: gdb > 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PROPOSAL] Checking for supported packets - revised 2006-05-10 7:21 ` [PROPOSAL] Checking for supported packets - revised Daniel Jacobowitz 2006-05-10 18:44 ` Eli Zaretskii @ 2006-05-11 0:19 ` Jim Blandy 2006-05-11 2:26 ` Daniel Jacobowitz 1 sibling, 1 reply; 18+ messages in thread From: Jim Blandy @ 2006-05-11 0:19 UTC (permalink / raw) To: gdb 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PROPOSAL] Checking for supported packets - revised 2006-05-11 0:19 ` Jim Blandy @ 2006-05-11 2:26 ` Daniel Jacobowitz 2006-05-11 2:36 ` Jim Blandy 0 siblings, 1 reply; 18+ messages in thread From: Daniel Jacobowitz @ 2006-05-11 2:26 UTC (permalink / raw) To: gdb 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PROPOSAL] Checking for supported packets - revised 2006-05-11 2:26 ` Daniel Jacobowitz @ 2006-05-11 2:36 ` Jim Blandy 2006-05-12 13:55 ` Daniel Jacobowitz 0 siblings, 1 reply; 18+ messages in thread From: Jim Blandy @ 2006-05-11 2:36 UTC (permalink / raw) To: gdb 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! :) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PROPOSAL] Checking for supported packets - revised 2006-05-11 2:36 ` Jim Blandy @ 2006-05-12 13:55 ` Daniel Jacobowitz 2006-05-12 18:24 ` PAUL GILLIAM 2006-05-23 22:11 ` Data for: " Daniel Jacobowitz 0 siblings, 2 replies; 18+ messages in thread From: Daniel Jacobowitz @ 2006-05-12 13:55 UTC (permalink / raw) To: gdb 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PROPOSAL] Checking for supported packets - revised 2006-05-12 13:55 ` Daniel Jacobowitz @ 2006-05-12 18:24 ` PAUL GILLIAM 2006-05-23 22:11 ` Data for: " Daniel Jacobowitz 1 sibling, 0 replies; 18+ messages in thread From: PAUL GILLIAM @ 2006-05-12 18:24 UTC (permalink / raw) To: Daniel Jacobowitz; +Cc: gdb 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 #=- ^ permalink raw reply [flat|nested] 18+ messages in thread
* Data for: [PROPOSAL] Checking for supported packets - revised 2006-05-12 13:55 ` Daniel Jacobowitz 2006-05-12 18:24 ` PAUL GILLIAM @ 2006-05-23 22:11 ` Daniel Jacobowitz 2006-05-26 22:12 ` Take three: [PROPOSAL] Checking for supported packets Daniel Jacobowitz 1 sibling, 1 reply; 18+ messages in thread From: Daniel Jacobowitz @ 2006-05-23 22:11 UTC (permalink / raw) To: gdb 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Take three: [PROPOSAL] Checking for supported packets 2006-05-23 22:11 ` Data for: " Daniel Jacobowitz @ 2006-05-26 22:12 ` Daniel Jacobowitz 0 siblings, 0 replies; 18+ messages in thread From: Daniel Jacobowitz @ 2006-05-26 22:12 UTC (permalink / raw) To: gdb 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2006-05-26 18:42 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2006-03-14 2:15 [remote] Checking for supported packets Daniel Jacobowitz 2006-03-21 14:28 ` Daniel Jacobowitz 2006-03-22 16:39 ` Paul Koning 2006-03-31 6:38 ` [PROPOSAL] " Daniel Jacobowitz 2006-03-31 9:51 ` Eli Zaretskii 2006-03-31 14:09 ` Daniel Jacobowitz [not found] ` <uvetuaep4.fsf@gnu.org> [not found] ` <20060331141958.GA28073@nevyn.them.org> 2006-04-01 10:22 ` Eli Zaretskii 2006-05-10 7:21 ` [PROPOSAL] Checking for supported packets - revised Daniel Jacobowitz 2006-05-10 18:44 ` Eli Zaretskii 2006-05-10 21:49 ` Daniel Jacobowitz 2006-05-11 6:02 ` Eli Zaretskii 2006-05-11 0:19 ` Jim Blandy 2006-05-11 2:26 ` Daniel Jacobowitz 2006-05-11 2:36 ` Jim Blandy 2006-05-12 13:55 ` Daniel Jacobowitz 2006-05-12 18:24 ` PAUL GILLIAM 2006-05-23 22:11 ` Data for: " Daniel Jacobowitz 2006-05-26 22:12 ` Take three: [PROPOSAL] Checking for supported packets Daniel Jacobowitz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).