On Fri, Oct 22, 2021 at 03:39:01PM +0200, Hrvoje Popovski wrote:
> On 22.10.2021. 13:39, Jan Klemkow wrote:
> > Thats because, you only see this flags, if the checksum offloading is
> > enabled for "sending".  I'm still working/debugging on the sending side.
> > Thus, I just send a diff with the receiving part for now.
> > 
> > You can see if its working for your card with the netstat(8) statistics.
> > 
> > # netstat -s | grep software-checksummed
> > 
> > These counters should not raise much on the receive side if you put some
> > traffic over the interface.
> 
> Thank you for explanation...
> 
> I'm sending 8 tcp streams with iperf3 from some box to openbsd ixl box
> and here are results:
> 
> without diff
> smc24# netstat -s | grep software-checksummed
>         5039250 input datagrams software-checksummed
>         2592718 output datagrams software-checksummed
>                 2592709 packets software-checksummed
>                 5039250 packets software-checksummed
>         0 input packets software-checksummed
>         0 output packets software-checksummed
> 
> cca 6.12 Gbits/sec
> 
> 
> 
> with diff
> smc24# netstat -s | grep software-checksummed
>         0 input datagrams software-checksummed
>         2956546 output datagrams software-checksummed
>                 2956537 packets software-checksummed
>                 0 packets software-checksummed
>         0 input packets software-checksummed
>         0 output packets software-checksummed
> 
> cca 6.70 Gbits/sec
> 
> are result like those expected?
> 
> is forwarding testing any good for checksum offload diffs?

Hi Hrvoje,

Thanks a lot for you big testing efforts!

In case of forwarding the forwarding box just checks the IPv4 header
checksum and ignores the UDP/TCP header.  Your setup from one Box to
another is fine.

Here is a new diff, which also includes send checksum offloading.
Thus, all software-checksummed numbers should stay low in both
directions.

Could you test this diff with your ospf{6}d and NFS tests?
If you see IPv4 fragments in the ospf and NFS traffic within tcpdump(8),
your test should find the bugs pointed out by deraadt@ and claudio@.

You can provoke large NFS packets with the following options on your NFS
mount point.

server:/export /mnt nfs ro,intr,-r65536,-w65536

Thanks,
Jan

Index: dev/pci/if_ixl.c
===================================================================
RCS file: /mount/openbsd/cvs/src/sys/dev/pci/if_ixl.c,v
retrieving revision 1.75
diff -u -p -r1.75 if_ixl.c
--- dev/pci/if_ixl.c    23 Jul 2021 00:29:14 -0000      1.75
+++ dev/pci/if_ixl.c    25 Oct 2021 15:11:46 -0000
@@ -82,6 +82,10 @@
 #endif
 
 #include <netinet/in.h>
+#include <netinet/ip.h>
+#include <netinet/ip6.h>
+#include <netinet/tcp.h>
+#include <netinet/udp.h>
 #include <netinet/if_ether.h>
 
 #include <dev/pci/pcireg.h>
@@ -1388,6 +1392,7 @@ static int        ixl_rxeof(struct ixl_softc *,
 static void    ixl_rxfill(struct ixl_softc *, struct ixl_rx_ring *);
 static void    ixl_rxrefill(void *);
 static int     ixl_rxrinfo(struct ixl_softc *, struct if_rxrinfo *);
+static void    ixl_rx_checksum(struct mbuf *, uint64_t);
 
 #if NKSTAT > 0
 static void    ixl_kstat_attach(struct ixl_softc *);
@@ -1942,9 +1947,9 @@ ixl_attach(struct device *parent, struct
        ifp->if_capabilities = IFCAP_VLAN_MTU;
 #if 0
        ifp->if_capabilities |= IFCAP_VLAN_HWTAGGING;
-       ifp->if_capabilities |= IFCAP_CSUM_IPv4 | IFCAP_CSUM_TCPv4 |
-           IFCAP_CSUM_UDPv4;
 #endif
+       ifp->if_capabilities |= IFCAP_CSUM_IPv4 | IFCAP_CSUM_TCPv4 |
+           IFCAP_CSUM_UDPv4 | IFCAP_CSUM_TCPv6 | IFCAP_CSUM_UDPv6;
 
        ifmedia_init(&sc->sc_media, 0, ixl_media_change, ixl_media_status);
 
@@ -2772,6 +2777,69 @@ ixl_load_mbuf(bus_dma_tag_t dmat, bus_dm
 }
 
 static void
+ixl_tx_setup_offload(struct mbuf *mp, uint64_t *cmd)
+{
+       uint64_t         ip_hdr_len;
+       int              ipoff = ETHER_HDR_LEN;
+       uint8_t          ipproto;
+       struct ip       *ip;
+#ifdef INET6
+       struct ip6_hdr  *ip6;
+#endif
+       struct tcphdr   *th;
+       struct mbuf     *m;
+
+       switch (ntohs(mtod(mp, struct ether_header *)->ether_type)) {
+       case ETHERTYPE_IP:
+               if (mp->m_pkthdr.len < ETHER_HDR_LEN + sizeof(*ip))
+                       return;
+               m = m_getptr(mp, ETHER_HDR_LEN, &ipoff);
+               KASSERT(m != NULL && m->m_len - ipoff >= sizeof(*ip));
+               ip = (struct ip *)(m->m_data + ipoff);
+
+               if (mp->m_pkthdr.csum_flags & M_IPV4_CSUM_OUT)
+                       *cmd |= IXL_TX_DESC_CMD_IIPT_IPV4_CSUM;
+               else
+                       *cmd |= IXL_TX_DESC_CMD_IIPT_IPV4;
+
+               ip_hdr_len = ip->ip_hl << 2;
+               ipproto = ip->ip_p;
+               break;
+#ifdef INET6
+       case ETHERTYPE_IPV6:
+               if (mp->m_pkthdr.len < ETHER_HDR_LEN + sizeof(*ip6))
+                       return;
+               m = m_getptr(mp, ETHER_HDR_LEN, &ipoff);
+               KASSERT(m != NULL && m->m_len - ipoff >= sizeof(*ip6));
+               ip6 = (struct ip6_hdr *)(m->m_data + ipoff);
+
+               *cmd |= IXL_TX_DESC_CMD_IIPT_IPV6;
+
+               ip_hdr_len = sizeof(*ip6);
+               ipproto = ip6->ip6_nxt;
+               break;
+#endif
+       default:
+               return;
+       }
+
+       *cmd |= (ETHER_HDR_LEN >> 1) << IXL_TX_DESC_MACLEN_SHIFT;
+       *cmd |= (ip_hdr_len >> 2) << IXL_TX_DESC_IPLEN_SHIFT;
+
+       if (ipproto == IPPROTO_TCP && m->m_pkthdr.csum_flags & M_TCP_CSUM_OUT) {
+               th = (struct tcphdr *)(m->m_data + ipoff + ip_hdr_len);
+
+               *cmd |= IXL_TX_DESC_CMD_L4T_EOFT_TCP;
+               *cmd |= (uint64_t)th->th_off << IXL_TX_DESC_L4LEN_SHIFT;
+       }
+
+       if (ipproto == IPPROTO_UDP && m->m_pkthdr.csum_flags & M_UDP_CSUM_OUT) {
+               *cmd |= IXL_TX_DESC_CMD_L4T_EOFT_UDP;
+               *cmd |= (sizeof(struct udphdr) >> 2) << IXL_TX_DESC_L4LEN_SHIFT;
+       }
+}
+
+static void
 ixl_start(struct ifqueue *ifq)
 {
        struct ifnet *ifp = ifq->ifq_if;
@@ -2781,7 +2849,7 @@ ixl_start(struct ifqueue *ifq)
        struct ixl_tx_map *txm;
        bus_dmamap_t map;
        struct mbuf *m;
-       uint64_t cmd;
+       uint64_t cmd, off = 0;
        unsigned int prod, free, last, i;
        unsigned int mask;
        int post = 0;
@@ -2828,12 +2896,15 @@ ixl_start(struct ifqueue *ifq)
                bus_dmamap_sync(sc->sc_dmat, map, 0,
                    map->dm_mapsize, BUS_DMASYNC_PREWRITE);
 
+               ixl_tx_setup_offload(m, &off);
+
                for (i = 0; i < map->dm_nsegs; i++) {
                        txd = &ring[prod];
 
                        cmd = (uint64_t)map->dm_segs[i].ds_len <<
                            IXL_TX_DESC_BSIZE_SHIFT;
                        cmd |= IXL_TX_DESC_DTYPE_DATA | IXL_TX_DESC_CMD_ICRC;
+                       cmd |= off;
 
                        htolem64(&txd->addr, map->dm_segs[i].ds_addr);
                        htolem64(&txd->cmd, cmd);
@@ -3190,6 +3261,7 @@ ixl_rxeof(struct ixl_softc *sc, struct i
                                        m->m_pkthdr.csum_flags |= M_FLOWID;
                                }
 
+                               ixl_rx_checksum(m, word);
                                ml_enqueue(&ml, m);
                        } else {
                                ifp->if_ierrors++; /* XXX */
@@ -3320,6 +3392,23 @@ ixl_rxrinfo(struct ixl_softc *sc, struct
        free(ifr, M_TEMP, ixl_nqueues(sc) * sizeof(*ifr));
 
        return (rv);
+}
+
+static void
+ixl_rx_checksum(struct mbuf *m, uint64_t word)
+{
+       if (!ISSET(word, IXL_RX_DESC_L3L4P))
+               return;
+
+       if (ISSET(word, IXL_RX_DESC_IPE))
+               return;
+
+       m->m_pkthdr.csum_flags |= M_IPV4_CSUM_IN_OK;
+
+       if (ISSET(word, IXL_RX_DESC_L4E))
+               return;
+
+       m->m_pkthdr.csum_flags |= M_TCP_CSUM_IN_OK | M_UDP_CSUM_IN_OK;
 }
 
 static int

Reply via email to