So it sounds like this is not a good candidate for _GENERIC. Shall I commit it as-is then? I've already updated all clients of the 2-argument versions to use the 3-argument versions.

I no longer have access to any hardware that can exercise this change. @slm do you want to commandeer the revision? Otherwise I'll abandon it.

In D14725#309659, @emaste wrote:

I think this kind of compatibility should be provided only if necessary for old code that cannot be changed (such as third party code). If it is just to avoid churn, we should keep using the old timespecadd and timespecsub (that is, abandon this change).

Completely agree.

Why don't you just combine this review with that one? It would be easier to review that way.

Where is route.subr? And where did ipv4_move_subnet_route come from? I think you used the wrong base revision to generate the review.

@mat do you approve this revision. I'll be forced to abandon it in two days if I don't get approval.

Are you satisfied, @cem ?

In D13088#306143, @avg wrote:

Alan, by the way, do you plan to open an upstream issue and a PR to upstream this change?

In D13088#306117, @bdrewery wrote:

In D13088#305562, @asomers wrote:

I can do it. I don't have the ability to run a full regression test on it. But this change is small enough that I think the risk is minimal.

I get a panic on boot after this commit.

panic: solaris assert: refcount_count(&spa->spa_refcount) > spa->spa_minref || MUTEX_HELD(&spa_namespace_lock), file: /usr/src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa_misc.c, line: 952
cpuid = 10
time = 1520207367
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe23f57a2420
vpanic() at vpanic+0x18d/frame 0xfffffe23f57a2480
panic() at panic+0x43/frame 0xfffffe23f57a24e0
assfail() at assfail+0x1a/frame 0xfffffe23f57a24f0
spa_close() at spa_close+0x5d/frame 0xfffffe23f57a2520
spa_get_stats() at spa_get_stats+0x481/frame 0xfffffe23f57a2700
zfs_ioc_pool_stats() at zfs_ioc_pool_stats+0x25/frame 0xfffffe23f57a2740
zfsdev_ioctl() at zfsdev_ioctl+0x76b/frame 0xfffffe23f57a27e0
devfs_ioctl() at devfs_ioctl+0xcb/frame 0xfffffe23f57a2830
VOP_IOCTL_APV() at VOP_IOCTL_APV+0x102/frame 0xfffffe23f57a2860
vn_ioctl() at vn_ioctl+0x124/frame 0xfffffe23f57a2970
devfs_ioctl_f() at devfs_ioctl_f+0x1f/frame 0xfffffe23f57a2990
kern_ioctl() at kern_ioctl+0x2c2/frame 0xfffffe23f57a29f0
sys_ioctl() at sys_ioctl+0x15c/frame 0xfffffe23f57a2ac0
amd64_syscall() at amd64_syscall+0x786/frame 0xfffffe23f57a2bf0
fast_syscall_common() at fast_syscall_common+0x101/frame 0xfffffe23f57a2bf0
--- syscall (54, FreeBSD ELF64, sys_ioctl), rip = 0x80049afda, rsp = 0x7fffffffbd18, rbp = 0x7fffffffbd90 ---
KDB: enter: panic
[ thread pid 56 tid 100606 ]
Stopped at      kdb_enter+0x3b: movq    $0,kdb_why
db>

Not sure if related but I only updated like 1-2 days ago and then today and hit this, nothing else seems to have touched ZFS code.

Needs a man page. Also, how is path interpreted? Is it the usual absolute or relative to PWD? To work with cloudabi, you may need another argument, ala unlinkfdat(int fd_for_path, const char *path, int fd_to_remove, int flag)

I can do it. I don't have the ability to run a full regression test on it. But this change is small enough that I think the risk is minimal.

I agree with @avg. Better to set ENOENT in this case. If the function is going to set errnos sometimes, then it should set them on every abnormal return.

Most of the other tests in this file relate specifically to multifib functionality. I think your test should go into a new file. You can pull the helper routines out into a .subr file.

In D14547#305376, @rstone wrote:

In D14547#305375, @asomers wrote:

The fibs must actually exist. Set net.fibs=6 in /boot/loader.conf and reboot. Also, are you sure you need so many? I don't think there are any tests there that require more than two.

I have done that. The problem is in kyua somewhere:

:
# sysctl net.fibs
net.fibs: 6
# kyua -c /usr/local/etc/kyua/kyua.conf test
fibs_test:arpresolve_checks_interface_fib -> skipped: Required configuration property 'fibs' not defined [0.002s]
fibs_test:default_route_with_multiple_fibs_on_same_subnet -> skipped: Required configuration property 'fibs' not defined [0.002s]
(and so on)
# cat /usr/local/etc/kyua/kyua.conf
-- $FreeBSD: head/devel/kyua/files/kyua.conf.in 364865 2014-08-14 20:21:56Z jmmv $
--
-- System-wide configuration file for kyua(1). See kyua.conf(5) for details
-- on the syntax.
--

syntax(2)


-- User to drop privileges to when invoking kyua(1) as root and a test case
-- requests to be run with non-root permissions.
unprivileged_user = 'tests'

-- An example to set a configuration property specific to FreeBSD.
--test_suites.FreeBSD.fstype = 'ffs'

test_suites.fibs_test.fibs = '1 2 3 4 5'

In D14547#305374, @rstone wrote:

In D14547#305342, @asomers wrote:

In tests/sys/netinet/fibs_test.sh there are some ATF tests that check this kind of functionality. They do it on separate FIBs so they won't upset the system running the tests, and they use tap(4) and/or epair(4) interfaces. You should be able to copy one of those to create a regression test for this problem.

How do I run the tests? Setting this in /usr/local/etc/kyua/kyua.conf doesn't seem to work:

test_suites.fibs_test.fibs = '1 2 3 4 5'

In tests/sys/netinet/fibs_test.sh there are some ATF tests that check this kind of functionality. They do it on separate FIBs so they won't upset the system running the tests, and they use tap(4) and/or epair(4) interfaces. You should be able to copy one of those to create a regression test for this problem.

Fix the build with GCC6

For the record, this PR is based on the content at https://github.com/Savagedlight/libifconfig/pull/46 . IMHO, all new libifconfig development should continue in SVN in the FreeBSD tree.

Remove the -m option and use the monontonic clock unconditionaly

@scottl @imp can either of you test this change? If not, I'll abandon it.

@imp doesn't like this revision, and I feel like his suggested changes would more or less nullify the whole thing. Plus, there are very few drives that actually have a physical sector size other than 512B or 4096B, and we never found any drives that require the lalba field. So I'm abandoning the revision.

This revision requires substantial rework as a result of r320156 (MFV r318946: 8021 ARC buf data scatter-ization). I don't have the time to do it, so I'm going to abandon the review.