netdev - Re: [PATCH net-next 6/6] net: macb: Add MACB_CAPS_QBV capability flag for IEEE 802.1Qbv 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] [day] [month] [year] [list]

Message-ID: <53ee7054-6cdb-4f2e-a4b5-16bfa590df44@amd.com>
Date: Tue, 29 Jul 2025 15:12:29 +0530
From: "Karumanchi, Vineeth" <vineeth@....com>
To: horms@...nel.org, vineeth.karumanchi@....com
Cc: nicolas.ferre@...rochip.com, claudiu.beznea@...on.dev,
 andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, git@....com, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 6/6] net: macb: Add MACB_CAPS_QBV capability flag
 for IEEE 802.1Qbv support

Hi Simon,

On 7/24/2025 12:35 AM, Simon Horman wrote:

<...>

>> @@ -5345,7 +5349,7 @@ static const struct macb_config sama7g5_emac_config = {
>>   static const struct macb_config versal_config = {
>>   	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
>>   		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH | MACB_CAPS_NEED_TSUCLK |
>> -		MACB_CAPS_QUEUE_DISABLE,
>> +		MACB_CAPS_QUEUE_DISABLE, MACB_CAPS_QBV,
> Hi Vineeth,
> 
> TL;DR: I think you mean
> 
> 		MACB_CAPS_QUEUE_DISABLE | MACB_CAPS_QBV,
> 		                       ^^^
> 

Yes, since there's no strict validation for the presence of 
NETIF_F_HW_TC, the tc add/replace command succeeded. I've submitted an 
RFC patch to enforce stricter checks for NETIF_F_HW_TC within 
dev->hw_features

> I assume that the intention here is to set the MACB_CAPS_QBV bit of .caps.
> However, because there is a comma rather than a pipe between
> it and MACB_CAPS_QUEUE_DISABLE the effect is to leave .caps as
> it was before, and set .dma_burst_length to MACB_CAPS_QBV.
> .dma_burst_length is then overwritten on the following line.
> 
> Flagged by W=1 builds with Clang 20.1.8 and 15.1.0.
> 
> Please build your patches with W=1 and try to avoid adding warnings
> it flags.
> 
> Also, while we are here, it would be nice to fix up the line wrapping so
> the adjacent code is 80 columns wide or less, as is still preferred in
> Networking code.
> 
> 	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
> 		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH |
> 		MACB_CAPS_NEED_TSUCLK | MACB_CAPS_QUEUE_DISABLE |
> 		MACB_CAPS_QBV,
> 

OK.

Thanks,
-- 
🙏 vineeth