From owner-svn-src-all@freebsd.org Thu Oct 17 20:10:34 2019 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 0586615C930; Thu, 17 Oct 2019 20:10:34 +0000 (UTC) (envelope-from cem@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 46vKzF67H3z4fFR; Thu, 17 Oct 2019 20:10:33 +0000 (UTC) (envelope-from cem@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id B59D525B50; Thu, 17 Oct 2019 20:10:33 +0000 (UTC) (envelope-from cem@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x9HKAXnu017831; Thu, 17 Oct 2019 20:10:33 GMT (envelope-from cem@FreeBSD.org) Received: (from cem@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x9HKAWMa017826; Thu, 17 Oct 2019 20:10:32 GMT (envelope-from cem@FreeBSD.org) Message-Id: <201910172010.x9HKAWMa017826@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: cem set sender to cem@FreeBSD.org using -f From: Conrad Meyer Date: Thu, 17 Oct 2019 20:10:32 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r353694 - in head: share/man/man4 sys/net sys/netinet/netdump X-SVN-Group: head X-SVN-Commit-Author: cem X-SVN-Commit-Paths: in head: share/man/man4 sys/net sys/netinet/netdump X-SVN-Commit-Revision: 353694 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Oct 2019 20:10:34 -0000 Author: cem Date: Thu Oct 17 20:10:32 2019 New Revision: 353694 URL: https://svnweb.freebsd.org/changeset/base/353694 Log: debugnet(4): Infer non-server connection parameters Loosen requirements for connecting to debugnet-type servers. Only require a destination address; the rest can theoretically be inferred from the routing table. Relax corresponding constraints in netdump(4) and move ifp validation to debugnet connection time. Submitted by: John Reimer (earlier version) Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D21482 Modified: head/share/man/man4/ddb.4 head/sys/net/debugnet.c head/sys/net/debugnet.h head/sys/net/debugnet_int.h head/sys/netinet/netdump/netdump_client.c Modified: head/share/man/man4/ddb.4 ============================================================================== --- head/share/man/man4/ddb.4 Thu Oct 17 19:53:55 2019 (r353693) +++ head/share/man/man4/ddb.4 Thu Oct 17 20:10:32 2019 (r353694) @@ -1192,7 +1192,7 @@ In remote GDB mode, another machine is required that r using the remote debug feature, with a connection to the serial console port on the target machine. .Pp -.It Ic netdump Fl s Ar server Oo Fl g Ar gateway Oc Fl c Ar client Fl i Ar iface +.It Ic netdump Fl s Ar server Oo Fl g Ar gateway Fl c Ar client Fl i Ar iface Oc Configure .Xr netdump 4 with the provided parameters, and immediately perform a netdump. Modified: head/sys/net/debugnet.c ============================================================================== --- head/sys/net/debugnet.c Thu Oct 17 19:53:55 2019 (r353693) +++ head/sys/net/debugnet.c Thu Oct 17 20:10:32 2019 (r353694) @@ -491,8 +491,12 @@ debugnet_free(struct debugnet_pcb *pcb) MPASS(pcb == &g_dnet_pcb); ifp = pcb->dp_ifp; - ifp->if_input = pcb->dp_drv_input; - ifp->if_debugnet_methods->dn_event(ifp, DEBUGNET_END); + if (ifp != NULL) { + if (pcb->dp_drv_input != NULL) + ifp->if_input = pcb->dp_drv_input; + if (pcb->dp_event_started) + ifp->if_debugnet_methods->dn_event(ifp, DEBUGNET_END); + } debugnet_mbuf_finish(); g_debugnet_pcb_inuse = false; @@ -527,8 +531,87 @@ debugnet_connect(const struct debugnet_conn_params *dc /* Switch to the debugnet mbuf zones. */ debugnet_mbuf_start(); + /* At least one needed parameter is missing; infer it. */ + if (pcb->dp_client == INADDR_ANY || pcb->dp_gateway == INADDR_ANY || + pcb->dp_ifp == NULL) { + struct sockaddr_in dest_sin, *gw_sin, *local_sin; + struct rtentry *dest_rt; + struct ifnet *rt_ifp; + + memset(&dest_sin, 0, sizeof(dest_sin)); + dest_sin = (struct sockaddr_in) { + .sin_len = sizeof(dest_sin), + .sin_family = AF_INET, + .sin_addr.s_addr = pcb->dp_server, + }; + + CURVNET_SET(vnet0); + dest_rt = rtalloc1((struct sockaddr *)&dest_sin, 0, + RTF_RNH_LOCKED); + CURVNET_RESTORE(); + + if (dest_rt == NULL) { + db_printf("%s: Could not get route for that server.\n", + __func__); + error = ENOENT; + goto cleanup; + } + + if (dest_rt->rt_gateway->sa_family == AF_INET) + gw_sin = (struct sockaddr_in *)dest_rt->rt_gateway; + else { + if (dest_rt->rt_gateway->sa_family == AF_LINK) + DNETDEBUG("Destination address is on link.\n"); + gw_sin = NULL; + } + + MPASS(dest_rt->rt_ifa->ifa_addr->sa_family == AF_INET); + local_sin = (struct sockaddr_in *)dest_rt->rt_ifa->ifa_addr; + + rt_ifp = dest_rt->rt_ifp; + + if (pcb->dp_client == INADDR_ANY) + pcb->dp_client = local_sin->sin_addr.s_addr; + if (pcb->dp_gateway == INADDR_ANY && gw_sin != NULL) + pcb->dp_gateway = gw_sin->sin_addr.s_addr; + if (pcb->dp_ifp == NULL) + pcb->dp_ifp = rt_ifp; + + RTFREE_LOCKED(dest_rt); + } + ifp = pcb->dp_ifp; + + if (debugnet_debug > 0) { + char serbuf[INET_ADDRSTRLEN], clibuf[INET_ADDRSTRLEN], + gwbuf[INET_ADDRSTRLEN]; + inet_ntop(AF_INET, &pcb->dp_server, serbuf, sizeof(serbuf)); + inet_ntop(AF_INET, &pcb->dp_client, clibuf, sizeof(clibuf)); + if (pcb->dp_gateway != INADDR_ANY) + inet_ntop(AF_INET, &pcb->dp_gateway, gwbuf, sizeof(gwbuf)); + DNETDEBUG("Connecting to %s:%d%s%s from %s:%d on %s\n", + serbuf, pcb->dp_server_port, + (pcb->dp_gateway == INADDR_ANY) ? "" : " via ", + (pcb->dp_gateway == INADDR_ANY) ? "" : gwbuf, + clibuf, pcb->dp_client_ack_port, if_name(ifp)); + } + + /* Validate iface is online and supported. */ + if (!DEBUGNET_SUPPORTED_NIC(ifp)) { + printf("%s: interface '%s' does not support debugnet\n", + __func__, if_name(ifp)); + error = ENODEV; + goto cleanup; + } + if ((if_getflags(ifp) & IFF_UP) == 0) { + printf("%s: interface '%s' link is down\n", __func__, + if_name(ifp)); + error = ENXIO; + goto cleanup; + } + ifp->if_debugnet_methods->dn_event(ifp, DEBUGNET_START); + pcb->dp_event_started = true; /* * We maintain the invariant that g_debugnet_pcb_inuse is always true @@ -842,37 +925,21 @@ debugnet_parse_ddb_cmd(const char *cmd, struct debugne t = db_read_token_flags(DRT_WSPACE); } - /* Currently, all three are required. */ - if (!opt_client.has_opt || !opt_server.has_opt || ifp == NULL) { - db_printf("%s needs all of client, server, and interface " - "specified.\n", cmd); + if (!opt_server.has_opt) { + db_printf("%s: need a destination server address\n", cmd); goto usage; } + result->dd_has_client = opt_client.has_opt; result->dd_has_gateway = opt_gateway.has_opt; - - /* Iface validation stolen from netdump_configure. */ - if (!DEBUGNET_SUPPORTED_NIC(ifp)) { - db_printf("%s: interface '%s' does not support debugnet\n", - cmd, if_name(ifp)); - error = ENODEV; - goto cleanup; - } - if ((if_getflags(ifp) & IFF_UP) == 0) { - db_printf("%s: interface '%s' link is down\n", cmd, - if_name(ifp)); - error = ENXIO; - goto cleanup; - } - result->dd_ifp = ifp; /* We parsed the full line to tEOL already, or bailed with an error. */ return (0); usage: - db_printf("Usage: %s -s [-g ] -c " - "-i \n", cmd); + db_printf("Usage: %s -s [-g -c " + "-i ]\n", cmd); error = EINVAL; /* FALLTHROUGH */ cleanup: Modified: head/sys/net/debugnet.h ============================================================================== --- head/sys/net/debugnet.h Thu Oct 17 19:53:55 2019 (r353693) +++ head/sys/net/debugnet.h Thu Oct 17 20:10:32 2019 (r353694) @@ -176,11 +176,12 @@ void debugnet_any_ifnet_update(struct ifnet *); /* * DDB parsing helper for common debugnet options. * - * -s [-g -i + * -s [-g -i ] * * Order is not significant. Interface is an online interface that supports * debugnet and can route to the debugnet server. The other parameters are all - * IP addresses. For now, all parameters are mandatory, except gateway. + * IP addresses. Only the server parameter is required. The others are + * inferred automatically from the routing table, if not explicitly provided. * * Provides basic '-h' using provided 'cmd' string. * @@ -191,6 +192,7 @@ struct debugnet_ddb_config { in_addr_t dd_client; in_addr_t dd_server; in_addr_t dd_gateway; + bool dd_has_client : 1; bool dd_has_gateway : 1; }; int debugnet_parse_ddb_cmd(const char *cmd, Modified: head/sys/net/debugnet_int.h ============================================================================== --- head/sys/net/debugnet_int.h Thu Oct 17 19:53:55 2019 (r353693) +++ head/sys/net/debugnet_int.h Thu Oct 17 20:10:32 2019 (r353694) @@ -69,6 +69,7 @@ struct debugnet_pcb { enum dnet_pcb_st dp_state; uint16_t dp_client_ack_port; + bool dp_event_started; }; /* TODO(CEM): Obviate this assertion by using a BITSET(9) for acks. */ Modified: head/sys/netinet/netdump/netdump_client.c ============================================================================== --- head/sys/netinet/netdump/netdump_client.c Thu Oct 17 19:53:55 2019 (r353693) +++ head/sys/netinet/netdump/netdump_client.c Thu Oct 17 20:10:32 2019 (r353694) @@ -138,8 +138,8 @@ static int nd_debug; SYSCTL_INT(_net_netdump, OID_AUTO, debug, CTLFLAG_RWTUN, &nd_debug, 0, "Debug message verbosity"); -SYSCTL_PROC(_net_netdump, OID_AUTO, enabled, CTLFLAG_RD | CTLTYPE_INT, - &nd_ifp, 0, netdump_enabled_sysctl, "I", "netdump configuration status"); +SYSCTL_PROC(_net_netdump, OID_AUTO, enabled, CTLFLAG_RD | CTLTYPE_INT, NULL, 0, + netdump_enabled_sysctl, "I", "netdump configuration status"); static char nd_path[MAXPATHLEN]; SYSCTL_STRING(_net_netdump, OID_AUTO, path, CTLFLAG_RW, nd_path, sizeof(nd_path), @@ -158,14 +158,23 @@ SYSCTL_INT(_net_netdump, OID_AUTO, arp_retries, CTLFLA &debugnet_arp_nretries, 0, "Number of ARP attempts before giving up"); +static bool nd_is_enabled; static bool netdump_enabled(void) { NETDUMP_ASSERT_LOCKED(); - return (nd_ifp != NULL); + return (nd_is_enabled); } +static void +netdump_set_enabled(bool status) +{ + + NETDUMP_ASSERT_LOCKED(); + nd_is_enabled = status; +} + static int netdump_enabled_sysctl(SYSCTL_HANDLER_ARGS) { @@ -296,10 +305,6 @@ netdump_start(struct dumperinfo *di) printf("netdump_start: can't netdump; no server IP given\n"); return (EINVAL); } - if (nd_client.s_addr == INADDR_ANY) { - printf("netdump_start: can't netdump; no client IP given\n"); - return (EINVAL); - } /* We start dumping at offset 0. */ di->dumpoff = 0; @@ -369,14 +374,16 @@ netdump_unconfigure(void) struct diocskerneldump_arg kda; NETDUMP_ASSERT_WLOCKED(); - KASSERT(netdump_enabled(), ("%s: nd_ifp NULL", __func__)); + KASSERT(netdump_enabled(), ("%s: not enabled", __func__)); bzero(&kda, sizeof(kda)); kda.kda_index = KDA_REMOVE_DEV; (void)dumper_remove(nd_conf.ndc_iface, &kda); - if_rele(nd_ifp); + if (nd_ifp != NULL) + if_rele(nd_ifp); nd_ifp = NULL; + netdump_set_enabled(false); log(LOG_WARNING, "netdump: Lost configured interface %s\n", nd_conf.ndc_iface); @@ -406,32 +413,25 @@ netdump_configure(struct diocskerneldump_arg *conf, st NETDUMP_ASSERT_WLOCKED(); - if (td != NULL) - vnet = TD_TO_VNET(td); - else - vnet = vnet0; - CURVNET_SET(vnet); - if (td != NULL && !IS_DEFAULT_VNET(curvnet)) { + if (conf->kda_iface[0] != 0) { + if (td != NULL) + vnet = TD_TO_VNET(td); + else + vnet = vnet0; + CURVNET_SET(vnet); + if (td != NULL && !IS_DEFAULT_VNET(curvnet)) { + CURVNET_RESTORE(); + return (EINVAL); + } + ifp = ifunit_ref(conf->kda_iface); CURVNET_RESTORE(); - return (EINVAL); - } - ifp = ifunit_ref(conf->kda_iface); - CURVNET_RESTORE(); + } else + ifp = NULL; - if (ifp == NULL) - return (ENOENT); - if ((if_getflags(ifp) & IFF_UP) == 0) { - if_rele(ifp); - return (ENXIO); - } - if (!DEBUGNET_SUPPORTED_NIC(ifp)) { - if_rele(ifp); - return (ENODEV); - } - - if (netdump_enabled()) + if (nd_ifp != NULL) if_rele(nd_ifp); nd_ifp = ifp; + netdump_set_enabled(true); #define COPY_SIZED(elm) do { \ _Static_assert(sizeof(nd_conf.ndc_ ## elm) == \ @@ -527,8 +527,9 @@ netdump_ioctl(struct cdev *dev __unused, u_long cmd, c break; } - strlcpy(conf12->ndc12_iface, nd_ifp->if_xname, - sizeof(conf12->ndc12_iface)); + if (nd_ifp != NULL) + strlcpy(conf12->ndc12_iface, nd_ifp->if_xname, + sizeof(conf12->ndc12_iface)); memcpy(&conf12->ndc12_server, &nd_server, sizeof(conf12->ndc12_server)); memcpy(&conf12->ndc12_client, &nd_client, @@ -549,8 +550,9 @@ netdump_ioctl(struct cdev *dev __unused, u_long cmd, c break; } - strlcpy(conf->kda_iface, nd_ifp->if_xname, - sizeof(conf->kda_iface)); + if (nd_ifp != NULL) + strlcpy(conf->kda_iface, nd_ifp->if_xname, + sizeof(conf->kda_iface)); memcpy(&conf->kda_server, &nd_server, sizeof(nd_server)); memcpy(&conf->kda_client, &nd_client, sizeof(nd_client)); memcpy(&conf->kda_gateway, &nd_gateway, sizeof(nd_gateway)); @@ -776,11 +778,17 @@ DB_FUNC(netdump, db_netdump_cmd, db_cmd_table, CS_OWN, /* Translate to a netdump dumper config. */ memset(&conf, 0, sizeof(conf)); - strlcpy(conf.kda_iface, if_name(params.dd_ifp), sizeof(conf.kda_iface)); + if (params.dd_ifp != NULL) + strlcpy(conf.kda_iface, if_name(params.dd_ifp), + sizeof(conf.kda_iface)); + conf.kda_af = AF_INET; conf.kda_server.in4 = (struct in_addr) { params.dd_server }; - conf.kda_client.in4 = (struct in_addr) { params.dd_client }; + if (params.dd_has_client) + conf.kda_client.in4 = (struct in_addr) { params.dd_client }; + else + conf.kda_client.in4 = (struct in_addr) { INADDR_ANY }; if (params.dd_has_gateway) conf.kda_gateway.in4 = (struct in_addr) { params.dd_gateway }; else