On Wed, Jan 26, 2022 at 01:29:42AM +0100, Alexander Bluhm wrote:
> Hi,
>
> There were some problems with ix(4) and ixl(4) hardware checksumming
> for the output path on strict alignment architectures.
>
> I have merged jan@'s diffs and added some sanity checks and
> workarounds.
>
> - If the first mbuf is not aligned or not contigous, use m_copydata()
> to extract the IP, IPv6, TCP header.
> - If the header is in the first mbuf, use m_data for the fast path.
> - Add netstat counter for invalid header chains. This makes
> us aware when hardware checksumming fails.
> - Add netstat counter for header copies. This indicates that
> better storage allocation in the network stack is possible.
> It also allows to recognize alignment problems on non-strict
> architectures.
> - There is not risk of crashes on sparc64.
>
> Does this aproach make sense?
>
> ix(4) works quite well, but finds some UDP packets that need copy.
> ixl(4) has not been tested yet. I would like to have some feedback
> for the idea first.
>
> bluhm
>
> Index: sys/dev/pci/if_ixl.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/dev/pci/if_ixl.c,v
> retrieving revision 1.78
> diff -u -p -r1.78 if_ixl.c
> --- sys/dev/pci/if_ixl.c 9 Jan 2022 05:42:54 -0000 1.78
> +++ sys/dev/pci/if_ixl.c 25 Jan 2022 23:50:01 -0000
> @@ -1388,6 +1398,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 *);
> @@ -3190,6 +3318,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 +3449,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
this is ok by me. tested on both amd64 and sparc64.
dlg
