Bug 822 - NetDevice::Mtu attribute not settable by default
NetDevice::Mtu attribute not settable by default
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: core
ns-3-dev
All All
: P2 critical
Assigned To: Tom Henderson
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-02-19 18:22 EST by Tom Henderson
Modified: 2010-04-10 16:49 EDT (History)
5 users (show)

See Also:


Attachments
simplified test case (2.81 KB, text/x-c++src)
2010-02-19 18:27 EST, Tom Henderson
Details
implement option 2. (38.71 KB, patch)
2010-04-07 04:26 EDT, Mathieu Lacage
Details | Diff
set wimax mtu default to 1400. (38.80 KB, patch)
2010-04-08 04:06 EDT, Mathieu Lacage
Details | Diff
update defaults as suggested by tom (38.80 KB, patch)
2010-04-08 04:10 EDT, Mathieu Lacage
Details | Diff
Blurb for tutorial warning about non-real attribute settings (6.03 KB, patch)
2010-04-09 20:31 EDT, Craig Dowell
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Henderson 2010-02-19 18:22:25 EST
Config::SetDefault ("ns3::NetDevice::Mtu", UintegerValue (1000));

has no effect on any derived classes; the indirection through virtual setters/getters does not get called.
Comment 1 Tom Henderson 2010-02-19 18:27:48 EST
Created attachment 767 [details]
simplified test case

This is a simplified test case.  In the actual code, this can be observed by trying to read the MTU of BridgeNetDevice, for example.
Comment 2 Mathieu Lacage 2010-02-20 13:17:10 EST
(In reply to comment #0)
> Config::SetDefault ("ns3::NetDevice::Mtu", UintegerValue (1000));
> 
> has no effect on any derived classes; the indirection through virtual
> setters/getters does not get called.


SetDefault does not call Set. It just stores the value in the global TypeId database so that the next time you create an object, it gets initialized with that value. Could that be the source of some confusion ?
Comment 3 Tom Henderson 2010-02-21 01:05:40 EST
(In reply to comment #2)
> (In reply to comment #0)
> > Config::SetDefault ("ns3::NetDevice::Mtu", UintegerValue (1000));
> > 
> > has no effect on any derived classes; the indirection through virtual
> > setters/getters does not get called.
> 
> 
> SetDefault does not call Set. It just stores the value in the global TypeId
> database so that the next time you create an object, it gets initialized with
> that value. Could that be the source of some confusion ?

The case that is failing is that attributes of abstract base classes (class C in the testcase, and attribute Mtu in our real codebase) are not settable by Config::SetDefault().  If you look at the ConfigStore output, it seems that these attributes may be missing in the default database, which may suggest where the problem might lie.

If you place the posted testcase in scratch/, comment out line 95 assertion, and run it, and examine the output config store file (output-attributes.xml) you will find:

1) there is no default value logged for class C's attribute in the config store output; i.e. there is no:
default ns3::C::TestUint16 "0xffff"

and this statement in the program has no effect, as the assertion on line 95 shows:
  Config::SetDefault ("ns3::C::TestUint16", UintegerValue (47));


2) although resetting A's attribute does seem to work for subsequently created objects, including subclasses:
  Config::SetDefault ("ns3::A::TestInt16", IntegerValue (-5));

it is still logged as the initial default when config store output is generated prior to Simulator::Run():
  default ns3::A::TestInt16 "-2"

These seem to be two related but separate bugs; maybe a second tracker issue should be opened.
Comment 4 Tom Henderson 2010-03-16 18:59:58 EDT
(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #0)
> > > Config::SetDefault ("ns3::NetDevice::Mtu", UintegerValue (1000));
> > > 
> > > has no effect on any derived classes; the indirection through virtual
> > > setters/getters does not get called.
> > 
> > 
> > SetDefault does not call Set. It just stores the value in the global TypeId
> > database so that the next time you create an object, it gets initialized with
> > that value. Could that be the source of some confusion ?
> 
> The case that is failing is that attributes of abstract base classes (class C
> in the testcase, and attribute Mtu in our real codebase) are not settable by
> Config::SetDefault().  If you look at the ConfigStore output, it seems that
> these attributes may be missing in the default database, which may suggest
> where the problem might lie.
> 
> If you place the posted testcase in scratch/, comment out line 95 assertion,
> and run it, and examine the output config store file (output-attributes.xml)
> you will find:
> 
> 1) there is no default value logged for class C's attribute in the config store
> output; i.e. there is no:
> default ns3::C::TestUint16 "0xffff"
> 
> and this statement in the program has no effect, as the assertion on line 95
> shows:
>   Config::SetDefault ("ns3::C::TestUint16", UintegerValue (47));

The problem is in these attribute flags:
                   TypeId::ATTR_SET | TypeId::ATTR_GET,
which disable setting the value in the constructor.  The reason that they are there for NetDevice::Mtu is that the setter indirects into the subclass which tries to set the underlying frame size accordingly, but there is some order of construction/initialization problem that errors out in some cases.

I'm changing this bug to "NetDevice::Mtu attribute not settable by default" to reflect the specific problem here.

> 
> 
> 2) although resetting A's attribute does seem to work for subsequently created
> objects, including subclasses:
>   Config::SetDefault ("ns3::A::TestInt16", IntegerValue (-5));
> 
> it is still logged as the initial default when config store output is generated
> prior to Simulator::Run():
>   default ns3::A::TestInt16 "-2"
> 
> These seem to be two related but separate bugs; maybe a second tracker issue
> should be opened.

I will open a second bug report for this issue.
Comment 5 Tom Henderson 2010-04-06 14:57:47 EDT
I'd like to try to resolve this for the current release.

To summarize, NetDevice::Mtu presently has two problems.  1) it leads to order-of-initialization problems since it is linked to subclass attributes like Csma FrameSize.  2) the default value doesn't get stored properly in the config store system (bug 845).

Here are the options I see:
1) simply remove this attribute, since each subclass has its own way of configuring frame size via attributes
  pro:  simple (attribute was broken anyway)
  con:  we lose this commonality among devices, especially user familiarity with the concept (ifconfig mtu ...).  If I do an "ifconfig mtu N" from a container bound to an ns-3 NetDevice, I will need to downcast the pointer and search for the right attribute (unless we preserve base class SetMtu/GetMtu methods).

2) move this attribute to each subclass, but remove the device-specific frame size attributes so that we don't have two attributes pointing to one member variable (again leading to initialization order problems and user confusion)
  pro:  MTU can be device specific (more realistic)
  con:  breaks ns-3 API

3) move Mtu attribute to each subclass, and decouple this attribute from the frame size attribute; these are two separate member variables.  This means that a user could e.g. set an Ethernet MTU of 1000 bytes even though the frame size was set to 1518 bytes.  Consistency checking would still be needed (that MTU <= (frame size - header size), for instance).
  pro: preserves a device-specific MTU and preserves ns-3 API
  con: still needs to be robust to initialization order issues; need to document to users that these concepts (FrameSize and Mtu) are still related in some way, but are not locked in-step to each other

Preferences, comments?
Comment 6 Tom Henderson 2010-04-06 15:00:28 EDT
(In reply to comment #5)
 2) the default value doesn't get stored properly in the config
> store system (bug 845).


I'd like to add that this bug 845 issue seems orthogonal to the real issue which is that Mtu and FrameSize, by being bound to the same underlying variable, have order of initialization issues that can cause errors by users not realizing that a value of one attribute has been overridden by another.
Comment 7 Craig Dowell 2010-04-06 16:02:50 EDT
The original idea was that both mtu and frame size were device-related things and if someone wanted to set frame size, we would do that and then make sure mtu was consistent.  If someone wanted to set mtu, we would do that and then make sure frame size was consistent -- leading to a simple, one-step process to, for example, enable jumbo datagrams. Conceptually, mtu and frame size are indirectly describing the same quantity -- the number of bits that can be sent across a channel. There are also encapsulation modes (Dix, Llc) to consider which also get into the picture, so there are really three inter-related quantities.

Trying to make this work for two classes of users who thought differently about which was the more "important" attribute, mtu or frame size, has gotten a bit out of hand and resulted in quite subtle problems.

Keeping in mind our guidance that when in doubt we should do it like Linux, I think the best answer here is to simply treat framesize as a maximum value, like in a linux device:

 #define  MAX_ETH_FRAME_SIZE       1536

We would then decouple mtu and frame size and to runtime checks to make sure that the asked-for MTU fits into the maximum frame size.

If someone wants to actually crank up the frame size to 9000 bytes, they could by setting the framesize attribute and also the mtu attribute (in that order).  This is an order-dependent two-step process (which was what I originally wanted to avoid).  However, it turns out that I did not actually avoid dependency issues but in fact created subtler ones.

So I favor Tom's option 3:

3) move Mtu attribute to each subclass, and decouple this attribute from the
frame size attribute; these are two separate member variables.  This means that
a user could e.g. set an Ethernet MTU of 1000 bytes even though the frame size
was set to 1518 bytes.  Consistency checking would still be needed (that MTU <=
(frame size - header size), for instance).

Frame size then means "maximum frame size."
Comment 8 Mathieu Lacage 2010-04-07 02:52:23 EDT
(In reply to comment #5)
> 2) move this attribute to each subclass, but remove the device-specific frame
> size attributes so that we don't have two attributes pointing to one member
> variable (again leading to initialization order problems and user confusion)
>   pro:  MTU can be device specific (more realistic)
>   con:  breaks ns-3 API
> 
> 3) move Mtu attribute to each subclass, and decouple this attribute from the
> frame size attribute; these are two separate member variables.  This means that
> a user could e.g. set an Ethernet MTU of 1000 bytes even though the frame size
> was set to 1518 bytes.  Consistency checking would still be needed (that MTU <=
> (frame size - header size), for instance).
>   pro: preserves a device-specific MTU and preserves ns-3 API
>   con: still needs to be robust to initialization order issues; need to
> document to users that these concepts (FrameSize and Mtu) are still related in
> some way, but are not locked in-step to each other


The only robust way of doing this is 2). 3) is not workable because it introduces dependencies between attributes. i.e., the result of a Set call will depend on a earlier Set call. If each attribute is not independent from all other attributes, this breaks many assumptions which in turn breaks code attempting to perform generic serialization/deserialization of attributes. For example ConfigStore will break in very subtle ways if we go with 3).
Comment 9 Mathieu Lacage 2010-04-07 04:26:04 EDT
Created attachment 810 [details]
implement option 2.

All tests pass with this patch. A couple of interesting notes:

1) this removes all the frame size code to avoid confusion between the frame size and mtu
2) this fixes a bug in udp-client-server-test.cc: it was setting the device mtu but this did not work correctly so, the test was working when it should not.
3) I have been really super careful to keep the existing default behavior so that existing code which did not change the mtu would keep working.
4) I have tried really hard to not introduce new indentation in all files I touched: a lot of them do not have coding-style indentation so, I had to do it by hand so, I might have gotten it wrong.
5) Requires a python rescan on top. Did not include it in this patch to avoid confusion.

I really would like to fix this bug for this release so, it would be helpful if someone could review it.
Comment 10 Tom Henderson 2010-04-07 11:55:01 EDT
(In reply to comment #9)
> Created an attachment (id=810) [details]
> implement option 2.
> 
> All tests pass with this patch. A couple of interesting notes:
> 
> 1) this removes all the frame size code to avoid confusion between the frame
> size and mtu
> 2) this fixes a bug in udp-client-server-test.cc: it was setting the device mtu
> but this did not work correctly so, the test was working when it should not.
> 3) I have been really super careful to keep the existing default behavior so
> that existing code which did not change the mtu would keep working.
> 4) I have tried really hard to not introduce new indentation in all files I
> touched: a lot of them do not have coding-style indentation so, I had to do it
> by hand so, I might have gotten it wrong.
> 5) Requires a python rescan on top. Did not include it in this patch to avoid
> confusion.
> 
> I really would like to fix this bug for this release so, it would be helpful if
> someone could review it.

I am fine with this solution in general and the approach of this patch; I assume you preserve the methods SetMtu/GetMtu for backward compatibility.

I disagree with a few of the defaults; suggest that they be changed as follows:
Bridge:  1500
CSMA:  1500
Emu:  ?  /* Should we delete Mtu attribute here? */
TapBridge:  ?  /* Should we delete Mtu attribute here? */
Virtual:  1500
WiMAX:  1400  /* Amine should confirm this value */

I am OK with not enforcing a frame size on Csma because users may want to turn Mtu to 9000 (Jumbograms) and it would be nice if they could do so without rebuilding the csma code.

Regarding wifi, how do you propose to handle MSDU aggregation?  A user may think that he/she can set the MTU on an 802.11n device to larger and will be blocked by the integer constant set to 2304.
Comment 11 Mathieu Lacage 2010-04-07 12:03:29 EDT
(In reply to comment #10)
>
> I am fine with this solution in general and the approach of this patch; I
> assume you preserve the methods SetMtu/GetMtu for backward compatibility.

I think that it makes sense to keep these methods around, for linux net_device alignment.

> I disagree with a few of the defaults; suggest that they be changed as follows:

I don't care much about the defaults: I merely copied what was in there already. I would support keeping the emu/tapbridge mtus but it's not something I care much about.

> Bridge:  1500
> CSMA:  1500
> Emu:  ?  /* Should we delete Mtu attribute here? */
> TapBridge:  ?  /* Should we delete Mtu attribute here? */
> Virtual:  1500
> WiMAX:  1400  /* Amine should confirm this value */

I have CCed amine.

> 
> I am OK with not enforcing a frame size on Csma because users may want to turn
> Mtu to 9000 (Jumbograms) and it would be nice if they could do so without
> rebuilding the csma code.
> 
> Regarding wifi, how do you propose to handle MSDU aggregation?  A user may
> think that he/she can set the MTU on an 802.11n device to larger and will be
> blocked by the integer constant set to 2304.

I think that if users think this, they are seriously misleaded. i.e., this is not what MSDU aggregation is about but I guess that is a question for our new wifi maintainer. CCing him.
Comment 12 Mohamed Amine ISMAIL 2010-04-07 12:37:02 EDT
default MTU for WiMAX:
======================
The 802.16 standard allows about 2039 bytes of payload (the len is coded on 11bits including the size of the header=6 and optional 2bytes crc).
16ng-ipv4-over-802-dot-16-ipcs IETF draft specifies that the default MTU should be 1500. However The WiMAX forum recommended 1400 bytes!:)
I think that it is wise to use the value recommended by the WiMAx forum (1400)
Comment 13 Tom Henderson 2010-04-07 12:40:00 EDT
(In reply to comment #11)
> > Regarding wifi, how do you propose to handle MSDU aggregation?  A user may
> > think that he/she can set the MTU on an 802.11n device to larger and will be
> > blocked by the integer constant set to 2304.
> 
> I think that if users think this, they are seriously misleaded. i.e., this is
> not what MSDU aggregation is about but I guess that is a question for our new
> wifi maintainer. CCing him.

I thought I had read in the past that 802.11n aggregation allowed larger MTUs, but I will defer to the experts here because I could have misunderstood it.
Comment 14 Craig Dowell 2010-04-07 14:17:34 EDT
Okay, I hadn't considered automatic attribute setter programs which can decide on their own setter execution order.  As you suggest, if there are any inter-attribute dependencies that can produce errors, these can fail in unexpected ways as the setters are called according to the convenience of the program, not the semantics of the attributes.

The corollary is that *any* consistency checks for attributes *must* be done after Simulator::Run() is called and not during configuration time.  This should be advertized somewhere fairly prominently.  I think that demanding that attributes be completely independent at *all* times is too restrictive.  It seems clear to me that attributes can have dependencies, though, so consistency checking at run-time seems appropriate.

With that in mind, having an mtu and a maximum frame size attribute (like in Linux) is still possible -- the consistency checks just need to be done while the device is running.  I still think having a maximum frame size is important because of the following:

Regarding the proposed patch, be aware that if you set the mtu to the default 1500 bytes and select LLC/SNAP framing you are going to be capable of generating illegally long Ethernet packets!

  MTU 1500, DIX encapsulation works out to 1518 byte frames.
  MTU 1492, LLC/SNAP encapsulation works out to 1518 byte frames.
  MTU 1500, LLC/SNAP encapsulation works out to 1526 byte frames.

This is why the frame size was there in the first place; and the mtu from framesize calculation was there so a user didn't have to get out a calculator to set mtu properly.  The fact that this behavior can happen so easily should also be advertized somewhere prominently if maximum frame size is removed.  Would most people consider it a bug?  

It's a small error, but it is a modelling error if you think CSMA is Ethernet with respect to framing.  Having a maximum frame size attribute that derfaults to 1518 bytes would at least cause a runtime error if a user neglects to reduce MTU to 1492 when setting Llc encapsulation.  Of course, CSMA is not Ethernet, and it could be argued that the maximum frame size in a CSMA device is coincidentally 1526 bytes not 1518 and we just don't use those extra eight bytes in the default case :-)  

That said, it's obvious that having a maximum frame size attribute is not very popular, so I'll defer to the majority on this one.
Comment 15 Tom Henderson 2010-04-07 16:10:46 EDT
(In reply to comment #14)

> 
> The corollary is that *any* consistency checks for attributes *must* be done
> after Simulator::Run() is called and not during configuration time.  This
> should be advertized somewhere fairly prominently.  I think that demanding that
> attributes be completely independent at *all* times is too restrictive.  It
> seems clear to me that attributes can have dependencies, though, so consistency
> checking at run-time seems appropriate.

I agree with these observations: 1) document this prominently somewhere, 2) we cannot escape any dependencies between attributes in general so run-time checking is needed

> 
> With that in mind, having an mtu and a maximum frame size attribute (like in
> Linux) is still possible -- the consistency checks just need to be done while
> the device is running.  I still think having a maximum frame size is important
> because of the following:
> 
> Regarding the proposed patch, be aware that if you set the mtu to the default
> 1500 bytes and select LLC/SNAP framing you are going to be capable of
> generating illegally long Ethernet packets!
> 
>   MTU 1500, DIX encapsulation works out to 1518 byte frames.
>   MTU 1492, LLC/SNAP encapsulation works out to 1518 byte frames.
>   MTU 1500, LLC/SNAP encapsulation works out to 1526 byte frames.
> 
> This is why the frame size was there in the first place; and the mtu from
> framesize calculation was there so a user didn't have to get out a calculator
> to set mtu properly.  The fact that this behavior can happen so easily should
> also be advertized somewhere prominently if maximum frame size is removed. 
> Would most people consider it a bug?  
> 
> It's a small error, but it is a modelling error if you think CSMA is Ethernet
> with respect to framing.  Having a maximum frame size attribute that derfaults
> to 1518 bytes would at least cause a runtime error if a user neglects to reduce
> MTU to 1492 when setting Llc encapsulation.  Of course, CSMA is not Ethernet,
> and it could be argued that the maximum frame size in a CSMA device is
> coincidentally 1526 bytes not 1518 and we just don't use those extra eight
> bytes in the default case :-)  
> 
> That said, it's obvious that having a maximum frame size attribute is not very
> popular, so I'll defer to the majority on this one.

I sympathize with the above; the main practical issue that I see in this specific case is that our abstract Csma device can represent, in practice, different varieties of Ethernet technology with different maximum frame sizes.  

If Csma was intended to model 10Base5 Ethernet specifically, then yes, I would argue we need to support the above restriction.  But various Ethernet frame sizes can range up to 9000 bytes.  

I think a more common use case from a usability perspective is that if a user wants to simulate jumbo frames, he or she would prefer to just do:
Config::SetDefault ("ns3::CsmaNetDevice::Mtu", UintegerValue ("9000"));
rather than set both an Mtu and a FrameSize attribute, and we should just document some disclaimer like the following in csma-net-device.h:

"The CsmaNetDevice does not model any particular Ethernet technology specifically, but Ethernet-like devices in general.  Since real-world Ethernet can support different frame sizes depending on the technology, this device does not enforce a maximum MTU.  It is possible to configure the MTU to a value that would not be supported by a particular underlying Ethernet technology."

I would be OK with setting an integer constant upper bound on the frame size such as 9100 bytes (since there are practical limits to frame sizes even for jumbo-capable devices due to the error checking capabilities of the codes); this would prevent obvious user error of setting it accidentally to very high values.  But I don't care strongly about this constant.
Comment 16 Mathieu Lacage 2010-04-08 03:16:44 EDT
(In reply to comment #14)
> Okay, I hadn't considered automatic attribute setter programs which can decide
> on their own setter execution order.  As you suggest, if there are any
> inter-attribute dependencies that can produce errors, these can fail in
> unexpected ways as the setters are called according to the convenience of the
> program, not the semantics of the attributes.
> 
> The corollary is that *any* consistency checks for attributes *must* be done
> after Simulator::Run() is called and not during configuration time.  This
> should be advertized somewhere fairly prominently.  I think that demanding that
> attributes be completely independent at *all* times is too restrictive.  It

Yes, in practice, the value of some attributes is valid only with regard to the value of other attributes. So, they are not independent strictly speaking.

The issue here is that we have conflicting requirements:
  - the value of attribute A is valid only with regard to the value of another attribute B.
  - we can't check consistency of both values when the Set happens because that makes the Setters have ordering requirements which breaks automatic setting programs
  - we can't check consistency upon entering ::Run because calls to Set can happen after ::Run is entered.

The only way to make this work is to check consistency upon _use_ of the values which is what you are describing below.

> With that in mind, having an mtu and a maximum frame size attribute (like in
> Linux) is still possible -- the consistency checks just need to be done while
> the device is running.  I still think having a maximum frame size is important

I think that the conclusion is that it would be nice if we could extend our existing models to automatically check consistency of the values they use upon each use. I will try to do this for csma and p2p for the encap+mtu attributes. 

What is interesting is that wifi (and I am pretty sure any non-trivial model) has similar problems but due to the complexity of checking consistency, we don't even try to do it and rely on the user to feed useful data in instead.
Comment 17 Mathieu Lacage 2010-04-08 04:05:39 EDT
(In reply to comment #16)

> I think that the conclusion is that it would be nice if we could extend our
> existing models to automatically check consistency of the values they use upon
> each use. I will try to do this for csma and p2p for the encap+mtu attributes. 

Based on tom's last comment, I suspect that this does not make much sense since it appears that we do want to allow long packet sizes to allow users to model the csma technology they want.

To summarize, it looks like what is really missing is for nicola to ack the wifi part. The attached patch does update the wimax part to use 1400 as the default mtu value.
Comment 18 Mathieu Lacage 2010-04-08 04:06:11 EDT
Created attachment 819 [details]
set wimax mtu default to 1400.
Comment 19 Mathieu Lacage 2010-04-08 04:10:09 EDT
Created attachment 820 [details]
update defaults as suggested by tom
Comment 20 Nicola Baldo 2010-04-08 08:16:10 EDT
(In reply to comment #13)
> (In reply to comment #11)
> > > Regarding wifi, how do you propose to handle MSDU aggregation?  A user may
> > > think that he/she can set the MTU on an 802.11n device to larger and will be
> > > blocked by the integer constant set to 2304.
> > 
> > I think that if users think this, they are seriously misleaded. i.e., this is
> > not what MSDU aggregation is about but I guess that is a question for our new
> > wifi maintainer. CCing him.
> 
> I thought I had read in the past that 802.11n aggregation allowed larger MTUs,
> but I will defer to the experts here because I could have misunderstood it.


my understanding of the standard is that the use of MSDU aggregation does not affect the requirement that a single MSDU has a maximum size of 2304. An A-MSDU can be larger, but it can only be created by sending to the MAC several smaller MSDUs (e.g., IP packets) which are then aggregated.

That said, I am ok with the proposed patch, because it is correct for wifi, and in general it simplifies things.
Comment 21 Tom Henderson 2010-04-08 09:40:00 EDT
(In reply to comment #17)
> (In reply to comment #16)
> 
> > I think that the conclusion is that it would be nice if we could extend our
> > existing models to automatically check consistency of the values they use upon
> > each use. I will try to do this for csma and p2p for the encap+mtu attributes. 
> 
> Based on tom's last comment, I suspect that this does not make much sense since
> it appears that we do want to allow long packet sizes to allow users to model
> the csma technology they want.

If we agree that Csma is abstracting many such underlying Ethernet technologies and don't want to restrict frame size, then we may not have a consistency issue here anymore.  

> 
> To summarize, it looks like what is really missing is for nicola to ack the
> wifi part. The attached patch does update the wimax part to use 1400 as the
> default mtu value.

What do you think about changing 0xffff defaults to 1500?
Comment 22 Mathieu Lacage 2010-04-08 11:44:57 EDT
(In reply to comment #21)

> What do you think about changing 0xffff defaults to 1500?

If the question is: "do I think it's a good idea ?", the answer is that I don't think it matters very much. It certainly is a better default from the perspective of being realistic.

If the question is: "why did you not do this ?", the answer is that I did just that in my last patch.
Comment 23 Tom Henderson 2010-04-08 11:59:08 EDT
(In reply to comment #22)
> (In reply to comment #21)
> 
> > What do you think about changing 0xffff defaults to 1500?
> 
> If the question is: "do I think it's a good idea ?", the answer is that I don't
> think it matters very much. It certainly is a better default from the
> perspective of being realistic.
> 
> If the question is: "why did you not do this ?", the answer is that I did just
> that in my last patch.

Ah, I see now (was looking at the "set wimax default to 1400" patch when I wrote that).
Comment 24 Craig Dowell 2010-04-08 15:23:40 EDT
> If we agree that Csma is abstracting many such underlying Ethernet
> technologies and don't want to restrict frame size, then we may not
> have a consistency issue here anymore.

Someone certainly has a consistency issue, it's just not us.

To paraphrase Mathieu, we have models with lots of knobs on them.  In some (many) cases, it's up to the end user designing the simulation to decide whether or not a given combination of knobs is meaningful / valid in his or her scenario.    This may or may not be news to our end users, but it has always been the case.

I am perfectly fine with this approach, favoring flexibility, but we just need to document very prominently that the knob settings need to be validated by the end user, since we aren't going to attempt it -- it will be very easy to configure a piece of the system that doesn't conform to an underlying spec.  If a user does something outrageous and impossible, ns-3 may abort at run-time, but we may go with the user's flow and try to do what he or she says even though it may mean something unusual is happening.

I support merging the above patch if we include words in appropriate places about how to deal with inter-related Attributes (i.e., no inter-Attribute interaction in a setter, and consistency checking only at simulator run time -- when the Attributes are used).  We also need to add words about how you can easily turn Attribute knobs to get non-standard behavior and it is up to the user to validate the results (even though that may be quite obvious to some of us).

I will add an example of knob-turning (the LLC/SNAP switch and super-jumbo packets in csma seem like good examples) and words of warning in the tutorial.
Comment 25 Craig Dowell 2010-04-09 20:31:04 EDT
Created attachment 823 [details]
Blurb for tutorial warning about non-real attribute settings

Added a patch with some proposed words reminding how easy it is to use attributes to model non-real/not-possible devices, and who's responsibility it is for that.
Comment 26 Tom Henderson 2010-04-10 16:49:52 EDT
applied Mathieu's latest patch:  changeset 8a5e1f9db873 
applied Craig's tutorial update