Changeset View
Standalone View
sys/net/debugnet.c
Show All 25 Lines | |||||
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY | * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY | ||||
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF | * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF | ||||
* SUCH DAMAGE. | * SUCH DAMAGE. | ||||
*/ | */ | ||||
#include <sys/cdefs.h> | #include <sys/cdefs.h> | ||||
__FBSDID("$FreeBSD$"); | __FBSDID("$FreeBSD$"); | ||||
#include "opt_ddb.h" | |||||
#include "opt_inet.h" | #include "opt_inet.h" | ||||
#include <sys/param.h> | #include <sys/param.h> | ||||
#include <sys/systm.h> | #include <sys/systm.h> | ||||
#include <sys/endian.h> | #include <sys/endian.h> | ||||
#include <sys/errno.h> | #include <sys/errno.h> | ||||
#include <sys/socket.h> | #include <sys/socket.h> | ||||
#include <sys/sysctl.h> | #include <sys/sysctl.h> | ||||
#ifdef DDB | |||||
#include <ddb/ddb.h> | |||||
#include <ddb/db_lex.h> | |||||
#endif | |||||
#include <net/ethernet.h> | #include <net/ethernet.h> | ||||
#include <net/if.h> | #include <net/if.h> | ||||
#include <net/if_arp.h> | #include <net/if_arp.h> | ||||
#include <net/if_dl.h> | #include <net/if_dl.h> | ||||
#include <net/if_types.h> | #include <net/if_types.h> | ||||
#include <net/if_var.h> | #include <net/if_var.h> | ||||
#include <netinet/in.h> | #include <netinet/in.h> | ||||
▲ Show 20 Lines • Show All 600 Lines • ▼ Show 20 Lines | |||||
static eventhandler_tag dn_attach_cookie; | static eventhandler_tag dn_attach_cookie; | ||||
static void | static void | ||||
dn_evh_init(void *ctx __unused) | dn_evh_init(void *ctx __unused) | ||||
{ | { | ||||
dn_attach_cookie = EVENTHANDLER_REGISTER(ifnet_link_event, | dn_attach_cookie = EVENTHANDLER_REGISTER(ifnet_link_event, | ||||
dn_ifnet_event, NULL, EVENTHANDLER_PRI_ANY); | dn_ifnet_event, NULL, EVENTHANDLER_PRI_ANY); | ||||
} | } | ||||
SYSINIT(dn_evh_init, SI_SUB_EVENTHANDLER + 1, SI_ORDER_ANY, dn_evh_init, NULL); | SYSINIT(dn_evh_init, SI_SUB_EVENTHANDLER + 1, SI_ORDER_ANY, dn_evh_init, NULL); | ||||
/* | |||||
* DDB parsing helpers for debugnet(4) consumers. | |||||
*/ | |||||
#ifdef DDB | |||||
struct my_inet_opt { | |||||
bool has_opt; | |||||
const char *printname; | |||||
in_addr_t *result; | |||||
}; | |||||
static int | |||||
dn_parse_optarg_ipv4(struct my_inet_opt *opt) | |||||
{ | |||||
in_addr_t tmp; | |||||
unsigned octet; | |||||
int t; | |||||
tmp = 0; | |||||
for (octet = 0; octet < 4; octet++) { | |||||
t = db_read_token(); | |||||
if (t != tNUMBER) { | |||||
db_printf("%s:%s: octet %u expected number; found %d\n", | |||||
__func__, opt->printname, octet, t); | |||||
return (EINVAL); | |||||
} | |||||
/* | |||||
* db_lex lexes '-' distinctly from the number itself, but | |||||
* let's document that invariant. | |||||
*/ | |||||
MPASS(db_tok_number >= 0); | |||||
if (db_tok_number > UINT8_MAX) { | |||||
db_printf("%s:%s: octet %u out of range: %ld\n", __func__, | |||||
opt->printname, octet, db_tok_number); | |||||
return (EDOM); | |||||
} | |||||
/* Constructed host-endian and converted to network later. */ | |||||
tmp = (tmp << 8) | db_tok_number; | |||||
if (octet < 3) { | |||||
t = db_read_token(); | |||||
if (t != tDOT) { | |||||
db_printf("%s:%s: octet %u expected '.'; found" | |||||
" %d\n", __func__, opt->printname, octet, | |||||
t); | |||||
return (EINVAL); | |||||
} | |||||
} | |||||
} | |||||
*opt->result = htonl(tmp); | |||||
opt->has_opt = true; | |||||
return (0); | |||||
} | |||||
int | |||||
debugnet_parse_ddb_cmd(const char *cmd, struct debugnet_ddb_config *result) | |||||
{ | |||||
struct ifnet *ifp; | |||||
int t, error; | |||||
bool want_ifp; | |||||
char ch; | |||||
/* Assume v4, dotted decimal encoding for now. */ | |||||
db_cmd_radix = 10; | |||||
struct my_inet_opt opt_client = { | |||||
.printname = "client", | |||||
.result = &result->dd_client, | |||||
}, | |||||
opt_server = { | |||||
.printname = "server", | |||||
.result = &result->dd_server, | |||||
}, | |||||
opt_gateway = { | |||||
.printname = "gateway", | |||||
.result = &result->dd_gateway, | |||||
}, | |||||
*cur_inet_opt; | |||||
ifp = NULL; | |||||
memset(result, 0, sizeof(*result)); | |||||
/* | |||||
* command [space] [-] [opt] [[space] [optarg]] ... | |||||
* | |||||
* db_command has already lexed 'command' for us. | |||||
*/ | |||||
t = db_read_token(); | |||||
if (t == tWSPACE) | |||||
t = db_read_token(); | |||||
while (t != tEOL) { | |||||
if (t != tMINUS) { | |||||
db_printf("%s: Bad syntax; expected '-', got %d\n", | |||||
cmd, t); | |||||
goto usage; | |||||
} | |||||
t = db_read_token(); | |||||
if (t != tIDENT) { | |||||
db_printf("%s: Bad syntax; expected tIDENT, got %d\n", | |||||
cmd, t); | |||||
goto usage; | |||||
} | |||||
if (strlen(db_tok_string) > 1) { | |||||
db_printf("%s: Bad syntax; expected single option " | |||||
"flag, got '%s'\n", cmd, db_tok_string); | |||||
goto usage; | |||||
} | |||||
want_ifp = false; | |||||
cur_inet_opt = NULL; | |||||
switch ((ch = db_tok_string[0])) { | |||||
default: | |||||
DNETDEBUG("Unexpected: '%c'\n", ch); | |||||
/* FALLTHROUGH */ | |||||
case 'h': | |||||
goto usage; | |||||
case 'c': | |||||
cur_inet_opt = &opt_client; | |||||
break; | |||||
case 'g': | |||||
cur_inet_opt = &opt_gateway; | |||||
break; | |||||
case 's': | |||||
cur_inet_opt = &opt_server; | |||||
break; | |||||
case 'i': | |||||
want_ifp = true; | |||||
break; | |||||
} | |||||
t = db_read_token(); | |||||
if (t != tWSPACE) { | |||||
db_printf("%s: Bad syntax; expected space after " | |||||
"flag %c, got %d\n", cmd, ch, t); | |||||
goto usage; | |||||
} | |||||
if (want_ifp) { | |||||
t = db_read_token(); | |||||
if (t != tIDENT) { | |||||
db_printf("%s: Expected interface but got %d\n", | |||||
cmd, t); | |||||
goto usage; | |||||
} | |||||
CURVNET_SET(vnet0); | |||||
/* | |||||
markj: Maybe add a comment explaining why you need to take a ref? I would have assumed that it wasn't… | |||||
Done Inline ActionsYes, that's exactly it. I actually thought I had already added that comment but apparently not. Will fix, thanks. (Yeah, and assuming this is in ddb(4) and !panic (which doesn't work yet for other reasons), netdump_configure will hold on to that nd_ifp and if_rele it later, if the device is removed or deconfigured. So this is for that case.) cem: Yes, that's exactly it. I actually thought I had already added that comment but apparently not. | |||||
Done Inline ActionsI see, it's briefly alluded to in the output structure definition in netdump.h: 194 struct ifnet *dd_ifp; /* ref'd */ But not documented. In fact, I don't think we actually need the ref at all, because the netdump DB_FUNC only grabs the if_name() and invokes netdump_configure(), which re-looks up the ifp and grabs its own ref. So I'll add a comment regardless, but drop the ref'd ifp. cem: I see, it's briefly alluded to in the output structure definition in netdump.h:
```
194… | |||||
Not Done Inline ActionsOh, I didn't read netdump_configure() closely enough. I think you can just use ifunit() instead then, though that variant doesn't check for IFF_DYING. markj: Oh, I didn't read netdump_configure() closely enough. I think you can just use ifunit() instead… | |||||
Done Inline ActionsI did go ahead and use ifunit(). Re DYING, any thoughts on:
In rough order of my preference. cem: I did go ahead and use ifunit(). Re DYING, any thoughts on:
- Just rely on… | |||||
Not Done Inline ActionsI think the first choice would be fine. markj: I think the first choice would be fine. | |||||
Done Inline ActionsOk, cool. cem: Ok, cool. | |||||
* We *don't* take a ref here because the only current | |||||
* consumer, db_netdump_cmd, does not need it. It | |||||
* (somewhat redundantly) extracts the if_name(), | |||||
* re-lookups the ifp, and takes its own reference. | |||||
*/ | |||||
ifp = ifunit(db_tok_string); | |||||
CURVNET_RESTORE(); | |||||
if (ifp == NULL) { | |||||
db_printf("Could not locate interface %s\n", | |||||
db_tok_string); | |||||
goto cleanup; | |||||
} | |||||
} else { | |||||
MPASS(cur_inet_opt != NULL); | |||||
/* Assume IPv4 for now. */ | |||||
error = dn_parse_optarg_ipv4(cur_inet_opt); | |||||
if (error != 0) | |||||
goto cleanup; | |||||
} | |||||
/* Skip (mandatory) whitespace after option, if not EOL. */ | |||||
t = db_read_token(); | |||||
if (t == tEOL) | |||||
break; | |||||
if (t != tWSPACE) { | |||||
db_printf("%s: Bad syntax; expected space after " | |||||
"flag %c option; got %d\n", cmd, ch, t); | |||||
goto usage; | |||||
} | |||||
t = db_read_token(); | |||||
} | |||||
/* 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); | |||||
goto usage; | |||||
} | |||||
result->dd_has_gateway = opt_gateway.has_opt; | |||||
/* Iface validation stolen from netdump_configure. */ | |||||
if ((if_getflags(ifp) & IFF_UP) == 0) { | |||||
db_printf("%s: interface '%s' link is down\n", cmd, | |||||
if_name(ifp)); | |||||
error = ENXIO; | |||||
goto cleanup; | |||||
} | |||||
if (!DEBUGNET_SUPPORTED_NIC(ifp)) { | |||||
markjUnsubmitted Done Inline ActionsWould it make a bit more sense to check DEBUGNET_SUPPORTED_NIC before IFF_UP? markj: Would it make a bit more sense to check DEBUGNET_SUPPORTED_NIC before IFF_UP? | |||||
cemAuthorUnsubmitted Done Inline ActionsIt doesn't matter too much from a happy case or single failure perspective. I guess the idea is if the NIC is down AND debugnet isn't supported, we'd rather the user find out netdump won't work on this NIC than that the NIC happened to be offline at the time? That's reasonable. cem: It doesn't matter too much from a happy case or single failure perspective.
I guess the idea… | |||||
cemAuthorUnsubmitted Done Inline Actions(I think I just copied this order from netdump_client.) cem: (I think I just copied this order from netdump_client.) | |||||
markjUnsubmitted Not Done Inline ActionsYeah, it just seems a bit more logical to check for "static" failure cases first. markj: Yeah, it just seems a bit more logical to check for "static" failure cases first. | |||||
cemAuthorUnsubmitted Done Inline ActionsYeah, that makes a lot of sense to me. cem: Yeah, that makes a lot of sense to me. | |||||
db_printf("%s: interface '%s' does not support debugnet\n", | |||||
cmd, if_name(ifp)); | |||||
error = ENODEV; | |||||
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); | |||||
error = EINVAL; | |||||
/* FALLTHROUGH */ | |||||
cleanup: | |||||
db_skip_to_eol(); | |||||
return (error); | |||||
} | |||||
#endif /* DDB */ |
Maybe add a comment explaining why you need to take a ref? I would have assumed that it wasn't necessary, but netdump_configure() expects to be passed a ref'ed interface.