Bugzilla – Bug 1955
The IP identification field should be unique per (source, destination, protocol) tuple
Last modified: 2014-07-20 05:32:40 UTC
For the Ipv4 headers and the protocol IPv4L3Protocol, the ip identification field is currently just an incrementing counter irrespective of the source and destination. According to RFC 6864, I believe that each source, destination, and protocol tuple should have its own IP identification space. Failure to conform to this part of the spec has caused me some problems when attempting to implement Robust Header Compression, ROHC, which assumes the above is correct. I will attempt to create a patch for this problem shortly.
This bug is a followup of bug #1817 In 3.20 the bug was fixed, but only considering the protocol. Source/destination addresses were "forgotten". The obvious patch is to use a more complex map to track the tuples. However, the simplest thing is to use a uint128_t key. 2 addresses and a protocol (32+32+8 bits) can safely be represented by a 128 bit integer. In this way the patch is straightforward. T.
For the records, this is the relevant part of RFC 6864: 4.3. IPv4 ID Requirements That Persist This document does not relax the IPv4 ID field uniqueness requirements of [RFC791] for non-atomic datagrams, that is: >> Sources emitting non-atomic datagrams MUST NOT repeat IPv4 ID values within one MDL for a given source address/destination address/protocol tuple.
Created attachment 1856 [details] Patch for ip identification. I've attached a patch with the proposed fixes. I haven't checked whether the tests still pass, but I imagine they will not, since they require new .pcap files? Hopefully the patch looks OK.
(In reply to l.salameh from comment #3) > Created attachment 1856 [details] > Patch for ip identification. > > I've attached a patch with the proposed fixes. I haven't checked whether > the tests still pass, but I imagine they will not, since they require new > .pcap files? Hopefully the patch looks OK. +1 and yes, python bindings and .pcap traces will have to be regenerated.
Pushed a slightly modified patch version in changeset: 10847:e72b8f4c25bb The changes with respect to the original patch are: 1) no need to add one more function used by just one place. 2) use std::pair instead of uint128_t T. PS: the test regression traces have been rebuilt.