Date: Sun, 27 Dec 2009 19:17:03 +0000 (UTC) From: Luigi Rizzo <luigi@FreeBSD.org> To: src-committers@freebsd.org, svn-src-user@freebsd.org Subject: svn commit: r201057 - in user/luigi/ipfw3-head/sys/netinet: . ipfw Message-ID: <200912271917.nBRJH3WH064688@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
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... */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200912271917.nBRJH3WH064688>