PERFORCE change 43027 for review

Sam Leffler sam at FreeBSD.org
Mon Nov 24 20:24:28 PST 2003


http://perforce.freebsd.org/chv.cgi?CH=43027

Change 43027 by sam at sam_ebb on 2003/11/24 20:23:57

	Poof! MT_TAG is gone.  Replace the last vestiges with m_tag's
	or, in the case of PACKET_TAG_IPFASTFWD_OURS (which should
	never have been created) use an mbuf proto flag.

Affected files ...

.. //depot/projects/netperf/sys/netinet/ip_fastfwd.c#5 edit
.. //depot/projects/netperf/sys/netinet/ip_input.c#27 edit
.. //depot/projects/netperf/sys/netinet/ip_output.c#20 edit
.. //depot/projects/netperf/sys/netinet/ip_var.h#15 edit
.. //depot/projects/netperf/sys/netinet/tcp_input.c#14 edit
.. //depot/projects/netperf/sys/sys/mbuf.h#9 edit

Differences ...

==== //depot/projects/netperf/sys/netinet/ip_fastfwd.c#5 (text+ko) ====

@@ -133,7 +133,7 @@
 	struct ip *tip;
 	struct mbuf *teem = NULL;
 #endif
-	struct mbuf *tag = NULL;
+	struct m_tag *mtag;
 	struct route ro;
 	struct sockaddr_in *dst = NULL;
 	struct in_ifaddr *ia = NULL;
@@ -151,16 +151,6 @@
 	if (!ipfastforward_active || !ipforwarding)
 		return 0;
 
-	/*
-	 * If there is any MT_TAG we fall back to ip_input because we can't
-	 * handle TAGs here. Should never happen as we get directly called
-	 * from the if_output routines.
-	 */
-	if (m->m_type == MT_TAG) {
-		KASSERT(0, ("%s: packet with MT_TAG not expected", __func__));
-		return 0;
-	}
-
 	M_ASSERTVALID(m);
 	M_ASSERTPKTHDR(m);
 
@@ -613,38 +603,24 @@
 			if (IA_SIN(ia)->sin_addr.s_addr == ip->ip_dst.s_addr) {
 forwardlocal:
 				if (args.next_hop) {
-					/* XXX leak */
-					MGETHDR(tag, M_DONTWAIT, MT_TAG);
-					if (tag == NULL) {
+					mtag = m_tag_get(PACKET_TAG_IPFORWARD,
+						   sizeof(struct sockaddr_in *),
+						   M_NOWAIT);
+					if (mtag == NULL) {
+						/* XXX statistic */
 						if (ro.ro_rt)
 							RTFREE(ro.ro_rt);
 						goto drop;
 					}
-					tag->m_flags = PACKET_TAG_IPFORWARD;
-					tag->m_data = (caddr_t)args.next_hop;
-					tag->m_next = m;
-					/* XXX: really bloody hack,
-					 * see ip_input */
-					tag->m_nextpkt = (struct mbuf *)1;
-					m = tag;
-					tag = NULL;
+					*(struct sockaddr_in **)(mtag+1) =
+						args.next_hop;
+					m_tag_prepend(m, mtag);
 				}
 #ifdef IPDIVERT
 droptoours:	/* Used for DIVERT */
 #endif
-				MGETHDR(tag, M_DONTWAIT, MT_TAG);
-				if (tag == NULL) {
-					if (ro.ro_rt)
-						RTFREE(ro.ro_rt);
-					goto drop;
-				}
-				tag->m_flags = PACKET_TAG_IPFASTFWD_OURS;
-				tag->m_data = NULL;
-				tag->m_next = m;
-				/* XXX: really bloody hack, see ip_input */
-				tag->m_nextpkt = (struct mbuf *)1;
-				m = tag;
-				tag = NULL;
+				/* NB: ip_input understands this */
+				m->m_flags |= M_FASTFWD_OURS;
 
 				/* ip still points to the real packet */
 				ip->ip_len = htons(ip->ip_len);

==== //depot/projects/netperf/sys/netinet/ip_input.c#27 (text+ko) ====

@@ -300,9 +300,9 @@
 	struct in_ifaddr *ia = NULL;
 	struct ifaddr *ifa;
 	int    i, checkif, hlen = 0;
-	int    ours = 0;
 	u_short sum;
 	struct in_addr pkt_dst;
+	struct m_tag *mtag;
 #ifdef IPDIVERT
 	u_int32_t divert_info;			/* packet divert/tee info */
 #endif
@@ -312,7 +312,6 @@
 	struct in_addr odst;			/* original dst address */
 #endif
 #ifdef FAST_IPSEC
-	struct m_tag *mtag;
 	struct tdb_ident *tdbi;
 	struct secpolicy *sp;
 	int s, error;
@@ -320,48 +319,15 @@
 
 	args.eh = NULL;
 	args.oif = NULL;
-	args.next_hop = NULL;
-
-	/*
-	 * Grab info from MT_TAG mbufs prepended to the chain.
-	 *
-	 * XXX: This is ugly. These pseudo mbuf prepend tags should really
-	 * be real m_tags.  Before these have always been allocated on the
-	 * callers stack, so we didn't have to free them.  Now with
-	 * ip_fastforward they are true mbufs and we have to free them
-	 * otherwise we have a leak.  Must rewrite ipfw to use m_tags.
-	 */
-	for (; m && m->m_type == MT_TAG;) {
-		struct mbuf *m0;
-
-		switch(m->_m_tag_id) {
-		default:
-			printf("ip_input: unrecognised MT_TAG tag %d\n",
-			    m->_m_tag_id);
-			break;
-
-		case PACKET_TAG_IPFORWARD:
-			args.next_hop = (struct sockaddr_in *)m->m_hdr.mh_data;
-			break;
-
-		case PACKET_TAG_IPFASTFWD_OURS:
-			ours = 1;
-			break;
-		}
-
-		m0 = m;
-		m = m->m_next;
-		/* XXX: This is set by ip_fastforward */
-		if (m0->m_nextpkt == (struct mbuf *)1)
-			m_free(m0);
-	}
+	args.next_hop = ip_claim_next_hop(m);
 	args.rule = ip_dn_find_rule(m);
 
 	M_ASSERTPKTHDR(m);
 
-	if (ours)		/* ip_fastforward firewall changed dest to local */
+	if (m->m_flags & M_FASTFWD_OURS) {
+		/* ip_fastforward firewall changed dest to local */
 		goto ours;
-
+	}
 	if (args.rule) {	/* dummynet already filtered us */
 		ip = mtod(m, struct ip *);
 		hlen = ip->ip_hl << 2;
@@ -960,19 +926,18 @@
 	ipstat.ips_delivered++;
 	NET_PICKUP_GIANT();
 	if (args.next_hop && ip->ip_p == IPPROTO_TCP) {
-		/* TCP needs IPFORWARD info if available */
-		struct m_hdr tag;
-
-		tag.mh_type = MT_TAG;
-		tag.mh_flags = PACKET_TAG_IPFORWARD;
-		tag.mh_data = (caddr_t)args.next_hop;
-		tag.mh_next = m;
-		tag.mh_nextpkt = NULL;
-
-		(*inetsw[ip_protox[ip->ip_p]].pr_input)(
-			(struct mbuf *)&tag, hlen);
-	} else
-		(*inetsw[ip_protox[ip->ip_p]].pr_input)(m, hlen);
+		/* attach next hop info for TCP */
+		mtag = m_tag_get(PACKET_TAG_IPFORWARD,
+				sizeof(struct sockaddr_in *), M_NOWAIT);
+		if (mtag == NULL) {
+			/* XXX statistic */
+			NET_DROP_GIANT();
+			goto bad;
+		}
+		*(struct sockaddr_in **)(mtag+1) = args.next_hop;
+		m_tag_prepend(m, mtag);
+	}
+	(*inetsw[ip_protox[ip->ip_p]].pr_input)(m, hlen);
 	NET_DROP_GIANT();
 	return;
 bad:
@@ -1755,6 +1720,7 @@
 	struct in_ifaddr *ia;
 	int error, type = 0, code = 0;
 	struct mbuf *mcopy;
+	struct m_tag *mtag;
 	n_long dest;
 	struct in_addr pkt_dst;
 	struct ifnet *destifp;
@@ -1887,21 +1853,18 @@
 			RTFREE(rt);
 	}
 
-    {
-	struct m_hdr tag;
-
 	if (next_hop) {
-		/* Pass IPFORWARD info if available */
- 
-		tag.mh_type = MT_TAG;
-		tag.mh_flags = PACKET_TAG_IPFORWARD;
-		tag.mh_data = (caddr_t)next_hop;
-		tag.mh_next = m;
-		tag.mh_nextpkt = NULL;
-		m = (struct mbuf *)&tag;
+		mtag = m_tag_get(PACKET_TAG_IPFORWARD,
+				sizeof(struct sockaddr_in *), M_NOWAIT);
+		if (mtag == NULL) {
+			/* XXX statistic */
+			m_freem(m);
+			return;
+		}
+		*(struct sockaddr_in **)(mtag+1) = next_hop;
+		m_tag_prepend(m, mtag);
 	}
 	error = ip_output(m, (struct mbuf *)0, NULL, IP_FORWARDING, 0, NULL);
-    }
 	if (error)
 		ipstat.ips_cantforward++;
 	else {

==== //depot/projects/netperf/sys/netinet/ip_output.c#20 (text+ko) ====

@@ -131,12 +131,11 @@
  * inserted, so must have a NULL opt pointer.
  */
 int
-ip_output(struct mbuf *m0, struct mbuf *opt, struct route *ro,
+ip_output(struct mbuf *m, struct mbuf *opt, struct route *ro,
 	int flags, struct ip_moptions *imo, struct inpcb *inp)
 {
 	struct ip *ip;
 	struct ifnet *ifp = NULL;	/* keep compiler happy */
-	struct mbuf *m;
 	int hlen = sizeof (struct ip);
 	int len, off, error = 0;
 	struct sockaddr_in *dst = NULL;	/* keep compiler happy */
@@ -145,12 +144,13 @@
 	struct in_addr pkt_dst;
 	struct route iproute;
 	struct m_tag *dummytag;		/* dummynet packet tag */
+	struct m_tag *mtag;
+	struct mbuf *m0;		/* XXX */
 #ifdef IPSEC
 	struct socket *so;
 	struct secpolicy *sp = NULL;
 #endif
 #ifdef FAST_IPSEC
-	struct m_tag *mtag;
 	struct secpolicy *sp = NULL;
 	struct tdb_ident *tdbi;
 	int s;
@@ -160,22 +160,7 @@
 
 	args.eh = NULL;
 	args.rule = NULL;
-	args.next_hop = NULL;
-
-	/* Grab info from MT_TAG mbufs prepended to the chain. */
-	for (; m0 && m0->m_type == MT_TAG; m0 = m0->m_next) {
-		switch(m0->_m_tag_id) {
-		default:
-			printf("ip_output: unrecognised MT_TAG tag %d\n",
-			    m0->_m_tag_id);
-			break;
-
-		case PACKET_TAG_IPFORWARD:
-			args.next_hop = (struct sockaddr_in *)m0->m_data;
-			break;
-		}
-	}
-	m = m0;
+	args.next_hop = ip_claim_next_hop(m);
 
 #ifdef IPSEC
 	so = ipsec_getsocket(m);
@@ -916,26 +901,31 @@
 					break;
 			}
 			if (ia) {	/* tell ip_input "dont filter" */
-				struct m_hdr tag;
+				mtag = m_tag_get(PACKET_TAG_IPFORWARD,
+						sizeof(struct sockaddr_in *),
+						M_NOWAIT);
+				if (mtag == NULL) {
+					/* XXX statistic */
+					error = ENOBUFS;	/* XXX */
+					goto bad;
+				}
+				*(struct sockaddr_in **)(mtag+1) =
+					args.next_hop;
+				m_tag_prepend(m, mtag);
 
-				tag.mh_type = MT_TAG;
-				tag.mh_flags = PACKET_TAG_IPFORWARD;
-				tag.mh_data = (caddr_t)args.next_hop;
-				tag.mh_next = m;
-				tag.mh_nextpkt = NULL;
-
 				if (m->m_pkthdr.rcvif == NULL)
 					m->m_pkthdr.rcvif = ifunit("lo0");
 				if (m->m_pkthdr.csum_flags & CSUM_DELAY_DATA) {
 					m->m_pkthdr.csum_flags |=
 					    CSUM_DATA_VALID | CSUM_PSEUDO_HDR;
-					m0->m_pkthdr.csum_data = 0xffff;
+					m->m_pkthdr.csum_data = 0xffff;
 				}
 				m->m_pkthdr.csum_flags |=
 				    CSUM_IP_CHECKED | CSUM_IP_VALID;
 				ip->ip_len = htons(ip->ip_len);
 				ip->ip_off = htons(ip->ip_off);
-				ip_input((struct mbuf *)&tag);
+				/* XXX netisr_queue(NETISR_IP, m); */
+				ip_input(m);
 				goto done;
 			}
 			/*

==== //depot/projects/netperf/sys/netinet/ip_var.h#15 (text+ko) ====

@@ -195,6 +195,23 @@
 extern void	(*ip_rsvp_force_done)(struct socket *);
 extern void	(*rsvp_input_p)(struct mbuf *m, int off);
 
+#define	M_FASTFWD_OURS	M_PROTO1	/* sent by ip_fastforward to ip_input */
+
+/*
+ * Return the next hop address associated with the mbuf; if any.
+ * If a tag is present it is also removed.
+ */
+static __inline struct sockaddr_in *
+ip_claim_next_hop(struct mbuf *m)
+{
+	struct m_tag *mtag = m_tag_find(m, PACKET_TAG_IPFORWARD, NULL);
+	if (mtag) {
+		struct sockaddr_in *sin = *(struct sockaddr_in **)(mtag+1);
+		m_tag_delete(m, mtag);
+		return sin;
+	} else
+		return NULL;
+}
 
 #ifdef PFIL_HOOKS
 extern	struct pfil_head inet_pfil_hook;

==== //depot/projects/netperf/sys/netinet/tcp_input.c#14 (text+ko) ====

@@ -359,7 +359,7 @@
 	struct tcpopt to;		/* options in this segment */
 	struct rmxp_tao tao;		/* our TAO cache entry */
 	int headlocked = 0;
-	struct sockaddr_in *next_hop = NULL;
+	struct sockaddr_in *next_hop;
 	int rstreason; /* For badport_bandlim accounting purposes */
 
 	struct ip6_hdr *ip6 = NULL;
@@ -379,11 +379,7 @@
 	short ostate = 0;
 #endif
 
-	/* Grab info from MT_TAG mbufs prepended to the chain. */
-	for (;m && m->m_type == MT_TAG; m = m->m_next) { 
-		if (m->_m_tag_id == PACKET_TAG_IPFORWARD)
-			next_hop = (struct sockaddr_in *)m->m_hdr.mh_data;
-	}
+	next_hop = ip_claim_next_hop(m);
 #ifdef INET6
 	isipv6 = (mtod(m, struct ip *)->ip_v == 6) ? 1 : 0;
 #endif

==== //depot/projects/netperf/sys/sys/mbuf.h#9 (text+ko) ====

@@ -220,8 +220,8 @@
 #if 0
 #define	MT_RIGHTS	12	/* access rights */
 #define	MT_IFADDR	13	/* interface address */
+#define	MT_TAG		13	/* deprecated: use m_tag's instead */
 #endif
-#define	MT_TAG		13	/* volatile metadata associated to pkts */
 #define	MT_CONTROL	14	/* extra-data protocol message */
 #define	MT_OOBDATA	15	/* expedited data  */
 #define	MT_NTYPES	16	/* number of mbuf types for mbtypes[] */
@@ -538,28 +538,11 @@
 #define	PACKET_TAG_IPSEC_SOCKET			12 /* IPSEC socket ref */
 #define	PACKET_TAG_IPSEC_HISTORY		13 /* IPSEC history */
 #define	PACKET_TAG_IPV6_INPUT			14 /* IPV6 input processing */
-
-/*
- * As a temporary and low impact solution to replace the even uglier
- * approach used so far in some parts of the network stack (which relies
- * on global variables), packet tag-like annotations are stored in MT_TAG
- * mbufs (or lookalikes) prepended to the actual mbuf chain.
- *
- *	m_type	= MT_TAG
- *	m_flags = m_tag_id
- *	m_next	= next buffer in chain.
- *
- * BE VERY CAREFUL not to pass these blocks to the mbuf handling routines.
- */
-#define	_m_tag_id	m_hdr.mh_flags
-
-/* Packet tags used in the FreeBSD network stack. */
 #define	PACKET_TAG_DUMMYNET			15 /* dummynet info */
 #define	PACKET_TAG_IPFW				16 /* ipfw classification */
 #define	PACKET_TAG_DIVERT			17 /* divert info */
 #define	PACKET_TAG_IPFORWARD			18 /* ipforward info */
 #define	PACKET_TAG_MACLABEL	(19 | MTAG_PERSISTENT) /* MAC label */
-#define	PACKET_TAG_IPFASTFWD_OURS		20 /* IP fastforward dropback */
 
 /* Packet tag routines. */
 struct	m_tag 	*m_tag_alloc(u_int32_t, int, int, int);


More information about the p4-projects mailing list