On 5/16/22 13:22, Ravi Pokala wrote:
-----Original Message-----
From: <owner-src-committ...@freebsd.org> on behalf of Mitchell Horne
<mho...@freebsd.org>
Date: 2022-05-14, Saturday at 06:28
To: <src-committ...@freebsd.org>, <dev-commits-src-...@freebsd.org>,
<dev-commits-src-main@FreeBSD.org>
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 <mho...@freebsd.org>
AuthorDate: 2022-05-14 13:25:21 +0000
Commit: Mitchell Horne <mho...@freebsd.org>
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.");
