Re: [PATCH v2 2/4] ethdev: Add in data rxtx callback support

[Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

On Fri, Feb 13, 2015 at 05:33:12PM +0100, Thomas Monjalon wrote:
> 2015-02-13 15:39, John McNamara:
> > From: Richardson, Bruce <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > 
> > Add in support for inline processing of packets inside the RX or
> > TX call. For an RX callback, what happens is that we get a set of
> > packets from the NIC and then pass them to a callback function, if
> > configured, to allow additional processing to be done on them, e.g.
> > filling in more mbuf fields, before passing back to the application.
> > On TX, the packets are similarly post-processed before being handed
> > to the NIC for transmission.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> [...]
> > @@ -2390,7 +2445,17 @@ rte_eth_rx_burst(uint8_t port_id, uint16_t queue_id,
> >  	struct rte_eth_dev *dev;
> >  
> >  	dev = &rte_eth_devices[port_id];
> > -	return (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id], rx_pkts, nb_pkts);
> > +	nb_pkts = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id], rx_pkts,
> > +			nb_pkts);
> > +	struct rte_eth_rxtx_callback *cb = dev->rx_cbs[queue_id];
> > +	if (unlikely(cb != NULL)) {
> > +		do {
> > +			nb_pkts = cb->fn(port_id, queue_id, rx_pkts, nb_pkts,
> > +					cb->param);
> > +			cb = cb->next;
> > +		} while (cb != NULL);
> > +	}
> > +	return nb_pkts;
> >  }
> >  #endif
> >  
> > @@ -2517,6 +2582,14 @@ rte_eth_tx_burst(uint8_t port_id, uint16_t queue_id,
> >  	struct rte_eth_dev *dev;
> >  
> >  	dev = &rte_eth_devices[port_id];
> > +	struct rte_eth_rxtx_callback *cb = dev->tx_cbs[queue_id];
> > +	if (unlikely(cb != NULL)) {
> > +		do {
> > +			nb_pkts = cb->fn(port_id, queue_id, tx_pkts, nb_pkts,
> > +					cb->param);
> > +			cb = cb->next;
> > +		} while (cb != NULL);
> > +	}
> >  	return (*dev->tx_pkt_burst)(dev->data->tx_queues[queue_id], tx_pkts, nb_pkts);
> >  }
> >  #endif
> 
> We all know how much the performance of these functions are important.
> So I wonder if we could reduce the impact of this change.
> I don't like the build options but maybe it should be discussed.

Performance impact is minimal, there was some discussion of it previously when
I published the earlier RFC draft. In my quick tests, with vector PMD in the 
fast path, the impact is <=1% for this change as is (i.e. no callbacks set up),
and a further 1% perf hit to actually call an empty callback.

http://article.gmane.org/gmane.comp.networking.dpdk.devel/10489
http://article.gmane.org/gmane.comp.networking.dpdk.devel/10735

Unless people start seeing a higher perf hit on some platforms, I don't think
a build-time option is worth having.

Regards,
/Bruce