On 3/3/2025 6:01 PM, Dan Carpenter wrote:
On Mon, Mar 03, 2025 at 05: 36: 41PM +0530, Malladi, Meghana wrote: > >
> +static int emac_run_xdp(struct prueth_emac *emac, struct xdp_buff
*xdp, > > > + struct page *page) > > > +{ > > > + struct net_device
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!
uldqV3eFFkc7oMXFHHkDX4AFLVsE3ldskf6bPMMFmxDOsNtMfZjUscGelUkBFpAeybNre38L_c2LiiUb7AZxLvAeqSk9ifgbPE1AYFU$>
ZjQcmQRYFpfptBannerEnd
On Mon, Mar 03, 2025 at 05:36:41PM +0530, Malladi, Meghana wrote:
> > +static int emac_run_xdp(struct prueth_emac *emac, struct xdp_buff *xdp,
> > + struct page *page)
> > +{
> > + struct net_device *ndev = emac->ndev;
> > + int err, result = ICSSG_XDP_PASS;
> > + struct bpf_prog *xdp_prog;
> > + struct xdp_frame *xdpf;
> > + int q_idx;
> > + u32 act;
> > +
> > + xdp_prog = READ_ONCE(emac->xdp_prog);
> > + act = bpf_prog_run_xdp(xdp_prog, xdp);
> > + switch (act) {
> > + case XDP_PASS:
> > + break;
> > + case XDP_TX:
> > + /* Send packet to TX ring for immediate transmission */
> > + xdpf = xdp_convert_buff_to_frame(xdp);
> > + if (unlikely(!xdpf))
>
> This is the second unlikely() macro which is added in this patchset.
> The rule with likely/unlikely() is that it should only be added if it
> likely makes a difference in benchmarking. Quite often the compiler
> is able to predict that valid pointers are more likely than NULL
> pointers so often these types of annotations don't make any difference
> at all to the compiled code. But it depends on the compiler and the -O2
> options.
>
Do correct me if I am wrong, but from my understanding, XDP feature depends
alot of performance and benchmarking and having unlikely does make a
difference. Atleast in all the other drivers I see this being used for XDP.
Which compiler are you on when you say that "having unlikely does make a
difference"?
I'm on gcc version 10.3.1.
I'm on gcc version 14.2.0 (Debian 14.2.0-16) and it doesn't make a
difference to the compiled code. This matches what one would expect from
a compiler. Valid pointers are fast path and NULL pointers are slow path.
Can you tell me how did you verify this? I actually don't know what
level of optimization to expect from a compiler. I said so, because I
have checked with other drivers which implemented XDP and everywhere
unlikely is used. But now I understand its not the driver but the
compiler that plays the major role in defining the optimization.
Adding an unlikely() is a micro optimization. There are so many other
things you can do to speed up the code. I wouldn't start with that.
Ok, if you believe that unlikely is doing more harm than good, I am ok
with dropping them off.
regards,
dan
