All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sagi Grimberg <sagig@dev.mellanox.co.il>
To: Chuck Lever <chuck.lever@oracle.com>
Cc: linux-rdma <linux-rdma@vger.kernel.org>,
	Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v1 05/10] svcrdma: Find rmsgp more reliably
Date: Tue, 13 Jan 2015 12:07:07 +0200	[thread overview]
Message-ID: <54B4EE4B.9050500@dev.mellanox.co.il> (raw)
In-Reply-To: <3C09A798-2BA9-46A1-AA60-122C2274974C@oracle.com>

On 1/12/2015 2:30 AM, Chuck Lever wrote:
> Hi Sagi-
>
> Thanks for the review.
>
> On Jan 11, 2015, at 12:37 PM, Sagi Grimberg <sagig@dev.mellanox.co.il> wrote:
>
>> On 1/9/2015 9:22 PM, Chuck Lever wrote:
>>> xdr_start() can return the wrong rmsgp address if an assumption
>>> about how the xdr_buf was constructed changes.  When it gets it
>>> wrong, the client receives a reply that has gibberish in the
>>> RPC/RDMA header, preventing it from matching a waiting RPC request.
>>>
>>> Instead, make (and document) just one assumption: that the RDMA
>>> header for the client's RPC call is at the start of the first page
>>> in rq_pages.
>>
>> Would it make more sense to add another pointer assigned at req
>> initialization (maybe in the RDMA request context) instead of hard
>> coding this assumption? I may be completely wrong here though...
>
> I considered this. I couldn’t find an appropriate place to add
> such a pointer.
>
> I think that’s why xdr_start() was there in the first place: there
> is no convenient place to save a pointer to the request’s RDMA
> header.
>
> Bruce might have other thoughts about this.

Yep, I didn't find any nice place to put that also, thought you might
have an idea...

Sagi.

WARNING: multiple messages have this Message-ID (diff)
From: Sagi Grimberg <sagig-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
To: Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Cc: linux-rdma <linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Linux NFS Mailing List
	<linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH v1 05/10] svcrdma: Find rmsgp more reliably
Date: Tue, 13 Jan 2015 12:07:07 +0200	[thread overview]
Message-ID: <54B4EE4B.9050500@dev.mellanox.co.il> (raw)
In-Reply-To: <3C09A798-2BA9-46A1-AA60-122C2274974C-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

On 1/12/2015 2:30 AM, Chuck Lever wrote:
> Hi Sagi-
>
> Thanks for the review.
>
> On Jan 11, 2015, at 12:37 PM, Sagi Grimberg <sagig-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
>
>> On 1/9/2015 9:22 PM, Chuck Lever wrote:
>>> xdr_start() can return the wrong rmsgp address if an assumption
>>> about how the xdr_buf was constructed changes.  When it gets it
>>> wrong, the client receives a reply that has gibberish in the
>>> RPC/RDMA header, preventing it from matching a waiting RPC request.
>>>
>>> Instead, make (and document) just one assumption: that the RDMA
>>> header for the client's RPC call is at the start of the first page
>>> in rq_pages.
>>
>> Would it make more sense to add another pointer assigned at req
>> initialization (maybe in the RDMA request context) instead of hard
>> coding this assumption? I may be completely wrong here though...
>
> I considered this. I couldn’t find an appropriate place to add
> such a pointer.
>
> I think that’s why xdr_start() was there in the first place: there
> is no convenient place to save a pointer to the request’s RDMA
> header.
>
> Bruce might have other thoughts about this.

Yep, I didn't find any nice place to put that also, thought you might
have an idea...

Sagi.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2015-01-13 10:07 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-09 19:21 [PATCH v1 00/10] NFS/RDMA server for 3.20 Chuck Lever
2015-01-09 19:21 ` Chuck Lever
2015-01-09 19:22 ` [PATCH v1 01/10] svcrdma: Clean up dprintk Chuck Lever
2015-01-09 19:22   ` Chuck Lever
2015-01-09 19:22 ` [PATCH v1 02/10] svcrdma: Remove unused variable Chuck Lever
2015-01-09 19:22   ` Chuck Lever
2015-01-09 19:22 ` [PATCH v1 03/10] svcrdma: Clean up read chunk counting Chuck Lever
2015-01-09 19:22   ` Chuck Lever
2015-01-09 19:22 ` [PATCH v1 04/10] svcrdma: Scrub BUG_ON() and WARN_ON() call sites Chuck Lever
2015-01-09 19:22   ` Chuck Lever
2015-01-09 19:22 ` [PATCH v1 05/10] svcrdma: Find rmsgp more reliably Chuck Lever
2015-01-09 19:22   ` Chuck Lever
2015-01-11 17:37   ` Sagi Grimberg
2015-01-11 17:37     ` Sagi Grimberg
2015-01-12  0:30     ` Chuck Lever
2015-01-12  0:30       ` Chuck Lever
2015-01-13 10:07       ` Sagi Grimberg [this message]
2015-01-13 10:07         ` Sagi Grimberg
2015-01-09 19:22 ` [PATCH v1 06/10] svcrdma: Plant reader function in struct svcxprt_rdma Chuck Lever
2015-01-09 19:22   ` Chuck Lever
2015-01-11 17:45   ` Sagi Grimberg
2015-01-11 17:45     ` Sagi Grimberg
2015-01-12  0:41     ` Chuck Lever
2015-01-12  0:41       ` Chuck Lever
2015-01-12 16:08       ` Steve Wise
2015-01-12 16:08         ` Steve Wise
2015-01-12 16:20         ` Chuck Lever
2015-01-12 16:20           ` Chuck Lever
2015-01-12 16:26           ` Steve Wise
2015-01-12 16:26             ` Steve Wise
2015-01-12 16:45             ` Steve Wise
2015-01-12 16:45               ` Steve Wise
2015-01-13 10:05               ` Sagi Grimberg
2015-01-13 10:05                 ` Sagi Grimberg
2015-01-13 15:40                 ` Steve Wise
2015-01-13 15:40                   ` Steve Wise
2015-01-09 19:22 ` [PATCH v1 07/10] svcrdma: rc_position sanity checking Chuck Lever
2015-01-09 19:22   ` Chuck Lever
2015-01-09 19:23 ` [PATCH v1 08/10] svcrdma: Support RDMA_NOMSG requests Chuck Lever
2015-01-09 19:23   ` Chuck Lever
2015-01-09 19:23 ` [PATCH v1 09/10] Move read list XDR round-up logic Chuck Lever
2015-01-09 19:23   ` Chuck Lever
2015-01-09 20:14   ` J. Bruce Fields
2015-01-09 20:14     ` J. Bruce Fields
2015-01-09 20:20     ` Chuck Lever
2015-01-09 20:20       ` Chuck Lever
2015-01-09 19:23 ` [PATCH v1 10/10] svcrdma: Handle additional inline content Chuck Lever
2015-01-09 19:23   ` Chuck Lever
2015-01-11 18:01   ` Sagi Grimberg
2015-01-11 18:01     ` Sagi Grimberg
2015-01-12  1:13     ` Chuck Lever
2015-01-12  1:13       ` Chuck Lever
2015-01-13 10:11       ` Sagi Grimberg
2015-01-13 10:11         ` Sagi Grimberg
2015-01-13 14:35         ` Chuck Lever
2015-01-13 14:35           ` Chuck Lever
2015-01-09 20:39 ` [PATCH v1 00/10] NFS/RDMA server for 3.20 J. Bruce Fields
2015-01-09 20:39   ` J. Bruce Fields
2015-01-09 20:40   ` Chuck Lever
2015-01-09 20:40     ` Chuck Lever
2015-01-09 20:44     ` J. Bruce Fields
2015-01-09 20:44       ` J. Bruce Fields

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=54B4EE4B.9050500@dev.mellanox.co.il \
    --to=sagig@dev.mellanox.co.il \
    --cc=chuck.lever@oracle.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.