Back in August I sent a diff to fix ICMP checksum calculation in
in_proto_cksum_out() and in_delayed_cksum() in cases where the ICMP
checksum field is not in the first mbuf of an mbuf chain (original post
at http://marc.info/?l=openbsd-tech&m=137571298511653&w=2 ).
bluhm@ replied on tech@ with the following feedback:
On Fri, Aug 09, 2013 at 02:21:29AM +0200, Alexander Bluhm wrote:
> On Mon, Aug 05, 2013 at 10:28:57AM -0400, Lawrence Teo wrote:
> > Index: ip_output.c
> > ===================================================================
> > RCS file: /cvs/src/sys/netinet/ip_output.c,v
> > retrieving revision 1.244
> > diff -U5 -p -r1.244 ip_output.c
> > --- ip_output.c 31 Jul 2013 15:41:52 -0000 1.244
> > +++ ip_output.c 5 Aug 2013 02:44:20 -0000
> > @@ -2058,25 +2058,35 @@ ip_mloopback(struct ifnet *ifp, struct m
> > */
> > void
> > in_delayed_cksum(struct mbuf *m)
> > {
> > struct ip *ip;
> > - u_int16_t csum, offset;
> > + u_int16_t csum = 0, offset;
> >
> > ip = mtod(m, struct ip *);
> > offset = ip->ip_hl << 2;
> > +
> > + if (ip->ip_p == IPPROTO_ICMP)
> > + if (m_copyback(m, offset + offsetof(struct icmp, icmp_cksum),
> > + sizeof(csum), &csum, M_NOWAIT))
> > + return;
>
> The code at the end of this function tries to avoid the m_copyback()
> in the common case unless (offset + sizeof(u_int16_t)) > m->m_len).
> Do we want this optimization here?
>
> bluhm
Here's my revised diff that preserves that optimization so that existing
behavior won't change in the common case where the protocol checksum
field is in the first mbuf.
The new diff also implements similar logic in in6_proto_cksum_out().
Comments/feedback appreciated. :)
Thanks,
Lawrence
Index: netinet/in.h
===================================================================
RCS file: /cvs/src/sys/netinet/in.h,v
retrieving revision 1.97
diff -u -p -u -p -r1.97 in.h
--- netinet/in.h 9 Oct 2013 09:33:43 -0000 1.97
+++ netinet/in.h 16 Oct 2013 15:14:39 -0000
@@ -835,7 +835,7 @@ int in_broadcast(struct in_addr, stru
int in_canforward(struct in_addr);
int in_cksum(struct mbuf *, int);
int in4_cksum(struct mbuf *, u_int8_t, int, int);
-void in_delayed_cksum(struct mbuf *);
+void in_delayed_cksum(struct mbuf *, u_int8_t, u_int16_t);
int in_localaddr(struct in_addr, u_int);
void in_socktrim(struct sockaddr_in *);
char *inet_ntoa(struct in_addr);
Index: netinet/ip_output.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.247
diff -u -p -u -p -r1.247 ip_output.c
--- netinet/ip_output.c 18 Oct 2013 09:04:03 -0000 1.247
+++ netinet/ip_output.c 18 Oct 2013 15:14:04 -0000
@@ -2050,34 +2050,35 @@ ip_mloopback(struct ifnet *ifp, struct m
* Process a delayed payload checksum calculation.
*/
void
-in_delayed_cksum(struct mbuf *m)
+in_delayed_cksum(struct mbuf *m, u_int8_t proto, u_int16_t p_off)
{
struct ip *ip;
- u_int16_t csum, offset;
+ u_int16_t csum = 0, hlen, *p = NULL;
ip = mtod(m, struct ip *);
- offset = ip->ip_hl << 2;
- csum = in4_cksum(m, 0, offset, m->m_pkthdr.len - offset);
- if (csum == 0 && ip->ip_p == IPPROTO_UDP)
- csum = 0xffff;
-
- switch (ip->ip_p) {
- case IPPROTO_TCP:
- offset += offsetof(struct tcphdr, th_sum);
- break;
-
- case IPPROTO_UDP:
- offset += offsetof(struct udphdr, uh_sum);
- break;
-
- default:
+ if (ip->ip_p != proto)
return;
+
+ hlen = ip->ip_hl << 2;
+ p_off += hlen;
+ if ((p_off + sizeof(u_int16_t)) <= m->m_len)
+ p = (u_int16_t *)(mtod(m, caddr_t) + p_off);
+
+ if (proto == IPPROTO_ICMP) {
+ if (p)
+ *p = 0;
+ else if (m_copyback(m, p_off, sizeof(csum), &csum, M_NOWAIT))
+ return;
}
- if ((offset + sizeof(u_int16_t)) > m->m_len)
- m_copyback(m, offset, sizeof(csum), &csum, M_NOWAIT);
+ csum = in4_cksum(m, 0, hlen, m->m_pkthdr.len - hlen);
+ if (csum == 0 && proto == IPPROTO_UDP)
+ csum = 0xffff;
+
+ if (p)
+ *p = csum;
else
- *(u_int16_t *)(mtod(m, caddr_t) + offset) = csum;
+ m_copyback(m, p_off, sizeof(csum), &csum, M_NOWAIT);
}
void
@@ -2086,25 +2087,20 @@ in_proto_cksum_out(struct mbuf *m, struc
if (m->m_pkthdr.csum_flags & M_TCP_CSUM_OUT) {
if (!ifp || !(ifp->if_capabilities & IFCAP_CSUM_TCPv4) ||
ifp->if_bridgeport != NULL) {
- in_delayed_cksum(m);
+ in_delayed_cksum(m, IPPROTO_TCP,
+ offsetof(struct tcphdr, th_sum));
m->m_pkthdr.csum_flags &= ~M_TCP_CSUM_OUT; /* Clear */
}
} else if (m->m_pkthdr.csum_flags & M_UDP_CSUM_OUT) {
if (!ifp || !(ifp->if_capabilities & IFCAP_CSUM_UDPv4) ||
ifp->if_bridgeport != NULL) {
- in_delayed_cksum(m);
+ in_delayed_cksum(m, IPPROTO_UDP,
+ offsetof(struct udphdr, uh_sum));
m->m_pkthdr.csum_flags &= ~M_UDP_CSUM_OUT; /* Clear */
}
} else if (m->m_pkthdr.csum_flags & M_ICMP_CSUM_OUT) {
- struct ip *ip = mtod(m, struct ip *);
- int hlen;
- struct icmp *icp;
-
- hlen = ip->ip_hl << 2;
- icp = (struct icmp *)(mtod(m, caddr_t) + hlen);
- icp->icmp_cksum = 0;
- icp->icmp_cksum = in4_cksum(m, 0, hlen,
- ntohs(ip->ip_len) - hlen);
+ in_delayed_cksum(m, IPPROTO_ICMP,
+ offsetof(struct icmp, icmp_cksum));
m->m_pkthdr.csum_flags &= ~M_ICMP_CSUM_OUT; /* Clear */
}
}
Index: netinet6/ip6_output.c
===================================================================
RCS file: /cvs/src/sys/netinet6/ip6_output.c,v
retrieving revision 1.144
diff -u -p -u -p -r1.144 ip6_output.c
--- netinet6/ip6_output.c 17 Oct 2013 16:27:46 -0000 1.144
+++ netinet6/ip6_output.c 18 Oct 2013 15:06:23 -0000
@@ -126,7 +126,7 @@ int ip6_splithdr(struct mbuf *, struct i
int ip6_getpmtu(struct route_in6 *, struct route_in6 *,
struct ifnet *, struct in6_addr *, u_long *, int *);
int copypktopts(struct ip6_pktopts *, struct ip6_pktopts *, int);
-void in6_delayed_cksum(struct mbuf *, u_int8_t);
+void in6_delayed_cksum(struct mbuf *, u_int8_t, u_int16_t);
void in6_proto_cksum_out(struct mbuf *, struct ifnet *);
/* Context for non-repeating IDs */
@@ -3173,44 +3173,35 @@ ip6_randomid_init(void)
* Process a delayed payload checksum calculation.
*/
void
-in6_delayed_cksum(struct mbuf *m, u_int8_t nxt)
+in6_delayed_cksum(struct mbuf *m, u_int8_t nxt, u_int16_t p_off)
{
int nxtp, offset;
- u_int16_t csum;
+ u_int16_t csum = 0, *p = NULL;
offset = ip6_lasthdr(m, 0, IPPROTO_IPV6, &nxtp);
if (offset <= 0 || nxtp != nxt)
/* If the desired next protocol isn't found, punt. */
return;
+ p_off += offset;
+ if ((p_off + sizeof(u_int16_t)) <= m->m_len)
+ p = (u_int16_t *)(mtod(m, caddr_t) + p_off);
+
if (nxt == IPPROTO_ICMPV6) {
- struct icmp6_hdr *icmp6;
- icmp6 = (struct icmp6_hdr *)(mtod(m, caddr_t) + offset);
- icmp6->icmp6_cksum = 0;
+ if (p)
+ *p = 0;
+ else if (m_copyback(m, p_off, sizeof(csum), &csum, M_NOWAIT))
+ return;
}
csum = (u_int16_t)(in6_cksum(m, nxt, offset, m->m_pkthdr.len - offset));
+ if (csum == 0 && nxt == IPPROTO_UDP)
+ csum = 0xffff;
- switch (nxt) {
- case IPPROTO_TCP:
- offset += offsetof(struct tcphdr, th_sum);
- break;
-
- case IPPROTO_UDP:
- offset += offsetof(struct udphdr, uh_sum);
- if (csum == 0)
- csum = 0xffff;
- break;
-
- case IPPROTO_ICMPV6:
- offset += offsetof(struct icmp6_hdr, icmp6_cksum);
- break;
- }
-
- if ((offset + sizeof(u_int16_t)) > m->m_len)
- m_copyback(m, offset, sizeof(csum), &csum, M_NOWAIT);
+ if (p)
+ *p = csum;
else
- *(u_int16_t *)(mtod(m, caddr_t) + offset) = csum;
+ m_copyback(m, p_off, sizeof(csum), &csum, M_NOWAIT);
}
void
@@ -3219,17 +3210,20 @@ in6_proto_cksum_out(struct mbuf *m, stru
if (m->m_pkthdr.csum_flags & M_TCP_CSUM_OUT) {
if (!ifp || !(ifp->if_capabilities & IFCAP_CSUM_TCPv6) ||
ifp->if_bridgeport != NULL) {
- in6_delayed_cksum(m, IPPROTO_TCP);
+ in6_delayed_cksum(m, IPPROTO_TCP,
+ offsetof(struct tcphdr, th_sum));
m->m_pkthdr.csum_flags &= ~M_TCP_CSUM_OUT; /* Clear */
}
} else if (m->m_pkthdr.csum_flags & M_UDP_CSUM_OUT) {
if (!ifp || !(ifp->if_capabilities & IFCAP_CSUM_UDPv6) ||
ifp->if_bridgeport != NULL) {
- in6_delayed_cksum(m, IPPROTO_UDP);
+ in6_delayed_cksum(m, IPPROTO_UDP,
+ offsetof(struct udphdr, uh_sum));
m->m_pkthdr.csum_flags &= ~M_UDP_CSUM_OUT; /* Clear */
}
} else if (m->m_pkthdr.csum_flags & M_ICMP_CSUM_OUT) {
- in6_delayed_cksum(m, IPPROTO_ICMPV6);
+ in6_delayed_cksum(m, IPPROTO_ICMPV6,
+ offsetof(struct icmp6_hdr, icmp6_cksum));
m->m_pkthdr.csum_flags &= ~M_ICMP_CSUM_OUT; /* Clear */
}
}
