Major overhaul and cleanup of ng_source node.

Functional changes:
- Cut struct source_hookinfo. Just use hook_p pointer.
- Remove "start_now" command. "start" command now requires number of
  packets to send as argument. "start" command actually starts sending.
  Move the code that actually starts sending from ng_source_rcvmsg()
  to ng_source_start().
- Remove check for NG_SOURCE_ACTIVE in ng_source_stop(). We can be called
  with flag cleared (see begin of ng_source_intr()).
- If NG_SEND_DATA_ONLY() use log(LOG_DEBUG) instead of printf(). Otherwise
  we will *flood* console.
- Add ng_connect_t method, which sends NGM_ETHER_GET_IFNAME command
  to "output" hook. Cut ng_source_request_output_ifp(). Refactor
  ng_source_store_output_ifp() to use ifunit() and don't muck through
  interface list.
- Add "setiface" command, which gives ability to configure interface
  in case when ng_source_connect() failed. This happens, when we are not
  connected directly to ng_ether(4) node.
- Remove KASSERTs, which can never fire.
- Don't check for M_PKTHDR in rcvdata method. netgraph(4) does this
  for us.

Style:
- Assign sc_p = NG_NODE_PRIVATE(node) in declaration, to be
  consistent with style of other nodes.
- Sort variables.
- u_intXX -> uintXX.
- Dots at ends of comments.

Sponsored by:   Rambler

sys/netgraph/ng_source.c

@@ -3,6 +3,7 @@
  */
 
 /*-
+ * Copyright (c) 2005 Gleb Smirnoff <glebius@FreeBSD.org>
  * Copyright 2002 Sandvine Inc.
  * All rights reserved.
  *
@@ -64,6 +65,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/malloc.h>
 #include <sys/mbuf.h>
 #include <sys/socket.h>
+#include <sys/syslog.h>
 #include <net/if.h>
 #include <net/if_var.h>
 #include <netgraph/ng_message.h>
@@ -75,22 +77,17 @@ __FBSDID("$FreeBSD$");
 #define NG_SOURCE_INTR_TICKS		1
 #define NG_SOURCE_DRIVER_IFQ_MAXLEN	(4*1024)
 
-/* Per hook info */
-struct source_hookinfo {
-	hook_p				hook;
-};
-
 /* Per node info */
 struct privdata {
 	node_p				node;
-	struct source_hookinfo		input;
+	hook_p				input;
-	struct source_hookinfo		output;
+	hook_p				output;
 	struct ng_source_stats		stats;
 	struct ifqueue			snd_queue;	/* packets to send */
 	struct ifnet			*output_ifp;
 	struct callout			intr_ch;
-	u_int64_t			packets;	/* packets to send */
+	uint64_t			packets;	/* packets to send */
-	u_int32_t			queueOctets;
+	uint32_t			queueOctets;
 };
 typedef struct privdata *sc_p;
 
@@ -102,18 +99,17 @@ static ng_constructor_t	ng_source_constructor;
 static ng_rcvmsg_t	ng_source_rcvmsg;
 static ng_shutdown_t	ng_source_rmnode;
 static ng_newhook_t	ng_source_newhook;
+static ng_connect_t	ng_source_connect;
 static ng_rcvdata_t	ng_source_rcvdata;
 static ng_disconnect_t	ng_source_disconnect;
 
 /* Other functions */
 static void		ng_source_intr(node_p, hook_p, void *, int);
-static int		ng_source_request_output_ifp (sc_p);
 static void		ng_source_clr_data (sc_p);
-static void		ng_source_start (sc_p);
+static int		ng_source_start (sc_p, uint64_t);
 static void		ng_source_stop (sc_p);
 static int		ng_source_send (sc_p, int, int *);
-static int		ng_source_store_output_ifp(sc_p sc,
+static int		ng_source_store_output_ifp(sc_p, char *);
-			    struct ng_mesg *msg);
 
 /* Parse type for timeval */
 static const struct ng_parse_struct_field ng_source_timeval_type_fields[] = {
@@ -180,9 +176,9 @@ static const struct ng_cmdlist ng_source_cmds[] = {
 	},
 	{
 	  NGM_SOURCE_COOKIE,
-	  NGM_SOURCE_START_NOW,
+	  NGM_SOURCE_SETIFACE,
-	  "start_now",
+	  "setiface",
-	  &ng_parse_uint64_type,
+	  &ng_parse_string_type,
 	  NULL
 	},
 	{ 0 }
@@ -196,13 +192,14 @@ static struct ng_type ng_source_typestruct = {
 	.rcvmsg =	ng_source_rcvmsg,
 	.shutdown =	ng_source_rmnode,
 	.newhook =	ng_source_newhook,
+	.connect =	ng_source_connect,
 	.rcvdata =	ng_source_rcvdata,
 	.disconnect =	ng_source_disconnect,
 	.cmdlist =	ng_source_cmds,
 };
 NETGRAPH_INIT(source, &ng_source_typestruct);
 
-static int ng_source_set_autosrc(sc_p, u_int32_t);
+static int ng_source_set_autosrc(sc_p, uint32_t);
 
 /*
  * Node constructor
@@ -230,20 +227,48 @@ ng_source_constructor(node_p node)
 static int
 ng_source_newhook(node_p node, hook_p hook, const char *name)
 {
-	sc_p sc;
+	sc_p sc = NG_NODE_PRIVATE(node);
 
-	sc = NG_NODE_PRIVATE(node);
-	KASSERT(sc != NULL, ("%s: null node private", __func__));
 	if (strcmp(name, NG_SOURCE_HOOK_INPUT) == 0) {
-		sc->input.hook = hook;
+		sc->input = hook;
-		NG_HOOK_SET_PRIVATE(hook, &sc->input);
 	} else if (strcmp(name, NG_SOURCE_HOOK_OUTPUT) == 0) {
-		sc->output.hook = hook;
+		sc->output = hook;
-		NG_HOOK_SET_PRIVATE(hook, &sc->output);
 		sc->output_ifp = 0;
 		bzero(&sc->stats, sizeof(sc->stats));
 	} else
 		return (EINVAL);
+
+	return (0);
+}
+
+/*
+ * Hook has been added
+ */
+static int
+ng_source_connect(hook_p hook)
+{
+	sc_p sc = NG_NODE_PRIVATE(NG_HOOK_NODE(hook));
+	struct ng_mesg *msg;
+	int dummy_error = 0;
+
+	/*
+	 * If this is "output" hook, then request information
+	 * from our downstream.
+	 */
+	if (hook == sc->output) {
+		NG_MKMESSAGE(msg, NGM_ETHER_COOKIE, NGM_ETHER_GET_IFNAME,
+		    0, M_NOWAIT);
+		if (msg == NULL)
+			return (ENOBUFS);
+
+		/*
+		 * Our hook and peer hook have HK_INVALID flag set,
+		 * so we can't use NG_SEND_MSG_HOOK() macro here.
+		 */
+		NG_SEND_MSG_ID(dummy_error, sc->node, msg,
+		    NG_NODE_ID(NG_PEER_NODE(sc->output)), NG_NODE_ID(sc->node));
+	}
+
 	return (0);
 }
 
@@ -253,14 +278,12 @@ ng_source_newhook(node_p node, hook_p hook, const char *name)
 static int
 ng_source_rcvmsg(node_p node, item_p item, hook_p lasthook)
 {
-	sc_p sc;
+	sc_p sc = NG_NODE_PRIVATE(node);
-	struct ng_mesg *resp = NULL;
+	struct ng_mesg *msg, *resp = NULL;
 	int error = 0;
-	struct ng_mesg *msg;
 
-	sc = NG_NODE_PRIVATE(node);
 	NGI_GET_MSG(item, msg);
-	KASSERT(sc != NULL, ("%s: null node private", __func__));
+
 	switch (msg->header.typecookie) {
 	case NGM_SOURCE_COOKIE:
 		if (msg->header.flags & NGF_RESP) {
@@ -298,49 +321,37 @@ ng_source_rcvmsg(node_p node, item_p item, hook_p lasthook)
 		    break;
 		case NGM_SOURCE_START:
 		    {
-			u_int64_t packets = *(u_int64_t *)msg->data;
+			uint64_t packets;
-			if (sc->output.hook == NULL) {
-				printf("%s: start on node with no output hook\n"
-				    , __func__);
-				error = EINVAL;
-				break;
-			}
-			/* TODO validation of packets */
-			sc->packets = packets;
-			ng_source_start(sc);
-		    }
-		    break;
-		case NGM_SOURCE_START_NOW:
-		    {
-			u_int64_t packets = *(u_int64_t *)msg->data;
-			if (sc->output.hook == NULL) {
-				printf("%s: start on node with no output hook\n"
-				    , __func__);
-				error = EINVAL;
-				break;
-			}
-			if (sc->node->nd_flags & NG_SOURCE_ACTIVE) {
-				error = EBUSY;
-				break;
-			}
-			/* TODO validation of packets */
-			sc->packets = packets;
-		        sc->output_ifp = NULL;
 
-			sc->node->nd_flags |= NG_SOURCE_ACTIVE;
+			if (msg->header.arglen != sizeof(uint64_t)) {
-			timevalclear(&sc->stats.elapsedTime);
+				error = EINVAL;
-			timevalclear(&sc->stats.endTime);
-			getmicrotime(&sc->stats.startTime);
-			ng_callout(&sc->intr_ch, node, NULL, 0,
-			    ng_source_intr, sc, 0);
-		    }
 				break;
+			}
+
+			packets = *(uint64_t *)msg->data;
+
+			error = ng_source_start(sc, packets);
+
+		    	break;
+		    }
 		case NGM_SOURCE_STOP:
 			ng_source_stop(sc);
 			break;
 		case NGM_SOURCE_CLR_DATA:
 			ng_source_clr_data(sc);
 			break;
+		case NGM_SOURCE_SETIFACE:
+		    {
+			char *ifname = (char *)msg->data;
+
+			if (msg->header.arglen < 2) {
+				error = EINVAL;
+				break;
+			}
+
+			ng_source_store_output_ifp(sc, ifname);
+			break;
+		    }
 		default:
 			error = EINVAL;
 			break;
@@ -352,17 +363,19 @@ ng_source_rcvmsg(node_p node, item_p item, hook_p lasthook)
 			break;
 		}
 		switch (msg->header.cmd) {
-		case NGM_ETHER_GET_IFINDEX:
+		case NGM_ETHER_GET_IFNAME:
-			if (ng_source_store_output_ifp(sc, msg) == 0) {
+		    {
-				ng_source_set_autosrc(sc, 0);
+			char *ifname = (char *)msg->data;
-				sc->node->nd_flags |= NG_SOURCE_ACTIVE;
+
-				timevalclear(&sc->stats.elapsedTime);
+			if (msg->header.arglen < 2) {
-				timevalclear(&sc->stats.endTime);
+				error = EINVAL;
-				getmicrotime(&sc->stats.startTime);
-				ng_callout(&sc->intr_ch, node, NULL, 0,
-				    ng_source_intr, sc, 0);
-			}
 				break;
+			}
+
+			if (ng_source_store_output_ifp(sc, ifname) == 0)
+				ng_source_set_autosrc(sc, 0);
+			break;
+		    }
 		default:
 			error = EINVAL;
 		}
@@ -373,9 +386,9 @@ ng_source_rcvmsg(node_p node, item_p item, hook_p lasthook)
 	}
 
 done:
-	/* Take care of synchronous response, if any */
+	/* Take care of synchronous response, if any. */
 	NG_RESPOND_MSG(error, node, item, resp);
-	/* Free the message and return */
+	/* Free the message and return. */
 	NG_FREE_MSG(msg);
 	return (error);
 }
@@ -389,33 +402,22 @@ ng_source_rcvmsg(node_p node, item_p item, hook_p lasthook)
 static int
 ng_source_rcvdata(hook_p hook, item_p item)
 {
-	sc_p sc;
+	sc_p sc = NG_NODE_PRIVATE(NG_HOOK_NODE(hook));
-	struct source_hookinfo *hinfo;
-	int error = 0;
 	struct mbuf *m;
+	int error = 0;
 
-	sc = NG_NODE_PRIVATE(NG_HOOK_NODE(hook));
 	NGI_GET_M(item, m);
 	NG_FREE_ITEM(item);
-	hinfo = NG_HOOK_PRIVATE(hook);
-	KASSERT(sc != NULL, ("%s: null node private", __func__));
-	KASSERT(hinfo != NULL, ("%s: null hook info", __func__));
 
 	/* Which hook? */
-	if (hinfo == &sc->output) {
+	if (hook == sc->output) {
 		/* discard */
 		NG_FREE_M(m);
 		return (error);
 	}
-	KASSERT(hinfo == &sc->input, ("%s: no hook!", __func__));
+	KASSERT(hook == sc->input, ("%s: no hook!", __func__));
 
-	if ((m->m_flags & M_PKTHDR) == 0) {
+	/* Enqueue packet. */
-		printf("%s: mbuf without PKTHDR\n", __func__);
-		NG_FREE_M(m);
-		return (EINVAL);
-	}
-
-	/* enque packet */
 	/* XXX should we check IF_QFULL() ? */
 	_IF_ENQUEUE(&sc->snd_queue, m);
 	sc->queueOctets += m->m_pkthdr.len;
@@ -429,15 +431,14 @@ ng_source_rcvdata(hook_p hook, item_p item)
 static int
 ng_source_rmnode(node_p node)
 {
-	sc_p sc;
+	sc_p sc = NG_NODE_PRIVATE(node);
 
-	sc = NG_NODE_PRIVATE(node);
-	KASSERT(sc != NULL, ("%s: null node private", __func__));
 	ng_source_stop(sc);
 	ng_source_clr_data(sc);
 	NG_NODE_SET_PRIVATE(node, NULL);
 	NG_NODE_UNREF(node);
 	free(sc, M_NETGRAPH);
+
 	return (0);
 }
 
@@ -447,65 +448,26 @@ ng_source_rmnode(node_p node)
 static int
 ng_source_disconnect(hook_p hook)
 {
-	struct source_hookinfo *hinfo;
 	sc_p sc;
 
-	hinfo = NG_HOOK_PRIVATE(hook);
 	sc = NG_NODE_PRIVATE(NG_HOOK_NODE(hook));
 	KASSERT(sc != NULL, ("%s: null node private", __func__));
-	hinfo->hook = NULL;
+	if (NG_NODE_NUMHOOKS(NG_HOOK_NODE(hook)) == 0 || hook == sc->output)
-	if (NG_NODE_NUMHOOKS(NG_HOOK_NODE(hook)) == 0 || hinfo == &sc->output)
 		ng_rmnode_self(NG_HOOK_NODE(hook));
 	return (0);
 }
 
-/*
- * 
- * Ask out neighbour on the output hook side to send us it's interface
- * information.
- */
-static int
-ng_source_request_output_ifp(sc_p sc)
-{
-	struct ng_mesg *msg;
-	int error = 0;
-
-	sc->output_ifp = NULL;
-
-	/* Ask the attached node for the connected interface's index */
-	NG_MKMESSAGE(msg, NGM_ETHER_COOKIE, NGM_ETHER_GET_IFINDEX, 0, M_NOWAIT);
-	if (msg == NULL)
-		return (ENOBUFS);
-
-	NG_SEND_MSG_HOOK(error, sc->node, msg, sc->output.hook, 0);
-	return (error);
-}
-
 /*
  * Set sc->output_ifp to point to the the struct ifnet of the interface
  * reached via our output hook.
  */
 static int
-ng_source_store_output_ifp(sc_p sc, struct ng_mesg *msg)
+ng_source_store_output_ifp(sc_p sc, char *ifname)
 {
 	struct ifnet *ifp;
-	u_int32_t if_index;
 	int s;
 
-	if (msg->header.arglen < sizeof(u_int32_t))
+	ifp = ifunit(ifname);
-		return (EINVAL);
-
-	if_index = *(u_int32_t *)msg->data;
-	/* Could use ifindex2ifnet[if_index] except that we have no
-	 * way of verifying if_index is valid since if_indexlim is
-	 * local to if_attach()
-	 */
-	IFNET_RLOCK();
-	TAILQ_FOREACH(ifp, &ifnet, if_link) {
-		if (ifp->if_index == if_index)
-			break;
-	}
-	IFNET_RUNLOCK();
 
 	if (ifp == NULL) {
 		printf("%s: can't find interface %d\n", __func__, if_index);
@@ -534,18 +496,18 @@ ng_source_store_output_ifp(sc_p sc, struct ng_mesg *msg)
  * Set the attached ethernet node's ethernet source address override flag.
  */
 static int
-ng_source_set_autosrc(sc_p sc, u_int32_t flag)
+ng_source_set_autosrc(sc_p sc, uint32_t flag)
 {
 	struct ng_mesg *msg;
 	int error = 0;
 
 	NG_MKMESSAGE(msg, NGM_ETHER_COOKIE, NGM_ETHER_SET_AUTOSRC,
-	    sizeof (u_int32_t), M_NOWAIT);
+	    sizeof (uint32_t), M_NOWAIT);
 	if (msg == NULL)
 		return(ENOBUFS);
 
-	*(u_int32_t *)msg->data = flag;
+	*(uint32_t *)msg->data = flag;
-	NG_SEND_MSG_HOOK(error, sc->node, msg, sc->output.hook, 0);
+	NG_SEND_MSG_HOOK(error, sc->node, msg, sc->output, 0);
 	return (error);
 }
 
@@ -569,31 +531,40 @@ ng_source_clr_data (sc_p sc)
 /*
  * Start sending queued data out the output hook
  */
-static void
+static int
-ng_source_start (sc_p sc)
+ng_source_start(sc_p sc, uint64_t packets)
 {
-	KASSERT(sc->output.hook != NULL,
+	if (sc->output_ifp == NULL) {
-			("%s: output hook unconnected", __func__));
+		printf("ng_source: start without iface configured\n");
-	if (((sc->node->nd_flags & NG_SOURCE_ACTIVE) == 0) &&
+		return (ENXIO);
-	    (sc->output_ifp == NULL))
+	}
-		ng_source_request_output_ifp(sc);
+
+	if (sc->node->nd_flags & NG_SOURCE_ACTIVE)
+		return (EBUSY);
+
+	sc->node->nd_flags |= NG_SOURCE_ACTIVE;
+
+	sc->packets = packets;
+	timevalclear(&sc->stats.elapsedTime);
+	timevalclear(&sc->stats.endTime);
+	getmicrotime(&sc->stats.startTime);
+	ng_callout(&sc->intr_ch, sc->node, NULL, 0,
+	    ng_source_intr, sc, 0);
+
+	return (0);
 }
 
 /*
  * Stop sending queued data out the output hook
  */
 static void
-ng_source_stop (sc_p sc)
+ng_source_stop(sc_p sc)
 {
-	if (sc->node->nd_flags & NG_SOURCE_ACTIVE) {
 	ng_uncallout(&sc->intr_ch, sc->node);
 	sc->node->nd_flags &= ~NG_SOURCE_ACTIVE;
 	getmicrotime(&sc->stats.endTime);
 	sc->stats.elapsedTime = sc->stats.endTime;
 	timevalsub(&sc->stats.elapsedTime, &sc->stats.startTime);
-		/* XXX should set this to the initial value instead */
-		ng_source_set_autosrc(sc, 1);
-	}
 }
 
 /*
@@ -610,7 +581,7 @@ ng_source_intr(node_p node, hook_p hook, void *arg1, int arg2)
 
 	KASSERT(sc != NULL, ("%s: null node private", __func__));
 
-	if (sc->packets == 0 || sc->output.hook == NULL
+	if (sc->packets == 0 || sc->output == NULL
 	    || (sc->node->nd_flags & NG_SOURCE_ACTIVE) == 0) {
 		ng_source_stop(sc);
 		return;
@@ -641,12 +612,11 @@ ng_source_send (sc_p sc, int tosend, int *sent_p)
 	int sent = 0;
 	int error = 0;
 
-	KASSERT(sc != NULL, ("%s: null node private", __func__));
 	KASSERT(tosend >= 0, ("%s: negative tosend param", __func__));
 	KASSERT(sc->node->nd_flags & NG_SOURCE_ACTIVE,
 	    ("%s: inactive node", __func__));
 
-	if ((u_int64_t)tosend > sc->packets)
+	if ((uint64_t)tosend > sc->packets)
 		tosend = sc->packets;
 
 	/* Copy the required number of packets to a temporary queue */
@@ -664,10 +634,10 @@ ng_source_send (sc_p sc, int tosend, int *sent_p)
 			break;
 		}
 
-		/* re-enqueue the original packet for us */
+		/* Re-enqueue the original packet for us. */
 		_IF_ENQUEUE(&sc->snd_queue, m);
 
-		/* queue the copy for sending at smplimp */
+		/* Queue the copy for sending at splimp. */
 		_IF_ENQUEUE(&tmp_queue, m2);
 	}
 
@@ -680,9 +650,9 @@ ng_source_send (sc_p sc, int tosend, int *sent_p)
 			++sent;
 			sc->stats.outFrames++;
 			sc->stats.outOctets += m2->m_pkthdr.len;
-			NG_SEND_DATA_ONLY(error, sc->output.hook, m2);
+			NG_SEND_DATA_ONLY(error, sc->output, m2);
 			if (error)
-				printf("%s: error=%d\n", __func__, error);
+				log(LOG_DEBUG, "%s: error=%d", __func__, error);
 		} else {
 			NG_FREE_M(m2);
 		}

sys/netgraph/ng_source.h

@@ -44,7 +44,7 @@
 
 /* Node type name and magic cookie */
 #define NG_SOURCE_NODE_TYPE	"source"
-#define NGM_SOURCE_COOKIE	1034346805
+#define NGM_SOURCE_COOKIE	1110646684
 
 /* Hook names */
 #define NG_SOURCE_HOOK_INPUT	"input"
@@ -82,7 +82,7 @@ enum {
 	NGM_SOURCE_START,		/* start sending queued data */
 	NGM_SOURCE_STOP,		/* stop sending queued data */
 	NGM_SOURCE_CLR_DATA,		/* clear the queued data */
-	NGM_SOURCE_START_NOW,		/* start on non-ether output */
+	NGM_SOURCE_SETIFACE,		/* configure downstream iface */
 };
 
 #endif /* _NETGRAPH_NG_SOURCE_H_ */