svn commit: r201057 - in user/luigi/ipfw3-head/sys/netinet: . ipfw

Luigi Rizzo luigi at FreeBSD.org
Sun Dec 27 19:17:03 UTC 2009


Author: luigi
Date: Sun Dec 27 19:17:03 2009
New Revision: 201057
URL: http://svn.freebsd.org/changeset/base/201057

Log:
  Historically, BSD keeps ip_len and ip_off in host format when doing
  layer 3 processing.  This often requires to translate the format
  back and forth.
  
  I suppose the original motivation was to avoid carrying additional
  metadata, and/or save a couple of local variables to store off and len.
  
  My impression is that nowadays there are much bigger disadvantages:
  - many unnecessary writes to the mbuf, which make it harder to
    share the data, and also cause cache traffic;
  - visual clutter in the code;
  - portability problems, either across layers (e.g. we cannot
    use the same code to do deep packet inspection for L2 and L3
    packets) or across operating systems;
  - potential source of confusion, because the format is different
    in different parts of the code.
  
  Eventually, I would like these fields to remain in network format
  across the lifetime of a packet, but this may take a long time,
  so let's see if we can find ways to at least make the switch easier.
  
  Here I define a couple of macros that do the format conversion
  back and forth. On BIG_ENDIAN machines, or when we will switch,
  they become no-ops.
  
  At the moment I am only using these macros in the ipfw code,
  I am not sure this is the best solution across the entire stack.
  
      #if (BYTE_ORDER == BIG_ENDIAN) || defined(HAVE_NET_IPLEN)
      #define SET_NET_IPLEN(p)       do {} while (0)
      #define SET_HOST_IPLEN(p)      do {} while (0)
      #else
      #define SET_NET_IPLEN(p)       do {         \
          struct ip *h_ip = (p);                  \
          h_ip->ip_len = htons(h_ip->ip_len);     \
          h_ip->ip_off = htons(h_ip->ip_off);     \
          } while (0)
  
      #define SET_HOST_IPLEN(p)      do {         \
          struct ip *h_ip = (p);                  \
          h_ip->ip_len = ntohs(h_ip->ip_len);     \
          h_ip->ip_off = ntohs(h_ip->ip_off);     \
          } while (0)
      #endif /* !HAVE_NET_IPLEN */

Modified:
  user/luigi/ipfw3-head/sys/netinet/in.h
  user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dummynet.c
  user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw2.c
  user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_dynamic.c
  user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_log.c
  user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_nat.c
  user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_pfil.c

Modified: user/luigi/ipfw3-head/sys/netinet/in.h
==============================================================================
--- user/luigi/ipfw3-head/sys/netinet/in.h	Sun Dec 27 18:32:44 2009	(r201056)
+++ user/luigi/ipfw3-head/sys/netinet/in.h	Sun Dec 27 19:17:03 2009	(r201057)
@@ -734,6 +734,32 @@ void	 in_ifdetach(struct ifnet *);
 #define	sintosa(sin)	((struct sockaddr *)(sin))
 #define	ifatoia(ifa)	((struct in_ifaddr *)(ifa))
 
+/*
+ * Historically, BSD keeps ip_len and ip_off in host format
+ * when doing layer 3 processing, and this often requires
+ * to translate the format back and forth.
+ * To make the process explicit, we define a couple of macros
+ * that also take into account the fact that at some point
+ * we may want to keep those fields always in net format.
+ */
+
+#if (BYTE_ORDER == BIG_ENDIAN) || defined(HAVE_NET_IPLEN)
+#define SET_NET_IPLEN(p)	do {} while (0)
+#define SET_HOST_IPLEN(p)	do {} while (0)
+#else
+#define SET_NET_IPLEN(p)	do {		\
+	struct ip *h_ip = (p);			\
+	h_ip->ip_len = htons(h_ip->ip_len);	\
+	h_ip->ip_off = htons(h_ip->ip_off);	\
+	} while (0)
+
+#define SET_HOST_IPLEN(p)	do {		\
+	struct ip *h_ip = (p);			\
+	h_ip->ip_len = ntohs(h_ip->ip_len);	\
+	h_ip->ip_off = ntohs(h_ip->ip_off);	\
+	} while (0)
+#endif /* !HAVE_NET_IPLEN */
+
 #endif /* _KERNEL */
 
 /* INET6 stuff */

Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dummynet.c
==============================================================================
--- user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dummynet.c	Sun Dec 27 18:32:44 2009	(r201056)
+++ user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dummynet.c	Sun Dec 27 19:17:03 2009	(r201057)
@@ -990,10 +990,7 @@ dummynet_send(struct mbuf *m)
 			break ;
 		case DIR_IN :
 			ip = mtod(m, struct ip *);
-#ifndef HAVE_NET_IPLEN
-			ip->ip_len = htons(ip->ip_len);
-			ip->ip_off = htons(ip->ip_off);
-#endif /* !HAVE_NET_IPLEN */
+			SET_NET_IPLEN(ip);
 			netisr_dispatch(NETISR_IP, m);
 			break;
 #ifdef INET6

Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw2.c
==============================================================================
--- user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw2.c	Sun Dec 27 18:32:44 2009	(r201056)
+++ user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw2.c	Sun Dec 27 19:17:03 2009	(r201057)
@@ -588,7 +588,7 @@ send_reject6(struct ip_fw_args *args, in
  * sends a reject message, consuming the mbuf passed as an argument.
  */
 static void
-send_reject(struct ip_fw_args *args, int code, int ip_len, struct ip *ip)
+send_reject(struct ip_fw_args *args, int code, int iplen, struct ip *ip)
 {
 
 #if 0
@@ -602,13 +602,10 @@ send_reject(struct ip_fw_args *args, int
 		m_adj(m, args->L3offset);
 #endif
 	if (code != ICMP_REJECT_RST) { /* Send an ICMP unreach */
-#ifndef HAVE_NET_IPLEN
 		/* We need the IP header in host order for icmp_error(). */
 		if (args->eh != NULL) {
-			ip->ip_len = ntohs(ip->ip_len);
-			ip->ip_off = ntohs(ip->ip_off);
+			SET_HOST_IPLEN(ip);
 		}
-#endif /* !HAVE_NET_IPLEN */
 		icmp_error(args->m, ICMP_UNREACH, code, 0L, 0);
 	} else if (args->f_id.proto == IPPROTO_TCP) {
 		struct tcphdr *const tcp =
@@ -850,12 +847,12 @@ ipfw_chk(struct ip_fw_args *args)
 	 * src_ip, dst_ip	ip addresses, in NETWORK format.
 	 *	Only valid for IPv4 packets.
 	 */
-	u_int8_t proto;
-	u_int16_t src_port = 0, dst_port = 0;	/* NOTE: host format	*/
+	uint8_t proto;
+	uint16_t src_port = 0, dst_port = 0;	/* NOTE: host format	*/
 	struct in_addr src_ip, dst_ip;		/* NOTE: network format	*/
-	u_int16_t ip_len=0;
+	uint16_t iplen=0;
 	int pktlen;
-	u_int16_t	etype = 0;	/* Host order stored ether type */
+	uint16_t	etype = 0;	/* Host order stored ether type */
 
 	/*
 	 * dyn_dir = MATCH_UNKNOWN when rules unchecked,
@@ -1096,14 +1093,14 @@ do {								\
 #ifndef HAVE_NET_IPLEN
 		if (args->eh == NULL) { /* on l3 these are in host format */
 			offset = ip->ip_off & IP_OFFMASK;
-			ip_len = ip->ip_len;
+			iplen = ip->ip_len;
 		} else
 #endif /* !HAVE_NET_IPLEN */
 		{	/* otherwise they are in net format */
 			offset = ntohs(ip->ip_off) & IP_OFFMASK;
-			ip_len = ntohs(ip->ip_len);
+			iplen = ntohs(ip->ip_len);
 		}
-		pktlen = ip_len < pktlen ? ip_len : pktlen;
+		pktlen = iplen < pktlen ? iplen : pktlen;
 
 		if (offset == 0) {
 			switch (proto) {
@@ -1517,7 +1514,7 @@ do {								\
 				    int i;
 
 				    if (cmd->opcode == O_IPLEN)
-					x = ip_len;
+					x = iplen;
 				    else if (cmd->opcode == O_IPTTL)
 					x = ip->ip_ttl;
 				    else /* must be IPID */
@@ -1552,7 +1549,7 @@ do {								\
 				    int i;
 
 				    tcp = TCP(ulp);
-				    x = ip_len -
+				    x = iplen -
 					((ip->ip_hl + tcp->th_off) << 2);
 				    if (cmdlen == 1) {
 					match = (cmd->arg1 == x);
@@ -2025,7 +2022,7 @@ do {								\
 				     is_icmp_query(ICMP(ulp))) &&
 				    !(m->m_flags & (M_BCAST|M_MCAST)) &&
 				    !IN_MULTICAST(ntohl(dst_ip.s_addr))) {
-					send_reject(args, cmd->arg1, ip_len, ip);
+					send_reject(args, cmd->arg1, iplen, ip);
 					m = args->m;
 				}
 				/* FALLTHROUGH */
@@ -2137,17 +2134,14 @@ do {								\
 				/* if not fragmented, go to next rule */
 				if ((ip_off & (IP_MF | IP_OFFMASK)) == 0)
 				    break;
-#ifndef HAVE_NET_IPLEN
 				/* 
 				 * ip_reass() expects len & off in host
 				 * byte order: fix them in case we come
 				 * from layer2.
 				 */
 				if (args->eh != NULL) {
-				    ip->ip_len = ntohs(ip->ip_len);
-				    ip->ip_off = ntohs(ip->ip_off);
+					SET_HOST_IPLEN(ip);
 				}
-#endif /* !HAVE_NET_IPLEN */
 
 				args->m = m = ip_reass(m);
 
@@ -2163,13 +2157,10 @@ do {								\
 
 				    ip = mtod(m, struct ip *);
 				    hlen = ip->ip_hl << 2;
-#ifndef HAVE_NET_IPLEN
 				    /* revert len. & off to net format if needed */
 				    if (args->eh != NULL) {
-					ip->ip_len = htons(ip->ip_len);
-					ip->ip_off = htons(ip->ip_off);
+					SET_NET_IPLEN(ip);
 				    }
-#endif /* !HAVE_NET_IPLEN */
 				    ip->ip_sum = 0;
 				    if (hlen == sizeof(struct ip))
 					ip->ip_sum = in_cksum_hdr(ip);

Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_dynamic.c
==============================================================================
--- user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_dynamic.c	Sun Dec 27 18:32:44 2009	(r201056)
+++ user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_dynamic.c	Sun Dec 27 19:17:03 2009	(r201057)
@@ -1002,7 +1002,7 @@ ipfw_send_pkt(struct mbuf *replyto, stru
 		h->ip_hl = sizeof(*h) >> 2;
 		h->ip_tos = IPTOS_LOWDELAY;
 		h->ip_off = 0;
-#ifdef HAVE_NET_IPLEN
+#ifdef HAVE_NET_IPLEN /* XXX do we handle layer2 ? */
 		h->ip_len = htons(len);
 #else
 		h->ip_len = len;

Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_log.c
==============================================================================
--- user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_log.c	Sun Dec 27 18:32:44 2009	(r201056)
+++ user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_log.c	Sun Dec 27 19:17:03 2009	(r201057)
@@ -165,22 +165,16 @@ ipfw_log(struct ip_fw *f, u_int hlen, st
 			 * more info in the header
 			 */
 			mh.mh_data = "DDDDDDSSSSSS\x08\x00";
-#ifndef HAVE_NET_IPLEN
 			if (args->f_id.addr_type == 4) {
 				/* restore wire format */
-				ip->ip_off = ntohs(ip->ip_off);
-				ip->ip_len = ntohs(ip->ip_len);
+				SET_NET_IPLEN(ip);
 			}
-#endif /* !HAVE_NET_IPLEN */
 		}
 		BPF_MTAP(log_if, (struct mbuf *)&mh);
-#ifndef HAVE_NET_IPLEN
 		if (args->eh == NULL && args->f_id.addr_type == 4) {
 			/* restore host format */
-			ip->ip_off = htons(ip->ip_off);
-			ip->ip_len = htons(ip->ip_len);
+			SET_HOST_IPLEN(ip);
 		}
-#endif /* !HAVE_NET_IPLEN */
 #endif /* !WITHOUT_BPF */
 		return;
 	}

Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_nat.c
==============================================================================
--- user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_nat.c	Sun Dec 27 18:32:44 2009	(r201056)
+++ user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_nat.c	Sun Dec 27 19:17:03 2009	(r201057)
@@ -219,12 +219,9 @@ ipfw_nat(struct ip_fw_args *args, struct
 		return (IP_FW_DENY);
 	}
 	ip = mtod(mcl, struct ip *);
-#ifndef HAVE_NET_IPLEN
 	if (args->eh == NULL) {
-		ip->ip_len = htons(ip->ip_len);
-		ip->ip_off = htons(ip->ip_off);
+		SET_NET_IPLEN(ip);
 	}
-#endif /* !HAVE_NET_IPLEN */
 
 	/*
 	 * XXX - Libalias checksum offload 'duct tape':
@@ -334,12 +331,9 @@ ipfw_nat(struct ip_fw_args *args, struct
 		}
 		ip->ip_len = htons(ip->ip_len);
 	}
-#ifndef HAVE_NET_IPLEN
 	if (args->eh == NULL) {
-		ip->ip_len = ntohs(ip->ip_len);
-		ip->ip_off = ntohs(ip->ip_off);
+		SET_HOST_IPLEN(ip);
 	}
-#endif /* !HAVE_NET_IPLEN */
 	args->m = mcl;
 	return (IP_FW_NAT);
 }

Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_pfil.c
==============================================================================
--- user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_pfil.c	Sun Dec 27 18:32:44 2009	(r201056)
+++ user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_pfil.c	Sun Dec 27 19:17:03 2009	(r201057)
@@ -307,10 +307,7 @@ ipfw_divert(struct mbuf **m0, int incomi
 		 */
 		ip = mtod(reass, struct ip *);
 		hlen = ip->ip_hl << 2;
-#ifndef HAVE_NET_IPLEN
-		ip->ip_len = htons(ip->ip_len);
-		ip->ip_off = htons(ip->ip_off);
-#endif /* !HAVE_NET_IPLEN */
+		SET_NET_IPLEN(ip);
 		ip->ip_sum = 0;
 		if (hlen == sizeof(struct ip))
 			ip->ip_sum = in_cksum_hdr(ip);
@@ -318,11 +315,8 @@ ipfw_divert(struct mbuf **m0, int incomi
 			ip->ip_sum = in_cksum(reass, hlen);
 		clone = reass;
 	} else {
-#ifndef HAVE_NET_IPLEN
 		/* Convert header to network byte order. */
-		ip->ip_len = htons(ip->ip_len);
-		ip->ip_off = htons(ip->ip_off);
-#endif /* !HAVE_NET_IPLEN */
+		SET_NET_IPLEN(ip);
 	}
 
 	/* Do the dirty job... */


More information about the svn-src-user mailing list