On Wed, Nov 14, 2018 at 11:49:02AM +1000, David Gwynne wrote:
> while looking at outgoing ttl processing and rfc 6040, i noted that gif
> and gre patched a packets ttl, but didn't update the checksum on the
> packet.
>
> i think there's two issues here. firstly, we need to update the checksum
> when the packet is patched, but only for v4, and secondly, we should
> clear the csum_flags on an mbuf after the first decap.
>
> this patch addresses the first issue. i have removed the ttl patching in
> gre and gif, but gif and the ipip code still patch the ecn on a packet.
> gif did not update the v4 checksum, and ipip recalculates the whole
> checksum.
>
> the stuff procter worked on in pf shows that you can do a small
> update to the checksum rather than recalcuate the whole thing. this
> adds a function to the ecn code that does just that for tos field,
> which is where the ecn is store.
>
> it also updates gif and ipip to use it.
>
> ok?
i forgot to say that having ipip code blindly recalculate the checksum
means that a faulty inner packet will look fine when it shouldnt. it is
much better to update the checksum with the ecn change, and let the
whole cksum check fail later on.
this is a better diff. this one even compiles.
Index: netinet/ip_ecn.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_ecn.c,v
retrieving revision 1.8
diff -u -p -r1.8 ip_ecn.c
--- netinet/ip_ecn.c 24 Sep 2016 14:51:37 -0000 1.8
+++ netinet/ip_ecn.c 14 Nov 2018 10:13:14 -0000
@@ -154,3 +154,24 @@ ip_ecn_egress(int mode, u_int8_t *outer,
}
return (1);
}
+
+/*
+ * Patch the checksum with the difference between the old and new tos.
+ * The patching is based on what pf_patch_8() and pf_cksum_fixkup() do,
+ * but they're in pf, so we can't rely on them being available.
+ */
+void
+ip_tos_patch(struct ip *ip, uint8_t tos)
+{
+ uint16_t old;
+ uint16_t new;
+ uint32_t x;
+
+ old = htons(ip->ip_tos);
+ new = htons(tos);
+
+ ip->ip_tos = tos;
+
+ x = ip->ip_sum + old - new;
+ ip->ip_sum = (x) + (x >> 16);
+}
Index: netinet/ip_ecn.h
===================================================================
RCS file: /cvs/src/sys/netinet/ip_ecn.h,v
retrieving revision 1.6
diff -u -p -r1.6 ip_ecn.h
--- netinet/ip_ecn.h 15 Mar 2012 16:37:11 -0000 1.6
+++ netinet/ip_ecn.h 14 Nov 2018 10:13:14 -0000
@@ -47,5 +47,6 @@
#if defined(_KERNEL)
extern void ip_ecn_ingress(int, u_int8_t *, u_int8_t *);
extern int ip_ecn_egress(int, u_int8_t *, u_int8_t *);
+void ip_tos_patch(struct ip *, uint8_t);
#endif /* _KERNEL */
#endif /* _NETINET_IP_ECN_H_ */
Index: netinet/ip_ipip.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_ipip.c,v
retrieving revision 1.88
diff -u -p -r1.88 ip_ipip.c
--- netinet/ip_ipip.c 28 Aug 2018 15:15:02 -0000 1.88
+++ netinet/ip_ipip.c 14 Nov 2018 10:13:14 -0000
@@ -239,16 +239,14 @@ ipip_input_if(struct mbuf **mp, int *off
itos = ip->ip_tos;
mode = m->m_flags & (M_AUTH|M_CONF) ?
ECN_ALLOWED_IPSEC : ECN_ALLOWED;
- if (!ip_ecn_egress(mode, &otos, &ip->ip_tos)) {
+ if (!ip_ecn_egress(mode, &otos, &itos)) {
DPRINTF(("%s: ip_ecn_egress() failed\n", __func__));
ipipstat_inc(ipips_pdrops);
goto bad;
}
/* re-calculate the checksum if ip_tos was changed */
- if (itos != ip->ip_tos) {
- ip->ip_sum = 0;
- ip->ip_sum = in_cksum(m, hlen);
- }
+ if (itos != ip->ip_tos)
+ ip_tos_patch(ip, itos);
break;
#ifdef INET6
case IPPROTO_IPV6:
Index: net/if_gif.c
===================================================================
RCS file: /cvs/src/sys/net/if_gif.c,v
retrieving revision 1.123
diff -u -p -r1.123 if_gif.c
--- net/if_gif.c 14 Nov 2018 03:20:03 -0000 1.123
+++ net/if_gif.c 14 Nov 2018 10:13:14 -0000
@@ -809,7 +809,8 @@ gif_input(struct gif_tunnel *key, struct
if (ip_ecn_egress(ECN_ALLOWED, &otos, &itos) == 0)
goto drop;
- ip->ip_tos = itos;
+ if (itos != ip->ip_tos)
+ ip_tos_patch(ip, itos);
m->m_pkthdr.ph_family = AF_INET;
