Page MenuHomeFreeBSD

gdb(4): Implement support for NoAckMode
ClosedPublic

Authored by cem on Sep 22 2019, 10:11 PM.

Details

Summary

When the underlying debugport transport is reliable, GDB's additional
checksums and acknowledgements come at some mild CPU cost, and more
importantly, additional RTTs. NoAckMode eliminates the redundant RTTs and
allows us to skip checking RX checksums. The GDB packet framing does not
change, so checksums are still included as message trailers.

The gdb(4) stub in FreeBSD advertises support for the feature in response to
the client's 'qSupported' request IFF the current debugport has the
gdb_dbfeatures flag GDB_DBGP_FEAT_RELIABLE set. Currently, only netgdb(4)
supports this feature.

If the remote GDB client supports the feature and does not have it disabled
via a GDB configuration knob, it may instruct our gdb(4) stub to enter
NoAckMode. Unless and until it issues that command, we must continue to
transmit acks as usual (and for now, we continue to wait until we receive
them as well, even if we know the debugport is on a reliable transport).

In the kernel sources, the sense of the flag representing the state of the
feature is reversed from that of the GDB command. (I.e., it is
'gdb_ackmode', not 'gdb_noackmode.') This is to avoid confusing double-
negative conditions.

For reference, see:

Test Plan

DEBUG output from the Python NetGDB proxy:

Waiting for connection from GDB client on port 8000.
Use 'target remote <ip>:8000' from gdb to connect.
Connection from GDB client received.
...
2019-09-22 15:02:10,846 - DEBUG - Received tcp data: +$qSupported:multiprocess+;swbreak+;hwbreak+;qRelocInsn+;fork-events+;vfork-events+;exec-events+;vContSupported+;QThreadEvents+;no-resumed+;xmlRegisters=i386#6a
2019-09-22 15:02:10,846 - DEBUG - Sending packet + to ...:20026.
2019-09-22 15:02:10,847 - DEBUG - Sending packet $qSupported:multiprocess+;swbreak+;hwbreak+;qRelocInsn+;fork-events+;vfork-events+;exec-events+;vContSupported+;QThreadEvents+;no-resumed+;xmlRegisters=i386#6a to ...:20026.
2019-09-22 15:02:10,848 - DEBUG - Received udp data packet: +
2019-09-22 15:02:10,849 - DEBUG - Received udp data packet: $PacketSize=1003;qXfer:threads:read+;QStartNoAckMode+#71
2019-09-22 15:02:10,955 - DEBUG - Received tcp data: +$vMustReplyEmpty#3a
2019-09-22 15:02:10,955 - DEBUG - Sending packet + to ...:20026.
2019-09-22 15:02:10,955 - DEBUG - Sending packet $vMustReplyEmpty#3a to ...:20026.
2019-09-22 15:02:10,957 - DEBUG - Received udp data packet: +
2019-09-22 15:02:10,958 - DEBUG - Received udp data packet: $#00
2019-09-22 15:02:11,063 - DEBUG - Received tcp data: +$QStartNoAckMode#b0
2019-09-22 15:02:11,063 - DEBUG - Sending packet + to ...:20026.
2019-09-22 15:02:11,063 - DEBUG - Sending packet $QStartNoAckMode#b0 to ...:20026.
2019-09-22 15:02:11,065 - DEBUG - Received udp data packet: +
2019-09-22 15:02:11,066 - DEBUG - Received udp data packet: $OK#9a
// Below is the final ACK ("+") GDB client sends, in response to NetGDB "OK", in response to GDB's "QStackNoAckMode."
2019-09-22 15:02:11,171 - DEBUG - Received tcp data: +$Hg0#df
2019-09-22 15:02:11,171 - DEBUG - Sending packet + to ...:20026.
2019-09-22 15:02:11,171 - DEBUG - Sending packet $Hg0#df to ...:20026.
// Below is first NetGDB client response without preceeding ACK
2019-09-22 15:02:11,172 - DEBUG - Received udp data packet: $OK#9a
// Followed by first GDB server command without preceeding ACK
2019-09-22 15:02:11,173 - DEBUG - Received tcp data: $qTStatus#49
// Followed by no more redundant ACKs.
...

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

cem created this revision.Sep 22 2019, 10:11 PM
cem updated this revision to Diff 62450.Sep 23 2019, 12:42 AM

Rebase on parent change. No functional change.

emaste added a subscriber: jhb.Sep 27 2019, 1:02 PM
jhb accepted this revision.Sep 27 2019, 7:34 PM

Strictly speaking the helpers are kind of redundant as all but one call are already under a 'if (gdb_ackmode)' check.

This revision is now accepted and ready to land.Sep 27 2019, 7:34 PM
cem added a comment.Sep 27 2019, 10:36 PM
In D21761#476464, @jhb wrote:

Strictly speaking the helpers are kind of redundant as all but one call are already under a 'if (gdb_ackmode)' check.

Yeah, that's very true. I added them before I had quantified how few places actually responded with acks. I would be happy to remove that change if you prefer.

markj accepted this revision.Sep 30 2019, 7:38 PM
markj added inline comments.
sys/gdb/gdb_main.c
773 ↗(On Diff #62450)

Add braces to keep consistency?

780 ↗(On Diff #62450)

Wrong style for multi-line comments.

cem added a comment.Oct 2 2019, 3:28 AM

Thanks!

sys/gdb/gdb_main.c
773 ↗(On Diff #62450)

I didn't really add this line, diff just ... borrowed it instead of showing the identical tx_empty/break lines I added below, at l. 787-788. The logical diff starts at case 'Q': and ends at break;.

(If anything, I'd remove the excess braces elsewhere in the compound statement. But since I only added the Q case in this diff I'd rather leave q alone.)

780 ↗(On Diff #62450)

Will fix, not sure what I was thinking.

markj added inline comments.Oct 2 2019, 5:13 PM
sys/gdb/gdb_main.c
773 ↗(On Diff #62450)

Oh, I missed that. Fair enough!

cem updated this revision to Diff 62914.Oct 4 2019, 6:16 PM
cem marked an inline comment as done.

Fix comment style

This revision now requires review to proceed.Oct 4 2019, 6:16 PM
This revision was not accepted when it landed; it landed in state Needs Review.Thu, Oct 17, 10:37 PM
This revision was automatically updated to reflect the committed changes.