[CFT & review] new in_control()

From: Gleb Smirnoff <glebius_at_FreeBSD.org>
Date: Fri, 1 Nov 2013 16:47:20 +0400
  Hi!

  I've got a patch that cleans up the way we configure
and delete IPv4 on interfaces. What it does:

1) separate function for SIOCAIFADDR, with clear code
   flow from beginning to the end.
2) separate function for SIOCDIFADDR, with clear code
   flow from beginning to the end.
3) provided 1) and 2) the in_control() got very thin
   and clear.

The above wasn't just a cut&paste job, instead every
step taken was evaluated. I've cut quite a lot of strange
code, added extra sanity checking and provided comments
on the strange code that remains.

4) sx(9) lock covers entire SIOCAIFADDR/SIOCDIFADDR
   operation, so we close races ifconfig vs ifconfig,
   or ifconfig vs mpd.
   On interface detach SIOCDIFADDR is called w/o sx(9),
   but its operation is covered by IF_ADDR_LOCK().

Also, except of redesign of SIOCAIFADDR/SIOCDIFADDR,
the following two related changes leaked into the patch.
It is possible to separate them out, but won't be easy.

5) Removed useloopback conditional. Rationale:
   - option was always on since pre-FreeBSD times
   - sysctl knob lives in invalid (ethernet) namespace,
     and documented in wrong (arp(8)) place.
   - since new-ARP, the knob was consulted on route
     addition, but was ignored on delete.
   - operation of network stack useloopback=0 is
     strange

   The only reason running useloopback=0 could be
   a router that doesn't want to pollute large network
   with its /32 announces. However, this can be achieved
   with filtering in routing daemons.

6) Implemented correctly code from r201282, that tried
   to keep localhost route in table when multiple P2P
   interfaces with same local address are created and
   deleted.

The check in of the code can cause problems. I could make
mistakes, and some program that relied on strange behavior
can pop up. Thus, early testing is appreciated.

So far I have tested simple address assignment, CARP,
and mpd5 as L2TP access concentrator.

Advice for reviewers is to not look at diff, but look at
patched in.c instead.

-- 
Totus tuus, Glebius.

Received on Fri Nov 01 2013 - 11:47:23 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:43 UTC