Re: [PATCH net-next v3] net-loopback: set lo dev initial state to UP

On Tue, Feb 9, 2021 at 11:06 AM Ido Schimmel [off-list ref] wrote:

On Tue, Feb 09, 2021 at 10:49:23AM -0800, Mahesh Bandewar (महेश बंडेवार) wrote:

quoted

On Tue, Feb 9, 2021 at 8:23 AM Jakub Kicinski [off-list ref] wrote:

quoted

On Tue, 9 Feb 2021 12:54:59 +0100 Petr Machata wrote:

quoted

Jian Yang [off-list ref] writes:

quoted

From: Jian Yang <redacted>

Traditionally loopback devices come up with initial state as DOWN for
any new network-namespace. This would mean that anyone needing this
device would have to bring this UP by issuing something like 'ip link
set lo up'. This can be avoided if the initial state is set as UP.

This will break user scripts, and it fact breaks kernel's very own
selftest. We currently have this internally:

    diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
    index 4c7d33618437..bf8ed24ab3ba 100755
    --- a/tools/testing/selftests/net/fib_nexthops.sh
    +++ b/tools/testing/selftests/net/fib_nexthops.sh
    @@ -121,8 +121,6 @@ create_ns()
      set -e
      ip netns add ${n}
      ip netns set ${n} $((nsid++))
    - ip -netns ${n} addr add 127.0.0.1/8 dev lo
    - ip -netns ${n} link set lo up

      ip netns exec ${n} sysctl -qw net.ipv4.ip_forward=1
      ip netns exec ${n} sysctl -qw net.ipv4.fib_multipath_use_neigh=1

This now fails because the ip commands are run within a "set -e" block,
and kernel rejects addition of a duplicate address.

Thanks for the report, could you send a revert with this explanation?

Rather than revert, shouldn't we just fix the self-test in that regard?

I reviewed such a patch internally and asked Petr to report it as a
regression instead. At the time the new behavior was added under a
sysctl, but nobody had examples for behavior that will break, so the
sysctl was removed. Now we have such an example, so the revert / sysctl
are needed.

OK, in that case I would prefer to send an incremental patch to
enclose the new behavior with the sysctl (proposed earlier) rather
than the revert. Would that help?