Re: [dpdk-dev] [PATCH v2 3/4] examples: example showing use of callbacks.

[Bruce Richardson <bruce.richardson@intel.com>]

On Tue, Feb 17, 2015 at 04:32:01PM +0100, Thomas Monjalon wrote:
> 2015-02-17 12:25, Bruce Richardson:
> > On Mon, Feb 16, 2015 at 06:34:37PM +0100, Thomas Monjalon wrote:
> > > 2015-02-16 15:16, Bruce Richardson:
> > > > On Mon, Feb 16, 2015 at 03:33:40PM +0100, Olivier MATZ wrote:
> > > > > Hi John,
> > > > > 
> > > > > On 02/13/2015 04:39 PM, John McNamara wrote:
> > > > > > From: Richardson, Bruce <bruce.richardson@intel.com>
> > > > > > 
> > > > > > Example showing how callbacks can be used to insert a timestamp
> > > > > > into each packet on RX. On TX the timestamp is used to calculate
> > > > > > the packet latency through the app, in cycles.
> > > > > > 
> > > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > > 
> > > > > 
> > > > > I'm looking at the example and I don't understand what is the advantage
> > > > > of having callbacks in ethdev layer, knowing that the application can
> > > > > do the same job by a standard function call.
> > > > > 
> > > > > What is the advantage of having callbacks compared to:
> > > > > 
> > > > > 
> > > > > for (port = 0; port < nb_ports; port++) {
> > > > > 	struct rte_mbuf *bufs[BURST_SIZE];
> > > > > 	const uint16_t nb_rx = rte_eth_rx_burst(port, 0,
> > > > > 			bufs, BURST_SIZE);
> > > > > 	if (unlikely(nb_rx == 0))
> > > > > 		continue;
> > > > > 	add_timestamp(bufs, nb_rx);
> > > > > 
> > > > > 	const uint16_t nb_tx = rte_eth_tx_burst(port ^ 1, 0,
> > > > > 			bufs, nb_rx);
> > > > > 	calc_latency(bufs, nb_tx);
> > > > > 
> > > > > 	if (unlikely(nb_tx < nb_rx)) {
> > > > > 		uint16_t buf;
> > > > > 		for (buf = nb_tx; buf < nb_rx; buf++)
> > > > > 			rte_pktmbuf_free(bufs[buf]);
> > > > > 	}
> > > > > }
> > > > > 
> > > > > 
> > > > > To me, doing like the code above has several advantages:
> > > > > 
> > > > > - code is more readable: the callback is explicitly invoked, so there is
> > > > >   no risk to forget it
> > > > > - code is faster: the functions calls can be inlined by the compiler
> > > > > - easier to handle error cases in the callback function as the error
> > > > >   code is accessible to the application
> > > > > - there is no need to add code in ethdev api to do this
> > > > > - if the application does not want to use callbacks (I suppose most
> > > > >   applications), it won't have any performance impact
> > > > > 
> > > > > Regards,
> > > > > Olivier
> > > > 
> > > > In this specific instance, given that the application does little else, there
> > > > is no real advantage to using the callbacks - it's just to have a simple example
> > > > of how they can be used.
> > > > 
> > > > Where callbacks are really designed to be useful, is for extending or augmenting
> > > > hardware capabilities. Taking the example of sequence numbers - to use the most
> > > > trivial example - an application could be written to take advantage of sequence
> > > > numbers written to packets by the hardware which received them. However, if such
> > > > an application was to be used with a NIC which does not provide sequence numbering
> > > > capability, for example, anything using ixgbe driver, the application writer has
> > > > two choices - either modify his application code to check each packet for
> > > > a sequence number in the data path, and add it there post-rx, or alternatively,
> > > > to check the NIC capabilities at initialization time, and add a callback there
> > > > at initialization, if the hardware does not support it. In the latter case,
> > > > the main packet processing body of the application can be written as though
> > > > hardware always has sequence numbering capability, safe in the knowledge that
> > > > any hardware not supporting it will be back-filled by a software fallback at 
> > > > initialization-time.
> > > > 
> > > > By the same token, we could also look to extend hardware capabilities. For
> > > > different filtering or hashing capabilities, there can be limits in hardware
> > > > which are far less than what we need to use in software. Again, callbacks will
> > > > allow the data path to be written in a way that is oblivious to the underlying
> > > > hardware limits, because software will transparently fill in the gaps.
> > > > 
> > > > Hope this makes the use case clear.
> > > 
> > > After thinking more about these callbacks, I realize these callbacks won't
> > > help, as Olivier said.
> > > 
> > > With callback,
> > > 1/ application checks device capability
> > > 2/ application provides hardware emulation as DPDK callback
> > > 3/ application forgets previous steps
> > > 4/ application calls DPDK Rx
> > > 5/ DPDK calls callback (without calling optimization)
> > > 
> > > Without callback,
> > > 1/ application checks device capability
> > > 2/ application provides hardware emulation as internal function
> > > 3/ application set an internal device-flag to enable this function
> > > 4/ application calls DPDK Rx
> > > 5/ application calls the hardware emulation if flag is set
> > > 
> > > So the only difference is to keep persistent the device information in
> > > the application instead of storing it as a function pointer in the
> > > DPDK struct.
> > > You can also be faster with this approach: at initialization time,
> > > you can check that your NIC supports the feature and use a specific
> > > mainloop that adds or not the sequence number without any runtime
> > > test.
> > 
> > That is assuming that all NICs are equal on your system. It's also assuming
> > that you only have a single point in your application where you call RX or
> > TX burst. In the case where you have a couple of different NICs on the system,
> > or where you want to write an application to take advantage of capabilities of
> > different NICs, the ability to resolve all these difference at initialization
> > time is useful. The main packet handling code can be written with just the
> > processing of packets in mind, rather than having to have a set of branches
> > after each RX burst call, or before each TX burst call, to "smooth out" the
> > different NIC capabilities. 
> > 
> > As for the option of maintaining different main loops for different NICs with
> > different capabilities - that sounds like a maintenance nightmare to
> > me, due to duplicated code! Callbacks is a far cleaner solution than that IMHO.
> 
> If you really prefer using callbacks intead of direct calls, why not implementing
> the callbacks hooks in your application by wrapping Rx and Tx burst functions?
>

Because sometimes things are generally useful and are better supplied in a
standard library than forcing multiple applications to constantly re-invent the
wheel. 

Furthermore, if we enable the hooks in DPDK, it gives us a standard API
prototype to code against, allowing us to provide reference implementation 
callbacks to create smarter ethdevs that can be used as higher-level abstractions
inside applications.

We don't require applications to know what the underlying NIC driver is that
is being used to receive pkts - we take care of all that at initialization time
by using a function pointer to allow NIC specific calls to be referenced using the
rx_burst API. An application could be written to call directly into the driver
receive or transmit functions - and such an API could be made faster than the
existing indirect calls - but instead we set things up so that all NICs look
the same to the data-path, irrespective of type or speed. In the same way, this
feature allows us to set things up at initialization time so that all NICs look
the same to the datapath in terms of capabilities offered. We won't always do
so, but it is a worthwhile use case that brings the same benefits as the generic
RX and TX function pointers do - a datapath that is agnostic to underlying
hardware.

Going further, once NICs can be made to provide similar capabilities in terms
of offloads - at the ethdev layer - then additional libraries which use ethdevs,
such as link bonding, as Declan has highlighted, can make use of that very easily.
Having support for that in the application won't allow such use cases in libraries,
and having it in the ethdev layer allows it to be conveniently used by any other
libraries other than link bonding that may want this in future.

Can I actually also flip the discussion on it's head a bit? We have presented
a number of use cases where we see this functionality being useful, and we
have plans to build upon this in future to enable smarter ethdevs. Given that
this is not a large amount of code by any means, what is the compelling reason
why it should not be merged in, if it would be useful to at least some users of
DPDK?

/Bruce