Re: hme(4) hardware RX checksum

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

christos%zoulas.com@localhost wrote:

> | Now I notice gem.c has the similar code and hme.c needs to
> | have one more `else' for swcsum cases. Which is better?
> 
> should be fixed too I guess.

Ah, gem.c has the else correctly.

> | --- hme.c   16 Dec 2008 22:35:31 -0000      1.68
> | +++ hme.c   6 Mar 2009 17:45:55 -0000
> | @@ -836,7 +836,7 @@
> |             }
> |  
> |             m0->m_pkthdr.csum_flags |= M_CSUM_DATA | M_CSUM_NO_PSEUDOHDR;
> | -   }
> | +   } else
> |  swcsum:
> |             m0->m_pkthdr.csum_flags = 0;
> |  #endif
> 
> right the else needs to be there.

Committed.

> | BTW, does hardware checksum work even for VLAN packets?
> 
> It must, because there is vlan stuff inside a hw capabilities if statement
> (I am just guessing).

It is not ETHERCAP_VLAN_HWTAGGING but ETHERCAP_VLAN_MTU
and hme(4) doesn't have HW capabilities for VLAN tags.

So once vlanif is configured on the hme(4) ETHERCAP_VLAN_MTU
is always set and all RX packets are treated as VLAN encapsulized.
In that case ETHERTYPE is not decoded correctly so all IP packets
are swcsum'ed.

> | At least it looks wrong to check sc_ethercom.ec_capenable
> | without checking eh->ether_type == ETHERTYPE_VLAN.
> 
> You are suggesting:
> 
> if (eh->ether_type == ETHERTYPE_VLAN && 
>     sc->sc_ethercom.ec_capenable & ETHERCAP_VLAN_MTU) {
> ?

hme(4) seems to provide a dumb checksum for data after the IP header,
so I'm not sure if it can provide proper checksum even for
ETHERTYPE_VLAN packets. With a quick test, VLAN packets are
also swcsum'ed.

Currently hme(4) and gem(4) have the similar code which decodes
RX packet headers to adjust (deducting IP option sum etc.)
a dumb checksum data provided by hardware.
fxp(4) (i82559) also has the similar RX checksum hardware so
I wonder how we can share a common function to decode RX packets
for such dumb RX M_CSUM_DATA checksums.

---
 :
        } else if ((sc->sc_flags & FXPF_82559_RXCSUM) != 0) {
                struct ifnet *ifp = &sc->sc_ethercom.ec_if;
                struct ether_header *eh;
                struct ip *ip;
                struct udphdr *uh;
                u_int hlen, pktlen;
                uint32_t optsum, optlen;
                uint16_t *opts;

                if (len < ETHER_HDR_LEN + sizeof(struct ip))
                        goto out;
                pktlen = len - ETHER_HDR_LEN;
                eh = mtod(m, struct ether_header *);
                if (ntohs(eh->ether_type) != ETHERTYPE_IP)
                        goto out;
                ip = (struct ip *)((uint8_t *)eh + ETHER_HDR_LEN);
                if (ip->ip_v != IPVERSION)
                        goto out;

                hlen = ip->ip_hl << 2;
                if (hlen < sizeof(struct ip))
                        goto out;

                /*
                 * Bail if too short, has random trailing garbage, truncated,
                 * fragment, or has ethernet pad.
                 */
                if (ntohs(ip->ip_len) < hlen ||
                    ntohs(ip->ip_len) != pktlen ||
                    (ntohs(ip->ip_off) & (IP_MF | IP_OFFMASK)) != 0)
                        goto out;

                switch (ip->ip_p) {
                case IPPROTO_TCP:
                        if ((ifp->if_csum_flags_rx & M_CSUM_TCPv4) == 0 ||
                            pktlen < (hlen + sizeof(struct tcphdr)))
                                goto out;
                        csum_flags =
                            M_CSUM_TCPv4 | M_CSUM_DATA | M_CSUM_NO_PSEUDOHDR;
                        break;
                case IPPROTO_UDP:
                        if ((ifp->if_csum_flags_rx & M_CSUM_UDPv4) == 0 ||
                            pktlen < (hlen + sizeof(struct udphdr)))
                                goto out;
                        uh = (struct udphdr *)((uint8_t *)ip + hlen);
                        if (uh->uh_sum == 0)
                                goto out;       /* no checksum */
                        csum_flags =
                            M_CSUM_UDPv4 | M_CSUM_DATA | M_CSUM_NO_PSEUDOHDR;
                        break;
                default:
                        goto out;
                }

                /* Extract computed checksum. */
                csum_data = be16dec(mtod(m, uint8_t *) + len);

                /* If the packet had IP options, we have to deduct them. */
                optlen = hlen - sizeof(struct ip);
                if (optlen > 0) {
                        optsum = 0;
                        opts = (uint16_t *)((uint8_t *)ip + sizeof(struct ip));

                        while (optlen > 1) {
                                optsum += ntohs(*opts++);
                                optlen -= sizeof(uint16_t);
                        }
                        while (optsum >> 16)
                                optsum = (optsum >> 16) + (optsum & 0xffff);

                        /* Deduct the IP opts sum from the hwsum (RFC 1624). */
                        csum_data = ~(~csum_data - ~optsum);

                        while (csum_data >> 16)
                                csum_data =
                                    (csum_data >> 16) + (csum_data & 0xffff);
                }
        }
 out:
        m->m_pkthdr.csum_flags = csum_flags;
        m->m_pkthdr.csum_data = csum_data;

---
Izumi Tsutsui