On 2 Apr 2015, at 21:54, Hans Petter Selasky <h...@selasky.org> wrote:
>>> Please go read about how IP fragmentation works. Having an identical IP
>>> ID in ip_fragment() is the point of the function!
>>
>> rwatson: You're right, the more fragment flag gets set there, I
>> overlooked that bit. Sorry.
>>
>> glebius: Given that you admit there is a small chance of an IP ID
>> collision in the previous e-mails exchanged in this thread, why don't we
>> have checks for that in ip_reass() when receiving fragmented IP packets?
>> For example when ip->ip_off == 0 we know the TCP and/or UDP port numbers
>> for TCP and UDP payloads and can check if a new fragment is starting
>> before the previous one is completed. Then we would know if a collision
>> has happened and could discard that packet. Not ideal, but better than
>> data corruption.
>
> I see from the code that if two frags have the same IP offset, the whole
> fragment list gets dropped, unless the IP payload is zero bytes long. Maybe a
> "last" variable should be added?
Are you solving an actual problem you've observed, or is this a speculative
proposal? RFC 791 requires that the minimum fragment size be 8 octets, and
ip_input() drops fragments below that size, so you shouldn't (in principle) be
able to get a fragment in the reassembly code that has the properties you've
described. If you are, it's a bug elsewhere in the stack. If you are worried,
you could add an assertion at the top of the reassembly function that the size
is >= 8 octets.
I think you would benefit a great deal from reading Stevens Volume I (second
edition) before proceeding with further changes to the TCP/IP code stack. A
number of the proposals you have made are contradictory to fundamental design
choices in the protocol. Although there isn't a second edition of Volume II, it
might still be a good idea to skim through the pertinent sections there as well
-- it applies to a much (much) earlier version of the stack, but includes a
detailed exposition of how the implementation tracks the protocol.
Robert
>> * only n will ever be stored. (n = maxfragsperpacket.)
>> *
>> */
>> next = 0;
> last = -1;
>> for (p = NULL, q = fp->ipq_frags; q; p = q, q = q->m_nextpkt) {
>> if (ntohs(GETIP(q)->ip_off) != next ||
> + ntohs(GETIP(q)->ip_off) == last
>> ) {
>> if (fp->ipq_nfrags > V_maxfragsperpacket) {
>> IPSTAT_ADD(ips_fragdropped, fp->ipq_nfrags);
>> ip_freef(head, fp);
>> }
>> goto done;
>> }
> last = next;
>> next += ntohs(GETIP(q)->ip_len);
>> }
>
> --HPS
>
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
