linux-kernel - Re: [PATCH net-next 4/5] net: ethernet: ti: am65_cpsw: move am65_cpsw_put_page() out of am65_cpsw_run

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <870ae0a7-dc04-4ae2-8485-df9db23db697@kernel.org>
Date: Mon, 24 Feb 2025 15:00:17 +0200
From: Roger Quadros <rogerq@...nel.org>
To: Simon Horman <horms@...nel.org>
Cc: Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
 Jesper Dangaard Brouer <hawk@...nel.org>,
 John Fastabend <john.fastabend@...il.com>,
 Siddharth Vadapalli <s-vadapalli@...com>,
 Md Danish Anwar <danishanwar@...com>, srk@...com, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH net-next 4/5] net: ethernet: ti: am65_cpsw: move
 am65_cpsw_put_page() out of am65_cpsw_run_xdp()

On 18/02/2025 20:03, Simon Horman wrote:
> On Mon, Feb 17, 2025 at 09:31:49AM +0200, Roger Quadros wrote:
>> This allows us to re-use am65_cpsw_run_xdp() for zero copy
>> case. Add AM65_CPSW_XDP_TX case for successful XDP_TX so we don't
>> free the page while in flight.
>>
>> Signed-off-by: Roger Quadros <rogerq@...nel.org>
> 
> Reviewed-by: Simon Horman <horms@...nel.org>
> 
>> ---
>>  drivers/net/ethernet/ti/am65-cpsw-nuss.c | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> 
> ...
> 
>> @@ -1230,9 +1230,6 @@ static int am65_cpsw_run_xdp(struct am65_cpsw_rx_flow *flow,
>>  		ndev->stats.rx_dropped++;
>>  	}
>>  
>> -	page = virt_to_head_page(xdp->data);
>> -	am65_cpsw_put_page(flow, page, true);
>> -
>>  	return ret;
> 
> It seems that before and after this patch ret is always initialised to
> AM65_CPSW_XDP_CONSUMED and never changed. So it can be removed.
> 
> Given that with this patch the function only returns after the switch
> statement, I think this would be a nice follow-up.
> 
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index 20a4fc3e579f..4052c9153632 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -1172,7 +1172,6 @@ static int am65_cpsw_run_xdp(struct am65_cpsw_rx_flow *flow,
>  {
>  	struct am65_cpsw_common *common = flow->common;
>  	struct net_device *ndev = port->ndev;
> -	int ret = AM65_CPSW_XDP_CONSUMED;
>  	struct am65_cpsw_tx_chn *tx_chn;
>  	struct netdev_queue *netif_txq;
>  	int cpu = smp_processor_id();
> @@ -1228,9 +1227,8 @@ static int am65_cpsw_run_xdp(struct am65_cpsw_rx_flow *flow,
>  		fallthrough;
>  	case XDP_DROP:
>  		ndev->stats.rx_dropped++;
> +		return AM65_CPSW_XDP_CONSUMED;
>  	}
> -
> -	return ret;
>  }
>  
>  /* RX psdata[2] word format - checksum information */
> 
> ...

Thank you for this suggestion. I will add this cleanup in my next set of patches.

-- 
cheers,
-roger