Re: git: 9e6d11ce9a51 - main - vtnet: Adjust rx buffer so IP header 32-bit aligned

On Thu, Dec 21, 2023 at 04:21:37AM +0000, Warner Losh wrote:
> The branch main has been updated by imp:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=9e6d11ce9a51d75ed6a94e180f2fb4e9188a2ba4
> 
> commit 9e6d11ce9a51d75ed6a94e180f2fb4e9188a2ba4
> Author:     Warner Losh <imp@FreeBSD.org>
> AuthorDate: 2023-12-20 19:09:09 +0000
> Commit:     Warner Losh <imp@FreeBSD.org>
> CommitDate: 2023-12-21 04:16:45 +0000
> 
>     vtnet: Adjust rx buffer so IP header 32-bit aligned
>     
>     Call madj(m, ETHER_ALIGN) to offset rx buffers when allocating them.
>     This improves performance everywhere, and allows armv7 to work at all.
>     
>     PR:                     271288 (PR had a different fix than I wound up with)
>     MFC After:              3 days
>     Sponsored by:           Netflix
>     Differential Revision:  https://reviews.freebsd.org/D43136
> ---
>  sys/dev/virtio/network/if_vtnet.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sys/dev/virtio/network/if_vtnet.c b/sys/dev/virtio/network/if_vtnet.c
> index 287af7751066..360176e4f845 100644
> --- a/sys/dev/virtio/network/if_vtnet.c
> +++ b/sys/dev/virtio/network/if_vtnet.c
> @@ -1532,8 +1532,8 @@ vtnet_rx_alloc_buf(struct vtnet_softc *sc, int nbufs, struct mbuf **m_tailp)
>  			m_freem(m_head);
>  			return (NULL);
>  		}
> -
>  		m->m_len = size;
> +		m_adj(m, ETHER_ALIGN);

The driver is expecting to get a cluster of size sc->vtnet_rx_clustersz,
but now it's getting one of size sc->vtnet_rx_clustersz - 2.  I don't
see how this change can be sufficient on its own: what prevents virtio
from writing sc->vtnet_rx_clustersz bytes and thereby overwriting the
two bytes following the cluster?

>  		if (m_head != NULL) {
>  			m_tail->m_next = m;
>  			m_tail = m;