On Mon, Feb 08, 2016 at 11:02:06PM +1000, David Gwynne wrote:
> On Sat, Feb 06, 2016 at 04:43:28PM -0500, Anthony Eden wrote:
> > >Synopsis: <alignment fault on armv7 (omap) using carp(4)>
> >
> > To me that behavior might suggest the problem is deeper than a
> > bookkeeping mistake of aligning memory in mbuf.
>
> nope, you were right, it's a screwup with alignment.
>
> the problem is multicast packets that arent to a carp interfaces
> mac address have to be duplicated and sent to all carp interfaces
> on a parent. the duplication is done with m_copym2, which doesn't
> respect the alignment requirements of the ip header inside the 14
> byte ethernet header.
>
> the following dups the packet inside carp, and makes sure the
> ethernet payload is aligned properly.
>
> i was able to reproduce this on sparc64, and i believe this fixes
> it. could you test it and see if it helps?
mpi@ pointed out that bridge@ has a special function to do a deep
copy of mbufs that get the ip payload alignment right, and that we
should share.
this moves the functionality in with the rest of the mbuf functions.
could a bridge user test this to see if it still works? carp seems
fine with this on sparc64 stil.
ok?
Index: kern/uipc_mbuf.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_mbuf.c,v
retrieving revision 1.218
diff -u -p -r1.218 uipc_mbuf.c
--- kern/uipc_mbuf.c 31 Jan 2016 00:18:07 -0000 1.218
+++ kern/uipc_mbuf.c 9 Feb 2016 09:49:21 -0000
@@ -1213,6 +1213,40 @@ m_dup_pkthdr(struct mbuf *to, struct mbu
return (0);
}
+struct mbuf *
+m_dup_pkt(struct mbuf *m0, unsigned int adj, int wait)
+{
+ struct mbuf *m;
+ int len;
+
+ len = m0->m_pkthdr.len + adj;
+ if (len > MAXMCLBYTES) /* XXX */
+ return (NULL);
+
+ m = m_get(m0->m_type, wait);
+ if (m == NULL)
+ return (NULL);
+
+ if (m_dup_pkthdr(m, m0, wait) != 0)
+ goto fail;
+
+ if (len > MHLEN) {
+ MCLGETI(m, len, NULL, wait);
+ if (!ISSET(m->m_flags, M_EXT))
+ goto fail;
+ }
+
+ m->m_len = m->m_pkthdr.len = len;
+ m_adj(m, adj);
+ m_copydata(m0, 0, m0->m_pkthdr.len, mtod(m, caddr_t));
+
+ return (m);
+
+fail:
+ m_freem(m);
+ return (NULL);
+}
+
#ifdef DDB
void
m_print(void *v,
Index: net/if_bridge.c
===================================================================
RCS file: /cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.275
diff -u -p -r1.275 if_bridge.c
--- net/if_bridge.c 5 Dec 2015 10:07:55 -0000 1.275
+++ net/if_bridge.c 9 Feb 2016 09:49:21 -0000
@@ -137,7 +137,6 @@ int bridge_ipsec(struct bridge_softc *,
int bridge_clone_create(struct if_clone *, int);
int bridge_clone_destroy(struct ifnet *ifp);
int bridge_delete(struct bridge_softc *, struct bridge_iflist *);
-struct mbuf *bridge_m_dup(struct mbuf *);
#define ETHERADDR_IS_IP_MCAST(a) \
/* struct etheraddr *a; */ \
@@ -800,7 +799,7 @@ bridge_output(struct ifnet *ifp, struct
used = 1;
mc = m;
} else {
- mc = bridge_m_dup(m);
+ mc = m_dup_pkt(m, ETHER_ALIGN, M_DONTWAIT);
if (mc == NULL) {
sc->sc_if.if_oerrors++;
continue;
@@ -1090,7 +1089,7 @@ bridge_process(struct ifnet *ifp, struct
(ifl->bif_state == BSTP_IFSTATE_DISCARDING))
goto reenqueue;
- mc = bridge_m_dup(m);
+ mc = m_dup_pkt(m, ETHER_ALIGN, M_DONTWAIT);
if (mc == NULL)
goto reenqueue;
@@ -1227,7 +1226,7 @@ bridge_broadcast(struct bridge_softc *sc
mc = m;
used = 1;
} else {
- mc = bridge_m_dup(m);
+ mc = m_dup_pkt(m, ETHER_ALIGN, M_DONTWAIT);
if (mc == NULL) {
sc->sc_if.if_oerrors++;
continue;
@@ -1277,7 +1276,7 @@ bridge_localbroadcast(struct bridge_soft
return;
}
- m1 = bridge_m_dup(m);
+ m1 = m_dup_pkt(m, ETHER_ALIGN, M_DONTWAIT);
if (m1 == NULL) {
sc->sc_if.if_oerrors++;
return;
@@ -2017,37 +2016,4 @@ bridge_copyaddr(struct sockaddr *src, st
memcpy(dst, src, src->sa_len);
else
dst->sa_family = AF_UNSPEC;
-}
-
-/*
- * Specialized deep copy to ensure that the payload after the Ethernet
- * header is nicely aligned.
- */
-struct mbuf *
-bridge_m_dup(struct mbuf *m)
-{
- struct mbuf *m1, *m2, *mx;
-
- m1 = m_copym2(m, 0, ETHER_HDR_LEN, M_DONTWAIT);
- if (m1 == NULL) {
- return (NULL);
- }
- m2 = m_copym2(m, ETHER_HDR_LEN, M_COPYALL, M_DONTWAIT);
- if (m2 == NULL) {
- m_freem(m1);
- return (NULL);
- }
-
- for (mx = m1; mx->m_next != NULL; mx = mx->m_next)
- /*EMPTY*/;
- mx->m_next = m2;
-
- if (m1->m_flags & M_PKTHDR) {
- int len = 0;
- for (mx = m1; mx != NULL; mx = mx->m_next)
- len += mx->m_len;
- m1->m_pkthdr.len = len;
- }
-
- return (m1);
}
Index: netinet/ip_carp.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_carp.c,v
retrieving revision 1.286
diff -u -p -r1.286 ip_carp.c
--- netinet/ip_carp.c 21 Jan 2016 11:23:48 -0000 1.286
+++ netinet/ip_carp.c 9 Feb 2016 09:49:21 -0000
@@ -210,6 +210,7 @@ int carp_input(struct ifnet *, struct mb
void carp_proto_input_c(struct ifnet *, struct mbuf *,
struct carp_header *, int, sa_family_t);
void carp_proto_input_if(struct ifnet *, struct mbuf *, int);
+int carp_input_mcast(struct carp_softc *, struct mbuf *);
void carpattach(int);
void carpdetach(struct carp_softc *);
int carp_prepare_ad(struct mbuf *, struct carp_vhost_entry *,
@@ -1429,9 +1430,10 @@ carp_input(struct ifnet *ifp0, struct mb
if (!(sc->sc_if.if_flags & IFF_UP))
continue;
- m0 = m_copym2(m, 0, M_COPYALL, M_DONTWAIT);
+ m0 = m_dup_pkt(m, ETHER_ALIGN, M_DONTWAIT);
+ /* if we cant send one we probably cant send more */
if (m0 == NULL)
- continue;
+ break;
ml_init(&ml);
ml_enqueue(&ml, m0);
Index: sys/mbuf.h
===================================================================
RCS file: /cvs/src/sys/sys/mbuf.h,v
retrieving revision 1.207
diff -u -p -r1.207 mbuf.h
--- sys/mbuf.h 31 Jan 2016 00:18:07 -0000 1.207
+++ sys/mbuf.h 9 Feb 2016 09:49:21 -0000
@@ -447,6 +447,7 @@ void m_cat(struct mbuf *, struct mbuf *)
struct mbuf *m_devget(char *, int, int);
int m_apply(struct mbuf *, int, int,
int (*)(caddr_t, caddr_t, unsigned int), caddr_t);
+struct mbuf *m_dup_pkt(struct mbuf *, unsigned int, int);
int m_dup_pkthdr(struct mbuf *, struct mbuf *, int);
/* Packet tag routines */
