--NextPart-20050416141745-1048100
Content-Type: Text/Plain; charset=us-ascii

hi,

>  > >Number:         29971
>  > >Category:       kern
>  > >Synopsis:       Loopback checksum optimizations cause UDP problems

if no one objects, i'll commit the attached patch.

- for ipv4, defer the decision to ip layer as h/w checksum offloading does,
  so that it can check the actual interface the packet is going to.
- for ipv6, don't bother to omit checksumming.  maybe will be revisited when
  it implements h/w checksum offloading.

YAMAMOTO Takashi

--NextPart-20050416141745-1048100
Content-Type: Text/Plain; charset=us-ascii
Content-Disposition: attachment; filename="a.diff"

Index: netinet/tcp_subr.c
===================================================================
--- netinet/tcp_subr.c	(revision 1137)
+++ netinet/tcp_subr.c	(revision 1138)
@@ -200,7 +200,6 @@ int	tcp_compat_42 = 0;
 #endif
 int	tcp_rst_ppslim = 100;	/* 100pps */
 int	tcp_ackdrop_ppslim = 100;	/* 100pps */
-int	tcp_do_loopback_cksum = 0;
 
 /* tcb hash */
 #ifndef TCBHASHSIZE
Index: netinet/ip_input.c
===================================================================
--- netinet/ip_input.c	(revision 1137)
+++ netinet/ip_input.c	(revision 1138)
@@ -200,7 +200,6 @@ int	ipprintfs = 0;
 #endif
 
 int	ip_do_randomid = 0;
-int	ip_do_loopback_cksum = 0;
 
 /*
  * XXX - Setting ip_checkinterface mostly implements the receive side of
Index: netinet/tcp_output.c
===================================================================
--- netinet/tcp_output.c	(revision 1137)
+++ netinet/tcp_output.c	(revision 1138)
@@ -1306,7 +1306,7 @@ send:
 
 	/*
 	 * Set ourselves up to be checksummed just before the packet
-	 * hits the wire.  Maybe skip checksums on loopback interfaces.
+	 * hits the wire.
 	 */
 	switch (af) {
 #ifdef INET
@@ -1316,13 +1316,7 @@ send:
 			m->m_pkthdr.segsz = txsegsize;
 			m->m_pkthdr.csum_flags = M_CSUM_TSOv4;
 		} else {
-			if (__predict_true(ro->ro_rt == NULL ||
-					   !(ro->ro_rt->rt_ifp->if_flags &
-					     IFF_LOOPBACK) ||
-					   tcp_do_loopback_cksum))
-				m->m_pkthdr.csum_flags = M_CSUM_TCPv4;
-			else
-				m->m_pkthdr.csum_flags = 0;
+			m->m_pkthdr.csum_flags = M_CSUM_TCPv4;
 			if (len + optlen) {
 				/* Fixup the pseudo-header checksum. */
 				/* XXXJRT Not IP Jumbogram safe. */
@@ -1344,13 +1338,7 @@ send:
 		m->m_pkthdr.len = sizeof(struct ip6_hdr)
 			+ sizeof(struct tcphdr) + optlen + len;
 #ifdef notyet
-		if (__predict_true(ro->ro_rt == NULL ||
-				   !(ro->ro_rt->rt_ifp->if_flags &
-				     IFF_LOOPBACK) ||
-				   tcp_do_loopback_cksum))
-			m->m_pkthdr.csum_flags = M_CSUM_TCPv6;
-		else
-			m->m_pkthdr.csum_flags = 0;
+		m->m_pkthdr.csum_flags = M_CSUM_TCPv6;
 		m->m_pkthdr.csum_data = offsetof(struct tcphdr, th_sum);
 #endif
 		if (len + optlen) {
@@ -1359,14 +1347,8 @@ send:
 			th->th_sum = in_cksum_addword(th->th_sum,
 			    htons((u_int16_t) (len + optlen)));
 		}
-#ifndef notyet
-		if (__predict_true(ro->ro_rt == NULL ||
-				   !(ro->ro_rt->rt_ifp->if_flags &
-				     IFF_LOOPBACK) ||
-				   tcp_do_loopback_cksum))
-			th->th_sum = in6_cksum(m, 0, sizeof(struct ip6_hdr),
-			    sizeof(struct tcphdr) + optlen + len);
-#endif
+		th->th_sum = in6_cksum(m, 0, sizeof(struct ip6_hdr),
+		    sizeof(struct tcphdr) + optlen + len);
 		break;
 #endif
 	}
Index: netinet/ip_output.c
===================================================================
--- netinet/ip_output.c	(revision 1137)
+++ netinet/ip_output.c	(revision 1138)
@@ -158,6 +158,16 @@ static void ip_mloopback(struct ifnet *,
 extern struct pfil_head inet_pfil_hook;			/* XXX */
 #endif
 
+int	udp_do_loopback_cksum = 0;
+int	tcp_do_loopback_cksum = 0;
+int	ip_do_loopback_cksum = 0;
+
+#define	IN_NEED_CHECKSUM(ifp, csum_flags) \
+	(__predict_true(((ifp)->if_flags & IFF_LOOPBACK) == 0 || \
+	(((csum_flags) & M_CSUM_UDPv4) != 0 && udp_do_loopback_cksum) || \
+	(((csum_flags) & M_CSUM_TCPv4) != 0 && tcp_do_loopback_cksum) || \
+	(((csum_flags) & M_CSUM_IPv4) != 0 && ip_do_loopback_cksum)))
+
 /*
  * IP output.  The packet in mbuf chain m contains a skeletal IP
  * header (with len, off, ttl, proto, tos, src, dst).
@@ -788,9 +798,9 @@ spd_done:
 #endif
 
 	/* Maybe skip checksums on loopback interfaces. */
-	if (__predict_true(!(ifp->if_flags & IFF_LOOPBACK) ||
-			   ip_do_loopback_cksum))
+	if (IN_NEED_CHECKSUM(ifp, M_CSUM_IPv4)) {
 		m->m_pkthdr.csum_flags |= M_CSUM_IPv4;
+	}
 	sw_csum = m->m_pkthdr.csum_flags & ~ifp->if_csum_flags_tx;
 	/*
 	 * If small enough for mtu of path, or if using TCP segmentation
@@ -817,11 +827,15 @@ spd_done:
 			 * XXX fields to be 0?
 			 */
 			if (sw_csum & M_CSUM_IPv4) {
+				KASSERT(IN_NEED_CHECKSUM(ifp, M_CSUM_IPv4));
 				ip->ip_sum = in_cksum(m, hlen);
 				m->m_pkthdr.csum_flags &= ~M_CSUM_IPv4;
 			}
 			if (sw_csum & (M_CSUM_TCPv4|M_CSUM_UDPv4)) {
-				in_delayed_cksum(m);
+				if (IN_NEED_CHECKSUM(ifp,
+				    sw_csum & (M_CSUM_TCPv4|M_CSUM_UDPv4))) {
+					in_delayed_cksum(m);
+				}
 				m->m_pkthdr.csum_flags &=
 				    ~(M_CSUM_TCPv4|M_CSUM_UDPv4);
 			}
@@ -842,7 +856,10 @@ spd_done:
 	 * XXX Some hardware can do this.
 	 */
 	if (m->m_pkthdr.csum_flags & (M_CSUM_TCPv4|M_CSUM_UDPv4)) {
-		in_delayed_cksum(m);
+		if (IN_NEED_CHECKSUM(ifp,
+		    m->m_pkthdr.csum_flags & (M_CSUM_TCPv4|M_CSUM_UDPv4))) {
+			in_delayed_cksum(m);
+		}
 		m->m_pkthdr.csum_flags &= ~(M_CSUM_TCPv4|M_CSUM_UDPv4);
 	}
 
Index: netinet/udp_usrreq.c
===================================================================
--- netinet/udp_usrreq.c	(revision 1137)
+++ netinet/udp_usrreq.c	(revision 1138)
@@ -146,7 +146,6 @@ int	udpcksum = 1;
 #else
 int	udpcksum = 0;		/* XXX */
 #endif
-int	udp_do_loopback_cksum = 0;
 
 struct	inpcbtable udbtable;
 struct	udpstat udpstat;
@@ -1080,18 +1079,11 @@ udp_output(struct mbuf *m, ...)
 		/*
 		 * XXX Cache pseudo-header checksum part for
 		 * XXX "connected" UDP sockets.
-		 * Maybe skip checksums on loopback interfaces.
 		 */
 		ui->ui_sum = in_cksum_phdr(ui->ui_src.s_addr,
 		    ui->ui_dst.s_addr, htons((u_int16_t)len +
 		    sizeof(struct udphdr) + IPPROTO_UDP));
-		if (__predict_true(ro->ro_rt == NULL ||
-				   !(ro->ro_rt->rt_ifp->if_flags &
-				     IFF_LOOPBACK) ||
-				   udp_do_loopback_cksum))
-			m->m_pkthdr.csum_flags = M_CSUM_UDPv4;
-		else
-			m->m_pkthdr.csum_flags = 0;
+		m->m_pkthdr.csum_flags = M_CSUM_UDPv4;
 		m->m_pkthdr.csum_data = offsetof(struct udphdr, uh_sum);
 	} else
 		ui->ui_sum = 0;
Index: netinet6/udp6_output.c
===================================================================
--- netinet6/udp6_output.c	(revision 1137)
+++ netinet6/udp6_output.c	(revision 1138)
@@ -330,15 +330,9 @@ udp6_output(in6p, m, addr6, control, p)
 		ip6->ip6_src	= *laddr;
 		ip6->ip6_dst	= *faddr;
 
-		/* Maybe skip checksum on loopback interfaces. */
-		if (__predict_true(in6p->in6p_route.ro_rt == NULL ||
-				   !(in6p->in6p_route.ro_rt->rt_ifp->if_flags &
-				     IFF_LOOPBACK) ||
-				   udp_do_loopback_cksum)) {
-			if ((udp6->uh_sum = in6_cksum(m, IPPROTO_UDP,
-					sizeof(struct ip6_hdr), plen)) == 0) {
-				udp6->uh_sum = 0xffff;
-			}
+		if ((udp6->uh_sum = in6_cksum(m, IPPROTO_UDP,
+		    sizeof(struct ip6_hdr), plen)) == 0) {
+			udp6->uh_sum = 0xffff;
 		}
 
 		if (in6p->in6p_flags & IN6P_MINMTU)
@@ -368,15 +362,9 @@ udp6_output(in6p, m, addr6, control, p)
 			 (SO_DONTROUTE | SO_BROADCAST));
 		bcopy(&faddr->s6_addr[12], &ui->ui_dst, sizeof(ui->ui_dst));
 
-		/* Maybe skip checksum on loopback interfaces. */
-		if (__predict_true(in6p->in6p_route.ro_rt == NULL ||
-				   !(in6p->in6p_route.ro_rt->rt_ifp->if_flags &
-				     IFF_LOOPBACK) ||
-				   udp_do_loopback_cksum)) {
-			udp6->uh_sum = in_cksum(m, hlen + plen);
-			if (udp6->uh_sum == 0)
-				udp6->uh_sum = 0xffff;
-		}
+		udp6->uh_sum = in_cksum(m, hlen + plen);
+		if (udp6->uh_sum == 0)
+			udp6->uh_sum = 0xffff;
 
 		ip->ip_len = htons(hlen + plen);
 		ip->ip_ttl = in6_selecthlim(in6p, NULL); /* XXX */

--NextPart-20050416141745-1048100--