-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use full range for sequence numbers #374
Conversation
additionally, remove unused type ethif from Arpv4 module
408e151
to
feca22b
Compare
…res ocaml 4.03, which provides result in Pervasives)
feca22b
to
0397b6c
Compare
0397b6c
to
a294769
Compare
let localport = | ||
10000 + (Randomconv.int (fun x -> Random.generate x) ~bound:10000) | ||
1024 + (Randomconv.int ~bound:(0xFFFF - 1024) Random.generate) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This patch looks like a definite improvement to me!
Out of curiosity, do we have to choose local ports > 1024? Is this a Unix convention or a TCP convention? (No big deal, I'm just interested)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, of course any port number is fine, according to the RFCs[tm]. in practise, there are servers which refuse to service you if your source port range is < 1024 (I experienced this with some authoritative nameservers) - thus I'd (for now at least) avoid to use something < 1024 -- but then our UDP implementation takes any source port -- maybe we should unify these two expressions!?
related to this is hannesm@562e6e4 together with hannesm/mirage@a24e88a and hannesm/charrua@56be972 and hannesm/mirage-qubes@4286ee7 -- which adds RANDOM (and MCLOCK, see #373 -- I want to avoid to have to do the same set of releases again next week just because I didn't integrate MCLOCK yet) to the Static_ipv4 interface. if someone could tell me that's the right direction or not, I'm happy to incorporate these 4 commits tomorrow in the (anyway) pending releases (towards 3.2.0) /cc @mirage/core the issue at hand is: the IPv4 stack (Static_ipv4, via Ipv4_common) still uses OCaml's Random number generator, while we have this nice Mirage RANDOM device, which can be used either with stdlib-random or fortuna/nocrypto. |
included in #371, together with the mentioned IPv4 functor adjustment. |
(on top of #371, relevant commit is only the last one)