From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: let tun/tap receive large frames
To: David Gwynne <david@gwynne.id.au>
Cc: tech@openbsd.org
Date: Sun, 17 Nov 2024 14:20:57 +0100

On Thu, Nov 14, 2024 at 02:13:44PM +1000, David Gwynne wrote:
> ie, let writes from userland go big.
> 
> there's no reason we shouldn't be able to take a large packet from
> userland. however, because there's "link" headers on packets, we may
> need to chain mbufs now.
> 
> it also reorganises the mbuf setup so tun_hdr (if present) isn't
> included in the size.
> 
> ok?

Not sure about this limit here: uio->uio_resid > (hlen + MAXMCLBYTES)
It feels like MAXMCLBYTES is not really the right define but I see nothing
better.

OK claudio@
 
> Index: if_tun.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_tun.c,v
> diff -u -p -r1.245 if_tun.c
> --- if_tun.c	14 Nov 2024 01:51:57 -0000	1.245
> +++ if_tun.c	14 Nov 2024 04:09:56 -0000
> @@ -909,9 +909,9 @@ tun_dev_write(dev_t dev, struct uio *uio
>  {
>  	struct tun_softc	*sc;
>  	struct ifnet		*ifp;
> -	struct mbuf		*m0;
> +	struct mbuf		*m0, *m, *n;
>  	int			error = 0;
> -	size_t			mlen;
> +	size_t			len, alen, mlen;
>  	size_t			hlen;
>  	struct tun_hdr		th;
>  
> @@ -925,30 +925,16 @@ tun_dev_write(dev_t dev, struct uio *uio
>  	if (ISSET(sc->sc_flags, TUN_HDR))
>  		hlen += sizeof(th);
>  	if (uio->uio_resid < hlen ||
> -	    uio->uio_resid > (hlen + ifp->if_hardmtu)) {
> +	    uio->uio_resid > (hlen + MAXMCLBYTES)) {
>  		error = EMSGSIZE;
>  		goto put;
>  	}
>  
> -	align += max_linkhdr;
> -	mlen = align + uio->uio_resid;
> -
>  	m0 = m_gethdr(M_DONTWAIT, MT_DATA);
>  	if (m0 == NULL) {
>  		error = ENOMEM;
>  		goto put;
>  	}
> -	if (mlen > MHLEN) {
> -		m_clget(m0, M_DONTWAIT, mlen);
> -		if (!ISSET(m0->m_flags, M_EXT)) {
> -			error = ENOMEM;
> -			goto drop;
> -		}
> -	}
> -
> -	m_align(m0, mlen);
> -	m0->m_pkthdr.len = m0->m_len = mlen;
> -	m_adj(m0, align);
>  
>  	if (ISSET(sc->sc_flags, TUN_HDR)) {
>  		error = uiomove(&th, sizeof(th), uio);
> @@ -996,9 +982,52 @@ tun_dev_write(dev_t dev, struct uio *uio
>  		}
>  	}
>  
> -	error = uiomove(mtod(m0, void *), m0->m_len, uio);
> -	if (error != 0)
> -		goto drop;
> +	align += roundup(max_linkhdr, sizeof(long));
> +	mlen = MHLEN; /* how much space in the mbuf */
> +
> +	len = uio->uio_resid;
> +	m0->m_pkthdr.len = len;
> +
> +	m = m0;
> +	for (;;) {
> +		alen = align + len; /* what we want to put in this mbuf */
> +		if (alen > mlen) {
> +			if (alen > MAXMCLBYTES)
> +				alen = MAXMCLBYTES;
> +			m_clget(m, M_DONTWAIT, alen);
> +			if (!ISSET(m->m_flags, M_EXT)) {
> +				error = ENOMEM;
> +				goto put;
> +			}
> +		}
> +
> +		m->m_len = alen;
> +		if (align > 0) {
> +			/* avoid m_adj to protect m0->m_pkthdr.len */
> +			m->m_data += align;
> +			m->m_len -= align;
> +		}
> +
> +		error = uiomove(mtod(m, void *), m->m_len, uio);
> +		if (error != 0)
> +			goto drop;
> +
> +		len = uio->uio_resid;
> +		if (len == 0)
> +			break;
> +
> +		n = m_get(M_DONTWAIT, MT_DATA);
> +		if (n == NULL) {
> +			error = ENOMEM;
> +			goto put;
> +		}
> +
> +		align = 0;
> +		mlen = MLEN;
> +
> +		m->m_next = n;
> +		m = n;
> +	}
>  
>  	NET_LOCK();
>  	if_vinput(ifp, m0);
> 

-- 
:wq Claudio