On 20/05/15(Wed) 07:40, Henning Brauer wrote:
> * Johan Ymerson <[email protected]> [2015-05-19 19:25]:
> > On Tue, 2015-05-19 at 11:16 +0000, Johan Ymerson wrote:
> > > On Tue, 2015-05-19 at 11:24 +0100, Stuart Henderson wrote:
> > > > On 2015/05/19 10:10, Johan Ymerson wrote:
> > > > Yes I understand that, but if carp init was counted in "LINK_STATE_DOWN"
> > > > then the metric would be 65535 which I think would still avoid the
> > > > problem you're seeing, and would involve less special-casing in ospfd.
> > > Yes, that would also resolve the issue, but it is a bit illogical to
> > > announce a network we cannot possibly route traffic to (due to hardware
> > > problems).
> > After some more testing I think we can conclude that this is most
> > definitely a kernel issue.
>
> hmm. there's definately more to it.
Indeed
> just for completeness: LINK_STATE_INVALID is 1, and that's what
> carp_set_state uses for everything but master and backup. so far so
> good.
>
> ifp is part of the sc which in turn is malloc'd with M_ZERO in
> carp_clone_create, so link state will be 0 aka LINK_STATE_UNKNOWN.
>
> however, at the end of carp_clone_create, we call
> carp_set_state_all(sc, INIT) which should take care of that.
Sadly it does not, because of:
2305: if (vhe->state == state)
2306: return;
I'd extend your diff a little bit to make this vhe->state transition
less confusing, see below. Do you confirm this also fixes your issue?
Index: netinet/ip_carp.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_carp.c,v
retrieving revision 1.256
diff -u -p -r1.256 ip_carp.c
--- netinet/ip_carp.c 15 May 2015 11:53:06 -0000 1.256
+++ netinet/ip_carp.c 20 May 2015 10:48:17 -0000
@@ -708,9 +708,7 @@ carpattach(int n)
}
int
-carp_clone_create(ifc, unit)
- struct if_clone *ifc;
- int unit;
+carp_clone_create(struct if_clone *ifc, int unit)
{
struct carp_softc *sc;
struct ifnet *ifp;
@@ -753,11 +751,11 @@ carp_clone_create(ifc, unit)
ifp->if_sadl->sdl_type = IFT_CARP;
ifp->if_output = carp_output;
ifp->if_priority = IF_CARP_DEFAULT_PRIORITY;
+ ifp->if_link_state = LINK_STATE_INVALID;
/* Hook carp_addr_updated to cope with address and route changes. */
sc->ah_cookie = hook_establish(sc->sc_if.if_addrhooks, 0,
carp_addr_updated, sc);
- carp_set_state_all(sc, INIT);
return (0);
}
@@ -774,6 +772,7 @@ carp_new_vhost(struct carp_softc *sc, in
vhe->parent_sc = sc;
vhe->vhid = vhid;
vhe->advskew = advskew;
+ vhe->state = INIT;
timeout_set(&vhe->ad_tmo, carp_send_ad, vhe);
timeout_set(&vhe->md_tmo, carp_master_down, vhe);
timeout_set(&vhe->md6_tmo, carp_master_down, vhe);
@@ -2276,8 +2275,12 @@ carp_set_state_all(struct carp_softc *sc
{
struct carp_vhost_entry *vhe;
- LIST_FOREACH(vhe, &sc->carp_vhosts, vhost_entries)
+ LIST_FOREACH(vhe, &sc->carp_vhosts, vhost_entries) {
+ if (vhe->state == state)
+ continue;
+
carp_set_state(vhe, state);
+ }
}
void
@@ -2287,8 +2290,8 @@ carp_set_state(struct carp_vhost_entry *
static const char *carp_states[] = { CARP_STATES };
int loglevel;
- if (vhe->state == state)
- return;
+ KASSERT(vhe->state != state);
+
if (vhe->state == INIT || state == INIT)
loglevel = LOG_WARNING;
else
