[pnfs] draft19 patches for wireshark trunk
J. Bruce Fields
bfields at fieldses.org
Mon Feb 11 14:00:29 EST 2008
On Mon, Feb 11, 2008 at 07:02:53PM +0100, Mkrtchyan, Tigran wrote:
> sorry, but I do not know who to force git to produce one patch.
If you just need to get a quick diff,
git diff HEAD^^..HEAD
will diff between HEAD and the tree two commits ago.
Also, "git rebase -i" (see man git-rebase) gives a pretty quick way to
squash two commits together.
Anyway, no problem.
I also think a (hopefully small) packet capture with a wide range of
operations (including layout operations, etc.) would be useful to the
wireshark maintainers, if anyone has a quick way to generate one.
--b.
> I can fix logr_layout decoding, but while our server do not support call backs yet
> it's difficult to fix it. I can pend some time dirung bakeathone to improve it.
>
> Regards,
> Tigran.
>
> -----Original Message-----
> From: Benny Halevy [mailto:bhalevy at panasas.com]
> Sent: Mon 11.02.2008 14:48
> To: Mkrtchyan, Tigran
> Cc: J. Bruce Fields; pnfs at linux-nfs.org
> Subject: Re: [pnfs] draft19 patches for wireshark trunk
>
> Tigran, Bruce probably meant sending a new, fixed patch, rather than the original
> and one fixing it. I took a stab at generating one using git diff -w over
> both your patches applied (to ignore white space) plus, I changed a few more sites
> where indentation got broke. It's attached here.
>
> And, a few comments... :)
>
> > @@ -7775,7 +7797,10 @@ dissect_nfs_devicelist4(tvbuff_t *tvb, int offset, proto_tree *tree)
> > offset = dissect_rpc_uint32(tvb, tree, hf_nfs_devicenum4, offset);
> > for (i = 0; i < count; i++) {
> > int opaque_devs, dev_limit;
> > - offset = dissect_rpc_uint32(tvb, tree, hf_nfs_deviceid4, offset);
> > + offset = dissect_rpc_opaque_data(tvb, offset, tree, NULL,
> > + hf_nfs_deviceid4, TRUE, 16,
> > + FALSE, NULL, NULL);
>
> Since this is repeated several times, how about defining a function for dissecting
> the 16-bytes deviceid?
>
> > @@ -8614,6 +8646,8 @@ dissect_nfs_resop4(tvbuff_t *tvb, int offset, packet_info *pinfo,
> > case NFS4_OP_LAYOUTGET:
> > offset = dissect_rpc_bool(tvb, newftree, hf_nfs_return_on_close4,
> > offset);
> > + offset = dissect_nfs_stateid4(tvb, offset, newftree);
> > + offset = dissect_rpc_uint32(tvb, newftree, hf_nfs_layout_count, offset);
> > offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_offset4, offset);
> > offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_length4, offset);
> > offset = dissect_rpc_uint32(tvb, newftree, hf_nfs_iomode4, offset);
>
> logr_layout is now an array, therefore dissecting the layout4 structure contents should
> be done in a loop, hf_nfs_layout_count times.
>
> Benny
>
> PS. do you plan on updating other ops (LAYOUTCOMMIT and LAYOUTRETURN) for draft-19?
>
> PS2. what about callbacks? is anyone planning on doing them as well?
>
> On Feb. 11, 2008, 15:03 +0200, Tigran Mkrtchyan <tigran.mkrtchyan at desy.de> wrote:
> > fixed.
> > Tigran.
> >
> >
> > J. Bruce Fields wrote:
> >> On Tue, Feb 05, 2008 at 04:52:12PM +0100, Tigran Mkrtchyan wrote:
> >>> Oh! sorry about that!
> >> Thanks!
> >>
> >>> - guint i;
> >>> - guint32 num_devs,num_indices,num_multipath;
> >>> + guint i,j;
> >>> + guint32 num_indices,num_multipath, num_addr;
> >>>
> >>> - /* No layout type - argh */
> >>> + /* No layout type - argh */
> >>>
> >>> - /* Assume file layout for now */
> >>> + /* Assume file layout for now */
> >> There's no need to reindent all this.
> >>
> >> Would you mind just fixing your indentation to agree with what's already
> >> used in the rest of the file (tabs, as opposed to 4 spaces), and then
> >> resubmitting?
> >>
> >> --b.
> >>
> >>>
> >>> - /* disect indices */
> >>> - num_indices = tvb_get_ntohl(tvb, offset);
> >>> - offset += 4;
> >>> - for (i = 0; i < num_indices; i++) {
> >>> - offset = dissect_rpc_uint32(tvb, tree, hf_nfs_deviceidx4,offset);
> >>> - }
> >>> + /* disect indices */
> >>> + num_indices = tvb_get_ntohl(tvb, offset);
> >>> + offset += 4;
> >>> + for (i = 0; i < num_indices; i++) {
> >>> + offset = dissect_rpc_uint32(tvb, tree, hf_nfs_deviceidx4,offset);
> >>> + }
> >>>
> >>> - /* disect devices */
> >>> - num_devs = tvb_get_ntohl(tvb, offset);
> >>> - offset += 4;
> >>> - for (i = 0; i < num_devs; i++) {
> >>> - num_multipath = tvb_get_ntohl(tvb, offset);
> >>> - offset += 4;
> >>> - for (i = 0; i < num_multipath; i++) {
> >>> - offset = dissect_nfsdata(tvb, offset, tree, hf_nfs_r_netid);
> >>> - offset = dissect_nfsdata(tvb, offset, tree, hf_nfs_r_addr);
> >>> - }
> >>> - }
> >>> + num_multipath = tvb_get_ntohl(tvb, offset);
> >>> + offset += 4;
> >>> + for (i = 0; i < num_multipath; i++) {
> >>> + num_addr = tvb_get_ntohl(tvb, offset);
> >>> + offset += 4;
> >>> + for( j = 0; j < num_addr; j++ ) {
> >>> + offset = dissect_nfsdata(tvb, offset, tree, hf_nfs_r_netid);
> >>> + offset = dissect_nfsdata(tvb, offset, tree, hf_nfs_r_addr);
> >>> + }
> >>> + }
> >>>
> >>> - return offset;
> >>> + return offset;
> >>> }
> >>>
> >>>
> >>> @@ -7760,6 +7779,9 @@ dissect_nfs_deviceaddr4(tvbuff_t *tvb, int offset, proto_tree *tree)
> >>> /* Assume file layout for now */
> >>> offset = dissect_rpc_uint32(tvb, tree, hf_nfs_layouttype4, offset);
> >>>
> >>> + /* skip da_addr_body size */
> >>> + offset+=4;
> >>> +
> >>> offset = dissect_nfs_devices4(tvb, offset, tree);
> >>>
> >>> return offset;
> >>> @@ -7775,7 +7797,10 @@ dissect_nfs_devicelist4(tvbuff_t *tvb, int offset, proto_tree *tree)
> >>> offset = dissect_rpc_uint32(tvb, tree, hf_nfs_devicenum4, offset);
> >>> for (i = 0; i < count; i++) {
> >>> int opaque_devs, dev_limit;
> >>> - offset = dissect_rpc_uint32(tvb, tree, hf_nfs_deviceid4, offset);
> >>> + offset = dissect_rpc_opaque_data(tvb, offset, tree, NULL,
> >>> + hf_nfs_deviceid4, TRUE, 16,
> >>> + FALSE, NULL, NULL);
> >>> +
> >>>
> >>> offset = dissect_rpc_uint32(tvb, tree, hf_nfs_layouttype4, offset);
> >>>
> >>> @@ -7925,9 +7950,13 @@ dissect_nfs_layout(tvbuff_t *tvb, int offset, packet_info *pinfo, proto_tree *tr
> >>> guint num;
> >>> guint i;
> >>> offset += 4; /* Skip past opaque count */
> >>> - offset = dissect_rpc_uint32(tvb, tree, hf_nfs_deviceid4, offset);
> >>> + offset = dissect_rpc_opaque_data(tvb, offset, tree, NULL,
> >>> + hf_nfs_deviceid4, TRUE, 16,
> >>> + FALSE, NULL, NULL);
> >>> +
> >>> offset = dissect_rpc_uint32(tvb, tree, hf_nfs_util4, offset);
> >>> offset = dissect_rpc_uint32(tvb, tree, hf_nfs_first_stripe_idx4, offset);
> >>> + offset = dissect_rpc_uint64(tvb, tree, hf_nfs_pattern_offset, offset);
> >>>
> >>> num = tvb_get_ntohl(tvb, offset); /* Len of dev list */
> >>> offset += 4;
> >>> @@ -8302,12 +8331,15 @@ dissect_nfs_argop4(tvbuff_t *tvb, int offset, packet_info *pinfo,
> >>> break;
> >>>
> >>> case NFS4_OP_GETDEVINFO:
> >>> - offset = dissect_rpc_uint32(tvb, newftree, hf_nfs_deviceid4,
> >>> - offset);
> >>> - offset = dissect_rpc_uint32(tvb, newftree, hf_nfs_layouttype4,
> >>> - offset);
> >>> - offset = dissect_rpc_uint32(tvb, newftree, hf_nfs_count4_maxcount,
> >>> - offset);
> >>> + offset = dissect_rpc_opaque_data(tvb, offset, newftree, NULL,
> >>> + hf_nfs_deviceid4, TRUE, 16,
> >>> + FALSE, NULL, NULL);
> >>> +
> >>> + offset = dissect_rpc_uint32(tvb, newftree, hf_nfs_layouttype4,
> >>> + offset);
> >>> + offset = dissect_rpc_uint32(tvb, newftree, hf_nfs_count4_maxcount,
> >>> + offset);
> >>> + offset = dissect_nfs_notification_bitmap4(tvb, newftree, offset);
> >>> break;
> >>>
> >>> case NFS4_OP_GETDEVLIST:
> >>> @@ -8614,6 +8646,8 @@ dissect_nfs_resop4(tvbuff_t *tvb, int offset, packet_info *pinfo,
> >>> case NFS4_OP_LAYOUTGET:
> >>> offset = dissect_rpc_bool(tvb, newftree, hf_nfs_return_on_close4,
> >>> offset);
> >>> + offset = dissect_nfs_stateid4(tvb, offset, newftree);
> >>> + offset = dissect_rpc_uint32(tvb, newftree, hf_nfs_layout_count, offset);
> >>> offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_offset4, offset);
> >>> offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_length4, offset);
> >>> offset = dissect_rpc_uint32(tvb, newftree, hf_nfs_iomode4, offset);
> >>> @@ -8629,6 +8663,7 @@ dissect_nfs_resop4(tvbuff_t *tvb, int offset, packet_info *pinfo,
> >>>
> >>> case NFS4_OP_GETDEVINFO:
> >>> offset = dissect_nfs_deviceaddr4(tvb, offset, newftree);
> >>> + offset = dissect_nfs_notification_bitmap4(tvb, newftree, offset);
> >>> break;
> >>>
> >>> case NFS4_OP_GETDEVLIST:
> >>> @@ -10035,6 +10070,14 @@ proto_register_nfs(void)
> >>> "first stripe index", "nfs.stripeindex", FT_UINT32, BASE_DEC,
> >>> NULL, 0, NULL, HFILL }},
> >>>
> >>> + { &hf_nfs_pattern_offset, {
> >>> + "layout pattern offset", "nfs.patternoffset", FT_UINT64, BASE_DEC,
> >>> + NULL, 0, "layout pattern offset", HFILL }},
> >>> +
> >>> + { &hf_nfs_notification_bitmap4, {
> >>> + "notification bitmap", "nfs.notificationbitmap", FT_UINT32, BASE_DEC,
> >>> + NULL, 0, "notification bitmap", HFILL }},
> >>> +
> >>> { &hf_nfs_newtime4, {
> >>> "new time?", "nfs.newtime", FT_BOOLEAN, BASE_NONE,
> >>> TFS(&tfs_yes_no), 0, NULL, HFILL }},
> >>> @@ -10060,8 +10103,8 @@ proto_register_nfs(void)
> >>> NULL, 0, NULL, HFILL }},
> >>>
> >>> { &hf_nfs_deviceid4, {
> >>> - "device ID", "nfs.deviceid", FT_UINT32, BASE_DEC,
> >>> - NULL, 0, NULL, HFILL }},
> >>> + "device ID", "nfs.deviceid", FT_BYTES, BASE_HEX,
> >>> + NULL, 0, "device ID", HFILL }},
> >>>
> >>> { &hf_nfs_devicenum4, {
> >>> "num devices", "nfs.devicenum4", FT_UINT32, BASE_DEC,
> >>> @@ -10075,6 +10118,11 @@ proto_register_nfs(void)
> >>> "layout", "nfs.layout", FT_BYTES, BASE_DEC,
> >>> NULL, 0, NULL, HFILL }},
> >>>
> >>> + { &hf_nfs_layout_count, {
> >>> + "layout", "nfs.layoutcount", FT_UINT32, BASE_DEC,
> >>> + NULL, 0, "layout count", HFILL }},
> >>> +
> >>> +
> >>> { &hf_nfs_stripedevs4, {
> >>> "stripe devs", "nfs.stripedevs", FT_UINT32, BASE_DEC,
> >>> NULL, 0, NULL, HFILL }},
> >>> --
> >>> 1.5.2.4
> >>>
> >
> >
> >
> > ------------------------------------------------------------------------
> >
> > _______________________________________________
> > pNFS mailing list
> > pNFS at linux-nfs.org
> > http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>
>
More information about the pNFS
mailing list