Page MenuHomeFreeBSD

Allow IPv4 subnet routes to move to a different ifa
Needs ReviewPublic

Authored by rstone on Feb 28 2018, 6:56 PM.

Details

Reviewers
eugen_grosbein.net
Group Reviewers
network
Summary

Suppose a user configures two IPv4 addresses with the same prefix on
their system (call them x.y.z.A/24 and x.y.z.B/24), and then
moves the subnet route from A to B with:

route change x.y.z.0/24 -ifa x.y.z.B

The kernel is left in a very confused state. ifa A has the
IFA_ROUTE flag still set on it, indicating that the kernel
believes that there is still a subnet route using A as its
source address. If the user subsequently tries to remove A from
the interface, things become even more confused. in_scrubprefix
tries to remove the subnet route and fails (because no subnet
route is associated with A), and as a result the static arp
entry for A is never destroyed. This leaves the system in a
state where it is impossible to assign A to any interface.

We hit this at $WORK in some code that attempted to fail over
some routes from one address to another, and later tried to fail
over an IP address. The system was left in a state where it was
no longer responding to one of the IP addresses that had been
configured on it, because we couldn't assign the IP to any
interface.

Fix this by detecting the case where a subnet route is being modified,
and migrate the IFA_ROUTE flag from the old ifa to the new one.

Sponsored by: Dell EMC Isilon

Test Plan

Ran this script: https://people.freebsd.org/~rstone/scripts/subnet-route-test

On stock FreeBSD, the final "ifconfig" command fails with "ifconfig: ioctl (SIOCAIFADDR): File exists" and if you run "arp -na" afterwards you see a stray 2.0.0.1 that cannot be removed.

With this fix, the IP address can be assigned as expected.

I also tested changing a 2.0.0.0/8 route and verified that the IFA_ROUTE flag was not migrated because the subnet width was different.


New ATF test for IPv4 and IPv6 cases:
With r330147:
# kyua test fibs_test:ipv4_move_subnet_route fibs_test:ipv6_move_subnet_route
fibs_test:ipv4_move_subnet_route -> failed: atf-check failed; see the output of the test for details [0.121s]
fibs_test:ipv6_move_subnet_route -> passed [0.107s]

Results file id is usr_tests_sys_netinet.20180301-204924-411726
Results saved to /root/.kyua/store/results.usr_tests_sys_netinet.20180301-204924-411726.db

1/2 passed (1 failed)
# kyua report --verbose fibs_test:ipv4_move_subnet_route
===> Execution context
Current directory: /usr/tests/sys/netinet
Environment variables:
BLOCKSIZE=K
HOME=/root
LOGNAME=root
MAIL=/var/mail/root
OLDPWD=/root
PAGER=more
PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin:/root/bin
PWD=/usr/tests/sys/netinet
SHELL=/bin/sh
TERM=vt102
USER=root
===> fibs_test:ipv4_move_subnet_route
Result: failed: atf-check failed; see the output of the test for details
Start time: 2018-03-01T20:49:25.098671Z
End time: 2018-03-01T20:49:25.219287Z
Duration: 0.121s

Metadata:
allowed_architectures is empty
allowed_platforms is empty
description = moving a subnet route to different ifa should be possible
has_cleanup = true
is_exclusive = false
required_configs = fibs
required_disk_space = 0
required_files is empty
required_memory = 0
required_programs is empty
required_user = root
timeout = 300

Standard output:
fib is 1
/sbin/pfctl
/sbin/ipf
ipf: IP Filter: v5.1.2 (608)
setfib 1 ifconfig epair0a inet 192.0.2.1/24 fib 1
setfib 1 ifconfig epair0b inet 192.0.2.2/24 fib 1
change net 192.0.2.0 fib 1
Executing command [ ifconfig epair0b inet 192.0.2.1/24 alias fib 1 ]
ifconfig epair0a destroy

Standard error:
pfctl: /dev/pf: No such file or directory
sysctl: unknown oid 'net.inet.ip.fw.enable'
[: =: unexpected operator
open device: No such file or directory
Fail: incorrect exit status: 1, expected: 0
stdout:

stderr:
ifconfig: ioctl (SIOCAIFADDR): File exists

Files left in work directory after failure: ifaces_to_cleanup
===> Failed tests
fibs_test:ipv4_move_subnet_route -> failed: atf-check failed; see the output of the test for details [0.121s]
===> Summary
Results read from /root/.kyua/store/results.usr_tests_sys_netinet.20180301-204924-411726.db
Test cases: 1 total, 0 skipped, 0 expected failures, 0 broken, 1 failed
Start time: 2018-03-01T20:49:25.098671Z
End time: 2018-03-01T20:49:25.219287Z
Total time: 0.121s

With fix:
# kyua test fibs_test:ipv4_move_subnet_route fibs_test:ipv6_move_subnet_route
fibs_test:ipv4_move_subnet_route -> passed [0.107s]
fibs_test:ipv6_move_subnet_route -> passed [0.113s]

Results file id is usr_tests_sys_netinet.20180301-204556-470959
Results saved to /root/.kyua/store/results.usr_tests_sys_netinet.20180301-204556-470959.db

2/2 passed (0 failed)

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 15374
Build 15425: arc lint + arc unit

Event Timeline

rstone created this revision.Feb 28 2018, 6:56 PM
rstone edited the test plan for this revision. (Show Details)Feb 28 2018, 7:03 PM
rstone updated this revision to Diff 39839.Feb 28 2018, 7:48 PM

Check for NULL pointers

What FreeBSD version (including revision) do you use for TEST PLAN script, e.g. what is your "stock FreeBSD"?

Are you sure that first command in the script is correct?

subnet=${1:-2.0}.0

It sets subnet to very different values like "2.0.0" with no arguments specified and to "1.0" for argument "1".

And you do you insist in doing things manually while moving IP (first route change, then reassign IP) instead of just allowing system do all housekeeping for you by reassigning IP first?

pi added a subscriber: pi.Feb 28 2018, 8:55 PM
asomers added a subscriber: asomers.Mar 1 2018, 3:14 PM

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.

rstone added a comment.Mar 1 2018, 6:46 PM

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.

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'

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.

rstone added a comment.EditedMar 1 2018, 7:01 PM

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'

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'

Oh, I see. You specified the test suite wrong. You're confusing it with the test program. The correct line should be:

test_suites.FreeBSD.fibs = '1 2 3 4 5'
rstone edited the test plan for this revision. (Show Details)Mar 1 2018, 8:50 PM
rstone updated this revision to Diff 39870.Mar 1 2018, 8:51 PM

Add ATF test cases for IPv4/IPv6 subnet route migration

rstone added a comment.Mar 1 2018, 8:54 PM

And you do you insist in doing things manually while moving IP (first route change, then reassign IP) instead of just allowing system do all housekeeping for you by reassigning IP first?

The route migration and the IP migration happened as a part of different failover processes IIRC. The subnet route migration happened in response to the route's interface losing link. The IP migration happened later in response to a different event (I can't remember off-hand what prompted it)

In any case, even if the use-case is weird, the current behaviour is clearly buggy and needs to be fixed in some way.

In any case, even if the use-case is weird, the current behaviour is clearly buggy and needs to be fixed in some way.

Well, I guess so. But original configuration may be supposed to be "buggy" too (read: not supported). What if you use /16 mask for single interface only and assign IP from the same prefix to other interfaces using /32 mask?

rstone added a comment.Mar 5 2018, 4:57 PM

In any case, even if the use-case is weird, the current behaviour is clearly buggy and needs to be fixed in some way.

Well, I guess so. But original configuration may be supposed to be "buggy" too (read: not supported). What if you use /16 mask for single interface only and assign IP from the same prefix to other interfaces using /32 mask?

In that configuration I can't move the subnet route from one address to the other, which is exactly the behaviour that my customer expects.

rstone updated this revision to Diff 39961.Mar 5 2018, 5:21 PM

Move route(8) tests to their own atf test file

rstone updated this revision to Diff 39962.Mar 5 2018, 5:32 PM

Uploaded the wrong test to this review; fixed now

Well, I guess so. But original configuration may be supposed to be "buggy" too (read: not supported). What if you use /16 mask for single interface only and assign IP from the same prefix to other interfaces using /32 mask?

In that configuration I can't move the subnet route from one address to the other, which is exactly the behaviour that my customer expects.

Care to elaborate what are you trying to achieve by moving subnet route between interfaces?

rstone added a comment.Mar 5 2018, 5:43 PM

Care to elaborate what are you trying to achieve by moving subnet route between interfaces?

My customer is using it as a lame form of failover. If the interface with address A goes down, we can fail over the subnet route to address B (on a different interface) and address B remains functional (address A remains down, of course. I did same it was a lame form of failover).

I have no ability to push back on the customer on this point. Their position is that this configuration was supported on previous versions and therefore must remain supported.

Care to elaborate what are you trying to achieve by moving subnet route between interfaces?

My customer is using it as a lame form of failover. If the interface with address A goes down, we can fail over the subnet route to address B (on a different interface) and address B remains functional (address A remains down, of course. I did same it was a lame form of failover).
I have no ability to push back on the customer on this point. Their position is that this configuration was supported on previous versions and therefore must remain supported.

In which "previous version" was it "supported"?

Also, why don't you just use "ifconfig em0 x.y.z.A/24 -alias; ifconfig em1 x.y.z.B/24 alias" to perform the switch? It should just work.

rstone added a comment.Mar 5 2018, 6:02 PM

I don't think that this conversation is going to go anywhere productive. Do you have specific technical objection to raise about this change?

I don't think that this conversation is going to go anywhere productive.

Why? I'm trying to understand if such non-trivial amount of kernel code changes is justified by the problem.
Perhaps, there is simplier way to solve original problem.

Do you have specific technical objection to raise about this change?

I thought it should not be supported to assign same prefix to different network interfaces (for example, Cisco IOS won't allow that) but I might be wrong. You have a chance to describe real world setup when that's needed and cannot be solved with more consistent configuration.

pi added a comment.Mar 5 2018, 7:23 PM

Also, why don't you just use "ifconfig em0 x.y.z.A/24 -alias; ifconfig em1 x.y.z.B/24 alias" to perform the switch? It should just work.

This has a short timewindow, during which packets will be rejected. On high-traffic ports this will cause connection drops or losses.

In D14547#306289, @pi wrote:

Also, why don't you just use "ifconfig em0 x.y.z.A/24 -alias; ifconfig em1 x.y.z.B/24 alias" to perform the switch? It should just work.

This has a short timewindow, during which packets will be rejected. On high-traffic ports this will cause connection drops or losses.

We are talking about switch after link failure detected, aren't we? Drops and losses have already occured then, and for much longer period, so some extra does not really change the picture.

mjoras added a subscriber: mjoras.Mar 6 2018, 4:16 AM

Care to elaborate what are you trying to achieve by moving subnet route between interfaces?

My customer is using it as a lame form of failover. If the interface with address A goes down, we can fail over the subnet route to address B (on a different interface) and address B remains functional (address A remains down, of course. I did same it was a lame form of failover).
I have no ability to push back on the customer on this point. Their position is that this configuration was supported on previous versions and therefore must remain supported.

In which "previous version" was it "supported"?

This worked on older versions of our product, which were based roughly on FreeBSD 7.x

Also, why don't you just use "ifconfig em0 x.y.z.A/24 -alias; ifconfig em1 x.y.z.B/24 alias" to perform the switch? It should just work.

There is another feature of our product that does this (the configuration is managed by a daemon, not manually), but the customer has rejected it. The point is that their existing configuration used to provide failover, and now it does not, so that is a regression.