In the splnet times, netgraph was functional and synchronous. Nowadays,

an item may be queued and processed later. While this is OK for mbufs,
this is a problem for control messages.

In the framework:
- Add optional callback function pointer to an item. When item gets
  applied the callback is executed from ng_apply_item().
- Add new flag NG_PROGRESS. If this flag is supplied, then return
  EINPROGRESS instead of 0 in case if item failed to deliver
  synchronously and was queued.
- Honor NG_PROGRESS in ng_snd_item().

In ng_socket:
- When userland sends control message add callback to the item.
- If ng_snd_item() returns EINPROGRESS, then sleep.

This change fixes possible races in ngctl(8) scripts.

Reviewed by:	julian
Approved by:	re (scottl)

sys/netgraph/netgraph.h

@@ -580,6 +580,7 @@ _ng_node_foreach_hook(node_p node, ng_fn_eachhook *fn, void *arg,
  *
  */
 typedef	void	ng_item_fn(node_p node, hook_p hook, void *arg1, int arg2);
+typedef	void	ng_apply_t(void *context, int error);
 struct ng_item {
 	u_long	el_flags;
 	item_p	el_next;
@@ -597,6 +598,12 @@ struct ng_item {
 			int		fn_arg2;
 		} fn;
 	} body;
+	/*
+	 * Optional callback called when item is being applied,
+	 * and its context.
+	 */
+	ng_apply_t	*apply;
+	void		*context;
 #ifdef	NETGRAPH_DEBUG /*----------------------------------------------*/
 	char *lastfile;
 	int  lastline;
@@ -1084,6 +1091,7 @@ int	ng_callout(struct callout *c, node_p node, hook_p hook, int ticks,
 #define	NG_NOFLAGS	0x00000000	/* no special options */
 #define	NG_QUEUE	0x00000001	/* enqueue item, don't dispatch */
 #define	NG_WAITOK	0x00000002	/* use M_WAITOK, etc. */
+#define	NG_PROGRESS	0x00000004	/* return EINPROGRESS if queued */
 
 /*
  * prototypes the user should DEFINITELY not use directly

sys/netgraph/ng_base.c

@@ -2211,7 +2211,11 @@ ng_snd_item(item_p item, int flags)
 			ng_setisr(node);
 		}
 		mtx_unlock_spin(&(ngq->q_mtx));
-		return (0);
+
+		if (flags & NG_PROGRESS)
+			return (EINPROGRESS);
+		else
+			return (0);
 	}
 	/*
 	 * Take a queue item and a node and see if we can apply the item to
@@ -2237,7 +2241,10 @@ ng_snd_item(item_p item, int flags)
 	 * have been queued in thises cases.
 	 */
 	if (item == NULL) {
-		return (0);
+		if (flags & NG_PROGRESS)
+			return (EINPROGRESS);
+		else
+			return (0);
 	}
 
 #ifdef	NETGRAPH_DEBUG
@@ -2333,11 +2340,25 @@ ng_apply_item(node_p node, item_p item)
 	int	error = 0;
 	ng_rcvdata_t *rcvdata;
 	ng_rcvmsg_t *rcvmsg;
+	ng_apply_t *apply = NULL;
+	void	*context = NULL;
 
 	NGI_GET_HOOK(item, hook); /* clears stored hook */
 #ifdef	NETGRAPH_DEBUG
         _ngi_check(item, __FILE__, __LINE__);
 #endif
+
+	/*
+	 * If item has apply callback, store it. Clear callback
+	 * immediately, two avoid another call in case if
+	 * item would be reused by destination node.
+	 */
+	if (item->apply != NULL) {
+		apply = item->apply;
+		context = item->context;
+		item->apply = NULL;
+	}
+
 	switch (item->el_flags & NGQF_TYPE) {
 	case NGQF_DATA:
 		/*
@@ -2453,6 +2474,11 @@ ng_apply_item(node_p node, item_p item)
 	} else {
 		ng_leave_write(&node->nd_input_queue);
 	}
+
+	/* Apply callback. */
+	if (apply != NULL)
+		(*apply)(context, error);
+
 	return (error);
 }
 

sys/netgraph/ng_socket.c

@@ -131,6 +131,7 @@ static int	ng_bind(struct sockaddr *nam, struct ngpcb *pcbp);
 static int	ngs_mod_event(module_t mod, int event, void *data);
 static int	ship_msg(struct ngpcb *pcbp, struct ng_mesg *msg,
 			struct sockaddr_ng *addr);
+static void	ng_socket_item_applied(void *context, int error);
 
 /* Netgraph type descriptor */
 static struct ng_type typestruct = {
@@ -208,6 +209,7 @@ ngc_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr,
 	 struct mbuf *control, struct thread *td)
 {
 	struct ngpcb *const pcbp = sotongpcb(so);
+	struct ngsock *const priv = NG_NODE_PRIVATE(pcbp->sockdata->node);
 	struct sockaddr_ng *const sap = (struct sockaddr_ng *) addr;
 	struct ng_mesg *msg;
 	struct mbuf *m0;
@@ -332,7 +334,29 @@ ngc_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr,
 		item->el_dest->nd_type->name);
 #endif
 	SAVE_LINE(item);
-	error = ng_snd_item(item, NG_NOFLAGS);
+	/*
+	 * We do not want to return from syscall until the item
+	 * is processed by destination node. We register callback
+	 * on the item, which will update priv->error when item
+	 * was applied.
+	 * If ng_snd_item() has queued item, we sleep until
+	 * callback wakes us up.
+	 */
+	item->apply = ng_socket_item_applied;
+	item->context = priv;
+	priv->error = -1;
+
+	error = ng_snd_item(item, NG_PROGRESS);
+
+	if (error == EINPROGRESS) { 
+		mtx_lock(&priv->mtx);
+		if (priv->error == -1)
+			msleep(priv, &priv->mtx, 0, "ngsock", 0);
+		mtx_unlock(&priv->mtx);
+		KASSERT(priv->error != -1,
+		    ("ng_socket: priv->error wasn't updated"));
+		error = priv->error;
+	}
 
 release:
 	if (path != NULL)
@@ -553,6 +577,8 @@ ng_attach_cntl(struct socket *so)
 	}
 	NG_NODE_SET_PRIVATE(privdata->node, privdata);
 
+	mtx_init(&privdata->mtx, "ng_socket", NULL, MTX_DEF);
+
 	/* Link the pcb and the node private data */
 	privdata->ctlsock = pcbp;
 	pcbp->sockdata = privdata;
@@ -813,6 +839,10 @@ ship_msg(struct ngpcb *pcbp, struct ng_mesg *msg, struct sockaddr_ng *addr)
 	return (0);
 }
 
+/***************************************************************
+	Netgraph node
+***************************************************************/
+
 /*
  * You can only create new nodes from the socket end of things.
  */
@@ -1020,10 +1050,23 @@ ngs_shutdown(node_p node)
 	}
 	NG_NODE_SET_PRIVATE(node, NULL);
 	NG_NODE_UNREF(node);
+	mtx_destroy(&priv->mtx);
 	FREE(priv, M_NETGRAPH_SOCK);
 	return (0);
 }
 
+static void
+ng_socket_item_applied(void *context, int error)
+{
+	struct ngsock *const priv = (struct ngsock *)context;
+
+	mtx_lock(&priv->mtx);
+	priv->error = error;
+	wakeup(priv);
+	mtx_unlock(&priv->mtx);
+
+}
+
 static	int
 dummy_disconnect(struct socket *so)
 {

sys/netgraph/ng_socketvar.h

@@ -59,6 +59,8 @@ struct ngsock {
 	struct ngpcb	*ctlsock;	/* optional control socket */
 	int    flags;
 	int    refs;
+	struct mtx	mtx;		/* mtx to wait on */
+	int		error;		/* place to store error */
 };
 #define	NGS_FLAG_NOLINGER	1	/* close with last hook */