Re: reassembly, addresses, hash calculation

[Evan Huus <eapache () gmail com>]

Those lines were added earlier this year by Guy Martin (who I've
explicitly copied on this email). The justification in the commit
message at the time was:

If an mp2t packet contains one full subpacket and the fragment of
another one, it happens that the first subpacket will set src or dst to
an ethernet or IP address. Adding the fragment of the second subpacket
will then use this information for calculating the hash in the fragment
table. However, later fragments in other mp2t packets will not have
these info and reassembly will fail.

(For those not familiar with git, I found this information via 'git
blame' which works quite similarly to 'svn blame' and 'cvs annotate').

Evan

On Sun, Oct 5, 2014 at 6:56 AM, Martin Kaiser <lists () kaiser cx> wrote:
> Hi,
>
> when I looked at bug 10505, I noticed the following code in
> packet-mp2t.c
>
>     /* It's possible that a fragment in the same packet set an address already
>      * This will change the hash value, we need to make sure it's NULL */
>
>     SET_ADDRESS_HF(&pinfo->src, AT_NONE, 0, NULL, 0);
>     SET_ADDRESS_HF(&pinfo->dst, AT_NONE, 0, NULL, 0);
>
>     [...]
>     frag_msg = fragment_add_check(&mp2t_reassembly_table,
>             tvb, offset, pinfo, frag_id, NULL,
>             [...]
>
>
> Is it really necessary to clear the addresses here in order to make
> reassembly work correctly?
>
> Without looking at the reassembly mechanism, I'd say that lower-layer
> addresses should be taken into account and mpeg ts packets from
> different src/dst addresses shouldn't be reassembled.
>
> mp2t_reassembly_table is initialized with
> addresses_reassembly_table_functions. One of these is
> fragment_addresses_hash(), which doesn't use the addresses at all.
> However, the comparison and key creation functions do use src and dst
> addresses...
>
> I'm still tempted to remove the lines where the addresses are cleared.
>
> Thoughts?
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>              mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe