fix vlan interface bring up on boot

[zhangyoufu]

Motivation for this change

fix #28620

When the parent interface of a vlan interface is not up (yet), ip link cannot bring the vlan interface up. The vlan interface will be automatically brought up when the parent interface is up later.

[coretemp]

I am against merging this, because it is a hack, not a solution.

[zhangyoufu]

Yes, it is a hack. And there're || true hacks all around in network-interfaces-scripted.nix.

In my opinion, a solution would require moving to networkd or a sophisticated refactoring of scripted networking, which is not possible in near future.

Consider #28620 has a severity: blocker label, one more hack inside scripted networking should be acceptable.

[coretemp]

This hack doesn't even explain why it's doing what it is doing in a comment in the source code. As such anyone who ever wants to implement a better method can only break everything else or hunt down GitHub (if that still is being used by the time someone gets around to implementing this).

Just because the code is bad doesn't mean we need to make it worse.

I am not a code owner, but that's how I -- as one of its users -- think about it.

[infinisil]

Is it possible to only run that command on a condition? I'd also prefer to to have a random unexplained || true, and only running it on a checked condition automatically documents it.

[yesbox]

A hack is better than intermittent boot failures. If the code is already using this pattern then using it once more doesn't make it worse if its omission a bug. Call it an intermediate workaround and save it for the refactoring or complete removal after swapping to networkd.

[infinisil]

Needs at least a comment explaining why that's needed

[coretemp]

What is a master interface? Additionally, this is not correct English.

[coretemp]

As indicated.

[jemilsson]

I believe the proper networking terminology is "physical interface".

I am suffering from the issue this PR seems to be solving. I have a system that more often than not fails to boot because of the interfaces being brought up in the wrong order. Since the the networking scripts are already littered with similar hacks I have trouble seeing the issue with using the same pattern once more.

English isn't my first language either, but I tried to formulate a better comment below:

"We try to bring up the logical VLAN interface. If the physical interface the logical interface is dependent upon is not up yet we will fail to immediately bring up the logical interface. "|| true" is a hacky workaround for this issue, resulting in the logical interface being brought up later on."

[fpletz]

@coretemp I agree with you but that's he mess we currently have. Refactoring those units would be a waste of time because there are already enough network configuration systems out there that do it right. We will probably move to networkd eventually.

[fpletz]

@GrahamcOfBorg test networking.scripted.vlan

[GrahamcOfBorg]

Success on aarch64-linux (full log)

Attempted: tests.networking.scripted.vlan

Partial log (click to expand)

client1: exit status 0
client2: running command: sync
client2: exit status 0
test script finished in 33.83s
cleaning up
killing client1 (pid 631)
killing client2 (pid 644)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
/nix/store/1i858pjg9gd26h0jdvyygmi1mqkyf2ny-vm-test-run-vlan-Networking-Scripted

[GrahamcOfBorg]

Failure on x86_64-linux (full log)

Attempted: tests.networking.scripted.vlan

Partial log (click to expand)

client2# [  247.095852] systemd-vconsole-setup[605]: Fonts will not be copied to remaining consoles
error: timed out waiting for the VM to connect
timed out waiting for the VM to connect
cleaning up
killing client2 (pid 600)
killing client1 (pid 612)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
builder for '/nix/store/vp55w09szpcx1y67fgl7kdbjsnm60i30-vm-test-run-vlan-Networking-Scripted.drv' failed with exit code 4
error: build of '/nix/store/vp55w09szpcx1y67fgl7kdbjsnm60i30-vm-test-run-vlan-Networking-Scripted.drv' failed

[yesbox]

@vcunat @samueldr Ping 18.09 release managers, I do believe this PR is crucial.

[grahamc]

@GrahamcOfBorg test networking.scripted.vlan

[GrahamcOfBorg]

Success on x86_64-linux (full log)

Attempted: tests.networking.scripted.vlan

Partial log (click to expand)

client2: exit status 0
client1: running command: sync
client1: exit status 0
test script finished in 26.40s
cleaning up
killing client2 (pid 597)
killing client1 (pid 609)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
/nix/store/3rv7lr5lypgh27mfc59rh2carmcq2ypx-vm-test-run-vlan-Networking-Scripted

[GrahamcOfBorg]

Success on aarch64-linux (full log)

Attempted: tests.networking.scripted.vlan

Partial log (click to expand)

client2: exit status 0
client1: running command: sync
client1: exit status 0
test script finished in 22.68s
cleaning up
killing client2 (pid 631)
killing client1 (pid 643)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
/nix/store/3bmdbfzgzjxsp0vrhng2wk74l5bkmkdh-vm-test-run-vlan-Networking-Scripted