On Thu, Nov 16, 2017 at 03:23:07PM +0100, Patrick Wildt wrote:
> when we detach the interface from the carp the hook that is called
> when an address is updated is disestablished. But it never again
> gets re-established, which makes the carp interface unusable. This
> happens for instance if you run carp on trunk and destroy the trunk
> interface.
While playing with vether, trunk, carp, and addresses I managed to
panic the kernel.
root@v74:.../~# ifconfig carp0 10.188.199.174 delete
root@v74:.../~# panic: kernel diagnostic assertion "ifa != NULL" failed: file
"/crypt/home/bluhm/openbsd/cvs/src/sys/netinet/ip_carp.c", line 1126
Stopped at db_enter+0x5 [/crypt/home/bluhm/openbsd/cvs/src/sys/arch/amd64/
amd64/db_interface.c:402]: popq %rbp
TID PID UID PRFLAGS PFLAGS CPU COMMAND
* 53328 30072 0 0x14000 0x40000200 0 softclock
db_enter(10,ffff80002101a0c0,202,8,ffffffff8127c1e5,0) at db_enter+0x5 [/crypt/
home/bluhm/openbsd/cvs/src/sys/arch/amd64/amd64/db_interface.c:402]
panic(ffff800000305800,ffff8000002e0000,1,ffffff007d568600,ffffffff816f045d,fff
f80002101a0d0) at panic+0x128 [/crypt/home/bluhm/openbsd/cvs/src/sys/kern/subr_
prf.c:208]
__assert(ffffffff8173a824,ffff80002101a160,ffff800000305800,ffff8000002e0000,1,
ffffff007d568600) at __assert+0x24 [/crypt/home/bluhm/openbsd/cvs/src/sys/kern/
subr_prf.c:155]
carp_send_ad(ffffffff81afbd30,ffff80002101a248,ffffffff8140c4f0,ffffffff81a783e
0,ffff8000002e0000,59837fc4f2b9e41d) at carp_send_ad+0x7ac [/crypt/home/bluhm/o
penbsd/cvs/src/sys/netinet/ip_carp.c:1213]
carp_timer_ad(ffff8000002e0000,d,ffffffff8140c50d,ffff80002101a230,ffffffff81af
bd30,ffff80002101a248) at carp_timer_ad+0x1d [/crypt/home/bluhm/openbsd/cvs/src
/sys/netinet/ip_carp.c:1045]
softclock_thread(0,0,ffff8000ffffe470,ffffffff81151ae0,0,100000001) at softcloc
k_thread+0xa6 [/crypt/home/bluhm/openbsd/cvs/src/sys/kern/kern_timeout.c:0]
end trace frame: 0x0, count: 9
> This diff seems to fix it for me, but I'm not sure what kind of re-
> purcussions there are if we keep the hook without an interface.
This diff also fixes my panic. Establishing and disestablishing
the hook in the same layer seems cleaner. Of course there are more
bugs in carp, but we should address them in a next step.
> What do you think?
This is an improvement, OK bluhm@
> diff --git a/sys/netinet/ip_carp.c b/sys/netinet/ip_carp.c
> index 649c7501798..074307ae79c 100644
> --- a/sys/netinet/ip_carp.c
> +++ b/sys/netinet/ip_carp.c
> @@ -878,6 +878,8 @@ carp_clone_destroy(struct ifnet *ifp)
>
> NET_LOCK();
> carpdetach(sc);
> + if (sc->ah_cookie != NULL)
> + hook_disestablish(sc->sc_if.if_addrhooks, sc->ah_cookie);
> NET_UNLOCK();
>
> ether_ifdetach(ifp);
> @@ -920,9 +922,6 @@ carpdetach(struct carp_softc *sc)
> carp_setrun_all(sc, 0);
> carp_multicast_cleanup(sc);
>
> - if (sc->ah_cookie != NULL)
> - hook_disestablish(sc->sc_if.if_addrhooks, sc->ah_cookie);
> -
> ifp0 = sc->sc_carpdev;
> if (ifp0 == NULL)
> return;
