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?
> + return -ENOMEM;
> +
> p = dev_alloc_pages(0);
> if (unlikely(!p))
> return -ENOMEM;
>
> /* Copy the data to the new buffer at a properly aligned offset */
> new_buff = page_address(p);
> - memcpy(new_buff + priv->tx_headroom, xdpf->data, xdpf->len);
> + memcpy(new_buff + headroom, xdpf->data, xdpf->len);
>
> /* Create an XDP frame around the new buffer in a similar fashion
> * to xdp_convert_buff_to_frame.
> */
> new_xdpf = new_buff;
> - new_xdpf->data = new_buff + priv->tx_headroom;
> + new_xdpf->data = new_buff + headroom;
> new_xdpf->len = xdpf->len;
> new_xdpf->headroom = priv->tx_headroom;
> new_xdpf->frame_sz = DPAA_BP_RAW_SIZE;
> --
> 2.17.1
>
