[Ns-developers] TCP and UDP layer refactoring

[Sébastien Vincent]

Mathieu Lacage wrote:
> On Fri, 2008-07-04 at 13:22 +0200, Sébastien Vincent wrote:
>
>   
>> II) Independant EndPoint
>>
>> - Have a single class EndPoint (and remove Ipv6EndPoint and Ipv4EndPoint).
>> - The members m_localAddr an m_peerAddress will be stored as Address
>> - Remove Ipv4Address argument from m_rxCallBack and SetRxCallback (see VI)).
>>     
>
> I think that, generally, if the address is really needed in a couple of
> places, passing it as explicit arguments is cleaner than storing it in a
> hidden tag.
>
>   
Ok so we will have InternetL4Protocol::Receive(Address src, Address dst);

For the "incomingInterface" issue, we filter at socket level ?
This will need to add a *dst* parameter in *SocketImpl::Forward() and 
then compare it to m_endPoint->GetLocalAddress()

Am I right ?

>> - Add a AfType (AF_INET or AF_INET6) to know the type of address.
>>
>> III) StorageSocketAddress
>>
>> Some functions in *SocketImpl use InetSocketAddress or 
>> Inet6SocketAddress. We can avoid this by having a class that have the 
>> same methods
>> than InetSocketAddress but with Address attributes called 
>> StorageSocketAddress. This is similar to the "struct sockaddr_storage" in C.
>>     
>
> Another way to do this would be to do what tom suggested and use Address
> and test for its type before calling one of these functions. If you find
> yourself writing this test code over and over again, you can also push
> it to a couple of helper internal functions such as:
>
> void Inet4SetPort (Address &address, uint16_t port);
> ...
>
>   
>> StorageSocketAddress
>>     -> AfType type
>>     -> Address addr
>>     -> uint16_t port
>>     -> ConvertToInetSocketAddress (???)
>>     -> ConvertToInet6SocketAddress (???)
>>
>> The InetSocketAddress and Inet6SocketAddress should have a operator 
>> StorageSocketAddress() to map automatically its contents to a 
>> StorageSocketAddress and set the AfType.
>> -> No changes to existing applications.
>>     
>
> We (I) have learned the hard way that adding more (and multiple)
> implicit conversion operators on a single class is a really bad idea so,
> let's forget this, please: I don't want to have to debug this later.
>
>
>   

OK, as Tom suggested, use Address should be sufficient.

>> IV) *SocketImpl
>>
>> - Use StorageSocketAddress;
>> - Use Address instead of Ipv*Address;
>> - In DoSendTo (Ptr<Packet> p, Address daddr, uint16_t dport), after
>>
>>   if (m_endPoint == 0)
>>     {
>>       if (Bind () == -1)
>>   {
>>           NS_ASSERT (m_endPoint == 0);
>>     return -1;
>>   }
>>       NS_ASSERT (m_endPoint != 0);
>>     }
>>   if (m_shutdownSend)
>>     {
>>       m_errno = ERROR_SHUTDOWN;
>>       return -1;
>>     }
>>
>> check m_endPoint->GetAfType() and do IPv6 or IPv4 stuff;
>>
>> - Add Ipv*SocketAddressesTag before sending to UdpL4Protocol.
>>
>>
>> V) Ipv*L3Protocol
>>
>> - Add the Ipv*SocketAddressesTag in ::ForwardUp();
>> - Add Ipv*InterfaceTag in ::ForwardUp().
>>
>> VI) *L4Protocol
>>
>> - Have Ipv6EndPointDemux and Ipv4EndPointDemux or one EndPointDemux (see 
>> VII));
>> - Remove Ipv*Address in ::Receive signature, use tag instead (added in 
>> Ipv*L3Protocol);
>> - Remove Ipv*Address in ::Send() signature, use tag instead (added in 
>> *SocketImpl::DoSendTo());
>> - Check tag (if/else) in ::Receive, and do appropriate behavior 
>> (checksum, *EndPointDemux);
>> - Check tag (if/else) in ::Send, and do appropriate behavior (checksum, 
>> send to the right L3 stack);
>> - Remove Ipv*Address in ::ForwardUp() signature, check 
>> Ipv*SocketAddressesTag and get the source addresse, create a 
>> StorageSocketAddress, and put in SocketAddressTag;
>> - Allocate() method takes Address arguments and return EndPoint*.
>>
>> VII) Ipv*EndPointDemux
>>
>> First solution :
>> - Keep Ipv6EndPointDemux and Ipv4EndPointDemux;
>> - In ::Lookup(), get the Ipv*InterfaceTag, remove incomingInterface and 
>> do stuff as usual with the address/netmask in the tag.
>>
>> Second solution
>> - Have a single EndPointDemux;
>> - Overload Lookup function (one for IPv6 and one for IPv4);
>> - Two set of EndPoints list (one for IPv6 and one for IPv4);
>> - Have a AllocateEphemeralPort() and AllocateEphemeralPort6() (because a 
>> service can be associated for a distinct IPv4 socket and a IPv6 socket);
>> - Remove Ipv6EndPointDemux and Ipv4EndPointDemux.
>>     
>
> ok. But, at this point, I have to ask: is there really some code shared
> in this class after you have done all that ? I mean, it looks like
> everything in this class is going to be if (ipv4) {do} else if (ipv6)
> {do}.
>   

I think we will see better with a prototype version.

>   
>> VIII) Other
>>
>> - When we *L4Protocol::CreateSocket(), it will give a socket that could 
>> be bound in IPv4 or IPv6 so no need to add Udp6/Tcp6SocketFactory;
>>     
>
> I am worried that the above is a bit too far away from the behavior of a
> real socket. i.e., when I create a socket with the 'socket' system call,
> I specify a family (AF_INET or AF_INET6). If so, what happens if I
> create a socket with AF_INET and give an inet6 address to bind ?
>   

Like the real implementation, syscall returns -1 and set errno : Bad 
address or something like that.

> regards,
> Mathieu
>
>
>   

Tomorrow I will begin a prototype version for UDP/TCP refactoring with 
the ideas of the comments.

--
Sebastien Vincent
Network and Protocol Team - University of Strasbourg, France