Date: Thu, 24 Jul 1997 21:24:54 -0700 From: David Greenman <dg@root.com> To: itojun@itojun.org Cc: hackers@FreeBSD.ORG Subject: Re: odd xxget() in some network drivers Message-ID: <199707250424.VAA16538@implode.root.com> In-Reply-To: Your message of "Fri, 25 Jul 1997 12:11:17 %2B0900." <16903.869800277@itojun.csl.sony.co.jp>
next in thread | previous in thread | raw e-mail | index | archive | help
> If my memory is correct, it is assumed that network packets received
> by NICs should be placed into either:
> - single internal mbuf
> - two internal mbufs
> - multiple external mbufs
> (for ground information, see m_devget() in /sys/kern/uipc_mbuf.c
> or "TCP/IP illustrated vol.2" p42)
># second case (two internal mbufs) can be omitted for simplicity
># in current memory-rich environment, IMHO.
I assume that by "external" mbuf you really mean an mbuf cluster.
"external" mbufs usually implies non-cluster memory is attached. Those
network interfaces in FreeBSD which do DMA normally do it into a single
mbuf cluster (attached to an mbuf header).
> However, there seems to be several drivers that does not meet this
> practice. For example, /sys/dev/vx/if_vx.c will return the packet
> received in one of the following condition:
> - single internal mbuf (only if len % 4 == 0)
> - two internal mbufs (if len % 4 != 0)
> mbuf 1: m_len = len - (len % 4)
> mbuf 2: m_len = len % 4
> - three internal mbufs
> mbuf 1: m_len = MLEN
> mbuf 2: m_len = len - mlen - (len % 4)
> mbuf 3: m_len = len % 4
> - one external mbuf (only if len % 4 == 0)
> - two external mbufs
> mbuf 1: m_len = len - (len % 4)
> mbuf 2: m_len = len % 4
> - more than two external mbuf
> mbuf 1: m_len = MCLBYTES
> ...
> mbuf n: m_len = len % 4
> I sent PR kernel/4020 but nobody seem to reply to this one.
I don't see how the last case can ever happen since an mbuf cluster is
2K bytes - larger than the MTU/MRU.
> Questions:
> - could I apply the patch in kernel/4020 to the current source?
> (who is the boss in /sys/dev/vx?)
The new logic in that PR is:
! if (MHLEN < len) {
! /* assumes ETHERMTU < MCLBYTES */
! MCLGET(m, M_DONTWAIT);
! if (! (m->m_flags & M_EXT)) {
! m_freem(m);
! goto fail;
! }
! }
The only problem with the above is that the first if () expression is wrong.
The test should be if (len > MHLEN). Not only is this correct for style, but
it is logically different: MHLEN < len will allocate an mbuf cluster when
len == MHLEN, and this is not desired.
> - could I make some check for other drivers, and apply patches if
> necessery?
I would prefer that you had your proposed changes reviewed first.
-DG
David Greenman
Core-team/Principal Architect, The FreeBSD Project
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199707250424.VAA16538>