Remove (almost all) global variables that were used to hold

packet forwarding state ("annotations") during ip processing.
The code is considerably cleaner now.

The variables removed by this change are:

        ip_divert_cookie        used by divert sockets
        ip_fw_fwd_addr          used for transparent ip redirection
        last_pkt                used by dynamic pipes in dummynet

Removal of the first two has been done by carrying the annotations
into volatile structs prepended to the mbuf chains, and adding
appropriate code to add/remove annotations in the routines which
make use of them, i.e. ip_input(), ip_output(), tcp_input(),
bdg_forward(), ether_demux(), ether_output_frame(), div_output().

On passing, remove a bug in divert handling of fragmented packet.
Now it is the fragment at offset 0 which sets the divert status of
the whole packet, whereas formerly it was the last incoming fragment
to decide.

Removal of last_pkt required a change in the interface of ip_fw_chk()
and dummynet_io(). On passing, use the same mechanism for dummynet
annotations and for divert/forward annotations.

option IPFIREWALL_FORWARD is effectively useless, the code to
implement it is very small and is now in by default to avoid the
obfuscation of conditionally compiled code.

NOTES:
 * there is at least one global variable left, sro_fwd, in ip_output().
   I am not sure if/how this can be removed.

 * I have deliberately avoided gratuitous style changes in this commit
   to avoid cluttering the diffs. Minor stule cleanup will likely be
   necessary

 * this commit only focused on the IP layer. I am sure there is a
   number of global variables used in the TCP and maybe UDP stack.

 * despite the number of files touched, there are absolutely no API's
   or data structures changed by this commit (except the interfaces of
   ip_fw_chk() and dummynet_io(), which are internal anyways), so
   an MFC is quite safe and unintrusive (and desirable, given the
   improved readability of the code).

MFC after: 10 days

1 files changed, 52 insertions, 35 deletions

diff --git a/sys/net/bridge.c b/sys/net/bridge.c
index 48a31cc..6fc888e 100644
--- a/sys/net/bridge.c
+++ b/sys/net/bridge.c
@@ -793,12 +793,12 @@ bridge_in(struct ifnet *ifp, struct ether_header *eh)
 static struct mbuf *
 bdg_forward(struct mbuf *m0, struct ether_header *const eh, struct ifnet *dst)
 {
-    struct ifnet *src = m0->m_pkthdr.rcvif; /* NULL when called by *_output */
-    struct ifnet *ifp, *last = NULL ;
+    struct ifnet *src;
+    struct ifnet *ifp, *last;
     int shared = bdg_copy ; /* someone else is using the mbuf */
     int once = 0;      /* loop only once */
     struct ifnet *real_dst = dst ; /* real dst from ether_output */
-    struct ip_fw *rule = NULL ; /* did we match a firewall rule ? */
+    struct ip_fw_args args;
 #ifdef PFIL_HOOKS
     struct packet_filter_hook *pfh;
     int rv;
@@ -813,16 +813,18 @@ bdg_forward(struct mbuf *m0, struct ether_header *const eh, struct ifnet *dst)
 
     DEB(quad_t ticks; ticks = rdtsc();)
 
-    if (m0->m_type == MT_DUMMYNET) {
-	/* extract info from dummynet header */
-	rule = (struct ip_fw *)(m0->m_data) ;
-	m0 = m0->m_next ;
-	src = m0->m_pkthdr.rcvif;
-	shared = 0 ; /* For sure this is our own mbuf. */
-    } else
-	bdg_thru++; /* count packet, only once */
+    args.rule = NULL;		/* did we match a firewall rule ? */
+    /* Fetch state from dummynet tag, ignore others */
+    for (;m0->m_type == MT_TAG; m0 = m0->m_next)
+	if (m0->m_tag_id == PACKET_TAG_DUMMYNET) {
+	    args.rule = ((struct dn_pkt *)m0)->rule;
+	    shared = 0;		/* For sure this is our own mbuf. */
+	}
+    if (args.rule == NULL)
+	bdg_thru++; /* first time through bdg_forward, count packet */
 
-    if (src == NULL) /* packet from ether_output */
+    src = m0->m_pkthdr.rcvif;
+    if (src == NULL)			/* packet from ether_output */
 	dst = bridge_dst_lookup(eh, ifp2sc[real_dst->if_index].cluster);
 
     if (dst == BDG_DROP) { /* this should not happen */
@@ -861,7 +863,7 @@ bdg_forward(struct mbuf *m0, struct ether_header *const eh, struct ifnet *dst)
 
 	int i;
 
-	if (rule != NULL) /* dummynet packet, already partially processed */
+	if (args.rule != NULL) /* packet already partially processed */
 	    goto forward; /* HACK! I should obey the fw_one_pass */
 	/*
 	 * i need some amt of data to be contiguous, and in case others need
@@ -910,15 +912,24 @@ bdg_forward(struct mbuf *m0, struct ether_header *const eh, struct ifnet *dst)
 #endif /* PFIL_HOOKS */
 
 	/*
-	 * The second parameter to the firewall code is the dst. interface.
-	 * Since we apply checks only on input pkts we use NULL.
-	 * The firewall knows this is a bridged packet as the cookie ptr
-	 * is NULL.
+	 * Prepare arguments and call the firewall.
 	 */
 	if (!IPFW_LOADED || bdg_ipfw == 0)
 	    goto forward;	/* not using ipfw, accept the packet */
-	i = ip_fw_chk_ptr(&m0, NULL, NULL /* cookie */, &rule,
-		(struct sockaddr_in **)&save_eh);
+
+	/*
+	 * XXX The following code is very similar to the one in
+	 * if_ethersubr.c:ether_ipfw_chk()
+	 */
+
+	args.m = m0;		/* the packet we are looking at 	*/
+	args.oif = NULL;	/* this is an input packet 		*/
+	args.divert_rule = 0;	/* we do not support divert yet 	*/
+	args.next_hop = NULL;	/* we do not support forward yet	*/
+	args.eh = &save_eh;	/* MAC header for bridged/MAC packets	*/
+	i = ip_fw_chk_ptr(&args);
+	m0 = args.m;		/* in case the firewall used the mbuf	*/
+
 	if ( (i & IP_FW_PORT_DENY_FLAG) || m0 == NULL) /* drop */
 	    return m0 ;
 
@@ -928,20 +939,21 @@ bdg_forward(struct mbuf *m0, struct ether_header *const eh, struct ifnet *dst)
 	    /*
 	     * Pass the pkt to dummynet, which consumes it.
 	     * If shared, make a copy and keep the original.
-	     * Need to prepend the ethernet header, optimize the common
-	     * case of eh pointing already into the original mbuf.
 	     */
 	    struct mbuf *m ;
+
 	    if (shared) {
 		m = m_copypacket(m0, M_DONTWAIT);
-		if (m == NULL) {
-		    printf("bdg_fwd: copy(1) failed\n");
+		if (m == NULL)	/* copy failed, give up */
 		    return m0;
-		}
 	    } else {
 		m = m0 ; /* pass the original to dummynet */
 		m0 = NULL ; /* and nothing back to the caller */
 	    }
+	    /*
+	     * Prepend the header, optimize for the common case of
+	     * eh pointing into the mbuf.
+	     */
 	    if ( (void *)(eh + 1) == (void *)m->m_data) {
 		m->m_data -= ETHER_HDR_LEN ;
 		m->m_len += ETHER_HDR_LEN ;
@@ -949,21 +961,20 @@ bdg_forward(struct mbuf *m0, struct ether_header *const eh, struct ifnet *dst)
 		bdg_predict++;
 	    } else {
 		M_PREPEND(m, ETHER_HDR_LEN, M_DONTWAIT);
-		if (!m && verbose)
-		    printf("M_PREPEND failed\n");
 		if (m == NULL) /* nope... */
 		    return m0 ;
 		bcopy(&save_eh, mtod(m, struct ether_header *), ETHER_HDR_LEN);
 	    }
-	    ip_dn_io_ptr((i & 0xffff),DN_TO_BDG_FWD,m,real_dst,NULL,0,rule,0);
+
+	    args.oif = real_dst;
+	    ip_dn_io_ptr(m, (i & 0xffff),DN_TO_BDG_FWD, &args);
 	    return m0 ;
 	}
 	/*
-	 * XXX add divert/forward actions...
+	 * XXX at some point, add support for divert/forward actions.
+	 * If none of the above matches, we have to drop the packet.
 	 */
-	/* if none of the above matches, we have to drop the pkt */
 	bdg_ipfw_drops++ ;
-	printf("bdg_forward: No rules match, so dropping packet!\n");
 	return m0 ;
     }
 forward:
@@ -975,14 +986,20 @@ forward:
 	int i = min(m0->m_pkthdr.len, max_protohdr) ;
 
 	m0 = m_pullup(m0, i) ;
-	if (m0 == NULL) {
-	    printf("-- bdg: pullup2 failed.\n") ;
+	if (m0 == NULL)
 	    return NULL ;
-	}
     }
-    /* now real_dst is used to determine the cluster where to forward */
-    if (src != NULL) /* pkt comes from ether_input */
+    /*
+     * now real_dst is used to determine the cluster where to forward.
+     * For packets coming from ether_input, this is the one of the 'src'
+     * interface, whereas for locally generated packets (src==NULL) it
+     * is the cluster of the original destination interface, which
+     * was already saved into real_dst.
+     */
+    if (src != NULL)
 	real_dst = src ;
+
+    last = NULL;
     for (;;) {
 	if (last) { /* need to forward packet leftover from previous loop */
 	    struct mbuf *m ;