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>