From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: igc(4): fix recv. jumbo frames
To: Jan Klemkow <jan@openbsd.org>
Cc: tech@openbsd.org
Date: Wed, 31 Jul 2024 17:56:22 +0200

On Tue, Jul 30, 2024 at 08:12:08AM +0200, Jan Klemkow wrote:
> The DMA mapping and allocation of mbufs for jumbo frames uses different
> sizes.  Thus, we are ending up with corrupt mbufs, which leads to panics
> in later part of the TCP/IP stack:
> 
> panic: kernel diagnostic assertion "M_DATABUF(m) + M_SIZE(m) >= (m->m_data + m->m_len)"
> failed: file "/usr/src/sys/kern/uipc_mbuf.c", line 1364
> 
> With the following diff, we use the same size for mapping and
> allocation.
> 
> ok?

I have tested this diff successfully.  Before jumbo frames with
igc(4) paniced, now TCP is faster.

But are we allocating too much in the non-jumbo case?
    sc->rx_mbuf_sz = MAX_JUMBO_FRAME_SIZE + ETHER_ALIGN;
    m = MCLGETL(NULL, M_DONTWAIT, sc->rx_mbuf_sz)

This means for each receive packet we allocate a 16k mbuf cluster.
Usually only 1500 byte are needed.

ix(4) has this code and manages to chain the mbufs correctly.
    /* Use 2k clusters, even for jumbo frames */
    sc->rx_mbuf_sz = MCLBYTES + ETHER_ALIGN;

This seems a better approach.

bluhm

> Index: dev/pci/if_igc.c
> ===================================================================
> RCS file: /mount/openbsd/cvs/src/sys/dev/pci/if_igc.c,v
> diff -u -p -r1.25 if_igc.c
> --- dev/pci/if_igc.c	24 May 2024 06:02:53 -0000	1.25
> +++ dev/pci/if_igc.c	29 Jul 2024 21:17:31 -0000
> @@ -881,7 +881,7 @@ igc_init(void *arg)
>  	}
>  	igc_initialize_transmit_unit(sc);
>  
> -	sc->rx_mbuf_sz = MCLBYTES + ETHER_ALIGN;
> +	sc->rx_mbuf_sz = MAX_JUMBO_FRAME_SIZE + ETHER_ALIGN;
>  	/* Prepare receive descriptors and buffers. */
>  	if (igc_setup_receive_structures(sc)) {
>  		printf("%s: Could not setup receive structures\n",
> @@ -1232,7 +1232,7 @@ igc_rxrinfo(struct igc_softc *sc, struct
>  
>  	for (i = 0; i < sc->sc_nqueues; i++) {
>  		rxr = &sc->rx_rings[i];
> -		ifr[n].ifr_size = MCLBYTES;
> +		ifr[n].ifr_size = sc->rx_mbuf_sz;
>  		snprintf(ifr[n].ifr_name, sizeof(ifr[n].ifr_name), "%d", i);
>  		ifr[n].ifr_info = rxr->rx_ring;
>  		n++;
> @@ -2159,7 +2159,7 @@ igc_allocate_receive_buffers(struct igc_
>  	rxbuf = rxr->rx_buffers;
>  	for (i = 0; i < sc->num_rx_desc; i++, rxbuf++) {
>  		error = bus_dmamap_create(rxr->rxdma.dma_tag,
> -		    MAX_JUMBO_FRAME_SIZE, 1, MAX_JUMBO_FRAME_SIZE, 0,
> +		    sc->rx_mbuf_sz, 1, sc->rx_mbuf_sz, 0,
>  		    BUS_DMA_NOWAIT, &rxbuf->map);
>  		if (error) {
>  			printf("%s: Unable to create RX DMA map\n",