Re: [net-next,v27,5/8] eea: implement packet receive logic

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
eea: implement packet receive logic

Implement the core logic for receiving packets in the EEA RX path,
including packet buffering and basic validation.

> diff --git a/drivers/net/ethernet/alibaba/eea/eea_net.h b/drivers/net/ethernet/alibaba/eea/eea_net.h
> index 0398e781dfdb..5a607947a76e 100644
> --- a/drivers/net/ethernet/alibaba/eea/eea_net.h
> +++ b/drivers/net/ethernet/alibaba/eea/eea_net.h

[ ... ]

> diff --git a/drivers/net/ethernet/alibaba/eea/eea_rx.c b/drivers/net/ethernet/alibaba/eea/eea_rx.c
> index a8695704d67d..db74580f296c 100644
> --- a/drivers/net/ethernet/alibaba/eea/eea_rx.c
> +++ b/drivers/net/ethernet/alibaba/eea/eea_rx.c

[ ... ]

> @@ -88,12 +180,391 @@ static int eea_alloc_rx_hdr(struct eea_net_init_ctx *ctx, struct eea_net_rx *rx)
>  	return 0;
>  }
>
> +static void eea_rx_meta_dma_sync_for_cpu(struct eea_net_rx *rx,
> +					 struct eea_rx_meta *meta, u32 len)
> +{
> +	dma_sync_single_for_cpu(rx->enet->edev->dma_dev,
> +				meta->dma + meta->offset + meta->headroom,
> +				len, DMA_FROM_DEVICE);
> +}
> +
> +static int eea_harden_check_overflow(struct eea_rx_ctx *ctx,
> +				     struct eea_net *enet)
> +{
> +	u32 max_len;
> +
> +	max_len = ctx->meta->truesize - ctx->meta->headroom -
> +		ctx->meta->tailroom;
> +
> +	if (unlikely(ctx->len > max_len)) {
> +		pr_debug("%s: rx error: len %u exceeds truesize %u\n",
> +			 enet->netdev->name, ctx->len, max_len);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int eea_harden_check_size(struct eea_rx_ctx *ctx, struct eea_net *enet)
> +{
> +	int err;
> +
> +	err = eea_harden_check_overflow(ctx, enet);
> +	if (err)
> +		return err;
> +
> +	if (ctx->hdr_len) {
> +		if (unlikely(ctx->hdr_len < ETH_HLEN)) {
> +			pr_debug("%s: short packet %u\n", enet->netdev->name,
> +				 ctx->len);
> +			return -EINVAL;
> +		}
> +
> +		if (unlikely(ctx->hdr_len > enet->cfg.split_hdr)) {
> +			pr_debug("%s: rx error: hdr len %u exceeds hdr buffer size %u\n",
> +				 enet->netdev->name, ctx->len,
> +				 enet->cfg.split_hdr);

Should the second pr_debug argument be ctx->hdr_len instead of ctx->len?

The condition checks 'ctx->hdr_len > enet->cfg.split_hdr', and the format
string says 'hdr len %u exceeds hdr buffer size %u', but the argument
passed is ctx->len (the data payload length) rather than ctx->hdr_len (the
header length that actually triggered the error). This would print the
wrong value when debugging split-header overflow issues.

> +			return -EINVAL;
> +		}
> +
> +		return 0;
> +	}
> +
> +	if (unlikely(ctx->len < ETH_HLEN)) {
> +		pr_debug("%s: short packet %u\n", enet->netdev->name, ctx->len);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}

[ ... ]