public inbox for linux-nvme@lists.infradead.org
 help / color / mirror / Atom feed
From: Max Gurtovoy <mgurtovoy@nvidia.com>
To: Keith Busch <kbusch@kernel.org>, Christoph Hellwig <hch@lst.de>
Cc: <linux-nvme@lists.infradead.org>, <sagi@grimberg.me>,
	<chaitanyak@nvidia.com>, <oren@nvidia.com>, <benishay@nvidia.com>,
	<borisp@nvidia.com>, <aviadye@nvidia.com>, <idanb@nvidia.com>,
	<jsmart2021@gmail.com>
Subject: Re: [PATCH v1 0/4] Add command id quirk for fabrics
Date: Tue, 9 Nov 2021 18:59:05 +0200	[thread overview]
Message-ID: <a70fd129-d69d-2905-c43b-41eae0b3b8f8@nvidia.com> (raw)
In-Reply-To: <20211109161547.GC2660170@dhcp-10-100-145-180.wdc.com>


On 11/9/2021 6:15 PM, Keith Busch wrote:
> On Tue, Nov 09, 2021 at 03:31:02PM +0100, Christoph Hellwig wrote:
>> On Tue, Nov 09, 2021 at 04:23:33PM +0200, Max Gurtovoy wrote:
>>> On 11/9/2021 3:15 PM, Christoph Hellwig wrote:
>>>> Max, if you can't point us to a broken target (and yes, it is broken)
>>>> this will not go anywhere.
>>> Any target that uses Apple device as backend can be harmed.
>>>
>>> Most simple example is Linux PT target that copy the sqe as-is and passes
>>> it to the NVMe Apple drive.
>> Take another close look at how command_id are assigned my Linux driver.
>> We obviously do not pass it through as that would be completely broken.
> Also worth noting this driver has always defined the command id as a
> __u16, not __le16, yet we don't have any bug reports from big-endian
> hosts.

Right, my bad. I thought that the pass-through target uses the same id.

Linux PT target works fine.

Bad example.

Linux kernel world is covered but I still think we need to add this 
ability for fabrics controllers as we did for pci controllers.

There are a lot of vendors out there with their optimizations and 
solutions and by adding some code to cover a broken TCP target (that no 
one said what is this target and why nobody fixed it) by default that 
hurts others (even if it's spec compliant) is not a good practice.



  reply	other threads:[~2021-11-09 16:59 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-08 14:46 [PATCH v1 0/4] Add command id quirk for fabrics Max Gurtovoy
2021-11-08 14:46 ` [PATCH 1/1 nvmecli] fabrics: add new --skip-cid-gen flag to connect cmd Max Gurtovoy
2021-11-08 14:46 ` [PATCH 1/1 libnvme] fabrics: add support for new cli --skip-cid-gen flag Max Gurtovoy
2021-11-08 14:47 ` [PATCH 1/4] nvme-fabrics: add command id quirk for fabrics controllers Max Gurtovoy
2021-11-08 14:47 ` [PATCH 2/4] nvme-rdma: add command id quirk for RDMA controllers Max Gurtovoy
2021-11-08 14:47 ` [PATCH 3/4] nvme-tcp: add command id quirk for TCP controllers Max Gurtovoy
2021-11-08 14:47 ` [PATCH 4/4] nvme-fc: add command id quirk for FC controllers Max Gurtovoy
2021-11-08 16:45 ` [PATCH v1 0/4] Add command id quirk for fabrics Keith Busch
2021-11-09  8:09   ` Christoph Hellwig
2021-11-09 12:08     ` Max Gurtovoy
2021-11-09 13:15       ` Christoph Hellwig
2021-11-09 14:23         ` Max Gurtovoy
2021-11-09 14:31           ` Christoph Hellwig
2021-11-09 16:15             ` Keith Busch
2021-11-09 16:59               ` Max Gurtovoy [this message]
2021-11-09 19:04                 ` Keith Busch
2021-11-10 19:45                   ` Sagi Grimberg
2021-11-11  9:29                     ` Max Gurtovoy
2021-11-11 17:36                       ` Keith Busch
2021-11-12 16:07                       ` Sagi Grimberg
2021-11-12 21:37                         ` Keith Busch
2021-11-18 11:19                         ` Max Gurtovoy
2021-11-21 10:05                           ` Sagi Grimberg
2021-11-10 10:32       ` Daniel Wagner
2021-11-10 10:56         ` Max Gurtovoy
2021-11-10 11:18           ` Daniel Wagner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a70fd129-d69d-2905-c43b-41eae0b3b8f8@nvidia.com \
    --to=mgurtovoy@nvidia.com \
    --cc=aviadye@nvidia.com \
    --cc=benishay@nvidia.com \
    --cc=borisp@nvidia.com \
    --cc=chaitanyak@nvidia.com \
    --cc=hch@lst.de \
    --cc=idanb@nvidia.com \
    --cc=jsmart2021@gmail.com \
    --cc=kbusch@kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=oren@nvidia.com \
    --cc=sagi@grimberg.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox