svn commit: r258328 - head/sys/net

[Robert Watson]

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)
 	{

 		...
 	}

 	...
 		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);

Robert

>
> Modified:
>  head/sys/net/if_ethersubr.c
>
> Modified: head/sys/net/if_ethersubr.c
> ==============================================================================
> --- head/sys/net/if_ethersubr.c	Mon Nov 18 22:55:50 2013	(r258327)
> +++ head/sys/net/if_ethersubr.c	Mon Nov 18 22:58:14 2013	(r258328)
> @@ -708,13 +708,25 @@ static void
> ether_input(struct ifnet *ifp, struct mbuf *m)
> {
>
> +	struct mbuf *mn;
> +
> 	/*
> -	 * We will rely on rcvif being set properly in the deferred context,
> -	 * so assert it is correct here.
> +	 * The drivers are allowed to pass in a chain of packets linked with
> +	 * m_nextpkt. We split them up into separate packets here and pass
> +	 * them up. This allows the drivers to amortize the receive lock.
> 	 */
> -	KASSERT(m->m_pkthdr.rcvif == ifp, ("%s: ifnet mismatch", __func__));
> +	while (m) {
> +		mn = m->m_nextpkt;
> +		m->m_nextpkt = NULL;
>
> -	netisr_dispatch(NETISR_ETHER, m);
> +		/*
> +		 * We will rely on rcvif being set properly in the deferred context,
> +		 * so assert it is correct here.
> +		 */
> +		KASSERT(m->m_pkthdr.rcvif == ifp, ("%s: ifnet mismatch", __func__));
> +		netisr_dispatch(NETISR_ETHER, m);
> +		m = mn;
> +	}
> }
>
> /*
>