Re: [PATCH v2 3/3] xen/netback: use same error messages for same errors

[Juergen Gross <jgross@xxxxxxxx>]

On 28.03.23 15:32, Jan Beulich wrote:
> On 28.03.2023 15:12, Juergen Gross wrote:
> > Issue the same error message in case an illegal page boundary crossing
> > has been detected in both cases where this is tested.
> >
> > Suggested-by: Jan Beulich <jbeulich@xxxxxxxx>
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > ---
> > V2:
> > - new patch
> > ---
> >   drivers/net/xen-netback/netback.c | 6 ++----
> >   1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/xen-netback/netback.c 
> > b/drivers/net/xen-netback/netback.c
> > index 9ca4b69d3b39..5dfdec44354a 100644
> > --- a/drivers/net/xen-netback/netback.c
> > +++ b/drivers/net/xen-netback/netback.c
> > @@ -996,10 +996,8 @@ static void xenvif_tx_build_gops(struct xenvif_queue 
> > *queue,
> >   
> >   		/* No crossing a page as the payload mustn't fragment. */
> >                 if (unlikely((txreq.offset + txreq.size) > XEN_PAGE_SIZE)) {
> > -                       netdev_err(queue->vif->dev,
> > -                                  "txreq.offset: %u, size: %u, end: %lu\n",
> > -                                  txreq.offset, txreq.size,
> > -                                  (unsigned long)(txreq.offset&~XEN_PAGE_MASK) 
> > + txreq.size);
> > +                       netdev_err(queue->vif->dev, "Cross page boundary, 
> > txp->offset: %u, size: %u\n",
> > +                                  txreq.offset, txreq.size);
> >                         xenvif_fatal_tx_err(queue->vif);
> >                         break;
> >                 }
> To be honest I'm of the opinion that this goes slightly too far:
> Making the two messages more similar is certainly helpful. But in
> case of problems I think it wouldn't hurt if they're still
> distinguishable - when the one here triggers it may e.g also mean
> that the calculation of the residual size is causing an issue. So
> maybe stick to txreq.offset in the message text, with everything
> else left as you have it?
Fine with me.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature