From nobody Mon May 16 17:56:05 2022 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 8D9181AE0891; Mon, 16 May 2022 17:56:07 +0000 (UTC) (envelope-from mhorne@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4L26PM3ccZz4dKK; Mon, 16 May 2022 17:56:07 +0000 (UTC) (envelope-from mhorne@freebsd.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1652723767; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=aT15zGRooKsXUgLvVcoaabJNQ5cZLoWi1eSDZOCGCJs=; b=GL1euwSs6HYeE0xWFrj0ZpdOWGRkh9J6mBbw8O63X2H/+TTPJvNQ+n3V4GFGCzXAyBpVkY IDri76h+P+WdPbqkH4BWayLL26PXSGC9BTlJKryMeVK20VvHbK03Ztj4cGKiD3pw+hwb68 N4TLN1KTnPiFMOQ/+k0z3ZHD40yu6CU7YR4XUyoEtSMyE4z3zcIHliOWqdRWgL3+qMeFEq h6zo2iJC0K1pyfuMqLaUUw6FDmzszMF1KkuQvz5guSmSbfTIxZ8rhCS28+k/ORKTGpSDLx Q7+KPZAsJDlywJYIY/nnjsoaUP1E/OJCkxbASIARn4wDD1bbqz+H35Sy4IHKxA== Received: from [192.168.1.106] (host-71-7-134-180.public.eastlink.ca [71.7.134.180]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) (Authenticated sender: mhorne) by smtp.freebsd.org (Postfix) with ESMTPSA id 264D931455; Mon, 16 May 2022 17:56:07 +0000 (UTC) (envelope-from mhorne@freebsd.org) Message-ID: Date: Mon, 16 May 2022 14:56:05 -0300 List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: 6543fa5a5c47 - main - dumpon: warn if the configured netdump link is down Content-Language: en-CA To: Ravi Pokala , src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org References: <202205141328.24EDSlW1051370@gitrepo.freebsd.org> <6D107983-F702-4931-AFBD-ED8CC62DAFF8@panasas.com> From: Mitchell Horne In-Reply-To: <6D107983-F702-4931-AFBD-ED8CC62DAFF8@panasas.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1652723767; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=aT15zGRooKsXUgLvVcoaabJNQ5cZLoWi1eSDZOCGCJs=; b=lVeweik/7o/TpuNb1uuT4GRzNTufAfTC693zF9cd4XoGJACdMorg58OHGHRZdLmwno7bdN eAe+FUlBVHOFlrPUweK5KnTCodbQoL249pEkBT+qlKNXqcoxzEx3V5Yz5k4hLARsAKN0FC 1AIttrcJFwiL8z6/LK+T13xUWyha2JSpulqYNwgRPPdhOz6CxPgkjVRPaD7PfYgxLdMy03 WiJeCZKY1Anobf5gwx4U6DVamN+ErRci+Q7mP+PaAYCzbwULGcKtqtIFl57/O2LO2U+TP0 GwcsRc3FaAyKxXuhFuuJlE0k+hp9ajrz2Th+E4OvMlNRnZj5K5gEYdiJFU8bgQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1652723767; a=rsa-sha256; cv=none; b=NbPokxDsiyPSNkOkk9sjmtmSfDeb/D6UgZJlDVHoHypUDpwHKoQ38enRfFQFguzXQ8Gdfk jRb4DHMdNoKHk1ZaKCDtPizXKb5wJb/a0BsZHrF7Y/YAXcuWZ3TZSp13PgslrTVg6Em/hy ITkqRWLIBmIh8optQwKkubroAKghWMIrAnale5n0w91Qp18u4/OO3uRbGoclu6x4i3Ku6w kKlKpjNofqx+uX33XB4U2MTjMDTh6lmH8Ghs57q4c3CK881SW29/h55hPOmK71N0B75Vb0 EkMB56JXryVS4xaXCKDAugyF2QbyUxic9EVAMrMU0yw+Se3SxI8MFtjw4LFMBg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N On 5/16/22 13:22, Ravi Pokala wrote: > -----Original Message----- > From: on behalf of Mitchell Horne > Date: 2022-05-14, Saturday at 06:28 > To: , , > Subject: git: 6543fa5a5c47 - main - dumpon: warn if the configured netdump link is down > > The branch main has been updated by mhorne: > > URL: https://cgit.FreeBSD.org/src/commit/?id=6543fa5a5c47cfbea92586f0994431fc8ba09f6a > > commit 6543fa5a5c47cfbea92586f0994431fc8ba09f6a > Author: Mitchell Horne > AuthorDate: 2022-05-14 13:25:21 +0000 > Commit: Mitchell Horne > CommitDate: 2022-05-14 13:27:54 +0000 > > dumpon: warn if the configured netdump link is down > > Previously we expected the DIOCSKERNELDUMP ioctl to return ENXIO if the > interface was down, but it does not actually do this. Grab the link > status using getifaddrs(3) instead, and downgrade this case from an > error to a warning; the user might bring the link back up at a later > time. > > Testing whether or not a given link is up sounds like something that should be in libifconfig. If it's there, wouldn't it be better to use that rather than creating check_link_status()? And if it's not there, shouldn't it be? > > Thanks, > > Ravi (rpokala@) > I did look for something like this in libifconfig while making this change but did not find anything useful there currently. Certainly it would make sense to have this kind of helper, even something that just returns the struct ifaddrs * corresponding to a given ifname. However, this change was already a bonus enhancement to complement 38a36057ae56, so I won't be the one to lead the charge of generalizing this pattern into libifconfig. Cheers, Mitchell > Reviewed by: cem > MFC after: 1 week > Differential Revision: https://reviews.freebsd.org/D35196 > --- > sbin/dumpon/dumpon.c | 27 +++++++++++++++++++++++---- > 1 file changed, 23 insertions(+), 4 deletions(-) > > diff --git a/sbin/dumpon/dumpon.c b/sbin/dumpon/dumpon.c > index 7d8f81d5eaaf..626350427595 100644 > --- a/sbin/dumpon/dumpon.c > +++ b/sbin/dumpon/dumpon.c > @@ -186,6 +186,25 @@ find_gateway(const char *ifname) > return (ret); > } > > +static void > +check_link_status(const char *ifname) > +{ > + struct ifaddrs *ifap, *ifa; > + > + if (getifaddrs(&ifap) != 0) > + err(EX_OSERR, "getifaddrs"); > + > + for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) { > + if (strcmp(ifname, ifa->ifa_name) != 0) > + continue; > + if ((ifa->ifa_flags & IFF_UP) == 0) { > + warnx("warning: %s's link is down", ifname); > + } > + break; > + } > + freeifaddrs(ifap); > +} > + > static void > check_size(int fd, const char *fn) > { > @@ -659,6 +678,9 @@ main(int argc, char *argv[]) > else > error = errno; > } > + /* Emit a warning if the user configured a downed interface. */ > + if (error == 0 && netdump) > + check_link_status(kdap->kda_iface); > explicit_bzero(kdap->kda_encryptedkey, kdap->kda_encryptedkeysize); > free(kdap->kda_encryptedkey); > explicit_bzero(kdap, sizeof(*kdap)); > @@ -669,10 +691,7 @@ main(int argc, char *argv[]) > * errors, especially as users don't have any great > * discoverability into which NICs support netdump. > */ > - if (error == ENXIO) > - errx(EX_OSERR, "Unable to configure netdump " > - "because the interface's link is down."); > - else if (error == ENODEV) > + if (error == ENODEV) > errx(EX_OSERR, "Unable to configure netdump " > "because the interface driver does not yet " > "support netdump."); > >