linux-kernel - Re: [PATCH 06/16] ntb: add check and comment for link up to mw_count and mw_get

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: <470752c9-53f5-aa21-96c5-3fdc9b3d9e7c@deltatee.com>
Date:   Thu, 29 Jun 2017 16:14:51 -0600
From:   Logan Gunthorpe <logang@...tatee.com>
To:     Allen Hubbe <Allen.Hubbe@...l.com>, linux-ntb@...glegroups.com,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     'Jon Mason' <jdmason@...zu.us>,
        'Dave Jiang' <dave.jiang@...el.com>,
        'Bjorn Helgaas' <bhelgaas@...gle.com>,
        'Greg Kroah-Hartman' <gregkh@...uxfoundation.org>,
        'Kurt Schwemmer' <kurt.schwemmer@...rosemi.com>,
        'Stephen Bates' <sbates@...thlin.com>,
        'Serge Semin' <fancer.lancer@...il.com>
Subject: Re: [PATCH 06/16] ntb: add check and comment for link up to mw_count
 and mw_get_align

On 29/06/17 03:35 PM, Allen Hubbe wrote:
>> It is not a workaround for alignment restrictions of the mws.  It is a restriction to avoid the use of
>> doorbells and scratchpads.  Memory windows are used exclusively.
>>
>> Read msi-x local <addr,data> and send that to the peer:
> 
> https://github.com/ntrdma/ntrdma/blob/master/drivers/ntc/ntc_ntb_msi.c#L583
> 
>> Transform peer's addr to the memory window region:
> 
> https://github.com/ntrdma/ntrdma/blob/master/drivers/ntc/ntc_ntb_msi.c#L603
> 
>> Append a dma immediate value operation after other operations, to write the data at addr:
> 
> https://github.com/ntrdma/ntrdma/blob/master/drivers/ntc/ntc_ntb_msi.c#L1195

Ok, I'm a little bit confused at the links you are pointing me to. The
patch you nak'd only makes changes to ntb_mw_get_align and ntb_mw_count:

1) ntb_mw_get_align gets the last two parameters of what was formerly
ntb_mw_get_range. However, the only two uses of get_range I can find in
the ntrdma code set those two parameters to NULL and thus would not use
ntb_mw_get_align.

2) ntb_mw_count is technically a new function (ntb_peer_mw_count is
equivalent to the old function of that name). However, the one place in
ntrdma where it would be called is during de-initialization. So if the
link goes down before deinit, the client will not correctly clean up all
the memory windows.

So, it seems to me that ntb_mw_get_align is fine the way I have it but
we should probably change ntb_mw_count: I'll get rid of the check for
link in that function and then have the switchtec driver locally cache
the peer's memory windows whenever the link goes up. So if the link is
down it will return the number of mws the last time the link was seen.
(And if the link was never seen it will return 0).

Thoughts?

Logan