netdev - Re: [PATCH net-next 01/10] net: stmmac: Let descriptor code set skbuff address

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: <cb1f62fb-fde1-58f3-0a2f-e27b3923e74f@synopsys.com>
Date:   Fri, 11 May 2018 10:09:24 +0100
From:   Jose Abreu <Jose.Abreu@...opsys.com>
To:     David Miller <davem@...emloft.net>, <Jose.Abreu@...opsys.com>
CC:     <netdev@...r.kernel.org>, <Joao.Pinto@...opsys.com>,
        <Vitor.Soares@...opsys.com>, <peppe.cavallaro@...com>,
        <alexandre.torgue@...com>
Subject: Re: [PATCH net-next 01/10] net: stmmac: Let descriptor code set
 skbuff address

On 10-05-2018 20:06, David Miller wrote:
> From: Jose Abreu <Jose.Abreu@...opsys.com>
> Date: Tue,  8 May 2018 15:45:24 +0100
>
>> Stop using if conditions depending on the GMAC version for setting the
>> the descriptor skbuff address and use instead a helper implemented in
>> the descriptor files.
>>
>> Signed-off-by: Jose Abreu <joabreu@...opsys.com>
> With Spectre mitigations, indirect calls are extremely expensive.  Much
> more expensive than conditional checks.
>
> And since this is the descriptor setup in the fast paths of the driver,

Hmm, no. This is only done in HW setup. The only patch in this
series that can impact performance because of what you mentioned
is 09.

> I advise that you keep these conditionals.

I see your point but I'm trying to make the code more flexible
because we will add support for XGMAC soon. This XGMAC will need
a similar structure to what we have in GMAC.

By removing this conditionals we make stmmac_main totally
agnostic of HW and we will prevent patterns like this in the future:

if (priv->has_xgmac)
    do_stuff()
else if (priv->synopsys_id > GMAC_SOME_VERSION)
    do_other_stuff()
else
    do_other_other_stuff()

Thanks and Best Regards,
Jose Miguel Abreu