"Invalid VRRPv3 checksum" in three-node VRRP v3 setup #642
Comments
|
Also experiencing this issue with VRRPv3. OK with VRRPv2. Observed the same with wireshark. I see the same behaviour on v1.3.5 & v1.2.4. On CentOS7.3. |
|
A couple of months ago I found a checksum problem, but I can't remember the details of it now, but I do remember that I first noticed it due to wireshark reporting it. I didn't push a fix for it since it would have created incompatibility between versions. I'll have a look this evening to see if what I found is the same as you are experiencing. Whatever fix we come up with we need to somehow ensure backward compatibility, but I would hope that we can make the default position going forward to generate the correct checksum. |
|
The good news is that this is the problem I found before and have a fix for. The only problem with the patch (attached) is that it is incompatible with unpatched versions of keepalived. VRRPv3 adds a pseudo IP header into the checksum, whereas VRRPv2 doesn't. The problem is that whether unicast or multicast is used, keepalived at the moment is putting the multicast address into the pseudo header before calculating the checksum, but since the receive end is also putting the multicast address into the pseudo header before checking the checksum, the checksums match even though they are incorrect, as reported by Wireshark. What I don't yet understand is why the checksum is 0000 when the advert is sent to the second unicast peer (ub1604-kl3) (I am seeing exactly the same behaviour on my system). I need to investigate this further so that I understand what is happening before pushing the patch upstream. With the attached patch applied, it sends the correct (non zero) checksum to each peer. Is there a reason why you are using unicast rather than the default of multicast. If you switched to multicast, not only would you be conforming to the RFC and it would be more efficient for keepalived, you won't get the checksum problem even without applying the patch. I have noticed that ub1604-kl2 and ub1604-kl3 are both using priority 100. This is probably not ideal, since if ub1604-kl1 goes away, both backup instances will attempt to become master at the same time (give or take a few microseconds). If both the backup instances transition to master at the same time (because they timeout before they see an advert from the other backup instance) the one with the lower primary IP address will then have to transition again to backup. It is probably better to make sure they have difference priorities. |
|
Many thanks for looking into this issue, @pqarmitage! I'll apply the patch and do the tests here too. The reason we use unicast is that multicast is not widely supported in public/private cloud networks; I absolutely get the reasons to use it on-premises in fully controlled environment, but in our experience those are rarely seen these days and unicast is just something that will mostly work everywhere. Thanks for the advice on the priorities for 2nd and 3rd node - it's actually a proof-of-concept setup to reproduce the issue and it makes perfect sense to have lower priority on the third node. |
|
I've now found the reason the checksum was changing from the (incorrectly calculated) checksum to 0. When constructing the packet for the next unicast destination, the same buffer was used as for the previous unicast destination, and the previous checksum was not cleared before calculating the next checksum, and so the checksum was alternating between the (incorrectly calculated) checksum and 0. This is the actual cause of your problem of invalid checksums being reported by keepalived. The patch i provided above resolves the issue of wireshark reporting incorrect checksums, but also solves your problem since it clears the checksum field before calculating the new checksum. The patch I previously posted will resolve your problem and also stop wireshark reporting invalid checksums; the patch from this update is the minimal change to allow keepalived to work but doesn't resolve the bad checksum reported by wireshark, but it does maintain backward compatibility with previous versions of keepalived. |
…sum failure, maintains backwards compatibility. acassen#642 (comment)
|
Thanks for looking in to this. I'm similarly using unicast mode because we're going to be in public cloud (and openstack in private). I've tested both the patches for my usecase (which only uses VRRP, no IPVS): Both patches work fine in unicast mode (unable to test multicast unfortunately) - no checksum errors, correct handovers etc. The first patch is clearly more ideal - wireshark checksum errors when trying to diagnose network issues would remain confusing. Is the backwards compatibility issue that all participating keepalived clients would have to be upgraded to this fixed version at the same time, (if so, maybe a major version release for this fix)? Either way though, one of these would be good to have upstream. |
|
Commit 67275d2 fixes the problem of alternate unicast hosts having the checksum set to 0. This does not resolve the issue of Wireshark reporting incorrect checksums. I think I have devised a way of fixing the checksum algorithm while maintaining backward compatibility. I'll write it up later and it would be useful to have feedback. |
|
Thanks @pqarmitage. I've backported the following four commits on top of 1.3.6: 67275d2 And it seems it the problem with invalid VRRP checksums went away and cluster is now operational as it should be. I've also checked a two-node vrrp3 setup migration from an older version (in my case, 1.3.4) to 1.3.6+patches, and the step-by-step solution outlined in 0d52573 appears to be correct. Hopefully it will make it into extended notes/changelog etc. I've also checked the packets with wireshark, they have correct checksums as well. |
…release has a problem with V3 when there are more than 2 nodes (acassen/keepalived#642). This has been fixed in master, but not part of a release yet. Using VRRP V2 bypasses the issue.
|
Closing since problem resolved in master branch. |
Automatic merge from submit-queue. Allow vrrp v2 to be specified as an argument in the container specification Add ability to specify VRRP V2 (as opposed to default of 3) in the keepalived template. Current and previous keepalived releases have a problem with VRRP V3 when there are more than 2 nodes (acassen/keepalived#642). This has been fixed in master, but not part of a release yet. Using VRRP V2 bypasses the issue.
We have a following setup on keepalived-1.3.6 with three nodes, running on Ubuntu 16.04:
I see the following messages on the node ub1604-kl3:
This also means that the ub1604-kl3 node is out of the VRRP elections, and isnt able to become a master in case bad things happen to first two nodes.
The messages, however, go away if I use vrrp_version 2 in the configuration files, and all three nodes are participating in elections & can become masters.
I believe vrrp3 checksum generation is broken for this particular case (judging by wireshark reports - as I did not read the VRRPv3 specifications and did not read keepalived code).
It is also possible that checksum check is broken as well, as ub1604-kl2 does not expose the same behaviour as the third node.
Here are the traffic dump files suitable to import to wireshark, taken on ub1604-kl1 node (being master in both cases) with 'tcpdump -w foo.dump -ni eth0 vrrp`:
checksums:
Here's what wireshark tells about the packet in the VRRPv2 case:
And here's what wireshark tells about the packet in the VRRPv3 case:
Thanks.
The text was updated successfully, but these errors were encountered: