i686-apple-darwin9-gcc-4.0.1

Processor Name:	Intel Core 2 Duo
System Version:	Mac OS X 10.5.4 (9E17)
Kernel Version:	Darwin 9.4.0

/usr/include/netinet/ip.h

That is the file that must be different in macosx.  It sounds like this header defines incomplete types in macosx.  Should a header file from nsc be included instead?  If so, from which of the network stacks should it be included?

My best guess is that one needs some magic define to get the
struct iphdr definition on OSX. Since we don't need to access everything in
the ip header, we could use uint32_t[5] instead and avoid including
netinet/ip.h completely.

I'd rather not include stack specific headers; that might cause a lot of trouble
depending on the build OS.

(In reply to comment #3)

> the ip header, we could use uint32_t[5] instead and avoid including
> netinet/ip.h completely.

would you care to attach a patch which does this to see what the impact of doing this would be ?

Created attachment 244 [details]
Do not use struct iphdr

(In reply to comment #5)
> Created an attachment (id=244) [details]
> Do not use struct iphdr


ok with me if you add a small comment explaining what you are doing in the code and why :)

Any objections to making nsc glue code conditionally compiled again?  MingW will never compile again without this change.

(In reply to comment #7)
> Any objections to making nsc glue code conditionally compiled again?  MingW
> will never compile again without this change.
> 

Why can't mingw compile this code ?

(In reply to comment #8)
> (In reply to comment #7)
> Why can't mingw compile this code ?

No unix socket headers:

[333/533] cxx: src\internet-stack\nsc-tcp-socket-impl.cc -> build\debug\src\inte
rnet-stack\nsc-tcp-socket-impl_1.o
..\src\internet-stack\nsc-tcp-socket-impl.cc:36:24: sys/socket.h: No such file o
r directory
..\src\internet-stack\nsc-tcp-socket-impl.cc:37:24: netinet/in.h: No such file o
r directory
..\src\internet-stack\nsc-tcp-socket-impl.cc:38:23: arpa/inet.h: No such file or
 directory
..\src\internet-stack\nsc-tcp-socket-impl.cc:39:24: netinet/ip.h: No such file o
r directory
..\src\internet-stack\nsc-tcp-socket-impl.cc:40:25: netinet/tcp.h: No such file
or directory
..\src\internet-stack\nsc-tcp-socket-impl.cc: In member function `virtual int ns
3::NscTcpSocketImpl::Connect(const ns3::Address&)':
..\src\internet-stack\nsc-tcp-socket-impl.cc:310: error: aggregate `ns3::in_addr
 remoteAddr' has incomplete type and cannot be defined
..\src\internet-stack\nsc-tcp-socket-impl.cc:316: error: `inet_ntoa' was not dec
lared in this scope
..\src\internet-stack\nsc-tcp-socket-impl.cc:316: warning: unused variable 'inet
_ntoa'
..\src\internet-stack\nsc-tcp-socket-impl.cc: In member function `virtual int ns
3::NscTcpSocketImpl::Send(ns3::Ptr<ns3::Packet>, uint32_t)':
..\src\internet-stack\nsc-tcp-socket-impl.cc:347: warning: converting of negativ
e value `-0x000000001' to `unsigned int'
..\src\internet-stack\nsc-tcp-socket-impl.cc: In member function `void ns3::NscT
cpSocketImpl::CompleteFork()':
..\src\internet-stack\nsc-tcp-socket-impl.cc:500: error: `ntohs' was not declare
d in this scope
..\src\internet-stack\nsc-tcp-socket-impl.cc:500: warning: unused variable 'ntoh
s'
..\src\internet-stack\nsc-tcp-socket-impl.cc: In member function `void ns3::NscT
cpSocketImpl::ConnectionSucceeded()':
..\src\internet-stack\nsc-tcp-socket-impl.cc:535: error: `ntohs' was not declare
d in this scope
..\src\internet-stack\nsc-tcp-socket-impl.cc:535: warning: unused variable 'ntoh
s'
Build failed
 -> task failed (err #129): [bld://P:\ns\ns-3-dev\src\internet-stack\nsc-tcp-soc
ket-impl_1.o]

Created attachment 245 [details]
Rip out all socket includes

Gustavo, could you please try this patch? Thanks.

After your patch the compilation errors in that file were reduced to a single one, I think it was reported already:

[333/533] cxx: src\internet-stack\nsc-tcp-socket-impl.cc -> build\debug\src\inte
rnet-stack\nsc-tcp-socket-impl_1.o
..\src\internet-stack\nsc-tcp-socket-impl.cc: In member function `virtual int ns
3::NscTcpSocketImpl::Send(ns3::Ptr<ns3::Packet>, uint32_t)':
..\src\internet-stack\nsc-tcp-socket-impl.cc:345: warning: converting of negativ
e value `-0x000000001' to `unsigned int'
Build failed

This patch fixes that error:

diff -r 00938a81ad10 src/internet-stack/nsc-tcp-socket-impl.cc
--- a/src/internet-stack/nsc-tcp-socket-impl.cc	Sat Sep 06 21:15:59 2008 -0700
+++ b/src/internet-stack/nsc-tcp-socket-impl.cc	Sun Sep 07 15:00:49 2008 +0100
@@ -344,7 +342,14 @@
           {
              if (m_errno == ERROR_AGAIN)
                {
-                 return txEmpty ? p->GetSize () : -1;
+                 if (txEmpty)
+                   {
+                     return p->GetSize ();
+                   }
+                 else
+                   {
+                     return -1;
+                   }
                }
              if (txEmpty)
                {

But then I get more errors when compiling the next source file:


[334/533] cxx: src\internet-stack\nsc-tcp-l4-protocol.cc -> build\debug\src\inte
rnet-stack\nsc-tcp-l4-protocol_1.o
..\src\internet-stack\nsc-tcp-l4-protocol.cc:39:19: dlfcn.h: No such file or dir
ectory
..\src\internet-stack\nsc-tcp-l4-protocol.cc: In destructor `virtual ns3::NscTcp
L4Protocol::~NscTcpL4Protocol()':
..\src\internet-stack\nsc-tcp-l4-protocol.cc:93: error: `dlclose' was not declar
ed in this scope
..\src\internet-stack\nsc-tcp-l4-protocol.cc:93: warning: unused variable 'dlclo
se'
..\src\internet-stack\nsc-tcp-l4-protocol.cc: In member function `void ns3::NscT
cpL4Protocol::SetNscLibrary(const std::string&)':
..\src\internet-stack\nsc-tcp-l4-protocol.cc:100: error: `RTLD_NOW' was not decl
ared in this scope
..\src\internet-stack\nsc-tcp-l4-protocol.cc:100: error: `dlopen' was not declar
ed in this scope
..\src\internet-stack\nsc-tcp-l4-protocol.cc:102: error: `dlerror' was not decla
red in this scope
..\src\internet-stack\nsc-tcp-l4-protocol.cc:102: warning: unused variable 'dler
ror'
..\src\internet-stack\nsc-tcp-l4-protocol.cc:100: warning: unused variable 'RTLD
_NOW'
..\src\internet-stack\nsc-tcp-l4-protocol.cc:100: warning: unused variable 'dlop
en'
..\src\internet-stack\nsc-tcp-l4-protocol.cc: In member function `void ns3::NscT
cpL4Protocol::SetNode(ns3::Ptr<ns3::Node>)':
..\src\internet-stack\nsc-tcp-l4-protocol.cc:117: error: `dlsym' was not declare
d in this scope
..\src\internet-stack\nsc-tcp-l4-protocol.cc:117: warning: unused variable 'dlsy
m'
Build failed
 -> task failed (err #129): [bld://P:\ns\ns-3-dev\src\internet-stack\nsc-tcp-l4-
protocol_1.o]


Naturally, dlopen and friends are not available in MinGW.

By the way, the patch is not sound:

-    m_remotePort = ntohs(port);
+    m_remotePort = nsc_bytswap_16 (port);

ntohs is _not_ a simple byte swap.  It only swaps bytes on little endian architectures.

(In reply to comment #12)
> By the way, the patch is not sound:
> 
> -    m_remotePort = ntohs(port);
> +    m_remotePort = nsc_bytswap_16 (port);
> 
> ntohs is _not_ a simple byte swap.  It only swaps bytes on little endian
> architectures.

no, it is safe. It is just suboptimal because it will use shifts and masks to access a value when it could just read it.

> 

(In reply to comment #13)
> (In reply to comment #12)
> > By the way, the patch is not sound:
> > 
> > -    m_remotePort = ntohs(port);
> > +    m_remotePort = nsc_bytswap_16 (port);
> > 
> > ntohs is _not_ a simple byte swap.  It only swaps bytes on little endian
> > architectures.
> 
> no, it is safe. It is just suboptimal because it will use shifts and masks to
> access a value when it could just read it.

It's safe?  I must be missing something.  Clearly ntohs and nsc_bytswap_16 yield different results in x86 and PowerPC?  Are you saying the code is safe even on PowerPC, or that you don't expect it to be used on PowerPC systems?

Well, we could say that, if ntohs swaps bytes and htons also swaps, then the system will be self-consistent as long as no packet is transmitted to another computer.  However, even then things will appear wrong if you capture packets to pcap file and display them in wireshark.

Gustavo is right wrt. the byteswap. I shouldn't churn out patches without thinking.
To make some progress here:
Since dlopen() isn't available anyway, its pointless to try to make this
work by removing the socket includes. So, I vote for the following:
- Revert 3635:cddd59578812 ('compile nsc code unconditionally.')
- Apply 'Do not use struct iphdr' patch to fix OSX build.

Perhaps we should also 'blacklist' mingw so --enable-nsc prints an error message?

(In reply to comment #15)
> Gustavo is right wrt. the byteswap. I shouldn't churn out patches without
> thinking.
> To make some progress here:
> Since dlopen() isn't available anyway, its pointless to try to make this
> work by removing the socket includes. So, I vote for the following:
> - Revert 3635:cddd59578812 ('compile nsc code unconditionally.')
> - Apply 'Do not use struct iphdr' patch to fix OSX build.
> 
> Perhaps we should also 'blacklist' mingw so --enable-nsc prints an error
> message?
> 

It's not impossible to support the equivalent of dlopen in mingw, using win32 API: http://msdn.microsoft.com/en-us/library/ms682599(VS.85).aspx

But I think developing this for MingW so close to NS 3.2 is risky, and besides does NSC itself even compile in MingW?  If not, I don't see the point on NS-3 supporting NSC in MingW.

I am +1 for leaving nsc glue code compiled in by default, but only if the needed header files are available on the system.

(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > By the way, the patch is not sound:
> > > 
> > > -    m_remotePort = ntohs(port);
> > > +    m_remotePort = nsc_bytswap_16 (port);
> > > 
> > > ntohs is _not_ a simple byte swap.  It only swaps bytes on little endian
> > > architectures.
> > 
> > no, it is safe. It is just suboptimal because it will use shifts and masks to
> > access a value when it could just read it.
> 
> It's safe?  I must be missing something.  Clearly ntohs and nsc_bytswap_16

ok, you are right: I misread the code. However, looking at this patch carefully, I noticed that it looks like getpeername seems to return a port number _in network byte order_. Is this is true ? If so, this is utterly broken !

(In reply to comment #15)
> Gustavo is right wrt. the byteswap. I shouldn't churn out patches without
> thinking.
> To make some progress here:
> Since dlopen() isn't available anyway, its pointless to try to make this
> work by removing the socket includes. So, I vote for the following:
> - Revert 3635:cddd59578812 ('compile nsc code unconditionally.')

Rather than revert that patchset (which added the sim_* headers in the src/internet-stack directory per sam's suggestion), I would like to apply the attached patch.

> - Apply 'Do not use struct iphdr' patch to fix OSX build.
> 
> Perhaps we should also 'blacklist' mingw so --enable-nsc prints an error
> message?

yes, please.

Created attachment 246 [details]
make nsc compilation conditional again to support mingwin.

Comment on attachment 246 [details]
make nsc compilation conditional again to support mingwin.

this patch is untested (no build machine available right now)

(In reply to comment #17)
> ok, you are right: I misread the code. However, looking at this patch
> carefully, I noticed that it looks like getpeername seems to return a port
> number _in network byte order_. Is this is true ? If so, this is utterly
> broken !

Sigh. This originally used a struct sockaddr*, just like the system call of the
same name, until i realized that struct sockaddr is different depending on
the stack. The port number is whats stored in sockaddr_in->sin_port,
which is in network byte order.

PLEASE PLEASE PLEASE don't use this bug entry to (rightfully) yell at me for
coming up with a crap API, do so on the mailing list so we can create a saner
NSC API.

(and for the record, I agree that having it in nb order is just stupid.)

(In reply to comment #21)

> which is in network byte order.
> 
> PLEASE PLEASE PLEASE don't use this bug entry to (rightfully) yell at me for
> coming up with a crap API, do so on the mailing list so we can create a saner
> NSC API.

ok, I will. I merely pointed this out because if it was in host order, we would not be having this discussion about ntohs :)

(In reply to comment #20)
> (From update of attachment 246 [details])
> this patch is untested (no build machine available right now)

The patch works fine on mingw.  Build doesn't complete, but that's due to a different problem, reported as bug #318.

applied conditional nsc patch.