xcp-networkd: dhclient cleanup#7138
Conversation
semarie
force-pushed
the
dev/srt/master/dhcp-client-cleanup
branch
from
9e0ffd5 to
e816f94
Compare
| if List.mem name (Sysfs.list ()) then ( | ||
| if Dhclient.is_running name then ignore (Dhclient.stop name) ; | ||
| Ip.flush_ip_addr name | ||
| Dhclient.stop name ; Ip.flush_ip_addr name |
There was a problem hiding this comment.
Ip.flush_ip_addr is not removed consistently here and below
There was a problem hiding this comment.
we can't remove it here.
we are configuring the network to None4. If the previous configuration is not Dhcp4, then Dhclient.stop will not flush the IPs (DHCP is already stopped). So we need to flush it whatever was the previous configuration
There was a problem hiding this comment.
Then flush_ip_addr should be put into a finally block, to make sure it always runs, even on exceptions. There's no case we wouldn't want it to run, right?
There was a problem hiding this comment.
I added a new commit to refactor set_ipv{4,6}_conf functions.
…erface if running and to unconfigure addresses. Signed-off-by: Sebastien Marie <semarie@kapouay.eu.org>
semarie
force-pushed
the
dev/srt/master/dhcp-client-cleanup
branch
from
e816f94 to
516ba53
Compare
… addresses. Signed-off-by: Sebastien Marie <semarie@kapouay.eu.org>
semarie
force-pushed
the
dev/srt/master/dhcp-client-cleanup
branch
from
516ba53 to
fd06a3d
Compare
Signed-off-by: Sebastien Marie <semarie@kapouay.eu.org>
semarie
force-pushed
the
dev/srt/master/dhcp-client-cleanup
branch
2 times, most recently
from
dcdc76a to
ed76ac5
Compare
semarie
force-pushed
the
dev/srt/master/dhcp-client-cleanup
branch
from
ed76ac5 to
32a76a4
Compare
| let previous_config = get_config name in | ||
| let previous = previous_config.ipv4_conf in | ||
| update_config name {previous_config with ipv4_conf= conf} ; | ||
| if previous = conf then |
There was a problem hiding this comment.
Hmm. This looks more complicated than expected.
If matching both, conceptually,
match prev, curr with
| None4, None4
| DHCP4, None4
| Static4, None4 ->
Dhclient.stop (* including Ip.flush_ip_addr now *)
| DHCP4, DHCP4 (* when conf same *) ->
()
| DHCP4, DHCP4 (* when conf different *) ->
Dhclient.stop (* including Ip.flush_ip_addr now *)
Dhclient.ensure_running
| None4, DHCP4
| Static4, DHCP4 ->
Ip.flush_ip_addr
Dhclient.ensure_running
| DHCP4, Static4 ->
Dhclient.stop (* including Ip.flush_ip_addr now *)
(* add/del addrs *)
| None4, Static4
| Static4, Static4 ->
(* add/del addrs *)
It seems you are just trying to get rid of Dhclient.is_running.
For the cases
| DHCP4, DHCP4 (* when conf same *)
| DHCP4, DHCP4 (* when conf different *)
would the Dhclient.ensure_running already cover it? Since the ensure_running will stop and start when the confs are different.
So eventually it could be:
match prev, curr with
| _, None4 ->
Dhclient.stop (* including Ip.flush_ip_addr now *)
| DHCP4, DHCP4 ->
Dhclient.ensure_running
| _, DHCP4 ->
Dhclient.stop (* including Ip.flush_ip_addr now *)
Dhclient.ensure_running
| DHCP4, Static4 ->
Dhclient.stop (* including Ip.flush_ip_addr now *)
(* add/del addrs *)
| _, Static4 ->
(* add/del addrs *)
There was a problem hiding this comment.
ok, I tried to simplify it. In the "deconfigure" part, I am calling Dhclient.stop only if conf isn't DHCP4. It makes DHCP -> DHCP to only call ensure_running.
I also applied the same method for Static (the configure part is doing careful rm+add to avoid disturbing the interface)
There was a problem hiding this comment.
I am still using separated "deconfigure" and "configure" parts, in order to carefully flush addresses when required (Dhclient.stop will only flush them if DHCP is running)
semarie
force-pushed
the
dev/srt/master/dhcp-client-cleanup
branch
from
32a76a4 to
aaa60f8
Compare
- deconfigure the previous configuration before configuring the new configuration (instead of unconfiguring all possible configurations) - ignore exception in deconfiguration to ensure configuration is always called - introduce Dhclient.reload to restart the DHCP client with minimal disruption on the interface Signed-off-by: Sebastien Marie <semarie@kapouay.eu.org>
- DHCP -> DHCP transition is properly managed with minimal disruption Signed-off-by: Sebastien Marie <semarie@kapouay.eu.org>
semarie
force-pushed
the
dev/srt/master/dhcp-client-cleanup
branch
from
aaa60f8 to
9833402
Compare
4 participants
The following commits are a first step for doing some cleanup in DHCP client.
The purpose is to abstract the module and clarify function contracts, in order to permit alternative DHCP client to current (unmaintained)
dhclientbinary from ISC.No functional changes are expected, even if some operations are reordered.