Re: Clear m->m_pkthdr.csum_flags when forwarding packets!

[Takahiro Kambe <taca%back-street.net@localhost>]

Hi,

In message <20080504165307.GA19753%panix.com@localhost>
        on Sun, 4 May 2008 12:53:07 -0400,
        Thor Lancelot Simon <tls%rek.tjls.com@localhost> wrote:
> > > Today, NetBSD 4.0_STABLE machine paniced in ip_output() when
> > > forwarding IPv4 multicast packet.  The packet was short (36 octets)
> > > UDP/IP pakcet.
> 
> This is a bug in the multicast forwarding code (it is related to a bug
> I have been investigating in ipf, pf, and bridge).  Look at ip_forward()
> and ip_flow(): on NetBSD, if you forward a packet using the same mbuf
> in which it was received, you must set csum_flags to 0 before handing
> that packet to ip_output.
Thanks very much for your explanation.

> This is because the same flags were used for "hardware checked checksum
> on receive" and "hardware should insert checksum on transmit" which, in
> my opinion, was a mistake.  The existing M_CSUM_DATA and M_CSUM_NO_PSEUDOHDR
> would have been sufficient for receive, leaving M_CSUM_TCPv4 (etc.) for
> transmit use.
I agree your opinion.

> Anyway, all code forwarding packets on NetBSD must explicitly set
> csum_flags to 0 after receive because of this.
Though I don't understand codes in ip_mroute.c very well, attached
patch might be things make better.  (Not tested since I don't have
testing environment now, hoping it could test in this month.)

-- 
Takahiro Kambe <taca%back-street.net@localhost>

Index: sys/netinet/ip_mroute.c
===================================================================
RCS file: /cvs/src-4/sys/netinet/ip_mroute.c,v
retrieving revision 1.1.1.1
diff -u -p -d -d -u -p -r1.1.1.1 ip_mroute.c
--- sys/netinet/ip_mroute.c     7 Feb 2007 01:50:26 -0000       1.1.1.1
+++ sys/netinet/ip_mroute.c     6 May 2008 04:51:24 -0000
@@ -1425,6 +1425,11 @@ ip_mforward(struct mbuf *m, struct ifnet
                return (1);
        }
 
+       /*
+        * Clear any in-bound checksum flags for this packet.
+        */
+       m->m_pkthdr.csum_flags = 0;
+
 #ifdef RSVP_ISI
        if (imo && ((vifi = imo->imo_multicast_vif) < numvifs)) {
                if (ip->ip_ttl < MAXTTL)