Hi,
dlg@ pointed out what looks to be a spurious call in the bridge to
in_proto_cksum_out(). I've checked the stack and found eight such calls
I think can be safely removed.
in_proto_cksum_out() computes, for packets flagged as needing it, the
transport checksum. It skips if the output interface is known and supports
checksum offload.
The removed calls are either leftovers from when PF was opaque to
checksums, or look to have been copied from these. (PF would modify
packets leaving the invalidated the checksums to be recomputed later on,
which induced recomputations on the input or forwarding paths.)
The correctness obligation: show for every call that it is not last on the
only path that may emit un-checksummed packets, viz. the IP output path
(i.e. non-forwarding, non-input).
if_bridge.c: the bridge operates at L2, so not on the IP output path.
ip_input.c: not in the IP output path
ip6_forward.c: not in the IP output path
ip6_output.c: ip6_output_ipsec_send() is called only by ip6_forward(),
so lies output the IP output path, too.
Testing: error symptoms would be bad transport checksums from
non-offloading interfaces or in encapsulated packets.
Looking for OKs, or test reports, particularly for:
- PF filtering the bridge
- IP6 forwarding
cheers,
Richard.
Index: net/if_bridge.c
===================================================================
RCS file: /cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.338
diff -u -p -U12 -p -r1.338 if_bridge.c
--- net/if_bridge.c 6 Nov 2019 03:51:26 -0000 1.338
+++ net/if_bridge.c 24 Jan 2020 04:37:01 -0000
@@ -1561,32 +1561,25 @@ bridge_ipsec(struct ifnet *ifp, struct e
* We don't need to do loop detection, the
* bridge will do that for us.
*/
#if NPF > 0
if ((encif = enc_getif(tdb->tdb_rdomain,
tdb->tdb_tap)) == NULL ||
pf_test(af, dir, encif, &m) != PF_PASS) {
m_freem(m);
return (1);
}
if (m == NULL)
return (1);
- else if (af == AF_INET)
- in_proto_cksum_out(m, encif);
-#ifdef INET6
- else if (af == AF_INET6)
- in6_proto_cksum_out(m, encif);
-#endif /* INET6 */
#endif /* NPF */
-
ip = mtod(m, struct ip *);
if ((af == AF_INET) &&
ip_mtudisc && (ip->ip_off & htons(IP_DF)) &&
tdb->tdb_mtu && ntohs(ip->ip_len) > tdb->tdb_mtu &&
tdb->tdb_mtutimeout > time_second)
bridge_send_icmp_err(ifp, eh, m,
hassnap, llc, tdb->tdb_mtu,
ICMP_UNREACH, ICMP_UNREACH_NEEDFRAG);
else
error = ipsp_process_packet(m, tdb, af, 0);
return (1);
} else
@@ -1715,25 +1708,24 @@ bridge_ip(struct ifnet *brifp, int dir,
/* Finally, we get to filter the packet! */
if (pf_test(AF_INET, dir, ifp, &m) != PF_PASS)
goto dropit;
if (m == NULL)
goto dropit;
#endif /* NPF > 0 */
/* Rebuild the IP header */
if (m->m_len < hlen && ((m = m_pullup(m, hlen)) == NULL))
return (NULL);
if (m->m_len < sizeof(struct ip))
goto dropit;
- in_proto_cksum_out(m, ifp);
ip = mtod(m, struct ip *);
ip->ip_sum = 0;
if (0 && (ifp->if_capabilities & IFCAP_CSUM_IPv4))
m->m_pkthdr.csum_flags |= M_IPV4_CSUM_OUT;
else {
ipstat_inc(ips_outswcsum);
ip->ip_sum = in_cksum(m, hlen);
}
break;
#ifdef INET6
@@ -1761,26 +1753,24 @@ bridge_ip(struct ifnet *brifp, int dir,
if ((brifp->if_flags & IFF_LINK2) == IFF_LINK2 &&
bridge_ipsec(ifp, eh, hassnap, &llc, dir, AF_INET6, hlen,
m))
return (NULL);
#endif /* IPSEC */
#if NPF > 0
if (pf_test(AF_INET6, dir, ifp, &m) != PF_PASS)
goto dropit;
if (m == NULL)
return (NULL);
#endif /* NPF > 0 */
- in6_proto_cksum_out(m, ifp);
-
break;
}
#endif /* INET6 */
default:
goto dropit;
break;
}
/* Reattach SNAP header */
if (hassnap) {
M_PREPEND(m, LLC_SNAPFRAMELEN, M_DONTWAIT);
Index: netinet/ip_input.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.347
diff -u -p -U12 -p -r1.347 ip_input.c
--- netinet/ip_input.c 23 Dec 2019 22:33:57 -0000 1.347
+++ netinet/ip_input.c 24 Jan 2020 04:37:01 -0000
@@ -575,36 +575,24 @@ ip_ours(struct mbuf **mp, int *offp, int
ipstat_inc(ips_##name) : ip6stat_inc(ip6s_##name))
#endif
int
ip_deliver(struct mbuf **mp, int *offp, int nxt, int af)
{
const struct protosw *psw;
int naf = af;
#ifdef INET6
int nest = 0;
#endif /* INET6 */
- /* pf might have modified stuff, might have to chksum */
- switch (af) {
- case AF_INET:
- in_proto_cksum_out(*mp, NULL);
- break;
-#ifdef INET6
- case AF_INET6:
- in6_proto_cksum_out(*mp, NULL);
- break;
-#endif /* INET6 */
- }
-
/*
* Tell launch routine the next header
*/
IPSTAT_INC(delivered);
while (nxt != IPPROTO_DONE) {
#ifdef INET6
if (af == AF_INET6 &&
ip6_hdrnestlimit && (++nest > ip6_hdrnestlimit)) {
ip6stat_inc(ip6s_toomanyhdr);
goto bad;
}
Index: netinet6/ip6_forward.c
===================================================================
RCS file: /cvs/src/sys/netinet6/ip6_forward.c,v
retrieving revision 1.97
diff -u -p -U12 -p -r1.97 ip6_forward.c
--- netinet6/ip6_forward.c 28 Nov 2017 15:32:51 -0000 1.97
+++ netinet6/ip6_forward.c 24 Jan 2020 04:37:01 -0000
@@ -300,26 +300,24 @@ reroute:
m->m_pkthdr.pf.flags &= ~(PF_TAG_GENERATED | PF_TAG_REROUTE);
} else if (m->m_pkthdr.pf.flags & PF_TAG_REROUTE) {
/* tag as generated to skip over pf_test on rerun */
m->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
srcrt = 1;
rtfree(rt);
rt = NULL;
if_put(ifp);
ifp = NULL;
goto reroute;
}
#endif
- in6_proto_cksum_out(m, ifp);
-
/* Check the size after pf_test to give pf a chance to refragment. */
if (m->m_pkthdr.len > ifp->if_mtu) {
if (mcopy)
icmp6_error(mcopy, ICMP6_PACKET_TOO_BIG, 0,
ifp->if_mtu);
m_freem(m);
goto out;
}
error = ifp->if_output(ifp, m, sin6tosa(dst), rt);
if (error) {
ip6stat_inc(ip6s_cantforward);
Index: netinet6/ip6_output.c
===================================================================
RCS file: /cvs/src/sys/netinet6/ip6_output.c,v
retrieving revision 1.245
diff -u -p -U12 -p -r1.245 ip6_output.c
--- netinet6/ip6_output.c 29 Nov 2019 16:41:01 -0000 1.245
+++ netinet6/ip6_output.c 24 Jan 2020 04:37:02 -0000
@@ -2775,25 +2775,24 @@ ip6_output_ipsec_send(struct tdb *tdb, s
m_freem(m);
return EHOSTUNREACH;
}
if (m == NULL)
return 0;
/*
* PF_TAG_REROUTE handling or not...
* Packet is entering IPsec so the routing is
* already overruled by the IPsec policy.
* Until now the change was not reconsidered.
* What's the behaviour?
*/
- in6_proto_cksum_out(m, encif);
#endif
m->m_flags &= ~(M_BCAST | M_MCAST); /* just in case */
/* Callee frees mbuf */
error = ipsp_process_packet(m, tdb, AF_INET6, tunalready);
if (error) {
ipsecstat_inc(ipsec_odrops);
tdb->tdb_odrops++;
}
return error;
}
#endif /* IPSEC */
