* Re: [RFC/PATCH] "strict" ipv4 reassembly
From: John Heffner @ 2005-05-17 19:31 UTC (permalink / raw)
To: Nivedita Singhvi, netdev; +Cc: Rick Jones
In-Reply-To: <428A452B.2010008@us.ibm.com>
On Tuesday 17 May 2005 03:25 pm, Nivedita Singhvi wrote:
> Yes, a different manifestation of the problem ;). I was talking
> in the context of current Linux code and the common ethernet drivers
> and typical current Internet/Intranet latencies.
The problem is latency independent.
-John
^ permalink raw reply
* Re: [RFC/PATCH] "strict" ipv4 reassembly
From: Ben Greear @ 2005-05-17 19:26 UTC (permalink / raw)
To: David S. Miller; +Cc: jheffner, ak, netdev, akepner
In-Reply-To: <20050517.120950.74749758.davem@davemloft.net>
David S. Miller wrote:
> From: John Heffner <jheffner@psc.edu>
> Date: Tue, 17 May 2005 14:57:38 -0400
>
>
>>It would be better still to have a per-route packet reassembly timeout in
>>milliseconds.
>
>
> I agree. And if we can setup the infrastructure such that the
> drivers can indicate the speed of the link they are communicating
> on, then we can set sane default values on the automatically
> created subnet routes.
I assume you mean more than local physical link speed? Imagine:
GigE server -- gige-core -- 10bt hub -- gige-core -- gige client
Now, this particular setup would be lame, but the 10bt hub could
really be a bridged WAN, wireless or other slow and not-so-easily
upgraded network link.
Local link speed is relatively easy to figure out using ethtool for the
NICs that support it at least....
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [RFC/PATCH] "strict" ipv4 reassembly
From: Nivedita Singhvi @ 2005-05-17 19:25 UTC (permalink / raw)
To: Rick Jones; +Cc: netdev
In-Reply-To: <428A425F.7000807@hp.com>
Rick Jones wrote:
>
>> This is a fast LAN problem - real internet latencies
>> don't allow for the wrapping of the id field that fast.
>
>
> What is a "fast" LAN these days? We were seeing IP ID wrap with NFS
> traffic back in the time of HP-UX 8.07 (early 1990's) over 10 Mbit/s
> Ethernet. Since I'm waxing historic, part of the problem at that time
> was giving the IP fragments to the driver one at a time rather than all
> at once. When the system was as fast or faster than the network, and
> the driver queue filled, the first couple fragments of a datagram would
> get into the queue, and the rest would be dropped, setting the stage for
> those wonderful Frankengrams. Part of the fix was to require a driver
> to take an entire IP datagram's worth of fragments or none of them.
Yes, a different manifestation of the problem ;). I was talking
in the context of current Linux code and the common ethernet drivers
and typical current Internet/Intranet latencies.
> And there was at least one CKO NIC from that era (the HP-PB FDDI NIC)
> where the first IP datagram fragment was sent last :)
>
> If the WAN connected system is (still?) using a global IP ID rather than
> per-route, it could quite easily be wrapping. And we have WAN links
> with bandwidths of 10's of Megabits, so it also comes-down to how much
> other traffic is going and the quantity of request parallelism in NFS
> right?
Actually, the problem is much worse now - we have virtual
partitions in the Xen environment for instance where some
packets are headed for local consumption (virtual network,
no actual network latency to speak of) and some going
out to the network. Having a global IP id generator just
won't be able to keep up - we could wrap in submilliseconds...
> The larger NFS UDP mount sizes mean more fragments, but intriguingly,
> they also mean slower wrap of the IP ID space :)
True, but in a 32 NIC environment, see how they wrap ;)...
thanks,
Nivedita
^ permalink raw reply
* Re: [RFC/PATCH] "strict" ipv4 reassembly
From: Rick Jones @ 2005-05-17 19:21 UTC (permalink / raw)
To: netdev
In-Reply-To: <20050517.120950.74749758.davem@davemloft.net>
David S. Miller wrote:
> From: John Heffner <jheffner@psc.edu>
> Date: Tue, 17 May 2005 14:57:38 -0400
>
>
>>It would be better still to have a per-route packet reassembly timeout in
>>milliseconds.
>
>
> I agree. And if we can setup the infrastructure such that the
> drivers can indicate the speed of the link they are communicating
> on, then we can set sane default values on the automatically
> created subnet routes.
Does the ingress link really tell us all that much about the path a given
datagram's fragments took to get to us? Even if the source IP is ostensibly a
local one?
rick jones
^ permalink raw reply
* Re: [RFC/PATCH] "strict" ipv4 reassembly
From: Andi Kleen @ 2005-05-17 19:17 UTC (permalink / raw)
To: John Heffner; +Cc: David S. Miller, netdev, akepner
In-Reply-To: <200505171457.38719.jheffner@psc.edu>
John Heffner <jheffner@psc.edu> writes:
>
> It would be better still to have a per-route packet reassembly timeout in
> milliseconds. Expecting perfect ordering seems very fragile even for a LAN.
In fact I implemented that a long time ago for the SUSE 2.4 kernel.
But for some reason nobody liked it very much, so I never submitted it.
I can dig out the old patches if there is interest.
-Andi
^ permalink raw reply
* Re: [RFC/PATCH] "strict" ipv4 reassembly
From: Rick Jones @ 2005-05-17 19:13 UTC (permalink / raw)
To: netdev
In-Reply-To: <428A3F86.1020000@us.ibm.com>
> This is a fast LAN problem - real internet latencies
> don't allow for the wrapping of the id field that fast.
What is a "fast" LAN these days? We were seeing IP ID wrap with NFS traffic
back in the time of HP-UX 8.07 (early 1990's) over 10 Mbit/s Ethernet. Since
I'm waxing historic, part of the problem at that time was giving the IP
fragments to the driver one at a time rather than all at once. When the system
was as fast or faster than the network, and the driver queue filled, the first
couple fragments of a datagram would get into the queue, and the rest would be
dropped, setting the stage for those wonderful Frankengrams. Part of the fix
was to require a driver to take an entire IP datagram's worth of fragments or
none of them.
And there was at least one CKO NIC from that era (the HP-PB FDDI NIC) where the
first IP datagram fragment was sent last :)
If the WAN connected system is (still?) using a global IP ID rather than
per-route, it could quite easily be wrapping. And we have WAN links with
bandwidths of 10's of Megabits, so it also comes-down to how much other traffic
is going and the quantity of request parallelism in NFS right?
The larger NFS UDP mount sizes mean more fragments, but intriguingly, they also
mean slower wrap of the IP ID space :)
rick jones
^ permalink raw reply
* Re: [RFC/PATCH] "strict" ipv4 reassembly
From: David S. Miller @ 2005-05-17 19:09 UTC (permalink / raw)
To: jheffner; +Cc: ak, netdev, akepner
In-Reply-To: <200505171457.38719.jheffner@psc.edu>
From: John Heffner <jheffner@psc.edu>
Date: Tue, 17 May 2005 14:57:38 -0400
> It would be better still to have a per-route packet reassembly timeout in
> milliseconds.
I agree. And if we can setup the infrastructure such that the
drivers can indicate the speed of the link they are communicating
on, then we can set sane default values on the automatically
created subnet routes.
These would need to be refreshed when link state changes, but
we have the mechanics for that kind of device event stuff
already.
^ permalink raw reply
* Re: [RFC/PATCH] "strict" ipv4 reassembly
From: Nivedita Singhvi @ 2005-05-17 19:01 UTC (permalink / raw)
To: Andi Kleen; +Cc: David S. Miller, netdev, akepner
In-Reply-To: <m1zmut7l5q.fsf@muc.de>
Andi Kleen wrote:
> "David S. Miller" <davem@davemloft.net> writes:
>
>
>>From: Arthur Kepner <akepner@sgi.com>
>>Date: Tue, 17 May 2005 09:18:26 -0700 (PDT)
>>
>>
>>> 1) Fragments must arrive in order (or in reverse order) -
>>> out of order fragments are dropped.
>>
>>Even the most simplistic flow over the real internet
>>can get slight packet reordering.
>>
>>Heck, reordering happens on SMP on any network.
>>
>>IP is supposed to be resilient to side effects of network
>>topology, and one such common side effect is packet reordering.
>>It's common, it's fine, and the networking stack deals with it
>>gracefully. Strict reassembly does not.
>
>
> If anything it would be better as a per route flag.
> Then you could set it only for your local network
> where you know Gigabit happens and reordering might
> be avoidable in some cases.
>
> -Andi
>
> P.S.: Arthur I think your arguments would have more
> force if you published the test program that demonstrates the
> corruption.
When we first ran into this, the dropping of
out-of-order fragments and overlapped fragments
was considered by us, but we finally did not
employ it precisely because of the ordering
requirement.
This is a fast LAN problem - real internet latencies
don't allow for the wrapping of the id field that fast.
Reordering does happen frequently on an SMP (this was
a non-NAPI environment, NAPI reduces it quite a bit)
so even local gigabit low latency LANs tend to suffer
from it. You really need to be running on a UP to be
entirely safe.
The problem is exacerbated by NFS mount sizes of at least
4K or 8K - thus running NFS over UDP is just an
environment you have to avoid in any case. That doesn't
take care of the other apps, of course.
So you cannot deploy a solution like this over all
interfaces and all routes - perhaps, as Andi says,
a per-route flag (turned on by the sysadmin when
running on a UP or NAPI case) might help. But you'd
have to do this very carefully.
thanks,
Nivedita
^ permalink raw reply
* Re: [RFC/PATCH] "strict" ipv4 reassembly
From: John Heffner @ 2005-05-17 18:57 UTC (permalink / raw)
To: Andi Kleen; +Cc: David S. Miller, netdev, akepner
In-Reply-To: <m1zmut7l5q.fsf@muc.de>
On Tuesday 17 May 2005 02:38 pm, Andi Kleen wrote:
> "David S. Miller" <davem@davemloft.net> writes:
> > From: Arthur Kepner <akepner@sgi.com>
> > Date: Tue, 17 May 2005 09:18:26 -0700 (PDT)
> >
> >> 1) Fragments must arrive in order (or in reverse order) -
> >> out of order fragments are dropped.
> >
> > Even the most simplistic flow over the real internet
> > can get slight packet reordering.
> >
> > Heck, reordering happens on SMP on any network.
> >
> > IP is supposed to be resilient to side effects of network
> > topology, and one such common side effect is packet reordering.
> > It's common, it's fine, and the networking stack deals with it
> > gracefully. Strict reassembly does not.
>
> If anything it would be better as a per route flag.
> Then you could set it only for your local network
> where you know Gigabit happens and reordering might
> be avoidable in some cases.
It would be better still to have a per-route packet reassembly timeout in
milliseconds. Expecting perfect ordering seems very fragile even for a LAN.
-John
^ permalink raw reply
* Re: [RFC/PATCH] "strict" ipv4 reassembly
From: Rick Jones @ 2005-05-17 18:56 UTC (permalink / raw)
To: netdev
In-Reply-To: <20050517.115020.119241057.davem@davemloft.net>
> I already know it happens, and that IBM has gotten NFS corruption due
> to this problem.
Frankengrams can be fun :)
> Using NFS over UDP is stupid.
>
> I find it ironic to claim that folks are "stuck with UDP over NFS" but
> were able to upgrade their networking technology to gigabit.
It would seem new NIC drivers ship more easily than new NFS/OS bits in some places.
rick jones
^ permalink raw reply
* Re: [RFC/PATCH] "strict" ipv4 reassembly
From: David S. Miller @ 2005-05-17 18:50 UTC (permalink / raw)
To: ak; +Cc: netdev, akepner
In-Reply-To: <m1zmut7l5q.fsf@muc.de>
From: Andi Kleen <ak@muc.de>
Date: Tue, 17 May 2005 20:38:25 +0200
> If anything it would be better as a per route flag.
> Then you could set it only for your local network
> where you know Gigabit happens and reordering might
> be avoidable in some cases.
This is still bogus, on SMP we get packet reordering all the time.
> P.S.: Arthur I think your arguments would have more
> force if you published the test program that demonstrates the
> corruption.
I already know it happens, and that IBM has gotten NFS corruption due
to this problem. Using NFS over UDP is stupid.
I find it ironic to claim that folks are "stuck with UDP over NFS" but
were able to upgrade their networking technology to gigabit.
^ permalink raw reply
* Re: [RFC/PATCH] "strict" ipv4 reassembly
From: David S. Miller @ 2005-05-17 18:48 UTC (permalink / raw)
To: akepner; +Cc: netdev
In-Reply-To: <Pine.LNX.4.61.0505171104030.29021@linux.site>
From: Arthur Kepner <akepner@sgi.com>
Date: Tue, 17 May 2005 11:28:05 -0700 (PDT)
> On Tue, 17 May 2005, David S.Miller wrote:
>
> > Decreasing ipfrag_time is also not an option, because then
> > you break fragmentation for packet radio folks :-)
>
> Different sysctls for different folks....
Can I tell users to call you when they enable the strict
fragmentation and they can no longer talk UDP to
remote sites outside of their subnet, or it breaks
on their heavily SMP machine due to natural system local
packet reordering?
Packet reordering happens on the local machine with SMP.
There is no way to avoid this. And when it triggers your
patch will drop frags on the ground all the time.
If you want to fix things, do it without knowingly breaking
stuff that does currently work.
^ permalink raw reply
* Re: [RFC/PATCH] "strict" ipv4 reassembly
From: Pekka Savola @ 2005-05-17 18:45 UTC (permalink / raw)
To: Andi Kleen; +Cc: David S. Miller, netdev, akepner
In-Reply-To: <m1zmut7l5q.fsf@muc.de>
On Tue, 17 May 2005, Andi Kleen wrote:
> P.S.: Arthur I think your arguments would have more
> force if you published the test program that demonstrates the
> corruption.
If more explicit analytical or theoretical background would be
convincing, look no further than
http://www.watersprings.org/pub/id/draft-mathis-frag-harmful-00.txt
--
Pekka Savola "You each name yourselves king, yet the
Netcore Oy kingdom bleeds."
Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings
^ permalink raw reply
* Re: [RFC/PATCH] "strict" ipv4 reassembly
From: Andi Kleen @ 2005-05-17 18:38 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, akepner
In-Reply-To: <20050517.104947.112621738.davem@davemloft.net>
"David S. Miller" <davem@davemloft.net> writes:
> From: Arthur Kepner <akepner@sgi.com>
> Date: Tue, 17 May 2005 09:18:26 -0700 (PDT)
>
>> 1) Fragments must arrive in order (or in reverse order) -
>> out of order fragments are dropped.
>
> Even the most simplistic flow over the real internet
> can get slight packet reordering.
>
> Heck, reordering happens on SMP on any network.
>
> IP is supposed to be resilient to side effects of network
> topology, and one such common side effect is packet reordering.
> It's common, it's fine, and the networking stack deals with it
> gracefully. Strict reassembly does not.
If anything it would be better as a per route flag.
Then you could set it only for your local network
where you know Gigabit happens and reordering might
be avoidable in some cases.
-Andi
P.S.: Arthur I think your arguments would have more
force if you published the test program that demonstrates the
corruption.
^ permalink raw reply
* Re: [RFC/PATCH] "strict" ipv4 reassembly
From: Arthur Kepner @ 2005-05-17 18:28 UTC (permalink / raw)
To: David S.Miller; +Cc: netdev
In-Reply-To: <20050517.104947.112621738.davem@davemloft.net>
On Tue, 17 May 2005, David S.Miller wrote:
> ....
> IP is supposed to be resilient to side effects of network
> topology, and one such common side effect is packet reordering.
> It's common, it's fine, and the networking stack deals with it
> gracefully. Strict reassembly does not.
>
IP was designed a looong time ago. I think it's reasonable to
make (or at least allow for) some accomodation when networking
bandwidths have gone up by several orders of magnitude. (And
while we wait for IPv6 to catch on ;-)
>
> Sure it's off by default, but isn't it a better idea
> to use NFS over TCP instead?
>
This isn't limited to NFS, of course, though that's the
application of most concern. I know that we have customers
who, for good or bad reasons, _do_ use NFS over UDP.
> Decreasing ipfrag_time is also not an option, because then
> you break fragmentation for packet radio folks :-)
Different sysctls for different folks....
--
Arthur
^ permalink raw reply
* Re: [RFC/PATCH] "strict" ipv4 reassembly
From: David S. Miller @ 2005-05-17 17:49 UTC (permalink / raw)
To: akepner; +Cc: netdev
In-Reply-To: <Pine.LNX.4.61.0505170914130.29021@linux.site>
From: Arthur Kepner <akepner@sgi.com>
Date: Tue, 17 May 2005 09:18:26 -0700 (PDT)
> 1) Fragments must arrive in order (or in reverse order) -
> out of order fragments are dropped.
Even the most simplistic flow over the real internet
can get slight packet reordering.
Heck, reordering happens on SMP on any network.
IP is supposed to be resilient to side effects of network
topology, and one such common side effect is packet reordering.
It's common, it's fine, and the networking stack deals with it
gracefully. Strict reassembly does not.
Sure it's off by default, but isn't it a better idea
to use NFS over TCP instead?
Decreasing ipfrag_time is also not an option, because then
you break fragmentation for packet radio folks :-)
^ permalink raw reply
* Re: Interdomain/interpartition communication without a checksum
From: Jon Mason @ 2005-05-17 16:34 UTC (permalink / raw)
To: David S. Miller; +Cc: jdmason, netdev, niv, habanero
In-Reply-To: <20050516.130737.77060495.davem@davemloft.net>
On Mon, May 16, 2005 at 01:07:37PM -0700, David S. Miller wrote:
> From: Jon Mason <jdmason@us.ibm.com>
> Date: Wed, 11 May 2005 13:35:54 -0500
>
> > I have been working on a Xen project to remove unnecessary TCP/UDP
> > checksums for local (interdomain) communication, while still having the
> > checksum for all external network communication.
>
> I have no objections to this idea. But let's think about
> the implementation.
Fantastic!
What I am attempting to do is identify the packets that have no
checksum, and checksum them (either by hardware or software). There is
already an existing infrastructure for checksumming the packets by
using HW_CSUM.
I've been mostly looking into the bridging case, but I would
like an implimentation which works for every packet forwarding case.
Here are the problems I see (there might be more):
1. skb->h.raw is set to skb->data in netif_receive_skb() (which prevents
the forwarding/backend driver from setting it to the proper value, so it
must be set on the output).
2. skb->ip_summed is set to CHECKSUM_NONE in both routing and bridging,
thereby preventing the forwarding driver from setting CHECKSUM_HW.
I would prefer to set the proper values for skb->h.raw and
skb->ip_summed in the forwarding/backend driver.
> > + /* If packet is forwarded to a device that needs a checksum and not
> > + * checksummed, correct the pointers and enable checksumming in the
> > + * next function.
> > + */
> > + if (!(dev->features & NETIF_F_FWD_NO_CSUM) && skb->csum) {
> > + skb->ip_summed = CHECKSUM_HW;
> > + skb->h.raw = (void *)skb->nh.iph + (skb->nh.iph->ihl * 4);
> > + }
> > +
>
> This means that every packet which the networking tries to checksum
> offload will pass this test, superfluously doing these assignments.
Even worse, every packet which comes from the local system and has a
checksum already will pass this test. I hit this problem last week,
and had to redo the implimentation. Silly mistake on my part, and
very obvious once I went looking for the cause of my problem.
> It also assumes ipv4. ipv6 is possible, and for NETIF_F_CHECKSUM_HW
> any protocol could be creating the packets as this flag indicates
> that the card implements a totally generic 16-bit two's complement
> checksum.
Yes, anything which encapsulates TCP/UDP packets is valid. Good catch.
I'll have to look at way to fix that.
After fixing the former problem, I came up with the code below
(with no fix for the latter problem yet). It was created against the
2.6.11.8 kernel.
Let me know what you think.
Thanks,
Jon
--- ../xen-unstable-pristine/linux-2.6.11-xen0/include/linux/skbuff.h 2005-03-02 01:38:38.000000000 -0600
+++ linux-2.6.11-xen0/include/linux/skbuff.h 2005-05-13 10:43:08.000000000 -0500
@@ -37,6 +37,10 @@
#define CHECKSUM_HW 1
#define CHECKSUM_UNNECESSARY 2
+#define SKB_CLONED 1
+#define SKB_NOHDR 2
+#define SKB_FDW_NO_CSUM 4
+
#define SKB_DATA_ALIGN(X) (((X) + (SMP_CACHE_BYTES - 1)) & \
~(SMP_CACHE_BYTES - 1))
#define SKB_MAX_ORDER(X, ORDER) (((PAGE_SIZE << (ORDER)) - (X) - \
@@ -238,7 +242,7 @@ struct sk_buff {
mac_len,
csum;
unsigned char local_df,
- cloned,
+ flags,
pkt_type,
ip_summed;
__u32 priority;
@@ -370,7 +374,7 @@ static inline void kfree_skb(struct sk_b
*/
static inline int skb_cloned(const struct sk_buff *skb)
{
- return skb->cloned && atomic_read(&skb_shinfo(skb)->dataref) != 1;
+ return (skb->flags & SKB_CLONED) && atomic_read(&skb_shinfo(skb)->dataref) != 1;
}
/**
--- ../xen-unstable-pristine/linux-2.6.11-xen0/net/core/skbuff.c 2005-03-02 01:38:17.000000000 -0600
+++ linux-2.6.11-xen0/net/core/skbuff.c 2005-05-13 11:47:51.000000000 -0500
@@ -240,7 +240,7 @@ static void skb_clone_fraglist(struct sk
void skb_release_data(struct sk_buff *skb)
{
- if (!skb->cloned ||
+ if (!(skb->flags & SKB_CLONED) ||
atomic_dec_and_test(&(skb_shinfo(skb)->dataref))) {
if (skb_shinfo(skb)->nr_frags) {
int i;
@@ -352,7 +352,7 @@ struct sk_buff *skb_clone(struct sk_buff
C(data_len);
C(csum);
C(local_df);
- n->cloned = 1;
+ n->flags = skb->flags | SKB_CLONED;
C(pkt_type);
C(ip_summed);
C(priority);
@@ -395,7 +395,7 @@ struct sk_buff *skb_clone(struct sk_buff
C(end);
atomic_inc(&(skb_shinfo(skb)->dataref));
- skb->cloned = 1;
+ skb->flags |= SKB_CLONED;
return n;
}
@@ -603,7 +603,7 @@ int pskb_expand_head(struct sk_buff *skb
skb->mac.raw += off;
skb->h.raw += off;
skb->nh.raw += off;
- skb->cloned = 0;
+ skb->flags &= SKB_CLONED;
atomic_set(&skb_shinfo(skb)->dataref, 1);
return 0;
--- ../xen-unstable-pristine/linux-2.6.11-xen0/net/core/dev.c 2005-03-02 01:38:09.000000000 -0600
+++ linux-2.6.11-xen0/net/core/dev.c 2005-05-13 11:47:01.000000000 -0500
@@ -98,6 +98,7 @@
#include <linux/stat.h>
#include <linux/if_bridge.h>
#include <linux/divert.h>
+#include <net/ip.h>
#include <net/dst.h>
#include <net/pkt_sched.h>
#include <net/checksum.h>
@@ -1182,7 +1183,7 @@ int __skb_linearize(struct sk_buff *skb,
skb->data += offset;
/* We are no longer a clone, even if we were. */
- skb->cloned = 0;
+ skb->flags &= ~SKB_CLONED;
skb->tail += skb->data_len;
skb->data_len = 0;
@@ -1236,6 +1237,15 @@ int dev_queue_xmit(struct sk_buff *skb)
__skb_linearize(skb, GFP_ATOMIC))
goto out_kfree_skb;
+ /* If packet is forwarded to a device that needs a checksum and not
+ * checksummed, correct the pointers and enable checksumming in the
+ * next function.
+ */
+ if (skb->flags & SKB_FDW_NO_CSUM) {
+ skb->ip_summed = CHECKSUM_HW;
+ skb->h.raw = (void *)skb->nh.iph + (skb->nh.iph->ihl * 4);
+ }
+
/* If packet is not checksummed and device does not support
* checksumming for this protocol, complete checksumming here.
*/
^ permalink raw reply
* [RFC/PATCH] "strict" ipv4 reassembly
From: Arthur Kepner @ 2005-05-17 16:18 UTC (permalink / raw)
To: netdev
The Problem
-----------
There's a well-known problem with IPv4 fragmentation/
reassembly - the 16-bit IP ID can uniquely identify
only 65535 datagrams, and a gigabit/sec source can
emit that many datagrams in seconds. Since fragments
can sit on a reassembly queue for a "long time"
(30 seconds is the default in Linux), there's a
possibility of reassembling fragments from different
datagrams.
The Bigger Problem
------------------
This is mainly a problem for UDP, where IP fragmentation
at the source is not uncommon. The UDP checksum is only
16 bits, so there's a not-insignificant possibility that
it won't detect when a datagram has been incorrectly
reassembled. The result can be silent data corruption,
and much unhappiness.
What next?
----------
This is a fundamental problem with IP(v4), which was
designed before people dreamed of gigabit/sec network
links. There's no simple, completely effective fix that
preserves compatability.
There has been at least a little discussion here
before:
http://marc.theaimsgroup.com/?l=linux-netdev&m=108987122412812&w=2
A simple, obvious, partial remedy is to tune
"sysctl_ipfrag_time" down.
Another simple change which preserves compatability in
practice, is "strict ipv4 reassembly". Being "strict" about
reassembly means that you don't allow gaps or overlaps in
the reassembly queue. Fragments which would introduce gaps
or overlaps are dropped. Disallowing gaps and overlaps also
implies that:
1) Fragments must arrive in order (or in reverse order) -
out of order fragments are dropped.
2) If the first (or last) fragment of a datagram arrives
and there's already a reassembly queue for that (proto,
ipid, src, dst), then the existing reassembly queue is
dropped, and a new one started.
Strict reassembly has been very effective in practice at
preventing incorrect IP reassembly. (We have tests which
attempt to produce and then detect corruption produced
by bad IP reassembly.)
Following is a patch which implements strict reassembly.
Strict reassembly is off by default (so the default
behavior is not changed.) Strict reassembly is enabled
by doing "echo 1 > /proc/sys/net/ipv4/strict_reassembly".
Patch is against 2.6.12-rc4.
include/linux/sysctl.h | 1
net/ipv4/ip_fragment.c | 189 ++++++++++++++++++++++++++++++++++++++++++++-
net/ipv4/sysctl_net_ipv4.c | 9 ++
3 files changed, 198 insertions(+), 1 deletion(-)
Signed-off-by: Arthur Kepner <akepner@sgi.com>
diff -pur linux.old/include/linux/sysctl.h linux.new/include/linux/sysctl.h
--- linux.old/include/linux/sysctl.h 2005-05-16 16:29:55.236056917 -0700
+++ linux.new/include/linux/sysctl.h 2005-05-17 00:57:16.889588271 -0700
@@ -347,6 +347,7 @@ enum
NET_TCP_MODERATE_RCVBUF=106,
NET_TCP_TSO_WIN_DIVISOR=107,
NET_TCP_BIC_BETA=108,
+ NET_IPV4_STRICT_REASM=109,
};
enum {
diff -pur linux.old/net/ipv4/ip_fragment.c linux.new/net/ipv4/ip_fragment.c
--- linux.old/net/ipv4/ip_fragment.c 2005-05-16 16:30:18.610281655 -0700
+++ linux.new/net/ipv4/ip_fragment.c 2005-05-17 00:53:53.243293277 -0700
@@ -56,6 +56,9 @@
int sysctl_ipfrag_high_thresh = 256*1024;
int sysctl_ipfrag_low_thresh = 192*1024;
+/* strict reassembly is off by default */
+int sysctl_strict_reassembly = 0;
+
/* Important NOTE! Fragment queue must be destroyed before MSL expires.
* RFC791 is wrong proposing to prolongate timer each fragment arrival by TTL.
*/
@@ -353,6 +356,24 @@ static struct ipq *ip_frag_create(unsign
{
struct ipq *qp;
+ if (sysctl_strict_reassembly) {
+ /* make a new reassembly queue iff this is the first or
+ * last fragment */
+ int frag_off = ntohs(iph->frag_off);
+ int offset = (frag_off & IP_OFFSET);
+
+ if ((offset == 0) && !(frag_off & IP_MF)) {
+ /* first fragment and there aren't anymore coming -
+ * nonsense, drop the fragment */
+ return NULL;
+ }
+ if ((offset != 0) && (frag_off & IP_MF)) {
+ /* if this isn't either the first or last fragment,
+ * don't make a new reassembly queue */
+ return NULL;
+ }
+ }
+
if ((qp = frag_alloc_queue()) == NULL)
goto out_nomem;
@@ -381,6 +402,35 @@ out_nomem:
return NULL;
}
+/* a reassembly queue match has been found and we're being
+ * "strict" about reassembly. If this fragment is first (last)
+ * in a reassembly queue which already has a first (last)
+ * fragment, then drop the existing reassembly queue. Return
+ * 1 if the existing queue was dropped, 0 otherwise.
+ *
+ * This function is called with (and returns with) a read_lock
+ * on ipfrag_lock
+ */
+static int __ip_find_strict_check(const struct iphdr *iph, struct ipq *qp)
+{
+ int frag_off = ntohs(iph->frag_off);
+ int offset = (frag_off & IP_OFFSET);
+
+ if (((offset == 0) && (qp->last_in & FIRST_IN)) ||
+ (!(frag_off & IP_MF) && (qp->last_in & LAST_IN))) {
+ atomic_inc(&qp->refcnt);
+ read_unlock(&ipfrag_lock);
+ spin_lock(&qp->lock);
+ if (!(qp->last_in&COMPLETE))
+ ipq_kill(qp);
+ spin_unlock(&qp->lock);
+ ipq_put(qp, NULL);
+ read_lock(&ipfrag_lock);
+ return 1;
+ }
+ return 0;
+}
+
/* Find the correct entry in the "incomplete datagrams" queue for
* this IP datagram, and create new one, if nothing is found.
*/
@@ -400,6 +450,10 @@ static inline struct ipq *ip_find(struct
qp->daddr == daddr &&
qp->protocol == protocol &&
qp->user == user) {
+ if (sysctl_strict_reassembly &&
+ __ip_find_strict_check(iph, qp)) {
+ break;
+ }
atomic_inc(&qp->refcnt);
read_unlock(&ipfrag_lock);
return qp;
@@ -549,6 +603,136 @@ err:
kfree_skb(skb);
}
+/* Add new segment to existing queue using "strict" semantics.
+ * The segment is rejected if it introduces gaps in the reassembly
+ * queue or overlaps with any existing fragments in the reassembly
+ * queue.
+ */
+static void ip_frag_queue_strict(struct ipq *qp, struct sk_buff *skb)
+{
+ struct sk_buff *prev, *next;
+ int flags, offset;
+ int ihl, end;
+
+ if (qp->last_in & COMPLETE)
+ goto err;
+
+ offset = ntohs(skb->nh.iph->frag_off);
+ flags = offset & ~IP_OFFSET;
+ offset &= IP_OFFSET;
+ offset <<= 3; /* offset is in 8-byte chunks */
+ ihl = skb->nh.iph->ihl * 4;
+
+ /* Determine the position of this fragment. */
+ end = offset + skb->len - ihl;
+
+ if ((!(flags & IP_MF) && (qp->last_in & LAST_IN)) ||
+ ((offset == 0) && (qp->last_in & FIRST_IN))) {
+ /* This can happen only if sysctl_strict_reassembly
+ * was toggled on after we did ip_find() for this
+ * fragment.
+ */
+ goto err;
+ }
+
+ /* Is this the final fragment? */
+ if ((flags & IP_MF) == 0) {
+ /* If we already have some bits beyond end
+ * drop this fragment.
+ */
+ if (end < qp->len)
+ goto err;
+ qp->len = end;
+ } else {
+ if (end&7) {
+ end &= ~7;
+ if (skb->ip_summed != CHECKSUM_UNNECESSARY)
+ skb->ip_summed = CHECKSUM_NONE;
+ }
+ if (end > qp->len) {
+ /* Some bits beyond end -> corruption. */
+ if (qp->last_in & LAST_IN)
+ goto err;
+ qp->len = end;
+ }
+ }
+ if (end == offset)
+ goto err;
+
+ if (pskb_pull(skb, ihl) == NULL)
+ goto err;
+ if (pskb_trim(skb, end-offset))
+ goto err;
+
+ /* Find out which fragments are in front and at the back of us
+ * in the chain of fragments so far. We must know where to put
+ * this fragment, right?
+ */
+ prev = NULL;
+ for(next = qp->fragments; next != NULL; next = next->next) {
+ if (FRAG_CB(next)->offset >= offset)
+ break; /* bingo! */
+ prev = next;
+ }
+
+ WARN_ON(prev && next);
+
+ /* We found where to put this one. Make sure there are no overlaps */
+
+ if (prev) {
+ int i = (FRAG_CB(prev)->offset + prev->len) - offset;
+
+ /* if i > 0, this fragment overlaps with the previous
+ * fragment. if i < 0, this fragment would introduce a
+ * "hole" in the reassembly chain. Drop it in either
+ * case.
+ */
+
+ if (i != 0) goto err;
+
+ }
+
+ if (next) {
+ int i = end - FRAG_CB(next)->offset;
+
+ /* if i > 0, this fragment overlaps with the next.
+ * if i < 0, this fragment would introduce a
+ * "hole" in the reassembly chain. Drop it in either
+ * case.
+ */
+
+ if (i != 0) goto err;
+ }
+
+ FRAG_CB(skb)->offset = offset;
+
+ /* Insert this fragment in the chain of fragments. */
+ skb->next = next;
+ if (prev)
+ prev->next = skb;
+ else
+ qp->fragments = skb;
+
+ if (skb->dev)
+ qp->iif = skb->dev->ifindex;
+ skb->dev = NULL;
+ qp->stamp = skb->stamp;
+ qp->meat += skb->len;
+ atomic_add(skb->truesize, &ip_frag_mem);
+ if (offset == 0)
+ qp->last_in |= FIRST_IN;
+ if ((flags & IP_MF) == 0)
+ qp->last_in |= LAST_IN;
+
+ write_lock(&ipfrag_lock);
+ list_move_tail(&qp->lru_list, &ipq_lru_list);
+ write_unlock(&ipfrag_lock);
+
+ return;
+
+err:
+ kfree_skb(skb);
+}
/* Build a new IP datagram from all its fragments. */
@@ -661,7 +845,10 @@ struct sk_buff *ip_defrag(struct sk_buff
spin_lock(&qp->lock);
- ip_frag_queue(qp, skb);
+ if (sysctl_strict_reassembly)
+ ip_frag_queue_strict(qp, skb);
+ else
+ ip_frag_queue(qp, skb);
if (qp->last_in == (FIRST_IN|LAST_IN) &&
qp->meat == qp->len)
diff -pur linux.old/net/ipv4/sysctl_net_ipv4.c linux.new/net/ipv4/sysctl_net_ipv4.c
--- linux.old/net/ipv4/sysctl_net_ipv4.c 2005-05-16 16:30:31.773480692 -0700
+++ linux.new/net/ipv4/sysctl_net_ipv4.c 2005-05-16 17:18:07.125138363 -0700
@@ -29,6 +29,7 @@ extern int sysctl_ipfrag_low_thresh;
extern int sysctl_ipfrag_high_thresh;
extern int sysctl_ipfrag_time;
extern int sysctl_ipfrag_secret_interval;
+extern int sysctl_strict_reassembly;
/* From ip_output.c */
extern int sysctl_ip_dynaddr;
@@ -258,6 +259,14 @@ ctl_table ipv4_table[] = {
.strategy = &sysctl_jiffies
},
{
+ .ctl_name = NET_IPV4_STRICT_REASM,
+ .procname = "strict_reassembly",
+ .data = &sysctl_strict_reassembly,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = &proc_dointvec
+ },
+ {
.ctl_name = NET_IPV4_TCP_KEEPALIVE_TIME,
.procname = "tcp_keepalive_time",
.data = &sysctl_tcp_keepalive_time,
--
Arthur
^ permalink raw reply
* Re: 2.4.30-hf1 do_IRQ stack overflows
From: Manfred Schwarb @ 2005-05-17 16:05 UTC (permalink / raw)
To: Herbert Xu; +Cc: marcelo.tosatti, linux-kernel, davem, netdev
In-Reply-To: <20050514110516.GA1238@gondor.apana.org.au>
> >
> > Trace; f8b531fc <[reiserfs]reiserfs_insert_item+14c/150>
> > Trace; c02387be <__kfree_skb+fe/160>
> > Trace; c02387be <__kfree_skb+fe/160>
> > Trace; f90dd5f4 <[8139too]rtl8139_start_xmit+84/180>
>
> Do you have any funky netfilter/iptables modules loaded?
I use a iptables based firewall, but no additional netfilter modules
are loaded. The network configuration is as it is shipped by SuSE
(i.e. no CONFIG_IP_NF_* modules, but most of the "Networking options"
are set to y).
And as I told in an earlier mail, I had scheduling built in
the kernel (CONFIG_NET_SCHED=y, CONFIG_NET_QOS=y,
CONFIG_NET_ESTIMATOR=y, CONFIG_NET_CLS=y, CONFIG_NET_CLS_ROUTE=y).
I have disabled NET_SCHED now (as Marcelo suggested), and I got no
overflows since then (4 days uptime).
Seems to work so far.
Thanks and regards,
Manfred
--
Weitersagen: GMX DSL-Flatrates mit Tempo-Garantie!
Ab 4,99 Euro/Monat: http://www.gmx.net/de/go/dsl
^ permalink raw reply
* Auslaender bevorzugt
From: mcook @ 2005-05-17 12:25 UTC (permalink / raw)
To: majordomo
Lese selbst:
http://www.npd.de/npd_info/deutschland/2005/d0305-14.html
Jetzt weiss man auch, wie es dazu kommt, dass Drogen, Waffen & Handy's in die Haende der Knacki's gelangen!
^ permalink raw reply
* net_device structure help
From: cranium2003 @ 2005-05-17 9:55 UTC (permalink / raw)
To: net dev
hello,
Is there any way in linux kernel network stack
at IP layer before adding IP header to know that
packet is transmitted to eth1 or forwarded to eth1 if
linux machine has 2 NIC's eth0 and eth1?
I check net_device structre and found that
ifindex is the field that gives proper incoming packet
network interface at IP layer but same for packet
trasmission not works.
regards,
cranium
Yahoo! Mail
Stay connected, organized, and protected. Take the tour:
http://tour.mail.yahoo.com/mailtour.html
^ permalink raw reply
* Re: tg3 crashes in scatter gather mode
From: David S. Miller @ 2005-05-17 6:53 UTC (permalink / raw)
To: razb; +Cc: netdev
In-Reply-To: <1116311948.3021.7.camel@raz-laptop>
From: raz ben jehuda <razb@bitband.com>
Date: Tue, 17 May 2005 09:39:08 +0300
> I have written a fast udp stack that implements a real scatter gather
> over user space pages. When i use it over an Intel e1000 card every
> thing works fine. doing the same thing over the tg3 crashes the system
> somewhere in tg3_poll area.
> Anyone ?
How about posting your code so we can have some hope
of analyzing your bug report?
^ permalink raw reply
* tg3 crashes in scatter gather mode
From: raz ben jehuda @ 2005-05-17 6:39 UTC (permalink / raw)
To: netdev
hello.
I have written a fast udp stack that implements a real scatter gather
over user space pages. When i use it over an Intel e1000 card every
thing works fine. doing the same thing over the tg3 crashes the system
somewhere in tg3_poll area.
Anyone ?
--
Raz
Long Live The Penguin
^ permalink raw reply
* Penis enhancement system that works for countless men worldwide.
From: Lawrence @ 2005-05-17 3:52 UTC (permalink / raw)
To: netdev
Buyer beware - Penis patches!
http://www.temline.info/ss/
Increase the length and girth of your penis
^ permalink raw reply
* Re: [git patches] 2.6.x net driver fixes
From: Linus Torvalds @ 2005-05-17 3:15 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Andrew Morton, Netdev, Linux Kernel
In-Reply-To: <42892F2B.8090908@pobox.com>
On Mon, 16 May 2005, Jeff Garzik wrote:
>
> Here's a first experimental git push from me. The git URL is
> rsync://rsync.kernel.org/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git
>
> but it should be noted that I would like you to pull the 'misc-fixes'
> branch. I'm told that branches are supposed to live in .git/refs/heads,
> so there you will find netdev-2.6.git/refs/heads/misc-fixes.
>
> Does that work?
Yes. Merged and pushed out.
> Changelog and patch for review attached.
It's wonderful if you also do a "diffstat" on the thing, since my merge
scripts will always show that to me, and I can verify at a glance that it
matches what you thought you sent me.
Also, if you are really nervous, the "git-diff-tree" thing is actually
quite good at generating changelogs, so you can do something like this:
git-rev-tree HEAD ^ORIG_HEAD | cut -d' ' -f2 | git-diff-tree -v -p --stdin | less -S
which means: give me a list of all commits that are in HEAD but not in
ORIG_HEAD (replace with whatever markers you have, in this case you'd use
"misc-fixes" instead of HEAD and some marker - maybe the SHA1 - for my
last base), then take just the SHA1 of that list of commits, and show a
verbose diff of each of the commits with the patch.
For example, the output of the above command (after I merged from you) is
appended, so you can see exactly what I merged.
Linus
----
diff-tree 99718699f5746cc365f3a9ab4769568a1da97635 (from f7a3aae1723e7ffc9c4fcdb489365da7a3d81255)
Author: Geoff Levand <geoffrey.levand@am.sony.com>
Date: Thu Apr 14 11:20:32 2005 -0700
[PATCH] {PATCH] Fix IBM EMAC driver ioctl bug
Fix IBM EMAC driver ioctl bug.
I found IBM EMAC driver bug.
So mii-tool command print wrong status.
# mii-tool
eth0: 10 Mbit, half duplex, no link
eth1: 10 Mbit, half duplex, no link
I can get correct status on fixed kernel.
# mii-tool
eth0: negotiated 100baseTx-FD, link okZZ
eth1: negotiated 100baseTx-FD, link ok
Hiroaki Fuse
Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com> for CELF
--- a/drivers/net/ibm_emac/ibm_emac_core.c
+++ b/drivers/net/ibm_emac/ibm_emac_core.c
@@ -1595,7 +1595,7 @@ static struct ethtool_ops emac_ethtool_o
static int emac_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
{
struct ocp_enet_private *fep = dev->priv;
- uint *data = (uint *) & rq->ifr_ifru;
+ uint16_t *data = (uint16_t *) & rq->ifr_ifru;
switch (cmd) {
case SIOCGMIIPHY:
diff-tree c4cc26d3310a6614a20e32276228a5d44159fc9b (from 99718699f5746cc365f3a9ab4769568a1da97635)
Author: Jiri Benc <jbenc@suse.cz>
Date: Wed Apr 27 08:48:56 2005 +0200
[PATCH] Typo in tulip driver
This patch fixes a typo in tulip driver in 2.6.12-rc3.
--- a/drivers/net/tulip/tulip_core.c
+++ b/drivers/net/tulip/tulip_core.c
@@ -1104,7 +1104,7 @@ static void set_rx_mode(struct net_devic
if (entry != 0) {
/* Avoid a chip errata by prefixing a dummy entry. Don't do
this on the ULI526X as it triggers a different problem */
- if (!(tp->chip_id == ULI526X && (tp->revision = 0x40 || tp->revision == 0x50))) {
+ if (!(tp->chip_id == ULI526X && (tp->revision == 0x40 || tp->revision == 0x50))) {
tp->tx_buffers[entry].skb = NULL;
tp->tx_buffers[entry].mapping = 0;
tp->tx_ring[entry].length =
diff-tree c8920ba041c8934b29370f5d62ab9ea8f147966b (from c4cc26d3310a6614a20e32276228a5d44159fc9b)
Author: Daniel Andersen <daniel@linux-user.net>
Date: Thu May 5 15:14:09 2005 -0700
[PATCH] wireless: 3CRWE154G72 Kconfig help fix
Version 2 of the 3com OfficeConnect 11g Cardbus Card aka 3CRWE154G72 is not
supported by the prism54 project. To stop confusion, the kernel
documentation should state so as 3com made a good job hiding the version.
Signed-off-by: Andrew Morton <akpm@osdl.org>
diff -puN drivers/net/wireless/Kconfig~wireless-3crwe154g72-kconfig-help-fix drivers/net/wireless/Kconfig
--- a/drivers/net/wireless/Kconfig
+++ b/drivers/net/wireless/Kconfig
@@ -323,7 +323,7 @@ config PRISM54
For a complete list of supported cards visit <http://prism54.org>.
Here is the latest confirmed list of supported cards:
- 3com OfficeConnect 11g Cardbus Card aka 3CRWE154G72
+ 3com OfficeConnect 11g Cardbus Card aka 3CRWE154G72 (version 1)
Allnet ALL0271 PCI Card
Compex WL54G Cardbus Card
Corega CG-WLCB54GT Cardbus Card
diff-tree f7a3aae1723e7ffc9c4fcdb489365da7a3d81255 (from 88d7bd8cb9eb8d64bf7997600b0d64f7834047c5)
Author: Al Viro <viro@www.linux.org.uk>
Date: Sun Apr 3 07:15:52 2005 +0100
[PATCH] drivers/net/wireless enabled by wrong option
NET_WIRELESS is only a subset of the stuff in drivers/net/wireless;
NET_RADIO is what covers all of them.
Signed-off-by: Al Viro <viro@parcelfarce.linux.theplanet.co.uk>
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -187,7 +187,7 @@ obj-$(CONFIG_TR) += tokenring/
obj-$(CONFIG_WAN) += wan/
obj-$(CONFIG_ARCNET) += arcnet/
obj-$(CONFIG_NET_PCMCIA) += pcmcia/
-obj-$(CONFIG_NET_WIRELESS) += wireless/
+obj-$(CONFIG_NET_RADIO) += wireless/
obj-$(CONFIG_NET_TULIP) += tulip/
obj-$(CONFIG_HAMRADIO) += hamradio/
obj-$(CONFIG_IRDA) += irda/
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox