Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

networking.service: fix start networking.service before network is marked online #190

Merged

Conversation

tjjh89017
Copy link
Contributor

In debian 10, ifupdown2 didn't ensure that it will start before network.target and network-online.target.
after local-fs.target because ifupdown2 need to read config file from local filesystem
Other network services will not start after networking.service and fail because no interface is up.
(e.g isc-dhcp-server, tftp-hpa)

In Cumulus, this patch shouldn't be a problem because all switch ports need to be up before starting console for login.
and also need to read config file in local filesystem (need "After=local-fs.target")

…rked online

In debian 10, ifupdown2 didn't ensure that it will start before network.target and network-online.target.
after local-fs.target because ifupdown2 need to read config file from local filesystem
Other network services will not start after networking.service and fail because no interface is up.
(e.g isc-dhcp-server, tftp-hpa)
@tjjh89017 tjjh89017 force-pushed the fix_systemd_start_order branch from a20a82d to 912b869 Compare January 20, 2021 09:22
@tjjh89017 tjjh89017 changed the title Fix start networking.service before network is marked online networking.service: fix start networking.service before network is marked online Jan 20, 2021
@tjjh89017
Copy link
Contributor Author

Hi @julienfortin
Could you please check this PR with Debian 10?
It will be a problem in debian system.
Thanks a lot

@tjjh89017
Copy link
Contributor Author

Related to #30

@tjjh89017
Copy link
Contributor Author

Related to dentproject/dentOS@1d553a9

@aderumier
Copy link
Contributor

Hi,
see proxmox patch too:

https://git.proxmox.com/?p=ifupdown2.git;a=blob;f=debian/patches/pve/0007-networking.service-fix-dependencies-and-ordering.patch;h=66d1e18dfc303231d8e0dc9a81623eae7e541e37;hb=HEAD

we also have added a ifupdown2-pre.service, like ifupdown-pre.service on debian, to wait for systemd-udevd

@tjjh89017
Copy link
Contributor Author

Hi,
see proxmox patch too:

https://git.proxmox.com/?p=ifupdown2.git;a=blob;f=debian/patches/pve/0007-networking.service-fix-dependencies-and-ordering.patch;h=66d1e18dfc303231d8e0dc9a81623eae7e541e37;hb=HEAD

we also have added a ifupdown2-pre.service, like ifupdown-pre.service on debian, to wait for systemd-udevd

It seems PVE will wait for all interface is created and named by udev.
Am I correct?

@aderumier
Copy link
Contributor

@tjjh89017

yes, like debian, because some users using infiniband need it

Initialy debian had added a simple "after=systemd-udev-settle.service"

https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=920623;filename=0001-Depend-on-systemd-udev-settle.service-in-networking..patch;msg=27

but some users with bad devices can have problem, so debian have done it in another ifupdown-pre.service, so user can mask it if it don't want to wait.

proxmox have done the same with ifupdown2-pre.service

@tjjh89017
Copy link
Contributor Author

It seems we should follow ifupdown-pre way to make sure

@julienfortin
Copy link
Contributor

@aderumier should I merge this? I don't want anything to break for anybody.
I honestly don't know much about systemd and dependencies. Dave was our systemd guy but he retired a few years ago 😅

I just tried it on Cumulus Linux 4.4, everything seems OK for us.

I also read the proxmox downstream patch, feel free to submit another PR (if you'd like more changes) as long as those changes work for every debian/ubuntu without breaking existing setups.

@aderumier
Copy link
Contributor

@julienfortin

About systemd ordering and depend, I think you could ask to proxmox team, as I'm not a big expert with systemd too ;)

This proxmox dev could help
https://github.com/blub
Wolfgang Bumiller w.bumiller@proxmox.com

I known he have tried to push to pull request about it some year ago, and they was closed.

See this commit:

https://git.proxmox.com/?p=ifupdown2.git;a=blob;f=debian/patches/pve/0007-networking.service-fix-dependencies-and-ordering.patch;h=66d1e18dfc303231d8e0dc9a81623eae7e541e37;hb=HEAD

#68
(I don't remember why it was refused at this time)

Also, in proxmox , for udevadm-settle, we have implement an ifupdown2-pre.service , to have same behaviour than debian classic ifupdown1 with ifupdown-service.
I can be usefull, as some user with buggy hardware, can have a hang with udevadm-settle, so they just need to mask the ifupdown2-pre.service to bypass it.

@tjjh89017
Copy link
Contributor Author

I'm sure we will need Before=network.target network-online.target to ensure any service depends on network should wait for ifupdown2.
but I'm not sure we need ifupdown2-pre.service in upstream or leave it in distro pkgs patch?

@julienfortin julienfortin merged commit bb29085 into CumulusNetworks:master Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants