Re: [libvirt] [PATCH v3 15/31] Teach wireshark plugin about VIR_NET_STREAM_HOLE
On 05/17/2017 01:44 PM, John Ferlan wrote:
On 05/16/2017 10:03 AM, Michal Privoznik wrote:
> Ideally, this would be generated, but to achieve that
> corresponding XDR definitions needed to go into a different .x
> file. But they belong just to the one that they are right now.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> tools/wireshark/src/packet-libvirt.c | 52 ++++++++++++++++++++++++++++++++++++
> tools/wireshark/src/packet-libvirt.h | 2 ++
> 2 files changed, 54 insertions(+)
>
> diff --git a/tools/wireshark/src/packet-libvirt.c
b/tools/wireshark/src/packet-libvirt.c
> index 260161e98..a1f5a34f4 100644
> --- a/tools/wireshark/src/packet-libvirt.c
> +++ b/tools/wireshark/src/packet-libvirt.c
> @@ -50,8 +50,12 @@ static int hf_libvirt_serial = -1;
> static int hf_libvirt_status = -1;
> static int hf_libvirt_stream = -1;
> static int hf_libvirt_num_of_fds = -1;
> +static int hf_libvirt_stream_hole_length = -1;
How will this work with a LL (hyper) stream hole size? [1]
> +static int hf_libvirt_stream_hole_flags = -1;
> +static int hf_libvirt_stream_hole = -1;
> int hf_libvirt_unknown = -1;
> static gint ett_libvirt = -1;
> +static gint ett_libvirt_stream_hole = -1;
>
> #define XDR_PRIMITIVE_DISSECTOR(xtype, ctype, ftype) \
> static gboolean \
> @@ -326,6 +330,33 @@ dissect_libvirt_payload_xdr_data(tvbuff_t *tvb, proto_tree
*tree, gint payload_l
> dissect_libvirt_fds(tvb, start + payload_length, nfds);
> }
>
> +static gboolean
> +dissect_xdr_stream_hole(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf)
int hf? [1]
Of course this does match the prototype I see:
static gboolean dissect_xdr_hyper(tvbuff_t *tvb, proto_tree *tree, XDR
*xdrs, int hf);
but seems strange. Then again, I know next to nothing about wireshark.
'int hf' is not there to represent LL @offset. @hf stands for 'header
field'. It's an index into an array where info on the field is to be
found, for instance:
{ &hf_libvirt_stream_hole,
{ "stream_hole", "libvirt.stream_hole",
FT_NONE, BASE_NONE,
NULL, 0x0,
NULL, HFILL}
},
"stream_hole" - abbreviated name of the field
"libvirt.stream_hole" - full name of the field
FT_NONE - field type (char, bool, integer of all sorts, ...)
BASE_NONE - base for decoding (decimal, hexa, octa, ...)
and so on.
Therefore, the 'int hf' in the dissect_xdr_stream_hole is not 'long long
offset' contained in the packed, rather than an index into an array.
This index is then used when telling wireshark how to display particular
piece of a packet.
Call this a "weak" (at best)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
Good enough for me :-)
Michal