Bug 320 - getsockname returns a port number in network byte order
getsockname returns a port number in network byte order
Status: RESOLVED FIXED
Product: nsc
Classification: Unclassified
Component: core
unspecified
All All
: P2 normal
Assigned To: Florian Westphal
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-08 12:43 EDT by Mathieu Lacage
Modified: 2008-11-19 16:38 EST (History)
3 users (show)

See Also:


Attachments
Make ns-3 use the current version of NSC (859 bytes, patch)
2008-11-19 15:37 EST, Sam Jansen
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mathieu Lacage 2008-09-08 12:43:59 EDT
it would make vastly more sense to return a port number in host byte order.
Comment 1 Sam Jansen 2008-09-11 13:15:17 EDT
This one is Florian's area.
Comment 2 Florian Westphal 2008-10-26 18:15:47 EDT
the API as-is SUCKS:
- its unexpected to get an integer filled in in network byte order.
- can't just convert the integer internally, since the address is also
in network byte order, so we'd have to convert the address inside
nsc, too 
- this API just doesn't work when you want to provide other information
  that could be stored in some sockaddr (e.g. flow label...)

Rationale:
Both getpeername and getsockname provided by NSC should be re-done.
I've posted a RFC patch that makes both use a struct sockaddr (its not that
simple due to network stack <-> host os conversion issues) to the ML:
http://mailman.isi.edu/pipermail/ns-developers/2008-October/004878.html

This of course means that the port (and address) will still be in
network byte order, but at least it will be more obvious then. The advantage
is also that it makes things simpler wrt. to other address families that might
be supported by future NSC releases.
Comment 3 Tom Henderson 2008-10-27 00:03:19 EDT
(In reply to comment #2)
> the API as-is SUCKS:
> - its unexpected to get an integer filled in in network byte order.
> - can't just convert the integer internally, since the address is also
> in network byte order, so we'd have to convert the address inside
> nsc, too 
> - this API just doesn't work when you want to provide other information
>   that could be stored in some sockaddr (e.g. flow label...)
> 
> Rationale:
> Both getpeername and getsockname provided by NSC should be re-done.
> I've posted a RFC patch that makes both use a struct sockaddr (its not that
> simple due to network stack <-> host os conversion issues) to the ML:
> http://mailman.isi.edu/pipermail/ns-developers/2008-October/004878.html
> 
> This of course means that the port (and address) will still be in
> network byte order, but at least it will be more obvious then. The advantage
> is also that it makes things simpler wrt. to other address families that might
> be supported by future NSC releases.
> 

This seems like a reasonable solution to me (to just adopt sockaddr in a standard manner at the API)
Comment 4 Mathieu Lacage 2008-10-28 04:08:36 EDT
looks good to me.
Comment 5 Sam Jansen 2008-11-18 01:01:20 EST
Florian has a good fix for this but it hasn't been pushed to NSC's mercurial yet.

Florian, would you like to push your fix? I can then do an NSC release and get it incorporated into this version of ns-3.
Comment 6 Florian Westphal 2008-11-18 16:39:04 EST
getpeername/sockname changes have been comitted to nsc repository.
Craig, do you want me to commit the ns-3 side change right now or
should I wait until sam releases the next nsc version?
Comment 7 Florian Westphal 2008-11-18 17:13:10 EST
Sam asked me to commit the ns-3 side of things as well.
Closing bug.
Comment 8 Sam Jansen 2008-11-19 15:36:36 EST
Reopening the bug until we get ns-3-dev depending on a released NSC version with this fix.

I've released NSC 0.5.0; I will announce this shortly. This includes this fix and another previous bug fix that is quite critical. We just need a one-line change to ns-3-dev to make it depend on this version (note that if you are using hg you already get the dev version, which includes our fixes).

Comment 9 Sam Jansen 2008-11-19 15:37:37 EST
Created attachment 307 [details]
Make ns-3 use the current version of NSC

Could somebody please apply this patch to ns-3-dev? We can then close this bug.
Comment 10 Florian Westphal 2008-11-19 16:38:42 EST
Applied, thanks.