[Lustre-devel] client-side reply handling

[Lustre-devel] client-side reply handling

[Eric Barton]

Edited from IRC...

> <maxim> eeb_: Liang: btw, shadow complained recently that it's not
>       great that both 'real' and early replies are being stored in
>       the same place. having in mind possible reordering of packets
>       -just a side note - not sure if it's applicable to your
>       discussion
> 
> <eeb_> the idea was to minimize the # of attach operations since
>        that was considered expensive
> 
> <eeb_> also, we'd need another portal or additional matchbits for
>        early replies if we register them separately
> 
> <eeb_> also, the current way of doing it allows the server to send
>        multiple early replies if it wants to
> 
> <eeb_> I'm not so worried about early replies sharing the same ME/MD
>        with the real reply provided....
> 
> <eeb_> 1. The client detects a protocol error if the "early" reply
>           overlaps the "real" reply part of the reply buffer or if
>           the "real" reply overlaps the "early" part of the reply
>           buffer
> 
> <eeb_> 2. Interpretation of replies (both early and real) is safe
>           against the network overwriting them - e.g. early replies
>           are copied out of the buffer before being interpreted and
>           the whole reply buffer is unlinked before the real reply
>           is interpreted in-place
> 
> <Liang> eeb_: do you mean, we can be 100% sure it's safe to unpack
>         in-place only when the buffer is unlinked? so it is better
>         to unregister reply buffer before calling into
>         after_reply()->unpack_reply()?

Yes, I think so.  While the reply buffer remains attached, it's
possible to overwrite it at any time.  This could happen if...

a) The server is buggy or malign

b) The request is re-sent and the same reply matchbits are used,
   which is what I think happens currently for non-bulk reqs.

...so unlikely, but possible.

    Cheers,
              Eric

[Lustre-devel] client-side reply handling

[Andreas Dilger]

On 2009-12-03, at 08:00, Eric Barton wrote:
> Edited from IRC...
>>
>> <Liang> eeb_: do you mean, we can be 100% sure it's safe to unpack
>>        in-place only when the buffer is unlinked? so it is better
>>        to unregister reply buffer before calling into
>>        after_reply()->unpack_reply()?
>
> Yes, I think so.  While the reply buffer remains attached, it's
> possible to overwrite it at any time.  This could happen if...
>
> a) The server is buggy or malign

Let's hope we never have to worry about malicious server nodes...

> b) The request is re-sent and the same reply matchbits are used,
>   which is what I think happens currently for non-bulk reqs.

In theory, the reply to the re-sent request should be identical due
to reply reconstruction, so it shouldn't matter if it happens to
overwrite the same buffer.

> ...so unlikely, but possible.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

[Lustre-devel] client-side reply handling

[Robert Read]

On Dec 3, 2009, at 13:17 , Andreas Dilger wrote:

> On 2009-12-03, at 08:00, Eric Barton wrote:
>> Edited from IRC...
>>> 
>>> <Liang> eeb_: do you mean, we can be 100% sure it's safe to unpack
>>>       in-place only when the buffer is unlinked? so it is better
>>>       to unregister reply buffer before calling into
>>>       after_reply()->unpack_reply()?
>> 
>> Yes, I think so.  While the reply buffer remains attached, it's
>> possible to overwrite it at any time.  This could happen if...
>> 
>> a) The server is buggy or malign
> 
> Let's hope we never have to worry about malicious server nodes...
> 
>> b) The request is re-sent and the same reply matchbits are used,
>>  which is what I think happens currently for non-bulk reqs.
> 
> In theory, the reply to the re-sent request should be identical due
> to reply reconstruction, so it shouldn't matter if it happens to
> overwrite the same buffer.

That's fine as long as the buffer is unlinked from the net before it gets swabbed, but hopefully that's the case already. 

robert

[Lustre-devel] client-side reply handling

[Isaac Huang]

On Thu, Dec 03, 2009 at 02:23:25PM -0800, Robert Read wrote:
> 
> On Dec 3, 2009, at 13:17 , Andreas Dilger wrote:
> 
> > On 2009-12-03, at 08:00, Eric Barton wrote:
> >> Edited from IRC...
> >>> 
> >>> <Liang> eeb_: do you mean, we can be 100% sure it's safe to unpack
> >>>       in-place only when the buffer is unlinked? so it is better
> >>>       to unregister reply buffer before calling into
> >>>       after_reply()->unpack_reply()?
> >> 
> >> Yes, I think so.  While the reply buffer remains attached, it's
> >> possible to overwrite it at any time.  This could happen if...
> >> 
> >> a) The server is buggy or malign
> > 
> > Let's hope we never have to worry about malicious server nodes...
> > 
> >> b) The request is re-sent and the same reply matchbits are used,
> >>  which is what I think happens currently for non-bulk reqs.
> > 
> > In theory, the reply to the re-sent request should be identical due
> > to reply reconstruction, so it shouldn't matter if it happens to
> > overwrite the same buffer.
> 
> That's fine as long as the buffer is unlinked from the net before it gets swabbed, but hopefully that's the case already. 

I think in-place unpack without unlinking the MD is OK if
LNET_MD_MANAGE_REMOTE is not set for the MD - e.g. request buffers -
because MD offset is maintained locally. However, all reply buffers
seem to have LNET_MD_MANAGE_REMOTE enabled.

Isaac

[Lustre-devel] client-side reply handling

[Liang Zhen]

Robert Read wrote:
>>
>>     
>>> b) The request is re-sent and the same reply matchbits are used,
>>>  which is what I think happens currently for non-bulk reqs.
>>>       
>> In theory, the reply to the re-sent request should be identical due
>> to reply reconstruction, so it shouldn't matter if it happens to
>> overwrite the same buffer.
>>     
>
> That's fine as long as the buffer is unlinked from the net before it gets swabbed, but hopefully that's the case already. 
>   

I'm afraid it's not the case in ptlrpc_check_set(), we begin to swab 
reply when the request is marked as replied and unregister(unlink) reply 
buffer after that, it's the reason Eric brought the topic...

Liang

> robert
>
> _______________________________________________
> Lustre-devel mailing list
> Lustre-devel at lists.lustre.org
> http://lists.lustre.org/mailman/listinfo/lustre-devel
>   

[Lustre-devel] client-side reply handling

[Eric Barton]

> On 2009-12-03, at 08:00, Eric Barton wrote:
> > Edited from IRC...
> >>
> >> <Liang> eeb_: do you mean, we can be 100% sure it's safe to unpack
> >>        in-place only when the buffer is unlinked? so it is better
> >>        to unregister reply buffer before calling into
> >>        after_reply()->unpack_reply()?
> >
> > Yes, I think so.  While the reply buffer remains attached, it's
> > possible to overwrite it at any time.  This could happen if...
> >
> > a) The server is buggy or malign
> 
> Let's hope we never have to worry about malicious server nodes...

We do.

> > b) The request is re-sent and the same reply matchbits are used,
> >   which is what I think happens currently for non-bulk reqs.
> 
> In theory, the reply to the re-sent request should be identical due
> to reply reconstruction, so it shouldn't matter if it happens to
> overwrite the same buffer.

We're at the mercy of the sender here.  The only robust option is never
to interpret volatile buffers.