Subject: RE: [PATCH v1 net-next 3/6] net: ena: Add validation for completion descriptors consistency

From: "Arinzon, David" <[email-masked]>
Date: Wed, 8 May 2024 06:01:33 +0000

> > +             if (unlikely((status & ENA_ETH_IO_RX_CDESC_BASE_FIRST_MASK)
> >>
> > +                 ENA_ETH_IO_RX_CDESC_BASE_FIRST_SHIFT && count != 0)) {
> > +                     struct ena_com_dev *dev =
> > + ena_com_io_cq_to_ena_dev(io_cq);
> > +
> > +                     netdev_err(dev->net_device,
> > +                                "First bit is on in descriptor #%d on q_id: %d, req_id:
> %u\n",
> > +                                count, io_cq->qid, cdesc->req_id);
> > +                     return -EFAULT;
> 
> This is really asking to be a devlink health reporter.
> You can dump the information to the user and get the event counter for
> free.
> 
> But if you don't want to use that - please at least rate limit the message.

Hi Jakub,

Thank you for reviewing the patch.

We have an action item to review the devlink health mechanism to see how it can benefit
our solution. Thank you for raising this point.

Regarding this print, I don't expect it to be printed more than once,
as a failure here returns -EFAULT which triggers an error flow and an ENA reset sequence,
which will also stop the TX and RX queues as part of it, and no further descriptors will
be processed until the reset sequence is complete.

Please let us know if you still have concerns regarding this print.