broken ip checksum after frag reassemble of nfs READDIR?

[Andrew Thompson]

On Wed, Apr 05, 2006 at 03:06:45PM +0200, Daniel Hartmeier wrote:
> On Wed, Apr 05, 2006 at 02:41:09PM +0200, Max Laier wrote:
> 
> > The other big problem that just crossed my mind:  Reassembly in the bridge 
> > path!?  It doesn't look like the current bridge code on either OS is ready to 
> > deal with packets > MTU coming out of the filter.  The question here is 
> > probably how much IP processing we want to do in the bridge code?
> 
> OpenBSD's bridge does, see bridge_fragment(). IIRC, we slightly adjusted
> ip_fragment() so it could be called from there, and not too much code
> had to be duplicated.
> 

Here is a patch that adds fragmenting, largely based on whats in
OpenBSD. I didnt bring over bridge_send_icmp_err() as we can only get a
large packet to fragment by reassembling a previous fragment, checking
for DF and sending an icmp doesnt apply to us.

Can I get a review, esp. the traversal of the mbufs.


cheers,
Andrew
-------------- next part --------------
Index: ../../net/if_bridge.c
===================================================================
RCS file: /home/ncvs/src/sys/net/if_bridge.c,v
retrieving revision 1.58
diff -u -p -r1.58 if_bridge.c
--- ../../net/if_bridge.c	26 Mar 2006 20:52:47 -0000	1.58
+++ ../../net/if_bridge.c	16 Apr 2006 05:23:03 -0000
@@ -263,6 +263,8 @@ static int	bridge_ip_checkbasic(struct m
 # ifdef INET6
 static int	bridge_ip6_checkbasic(struct mbuf **mp);
 # endif /* INET6 */
+static int	bridge_fragment(struct ifnet *, struct mbuf *,
+		    struct ether_header *, int, struct llc *);
 
 SYSCTL_DECL(_net_link);
 SYSCTL_NODE(_net_link, IFT_BRIDGE, bridge, CTLFLAG_RW, 0, "Bridge");
@@ -1497,13 +1499,21 @@ bridge_stop(struct ifnet *ifp, int disab
 __inline void
 bridge_enqueue(struct bridge_softc *sc, struct ifnet *dst_ifp, struct mbuf *m)
 {
-	int len, err;
+	int len, err = 0;
 	short mflags;
+	struct mbuf *m0;
 
 	len = m->m_pkthdr.len;
 	mflags = m->m_flags;
 
-	IFQ_ENQUEUE(&dst_ifp->if_snd, m, err);
+	for (; m; m = m0) {
+		m0 = m->m_nextpkt;
+		m->m_nextpkt = NULL;
+		
+		if (err == 0)
+			IFQ_ENQUEUE(&dst_ifp->if_snd, m, err);
+	}
+
 	if (err == 0) {
 
 		sc->sc_ifp->if_opackets++;
@@ -2761,13 +2771,24 @@ ipfwpass:
 			error = pfil_run_hooks(&inet_pfil_hook, mp, bifp,
 					dir, NULL);
 
-		/* Restore ip and the fields ntohs()'d. */
-		if (*mp != NULL && error == 0) {
-			ip = mtod(*mp, struct ip *);
-			ip->ip_len = htons(ip->ip_len);
-			ip->ip_off = htons(ip->ip_off);
+		if (*mp == NULL || error != 0) /* filter may consume */
+			break;
+
+		/* check if we need to fragment the packet */
+		if (pfil_member && ifp != NULL && dir == PFIL_OUT) {
+			i = (*mp)->m_pkthdr.len;
+			if (i > ifp->if_mtu) {
+				error = bridge_fragment(ifp, *mp, &eh2, snap,
+					    &llc1);
+				return error;
+			}
 		}
 
+		/* Restore ip and the fields ntohs()'d. */
+		ip = mtod(*mp, struct ip *);
+		ip->ip_len = htons(ip->ip_len);
+		ip->ip_off = htons(ip->ip_off);
+
 		break;
 # ifdef INET6
 	case ETHERTYPE_IPV6 :
@@ -2979,3 +3000,59 @@ bad:
 	return -1;
 }
 # endif /* INET6 */
+
+/*
+ * bridge_fragment:
+ *
+ *	Return a fragmented mbuf chain.
+ */
+static int
+bridge_fragment(struct ifnet *ifp, struct mbuf *m, struct ether_header *eh,
+    int hassnap, struct llc *llc)
+{
+	struct mbuf *m0;
+	int error = -1;
+	struct ip *ip;
+
+	if (m->m_len < sizeof(struct ip) &&
+	    (m = m_pullup(m, sizeof(struct ip))) == NULL)
+		goto dropit;
+	ip = mtod(m, struct ip *);
+
+	error = ip_fragment(ip, &m, ifp->if_mtu, ifp->if_hwassist,
+		    CSUM_DELAY_IP);
+	if (error)
+		goto dropit;
+
+	/* walk the chain and re-add the Ethernet header */
+	for (m0 = m; m0; m0 = m0->m_nextpkt) {
+		if (error == 0) {
+			if (hassnap) {
+				M_PREPEND(m0, sizeof(struct llc), M_DONTWAIT);
+				if (m0 == NULL) {
+					error = ENOBUFS;
+					continue;
+				}
+				bcopy(llc, mtod(m0, caddr_t),
+				    sizeof(struct llc));
+			}
+			M_PREPEND(m0, ETHER_HDR_LEN, M_DONTWAIT);
+			if (m0 == NULL) {
+				error = ENOBUFS;
+				continue;
+			}
+			bcopy(eh, mtod(m0, caddr_t), ETHER_HDR_LEN);
+		} else 
+			m_freem(m);
+	}
+
+	if (error == 0)
+		ipstat.ips_fragmented++;
+
+	return (error);
+
+dropit:
+	if (m != NULL)
+		m_freem(m);
+	return (error);
+}