linux-kernel - Re: [PATCH V2 net 2/9] net: hns3: add sync command to sync io-pgtable

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: <79005e6e-543e-444b-9acc-f59ac7b04675@redhat.com>
Date: Thu, 24 Oct 2024 13:05:24 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: "shenjian (K)" <shenjian15@...wei.com>, Jijie Shao
 <shaojijie@...wei.com>, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, salil.mehta@...wei.com
Cc: liuyonglong@...wei.com, wangpeiyang1@...wei.com, lanhao@...wei.com,
 chenhao418@...wei.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2 net 2/9] net: hns3: add sync command to sync io-pgtable

On 10/24/24 11:38, shenjian (K) wrote:
> 
> 在 2024/10/24 16:36, Paolo Abeni 写道:
>> On 10/18/24 12:10, Jijie Shao wrote:
>>> From: Jian Shen <shenjian15@...wei.com>
>>>
>>> To avoid errors in pgtable prefectch, add a sync command to sync
>>> io-pagtable.
>>>
>>> In the case of large traffic, the TX bounce buffer may be used up.
>> It's unclear to me what do you mean for large traffic. Is that large
>> packets instead?
>>
>> Skimming over the previous patch, it looks like the for the bugger H/W
>> driver will use the bounce buffer for all packets with len < 64K. As
>> this driver does not support big tcp, such condition means all packets.
>>
>> So its not clear to me the 'may' part - it looks like the critical path
>> will always happen on the bugged H/W
> 
> Sorry for the unclear commit log.
> 
> Yes, we don't support big tcp, so <64K is worked for all packets. The 
> large traffic
> 
> here is just want to describe a case that tx bounce buffer is used up, 
> and there is
> 
> no enough space for new tx packets.
> 
> 
>>> At this point, we go to mapping/unmapping on TX path again.
>>> So we added the sync command in driver to avoid hardware issue.
>> I thought the goal of the previous patch was to avoid such sync-up.
>>
>> So I don't understand why it's there.
>>
>> A more verbose explanation will help.
> 
> This is a supplement for the previous patch. We want all the tx packet can
> 
> be handled with tx bounce buffer path. But it depends on the remain space
> 
> of the spare buffer, checked by the function hns3_can_use_tx_bounce(). In
> 
> most cases, maybe 99.99%, it returns true. But once it return false by no
> 
> available space, the packet will be handled with the former path, which
> 
> will map/unmap the skb buffer. Then we will face the smmu prefetch risk 
> again.
> 
> So I add a sync command in this case to avoid smmu prefectch,
> 
> just protects corner scenes.
> 
> 
>>> Signed-off-by: Jian Shen <shenjian15@...wei.com>
>>> Signed-off-by: Peiyang Wang <wangpeiyang1@...wei.com>
>>> Signed-off-by: Jijie Shao <shaojijie@...wei.com>
>> Also we need a fixes tag.
> 
> We considered this issue, and since this is not a software defect, we 
> were not too
> 
> sure which commit should be blamed.
> 
> It makes sense to choose the commit introducing the support for the 
> buggy H/W, we will add
> 
> it.

Please additionally rephrase the commit message including the more
verbose explanation above, thanks!

Paolo