The branch main has been updated by bdrewery:
URL:
https://cgit.FreeBSD.org/src/commit/?id=b498331b037f20bde90b46fc37678fbd421dcf09
commit b498331b037f20bde90b46fc37678fbd421dcf09
Author: John Reimer <john_rei...@dell.com>
AuthorDate: 2023-05-27 02:48:14 +0000
Commit: Bryan Drewery <bdrew...@freebsd.org>
CommitDate: 2023-05-27 16:34:17 +0000
netgdb: Fix netgdb double ack, print proxy address
- Fixes netgdb's double ack
- Moving ack responsibility to debugnet, decision to ack made by
netdump/netgdb.
- Finish responsibility moved to debugnet, new finish handler.
- netgdb now prints the address to connect to in case the user doesn't have
access to the proxy machine.
Sponsored by: Dell EMC
Reviewed By: markj, bdrewery (earlier version)
Differential Revision: https://reviews.freebsd.org/D40064
---
sys/gdb/netgdb.c | 76 ++++++++++++++++++++------------------------------
sys/net/debugnet.c | 57 ++++++++++++++++++++++++++++---------
sys/net/debugnet.h | 15 +++++++++-
sys/net/debugnet_int.h | 6 ++--
4 files changed, 93 insertions(+), 61 deletions(-)
diff --git a/sys/gdb/netgdb.c b/sys/gdb/netgdb.c
index dc426adea766..58e02bb97d28 100644
--- a/sys/gdb/netgdb.c
+++ b/sys/gdb/netgdb.c
@@ -57,6 +57,7 @@ __FBSDID("$FreeBSD$");
#error "NetGDB cannot be used without DDB at this time"
#endif
+#include <sys/errno.h>
#include <sys/param.h>
#include <sys/kdb.h>
#include <sys/sbuf.h>
@@ -110,64 +111,27 @@ static int *netgdb_prev_kdb_inactive;
/* TODO(CEM) disable ack mode */
/*
- * Receive non-TX ACK packets on the client port.
+ * Attempt to accept the incoming packet. If we run into ENOBUFS or another
+ * error, return it.
*
- * The mbuf chain will have all non-debugnet framing headers removed
- * (ethernet, inet, udp). It will start with a debugnet_msg_hdr, of
- * which the header is guaranteed to be contiguous. If m_pullup is
- * used, the supplied in-out mbuf pointer should be updated
- * appropriately.
- *
- * If the handler frees the mbuf chain, it should set the mbuf pointer
- * to NULL. Otherwise, the debugnet input framework will free the
- * chain.
+ * The mbuf chain will have all framing headers removed (ethernet, inet, udp,
+ * debugnet).
*/
-static void
-netgdb_rx(struct debugnet_pcb *pcb, struct mbuf **mb)
+static int
+netgdb_rx(struct mbuf *m)
{
- const struct debugnet_msg_hdr *dnh;
- struct mbuf *m;
uint32_t rlen, count;
- int error;
-
- m = *mb;
- dnh = mtod(m, const void *);
-
- if (ntohl(dnh->mh_type) == DEBUGNET_FINISHED) {
- sbuf_putc(&netgdb_rxsb, CTRL('C'));
- return;
- }
-
- if (ntohl(dnh->mh_type) != DEBUGNET_DATA) {
- printf("%s: Got unexpected debugnet message %u\n",
- __func__, ntohl(dnh->mh_type));
- return;
- }
- rlen = ntohl(dnh->mh_len);
+ rlen = m->m_pkthdr.len;
#define _SBUF_FREESPACE(s) ((s)->s_size - ((s)->s_len + 1))
if (_SBUF_FREESPACE(&netgdb_rxsb) < rlen) {
NETGDB_DEBUG("Backpressure: Not ACKing RX of packet that "
"would overflow our buffer (%zd/%zd used).\n",
netgdb_rxsb.s_len, netgdb_rxsb.s_size);
- return;
+ return (ENOBUFS);
}
#undef _SBUF_FREESPACE
- error = debugnet_ack_output(pcb, dnh->mh_seqno);
- if (error != 0) {
- printf("%s: Couldn't ACK rx packet %u; %d\n", __func__,
- ntohl(dnh->mh_seqno), error);
- /*
- * Sender will re-xmit, and assuming the condition is
- * transient, we'll process the packet's contentss later.
- */
- return;
- }
-
- m_adj(m, sizeof(*dnh));
- dnh = NULL;
-
/*
* Inlined m_apply -- why isn't there a macro or inline function
* version?
@@ -181,6 +145,13 @@ netgdb_rx(struct debugnet_pcb *pcb, struct mbuf **mb)
rlen -= count;
m = m->m_next;
}
+ return (0);
+}
+
+static void
+netgdb_finish(void)
+{
+ sbuf_putc(&netgdb_rxsb, CTRL('C'));
}
/*
@@ -338,6 +309,7 @@ DB_COMMAND_FLAGS(netgdb, db_netgdb_cmd, CS_OWN)
struct debugnet_ddb_config params;
struct debugnet_conn_params dcp;
struct debugnet_pcb *pcb;
+ char proxy_buf[INET_ADDRSTRLEN];
int error;
if (!KERNEL_PANICKED()) {
@@ -374,6 +346,7 @@ DB_COMMAND_FLAGS(netgdb, db_netgdb_cmd, CS_OWN)
.dc_client_port = NETGDB_CLIENTPORT,
.dc_herald_aux2 = NETGDB_PROTO_V1,
.dc_rx_handler = netgdb_rx,
+ .dc_finish_handler = netgdb_finish,
};
error = debugnet_connect(&dcp, &pcb);
@@ -398,6 +371,19 @@ DB_COMMAND_FLAGS(netgdb, db_netgdb_cmd, CS_OWN)
db_cmd_loop_done = 1;
gdb_return_to_ddb = true;
db_printf("(detaching GDB will return control to DDB)\n");
+
+ const in_addr_t *proxy_addr = debugnet_get_server_addr(netgdb_conn);
+ const uint16_t proxy_port = debugnet_get_server_port(netgdb_conn) + 1;
+ inet_ntop(AF_INET, proxy_addr, proxy_buf, sizeof(proxy_buf));
+ if (inet_ntop(AF_INET, proxy_addr, proxy_buf, sizeof(proxy_buf)) ==
NULL) {
+ db_printf("Connected to proxy. "
+ "Use target remote <proxy address>:%hu to begin
debugging.\n",
+ proxy_port);
+ } else {
+ db_printf("Connected to proxy. "
+ "Use target remote %s:%hu to begin debugging.\n",
+ proxy_buf, proxy_port);
+ }
#if 0
/* Aspirational, but does not work reliably. */
db_printf("(ctrl-c will return control to ddb)\n");
diff --git a/sys/net/debugnet.c b/sys/net/debugnet.c
index 96cd61b4dd10..cefdd4f582ce 100644
--- a/sys/net/debugnet.c
+++ b/sys/net/debugnet.c
@@ -115,6 +115,22 @@ debugnet_get_gw_mac(const struct debugnet_pcb *pcb)
return (pcb->dp_gw_mac.octet);
}
+const in_addr_t *
+debugnet_get_server_addr(const struct debugnet_pcb *pcb)
+{
+ MPASS(g_debugnet_pcb_inuse && pcb == &g_dnet_pcb &&
+ pcb->dp_state >= DN_STATE_GOT_HERALD_PORT);
+ return (&pcb->dp_server);
+}
+
+const uint16_t
+debugnet_get_server_port(const struct debugnet_pcb *pcb)
+{
+ MPASS(g_debugnet_pcb_inuse && pcb == &g_dnet_pcb &&
+ pcb->dp_state >= DN_STATE_GOT_HERALD_PORT);
+ return (pcb->dp_server_port);
+}
+
/*
* Start of network primitives, beginning with output primitives.
*/
@@ -365,6 +381,8 @@ debugnet_handle_rx_msg(struct debugnet_pcb *pcb, struct
mbuf **mb)
{
const struct debugnet_msg_hdr *dnh;
struct mbuf *m;
+ uint32_t hdr_type;
+ uint32_t seqno;
int error;
m = *mb;
@@ -383,33 +401,46 @@ debugnet_handle_rx_msg(struct debugnet_pcb *pcb, struct
mbuf **mb)
return;
}
}
- dnh = mtod(m, const void *);
+ dnh = mtod(m, const void *);
if (ntohl(dnh->mh_len) + sizeof(*dnh) > m->m_pkthdr.len) {
DNETDEBUG("Dropping short packet.\n");
return;
}
+ hdr_type = ntohl(dnh->mh_type);
+ if (hdr_type != DEBUGNET_DATA) {
+ if (hdr_type == DEBUGNET_FINISHED) {
+ printf("Remote shut down the connection on us!\n");
+ pcb->dp_state = DN_STATE_REMOTE_CLOSED;
+ if (pcb->dp_finish_handler != NULL) {
+ pcb->dp_finish_handler();
+ }
+ } else {
+ DNETDEBUG("Got unexpected debugnet message %u\n",
hdr_type);
+ }
+ return;
+ }
+
/*
* If the issue is transient (ENOBUFS), sender should resend. If
* non-transient (like driver objecting to rx -> tx from the same
* thread), not much else we can do.
*/
- error = debugnet_ack_output(pcb, dnh->mh_seqno);
- if (error != 0)
+ seqno = dnh->mh_seqno; /* net endian */
+ m_adj(m, sizeof(*dnh));
+ dnh = NULL;
+ error = pcb->dp_rx_handler(m);
+ if (error != 0) {
+ DNETDEBUG("RX handler was not able to accept message, error %d.
"
+ "Skipping ack.\n", error);
return;
-
- if (ntohl(dnh->mh_type) == DEBUGNET_FINISHED) {
- printf("Remote shut down the connection on us!\n");
- pcb->dp_state = DN_STATE_REMOTE_CLOSED;
-
- /*
- * Continue through to the user handler so they are signalled
- * not to wait for further rx.
- */
}
- pcb->dp_rx_handler(pcb, mb);
+ error = debugnet_ack_output(pcb, seqno);
+ if (error != 0) {
+ DNETDEBUG("Couldn't ACK rx packet %u; %d\n", ntohl(seqno),
error);
+ }
}
static void
diff --git a/sys/net/debugnet.h b/sys/net/debugnet.h
index 4d209df4cd8b..81962070c9e3 100644
--- a/sys/net/debugnet.h
+++ b/sys/net/debugnet.h
@@ -134,7 +134,10 @@ struct debugnet_conn_params {
*
* The handler should ACK receieved packets with debugnet_ack_output.
*/
- void (*dc_rx_handler)(struct debugnet_pcb *, struct mbuf **);
+ int (*dc_rx_handler)(struct mbuf *);
+
+ /* Cleanup signal for bidirectional protocols. */
+ void (*dc_finish_handler)(void);
};
/*
@@ -207,6 +210,16 @@ void debugnet_network_poll(struct debugnet_pcb *);
*/
const unsigned char *debugnet_get_gw_mac(const struct debugnet_pcb *);
+/*
+ * Get the connected server address.
+ */
+const in_addr_t *debugnet_get_server_addr(const struct debugnet_pcb *);
+
+/*
+ * Get the connected server port.
+ */
+const uint16_t debugnet_get_server_port(const struct debugnet_pcb *);
+
/*
* Callbacks from core mbuf code.
*/
diff --git a/sys/net/debugnet_int.h b/sys/net/debugnet_int.h
index 982000248374..b6c5f2cecff1 100644
--- a/sys/net/debugnet_int.h
+++ b/sys/net/debugnet_int.h
@@ -69,8 +69,10 @@ struct debugnet_pcb {
void (*dp_drv_input)(struct ifnet *, struct mbuf *);
/* RX handler for bidirectional protocols. */
- void (*dp_rx_handler)(struct debugnet_pcb *,
- struct mbuf **);
+ int (*dp_rx_handler)(struct mbuf *);
+
+ /* Cleanup signal for bidirectional protocols. */
+ void (*dp_finish_handler)(void);
enum dnet_pcb_st dp_state;
uint16_t dp_client_port;
