On Tue, Oct 26, 2021 at 5:57 PM Thomas Monjalon <tho...@monjalon.net> wrote:
>
> 26/10/2021 16:58, David Marchand:
> > Warning continuously is a pain when developping or if a unit test
> > is/gets broken.
> >
> > It could also be a problem if application behaves badly only in some
> > corner cases and a DoS results of those logs being continuously displayed.
> >
> > Let's warn once per port and per rx/tx.
> >
> > Getting such a log is scary, but let's make it more eye catching by
> > dumping a backtrace with it.
> [...]
> > Fixes: c87d435a4d79 ("ethdev: copy fast-path API into separate structure")
> >
> > Signed-off-by: David Marchand <david.march...@redhat.com>
> [...]
> > +static struct dummy_queue
> > *dummy_queues_ref[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
> > +static struct dummy_queue dummy_queues[RTE_MAX_ETHPORTS];
>
> I feel we could better name those arrays, maybe adding a comment.
> First one is really queues array while the second one is to share
> the same value with all queues of a port. Right?
Yes, look fwd to v2 for better names.
>
> > +RTE_INIT(dummy_queue_init)
> > +{
> > + uint16_t port_id;
> > +
> > + for (port_id = 0; port_id < RTE_DIM(dummy_queues); port_id++) {
> > + unsigned int i;
>
> q would be a better name than i
Ok, and I'll rename other variable q for actual queue objects later in
the patch.
> > eth_dev_fp_ops_reset(struct rte_eth_fp_ops *fpo)
> > {
> > static void *dummy_data[RTE_MAX_QUEUES_PER_PORT];
> > - static const struct rte_eth_fp_ops dummy_ops = {
> > + uint16_t port_id = fpo - rte_eth_fp_ops;
> > +
> > + dummy_queues[port_id].rx_warn_once = false;
> > + dummy_queues[port_id].tx_warn_once = false;
> > + *fpo = (struct rte_eth_fp_ops) {
> > .rx_pkt_burst = dummy_eth_rx_burst,
> > .tx_pkt_burst = dummy_eth_tx_burst,
> > - .rxq = {.data = dummy_data, .clbk = dummy_data,},
> > - .txq = {.data = dummy_data, .clbk = dummy_data,},
> > + .rxq = (struct rte_ethdev_qdata) {
>
> Why this cast? rte_eth_fp_ops.rxq is of type rte_ethdev_qdata.
Funny how the compiler complains about:
../lib/ethdev/ethdev_private.c: In function ‘eth_dev_fp_ops_reset’:
../lib/ethdev/ethdev_private.c:243:9: error: expected expression
before ‘{’ token
*fpo = {
^
if we don't explicitely tell this anonymous struct is of type struct
rte_eth_fp_ops (note that *fpo is of type struct rte_eth_fp_ops).
But otoh, compiler silently understands that, in .rxq case, the
anonymous struct is of type rte_ethdev_qdata.
So indeed, it works without the cast on .rxq and .txq.
I applied the cast on all anonymous struct in my patch once I hit the
first compiler complaint.
Do you have the explanation or can you point me at some standard
explaining the difference in treatment?
>
> > + .data = (void **)&dummy_queues_ref[port_id],
> > + .clbk = dummy_data,
> > + },
> > + .txq = (struct rte_ethdev_qdata) {
> > + .data = (void **)&dummy_queues_ref[port_id],
> > + .clbk = dummy_data,
> > + },
> > };
> > -
> > - *fpo = dummy_ops;
> > }
--
David Marchand
