Date: Mon, 04 May 2009 17:04:07 +0100 From: Bruce Simpson <bms@incunabulum.net> To: Will Andrews <will@firepipe.net> Cc: freebsd-net@freebsd.org, "Bruce M. Simpson" <bms@freebsd.org> Subject: Re: CARP as a module; followup thoughts Message-ID: <49FF11F7.6090108@incunabulum.net> In-Reply-To: <2aada3410905032103g734e7025nad7f7b13137572ed@mail.gmail.com> References: <2aada3410904212216o128e1fdfx8c299b3531adc694@mail.gmail.com> <49EF11E8.508@FreeBSD.org> <2aada3410905032103g734e7025nad7f7b13137572ed@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Will Andrews wrote: > Thank you very much for your feedback. I have implemented your > suggestion as follows: > > http://firepipe.net/patches/carp-as-module-20090503-2.diff > Great stuff. Overall this does make things that much cleaner. > This version doesn't reinvent the wheel as far as registering the > protocol goes. Personally, I think that notwithstanding your other > objection, this should get committed, since it is a step in the right > direction (perhaps minus the netinet6/in6_proto.c change that adds > spacers). One step at a time! > Yeah, I wouldn't worry about it too much for now. It is something which would be nice to have -- some NICs will perfect hash in hardware on more than one MAC address -- but I believe we've got other issues in this area to do with per-AF locking, which would probably be touched by exactly the issues you raise in the last part of your post... well spotted... > carp_encapcheck() is simplistic, but probably suffices (maybe also > check to see if the vh MAC matches). This patch does work fine with > my test setup, one system using GENERIC+if_carp and the other using a > static build without the patch. > I'll have to take your word for that as I'm not using CARP just at the moment. I had to touch the mcast setup for the IPv6 SSM implementation. All compiles OK, but I haven't tested the code other than loading it. Only IPv6 multicast group setup should be affected. Does your patch apply against these revisions OK? > I also found a "memory leak" in the original code, where it calls > free(cif, M_IFADDR), which is wrong, it should be free(cif, M_CARP), > since the original malloc uses M_CARP -- this fix is also included in > the patch above. > Great stuff. Can this bug fix be merged separately, i.e. before other code is committed? That way it can get merged back to -STABLE more quickly, once RELENG_7 is unfrozen. > ... > > Another way would be to have a separate list/hash table for virtual > lladdr's (ifnet.ifvirt_lladdrhead?). I considered that but it seems > better and more general to simply upgrade ifnet.if_addrhead. > It would be good to have a more general code path for stuff like this to benefit from using the perfect hash filters in modern NICs, the main thing is that everything continues to work with no regressions :-) Thanks for the effort you've put into this, it will certainly help a lot of folk to be able to ship a CARP-capable GENERIC kernel. cheers, BMS
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?49FF11F7.6090108>