Date: Thu, 02 Apr 2015 22:54:54 +0200 From: Hans Petter Selasky <hps@selasky.org> To: Robert Watson <rwatson@FreeBSD.org> Cc: Mateusz Guzik <mjguzik@gmail.com>, Ian Lepore <ian@freebsd.org>, svn-src-all@freebsd.org, src-committers@freebsd.org, Gleb Smirnoff <glebius@FreeBSD.org>, svn-src-head@freebsd.org Subject: Re: svn commit: r280971 - in head: contrib/ipfilter/tools share/man/man4 sys/contrib/ipfilter/netinet sys/netinet sys/netipsec sys/netpfil/pf Message-ID: <551DAC9E.9010303@selasky.org> In-Reply-To: <551DA5EA.1080908@selasky.org> References: <201504012226.t31MQedN044443@svn.freebsd.org> <1427929676.82583.103.camel@freebsd.org> <20150402123522.GC64665@FreeBSD.org> <20150402133751.GA549@dft-labs.eu> <20150402134217.GG64665@FreeBSD.org> <20150402135157.GB549@dft-labs.eu> <1427983109.82583.115.camel@freebsd.org> <20150402142318.GC549@dft-labs.eu> <20150402143420.GI64665@FreeBSD.org> <20150402153805.GD549@dft-labs.eu> <alpine.BSF.2.11.1504021657440.27263@fledge.watson.org> <551D8143.4060509@selasky.org> <551D8945.8050906@selasky.org> <8900318B-8155-4131-A0C3-3DE169782EFC@FreeBSD.org> <551D8C6C.9060504@selasky.org> <alpine.BSF.2.11.1504021939390.64391@fledge.watson.org> <551DA5EA.1080908@selasky.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 04/02/15 22:26, Hans Petter Selasky wrote:
> On 04/02/15 20:46, Robert Watson wrote:
>> On Thu, 2 Apr 2015, Hans Petter Selasky wrote:
>>
>>>>> Does somebody here know what happens in these two cases:
>>>>>
>>>>> If we are transmitting using TSO, will the network adapter increment
>>>>> the IP ID field somehow? What happens if an outgoing IP packet
>>>>> resulting from a TSO packet get fragmented by a router?
>>>>
>>>> Quite possibly -- this is presumably specified by the NIC vendor, but
>>>> it would be good to do a bit of a survey and see what happens in
>>>> practice.
>>>>
>>>>> In ip_fragment() when we create fragments we should increment the
>>>>> ip_id value for each fragment?
>>>
>>> I'm asking because the code in FreeBSD, since the beginning probably,
>>> just copies the IP header, and use the same IP ID for all the
>>> fragments ! This just hit my mind after some recent work in this area.
>>
>> I honestly cannot believe you are proposing that.
>>
>> Please go read about how IP fragmentation works. Having an identical IP
>> ID in ip_fragment() is the point of the function!
>>
>
> Hi,
>
> 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.
>
Hi,
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?
> * 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?551DAC9E.9010303>