On Mon, Feb 01, 2021 at 02:04:51AM +0100, Alexander Bluhm wrote:
> On Mon, Feb 01, 2021 at 08:08:56AM +1300, Richard Procter wrote:
> > - Might the rule disabling checksum offload for broadcasts on IFF_SIMPLEX
> > interfaces be weakened to disable checksum offload for all broadcast
> > packets instead?
> > - what motivates the new '!m->m_pkthdr.pf.routed??? term?
I think the best way to answer your questions, is to add a comment
to both if conditions.
ok?
bluhm
Index: net/if_ethersubr.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_ethersubr.c,v
retrieving revision 1.268
diff -u -p -r1.268 if_ethersubr.c
--- net/if_ethersubr.c 4 Jan 2021 21:21:41 -0000 1.268
+++ net/if_ethersubr.c 5 Feb 2021 09:40:46 -0000
@@ -227,7 +227,11 @@ ether_resolve(struct ifnet *ifp, struct
return (error);
eh->ether_type = htons(ETHERTYPE_IP);
- /* If broadcasting on a simplex interface, loopback a copy */
+ /*
+ * If broadcasting on a simplex interface, loopback a copy.
+ * The checksum must be calculated in software. Keep the
+ * contition in sync with in_ifcap_cksum().
+ */
if (ISSET(m->m_flags, M_BCAST) &&
ISSET(ifp->if_flags, IFF_SIMPLEX) &&
!m->m_pkthdr.pf.routed) {
Index: netinet/ip_output.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.363
diff -u -p -r1.363 ip_output.c
--- netinet/ip_output.c 2 Feb 2021 17:47:42 -0000 1.363
+++ netinet/ip_output.c 5 Feb 2021 09:38:09 -0000
@@ -79,6 +79,7 @@ void ip_mloopback(struct ifnet *, struct
static __inline u_int16_t __attribute__((__unused__))
in_cksum_phdr(u_int32_t, u_int32_t, u_int32_t);
void in_delayed_cksum(struct mbuf *);
+int in_ifcap_cksum(struct mbuf *, struct ifnet *, int);
#ifdef IPSEC
struct tdb *
@@ -458,8 +459,7 @@ sendit:
*/
if (ntohs(ip->ip_len) <= mtu) {
ip->ip_sum = 0;
- if ((ifp->if_capabilities & IFCAP_CSUM_IPv4) &&
- (ifp->if_bridgeidx == 0))
+ if (in_ifcap_cksum(m, ifp, IFCAP_CSUM_IPv4))
m->m_pkthdr.csum_flags |= M_IPV4_CSUM_OUT;
else {
ipstat_inc(ips_outswcsum);
@@ -719,9 +719,7 @@ ip_fragment(struct mbuf *m, struct ifnet
m->m_pkthdr.ph_ifidx = 0;
mhip->ip_off = htons((u_int16_t)mhip->ip_off);
mhip->ip_sum = 0;
- if ((ifp != NULL) &&
- (ifp->if_capabilities & IFCAP_CSUM_IPv4) &&
- (ifp->if_bridgeidx == 0))
+ if (in_ifcap_cksum(m, ifp, IFCAP_CSUM_IPv4))
m->m_pkthdr.csum_flags |= M_IPV4_CSUM_OUT;
else {
ipstat_inc(ips_outswcsum);
@@ -740,9 +738,7 @@ ip_fragment(struct mbuf *m, struct ifnet
ip->ip_len = htons((u_int16_t)m->m_pkthdr.len);
ip->ip_off |= htons(IP_MF);
ip->ip_sum = 0;
- if ((ifp != NULL) &&
- (ifp->if_capabilities & IFCAP_CSUM_IPv4) &&
- (ifp->if_bridgeidx == 0))
+ if (in_ifcap_cksum(m, ifp, IFCAP_CSUM_IPv4))
m->m_pkthdr.csum_flags |= M_IPV4_CSUM_OUT;
else {
ipstat_inc(ips_outswcsum);
@@ -1855,15 +1851,15 @@ 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) ||
- ip->ip_hl != 5 || ifp->if_bridgeidx != 0) {
+ if (!in_ifcap_cksum(m, ifp, IFCAP_CSUM_TCPv4) ||
+ ip->ip_hl != 5) {
tcpstat_inc(tcps_outswcsum);
in_delayed_cksum(m);
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) ||
- ip->ip_hl != 5 || ifp->if_bridgeidx != 0) {
+ if (!in_ifcap_cksum(m, ifp, IFCAP_CSUM_UDPv4) ||
+ ip->ip_hl != 5) {
udpstat_inc(udps_outswcsum);
in_delayed_cksum(m);
m->m_pkthdr.csum_flags &= ~M_UDP_CSUM_OUT; /* Clear */
@@ -1872,4 +1868,23 @@ in_proto_cksum_out(struct mbuf *m, struc
in_delayed_cksum(m);
m->m_pkthdr.csum_flags &= ~M_ICMP_CSUM_OUT; /* Clear */
}
+}
+
+int
+in_ifcap_cksum(struct mbuf *m, struct ifnet *ifp, int ifcap)
+{
+ if ((ifp == NULL) ||
+ !ISSET(ifp->if_capabilities, ifcap) ||
+ (ifp->if_bridgeidx != 0))
+ return (0);
+ /*
+ * Simplex interface sends packet back without hardware cksum.
+ * Keep this check in sync with the condition where ether_resolve()
+ * calls if_input_local().
+ */
+ if (ISSET(m->m_flags, M_BCAST) &&
+ ISSET(ifp->if_flags, IFF_SIMPLEX) &&
+ !m->m_pkthdr.pf.routed)
+ return (0);
+ return (1);
}
