> -----Original Message-----
> From: Maciej Fijalkowski <maciej.fijalkow...@intel.com>
> Sent: Thursday, February 4, 2021 10:52
> To: Camelia Alexandra Groza <camelia.gr...@nxp.com>
> Cc: k...@kernel.org; da...@davemloft.net; Madalin Bucur (OSS)
> <madalin.bu...@oss.nxp.com>; netdev@vger.kernel.org
> Subject: Re: [PATCH net 1/3] dpaa_eth: reserve space for the xdp_frame
> under the A050385 erratum
>
> On Tue, Feb 02, 2021 at 07:34:42PM +0200, Camelia Groza wrote:
> > When the erratum workaround is triggered, the newly created xdp_frame
> > structure is stored at the start of the newly allocated buffer. Avoid
> > the structure from being overwritten by explicitly reserving enough
> > space in the buffer for storing it.
> >
> > Account for the fact that the structure's size might increase in time by
> > aligning the headroom to DPAA_FD_DATA_ALIGNMENT bytes, thus
> guaranteeing
> > the data's alignment.
> >
> > Fixes: ae680bcbd06a ("dpaa_eth: implement the A050385 erratum
> workaround for XDP")
> > Signed-off-by: Camelia Groza <camelia.gr...@nxp.com>
> > ---
> > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 17
> +++++++++++++++--
> > 1 file changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > index 4360ce4d3fb6..e1d041c35ad9 100644
> > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > @@ -2182,6 +2182,7 @@ static int dpaa_a050385_wa_xdpf(struct
> dpaa_priv *priv,
> > struct xdp_frame *new_xdpf, *xdpf = *init_xdpf;
> > void *new_buff;
> > struct page *p;
> > + int headroom;
> >
> > /* Check the data alignment and make sure the headroom is large
> > * enough to store the xdpf backpointer. Use an aligned headroom
> > @@ -2197,19 +2198,31 @@ static int dpaa_a050385_wa_xdpf(struct
> dpaa_priv *priv,
> > return 0;
> > }
> >
> > + /* The new xdp_frame is stored in the new buffer. Reserve enough
> space
> > + * in the headroom for storing it along with the driver's private
> > + * info. The headroom needs to be aligned to
> DPAA_FD_DATA_ALIGNMENT to
> > + * guarantee the data's alignment in the buffer.
> > + */
> > + headroom = ALIGN(sizeof(*new_xdpf) + priv->tx_headroom,
> > + DPAA_FD_DATA_ALIGNMENT);
> > +
> > + /* Assure the extended headroom and data fit in a one-paged buffer
> */
> > + if (headroom + xdpf->len > DPAA_BP_RAW_SIZE)
>
> This check might make more sense if you would be accounting for
> skb_shared_info as well I suppose, so that you know you'll still provide
> enough tailroom for future xdp multibuf support. Didn't all the previous
> code path make sure that there's a room for that?
Hi Maciej
This function will have to go through large changes anyway once we enable
multibuf support, so making it future-proof isn't a priority.
Instead, I do want to return a valid xdp_frame so I need to guarantee the
tailroom is there, even if we don't access it . I'll send a v2. Thanks.
Camelia
