-
-
Notifications
You must be signed in to change notification settings - Fork 734
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
netlink: do not ignore vip as system owner #2229
base: master
Are you sure you want to change the base?
Conversation
When a VRRP instance is waiting for an IP address on the interface to start, if the vip is added on the interface, keepalived ignores it and remains in FAULT state, even if the VRRP instance is the system owner (priority 255 and vip = system IP). However, when adding an IP address different than the vip, the instance can change the state. This makes nonsense for an instance is configured as the system owner transit to FAULT state since the system has not yet the vip. When the vip is added to the system, keepalived ignores it and this instance remains in FAULT state, whereas an IP address different than the vip can change the state of this instance. Thus, do not ignore if a vip is added to the system when a VRRP instance is configured with 255 priority. Signed-off-by: Loïc Sang <[email protected]>
Hello, As discussed in: It is possible to integrate this commit to make VRRP work as a simple router? |
I notice that the first two versions of this patch didn't work (corrected in this latest version), and this gives me some concern when patches are being submitted with pull requests that evidently haven't been fully tested. I do appreciate that the problem has now been fixed, and I appreciate that. One comment I would make on the style of the patch is:
This should always be written as:
since the else is not only unnecessary but also gives the impression that this branch of the code can continue after the true block of the if statement has been executed. I agree that your patch is correct and will need to be merged, but I have a significant concern that relates to the misconfiguration issues I have previously mentioned. Suppose that, rather than a single VIP address, the configuration has the following:
If the VRRP instance is configured to be the address owner, then if any of those addresses are not configured on the system, the VRRP instance should not go to master state. The current code, although due to the functionality not being fully correct, will ensure that the VRRP instance will not go to master if there is no other IP address configured on the interface. This provides some protection, albeit somewhat imperfect, of the VIPs not being configured on the system. If just one of the VIPs is added, then with your patch the VRRP instance will be able to transition to master despite the other VIPs not being configured. It may well be that there are users who rely on the current functionality to stop the address owner becoming master if the VIPs are not configured (if there are no other addresses that will be configured on the interface) and we need to make sure we don't break this for them. The question is, "what is the right thing to do if VIPs of an address-owning VRRP instance are missing?" I think there are three possible answers: iii) has the benefit that consistency is maintained; if any of the VIPs exist, they all will and we will be master. ii) similarly ensures consistency but has less flexibility. i) causes a problem if some of the VIPs exist but not all, since this instance will be in FAULT state, and another system will be in master state, so there will be some duplicate IP addresses configure. Similarly iv) can cause a problem of duplicate IP addresses. However, we need to maintain iv) since it is how keepalived currently operates. I think we will need to implement all solutions, with iv) being the default since that is how keepalived works now. My view is that once this has been implemented, then it is safe to merge your patch. If you would like to implement the above options, then I would be very grateful if you would implement it and submit a pull request. Otherwise I will do it when I have time, but that won't be for a few weeks. |
Hello, Thanks for your answer, I was not sure that you will respond to me as my patch is so minimalist. "I notice that the first two versions of this patch didn't work (corrected in this latest version), and this gives me some concern when patches are being submitted with pull requests that evidently haven't been fully tested. I do appreciate that the problem has now been fixed, and I appreciate that." Sorry for the multiple pull requests, as I am working on a maintenance branch, I manually copied the patch to the master with some errors. "I think we will need to implement all solutions,..." I will do my best for that but, how to implement all solutions? by adding an option when starting keepalived? |
Yes, there will need to be configuration options. Generally I would add an option for the VRRP instance, and also a global_defs option that changes the default setting. |
hello, "Yes, there will need to be configuration options. Generally I would add an option for the VRRP instance, and also a global_defs option that changes the default setting." OK, I get it , we can configure the behavior by VRRP instance or by global_defs (the instance config can override global_defs).
|
When a VRRP instance is waiting for an IP address on the interface to start, if the vip is added on the interface, keepalived ignores it and remains in FAULT state, even if the VRRP instance is the system owner (priority 255 and vip = system IP). However, when adding an IP address different than the vip, the instance can change the state. This makes nonsense for an instance is configured as the system owner transit to FAULT state since the system has not yet the vip. When the vip is added to the system, keepalived ignores it and this instance remains in FAULT state, whereas an IP address different than the vip can change the state of this instance. Thus, do not ignore if a vip is added to the system when a VRRP instance is configured with 255 priority. Signed-off-by: Loïc Sang <[email protected]>
Add a global option "vrrp_system_owner" to define the behavior of a vrrp configured with 255 priority (system owner): - The VRRP_FLAG_SYSTEM_OWNER_DFT is the current implementation. A system owner instance can transit with any IP address on the physical interafce. This is the default option when vrrp_system_owner is not defined. - With VRRP_FLAG_SYSTEM_OWNER_ANY, a system owner instance can transit with any VIP configured on the physical interface. To use, that define "vrrp_system_owner any" in keepalived.conf. - With VRRP_FLAG_SYSTEM_OWNER_STRICT, a system owner instance can transit when all VIPs are configured on the physical interface. To use that, define "vrrp_system_owner strict" in keepalived.conf. Signed-off-by: Loïc Sang <[email protected]>
Add two more options for a vrrp instance with 255 priority (system owner) i) Remain in fault state until all the VIPs have been added. For that, set "vrrp_system_owner strict" in keepalived.conf. ii) If any of the VIPs exists, add all the remaining VIPs. For that, set "vrrp_system_owner any" in keepalived.conf. iii) Go to MASTER state if there is an IP address on the VRRP instance's interface (this it current implemenation). This is the default behaviour, if vrrp_system_owner is not configured. Signed-off-by: Loïc Sang <[email protected]>
Hello, I have implemented the I) and III) options. I will continue on II) and will fix the problem for IPv6. |
When a VRRP instance is waiting for an IP address on the interface to start, if the vip is added on the interface, keepalived ignores it and remains in FAULT state, even if the VRRP instance is the system owner (priority 255 and vip = system IP). However, when adding an IP address different than the vip, the instance can change the state.
This makes nonsense for an instance is configured as the system owner transit to FAULT state since the system has not yet the vip. When the vip is added to the system, keepalived ignores it and this instance remains in FAULT state, whereas an IP address different than the vip can change the state of this instance.
Thus, do not ignore if a vip is added to the system when a VRRP instance is configured with 255 priority.
Signed-off-by: Loïc Sang [email protected]