[Ns-developers] [ns3] socket API

[Mathieu Lacage]

hi tom,

comments below,

On Tue, 2008-05-13 at 16:44 +0000, Tom Henderson wrote:

> I did this mostly along the lines Mathieu suggested, except for:
> > * If the size of
> > * the next packet to return is bigger than maxSize, return 0 and
> > * set ENOMEM.
> 
> it seemed to me that ENOMEM is not the right errno response here, so
> that is an open issue.

Do you have a better suggestion ? Otherwise, let's just use ENOMEM.

> I also implemented the overloaded Recv (void) method (and Send())
> along the lines of Gustavo's suggestion on the list.
> 
> As for the implementation status of different socket types, finite
> recv should be supported on Udp and Packet socket types now (untested)
> but Tcp needs some further integration.


ok

> 
> 2) Finite send buffer
> 
> The API is mostly what Mathieu had suggested, except for this piece
> which has not changed.
> 
> > * return -1 if packet does not fit in tx buffer (current behavior
> is 
> > * to send the part of the packet which fits in the tx buffer).
> 
> Right now, it still takes a partial send() but I think Mathieu is
> correct to make this return -1 (will fix it later).

ok

> 
> Also, Mathieu remarked:
> > If we implement this, we should remove the SendTo method. Problem:
> if we
> > do this, we cannot specify the "to" address for
> > int Socket::SendTo (uint8_t *buffer, uint32_t size);
> > because there is no Ptr<Packet> to attach the "to" address to.
> >
> > Solution: get rid of the Socket::Send* methods which take raw
> buffers
> > and implement them exclusively at the "synchronous" layer.
> 
> I have not removed these raw buffer calls, although they could be
> replaced with Packet-like variants such as:
> 
> replace
> int Socket::Send (buffer, size); 
> with
> int Socket::Send (Create <Packet> (buffer, size));
> 
> and 
> 
> uint32_t size = Recv (buffer, len);
> with something like:
> uint32_t size = Recv (len)->CopyTo (buffer, len);
> 
> 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.

> As for the implementation status, finite send is supported on Tcp but
> not on Udp or Packet socket types (that is, you can set a finite
> buffer at the API but it has no effect on behavior).  For Tcp, a
> question has arisen as to how to encode the semantics of "infinite
> send" buffer.  Raj and I have discussed the relative merits of using
> the special value "0" to denote "infinite" send buffer.  I tend to
> prefer "maxint" to "0" to encode the concept of infinite buffer,
> because in practice we have to support some finite value else our data
> types will overflow.
> 
> In either case, I think we can make the default value for socket
> buffers "infinite" or "very large" and let users configure it downward
> if they want to model that aspect.

ok.


> 
> 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.

>  did not go so far as to implement (yet) the UdpSocketOptions or
> TcpSocketOptions class.  Users who hold a reference to a Ptr<Socket>
> can call GetObject() to get whichever one they want.  Then, calls to
> Set..() or Get..() these values indirect through the Socket base class
> to a setter or getter implemented in the socket implementation class.
> 
> Only two options are supported for now (just proof of concept),
> equivalent to SO_SNDBUF and SO_RCVBUF at the socket level.
> 
> 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. 

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/*

  - 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));

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.

> - Socket::Create (Ptr<Node> node, TypeId tid)
> 
> I added a helper method to avoid having users to create sockets
> through the socket factory.  Where one once typed:
> 
>   Ptr<SocketFactory> socketFactory = node->GetObject<SocketFactory>
> (tid);
>   m_socket = socketFactory->CreateSocket ();
> 
> now one can type:
>   m_socket = Socket::CreateSocket (node, tid);
> 
> However, we need to keep these socket factories around IMO to host the
> default values (attributes) for the various socket types.
> 
> Finally, I haven't developed further the idea of a socket helper class
> since the API seemed very similar to the class Socket API other than
> perhaps moving the raw buffer variants there; it seems to me that if
> the API is different in only that aspect, that the potential for
> confusion of having this additional class might outweigh the benefits.

ok

> 
> If there are no objections in principle to what is proposed in
> ns-3-dev-socket, I would like to start to marshal in these changes
> (perhaps tomorrow) so that it doesn't remain as a blocker.  If there
> are still further changes/discussions needed on portions of it,
> perhaps we can still start to marshal in the portions that are
> settled.

There are two remaining issues which need to be dealt with (decided
upon) before merging:
  - removal of overloaded Socket::Send
  - how do we deal with socket options

None of the two need to be necessarily fully implemented, but deciding
what we are going to do seems like a reasonable requirement before
merging.

regards,
Mathieu