Page MenuHomeFreeBSD

inet: Fix routing issue by calling if_up()
AbandonedPublic

Authored by sepherosa_gmail.com on Dec 26 2016, 3:28 AM.

Details

Summary
This is mainly intended to fix the following case at least:

ifconfig iface0 192.168.5.1
ifconfig iface0 down
ifconfig iface0 alias 192.168.6.1

Before this commit, the related part of the routing table is:
192.168.5.1        link#3             UHS         lo0
192.168.6.0/24     link#3             U           hn1
192.168.6.1        link#3             UHS         lo0

The 192.168.5.0/24 can't be reached.

After this fix, the related part of the routing table is:
192.168.5.0/24     link#3             U           hn1
192.168.5.1        link#3             UHS         lo0
192.168.6.0/24     link#3             U           hn1
192.168.6.1        link#3             UHS         lo0

Everything works as expected.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

sepherosa_gmail.com retitled this revision from to inet: Fix routing issue by calling if_up().
sepherosa_gmail.com updated this object.
sepherosa_gmail.com edited the test plan for this revision. (Show Details)
hrs added a subscriber: hrs.Dec 26 2016, 7:43 AM

The cause is that the prefix route was removed by in_scrubprefix() in the PRC_IFDOWN handler and never reinstalled upon PRC_IFUP because the reinstallation is done only for ifa passed to SIOCAIFADDR. Just calling if_up(ifp) looks too heavy to me because it causes extra pr_ctlinput() calls of each protocol, not only inetdomain, in order to recover the routes. What do you think about adding reinstalltion of the prefix routes in the protocol specific PRC_IFUP handler instead?

In D8904#184430, @hrs wrote:

The cause is that the prefix route was removed by in_scrubprefix() in the PRC_IFDOWN handler and never reinstalled upon PRC_IFUP because the reinstallation is done only for ifa passed to SIOCAIFADDR. Just calling if_up(ifp) looks too heavy to me because it causes extra pr_ctlinput() calls of each protocol, not only inetdomain, in order to recover the routes. What do you think about adding reinstalltion of the prefix routes in the protocol specific PRC_IFUP handler instead?

Well, I believe it mirrors the behavior of 'ifconfig iface0 up'; w/o the if_up(), we just bring the interface up w/o notifying any domains, which probably is not intended.

karels edited edge metadata.Dec 31 2016, 5:47 PM

I think the change is a step in the right direction. Certainly, "ifconfig xxN down" followed by an implicit UP should not cause any change to the routing table. Does anyone know why the "down" is removing the route? That seems wrong to me.

I think the change is a step in the right direction. Certainly, "ifconfig xxN down" followed by an implicit UP should not cause any change to the routing table. Does anyone know why the "down" is removing the route? That seems wrong to me.

IMHO, it's mainly because the radix tree only uses the target address to do the search. So if you don't remove the prefix routes when bringing down the interface, radix search will end up w/ the route w/ a stopped interface; callers of the radix search don't have options to change this behaviour.

If we don't delete the prefix routes if the interface is down, the following senario will not work:

GW --- NET_A --- if0 HOST1
 |                    if1
 |                     |
 +---- NET_B ----------+

Given HOST1's default gwy is GW. If if1 is brought down, but the NET_B's prefix route is not deleted, HOST1 will not be able to reach NET_B anymore, since the radix search always ends up w/ the route backed by if1.

glebius edited edge metadata.Jan 6 2017, 5:56 AM

I don't think that the patch is in the right direction. The problem comes from historical behavior that assigning an address is implicit UP. For a modern networking equipment it is a normal administrative situation that sysadmin wants to assign an address or an alias and at the same time don't implicitly bring the interface up. The problem get worse when you add CARP into the mix.

For the sake of historical behavior preservation, I would prefer to see hacks to ifconfig(8) rather than to in_control(). Ifconfig could issue SIOCSIFFLAGS after SIOCAIFADDR for the sake of compatibility.

In the projects/ifnet branch this all is somewhat resolved. The only place that writes IFF_UP is if_up() and drivers aren't allowed to do that. The if_init method disappears and logic of initing and assigning address is split. You may look into that branch.