ia64/81284: Unaligned Reference with pf on 5.4/IA64
Andrew Thompson
thompsa at freebsd.org
Tue Jun 28 20:20:21 GMT 2005
The following reply was made to PR ia64/81284; it has been noted by GNATS.
From: Andrew Thompson <thompsa at freebsd.org>
To: bug-followup at 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 */
More information about the freebsd-pf
mailing list