netdev - Re: [PATCH 03/10] stmmac: sanitize the rx coe and add the type-1 csum

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: <4F7AF50B.3030708@st.com>
Date:	Tue, 03 Apr 2012 15:03:07 +0200
From:	Giuseppe CAVALLARO <peppe.cavallaro@...com>
To:	deepaksi <deepak.sikri@...com>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"davem@...emloft.net" <davem@...emloft.net>,
	Srinivas KANDAGATLA <srinivas.kandagatla@...com>,
	spear-devel <spear-devel@...t.st.com>,
	Shiraz HASHIM <shiraz.hashim@...com>,
	Viresh KUMAR <viresh.kumar@...com>,
	"bhutchings@...arflare.com" <bhutchings@...arflare.com>
Subject: Re: [PATCH 03/10] stmmac: sanitize the rx coe and add the type-1
 csum

On 4/3/2012 9:56 AM, deepaksi wrote:
> Ciao Peppe,
> 
> On 4/3/2012 12:19 PM, Giuseppe CAVALLARO wrote:
>>
>> [snip]
>>> reset.
>> Previously, the stmmac called the dwmac1000_rx_coe_supported to verify
>> it could do the CSUM in Hw. If true the driver used the type 2 by
>> default.
>>
>> I've voluntarily removed this function because not necessary anymore.
>> In fact, YOU improved the rx_coe from the platform. If it is passed as
>> STMMAC_RX_COE_NONE then it means the driver is not able to perform any
>> csum for the incoming frames. This is actually used on old gmac/mac
>> cores. In new cores, the HW cap register will be used to manage and fix
>> this logic.
>> I could restore the core you are mentioning  but just to do another
>> safety check at run-time in case of the user provided a broken setting
>> from the platform and there is not the HW cap register. Hmm, I do not
>> know if this actually could help indeed... just an extra check IMHO.
> 
> These are the updates required in the code.
> 
> In function dwmac1000_core_init(), you may need to set the IPC bit based
> on the fact that
> if rx_coe has been setup through the platform code.
> This bit set is a must for the checksum offload to be enabled. Rest of
> the code looks good.
> 

Great Deepak,
sorry I had not understood your good question and thx for you call.
I was wrong and the IPC Checksum Offload has to be set in the MAC
Configuration Register (*).

static int dwmac1000_rx_coe_supported(void __iomem *ioaddr)
{
    u32 value = readl(ioaddr + GMAC_CONTROL);  |
                                               |==> :-) (*)
    value |= GMAC_CONTROL_IPC;                 |
    writel(value, ioaddr + GMAC_CONTROL);      |

    NOTE ===>>> I'm going to remove the extra check below because
                useless as I had told in my previous email.
                Is it ok for you? Let me know.

    value = readl(ioaddr + GMAC_CONTROL);

    return !!(value&  GMAC_CONTROL_IPC);
}


I'll rework it and re-send all the patches with this fix soon.

ciao
Peppe

> Deepak
> 
> 
> 
> 
> 
>>> I hope this clears the things a bit. Sorry for the miscommunication.
>> No problem for the miscommunication ;-)
>>
>> Let me know
>> Ciao
>> Peppe
>>
>>> Rgds
>>> Deepak
>>>
>>>
>>>
>>>
>>>
>>>
>>>>> Regards
>>>>> Deepak
>>>>>
>>>> .
>>>>
>>>
>> .
>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html