Date: Tue, 28 Jun 2005 20:20:20 GMT From: Andrew Thompson <thompsa@freebsd.org> To: freebsd-pf@FreeBSD.org Subject: Re: ia64/81284: Unaligned Reference with pf on 5.4/IA64 Message-ID: <200506282020.j5SKKKwc059265@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR ia64/81284; it has been noted by GNATS. From: Andrew Thompson <thompsa@freebsd.org> To: bug-followup@FreeBSD.org Cc: Subject: Re: ia64/81284: Unaligned Reference with pf on 5.4/IA64 Date: Wed, 29 Jun 2005 08:19:12 +1200 Here is a patch for if_bridge that ensures it passes aligned packets up to the higher levels (packet filtering). The alignment checks and fixups are straight from NetBSD, this code was in the original implementation that was ported from NetBSD but was removed in our version due to the missing alignment routines. The code has been in NetBSD for two years since r1.9 Two macros are added that check that the IP header is 4 byte aligned, they can be changed for different architectures and IP v[46] alignment requirements. m_copyup() does the actual alignment and has been in the tree for three months (uipc_mbuf.c r1.147) IP_HDR_ALIGNED_P() IP6_HDR_ALIGNED_P() The macros are compile time dependant on __NO_STRICT_ALIGNMENT, and if defined they are always true. This is set for i386 and amd64 where alignment isn't needed so we wont pay the cost. These macros can also be used in other parts of the networking code, for example ieee80211_input.c. Alignment checks also need to be added to bridge.c http://people.freebsd.org/~thompsa/if_bridge.align.diff --- Index: sys/amd64/include/_types.h =================================================================== RCS file: /home/ncvs/src/sys/amd64/include/_types.h,v retrieving revision 1.8 diff -u -r1.8 _types.h --- sys/amd64/include/_types.h 11 Mar 2005 22:16:09 -0000 1.8 +++ sys/amd64/include/_types.h 22 Jun 2005 04:15:38 -0000 @@ -43,6 +43,8 @@ #error this file needs sys/cdefs.h as a prerequisite #endif +#define __NO_STRICT_ALIGNMENT + /* * Basic types upon which most other types are built. */ Index: sys/i386/include/_types.h =================================================================== RCS file: /home/ncvs/src/sys/i386/include/_types.h,v retrieving revision 1.11 diff -u -r1.11 _types.h --- sys/i386/include/_types.h 2 Mar 2005 21:33:26 -0000 1.11 +++ sys/i386/include/_types.h 22 Jun 2005 04:15:13 -0000 @@ -43,6 +43,8 @@ #error this file needs sys/cdefs.h as a prerequisite #endif +#define __NO_STRICT_ALIGNMENT + /* * Basic types upon which most other types are built. */ Index: sys/net/if_bridge.c =================================================================== RCS file: /home/ncvs/src/sys/net/if_bridge.c,v retrieving revision 1.7 diff -u -r1.7 if_bridge.c --- sys/net/if_bridge.c 10 Jun 2005 23:52:01 -0000 1.7 +++ sys/net/if_bridge.c 27 Jun 2005 23:40:28 -0000 @@ -2249,7 +2249,28 @@ m_adj(*mp, sizeof(struct llc)); } + /* + * Check the IP header for alignment and errors + */ + if (dir == PFIL_IN) { + switch (ether_type) { + case ETHERTYPE_IP: + error = bridge_ip_checkbasic(mp); + break; +# ifdef INET6 + case ETHERTYPE_IPV6: + error = bridge_ip6_checkbasic(mp); + break; +# endif /* INET6 */ + default: + error = 0; + } + if (error) + goto bad; + } + if (IPFW_LOADED && pfil_ipfw != 0 && dir == PFIL_OUT) { + error = -1; args.rule = ip_dn_claim_rule(*mp); if (args.rule != NULL && fw_one_pass) goto ipfwpass; /* packet already partially processed */ @@ -2286,26 +2307,22 @@ } ipfwpass: + error = 0; + /* - * Check basic packet sanity and run pfil through pfil. + * Run the packet through pfil */ switch (ether_type) { case ETHERTYPE_IP : - error = (dir == PFIL_IN) ? bridge_ip_checkbasic(mp) : 0; /* * before calling the firewall, swap fields the same as * IP does. here we assume the header is contiguous */ - if (error == 0) { - ip = mtod(*mp, struct ip *); + ip = mtod(*mp, struct ip *); - ip->ip_len = ntohs(ip->ip_len); - ip->ip_off = ntohs(ip->ip_off); - } else { - error = -1; - break; - } + ip->ip_len = ntohs(ip->ip_len); + ip->ip_off = ntohs(ip->ip_off); /* * Run pfil on the member interface and the bridge, both can @@ -2314,21 +2331,21 @@ * Keep the order: * in_if -> bridge_if -> out_if */ - if (error == 0 && pfil_bridge && dir == PFIL_OUT) + if (pfil_bridge && dir == PFIL_OUT) error = pfil_run_hooks(&inet_pfil_hook, mp, bifp, dir, NULL); - if (*mp == NULL) /* filter may consume */ + if (*mp == NULL || error != 0) /* filter may consume */ break; - if (error == 0 && pfil_member) + if (pfil_member) error = pfil_run_hooks(&inet_pfil_hook, mp, ifp, dir, NULL); - if (*mp == NULL) /* filter may consume */ + if (*mp == NULL || error != 0) /* filter may consume */ break; - if (error == 0 && pfil_bridge && dir == PFIL_IN) + if (pfil_bridge && dir == PFIL_IN) error = pfil_run_hooks(&inet_pfil_hook, mp, bifp, dir, NULL); @@ -2342,23 +2359,21 @@ break; # ifdef INET6 case ETHERTYPE_IPV6 : - error = (dir == PFIL_IN) ? bridge_ip6_checkbasic(mp) : 0; - - if (error == 0 && pfil_bridge && dir == PFIL_OUT) + if (pfil_bridge && dir == PFIL_OUT) error = pfil_run_hooks(&inet6_pfil_hook, mp, bifp, dir, NULL); - if (*mp == NULL) /* filter may consume */ + if (*mp == NULL || error != 0) /* filter may consume */ break; - if (error == 0 && pfil_member) + if (pfil_member) error = pfil_run_hooks(&inet6_pfil_hook, mp, ifp, dir, NULL); - if (*mp == NULL) /* filter may consume */ + if (*mp == NULL || error != 0) /* filter may consume */ break; - if (error == 0 && pfil_bridge && dir == PFIL_IN) + if (pfil_bridge && dir == PFIL_IN) error = pfil_run_hooks(&inet6_pfil_hook, mp, bifp, dir, NULL); break; @@ -2421,7 +2436,14 @@ if (*mp == NULL) return -1; - if (__predict_false(m->m_len < sizeof (struct ip))) { + if (IP_HDR_ALIGNED_P(mtod(m, caddr_t)) == 0) { + if ((m = m_copyup(m, sizeof(struct ip), + (max_linkhdr + 3) & ~3)) == NULL) { + /* XXXJRT new stat, please */ + ipstat.ips_toosmall++; + goto bad; + } + } else if (__predict_false(m->m_len < sizeof (struct ip))) { if ((m = m_pullup(m, sizeof (struct ip))) == NULL) { ipstat.ips_toosmall++; goto bad; @@ -2509,18 +2531,17 @@ * mbuf with space for link headers, in the event we forward * it. Otherwise, if it is aligned, make sure the entire base * IPv6 header is in the first mbuf of the chain. - + */ if (IP6_HDR_ALIGNED_P(mtod(m, caddr_t)) == 0) { struct ifnet *inifp = m->m_pkthdr.rcvif; if ((m = m_copyup(m, sizeof(struct ip6_hdr), (max_linkhdr + 3) & ~3)) == NULL) { - * XXXJRT new stat, please * + /* XXXJRT new stat, please */ ip6stat.ip6s_toosmall++; in6_ifstat_inc(inifp, ifs6_in_hdrerr); goto bad; } - } else */ - if (__predict_false(m->m_len < sizeof(struct ip6_hdr))) { + } else if (__predict_false(m->m_len < sizeof(struct ip6_hdr))) { struct ifnet *inifp = m->m_pkthdr.rcvif; if ((m = m_pullup(m, sizeof(struct ip6_hdr))) == NULL) { ip6stat.ip6s_toosmall++; Index: sys/netinet/ip_var.h =================================================================== RCS file: /home/ncvs/src/sys/netinet/ip_var.h,v retrieving revision 1.94 diff -u -r1.94 ip_var.h --- sys/netinet/ip_var.h 7 Jan 2005 01:45:44 -0000 1.94 +++ sys/netinet/ip_var.h 22 Jun 2005 03:25:31 -0000 @@ -136,6 +136,12 @@ /* mbuf flag used by ip_fastfwd */ #define M_FASTFWD_OURS M_PROTO1 /* changed dst to local */ +#ifdef __NO_STRICT_ALIGNMENT +#define IP_HDR_ALIGNED_P(ip) 1 +#else +#define IP_HDR_ALIGNED_P(ip) ((((intptr_t) (ip)) & 3) == 0) +#endif + struct ip; struct inpcb; struct route; Index: sys/netinet6/ip6_var.h =================================================================== RCS file: /home/ncvs/src/sys/netinet6/ip6_var.h,v retrieving revision 1.29 diff -u -r1.29 ip6_var.h --- sys/netinet6/ip6_var.h 7 Jan 2005 02:30:34 -0000 1.29 +++ sys/netinet6/ip6_var.h 22 Jun 2005 03:43:06 -0000 @@ -282,6 +282,12 @@ #define IPV6_FORWARDING 0x02 /* most of IPv6 header exists */ #define IPV6_MINMTU 0x04 /* use minimum MTU (IPV6_USE_MIN_MTU) */ +#ifdef __NO_STRICT_ALIGNMENT +#define IP6_HDR_ALIGNED_P(ip) 1 +#else +#define IP6_HDR_ALIGNED_P(ip) ((((intptr_t) (ip)) & 3) == 0) +#endif + extern struct ip6stat ip6stat; /* statistics */ extern int ip6_defhlim; /* default hop limit */ extern int ip6_defmcasthlim; /* default multicast hop limit */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200506282020.j5SKKKwc059265>