netdev - Re: [RFC PATCH 1/9] ipv4/GRO: Allow multiple frames to use the same IP ID

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEh+42iAWwKFLuorcOEM+=uUr0qOkwJWv9bE6Vvhyv4j9m3wrw@mail.gmail.com>
Date:	Sun, 27 Mar 2016 21:57:55 -0700
From:	Jesse Gross <jesse@...nel.org>
To:	Alexander Duyck <alexander.duyck@...il.com>
Cc:	Alexander Duyck <aduyck@...antis.com>,
	Edward Cree <ecree@...arflare.com>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	David Miller <davem@...emloft.net>,
	Tom Herbert <tom@...bertland.com>
Subject: Re: [RFC PATCH 1/9] ipv4/GRO: Allow multiple frames to use the same
 IP ID

On Wed, Mar 23, 2016 at 7:21 PM, Alexander Duyck
<alexander.duyck@...il.com> wrote:
> On Wed, Mar 23, 2016 at 6:43 PM, Jesse Gross <jesse@...nel.org> wrote:
>> On Fri, Mar 18, 2016 at 4:24 PM, Alexander Duyck <aduyck@...antis.com> wrote:
>>> In RFC 6864 it is stated that we can essentially ignore the IPv4 ID field
>>> if we have not and will not use fragmentation.  Such a frame is defined
>>> as having the DF flag set to 1, and the MF and frag_offset as 0.  Currently
>>> for GRO we were requiring that the inner header always have an increasing
>>> IPv4 ID, but we are ignoring the outer value.
>>>
>>> This patch is a first step in trying to reverse some of that.  Specifically
>>> what this patch does is allow us to coalesce frames that have a static IPv4
>>> ID value.  So for example if we had a series of frames where the DF flag
>>> was set we would allow the same IPv4 ID value to be used for all the frames
>>> belonging to that flow.  This would become the standard behavior for TCP so
>>> it would support either a fixed IPv4 ID value, or one in which the value
>>> increments.
>>>
>>> Signed-off-by: Alexander Duyck <aduyck@...antis.com>
>>
>> One thing that is a bit odd is that the TSO output procedure stays the
>> same. So that means that if we get a stream of packets with the DF bit
>> set and a constant IP ID, aggregate them with GRO, and the retransmit
>> with GSO/TSO then we'll get packets with IDs that increment for each
>> burst and then start back again to the original value. I guess it
>> doesn't matter in practice since the IDs are supposed to be ignored
>> but it does seem a little strange - especially because the new packets
>> will now be violating the rules of the same GRO implementation that
>> produced them.
>
> Yes and no.  The rule for GRO with this patch is that the IP ID has to
> be either incrementing or if DF is set it has the option to be a fixed
> value for a given grouping of packets.  In that regard either GSO
> partial or standard GSO are still both reversible via GRO so that you
> can aggregate to get back to the original frame (ignoring the IP ID)
> that GSO segmented.  The bit I am still trying to work out is what to
> do about the case where we GRO 2 frames out of one GSO segment.  I
> wonder if I should just totally ignore the IP ID value since it ends
> up creating an artificial boundary between the two frames if they are
> segmented using the incrementing GSO versus the fixed IP ID GSO.

Yeah, I agree that it should work in practice, it just seems a bit odd
to have the IP IDs skip around like that. It does also mean that GRO
will no longer be completely, transparently reversible.

I guess that's fine though as long as we fully embrace the idea that
the DF bit means that IP IDs are not used. In that case, it seems best
to allow any ID when DF is set so we are at least self-consistent.