netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* IPV4: Enable IP_ID sequencing for all traffic (inquiring minds want to know why its set to zero)
@ 2008-07-16 14:56 Christoph Lameter
  2008-07-16 19:03 ` David Stevens
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Christoph Lameter @ 2008-07-16 14:56 UTC (permalink / raw)
  To: netdev


There is some security reason why we set IP_ID == 0 right? Could someone point me at the discussions that lead to having IP_ID be zero? Have not been involved in network that much so forgive my ignorance.... and I can only find some hints about why this was done here and there using google.



Here is a patch that would reenable IP_ID sequencing


Subject: [IPV4] Enable IP_ID sequencing on all traffic

The current Linux sources disable IP_ID if the DF flag is set (not always. There
is an exception for VJ compression on certain windows platforms).

Just remove the check for the DF flag and always generate the ipid.

Signed-off-by: Christoph Lameter <cl@linux-foundation.org>

Index: linux-2.6/include/net/ip.h
===================================================================
--- linux-2.6.orig/include/net/ip.h	2008-07-16 08:47:50.000000000 -0500
+++ linux-2.6/include/net/ip.h	2008-07-16 08:48:01.000000000 -0500
@@ -217,16 +217,7 @@
 
 static inline void ip_select_ident(struct iphdr *iph, struct dst_entry *dst, struct sock *sk)
 {
-	if (iph->frag_off & htons(IP_DF)) {
-		/* This is only to work around buggy Windows95/2000
-		 * VJ compression implementations.  If the ID field
-		 * does not change, they drop every other packet in
-		 * a TCP stream using header compression.
-		 */
-		iph->id = (sk && inet_sk(sk)->daddr) ?
-					htons(inet_sk(sk)->id++) : 0;
-	} else
-		__ip_select_ident(iph, dst, 0);
+	__ip_select_ident(iph, dst, 0);
 }
 
 static inline void ip_select_ident_more(struct iphdr *iph, struct dst_entry *dst, struct sock *sk, int more)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: IPV4: Enable IP_ID sequencing for all traffic (inquiring minds want to know why its set to zero)
  2008-07-16 14:56 IPV4: Enable IP_ID sequencing for all traffic (inquiring minds want to know why its set to zero) Christoph Lameter
@ 2008-07-16 19:03 ` David Stevens
  2008-07-16 19:06   ` David Stevens
  2008-07-16 20:27 ` IPV4: Enable IP_ID sequencing for all traffic (inquiring minds want " David Miller
  2008-07-17 16:12 ` Andi Kleen
  2 siblings, 1 reply; 11+ messages in thread
From: David Stevens @ 2008-07-16 19:03 UTC (permalink / raw)
  To: Christoph Lameter; +Cc: netdev, netdev-owner

IP_ID is needed for reassembly of fragments, but a fast network can
go through all the ID's available before datagrams you've already sent
have reached their time to live.

That means you can end up with two different datagrams with the same ID
and incorrectly reassemble parts of them together (which you can hope
the checksum will catch and cause a drop).

By not using IDs for non-fragmented datagrams, the time to wrap the ID
space is increased, which reduces the chance of getting into that
situation.

That really is the only protocol use of IP_IDs, so there isn't any point
in consuming them for non-fragment datagrams. It's useful sometimes
when looking at packet traces to correlate pieces on two different
networks, which I'm guessing is why you want it, but I don't think it's
a good idea to do it all the time.

                                                +-DLS


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: IPV4: Enable IP_ID sequencing for all traffic (inquiring minds want to know why its set to zero)
  2008-07-16 19:03 ` David Stevens
@ 2008-07-16 19:06   ` David Stevens
  2008-07-16 19:22     ` Christoph Lameter
  0 siblings, 1 reply; 11+ messages in thread
From: David Stevens @ 2008-07-16 19:06 UTC (permalink / raw)
  To: David Stevens; +Cc: Christoph Lameter, netdev, netdev-owner

Oops, I didn't see all of your message when I replied....

But the explanation is still true. I don't think we should put an ID
on every packet.

                                                +-DLS


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: IPV4: Enable IP_ID sequencing for all traffic (inquiring minds want to know why its set to zero)
  2008-07-16 19:06   ` David Stevens
@ 2008-07-16 19:22     ` Christoph Lameter
  2008-07-16 20:30       ` David Miller
  0 siblings, 1 reply; 11+ messages in thread
From: Christoph Lameter @ 2008-07-16 19:22 UTC (permalink / raw)
  To: David Stevens; +Cc: netdev, netdev-owner

David Stevens wrote:
> Oops, I didn't see all of your message when I replied....
> 
> But the explanation is still true. I don't think we should put an ID
> on every packet.

Wasnt there a time when we did IP_ID on every packet? Any changelogs from that time?


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: IPV4: Enable IP_ID sequencing for all traffic (inquiring minds want to know why its set to zero)
  2008-07-16 14:56 IPV4: Enable IP_ID sequencing for all traffic (inquiring minds want to know why its set to zero) Christoph Lameter
  2008-07-16 19:03 ` David Stevens
@ 2008-07-16 20:27 ` David Miller
  2008-07-17 16:12 ` Andi Kleen
  2 siblings, 0 replies; 11+ messages in thread
From: David Miller @ 2008-07-16 20:27 UTC (permalink / raw)
  To: cl; +Cc: netdev

From: Christoph Lameter <cl@linux-foundation.org>
Date: Wed, 16 Jul 2008 09:56:21 -0500

> There is some security reason why we set IP_ID == 0 right? Could
> someone point me at the discussions that lead to having IP_ID be
> zero? Have not been involved in network that much so forgive my
> ignorance.... and I can only find some hints about why this was done
> here and there using google.

As David mentioned, one reason is that the less you allocate the less often
ID wrap occurs, which is a very real issue on high speed networks.

I think a more important question is what do you want it re-enabled?
You must have stumbled upon this for a reason. :-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: IPV4: Enable IP_ID sequencing for all traffic (inquiring minds want to know why its set to zero)
  2008-07-16 19:22     ` Christoph Lameter
@ 2008-07-16 20:30       ` David Miller
  2008-07-16 20:39         ` Rick Jones
  0 siblings, 1 reply; 11+ messages in thread
From: David Miller @ 2008-07-16 20:30 UTC (permalink / raw)
  To: cl; +Cc: dlstevens, netdev, netdev-owner

From: Christoph Lameter <cl@linux-foundation.org>
Date: Wed, 16 Jul 2008 14:22:19 -0500

> Wasnt there a time when we did IP_ID on every packet? Any changelogs from that time?

The current behavior existed way before GIT and BK so this might be
hard to come by.

A quick check shows that 2.2.x does an unconditional iph->id = ip_id_count++
so you can work forwards from there :-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: IPV4: Enable IP_ID sequencing for all traffic (inquiring minds want to know why its set to zero)
  2008-07-16 20:30       ` David Miller
@ 2008-07-16 20:39         ` Rick Jones
  2008-07-16 21:01           ` David Miller
  0 siblings, 1 reply; 11+ messages in thread
From: Rick Jones @ 2008-07-16 20:39 UTC (permalink / raw)
  To: David Miller; +Cc: cl, dlstevens, netdev, netdev-owner

I have vague recollections of wanting to avoid (then) locks on the 
increment in the non-fragmented paths, assertions that covering cases 
like intermediate devices ignoring DF and fragmenting anyway didn't 
warrant concern etc.

rick jones

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: IPV4: Enable IP_ID sequencing for all traffic (inquiring minds want to know why its set to zero)
  2008-07-16 20:39         ` Rick Jones
@ 2008-07-16 21:01           ` David Miller
  2008-07-16 21:16             ` IPV4: Enable IP_ID sequencing for all traffic (inquiring mindswant " Templin, Fred L
  0 siblings, 1 reply; 11+ messages in thread
From: David Miller @ 2008-07-16 21:01 UTC (permalink / raw)
  To: rick.jones2; +Cc: cl, dlstevens, netdev, netdev-owner

From: Rick Jones <rick.jones2@hp.com>
Date: Wed, 16 Jul 2008 13:39:37 -0700

> I have vague recollections of wanting to avoid (then) locks on the 
> increment in the non-fragmented paths, assertions that covering cases 
> like intermediate devices ignoring DF and fragmenting anyway didn't 
> warrant concern etc.

Partially, yes.

If you look, we do increment the ID for TCP connections which will
be setting IP_DF, but we allocate such IDs in a socket-local manner
which keeps these allocations from depleating the real ID cache
for that destination.

This is legal because when IP_DF is set, the IDs serve no purpose.

We had to do this because TCP header compression, used by SLIP et
al., depends upon the ID value incrementing.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: IPV4: Enable IP_ID sequencing for all traffic (inquiring mindswant to know why its set to zero)
  2008-07-16 21:01           ` David Miller
@ 2008-07-16 21:16             ` Templin, Fred L
  2008-07-16 22:37               ` Christoph Lameter
  0 siblings, 1 reply; 11+ messages in thread
From: Templin, Fred L @ 2008-07-16 21:16 UTC (permalink / raw)
  To: David Miller, rick.jones2; +Cc: cl, dlstevens, netdev, netdev-owner


 >-----Original Message-----
>From: David Miller [mailto:davem@davemloft.net] 
>Sent: Wednesday, July 16, 2008 2:01 PM
>To: rick.jones2@hp.com
>Cc: cl@linux-foundation.org; dlstevens@us.ibm.com; 
>netdev@vger.kernel.org; netdev-owner@vger.kernel.org
>Subject: Re: IPV4: Enable IP_ID sequencing for all traffic 
>(inquiring mindswant to know why its set to zero)
>
>From: Rick Jones <rick.jones2@hp.com>
>Date: Wed, 16 Jul 2008 13:39:37 -0700
>
>> I have vague recollections of wanting to avoid (then) locks on the 
>> increment in the non-fragmented paths, assertions that 
>covering cases 
>> like intermediate devices ignoring DF and fragmenting anyway didn't 
>> warrant concern etc.
>
>Partially, yes.
>
>If you look, we do increment the ID for TCP connections which will
>be setting IP_DF, but we allocate such IDs in a socket-local manner
>which keeps these allocations from depleating the real ID cache
>for that destination.
>
>This is legal because when IP_DF is set, the IDs serve no purpose.

The other implied use for IP_ID is as a uniquifier for
duplicate packet detection (DPD), e.g., to detect routing
loops in the network. But, 16 bits doesn't give sufficient
uniqueness for today's data rates anway, so flooding-based
protocols like Simplified Multicast Forwarding (SMF) really
can't use IP_ID by itself for that purpose.

SEAL extends the IP_ID to 32 bits. With 32 bits, it makes
sense to set it as monotonically-incrementing for every
packet out since a network-based DPD mechanism could
potentially make use of it. Also, 32 bits avoids the ID
wraparound issue such that a segmentation and reassembly
mechanism can be used even at high data rates.

SEAL is specified in 'draft-templin-seal', and is also
implemented at:

  http://osprey67.com/seal

This is linux kernel code, but it is not yet ready to be
submitted to netdev. It would be good to get some extra
eyes on the code to get some independent verification
testing and to weed out any bugs. If anyone is interested
in checking it out, please let me know.

Thanks - Fred
fred.l.templin@boeing.com 

>We had to do this because TCP header compression, used by SLIP et
>al., depends upon the ID value incrementing.
>
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: IPV4: Enable IP_ID sequencing for all traffic (inquiring mindswant to know why its set to zero)
  2008-07-16 21:16             ` IPV4: Enable IP_ID sequencing for all traffic (inquiring mindswant " Templin, Fred L
@ 2008-07-16 22:37               ` Christoph Lameter
  0 siblings, 0 replies; 11+ messages in thread
From: Christoph Lameter @ 2008-07-16 22:37 UTC (permalink / raw)
  To: Templin, Fred L
  Cc: David Miller, rick.jones2, dlstevens, netdev, netdev-owner

Templin, Fred L wrote:

> The other implied use for IP_ID is as a uniquifier for
> duplicate packet detection (DPD), e.g., to detect routing
> loops in the network. But, 16 bits doesn't give sufficient
> uniqueness for today's data rates anway, so flooding-based
> protocols like Simplified Multicast Forwarding (SMF) really
> can't use IP_ID by itself for that purpose.

Well yes that was how the issue came about.

> SEAL extends the IP_ID to 32 bits. With 32 bits, it makes
> sense to set it as monotonically-incrementing for every
> packet out since a network-based DPD mechanism could
> potentially make use of it. Also, 32 bits avoids the ID
> wraparound issue such that a segmentation and reassembly
> mechanism can be used even at high data rates.
> 
> SEAL is specified in 'draft-templin-seal', and is also
> implemented at:
> 
>   http://osprey67.com/seal

Ahh... interesting. Maybe that would help. I will have a look at that.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: IPV4: Enable IP_ID sequencing for all traffic (inquiring minds want to know why its set to zero)
  2008-07-16 14:56 IPV4: Enable IP_ID sequencing for all traffic (inquiring minds want to know why its set to zero) Christoph Lameter
  2008-07-16 19:03 ` David Stevens
  2008-07-16 20:27 ` IPV4: Enable IP_ID sequencing for all traffic (inquiring minds want " David Miller
@ 2008-07-17 16:12 ` Andi Kleen
  2 siblings, 0 replies; 11+ messages in thread
From: Andi Kleen @ 2008-07-17 16:12 UTC (permalink / raw)
  To: Christoph Lameter; +Cc: netdev

Christoph Lameter <cl@linux-foundation.org> writes:

> There is some security reason why we set IP_ID == 0 right? Could someone point me at the discussions that lead to having IP_ID be zero? Have not been involved in network that much so forgive my ignorance.... and I can only find some hints about why this was done here and there using google.

AFAIK it was just for efficiency. The way Linux generates ipids
currently is somewhat expensive, because it aims to generate a per ip
ipid counter, so it was turned off when not needed. I used to patch
that out because it always seemed like overkill to me.

-Andi

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2008-07-17 16:13 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-16 14:56 IPV4: Enable IP_ID sequencing for all traffic (inquiring minds want to know why its set to zero) Christoph Lameter
2008-07-16 19:03 ` David Stevens
2008-07-16 19:06   ` David Stevens
2008-07-16 19:22     ` Christoph Lameter
2008-07-16 20:30       ` David Miller
2008-07-16 20:39         ` Rick Jones
2008-07-16 21:01           ` David Miller
2008-07-16 21:16             ` IPV4: Enable IP_ID sequencing for all traffic (inquiring mindswant " Templin, Fred L
2008-07-16 22:37               ` Christoph Lameter
2008-07-16 20:27 ` IPV4: Enable IP_ID sequencing for all traffic (inquiring minds want " David Miller
2008-07-17 16:12 ` Andi Kleen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).