Re: Multiple VRRP instances with 1.2.17
Michael Littlejohn <[email protected]>
| Newsgroups | gmane.linux.keepalived.devel |
|---|---|
| Message-ID | <[email protected]> |
Hi Alexandre, Ryan, et al,
[Preface: Forgive me, I'm not familiar with SourceForge tooling or protocol, so if I make a mess of this or get into too much detail let me know.]
Regarding this regression, I have found the root-cause of the issue and, tentatively, I have a possible fix for it.
The regression _was_ caused by my earlier commit ( 76f9ce36ae389983b48a83a933182739786908c9) which inadvertently changed the behaviour of vrrp_init_sands_instance(vrrp_t *) for Ryan's use-case. We don't have a similar use-case so the regression went undetected.
In the previous version of the code the following would have happened to Ryan's use-case:
1. vrrp_init_sands would have initialised the sands timer of each VRRP group (such that the sands were non-zero).
2. The interface those groups were on would have been given to a thread and enqueued to the READ queue
3. The above thread would have timed-out of the READ queue and been passed to vrrp_dispatcher_read_to()
4. vrrp_dispatcher_read_to() would have used vrrp_timer_vrid_timeout() to determine which group on that interface was the "most expired"
5. The "most expired" group would have been run through the FSM and would have had it's sands timer's updated.
6. Generally return to step 2 and repeat.
In the current version of the code, Ryan's use-case goes through the following:
1. vrrp_init_sands tries to initialise the groups but the change made to vrrp_init_sands_instance mean that the sands **are not** initialised (they are ZERO when vrrp_init_sands_instance returns).
2. The interface those groups were on would have been given to a thread and enqueued to the READ queue
3. The above thread would have timed-out of the READ queue and been passed to vrrp_dispatcher_read_to()
4. vrrp_dispatcher_read_to() tries to use vrrp_timer_vrid_timeout() to determine which group on that interface is "most expired" however, it's logic is such that vrrp sands timers which have **not** been initialised are ignored. Consequently, it defaults to using the last group of the pair(^1).
5. The last group is run through the FSM and has it's timers updated.
6. Generally return to step 2 and repeat.
1. Extrapolating this where n > 2, I believe the last group on the interface is always processed. Any other groups will be ignored.
So ends the facts.
---
As for my tentative solution.
I have *not* reverted my previous changes to vrrp_init_sands_instance(vrrp_t *) on my local copy. This is because those changes prevent a different issue where in ~10% of cases, preemption behaviour becomes stuck and does not transition over to the correct MASTER router (this is due to a timing issue caused by the old logic of vrrp_init_sands_instance where a group would get stuck in GOTO_MASTER, but would have it's timers updated while still in that state because it was still receiving packets).
Instead, I have modified the tie-breaker mechanism vrrp_timer_vrid_timeout() so that if a group has uninitialised sands timers it will be returned by the tie-breaker immediately and processed by steps 6 and 6 (above). However, I have concerns about that approach in the cold light of day as I don't think it will perform well at scale. Testing required.
Another tentative solution would be to change how the timers were initialised at startup, so that they were set to time_now rather than being processed by the modified logic of vrrp_init_sands_instance(). This would overcome the issue with the logic in vrrp_timer_vrid_timeout() and should also scale nicely. I'll test this solution later today and report back.
---
As for the changes as they stand in vrrp_init_sands_instance(), what other use-cases are broken by my change and are there ways of working around them that I could help out with?
Regards,
M
On 17/06/15 22:30, [email protected]<mailto:[email protected]> wrote:
Send Keepalived-devel mailing list submissions to
[email protected]<mailto:[email protected]>
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.sourceforge.net/lists/listinfo/keepalived-devel
or, via email, send a message with subject or body 'help' to
[email protected]<mailto:[email protected]>
You can reach the person managing the list at
[email protected]<mailto:[email protected]>
When replying, please edit your Subject line so it is more specific
than "Re: Contents of Keepalived-devel digest..."
Today's Topics:
1. Re: Keepalived-devel Digest, Vol 107, Issue 2 (Alexandre Cassen)
----------------------------------------------------------------------
Message: 1
Date: Wed, 17 Jun 2015 23:24:00 +0200
From: Alexandre Cassen <[email protected]><mailto:[email protected]>
Subject: Re: [Keepalived-devel] Keepalived-devel Digest, Vol 107,
Issue 2
To: David Stapleton <[email protected]><mailto:[email protected]>
Cc: [email protected]<mailto:[email protected]>
Message-ID: <[email protected]><mailto:[email protected]>
Content-Type: text/plain; charset="utf-8"
please provide feedback here tomorrow, otherwise I will revert this commit? which anyway seems strange reading the code, since it breaks some use-cases? VRRP FSM is very sensitive stuff, most of the time trying to optimise stuff is ending on breaking stuff :/
regs,
Alexandre
On 17 Jun 2015, at 21:13, David Stapleton <[email protected]><mailto:[email protected]> wrote:
Hi Ryan,
I am also seeing this issue. I work with the person who's commit you suspect is the root cause (mlittlej-brcd) so I will speak to him about this tomorrow. Also, the person who signed off on the commit (cdochert) will be off work for a while, so please feel free to CC Anthony ([email protected]<mailto:[email protected]> <mailto:[email protected]><mailto:[email protected]>) and myself ([email protected]<mailto:[email protected]> <mailto:[email protected]><mailto:[email protected]>) on any other issues regarding keepalived.
In the meantime, there seems to be two workarounds to this problem. 1) Instead of setting each instance's initial state to master, when you set them to backup the issue is not observed. 2) If you add one instance at a time to the config and then do a reload, the issue doesn't seem to happen. Of course, this isn't very practical, especially if you want to configure multiple groups on the one interface.
Once again, I'll have a chat with mlittlej tomorrow and will let you know what he says.
Thanks,
David
On 17 June 2015 at 17:39, <[email protected]<mailto:[email protected]> <mailto:[email protected]><mailto:[email protected]>> wrote:
Send Keepalived-devel mailing list submissions to
[email protected]<mailto:[email protected]> <mailto:[email protected]><mailto:[email protected]>
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.sourceforge.net/lists/listinfo/keepalived-devel <https://lists.sourceforge.net/lists/listinfo/keepalived-devel><https://lists.sourceforge.net/lists/listinfo/keepalived-devel>
or, via email, send a message with subject or body 'help' to
[email protected]<mailto:[email protected]> <mailto:[email protected]><mailto:[email protected]>
You can reach the person managing the list at
[email protected]<mailto:[email protected]> <mailto:[email protected]><mailto:[email protected]>
When replying, please edit your Subject line so it is more specific
than "Re: Contents of Keepalived-devel digest..."
Today's Topics:
1. [acassen/keepalived] d80c17: Fix vrrp removes incorrect IPv4
address when VIPs ... (Alexandre Cassen)
2. Multiple VRRP instances with 1.2.17 (Ryan O'Hara)
3. Re: Multiple VRRP instances with 1.2.17 (Ryan O'Hara)
4. Re: Multiple VRRP instances with 1.2.17 (Ryan O'Hara)
5. Re: Multiple VRRP instances with 1.2.17 (Alexandre Cassen)
-------------- next part --------------
An HTML attachment was scrubbed...
------------------------------
------------------------------------------------------------------------------
------------------------------
_______________________________________________
Keepalived-devel mailing list
[email protected]<mailto:[email protected]>
https://lists.sourceforge.net/lists/listinfo/keepalived-devel
End of Keepalived-devel Digest, Vol 107, Issue 4
************************************************
------------------------------------------------------------------------------
_______________________________________________
Keepalived-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/keepalived-devel