[dpdk-dev] [PATCH v2 1/2] ethdev: add buffered tx api

[Kulasek, TomaszX]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Tuesday, March 8, 2016 23:52
> To: Kulasek, TomaszX <tomaszx.kulasek at intel.com>
> Cc: dev at dpdk.org; Ananyev, Konstantin <konstantin.ananyev at intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 1/2] ethdev: add buffered tx api
> 
> Hi,
> 

[...]

> > +/**
> > + * Callback function for tracking unsent buffered packets.
> > + *
> > + * This function can be passed to
> > +rte_eth_tx_buffer_set_err_callback() to
> > + * adjust the default behaviour when buffered packets cannot be sent.
> > +This
> > + * function drops any unsent packets, but also updates a
> > +user-supplied counter
> > + * to track the overall number of packets dropped. The counter should
> > +be an
> > + * uint64_t variable.
> > + *
> > + * NOTE: this function should not be called directly, instead it should
> be used
> > + *       as a callback for packet buffering.
> > + *
> > + * NOTE: when configuring this function as a callback with
> > + *       rte_eth_tx_buffer_set_err_callback(), the final, userdata
> parameter
> > + *       should point to an uint64_t value.
> 
> Please forget this idea of counter in the default callback.
> 

Ok, I forgot.

> [...]
> > +void
> > +rte_eth_count_unsent_packet_callback(struct rte_mbuf **pkts, uint16_t
> unsent,
> > +		void *userdata);
> 
> What about rte_eth_tx_buffer_default_callback as name?

This function is used now as default to count silently dropped packets and update error counter in tx_buffer structure. When I remove error counter and set silent drop as default behavior, it's better to have two callbacks to choice:

1) silently dropping packets (set as default)
2) as defined above to dropping with counter.

Maybe better is to define two default callbacks while many applications can still update it's internal error counter,
So IHMO these names are more descriptive:

rte_eth_tx_buffer_drop_callback
rte_eth_tx_buffer_count_callback

What you think?

Tomasz