svn commit: r258328 - head/sys/net

[John-Mark Gurney]

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."