On 18/02/16(Thu) 16:46, Florian Riehm wrote: > On 02/16/16 11:23, Martin Pieuchot wrote: > > On 12/02/16(Fri) 16:33, Florian Riehm wrote: > >> Hi Tech, > >> > >> I have noticed that CARP IP-Balancing is broken, so I am testing and > >> fixing it. > >> > >> The first problem came in with this commit: > >> http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/if_ethersubr.c.diff?r1=1.176&r2=1.177 > >> It enforces that outgoing packets use the src mac of the carp interface > >> instead > >> the mac of its carpdev. That's a good idea for failover carp but it breaks > >> balancing ip(-stealth), because the same source MAC is seen by multiple > >> switchports now. That leads to continues MAC flapping at switches. > >> My attached patch fixes the problem. Thanks Martin's refactoring, we can > >> enforce the right MAC on a central place in carp_start() now. > > > > Can't you completely get rid of the else chunk then? It looks like a > > bad refactoring. > > I was also thinking about deleting the else chunk. At the end I decided to > keep > it, because I wasn't sure about its purpose. > My intention was: > Fix the more or less obviously broken balancing case and don't change the > behavior of other CARP modes. > I will remove the else chunk and do some more testing to determine if it's > really unnecessary. > > > I'm also wondering can't we use "sc_realmac" here? > > I have not know "sc_realmac" until now. As far as I understand the intention > of > the sc_realmac flag is, to indicate that the user has overwritten the mac > address of the carp interface with the mac of its parent. I have never had a > use case to do this. Without overwriting the mac by hand sc_realmac is 0 for > carp with balancing ip/ip-stealth AND carp without balancing. > > I don't see how sc_realmac could help us by this problem?! > Can you give me a hint please?
It was an open question, if that does not make any sense, then don't do it ;) > >> A second problem was introduced two weeks ago. Since we always check the > >> destination MAC address of received unicast packets, not only when in > >> promiscuous mode, carp balancing ip is always broken, not only when in > >> promiscuous mode. The new check collides with "Clear mcast if received on a > >> carp IP balanced address." in carp_input(). > > > > Could you describe the problem? Which Destination MAC the packet > > contains and why it doesn't match the interface? I still don't grok why > > a carp(4) interface in balancing mode cannot have the right MAC > > configured... Do you know? But if what you want is the parent's MAC, > > then I'd rather go with the symmetric of your other change: modify > > carp_input() to overwrite ``ether_dhost''. > > If using carp balancing, incoming packets contain the destination mac address > of the carp interface, let's say: 01:00:5e:00:01:01. > The MCAST-Bit is set here! > > carp_input() removes the MCAST-Bit: > /* > * Clear mcast if received on a carp IP balanced address. > */ > if (sc->sc_balancing == CARP_BAL_IP && > ETHER_IS_MULTICAST(eh->ether_dhost)) > *(eh->ether_dhost) &= ~0x01; > > Then we run into ether_input() and > memcmp(ac->ac_enaddr, eh->ether_dhost, ETHER_ADDR_LEN) > evaluates to false because > ac->ac_enaddr (01:00:5e:00:01:01) != eh->ether_dhost (00:00:5e:00:01:01) Taking a step back why do you want to clear the MULTICAST bit? If it is just to differentiate between advertisement and normal traffic couldn't we use a mbuf_tag(9) for CARP advertisement instead? The tag could be set in carp_input() and checked in carp_lsdrop() in the input path. It could also be used in the output path to get rid of the horrible ``cur_vhe'' hack.
