Re: divert(4) checksum offload

2014-07-10 Thread Christian Weisgerber
On 2014-07-10, Henning Brauer  wrote:

>> 1.  Zero the protocol checksum.
>
> that should not be needed. at least afair.

Indeed, we'll overwrite it with the pseudo-header checksum anyway.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: divert(4) checksum offload

2014-07-10 Thread Henning Brauer
* Lawrence Teo  [2014-07-10 06:14]:
> This diff does the following for reinjected packets:
> 
> 1.  Zero the protocol checksum.

that should not be needed. at least afair.

> 2.  Set the checksum flag in pkthdr.
> 3a. For outbound packets, let the stack take care of the checksum.

i like.
might be biased :)

> 3b. For inbound packets, calculate the checksum immediately with
> in_proto_cksum_out(m, NULL).
> 
> I'm not sure if it's all right to use in_proto_cksum_out() for inbound
> packets (its name ends with "_out" after all :)) but using it really
> helps to simplify things and avoid redundant code.

well, could argue it goes out to divert...

-- 
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS. Virtual & Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/



divert(4) checksum offload

2014-07-09 Thread Lawrence Teo
Packets that are reinjected via a divert(4) socket will have their IP
and protocol checksums recalculated, since the userspace application
could have modified them.

Currently, these checksums are manually recalculated by divert_output().
But now that the new checksum offloading system is in place, we can use
that instead, at least for reinjected outbound packets.

This diff does the following for reinjected packets:

1.  Zero the protocol checksum.
2.  Set the checksum flag in pkthdr.
3a. For outbound packets, let the stack take care of the checksum.
3b. For inbound packets, calculate the checksum immediately with
in_proto_cksum_out(m, NULL).

I'm not sure if it's all right to use in_proto_cksum_out() for inbound
packets (its name ends with "_out" after all :)) but using it really
helps to simplify things and avoid redundant code.

Thoughts/ok?


Index: netinet/ip_divert.c
===
RCS file: /cvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.23
diff -U7 -p -r1.23 ip_divert.c
--- netinet/ip_divert.c 10 Jul 2014 03:17:59 -  1.23
+++ netinet/ip_divert.c 10 Jul 2014 03:35:07 -
@@ -81,18 +81,17 @@ int
 divert_output(struct inpcb *inp, struct mbuf *m, struct mbuf *nam,
 struct mbuf *control)
 {
struct ifqueue *inq;
struct sockaddr_in *sin;
struct socket *so;
struct ifaddr *ifa;
-   int s, error = 0, p_hdrlen = 0;
+   int s, error = 0, p_hdrlen = 0, dir;
struct ip *ip;
-   u_int16_t off, csum = 0;
-   u_int8_t nxt;
+   u_int16_t off, csum_flag = 0;
size_t p_off = 0;
 
m->m_pkthdr.rcvif = NULL;
m->m_nextpkt = NULL;
m->m_pkthdr.ph_rtableid = inp->inp_rtableid;
 
if (control)
@@ -113,64 +112,72 @@ divert_output(struct inpcb *inp, struct 
if (ip->ip_v != IPVERSION)
goto fail;
off = ip->ip_hl << 2;
if (off < sizeof(struct ip) || ntohs(ip->ip_len) < off ||
m->m_pkthdr.len < ntohs(ip->ip_len))
goto fail;
 
-   /*
-* Recalculate IP and protocol checksums since the userspace application
-* may have modified the packet prior to reinjection.
-*/
-   ip->ip_sum = 0;
-   ip->ip_sum = in_cksum(m, off);
-   nxt = ip->ip_p;
+   dir = (sin->sin_addr.s_addr == INADDR_ANY ? PF_OUT : PF_IN);
+
switch (ip->ip_p) {
case IPPROTO_TCP:
p_hdrlen = sizeof(struct tcphdr);
-   p_off = offsetof(struct tcphdr, th_sum);
+   p_off = off + offsetof(struct tcphdr, th_sum);
+   csum_flag = M_TCP_CSUM_OUT;
break;
case IPPROTO_UDP:
p_hdrlen = sizeof(struct udphdr);
-   p_off = offsetof(struct udphdr, uh_sum);
+   p_off = off + offsetof(struct udphdr, uh_sum);
+   csum_flag = M_UDP_CSUM_OUT;
break;
case IPPROTO_ICMP:
p_hdrlen = sizeof(struct icmp);
-   p_off = offsetof(struct icmp, icmp_cksum);
-   nxt = 0;
+   p_off = off + offsetof(struct icmp, icmp_cksum);
+   csum_flag = M_ICMP_CSUM_OUT;
break;
default:
/* nothing */
break;
}
-   if (p_hdrlen) {
-   if (m->m_pkthdr.len < off + p_hdrlen)
-   goto fail;
+   if (p_hdrlen && m->m_pkthdr.len < off + p_hdrlen)
+   goto fail;
 
-   if ((error = m_copyback(m, off + p_off, sizeof(csum), &csum, 
M_NOWAIT)))
-   goto fail;
-   csum = in4_cksum(m, nxt, off, m->m_pkthdr.len - off);
-   if (ip->ip_p == IPPROTO_UDP && csum == 0)
-   csum = 0x;
-   if ((error = m_copyback(m, off + p_off, sizeof(csum), &csum, 
M_NOWAIT)))
-   goto fail;
+   if (csum_flag) {
+   u_int16_t csum = 0;
+
+   if ((p_off + sizeof(u_int16_t)) > m->m_len) {
+   if ((error = m_copyback(m, p_off, sizeof(csum), &csum,
+   M_NOWAIT)))
+   goto fail;
+   } else
+   *(u_int16_t *)(mtod(m, caddr_t) + p_off) = 0;
+   m->m_pkthdr.csum_flags |= csum_flag;
}
 
m->m_pkthdr.pf.flags |= PF_TAG_DIVERTED_PACKET;
 
-   if (sin->sin_addr.s_addr != INADDR_ANY) {
+   if (dir == PF_IN) {
ipaddr.sin_addr = sin->sin_addr;
ifa = ifa_ifwithaddr(sintosa(&ipaddr), m->m_pkthdr.ph_rtableid);
if (ifa == NULL) {
error = EADDRNOTAVAIL;
goto fail;
}
m->m_pkthdr.rcvif = ifa->ifa_ifp;
 
inq = &ipintrq;
+
+   /*
+* Recalculate IP and protocol checksums for the inbound packet
+* sinc