[Ns-developers] [ns3] socket API

[Tom Henderson]

Mathieu,

Inline below,

>-----Original Message-----
>From: Mathieu Lacage [mailto:mathieu.lacage at sophia.inria.fr]
>Sent: Thursday, May 15, 2008 08:09 AM
>To: 'Tom Henderson'
>Cc: ns-developers at isi.edu
>Subject: Re: [Ns-developers] [ns3] socket API
>
>(sorry for the late reply) 
>
>On Tue, 2008-05-13 at 22:45 -0700, Tom Henderson wrote:
>
>> > Do you have a better suggestion ? Otherwise, let's just use ENOMEM.
>> 
>> EMSGSIZE?  I think of ENOMEM as typically referring to failed malloc().
>
>good for me.

OK, I will change it.

>
>> >> I would support whatever consensus would emerge on whether to keep or
>> >> remove these variants, but my sense is that opinions still differ.
>> > 
>> > So, what do you suggest we do to make progress on this issue ? Not
>> > making a decision before merging and leaving your current tree as-is is
>> > making an implicit decision so, please, let's make this explicit before
>> > merging. 
>> > 
>> > The main argument for not implementing a SocketHelper as was discussed a
>> > week ago is potential user confusion about slightly-different and
>> > duplicated API: the _exact same_ argument applies equally to all the
>> > one-liner overloads of Send in socket.h and is the main reason why I
>> > originally proposed to remove them.
>> 
>> This is revisiting an old contentious issue (November developers 
>> meeting) and also seemed to not have universal agreement when raised on 
>> the list in April.  So I think the default should probably be to keep 
>> what we have unless discussion results in different consensus now.
>
>Since then, a number of issues have been raised, most notably the fact
>that we were wrongly trying to make our users believe that we were
>providing a posix socket API while we are not. 

I don't think we are wrongly trying to make our users believe we are providing a posix socket API; rather, we are trying to give our users a low-level socket API that both matches our software model and that matches longstanding API syntactical conventions, and therein lies the rub.  The issue is one of API syntax and naming where people expect certain semantics on the stream socket.

> It is based on these new
>discussions that I proposed that removal so, arguing that you would like
>to keep the current state based on discussions which predate these new
>arguments would be misleading. 
>

I certainly don't think it is misleading in any way.  Even in November when we discussed this, you had already suggested that a posix API overlay was in the future plans.  Moreover, I have not tried to shut down your suggestion to revisit this decision in any way, and I have tried to spend time to both develop it further and socialize it, on and off the list.  My post from a few days ago went into some detail along the lines of "this is what Mathieu is suggesting to do now" and asked for comment on the possible options, and no one provided any support (or dissent) on the list for it.  So, from a process standpoint, should we revert the previous agreement based on that?

>A better way to deal with this issue would be to revisit the previous
>decision and rationale based on the new discussions we have had recently
>about actual user confusion. Since I don't remember anything (I honestly
>don't) about the november discussion you allude to, it might be helpful
>to dig these and post them here: it might be that there is a showstopper
>which I forgot about. 

This was documented in the meeting minutes:
http://www.nsnam.org/docs/meetings/seattle07.summary.txt
http://www.nsnam.org/docs/meetings/seattle07.full.txt
I realize that you don't remember and I accept that, but you were party to the agreement.

>
>Another alternative is for you to state an explicit decision about this
>and ignore the pointless bickering we are into. I would be fine with
>this but I want to make progress and I don't want to see decisions being
>made implicitely.

If you want me to decide it now, I would have to say that I did not see other support from the list to revert the previous decision, and in the past, we have previously said that in ns-3, one of the design principles is "when in doubt, do what real implementations do."  Whereas George has said many times that he is opposed to removing (or not including) buffer variants for those socket calls, and Sam and Craig have both commented in the recent past about trying to reduce unfamiliar constructs in our APIs.

I have thought about this issue a lot over the past two months and came to the conclusion that there is no magic bullet that will make everyone happy (obviously), but we're just talking about supporting a few overloaded socket calls to make them look more familiar to C sockets programmers (who may not want to use the process variant for their applications).  Over the past two weeks, I started to think that the proposal of putting those calls into a helper API might just be pushing the problem around, if the helper API didn't do anything else special.  I will state again that I could move forward with any consensus that emerged, because I personally don't think it is a big issue, but I don't sense that opinions have changed.  

>
>> >> 3) How should socket options and ioctls be supported ?
>> >>
>> >> I implemented this along the lines of Mathieu's suggestion (define
>> >> SocketOptions class and aggregate the object to a socket) although
>> I
>> > 
>> > I looked at the code and did not find that class: it seems that you
>> have
>> > implemented the options in the socket subclasses directly.
>> 
>> see the top of sockets.h file
>
>I see.
>
>
>> >> The question arose as to where to put the defaults for these
>> options,
>> >> since class SocketFactory is abstract base class.  I implemented a
>> >> class SocketDefaults and made these attribute-based, and aggregated
>> a
>> >> SocketDefaults object to the Node (in
>> >> InternetStack::AddInternetStack()).
>> >>
>> >> Maybe there is a better implementation strategy that avoids this
>> >> class, but the above seems workable at least.
>> > 
>> > The problem with a single SocketDefaults class per node is that it
>> will
>> > have to contain all the defaults of all protocols, including the
>> > protocols developed by users which will kill one of the "features"
>> we
>> > have been working on for a long time, that is, the ability to add a
>> new
>> > type of socket/protocol to the system without having to patch
>> anything
>> > src/node/. So, that solution is really not an option. 
>> 
>> I was thinking that the SocketDefaults class just held "sockets"
>> level 
>> defaults such as SO_SNDBUF.  The per-protocol defaults could be
>> hosted 
>> by the respective socket factory objects.  I do not think such an 
>> approach requires to edit src/node.
>
>The issue here is that I can imagine sockets for which the "TxBufSize"
>option makes zero sense (for example, a netlink socket) so, I don't
>think you will be able to come up with a useful set of generic options
>which apply to everyone to stick in there.

I agree it may be simpler to keep the options aligned with the true capabilities of the respective socket types.  We have the issue you cite above already with TxBufSize which is not a functional socket option call for the current UdpSocket or PacketSocket.

>
>> For instance, if one implements SctpSocket with SctpSocketOptions, then 
>> SctpSocketOptions could be aggregated to SctpSocket by the SctpImpl 
>> factory.  A user holding Ptr<Socket> sock could call 
>> sock->GetObject<SctpSocketOptions> (); and then could use this class to 
>> set options on the SctpSocket.  There wouldn't be a need to edit class 
>> Socket either because the user could get the SctpSocket public API via 
>> GetObject().  Am I missing something?
>
>You are right.
>
>> > Given all the constraints we have here, I would like to suggest that,
>> > instead, we do something potentially simpler: for each subclass of
>> > Socket, define an attribute for each socket option relevant to that
>> > specific subclass and define its initial value there.   This will allow
>> > you to:
>> > 
>> >   - define any number of options for each socket subclass independentely
>> > from the other socket subclasses, hence allowing a user to create new
>> > socket subclasses without having to patch src/node/*
>> 
>> Again, I don't think my suggestion was requiring such a patch.
>
>I see this now.
>
>> 
>> > 
>> >   - override on a simulation-wide basis the initial value of each of
>> > these socket options with the Config::SetDefault function
>> > 
>> >   - if you want to allow the user of a socket to write code which sets
>> > or gets a socket option in the same way across multiple implementations
>> > of the same kind of socket, all you have to do is use the same attribute
>> > name and type to allow the following to work consistently:
>> > socket->SetAttribute ("RcvBufSize", UintegerValue (10));
>> 
>> Yes, but the Config::SetDefault is no longer generic, right?
>
>Not really. If you really think that RcvBufSize is a generic attribute,
>you could conceivably move it to the Socket base class and make it
>indirect through a pair of private virtual methods:
>
>class Socket
>{
>public:
>  static TypeId GetTypeId (void) {
>    static TypeTid tid = TypeId ("ns3::Socket")
>      ...
>      .AddAttribute ("RcvBufSize", "help text",
>                     UintegerValue (10),
>                     MakeUintegerAccessor (&Socket::GetRcvBufSize,
>                                           &Socket::SetRcvBufSize)
>                     ...)
>      ;
>    return tid;
>  }
>private:
>  virtual void SetRcvBufSize (uint32_t size) = 0;
>  virtual uint32_t GetRcvBufSize (void) = 0;

OK, yes, I agree.

>}
>
>> > The above proposal is not optimal since it will not allow you to specify
>> > a default value on a per-node basis but, I view this as a much lesser
>> > evil than the alternatives.
>> > 
>> 
>> If what I said above was correct (about how it could work) do you still 
>> view it as evil?
>
>Given your comments above, no, I do not see this as evil anymore
>(although I _really_ feel uncomfortable about these "generic" socket
>options stored in the class SocketOptions).
>
>I know that we discussed this at length last friday in my office but the
>more I look at this, the less I am convinced that we need the extra
>complexity (both from an implementation and from a user point of view)
>provided by these aggregated socket option classes. It seems just
>simpler to go with an attribute-based approach and I have a hard time
>convincing myself that the extra features provided by the aggregate
>classes are worth the extra pain. 
>
>To summarize, I tend to favor giving up a single feature (namely, the
>ability to specify per-node defaults) for a net decrease in terms of
>implementation and user-visible complexity. i.e., users don't have to
>call GetObject before calling SetAttribute or GetAttribute to set or get
>an option.
>

I would be fine to try to prototype that approach; I agree that it has the potential to simplify some things.

- Tom