Bugzilla – Bug 1817
IPv4 Identification field should consider protocol as well.
Last modified: 2014-04-29 04:08:18 EDT
Created attachment 1746 [details]
RFC 6864 clearly states
>> The IPv4 ID field MUST NOT be used for purposes other than
fragmentation and reassembly.
The proposed patch fixes that.
A further analysis is needed to enforce the ID uniqueness within the maximum datagram lifetime (MDL).
BTW, this patch will require the regressions tests to be rebuilt.
Also, I have no idea id DSR is affected by this. DsrOptionSR::Process() is using the IP header ID field, but it should be a safe use.
(In reply to Tommaso Pecorella from comment #0)
> Created attachment 1746 [details]
> RFC 6864 clearly states
> >> The IPv4 ID field MUST NOT be used for purposes other than
> fragmentation and reassembly.
> The proposed patch fixes that.
I'm not sure that I agree with this interpretation (that this patch fixes ID assignment). The statement above pertains to using (not generating) IPv4 ID field (as it using it for things like duplicate packet detection).
RFC 6864 specifies that the ID should be set to a unique value for packets that may be fragmented in the future. It also says that it doesn't care about what value is assigned to packets that can't be fragmented. Note that it doesn't complain if the same process is used for both types of packets.
The current code (monotonically increasing sequence number for all) seems to already loosely follow this guidance. It has two limitations:
- it burns sequence numbers on packets that don't need a unique ID
- it does not track on a per-session (src, dst, protocol) basis, but instead for all datagrams, thereby causing cycling of the 16-bit space more quickly than necessary
However, I don't see how the proposed patch improves upon this; if anything, it makes it harder to track uniqueness properties.
> A further analysis is needed to enforce the ID uniqueness within the maximum
> datagram lifetime (MDL).
It seems that a monotonically increasing number is easier to enforce this than a random number.
Can you produce a failing test case of fragmentation due to IP ID generation problems (that the patch then fixes)? Otherwise, I might suggest to mark as WONTFIX or else work on an implementation that tracks uniqueness separately across (src,dst,proto) combinations.
After re-reading carefully RFC 6864, I agree.
>> Sources emitting non-atomic datagrams MUST NOT repeat IPv4 ID
values within one MDL for a given source address/destination
As a result, the patch is invalid, and a more elaborate patch is needed.
Created attachment 1762 [details]
It turns out that the patch is far easier than expected :)
A map is, by default, returning zero for a non-existent key. I love maps.
Created attachment 1830 [details]
patch + fresh regression pcaps
This patch includes fresh pcaps for AODV and OLSR. The tests were failing due to the different IP id numbers.