From: Dan Aloni <dan.aloni@vastdata.com>
To: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Anna Schumaker <Anna.Schumaker@netapp.com>, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 5/9] NFSv4: Clean up encode_attrs
Date: Tue, 17 May 2022 20:00:57 +0300 [thread overview]
Message-ID: <20220517170057.lt6prmdl2kaupo54@gmail.com> (raw)
In-Reply-To: <20180320210313.94429-6-trond.myklebust@primarydata.com>
Hi Trond,
Wireshark claims that this commit broke encoding of SETATTR (see below).
Is Wireshark correct in reference to the RFC?
Frame 56: 274 bytes on wire (2192 bits), 274 bytes captured (2192 bits)
Ethernet II, Src: RealtekU_d0:d8:ff (52:54:00:d0:d8:ff), Dst: RealtekU_7a:80:63 (52:54:00:7a:80:63)
Internet Protocol Version 4, Src: 192.168.40.1, Dst: 192.168.40.11
Transmission Control Protocol, Src Port: 999, Dst Port: 2049, Seq: 4325, Ack: 4489, Len: 208
Remote Procedure Call, Type:Call XID:0x3725a760
Network File System, Ops(4): SEQUENCE, PUTFH, SETATTR, GETATTR
[Program Version: 4]
[V4 Procedure: COMPOUND (1)]
Tag: <EMPTY>
minorversion: 1
Operations (count: 4): SEQUENCE, PUTFH, SETATTR, GETATTR
Opcode: SEQUENCE (53)
Opcode: PUTFH (22)
Opcode: SETATTR (34)
StateID
[StateID Hash: 0xafa9]
StateID seqid: 0
StateID Other: 000000000000000000000000
[StateID Other hash: 0x60de]
[Expert Info (Warning/Protocol): Per RFCs 3530 and 5661 an attribute mask is required but was not provided.]
[Per RFCs 3530 and 5661 an attribute mask is required but was not provided.]
[Severity level: Warning]
[Group: Protocol]
Opcode: GETATTR (9)
[Main Opcode: SETATTR (34)]
0000 52 54 00 7a 80 63 52 54 00 d0 d8 ff 08 00 45 00 RT.z.cRT......E.
0010 01 04 a6 16 40 00 40 06 c2 80 c0 a8 28 01 c0 a8 ....@.@.....(...
0020 28 0b 03 e7 08 01 14 fe a6 02 de f4 09 10 80 18 (...............
0030 01 7b d2 53 00 00 01 01 08 0a 68 ba 83 03 e9 ee .{.S......h.....
0040 7c 0b 80 00 00 cc 37 25 a7 60 00 00 00 00 00 00 |.....7%.`......
0050 00 02 00 01 86 a3 00 00 00 04 00 00 00 01 00 00 ................
0060 00 01 00 00 00 30 00 41 88 3d 00 00 00 16 63 6c .....0.A.=....cl
0070 69 65 6e 74 2e 6e 66 73 2d 74 65 73 74 69 6e 67 ient.nfs-testing
0080 2e 63 6f 6d 00 00 00 00 00 00 00 00 00 00 00 00 .com............
0090 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00a0 00 00 00 00 00 01 00 00 00 04 00 00 00 35 37 00 .............57.
00b0 00 00 00 a0 00 00 36 00 00 00 00 a0 00 00 00 00 ......6.........
00c0 00 14 00 00 00 00 00 00 00 00 00 00 00 01 00 00 ................
00d0 00 16 00 00 00 10 00 00 69 8d 44 d8 0e 34 0f 7d ........i.D..4.}
00e0 00 00 00 00 00 00 00 00 00 22 00 00 00 00 00 00 ........."......
00f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
0100 00 00 00 00 00 09 00 00 00 02 00 10 01 1a 00 30 ...............0
0110 a2 3a .:
On 2018-03-20 17:03:09, Trond Myklebust wrote:
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
> ---
> fs/nfs/nfs4xdr.c | 17 ++---------------
> 1 file changed, 2 insertions(+), 15 deletions(-)
>
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 80c5b519fd6a..3d088230c975 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -1052,9 +1052,7 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
> int owner_namelen = 0;
> int owner_grouplen = 0;
> __be32 *p;
> - unsigned i;
> uint32_t len = 0;
> - uint32_t bmval_len;
> uint32_t bmval[3] = { 0 };
>
> /*
> @@ -1123,19 +1121,8 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
> bmval[2] |= FATTR4_WORD2_SECURITY_LABEL;
> }
>
> - if (bmval[2] != 0)
> - bmval_len = 3;
> - else if (bmval[1] != 0)
> - bmval_len = 2;
> - else
> - bmval_len = 1;
> -
> - p = reserve_space(xdr, 4 + (bmval_len << 2) + 4 + len);
> -
> - *p++ = cpu_to_be32(bmval_len);
> - for (i = 0; i < bmval_len; i++)
> - *p++ = cpu_to_be32(bmval[i]);
> - *p++ = cpu_to_be32(len);
> + xdr_encode_bitmap4(xdr, bmval, ARRAY_SIZE(bmval));
> + xdr_stream_encode_opaque_inline(xdr, (void **)&p, len);
>
> if (bmval[0] & FATTR4_WORD0_SIZE)
> p = xdr_encode_hyper(p, iap->ia_size);
> --
> 2.14.3
>
--
Dan Aloni
next prev parent reply other threads:[~2022-05-17 17:01 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-20 21:03 [PATCH 0/9] Misc cleanups Trond Myklebust
2018-03-20 21:03 ` [PATCH 1/9] SUNRPC: Add helpers for decoding opaque and string types Trond Myklebust
2018-03-20 21:03 ` [PATCH 2/9] SUNRPC: Add a helper for encoding opaque data inline Trond Myklebust
2018-03-20 21:03 ` [PATCH 3/9] NFSv4: Allow GFP_NOIO sleeps in decode_attr_owner/decode_attr_group Trond Myklebust
2018-03-20 21:03 ` [PATCH 4/9] NFSv4; Clean up XDR encoding of type bitmap4 Trond Myklebust
2018-03-20 21:03 ` [PATCH 5/9] NFSv4: Clean up encode_attrs Trond Myklebust
2018-03-20 21:03 ` [PATCH 6/9] NFSv4: Add a helper to encode/decode struct timespec Trond Myklebust
2018-03-20 21:03 ` [PATCH 7/9] NFSv4: Don't ask for attributes when ACCESS is protected by a delegation Trond Myklebust
2018-03-20 21:03 ` [PATCH 8/9] NFSv4: Clean up CB_GETATTR encoding Trond Myklebust
2018-03-20 21:03 ` [PATCH 9/9] NFSv4: Fix the nfs_inode_set_delegation() arguments Trond Myklebust
2022-05-17 17:00 ` Dan Aloni [this message]
2022-05-17 18:20 ` [PATCH 5/9] NFSv4: Clean up encode_attrs Trond Myklebust
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=20220517170057.lt6prmdl2kaupo54@gmail.com \
--to=dan.aloni@vastdata.com \
--cc=Anna.Schumaker@netapp.com \
--cc=linux-nfs@vger.kernel.org \
--cc=trond.myklebust@primarydata.com \
/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;
as well as URLs for NNTP newsgroup(s).