Date: Sat, 2 Mar 2013 01:31:18 -0000 From: "Steven Hartland" <killing@multiplay.co.uk> To: <freebsd-net@freebsd.org> Subject: Review of carp patch for stable Message-ID: <23773B97555741F68384CD67B2123077@multiplay.co.uk>
next in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. ------=_NextPart_000_079B_01CE16E5.A326E480 Content-Type: text/plain; format=flowed; charset="iso-8859-1"; reply-type=original Content-Transfer-Encoding: 7bit We've got a lovely shiny new carp thanks to glebius in head but unfortunately its not going to get MFC'ed so for the time being those users on -RELEASE / -STABLE need to use the current implementation which has a nasty issue where by it doesn't cleanup when IP's are removed hence gets into an invalid state. The code was there for address cleanup but was never being called because in_control was never calling ifp->if_ioctl in the SIOCDIFADDR case. Once I wired this up it uncovered a few more issues with carp_del_addr* methods, so I've patched those too. The call from in_control ignores EINVAL and ENOTTY (which seems like compat and appears to only be used in if_mxge) to try and ensure backwards compatibility, I could go further and ignore all error returns but I don't think that would be a good idea. Given this can't go into head first as carp has changed so much I'd like to get feedback to confirm it all looks good and isn't going to break the world. What do people think? Did I miss anything or is there a better way to do this that anyone knows of? I should mention Glebius has already looked at it and seemed to think it looked sane. Regards Steve ================================================ This e.mail is private and confidential between Multiplay (UK) Ltd. and the person or entity to whom it is addressed. In the event of misdirection, the recipient is prohibited from using, copying, printing or otherwise disseminating it or any information contained in it. In the event of misdirection, illegible or incomplete transmission please telephone +44 845 868 1337 or return the E.mail to postmaster@multiplay.co.uk. ------=_NextPart_000_079B_01CE16E5.A326E480 Content-Type: application/octet-stream; name="carp-addr-del.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="carp-addr-del.patch" Fix carp not deleting addresses correctly, which breaks carp interfaces = setup using the rc.d/jail.=0A= --- sys/netinet/in.c.orig 2013-02-19 17:06:48.000000000 +0000=0A= +++ sys/netinet/in.c 2013-02-26 14:52:53.293628430 +0000=0A= @@ -583,6 +583,20 @@=0A= =0A= case SIOCDIFADDR:=0A= /*=0A= + * Carp and possibly others expect their if_ioctl handler=0A= + * called for SIOCDIFADDR=0A= + */=0A= + if (ifp->if_ioctl !=3D NULL) {=0A= + error =3D (*ifp->if_ioctl)(ifp, SIOCDIFADDR, (caddr_t)ia);=0A= + /*=0A= + * We ignore EINVAL and ENOTTY as this call is likely not=0A= + * supported by most drivers=0A= + */=0A= + if (error && error !=3D EINVAL && error !=3D ENOTTY)=0A= + goto out;=0A= + }=0A= +=0A= + /*=0A= * in_ifscrub kills the interface route.=0A= */=0A= in_ifscrub(ifp, ia, LLE_STATIC);=0A= --- sys/netinet/ip_carp.c.orig 2013-02-25 19:03:18.801241179 +0000=0A= +++ sys/netinet/ip_carp.c 2013-02-25 19:30:54.401573772 +0000=0A= @@ -1628,21 +1628,17 @@=0A= =0A= if (!--sc->sc_naddrs) {=0A= struct carp_if *cif =3D (struct carp_if *)sc->sc_carpdev->if_carp;=0A= - struct ip_moptions *imo =3D &sc->sc_imo;=0A= =0A= CARP_LOCK(cif);=0A= - callout_stop(&sc->sc_ad_tmo);=0A= - SC2IFP(sc)->if_flags &=3D ~IFF_UP;=0A= - SC2IFP(sc)->if_drv_flags &=3D ~IFF_DRV_RUNNING;=0A= - sc->sc_vhid =3D -1;=0A= - in_delmulti(imo->imo_membership[--imo->imo_num_memberships]);=0A= - imo->imo_multicast_ifp =3D NULL;=0A= - TAILQ_REMOVE(&cif->vhif_vrs, sc, sc_list);=0A= - if (!--cif->vhif_nvrs) {=0A= - sc->sc_carpdev->if_carp =3D NULL;=0A= - CARP_LOCK_DESTROY(cif);=0A= - free(cif, M_CARP);=0A= + if (cif->vhif_nvrs =3D=3D 1) {=0A= + /* Last address so detach */=0A= + carpdetach(sc, 1);=0A= } else {=0A= + /* Just the last IPv4 address */=0A= + callout_stop(&sc->sc_md_tmo);=0A= + carp_multicast_cleanup(sc, 1);=0A= + TAILQ_REMOVE(&cif->vhif_vrs, sc, sc_list);=0A= + cif->vhif_nvrs--;=0A= CARP_UNLOCK(cif);=0A= }=0A= }=0A= @@ -1836,18 +1831,17 @@=0A= struct carp_if *cif =3D (struct carp_if *)sc->sc_carpdev->if_carp;=0A= =0A= CARP_LOCK(cif);=0A= - callout_stop(&sc->sc_ad_tmo);=0A= - SC2IFP(sc)->if_flags &=3D ~IFF_UP;=0A= - SC2IFP(sc)->if_drv_flags &=3D ~IFF_DRV_RUNNING;=0A= - sc->sc_vhid =3D -1;=0A= - carp_multicast6_cleanup(sc, 1);=0A= - TAILQ_REMOVE(&cif->vhif_vrs, sc, sc_list);=0A= - if (!--cif->vhif_nvrs) {=0A= - CARP_LOCK_DESTROY(cif);=0A= - sc->sc_carpdev->if_carp =3D NULL;=0A= - free(cif, M_CARP);=0A= - } else=0A= + if (cif->vhif_nvrs =3D=3D 1) {=0A= + /* Last address so detach */=0A= + carpdetach(sc, 1);=0A= + } else {=0A= + /* Just the last IPv6 address */=0A= + callout_stop(&sc->sc_md6_tmo);=0A= + carp_multicast6_cleanup(sc, 1);=0A= + TAILQ_REMOVE(&cif->vhif_vrs, sc, sc_list);=0A= + cif->vhif_nvrs--;=0A= CARP_UNLOCK(cif);=0A= + }=0A= }=0A= =0A= return (error);=0A= ------=_NextPart_000_079B_01CE16E5.A326E480--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?23773B97555741F68384CD67B2123077>