Re: kern/40605

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

The following reply was made to PR kern/40605; it has been noted by GNATS.

From: Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: kern-bug-people%NetBSD.org@localhost, gnats-admin%NetBSD.org@localhost,
        netbsd-pr%pool.julien-oster.de@localhost, 
tsutsui%ceres.dti.ne.jp@localhost
Subject: Re: kern/40605
Date: Tue, 21 Apr 2009 20:34:55 +0900

 > From: Michael Stapelberg <michael+netbsd%stapelberg.de@localhost>
 >  By reading some more of the code, I think that the crong checksum flag is 
 > set
 >  when receiving the packet, to be specific in sys/dev/ic/rtl8169.c in
 >  re_rxeof(), at the end of the function. What seems a bit odd to me is that 
 > the
 >  protocol family is not checked, which is done in other drivers. Instead, the
 >  flags for IPv4 are just added, if the hardware says so. Probably the cards
 >  affected by this bug *do* say so.
 
 It looks newer RTL8168C/8111C chips also recognize IPv6 packets
 (and support IPv6/TCPv6/UDPv6 hwcsums?), so driver should check
 not only PROTOID but also IPV4 bit in RX descs for M_CSUM_TCPv4
 and M_CSUM_UDPv4 on those RTKQ_DESCV2 variants.
 
 How about the attached patch (against -current)?
 
 Note netbsd-5 doesn't have hwcsum support for those newer chips (yet),
 so you also have to pull the following revisions:
 rtl8169.c:     1.109, 1.110, 1.111, 1.112, 1.113
 rtl81x9reg.h:  1.34, 1.35
 rtl81x9var.h:  1.43, 1.44, 1.45
 
 
 Index: rtl8169.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/ic/rtl8169.c,v
 retrieving revision 1.116
 diff -u -r1.116 rtl8169.c
 --- rtl8169.c  13 Apr 2009 12:38:06 -0000      1.116
 +++ rtl8169.c  21 Apr 2009 11:20:13 -0000
 @@ -1272,25 +1272,49 @@
                m->m_pkthdr.rcvif = ifp;
  
                /* Do RX checksumming */
 +              if ((sc->sc_quirk & RTKQ_DESCV2) == 0) {
 +                      /* Check IP header checksum */
 +                      if ((rxstat & RE_RDESC_STAT_PROTOID) != 0) {
 +                              m->m_pkthdr.csum_flags |= M_CSUM_IPv4;
 +                              if (rxstat & RE_RDESC_STAT_IPSUMBAD)
 +                                      m->m_pkthdr.csum_flags |=
 +                                          M_CSUM_IPv4_BAD;
 +                      }
  
 -              /* Check IP header checksum */
 -              if ((rxstat & RE_RDESC_STAT_PROTOID) != 0 &&
 -                  ((sc->sc_quirk & RTKQ_DESCV2) == 0 ||
 -                   (rxvlan & RE_RDESC_VLANCTL_IPV4) != 0)) {
 -                      m->m_pkthdr.csum_flags |= M_CSUM_IPv4;
 -                      if (rxstat & RE_RDESC_STAT_IPSUMBAD)
 -                              m->m_pkthdr.csum_flags |= M_CSUM_IPv4_BAD;
 -              }
 -
 -              /* Check TCP/UDP checksum */
 -              if (RE_TCPPKT(rxstat)) {
 -                      m->m_pkthdr.csum_flags |= M_CSUM_TCPv4;
 -                      if (rxstat & RE_RDESC_STAT_TCPSUMBAD)
 -                              m->m_pkthdr.csum_flags |= M_CSUM_TCP_UDP_BAD;
 -              } else if (RE_UDPPKT(rxstat)) {
 -                      m->m_pkthdr.csum_flags |= M_CSUM_UDPv4;
 -                      if (rxstat & RE_RDESC_STAT_UDPSUMBAD)
 -                              m->m_pkthdr.csum_flags |= M_CSUM_TCP_UDP_BAD;
 +                      /* Check TCP/UDP checksum */
 +                      if (RE_TCPPKT(rxstat)) {
 +                              m->m_pkthdr.csum_flags |= M_CSUM_TCPv4;
 +                              if (rxstat & RE_RDESC_STAT_TCPSUMBAD)
 +                                      m->m_pkthdr.csum_flags |=
 +                                          M_CSUM_TCP_UDP_BAD;
 +                      } else if (RE_UDPPKT(rxstat)) {
 +                              m->m_pkthdr.csum_flags |= M_CSUM_UDPv4;
 +                              if (rxstat & RE_RDESC_STAT_UDPSUMBAD)
 +                                      m->m_pkthdr.csum_flags |=
 +                                          M_CSUM_TCP_UDP_BAD;
 +                      }
 +              } else {
 +                      /* Check IPv4 header checksum */
 +                      if ((rxstat & RE_RDESC_STAT_PROTOID) != 0 &&
 +                           (rxvlan & RE_RDESC_VLANCTL_IPV4) != 0) {
 +                              m->m_pkthdr.csum_flags |= M_CSUM_IPv4;
 +                              if (rxstat & RE_RDESC_STAT_IPSUMBAD)
 +                                      m->m_pkthdr.csum_flags |=
 +                                          M_CSUM_IPv4_BAD;
 +
 +                              /* Check TCPv4/UDPv4 checksum */
 +                              if (RE_TCPPKT(rxstat)) {
 +                                      m->m_pkthdr.csum_flags |= M_CSUM_TCPv4;
 +                                      if (rxstat & RE_RDESC_STAT_TCPSUMBAD)
 +                                              m->m_pkthdr.csum_flags |=
 +                                                  M_CSUM_TCP_UDP_BAD;
 +                              } else if (RE_UDPPKT(rxstat)) {
 +                                      m->m_pkthdr.csum_flags |= M_CSUM_UDPv4;
 +                                      if (rxstat & RE_RDESC_STAT_UDPSUMBAD)
 +                                              m->m_pkthdr.csum_flags |=
 +                                                  M_CSUM_TCP_UDP_BAD;
 +                              }
 +                      }
                }
  
                if (rxvlan & RE_RDESC_VLANCTL_TAG) {
 
 
 ---
 Izumi Tsutsui