Wireshark-dev: Re: [Wireshark-dev] Display more digits for NTP packet's root dispersion

From: Martin Burnicki <martin.burnicki@xxxxxxxxxxx>
Date: Mon, 6 Aug 2018 13:20:04 +0200
Hi Dario,

Dario Lombardo wrote:
> Hi Martin
> If you can code the patch, feel free to push it to gerrit for code
> review.

I think to do this would be easy for someone who is a bit familiar with
the wireshark source code. However, I'm new to this stuff so I can
potentially get it wrong unless I'm doing quite a bit of investigation
first. ;-)

The current code reads:

------------------------------------------------------------------
/* Root Dispersion, 32-bit unsigned fixed-point number indicating
 * the nominal error relative to the primary reference source, in
 * seconds with fraction point between bits 15 and 16.
 */
rootdispersion = tvb_get_ntohis(tvb, 8) +
			(tvb_get_ntohs(tvb, 10) / 65536.0);
proto_tree_add_double(ntp_tree, hf_ntp_rootdispersion, tvb, 8, 4,
			rootdispersion);
------------------------------------------------------------------

As far as I have seen, proto_tree_add_double() seems to add a double
value to the output tree, with a fixed format string for 4 fractional
digits.


A little bit above this code there is a code section which reports the
precision:

------------------------------------------------------------------
/* Precision, 1 byte field indicating the precision of the
 * local clock, in seconds to the nearest power of two.
 */
precision = tvb_get_guint8(tvb, 3);
proto_tree_add_int_format_value(ntp_tree, hf_ntp_precision, tvb, 3, 1,
			   precision,
			   "%8.6f sec",
			   pow(2, precision));
------------------------------------------------------------------

The proto_tree_add_int_format_value() used here expects a format string
and a "double" parameter, so this also seems to be useful for the root
dispersion, and even the root delay, e.g. something like:

------------------------------------------------------------------
rootdispersion = tvb_get_ntohis(tvb, 8) +
			(tvb_get_ntohs(tvb, 10) / 65536.0);
proto_tree_add_int_format_value(ntp_tree, hf_ntp_rootdispersion, tvb,
			8, 4, ... ,
			"%8.6f sec",
			rootdispersion));
------------------------------------------------------------------

I'm not yet sure, however, what exactly the functions

tvb_get_ntohis(tvb, 8)
tvb_get_ntohs(tvb, 10)

do. I'm assuming they need to be called to get the raw integer value
that would also need to be passed to the
proto_tree_add_int_format_value() function as the "value" parameter
"..." in the example above, before the format string parameter.

Anyway, the same decoding would also be useful for the root *delay*
parameter, IMO.

If you can confirm that this approach could be appropriate, I can
continue to investigate, and try to create and submit the patch.

> Otherwise, your best bet is bugzilla. Which login issues are you
> experiencing?

When I started to look at the wireshark project I first saw "gerrit".
Never heard of this before, but I could sign in using my github account
which is assiociated with my private email address,
martin.burnicki@xxxxxxxxxxxx.

Then I tried to create account on bugzilla with that private email
address, and bugzilla said a confirmation email had been sent. However,
that email never arrived.

Then tried to create an account for ask.wireshark.com, also with my
private email address, and similary, I never received a confirmation email.

Next I tried the dev mailing list, but used my business email account,
and in fact this worked.

So it looks like emails from your project to my private email address
are blocked somewhere. I get a bunch of email addresses from different
projects and mailing lists every day, and there are no known problems.
Eventually you (or the email admin) can find out why this happens.

Haven't yet tried to create a bugzilla account with my business email
address.


Regards,

Martin
-- 
Martin Burnicki

Senior Software Engineer

MEINBERG Funkuhren GmbH & Co. KG
Email: martin.burnicki@xxxxxxxxxxx
Phone: +49 5281 9309-414
Linkedin: https://www.linkedin.com/in/martinburnicki/

Lange Wand 9, 31812 Bad Pyrmont, Germany
Amtsgericht Hannover 17HRA 100322
Geschäftsführer/Managing Directors: Günter Meinberg, Werner Meinberg,
Andre Hartmann, Heiko Gerstung
Websites: https://www.meinberg.de  https://www.meinbergglobal.com
Training: https://www.meinberg.academy