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 <john.reimer AT emc.com> (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 <server> [-g <gateway>] -c <localip> "
-           "-i <interface>\n", cmd);
+       db_printf("Usage: %s -s <server> [-g <gateway> -c <localip> "
+           "-i <interface>]\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 <server> [-g <gateway] -c <localip> -i <interface>
+ * -s <server> [-g <gateway -c <localip> -i <interface>]
  *
  * 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
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to