Date: Tue, 19 Nov 2013 10:04:26 -0800 From: John-Mark Gurney <jmg@funkthat.com> To: Robert Watson <rwatson@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, "George V. Neville-Neil" <gnn@FreeBSD.org>, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r258328 - head/sys/net Message-ID: <20131119180426.GC2279@funkthat.com> In-Reply-To: <alpine.BSF.2.00.1311191101060.50802@fledge.watson.org> References: <201311182258.rAIMwEFd048783@svn.freebsd.org> <alpine.BSF.2.00.1311191101060.50802@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Robert Watson wrote this message on Tue, Nov 19, 2013 at 11:04 +0000:
> On Mon, 18 Nov 2013, George V. Neville-Neil wrote:
>
> > Allow ethernet drivers to pass in packets connected via the nextpkt
> > pointer.
> > Handling packets in this way allows drivers to amortize work during
> > packet reception.
> >
> > Submitted by: Vijay Singh
> > Sponsored by: NetApp
>
> Currently, it is quite easy to make mistakes regarding individual mbuf
> chains vs. lists of mbuf chains. This leads me to wonder whether a new
> type, perhaps simply constructed on the stack before passing in, should be
> used for KPIs that accept lists of packets. E.g.,
>
> /*
> * This structure is almost always allocated on a caller stack, so
> * cannot itself be queued without memory allocation in most cases.
> */
> struct mbuf_queue {
> struct mbuf *mq_head;
> };
>
> int
> ether_input(struct ifnet *ifp, struct mbuf_queue *m)
Why not pass in the structure (not a pointer to the struct) if the
struct really is the above? It would even be able to save the stack
allocation (not that it's that expensive)...
so instead:
int
ether_input(struct ifnet *ifp, struct mbuf_queue m)
You can also create the struct via a macro like:
#define MAKE_MQ(mbuf) ((struct mbuf_queue){ (mbuf) })
so below would become:
return (ether_input(ifp, MAKE_MQ(m)));
Just a thought... But I do like using the compiler for this... The
above makes the compiler do the work, and it be transparent from the
code side...
> {
>
> ...
> }
>
> ...
> struct mbuf_queue mq = { m };
>
> return (ether_input(ifp, &mq));
> ...
>
> That way the compiler can help us figure out where we expect an individual
> packet but have accidentally leaked a queue. Functions that accept only a
> single packet could also more agressively assert that m->m_nextpkt is NULL:
>
> M_ASSERT_ONEPACKET(m);
--
John-Mark Gurney Voice: +1 415 225 5579
"All that I will do, has been done, All that I have, has not."
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20131119180426.GC2279>