Re: Different reassembly needs

[John Thacker <johnthacker () gmail com>]

Hi Paul,

On Thu, Jan 19, 2017 at 12:20 PM, Paul Williamson <paul () mustbeart com>
wrote:

> I want to add fragmentation reassembly to the dissector I'm improving for
> DVB-S2-BB. To get things exactly right, I'll need to reassemble in a way
> that doesn't seem to be supported by the existing code in
> epan/reassemble.[ch]. What approach is considered most preferable?
>
> 1. Modify epan/reassemble.[ch] to do what I need...
>
> If you're interested in the details, here are some. The protocol I'm
> dissecting is an encapsulation protocol running encapsulated in UDP.
> Fragments are tagged as first, last, or in between, and arrive in order
> without a sequence number, so the existing function fragment_add_seq_next()
> is close to what I need. I also need to check whether a first fragment has
> already been received before adding a last or in-between fragment, and I
> think I can do that with fragment_get().
>
> However, I also need to age out old fragments. There isn't currently a
> function like fragment_add_seq_next_aging() in the code, but perhaps it
> would be easy enough to add. Unfortunately, the existing aging mechanism in
> epan/reassemble.[ch] is based on pinfo->num, and to comply with the spec I
> need it to be based on a count of frames of my specific protocol.This is
> where I think I'd need to modify epan/reassemble.[ch].

Hi Paul,

When I recently wrote the add_seq_single* functions because of PPP
Multilink Protocol (though a number of other protocols use a similar method
that just isn't handled now), I asked a similar question before starting,
and got the recommendation on this list to modify epan/reassemble.[ch] to
add necessary functions. After all, it's quite possible that other
protocols will come along that will need the same functions.

Regarding aging, I wrote the _single_aging function because it was
necessary to make things work especially with the short sequence number
option causing overflow. It's not defined in the spec, so I picked
pinfo->num simply because that was the easiest thing to implement without
changing the existing structure. Looking specifically at the packet count
for the corresponding protocol only would almost surely be better, just a
little more work. For that reason, if you implemented it well I would be up
for changing the existing function, especially since there's only the one
dissector that uses it yet last I looked. That said, the dissector will
probably work in most practical cases without doing so, but not be quite
compliant with the spec.

John Thacker

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