Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
To:     Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
Cc:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
        Christoph Hellwig <hch@lst.de>,
        Sagi Grimberg <sagi@grimberg.me>,
        Keith Busch <kbusch@kernel.org>, Jens Axboe <axboe@fb.com>,
        Max Gurtovoy <maxg@mellanox.com>,
        Stephen Bates <sbates@raithlin.com>
References: <20200514172253.6445-1-logang@deltatee.com>
 <BYAPR04MB4965CEDB8E3AE4B7F4C527AF86800@BYAPR04MB4965.namprd04.prod.outlook.com>
From:   Logan Gunthorpe <logang@deltatee.com>
Message-ID: <973eb071-45ad-26f1-17d0-325d637a9750@deltatee.com>
Date:   Thu, 11 Jun 2020 13:19:21 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.8.0
MIME-Version: 1.0
In-Reply-To: <BYAPR04MB4965CEDB8E3AE4B7F4C527AF86800@BYAPR04MB4965.namprd04.prod.outlook.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH v13 0/9] nvmet: add target passthru commands support
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 2020-06-11 1:16 p.m., Chaitanya Kulkarni wrote:
> On 5/14/20 10:23 AM, Logan Gunthorpe wrote:
>> This is v13 of the passthru patchset which is mostly a resend of v12
>> with Sagi's reviewed-by tags collected.
>>
>> Below, I'll reiterrate some points I made previously that haven't been
>> responded to:
>>
>> I don't think cloning the ctrl_id or the subsysnqn is a good idea.
>> I sent an email trying to explain why here[1] but there was no response.
>> In short, I think cloning the ctrl_id will break multipathing over
>> fabrics and copying the subsysnqn only has the effect of breaking
>> loopback; the user can always copy the underlying subsysnqn if it
>> makes sense for their overall system.
>>
>> I maintain overriding the CMIC bit in the ctrl id is necessary to
>> allow multipath over fabrics even if the underlying device did
>> not support multipath.
>>
>> I also think the black list for admin commands is appropriate, and I
>> added it based on Sagi's feedback[2]. There are plenty of commands that
>> may be dangerous like firmware update and format NVM commands, and NS
>> attach commands won't work out of the box because we don't copy the
>> ctrl_id. It seems like there's more commands to be careful of than ones
>> that are that are obviously acceptable. So, I think the prudent course
>> is blacklisting by default until someone has a usecase and can show
>> the command is safe seems and makes sense. For our present use cases,
>> the identify, log page and vendor specific commands are all that we
>> care about.
>>
>> A git branch is available here and is based on v5.7-rc5:
>>
>> https://github.com/sbates130272/linux-p2pmem  nvmet_passthru_v13
>>
>> [1]https://lore.kernel.org/linux-block/247eca47-c3bc-6452-fb19-f7aa27b05a60@deltatee.com/
>> [2]https://lore.kernel.org/linux-block/e4430207-7def-8776-0289-0d58689dc0cd@grimberg.me/
>>
>> --
>>
>> v13 Changes:
>>    1. Rebased onto v5.7-rc5
>>    2. Collected Sagi's Reviewed-by tags
> 
> Are you planning to send V14 based on nvme-5.9 branch ?
> 

I don't have any plans to yet, but I certainly can. I usually wait until
after the merge window to resend. I can certainly look at rebasing onto
nvme-5.9 for next time.

Logan