linux-kernel - Re: [EXTERNAL] Re: [PATCH net-next v3 3/3] net: ti: icssg-prueth: Add XDP support

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <fd989751-e7f6-40bb-a0bf-058c752cc7bc@ti.com>
Date: Wed, 5 Mar 2025 14:53:07 +0530
From: "Malladi, Meghana" <m-malladi@...com>
To: Dan Carpenter <dan.carpenter@...aro.org>
CC: <rogerq@...nel.org>, <danishanwar@...com>, <pabeni@...hat.com>,
        <kuba@...nel.org>, <edumazet@...gle.com>, <davem@...emloft.net>,
        <andrew+netdev@...n.ch>, <bpf@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
        <netdev@...r.kernel.org>, <u.kleine-koenig@...libre.com>,
        <matthias.schiffer@...tq-group.com>, <schnelle@...ux.ibm.com>,
        <diogo.ivo@...mens.com>, <glaroque@...libre.com>, <macro@...am.me.uk>,
        <john.fastabend@...il.com>, <hawk@...nel.org>, <daniel@...earbox.net>,
        <ast@...nel.org>, <srk@...com>, Vignesh Raghavendra <vigneshr@...com>
Subject: Re: [EXTERNAL] Re: [PATCH net-next v3 3/3] net: ti: icssg-prueth: Add
 XDP support

Hi Dan,

On 3/3/2025 7:38 PM, Dan Carpenter wrote:
> What I mean is just compile the .o file with and without the unlikely(). 
> $ md5sum drivers/net/ethernet/ti/icssg/icssg_common. o* 
> 2de875935222b9ecd8483e61848c4fc9 drivers/net/ethernet/ti/icssg/ 
> icssg_common. o. annotation 2de875935222b9ecd8483e61848c4fc9
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source 
> of this email and know the content is safe.
> Report Suspicious
> <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK! 
> uldq3TevVoc7KuXEXHnDf- 
> TXtuZ0bON9iO0jTE7PyIS1jjfs_CzpvIiMi93PVt0MVDzjHGQSK__vY_-6rO7q86rFmBMGW4SSqK5pvNE$>
> ZjQcmQRYFpfptBannerEnd
> 
> What I mean is just compile the .o file with and without the unlikely().
> 
> $ md5sum drivers/net/ethernet/ti/icssg/icssg_common.o*
> 2de875935222b9ecd8483e61848c4fc9  drivers/net/ethernet/ti/icssg/icssg_common.o.annotation
> 2de875935222b9ecd8483e61848c4fc9  drivers/net/ethernet/ti/icssg/icssg_common.o.no_anotation
> 
> Generally the rule is that you should leave likely/unlikely() annotations
> out unless it's going to make a difference on a benchmark.  I'm not going
> to jump down people's throat about this, and if you want to leave it,
> it's fine.  But it just struct me as weird so that's why I commented on
> it.
> 

I have done some performance tests to see if unlikely() is gonna make 
any impact and I see around ~9000 pps and 6Mbps drop without unlikely() 
for small packet sizes (60 Bytes)

You can see summary of the tests here:

packet size   with unlikely(pps)  without unlikely(pps)   regression

       60        462377                453251                 9126

       80        403020                399372                 3648

       96        402059                396881                 5178

      120        392725                391312                 4413

      140        327706                327099                 607

packet size  with unlikely(Mbps)  without unlikely(Mbps)  regression

      60         311                   305                    6

      80         335                   332                    3

      96         386                   381                    5

      120        456                   451                    5

      140        430                   429                    1

For more details on the logs, please 
refer:https://gist.github.com/MeghanaMalladiTI/cc6cc7709791376cb486eb1222de67be

Regards,
Meghana Malladi

> regards,
> dan carpenter
> 
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
> index 34d16e00c2ec..3db5bae44e61 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
> @@ -672,7 +672,7 @@ static int emac_run_xdp(struct prueth_emac *emac, struct xdp_buff *xdp,
>   	case XDP_TX:
>   		/* Send packet to TX ring for immediate transmission */
>   		xdpf = xdp_convert_buff_to_frame(xdp);
> -		if (unlikely(!xdpf))
> +		if (!xdpf)
>   			goto drop;
>   
>   		q_idx = smp_processor_id() % emac->tx_ch_num;
>