Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 8 Oct 2015 18:51:25 +0200
From:      Kristof Provost <kp@FreeBSD.org>
To:        Huub Schuurmans <huubsch@xs4all.nl>
Cc:        freebsd-embedded@freebsd.org
Subject:   Re: Ue0/ue1 flapping
Message-ID:  <E8BF1228-9A08-42DD-89CC-989EAA608646@FreeBSD.org>
In-Reply-To: <56163A55.5050902@xs4all.nl>
References:  <55D6217B.8000805@denninger.net> <55D636FE.9020005@xs4all.nl> <55D64AA7.40305@denninger.net> <55D69C73.9030909@denninger.net> <56163A55.5050902@xs4all.nl>

next in thread | previous in thread | raw e-mail | index | archive | help

--Apple-Mail=_D1910463-3104-4C23-9875-19E184F3E8D0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8


> On 08 Oct 2015, at 11:41, Huub Schuurmans <huubsch@xs4all.nl> wrote:
> We have now finally found that in our case a running pf is causing
> problems with the usb-lan adapters, even if all rules are flushed (!).
> Problem doesnot occur with ipfw. We have not tested flapping yet.
>=20
It=E2=80=99s possible that this is another case of=20
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D198868

I=E2=80=99m working on a checksum/TSO problem that I think is related to =
this.
Could you give the attached patch a try?
If I=E2=80=99m right about the cause this should fix things for you.

Regards,
Kristof


--Apple-Mail=_D1910463-3104-4C23-9875-19E184F3E8D0
Content-Disposition: attachment;
	filename=pf_xen.patch
Content-Type: application/octet-stream;
	name="pf_xen.patch"
Content-Transfer-Encoding: 7bit

diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index ea90dc8..91728f1 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1552,6 +1552,8 @@ extern void			 pf_print_state(struct pf_state *);
 extern void			 pf_print_flags(u_int8_t);
 extern u_int16_t		 pf_cksum_fixup(u_int16_t, u_int16_t, u_int16_t,
 				    u_int8_t);
+extern u_int16_t		 pf_tcp_cksum_fixup(struct mbuf *, u_int16_t,
+				    u_int16_t, u_int16_t);
 
 VNET_DECLARE(struct ifnet *,		 sync_ifp);
 #define	V_sync_ifp		 	 VNET(sync_ifp);
@@ -1581,6 +1583,7 @@ u_int32_t	pf_new_isn(struct pf_state *);
 void   *pf_pull_hdr(struct mbuf *, int, void *, int, u_short *, u_short *,
 	    sa_family_t);
 void	pf_change_a(void *, u_int16_t *, u_int32_t, u_int8_t);
+void	pf_change_tcp_a(struct mbuf *, void *, u_int16_t *, u_int32_t);
 void	pf_send_deferred_syn(struct pf_state *);
 int	pf_match_addr(u_int8_t, struct pf_addr *, struct pf_addr *,
 	    struct pf_addr *, sa_family_t);
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 1f6d5a2..3ed094a 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -202,7 +202,7 @@ static void		 pf_init_threshold(struct pf_threshold *, u_int32_t,
 static void		 pf_add_threshold(struct pf_threshold *);
 static int		 pf_check_threshold(struct pf_threshold *);
 
-static void		 pf_change_ap(struct pf_addr *, u_int16_t *,
+static void		 pf_change_ap(struct mbuf *, struct pf_addr *, u_int16_t *,
 			    u_int16_t *, u_int16_t *, struct pf_addr *,
 			    u_int16_t, u_int8_t, sa_family_t);
 static int		 pf_modulate_sack(struct mbuf *, int, struct pf_pdesc *,
@@ -2006,9 +2006,19 @@ pf_cksum_fixup(u_int16_t cksum, u_int16_t old, u_int16_t new, u_int8_t udp)
 	return (l);
 }
 
+u_int16_t
+pf_tcp_cksum_fixup(struct mbuf *m, u_int16_t cksum, u_int16_t old, u_int16_t new)
+{
+	if (m->m_pkthdr.csum_flags & (CSUM_TCP | CSUM_TCP_IPV6))
+		return (cksum);
+
+	return (pf_cksum_fixup(cksum, old, new, 0));
+}
+
 static void
-pf_change_ap(struct pf_addr *a, u_int16_t *p, u_int16_t *ic, u_int16_t *pc,
-    struct pf_addr *an, u_int16_t pn, u_int8_t u, sa_family_t af)
+pf_change_ap(struct mbuf *m, struct pf_addr *a, u_int16_t *p, u_int16_t *ic,
+        u_int16_t *pc, struct pf_addr *an, u_int16_t pn, u_int8_t u,
+        sa_family_t af)
 {
 	struct pf_addr	ao;
 	u_int16_t	po = *p;
@@ -2025,17 +2035,22 @@ pf_change_ap(struct pf_addr *a, u_int16_t *p, u_int16_t *ic, u_int16_t *pc,
 		    ao.addr16[0], an->addr16[0], 0),
 		    ao.addr16[1], an->addr16[1], 0);
 		*p = pn;
-		*pc = pf_cksum_fixup(pf_cksum_fixup(pf_cksum_fixup(*pc,
+
+		*pc = pf_cksum_fixup(pf_cksum_fixup(*pc,
 		    ao.addr16[0], an->addr16[0], u),
-		    ao.addr16[1], an->addr16[1], u),
-		    po, pn, u);
+		    ao.addr16[1], an->addr16[1], u);
+
+		if (u)
+			*pc = pf_cksum_fixup(*pc, po, pn, u);
+		else
+			*pc = pf_tcp_cksum_fixup(m, *pc, po, pn);
 		break;
 #endif /* INET */
 #ifdef INET6
 	case AF_INET6:
 		*pc = pf_cksum_fixup(pf_cksum_fixup(pf_cksum_fixup(
 		    pf_cksum_fixup(pf_cksum_fixup(pf_cksum_fixup(
-		    pf_cksum_fixup(pf_cksum_fixup(pf_cksum_fixup(*pc,
+		    pf_cksum_fixup(pf_cksum_fixup(*pc,
 		    ao.addr16[0], an->addr16[0], u),
 		    ao.addr16[1], an->addr16[1], u),
 		    ao.addr16[2], an->addr16[2], u),
@@ -2043,13 +2058,30 @@ pf_change_ap(struct pf_addr *a, u_int16_t *p, u_int16_t *ic, u_int16_t *pc,
 		    ao.addr16[4], an->addr16[4], u),
 		    ao.addr16[5], an->addr16[5], u),
 		    ao.addr16[6], an->addr16[6], u),
-		    ao.addr16[7], an->addr16[7], u),
-		    po, pn, u);
+		    ao.addr16[7], an->addr16[7], u);
+
+		if (u)
+			*pc = pf_cksum_fixup(*pc, po, pn, u);
+		else
+			*pc = pf_tcp_cksum_fixup(m, *pc, po, pn);
 		break;
 #endif /* INET6 */
 	}
 }
 
+static void
+pf_change_pseudo_ap(struct mbuf *m, struct pf_addr *a, u_int16_t *p, u_int16_t *ic,
+        u_int16_t *pc, struct pf_addr *an, u_int16_t pn, u_int8_t u,
+        sa_family_t af)
+{
+	if (m->m_pkthdr.csum_flags & (CSUM_TCP | CSUM_TCP_IPV6))
+		*pc = ~*pc;
+
+	pf_change_ap(m, a, p, ic, pc, an, pn, u, af);
+
+	if (m->m_pkthdr.csum_flags & (CSUM_TCP | CSUM_TCP_IPV6))
+		*pc = ~*pc;
+}
 
 /* Changes a u_int32_t.  Uses a void * so there are no align restrictions */
 void
@@ -2063,6 +2095,19 @@ pf_change_a(void *a, u_int16_t *c, u_int32_t an, u_int8_t u)
 	    ao % 65536, an % 65536, u);
 }
 
+void
+pf_change_tcp_a(struct mbuf *m, void *a, u_int16_t *c, u_int32_t an)
+{
+	u_int32_t	ao;
+
+	memcpy(&ao, a, sizeof(ao));
+	memcpy(a, &an, sizeof(u_int32_t));
+
+	*c = pf_tcp_cksum_fixup(m,
+	    pf_tcp_cksum_fixup(m, *c, ao / 65536, an / 65536),
+	    ao % 65536, an % 65536);
+}
+
 #ifdef INET6
 static void
 pf_change_a6(struct pf_addr *a, u_int16_t *c, struct pf_addr *an, u_int8_t u)
@@ -2208,12 +2253,10 @@ pf_modulate_sack(struct mbuf *m, int off, struct pf_pdesc *pd,
 				for (i = 2; i + TCPOLEN_SACK <= olen;
 				    i += TCPOLEN_SACK) {
 					memcpy(&sack, &opt[i], sizeof(sack));
-					pf_change_a(&sack.start, &th->th_sum,
-					    htonl(ntohl(sack.start) -
-					    dst->seqdiff), 0);
-					pf_change_a(&sack.end, &th->th_sum,
-					    htonl(ntohl(sack.end) -
-					    dst->seqdiff), 0);
+					pf_change_tcp_a(m, &sack.start, &th->th_sum,
+					    htonl(ntohl(sack.start) - dst->seqdiff));
+					pf_change_tcp_a(m, &sack.end, &th->th_sum,
+					    htonl(ntohl(sack.end) - dst->seqdiff));
 					memcpy(&opt[i], &sack, sizeof(sack));
 				}
 				copyback = 1;
@@ -3117,7 +3160,7 @@ pf_test_rule(struct pf_rule **rm, struct pf_state **sm, int direction,
 
 			if (PF_ANEQ(saddr, &nk->addr[pd->sidx], af) ||
 			    nk->port[pd->sidx] != sport) {
-				pf_change_ap(saddr, &th->th_sport, pd->ip_sum,
+				pf_change_pseudo_ap(m, saddr, &th->th_sport, pd->ip_sum,
 				    &th->th_sum, &nk->addr[pd->sidx],
 				    nk->port[pd->sidx], 0, af);
 				pd->sport = &th->th_sport;
@@ -3126,7 +3169,7 @@ pf_test_rule(struct pf_rule **rm, struct pf_state **sm, int direction,
 
 			if (PF_ANEQ(daddr, &nk->addr[pd->didx], af) ||
 			    nk->port[pd->didx] != dport) {
-				pf_change_ap(daddr, &th->th_dport, pd->ip_sum,
+				pf_change_pseudo_ap(m, daddr, &th->th_dport, pd->ip_sum,
 				    &th->th_sum, &nk->addr[pd->didx],
 				    nk->port[pd->didx], 0, af);
 				dport = th->th_dport;
@@ -3140,7 +3183,7 @@ pf_test_rule(struct pf_rule **rm, struct pf_state **sm, int direction,
 
 			if (PF_ANEQ(saddr, &nk->addr[pd->sidx], af) ||
 			    nk->port[pd->sidx] != sport) {
-				pf_change_ap(saddr, &pd->hdr.udp->uh_sport,
+				pf_change_ap(m, saddr, &pd->hdr.udp->uh_sport,
 				    pd->ip_sum, &pd->hdr.udp->uh_sum,
 				    &nk->addr[pd->sidx],
 				    nk->port[pd->sidx], 1, af);
@@ -3150,7 +3193,7 @@ pf_test_rule(struct pf_rule **rm, struct pf_state **sm, int direction,
 
 			if (PF_ANEQ(daddr, &nk->addr[pd->didx], af) ||
 			    nk->port[pd->didx] != dport) {
-				pf_change_ap(daddr, &pd->hdr.udp->uh_dport,
+				pf_change_ap(m, daddr, &pd->hdr.udp->uh_dport,
 				    pd->ip_sum, &pd->hdr.udp->uh_sum,
 				    &nk->addr[pd->didx],
 				    nk->port[pd->didx], 1, af);
@@ -3502,8 +3545,8 @@ pf_create_state(struct pf_rule *r, struct pf_rule *nr, struct pf_rule *a,
 			if ((s->src.seqdiff = pf_tcp_iss(pd) - s->src.seqlo) ==
 			    0)
 				s->src.seqdiff = 1;
-			pf_change_a(&th->th_seq, &th->th_sum,
-			    htonl(s->src.seqlo + s->src.seqdiff), 0);
+			pf_change_tcp_a(m, &th->th_seq, &th->th_sum,
+			    htonl(s->src.seqlo + s->src.seqdiff));
 			*rewrite = 1;
 		} else
 			s->src.seqdiff = 0;
@@ -3826,9 +3869,9 @@ pf_tcp_track_full(struct pf_state_peer *src, struct pf_state_peer *dst,
 			while ((src->seqdiff = arc4random() - seq) == 0)
 				;
 			ack = ntohl(th->th_ack) - dst->seqdiff;
-			pf_change_a(&th->th_seq, &th->th_sum, htonl(seq +
-			    src->seqdiff), 0);
-			pf_change_a(&th->th_ack, &th->th_sum, htonl(ack), 0);
+			pf_change_tcp_a(m, &th->th_seq, &th->th_sum, htonl(seq +
+			    src->seqdiff));
+			pf_change_tcp_a(m, &th->th_ack, &th->th_sum, htonl(ack));
 			*copyback = 1;
 		} else {
 			ack = ntohl(th->th_ack);
@@ -3878,9 +3921,9 @@ pf_tcp_track_full(struct pf_state_peer *src, struct pf_state_peer *dst,
 		ack = ntohl(th->th_ack) - dst->seqdiff;
 		if (src->seqdiff) {
 			/* Modulate sequence numbers */
-			pf_change_a(&th->th_seq, &th->th_sum, htonl(seq +
-			    src->seqdiff), 0);
-			pf_change_a(&th->th_ack, &th->th_sum, htonl(ack), 0);
+			pf_change_tcp_a(m, &th->th_seq, &th->th_sum, htonl(seq +
+			    src->seqdiff));
+			pf_change_tcp_a(m, &th->th_ack, &th->th_sum, htonl(ack));
 			*copyback = 1;
 		}
 		end = seq + pd->p_len;
@@ -4334,14 +4377,14 @@ pf_test_state_tcp(struct pf_state **state, int direction, struct pfi_kif *kif,
 
 		if (PF_ANEQ(pd->src, &nk->addr[pd->sidx], pd->af) ||
 		    nk->port[pd->sidx] != th->th_sport)
-			pf_change_ap(pd->src, &th->th_sport, pd->ip_sum,
-			    &th->th_sum, &nk->addr[pd->sidx],
+			pf_change_pseudo_ap(m, pd->src, &th->th_sport,
+			    pd->ip_sum, &th->th_sum, &nk->addr[pd->sidx],
 			    nk->port[pd->sidx], 0, pd->af);
 
 		if (PF_ANEQ(pd->dst, &nk->addr[pd->didx], pd->af) ||
 		    nk->port[pd->didx] != th->th_dport)
-			pf_change_ap(pd->dst, &th->th_dport, pd->ip_sum,
-			    &th->th_sum, &nk->addr[pd->didx],
+			pf_change_pseudo_ap(m, pd->dst, &th->th_dport,
+			    pd->ip_sum, &th->th_sum, &nk->addr[pd->didx],
 			    nk->port[pd->didx], 0, pd->af);
 		copyback = 1;
 	}
@@ -4405,13 +4448,13 @@ pf_test_state_udp(struct pf_state **state, int direction, struct pfi_kif *kif,
 
 		if (PF_ANEQ(pd->src, &nk->addr[pd->sidx], pd->af) ||
 		    nk->port[pd->sidx] != uh->uh_sport)
-			pf_change_ap(pd->src, &uh->uh_sport, pd->ip_sum,
+			pf_change_ap(m, pd->src, &uh->uh_sport, pd->ip_sum,
 			    &uh->uh_sum, &nk->addr[pd->sidx],
 			    nk->port[pd->sidx], 1, pd->af);
 
 		if (PF_ANEQ(pd->dst, &nk->addr[pd->didx], pd->af) ||
 		    nk->port[pd->didx] != uh->uh_dport)
-			pf_change_ap(pd->dst, &uh->uh_dport, pd->ip_sum,
+			pf_change_ap(m, pd->dst, &uh->uh_dport, pd->ip_sum,
 			    &uh->uh_sum, &nk->addr[pd->didx],
 			    nk->port[pd->didx], 1, pd->af);
 		m_copyback(m, off, sizeof(*uh), (caddr_t)uh);
diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c
index ba43de8..a5a516f 100644
--- a/sys/netpfil/pf/pf_ioctl.c
+++ b/sys/netpfil/pf/pf_ioctl.c
@@ -3566,12 +3566,6 @@ pf_check_out(void *arg, struct mbuf **m, struct ifnet *ifp, int dir,
 {
 	int chk;
 
-	/* We need a proper CSUM befor we start (s. OpenBSD ip_output) */
-	if ((*m)->m_pkthdr.csum_flags & CSUM_DELAY_DATA) {
-		in_delayed_cksum(*m);
-		(*m)->m_pkthdr.csum_flags &= ~CSUM_DELAY_DATA;
-	}
-
 	chk = pf_test(PF_OUT, ifp, m, inp);
 	if (chk && *m) {
 		m_freem(*m);
@@ -3610,13 +3604,6 @@ pf_check6_out(void *arg, struct mbuf **m, struct ifnet *ifp, int dir,
 {
 	int chk;
 
-	/* We need a proper CSUM before we start (s. OpenBSD ip_output) */
-	if ((*m)->m_pkthdr.csum_flags & CSUM_DELAY_DATA_IPV6) {
-		in6_delayed_cksum(*m,
-		    (*m)->m_pkthdr.len - sizeof(struct ip6_hdr),
-		    sizeof(struct ip6_hdr));
-		(*m)->m_pkthdr.csum_flags &= ~CSUM_DELAY_DATA_IPV6;
-	}
 	CURVNET_SET(ifp->if_vnet);
 	chk = pf_test6(PF_OUT, ifp, m, inp);
 	CURVNET_RESTORE();
diff --git a/sys/netpfil/pf/pf_norm.c b/sys/netpfil/pf/pf_norm.c
index 8605554..5fb099f 100644
--- a/sys/netpfil/pf/pf_norm.c
+++ b/sys/netpfil/pf/pf_norm.c
@@ -1215,13 +1215,13 @@ pf_normalize_tcp(int dir, struct pfi_kif *kif, struct mbuf *m, int ipoff,
 		th->th_x2 = 0;
 		nv = *(u_int16_t *)(&th->th_ack + 1);
 
-		th->th_sum = pf_cksum_fixup(th->th_sum, ov, nv, 0);
+		th->th_sum = pf_tcp_cksum_fixup(m, th->th_sum, ov, nv);
 		rewrite = 1;
 	}
 
 	/* Remove urgent pointer, if TH_URG is not set */
 	if (!(flags & TH_URG) && th->th_urp) {
-		th->th_sum = pf_cksum_fixup(th->th_sum, th->th_urp, 0, 0);
+		th->th_sum = pf_tcp_cksum_fixup(m, th->th_sum, th->th_urp, 0);
 		th->th_urp = 0;
 		rewrite = 1;
 	}
@@ -1422,11 +1422,10 @@ pf_normalize_tcp_stateful(struct mbuf *m, int off, struct pf_pdesc *pd,
 					    (src->scrub->pfss_flags &
 					    PFSS_TIMESTAMP)) {
 						tsval = ntohl(tsval);
-						pf_change_a(&opt[2],
+						pf_change_tcp_a(m, &opt[2],
 						    &th->th_sum,
 						    htonl(tsval +
-						    src->scrub->pfss_ts_mod),
-						    0);
+						    src->scrub->pfss_ts_mod));
 						copyback = 1;
 					}
 
@@ -1438,9 +1437,8 @@ pf_normalize_tcp_stateful(struct mbuf *m, int off, struct pf_pdesc *pd,
 					    PFSS_TIMESTAMP)) {
 						tsecr = ntohl(tsecr)
 						    - dst->scrub->pfss_ts_mod;
-						pf_change_a(&opt[6],
-						    &th->th_sum, htonl(tsecr),
-						    0);
+						pf_change_tcp_a(m, &opt[6],
+						    &th->th_sum, htonl(tsecr));
 						copyback = 1;
 					}
 					got_ts = 1;
@@ -1765,8 +1763,8 @@ pf_normalize_tcpopt(struct pf_rule *r, struct mbuf *m, struct tcphdr *th,
 		case TCPOPT_MAXSEG:
 			mss = (u_int16_t *)(optp + 2);
 			if ((ntohs(*mss)) > r->max_mss) {
-				th->th_sum = pf_cksum_fixup(th->th_sum,
-				    *mss, htons(r->max_mss), 0);
+				th->th_sum = pf_tcp_cksum_fixup(m, th->th_sum,
+				    *mss, htons(r->max_mss));
 				*mss = htons(r->max_mss);
 				rewrite = 1;
 			}

--Apple-Mail=_D1910463-3104-4C23-9875-19E184F3E8D0--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?E8BF1228-9A08-42DD-89CC-989EAA608646>