[Ns-developers] NetDevice::SetAddress

[Tom Henderson]

>-----Original Message-----
>From: Mathieu Lacage [mailto:mathieu.lacage at sophia.inria.fr]
>Sent: Friday, June 26, 2009 01:35 AM
>To: 'Tom Henderson'
>Cc: ns-developers at ISI.EDU
>Subject: Re: [Ns-developers] NetDevice::SetAddress
>
>On Thu, 2009-06-25 at 16:11 +0000, Tom Henderson wrote:
>
>> >I can't comment on how this thing relates to the tap bridge (I did not
>> >look carefully at bug 569) but, I wanted to point out a couple of things
>> >about this change which make it look a bit fishy without more
>> >information. Hopefully, someone can chime in and comment on my concerns.
>> 
>> I am not sure of where to point you to past list discussions on this
>> but in the tracker recently for bug 569, I had suggested
>> NetDevice::SetAddress() while this current patch was cooking.
>
>I looked carefully at this specific change: my gut feeling is that it's
>horribly wrong because this change feels way too much like the dreaded
>NetDevice::SetChannel method I spent so much time killing. Sadly, I
>can't think of any reasonable alternative right now: my intuition is
>that changing the mac address of a device once it's constructed is wrong
>and that we should instead focus on initializing it to the right value
>in the first place from where we know the type of the address _and_ the
>underlying device (we could get a Mac48AddressFactory from
>TapBridgeHelper and feed it to another XXXHelper). 

I think I understand why you do not like this change, although not agreeing about being horribly wrong.  I thought about it some more today and discussed it with Craig and still haven't seen what seems to be a better way.  There is perhaps a general danger of allowing users to change mac addresses after simulation start, but we do offer all of these SetAddress methods at the subclass level already, so that danger exists prior to this change.

The problems that I see in initializing the underlying address to the right value in the first place are that there is no existing framework in place to pass these learned mac addresses (there may be dozens of them) into the ns-3 program and have them get associated to the right devices.  Plus, how to learn them from outside the containers?  In addition, it may impose a new requirement that ns-3 be started after the containers.

>Anyway, it's clearly
>not something I can invest a lot of time thinking about now, a couple of
>days before the release so, I am not going to push for a change there
>right now.

If you are concerned about exposure of this API, what about making this a protected member of NetDevice and making the TapBridge a friend?  

>
>A more important comment I have, though, is that it's probably not a
>good idea to rely exclusively on a comment in a bug report to signal an
>API change to other developers. With hindsight, I believe that you
>mentioned that change to me on the phone but it probably did not even
>cross my mind that to fix a bug in the TapBridge, you had to add API to
>the NetDevice base class. So, I am at fault for not spotting this
>potential change before but I think that, as a general rule, we should
>be more careful in discussing API changes for src/node/* on
>ns-developers. I know that it's more painful than private communications
>and I can't claim I am a reference when it gets to this but I think that
>we need to keep working on this.
>

Sorry, with hindsight, yes, I could have socialized this more prominently, but I mistakenly thought you would have reacted to the tracker comment and that you had this previous context. 

Tom