On Sun, Nov 23, 2014 at 12:06 +0100, Martin Pieuchot wrote:
> On 23/11/14(Sun) 02:10, Mike Belopuhov wrote:
> > Hi,
> >
> > This removes the system wide if_slowtimo timeout and lets every
> > interface with a valid if_watchdog method register it's own.
> > The rational is to get rid of the ifnet loop in the softclock
> > context to avoid further complications with concurrent access
> > to the ifnet list. This might also save some cpu cycles in the
> > runtime if number of interfaces is large while only a fraction
> > all of them have a watchdog routine.
> >
> > This has originally come up on NetBSD mailing lists with some
> > rather complicated locking strategy. Masao Uebayashi has later
> > proposed a similar solution.
> >
> > The diff was looked at and corrected by mpi@. I've tested the
> > trunk and a system build as well. OK?
>
> I'm basically ok, some remarks below.
>
>
> What about putting the allocation inside if_attach_common() with the
> hooks that are allocated for the same reason?
Sure.
> Maybe we should explain why we allocate them?
>
We all know why, don't think it will help anything.
New version. OK?
diff --git sys/net/if.c sys/net/if.c
index 0632eec..679c8c8 100644
--- sys/net/if.c
+++ sys/net/if.c
@@ -133,11 +133,10 @@ void if_attachdomain1(struct ifnet *);
void if_attach_common(struct ifnet *);
void if_detach_queues(struct ifnet *, struct ifqueue *);
void if_detached_start(struct ifnet *);
int if_detached_ioctl(struct ifnet *, u_long, caddr_t);
-void if_detached_watchdog(struct ifnet *);
int if_getgroup(caddr_t, struct ifnet *);
int if_getgroupmembers(caddr_t);
int if_getgroupattribs(caddr_t);
int if_setgroupattribs(caddr_t);
@@ -169,16 +168,12 @@ int net_livelocked(void);
* parameters.
*/
void
ifinit()
{
- static struct timeout if_slowtim;
-
- timeout_set(&if_slowtim, if_slowtimo, &if_slowtim);
timeout_set(&net_tick_to, net_tick, &net_tick_to);
- if_slowtimo(&if_slowtim);
net_tick(&net_tick_to);
}
static unsigned int if_index = 0;
static unsigned int if_indexlim = 0;
@@ -270,10 +265,13 @@ if_attachsetup(struct ifnet *ifp)
if_attachdomain1(ifp);
#if NPF > 0
pfi_attach_ifnet(ifp);
#endif
+ timeout_set(ifp->if_slowtimo, if_slowtimo, ifp);
+ if_slowtimo(ifp);
+
/* Announce the interface. */
rt_ifannouncemsg(ifp, IFAN_ARRIVAL);
}
/*
@@ -426,10 +424,13 @@ if_attach_common(struct ifnet *ifp)
M_TEMP, M_WAITOK);
TAILQ_INIT(ifp->if_linkstatehooks);
ifp->if_detachhooks = malloc(sizeof(*ifp->if_detachhooks),
M_TEMP, M_WAITOK);
TAILQ_INIT(ifp->if_detachhooks);
+
+ ifp->if_slowtimo = malloc(sizeof(*ifp->if_slowtimo), M_TEMP,
+ M_WAITOK|M_ZERO);
}
void
if_start(struct ifnet *ifp)
{
@@ -481,19 +482,22 @@ if_detach(struct ifnet *ifp)
struct domain *dp;
ifp->if_flags &= ~IFF_OACTIVE;
ifp->if_start = if_detached_start;
ifp->if_ioctl = if_detached_ioctl;
- ifp->if_watchdog = if_detached_watchdog;
+ ifp->if_watchdog = NULL;
/*
* Call detach hooks from head to tail. To make sure detach
* hooks are executed in the reverse order they were added, all
* the hooks have to be added to the head!
*/
dohooks(ifp->if_detachhooks, HOOK_REMOVE | HOOK_FREE);
+ /* Remove the watchdog timeout */
+ timeout_del(ifp->if_slowtimo);
+
#if NBRIDGE > 0
/* Remove the interface from any bridge it is part of. */
if (ifp->if_bridgeport)
bridge_ifdetach(ifp);
#endif
@@ -577,10 +581,12 @@ do { \
free(ifp->if_addrhooks, M_TEMP, 0);
free(ifp->if_linkstatehooks, M_TEMP, 0);
free(ifp->if_detachhooks, M_TEMP, 0);
+ free(ifp->if_slowtimo, M_TEMP, sizeof(*ifp->if_slowtimo));
+
for (dp = domains; dp; dp = dp->dom_next) {
if (dp->dom_ifdetach && ifp->if_afdata[dp->dom_family])
(*dp->dom_ifdetach)(ifp,
ifp->if_afdata[dp->dom_family]);
}
@@ -1160,29 +1166,26 @@ if_link_state_change_task(void *arg, void *unused)
}
splx(s);
}
/*
- * Handle interface watchdog timer routines. Called
- * from softclock, we decrement timers (if set) and
+ * Handle interface watchdog timer routine. Called
+ * from softclock, we decrement timer (if set) and
* call the appropriate interface routine on expiration.
*/
void
if_slowtimo(void *arg)
{
- struct timeout *to = (struct timeout *)arg;
- struct ifnet *ifp;
+ struct ifnet *ifp = arg;
int s = splnet();
- TAILQ_FOREACH(ifp, &ifnet, if_list) {
- if (ifp->if_timer == 0 || --ifp->if_timer)
- continue;
- if (ifp->if_watchdog)
+ if (ifp->if_watchdog) {
+ if (ifp->if_timer > 0 && --ifp->if_timer == 0)
(*ifp->if_watchdog)(ifp);
+ timeout_add(ifp->if_slowtimo, hz / IFNET_SLOWHZ);
}
splx(s);
- timeout_add(to, hz / IFNET_SLOWHZ);
}
/*
* Map interface name to interface structure pointer.
*/
@@ -1824,16 +1827,10 @@ int
if_detached_ioctl(struct ifnet *ifp, u_long a, caddr_t b)
{
return ENODEV;
}
-void
-if_detached_watchdog(struct ifnet *ifp)
-{
- /* nothing */
-}
-
/*
* Create interface group without members
*/
struct ifg_group *
if_creategroup(const char *groupname)
diff --git sys/net/if_trunk.c sys/net/if_trunk.c
index 90e7fe9..c4bc760 100644
--- sys/net/if_trunk.c
+++ sys/net/if_trunk.c
@@ -342,15 +342,17 @@ trunk_port_create(struct trunk_softc *tr, struct ifnet
*ifp)
/* Change the interface type */
tp->tp_iftype = ifp->if_type;
ifp->if_type = IFT_IEEE8023ADLAG;
ifp->if_tp = (caddr_t)tp;
- tp->tp_watchdog = ifp->if_watchdog;
- ifp->if_watchdog = trunk_port_watchdog;
tp->tp_ioctl = ifp->if_ioctl;
ifp->if_ioctl = trunk_port_ioctl;
+ timeout_del(ifp->if_slowtimo);
+ tp->tp_watchdog = ifp->if_watchdog;
+ ifp->if_watchdog = trunk_port_watchdog;
+
tp->tp_if = ifp;
tp->tp_trunk = tr;
/* Save port link layer address */
bcopy(((struct arpcom *)ifp)->ac_enaddr, tp->tp_lladdr, ETHER_ADDR_LEN);
@@ -463,10 +465,13 @@ trunk_port_destroy(struct trunk_port *tp)
free(tp, M_DEVBUF, 0);
/* Update trunk capabilities */
tr->tr_capabilities = trunk_capabilities(tr);
+ /* Reestablish watchdog timeout */
+ if_slowtimo(ifp);
+
return (0);
}
void
trunk_port_watchdog(struct ifnet *ifp)
diff --git sys/net/if_var.h sys/net/if_var.h
index cd40d8b..19f4f56 100644
--- sys/net/if_var.h
+++ sys/net/if_var.h
@@ -70,10 +70,11 @@
struct mbuf;
struct proc;
struct rtentry;
struct socket;
+struct timeout;
struct ether_header;
struct arpcom;
struct rt_addrinfo;
struct ifnet;
struct hfsc_if;
@@ -145,10 +146,11 @@ struct ifnet { /* and the
entries */
u_int32_t if_hardmtu; /* maximum MTU device supports */
u_int if_rdomain; /* routing instance */
char if_description[IFDESCRSIZE]; /* interface description */
u_short if_rtlabelid; /* next route label */
u_int8_t if_priority;
+ struct timeout *if_slowtimo; /* watchdog timeout */
/* procedure handles */
/* output routine (enqueue) */
int (*if_output)(struct ifnet *, struct mbuf *, struct sockaddr *,
struct rtentry *);
