Re: Csma, ethernet length field, NSC and the linkmtu...

[Tom Henderson-2]

>-----Original Message-----
>From: craigdo@... [mailto:craigdo@...]
>Sent: Wednesday, July 23, 2008 01:30 PM
>To: ''Tom Henderson'', ''Mathieu Lacage''
>Cc: ''ns-developers''
>Subject: RE: [Ns-developers] Csma, ethernet length field, NSC and the     linkmtu...
>
>
>> I suggest to avoid "MTU" term for anything but the (Mac) MTU,
>> or else we have another round of ifIndex problems.  
>
>Smile.  I was already caught up in an ifIndex equivalent problem here :-)
>
>Consider it done.  I'm dealing with a problem in the emulated net device
>now, but I'll fix up both point-to-point and csma to reflect these
>discussions this afternoon).
>

This will be a case where we change a default value, and possibly rename an attribute (don't know whether attribute renaming is in your plan).  Please make a recommendation, when you are done with this exercise, on how we can document and minimize impact along the lines of "avoid making subtle, transparent changes to the simulator underneath users".

Thanks,
Tom

[Mathieu Lacage]

On Wed, 2008-07-23 at 21:59 +0000, Tom Henderson wrote:

> >-----Original Message-----
> >From: craigdo@... [mailto:craigdo@...]
> >Sent: Wednesday, July 23, 2008 01:30 PM
> >To: ''Tom Henderson'', ''Mathieu Lacage''
> >Cc: ''ns-developers''
> >Subject: RE: [Ns-developers] Csma, ethernet length field, NSC and the     linkmtu...
> >
> >
> >> I suggest to avoid "MTU" term for anything but the (Mac) MTU,
> >> or else we have another round of ifIndex problems.  
> >
> >Smile.  I was already caught up in an ifIndex equivalent problem here :-)
> >
> >Consider it done.  I'm dealing with a problem in the emulated net device
> >now, but I'll fix up both point-to-point and csma to reflect these
> >discussions this afternoon).
> >
>
> This will be a case where we change a default value, and possibly
> rename an attribute (don't know whether attribute renaming is in your
> plan).  Please make a recommendation, when you are done with this
> exercise, on how we can document and minimize impact along the lines
> of "avoid making subtle, transparent changes to the simulator
> underneath users".

I don't think we have any attributes for these MTUs in p2p and csma.
There is one in wifi/wifi-mac.cc and it is named MaxMsduSize

regards,
Mathieu

[craigdo]

Hi all,

I took a quick cut at beginning to resolve this situation this afternoon.
The affected files are csma-net-device.{cc,h} of course.  The change
required updating the reference traces for csma-related regression tests.

I've added two attributes to the csma net device,

    .AddAttribute ("PayloadLength",
       "The max PHY-level payload length of packets sent over this device.",
       UintegerValue (DEFAULT_FRAME_LENGTH),
       MakeUintegerAccessor (&CsmaNetDevice::m_maxPayloadLength),
       MakeUintegerChecker<uint16_t> ())

    .AddAttribute ("MTU",
       "The MAC-level MTU (client payload) of packets sent over this
device.",
       UintegerValue (DEFAULT_MTU),
       MakeUintegerAccessor (&CsmaNetDevice::m_mtu),
       MakeUintegerChecker<uint16_t> ())

The default frame length is 1500 and the default mtu is 1492.

As you can see, I don't have any custom checker to ensure the attributes are
correctly "intertwined."  We need to make sure that two attributes on an
object are synchronized (one possibly changes along with another) and
consistent.  That seems a great subject for a HOWTO, so I think I'll work on
that tomorrow taking the current code as a base.

Anyway, the code in csma-net-device.cc that started this whole discussion
ended up as:

  switch (m_encapMode)
    {
    case IP_ARP:
      NS_LOG_LOGIC ("Encapsulating packet as IP_ARP (type interpretation)");
//
// This corresponds to the type interpretation of the lengthType field.
//
      lengthType = protocolNumber;
      break;
    case ETHERNET_V1:
      NS_LOG_LOGIC ("Encapsulating packet as ETHERNET_V1 "
        "(length interpretation)");
//
// This corresponds to the length interpretation of the lengthType field.
// The ethernet header and trailer are not counted, see RFC 1042 and
// http://standards.ieee.org/getieee802/download/802.3-2005_section1.pdf,
// Section 3.2.6 a.  We just include the size of the "payload."
//
      lengthType = p->GetSize ();
      NS_ASSERT_MSG (lengthType <= m_maxPayloadLength,
        "CsmaNetDevice::AddHeader(): 802.3 Length/Type field: "
        "length interpretation must not exceed device max payload length");
      break;
    case LLC:
      {
        NS_LOG_LOGIC ("Encapsulating packet as LLC (length
interpretation)");

        LlcSnapHeader llc;
        llc.SetType (protocolNumber);
        p->AddHeader (llc);
//
// This corresponds to the length interpretation of the lengthType field,
// but with an LLC/SNAP header added to the payload.
//
        lengthType = llc.GetSerializedSize () + p->GetSize ();
        NS_ASSERT_MSG (lengthType <= m_maxPayloadLength,
          "CsmaNetDevice::AddHeader(): 802.3 Length/Type field with
LLC/SNAP: "
          "length interpretation must not exceed device max payload
length");
      }
      break;
    case RAW:
      NS_LOG_LOGIC ("Encapsulating packet as RAW");
      NS_ASSERT (false);
      break;
    }

  NS_LOG_LOGIC ("header.SetLengthType (" << lengthType << ")");

Does this solve the basic problems to everyone's satisfaction?  I can change
the default to DIX at the end of the process if you like.

-- Craig

[Florian Westphal-4]

craigdo@... <craigdo@...> wrote:

>     case LLC:
>       {
>         NS_LOG_LOGIC ("Encapsulating packet as LLC (length
> interpretation)");
>
>         LlcSnapHeader llc;
>         llc.SetType (protocolNumber);
>         p->AddHeader (llc);
> //
> // This corresponds to the length interpretation of the lengthType field,
> // but with an LLC/SNAP header added to the payload.
> //
>         lengthType = llc.GetSerializedSize () + p->GetSize ();

The llc header has already been added to the packet here, so its
size is already accounted for by p->GetSize(), no?

Florian

[craigdo]

[ ... ]

> >         lengthType = llc.GetSerializedSize () + p->GetSize ();
>
> The llc header has already been added to the packet here, so its
> size is already accounted for by p->GetSize(), no?

Sigh, yes, thanks.  Fixed.  Trying to do too much at one time ...

-- C