[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wireshark-bugs
Subject:    [Wireshark-bugs] [Bug 16372] Relative sequence number should be in brackets
From:       bugzilla-daemon () wireshark ! org
Date:       2020-07-30 19:00:44
Message-ID: 01000173a1191d55-84b67955-8007-4d6a-926e-2a686cd64f88-000000 () us-east-1 ! amazonses ! com
[Download RAW message or body]

--15961356421.a7AAC62D.13680
Date: Thu, 30 Jul 2020 19:00:42 +0000
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Bugzilla-URL: https://bugs.wireshark.org/bugzilla/
Auto-Submitted: auto-generated

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=16372

--- Comment #7 from David Perry <boolean263@gmail.com> ---
(In reply to Peter Wu from comment #6)
> At that time I decided it was not worth persuing the change to make tcp.seq
> show relative numbers in all cases. Apart from a purity perspective, is
> there any reason why you would like to make this change?

A large majority of the fields in Wireshark have fixed meanings. tcp.seq et al
breaks this mold by changing their meaning from user to user based on their
preferences. (Is this what you mean by purity?)

Perhaps the same point from a slightly different perspective, but: absolute
sequence number is always available, but the field it's available in is either
tcp.seq or tcp.seq_raw depending on preferences. If one imagines a program that
operates on tshark output, this becomes a hunt for the right value, managed by
careful control of preferences when generating the output, or by adding extra
logic when reading the output.

The original point made by dandav1992+bug in raising this bug was that relative
sequence numbers are not the values found on the wire, so they should be marked
as generated fields. This would be consistent with the rest of Wireshark too.
One might even argue it's misleading to *not* do this because it implies
tcp.seq=0 was what was seen on the wire.

I have no skin in this game myself, I just saw this bug as a way to try and
give back to a project that's been so helpful to me, so I decided to see what
would happen.

With that said, packet-tcp.c is one of the more complex dissectors --
understandably, since it's at the root of much of the internet, and depended on
by higher level dissectors. From a coding perspective I feel like it's a bit
easier to understand what's going on in the file when seq#'s are clearly
distinguished as absolute or relative, independent of a global variable.

I'm not married to my particular implementation, but I'm a fan of the
consistent distinction of types of sequence. (In case you couldn't tell!) I'd
be just as happy if tcp.seq_raw was always present and tcp.seq was always
relative.

--

On this topic, can you help me understand why the calculation of relative
sequence numbers is said to also require "Analyze TCP sequence numbers"? I've
added a comment in the review to the piece of code where this happens in
packet-tcp.c. I left it in the `if(tcp_analyze_seq)` block, but
`tcp_analyze_sequence_number()` doesn't appear to transform anything that would
affect the calculation of relative sequence numbers.

-- 
You are receiving this mail because:
You are watching all bug changes.
--15961356421.a7AAC62D.13680
Date: Thu, 30 Jul 2020 19:00:42 +0000
MIME-Version: 1.0
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Bugzilla-URL: https://bugs.wireshark.org/bugzilla/
Auto-Submitted: auto-generated

<html>
    <head>
      <base href="https://bugs.wireshark.org/bugzilla/" />
      <style>
        body, th, td {
            font-size: 12px;
            font-family: Arial, Helvetica, sans-serif; }
        p, pre { margin-top: 1em; }
        pre {
            font-family: Bitstream Vera Sans Mono, Consolas, Lucida Console, \
monospace;  white-space: pre-wrap;
	}
        table { border: 0; border-spacing: 0; border-collapse: collapse; }
        th, td {
            padding: 0.25em;
            padding-left: 0.5em;
            padding-right: 0.5em;
        }
        th { background: rgb(240, 240, 240); }
        th.th_top { border-bottom: 1px solid rgb(116, 126, 147); }
        th.th_left { border-right: 1px solid rgb(116, 126, 147); }
        td.removed { background-color: #ffcccc; }
        td.added { background-color: #e4ffc7; }
      </style>
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_UNCONFIRMED "
   title="UNCONFIRMED - Relative sequence number should be in brackets"
   href="https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=16372#c7">Comment # \
7</a>  on <a class="bz_bug_link 
          bz_status_UNCONFIRMED "
   title="UNCONFIRMED - Relative sequence number should be in brackets"
   href="https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=16372">bug 16372</a>
              from <span class="vcard"><a class="email" \
href="mailto:boolean263&#64;gmail.com" title="David Perry \
&lt;boolean263&#64;gmail.com&gt;"> <span class="fn">David Perry</span></a> \
                </span></b>
        <pre>(In reply to Peter Wu from <a href="show_bug.cgi?id=16372#c6">comment \
#6</a>) <span class="quote">&gt; At that time I decided it was not worth persuing the \
change to make tcp.seq &gt; show relative numbers in all cases. Apart from a purity \
perspective, is &gt; there any reason why you would like to make this change?</span >

A large majority of the fields in Wireshark have fixed meanings. tcp.seq et al
breaks this mold by changing their meaning from user to user based on their
preferences. (Is this what you mean by purity?)

Perhaps the same point from a slightly different perspective, but: absolute
sequence number is always available, but the field it's available in is either
tcp.seq or tcp.seq_raw depending on preferences. If one imagines a program that
operates on tshark output, this becomes a hunt for the right value, managed by
careful control of preferences when generating the output, or by adding extra
logic when reading the output.

The original point made by dandav1992+bug in raising this bug was that relative
sequence numbers are not the values found on the wire, so they should be marked
as generated fields. This would be consistent with the rest of Wireshark too.
One might even argue it's misleading to *not* do this because it implies
tcp.seq=0 was what was seen on the wire.

I have no skin in this game myself, I just saw this bug as a way to try and
give back to a project that's been so helpful to me, so I decided to see what
would happen.

With that said, packet-tcp.c is one of the more complex dissectors --
understandably, since it's at the root of much of the internet, and depended on
by higher level dissectors. From a coding perspective I feel like it's a bit
easier to understand what's going on in the file when seq#'s are clearly
distinguished as absolute or relative, independent of a global variable.

I'm not married to my particular implementation, but I'm a fan of the
consistent distinction of types of sequence. (In case you couldn't tell!) I'd
be just as happy if tcp.seq_raw was always present and tcp.seq was always
relative.

--

On this topic, can you help me understand why the calculation of relative
sequence numbers is said to also require &quot;Analyze TCP sequence numbers&quot;? \
I've added a comment in the review to the piece of code where this happens in
packet-tcp.c. I left it in the `if(tcp_analyze_seq)` block, but
`tcp_analyze_sequence_number()` doesn't appear to transform anything that would
affect the calculation of relative sequence numbers.</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are watching all bug changes.</li>
      </ul>
    </body>
</html>
--15961356421.a7AAC62D.13680--


[Attachment #3 (text/plain)]

___________________________________________________________________________
Sent via:    Wireshark-bugs mailing list <wireshark-bugs@wireshark.org>
Archives:    https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs
             mailto:wireshark-bugs-request@wireshark.org?subject=unsubscribe

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic