patchlwIP - A Lightweight TCP/IP stack - Patches: patch #5834, Don't fragment flag

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #5834: Don't fragment flag

Submitter:  Frédéric Bernon <fbernon>
Submitted:  Thu 29 Mar 2007 03:18:14 PM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  fbernon Open/Closed:  Closed
Planned Release:  None

Jump to the original submission

Tue 03 Apr 2007 01:59:13 PM UTC, comment #24: 

Ok, I change it :)

Frédéric Bernon <fbernon>
Group Member
Tue 03 Apr 2007 01:53:12 PM UTC, comment #23: 

I don't have strong opinions either way, but now we seem to have some understanding about why it was there, I'm happy to remove it and just set to zero.  Seems simplest.

Kieran Mansley <kieranm>
Group Member
Tue 03 Apr 2007 01:33:31 PM UTC, comment #22: 

I'm happy if it is set to zero for good.

If Frédéric is worried that someone might depend on it, perhaps a message in lwip-users is in order, with reference to this patch to avoid repeating the technical details.

If someone comes along later and complains that they were dependent on DF (for some obscure reason) then they can add it back in their port, or an IP_FRAG_FLAGS mechanism could be added then.

David Empson <dempson>
Tue 03 Apr 2007 01:14:47 PM UTC, comment #21: 

Right, but IP_FRAG_FLAGS in ip.h give the possibility to someone to keep the current behavior (I don't know, perhaps there is a very strange reason I don't understand). And it's not a Heavy change :)

>I think we can safely set it to 0 and be done

And that was a very first idea (see file #12332) :)

Frédéric Bernon <fbernon>
Group Member
Tue 03 Apr 2007 01:09:10 PM UTC, comment #20: 

Re comment #19:
Hm, given this, I wouldn't really include an option, neither as in comment #5, nor including a flag in ip_output*(). I think it's simply not necessary and bloats the code. Nobody ever really seems to need this flag until looking for the smallest MTU on a given route. Since lwIP does not support this (and given the lw in it's name, probably never will), I think we can safely set it to 0 and be done. Right?

Simon Goldschmidt <goldsimon>
Group administrator
Tue 03 Apr 2007 01:03:56 PM UTC, comment #19: 

Yep, yep , yep...

Ok, I propose to come back to my idea in comment #5. Ok for all?

For information, to implement IP_DONTFRAGMENT, we have to :

- add a "frag" or "options" parameter in ip_output_if() and in ip_output()

- define a new SOF_DONTFRAGMENT, and a IP_DONTFRAGMENT to set a bit in "options" field, and update get/setsockopt

- update raw_sendto, udp_send, tcp_output, tcp_output_segment, tcp_rst, tcp_keepalive, icmp_dest_unreach & icmp_time_exceeded

Not a big work, but not really useful...

Frédéric Bernon <fbernon>
Group Member
Tue 03 Apr 2007 12:32:39 PM UTC, comment #18: 

In our initial port (based on 1.2.0) we noticed the DF flag was set, and couldn't see any reason for it to be there (without path MTU discovery), so we simply turned it off. We have reassembly enabled at present, and are only talking to full TCP/IP implementations which support reassembly, so it won't be a problem yet. We aren't particularly short of memory so I see no need for us to disable reassembly.

In future we might want to talk to simpler devices that don't support reassembly. These are likely to be on the local network so path MTU won't be an issue.

We have no need for a complex scheme allowing the DF flag to be set on a per-connection basis. I'm working from a theoretical basis, considering how other people might use the stack.

We have made one other change I know of in this area: our LWIP implementation is configured with separate MTU values for local traffic (same subnet) and distant traffic. This allows us to configure a pessimistic value for going over unreliable networks that use short MTUs (in the order of 576) to keep the error rate down, while allowing best performance over the local network. This feature should only be needed if the receiving host doesn't support reassembly, and the network has a shorter path MTU than the LWIP end's LAN (given that DF=0 in LWIP's transmissions).

I'll look into whether we can supply a patch for this so that others can use it. (Lack of familiarity with the diff/patch mechanism is a temporary stumbling block.) Our LWIP implementation has diverged a fair amount from the reference release, mostly due to changes integrating it into our in-house OS (with cooperative multitasking) and memory management.

Re Simon's comment #16:

> I think a reason to associate it with IP_REASSEMBLY is that if you can't send to the other host without fragmentation, the other host won't succeed in that, either.


This assumes that both hosts are configured with the same MTU. LWIP might be sending large packets to a host which does support reassembly, through a constrained network, and therefore should be sending them without DF set, but might be receiving short packets (with or without DF set) that will make it through the path MTU without being fragmented.

If LWIP has reassembly disabled, it is required that the other host know this in advance, and be configured with a small enough MTU for the network (or implement path MTU discovery) so it can get data through.

This has nothing to do with whether LWIP should be setting the DF flag when transmitting data to that host.

If the other host supports reassembly, it is safe for LWIP to send packets without DF.

If the other host doesn't support reassembly then LWIP must be configured with a small enough MTU to be able to get through the network, since it doesn't implement path MTU discovery. It doesn't matter whether it sets DF: in either case, an packet larger than the path MTU will fail to get through.

If LWIP sends a packet which is too big and it has DF set, then a router along the way will discard it and should send back an ICMP error.

If LWIP sends a packet which is too big and it doesn't have DF set, then a router along the way will fragment it, and the receiving host will discard it (because it doesn't reassemble fragments). This won't produce any feedback.

It is probably better to assume most hosts support reassembly, therefore it is better to never set DF.

The main benefit I can see for setting DF is that sending a packet larger than than the path MTU will generate ICMP errors from the router, which might be useful to identify the problem even if LWIP ignores it. It will also eliminate useless traffic over a constrained network (and distant portions of the route).

Some documentation is probably in order. Since LWIP doesn't support path MTU discovery, it is essential that if dealing with other hosts that don't support reassembly, LWIP must be configured with a small enough MTU to make it through the network to that host.

DF should not be tied to !IP_REASSEMBLY because transmission of DF has nothing to do with LWIP's receive capabilities.

I think that the DF flag should be permanently disabled (except for special applications like ping), until such time that LWIP gains support for path MTU discovery.

It might be useful to provide an option for setting DF (to allow ICMP error messages to give hints as to MTU configuration errors), but this could be tested manually using pings with DF set.

David Empson <dempson>
Tue 03 Apr 2007 08:19:51 AM UTC, comment #17: 


>so I think this should be a configurable option for each UDP or TCP connection,


IP_DONTFRAGMENT would be the solution for you ? I think it's only useful to do a "Path MTU discovery" (others uses seem to be difficult to integrate in lwIP: handling ICMP messages, change mtu size per connection, etc...)

>possibly with a default setting which is configurable in lwipopts.h or similar.


I agree with you (comment #5), even if it's not to set a default value.

But I think Simon comment #16 is right. IP_REASSEMBLY is a good "solution". I don't think to propose a patch for IP_DONTFRAGMENT, except if someone really need it... But if you got one, please send, I will study it and integrate if it's ok for others developers...

Frédéric Bernon <fbernon>
Group Member
Tue 03 Apr 2007 06:45:04 AM UTC, comment #16: 

I think a reason to associate it with IP_REASSEMBLY is that if you can't send to the other host without fragmentation, the other host won't succeed in that, either.
UNLESS the other host
a) tries to find out the maximum MTU for the transmission path or
b) by default uses an MTU equal to or smaller than the maximum MTU of the transmission path.

And since we don't inform the source of a fragmented packet that we can't receive it (when IP_REASS==0), I think it's better to not generate them, isn't it?

Simon Goldschmidt <goldsimon>
Group administrator
Mon 02 Apr 2007 11:12:24 PM UTC, comment #15: 

It doesn't seem to me that associating the state of the IP_DF flag with IP_REASSEMBLY is a reasonable choice. They apply to opposite directions of the communication dialogue.

If a host is sending frames with DF set, it is an instruction to routers along the way not to fragment the packet, so the other host will not receive fragmented IP packets. This makes sense if the other host doesn't support reassembly.

The receiver will generally pay no attention to the state of this flag (except possibly in a case like replying to a ping).

DF flags in the transmit direction therefore convey little or no information to the other host that they should also be using DF in the opposite direction.

It only makes sense to associate it with IP_REASSEMBLY if both ends of the communication are running LWIP with the same options.

To do it correctly, each host needs prior knowledge of whether the other host supports fragmentation, so I think this should be a configurable option for each UDP or TCP connection, possibly with a default setting which is configurable in lwipopts.h or similar.

The default should be to allow fragmentation, as in most cases LWIP is probably being used to talk to a "full" TCP/IP implementation.

David Empson <dempson>
Fri 30 Mar 2007 01:35:38 PM UTC, comment #14: 

Remember if one day it's necessary (by example, to implement a "MTU discovery" client), that we could add a IP_DONTFRAGMENT option (for IPPROTO_IP level) for get/setsockopt...

Frédéric Bernon <fbernon>
Group Member
Fri 30 Mar 2007 01:27:54 PM UTC, comment #13: 

OK.

Kieran Mansley <kieranm>
Group Member
Fri 30 Mar 2007 01:24:24 PM UTC, comment #12: 

Proposed patch attached.

(file #12346)

Frédéric Bernon <fbernon>
Group Member
Fri 30 Mar 2007 12:37:37 PM UTC, comment #11: 

Not sure about the reason of IP_DF per default. Look the original file :

http://cvs.savannah.nongnu.org/viewcvs/lwip/src/core/ipv4/ip.c?rev=1.1&root=lwip&view=markup

Seems IP_REASSEMBLY was already existing.

About testing IP_REASSEMBLY, why not, even if the problem can be that lwIP device can send datas which have to be fragmented per network to "target" (packets size = LAN MTU, but > Path MTU), and it's not necessary the case in the other direction.

But I'm agree, and I will commit like this if not objection...

Frédéric Bernon <fbernon>
Group Member
Fri 30 Mar 2007 11:20:34 AM UTC, comment #10: 

I would only set it to 0 if lwIP supports reassembling. In other words ifdef it with IP_REASSEMBLY. Maybe that is why it was included in the initial version of ip.c (maybe reassembly was not supported then, I didn't check CVS, though)

Simon Goldschmidt <goldsimon>
Group administrator
Fri 30 Mar 2007 10:11:41 AM UTC, comment #9: 

In "RFC 791 - Internet Protocol", we can read :

"If the Don't Fragment flag (DF) bit is set, then internet fragmentation of this datagram is NOT permitted, although it may be discarded.  This can be used to prohibit fragmentation in cases where the receiving host does not have sufficient resources to reassemble internet fragments."

When I read the "can be used", I think "is not per default".


Ok to disable IP_DF ?

(just for information, read that, it's an good paper on fragmentation subject : http://www.cs.binghamton.edu/~nael/classes/cs528/fragment.pdf )

Frédéric Bernon <fbernon>
Group Member
Fri 30 Mar 2007 09:02:51 AM UTC, comment #8: 

I search in the web CVS, and it was already here in the first file version:

Revision 1.1  - (view) (download) (as text) (annotate) - [select for diffs]
Sat Oct 19 13:00:35 2002 UTC (4 years, 5 months ago) by likewise
Branch: MAIN
Branch point for: adamdunkels

Initial revision


So, I suppose it's something forget, which has never cause problems because most of networks support normal MTU (I have see than only on one of our customer network, a very old and specific network...)

Frédéric Bernon <fbernon>
Group Member
Fri 30 Mar 2007 08:55:06 AM UTC, comment #7: 

The only use I know about that is to discover max MTU on a path (like with the ping -f). I suppose it's just a mistake...

Frédéric Bernon <fbernon>
Group Member
Fri 30 Mar 2007 08:48:23 AM UTC, comment #6: 

No, I don't think we need to have an option for this.  I just wish I understood why that line of code was written in the first place.  I wonder if someone knew something we don't!

Kieran Mansley <kieranm>
Group Member
Fri 30 Mar 2007 08:34:13 AM UTC, comment #5: 

If someone really need it as default (but will be strange, but, why not), I can add in opt.h a

#ifndef IP_FRAG_FLAGS
#define IP_FRAG_FLAGS 0
#endif

and use it like this :

PH_OFFSET_SET(iphdr, htons(IP_FRAG_FLAGS));
 
Or better than opt.h, ip.h?

What is your preference?

Frédéric Bernon <fbernon>
Group Member
Fri 30 Mar 2007 08:27:04 AM UTC, comment #4: 

I suppose so.  I'd like to understand this a bit better and work out why that line was there in the first place, but don't have time really!


Kieran Mansley <kieranm>
Group Member
Fri 30 Mar 2007 08:08:48 AM UTC, comment #3: 

I test to remove, but I got problems (I have supposed that this field wasn't initialized and could contains anything, but I don't search more...). Ok for you?

Frédéric Bernon <fbernon>
Group Member
Fri 30 Mar 2007 07:59:22 AM UTC, comment #2: 

Would it be better to just remove that line rather than replace it with one that sets it to zero?  I'm not sure.

Kieran Mansley <kieranm>
Group Member
Thu 29 Mar 2007 05:51:39 PM UTC, comment #1: 

Proposed patch file attached (just set to 0 fragment flags).


(file #12332)

Frédéric Bernon <fbernon>
Group Member
Thu 29 Mar 2007 03:18:14 PM UTC, original submission:  

In last March (March 22th 2006), a user tell us there was a problem with ip.c: "IPH_OFFSET_SET(iphdr, htons(IP_DF))" is always called in ip_output_if (even if you have enable IP_FRAG and IP_REASSEMBLY). This limitation is really important, because if big packets are sent by a circuit with several routers, where MTU is lower than the one lwIP use, packets are lost.

Kieran, you answer was :

"I'm not sure why it's like that in lwIP, but more generally you might set this bit if you're doing path MTU discovery, and so believe you have chosen an MTU that should not need fragmenting.  If you set this bit, and it turns out the packet does need fragmenting, you'll get an ICMP message telling you this from whoever drops the packet, and so you can update your MTU for future packets."

I propose to remove that line, and in a second time, to propose a new IP_DONTFRAGMENT option (for IPPROTO_IP level) for set/getsocketopt, if some users need it...



Frédéric Bernon <fbernon>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #12346:  IP_DF.patch added by fbernon (1KiB - text/x-patch)
file #12332:  IP_DF.patch added by fbernon (564B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dempson (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by fbernon (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-04-03 fbernon Open/ClosedOpen Closed
    2007-04-03 fbernon Open/ClosedClosed Open
    2007-03-30 fbernon StatusNone Done
        Open/ClosedOpen Closed
    2007-03-30 fbernon Attached File- Added IP_DF.patch, #12346
    2007-03-29 fbernon Attached File- Added IP_DF.patch, #12332

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code