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

IPv6 support (work in progress) #70

Merged
merged 160 commits into from Dec 12, 2014

Conversation

Projects
None yet
5 participants
@nojb
Contributor

nojb commented Oct 3, 2014

Started working on adding IPv6 support (see #43). Right now there is very little code, but I wanted to put it here so that there is a central place for discussion for those interested.

Right now the code only handles incoming tcp and udp payloads when there are no ipv6 extensions present. I am looking into implementing icmp6 echo handling next.

@mor1

This comment has been minimized.

Member

mor1 commented Oct 3, 2014

cool! notifying @yomimono as she had been interested in this too :)

@yomimono

This comment has been minimized.

Member

yomimono commented Oct 3, 2014

Excellent! Thanks for the ping @mor1, and for your work @nojb . :)

@nojb

This comment has been minimized.

Contributor

nojb commented Oct 6, 2014

It is ... alive!

@avsm

This comment has been minimized.

Member

avsm commented Oct 6, 2014

Yay!!

On 6 Oct 2014, at 14:32, Nicolas Ojeda Bar notifications@github.com wrote:

It is ... alive!


Reply to this email directly or view it on GitHub.

@mor1

This comment has been minimized.

Member

mor1 commented Oct 6, 2014

cool. though it's now simply a Small Matter of Programming until the
nanobots take over of course.

http://xkcd.com/865/

On 6 October 2014 20:01, Anil Madhavapeddy notifications@github.com wrote:

Yay!!

On 6 Oct 2014, at 14:32, Nicolas Ojeda Bar notifications@github.com
wrote:

It is ... alive!

Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHub
#70 (comment).

Richard Mortier
mort@cantab.net

@avsm

This comment has been minimized.

Member

avsm commented Nov 2, 2014

releasing 2.0.0 now, so can merge this to the master branch as wip quite soon...

@nojb

This comment has been minimized.

Contributor

nojb commented Nov 3, 2014

Great! There is still some more work to do before this patch is ready for general consumption. At some point I will also submit a PR for an IPV6 signature for V2.mli.

Also related is integrating this to the existing (IPv4) codebase (making a common base signature for IPV4 and IPV6 and making UDP & TCP functors of that). This will require changes both to mirage-tcpip and mirage-types. But I am focused on getting IPv6 working well before concentrating on this.

@nojb nojb referenced this pull request Nov 10, 2014

Merged

RFC: Initial support for IPv6 #319

nojb added some commits Oct 2, 2014

Rearrange code
Icmpv6 and Ipv6 will be implemented in the same module, and Ndv6 will be
implemented in a different module, mirroring the existing Ipv4/Arpv4
arrangement.
Allocate frame in `nd_ns_output'
This required shuffling code around quite a bit and in particular
removing the module `Ndpv6' and `Routing' from the picture, leaving a
single, monolithic module `Ipv6'.

The reason is that IPv6, ICMPv6, and NDPv6 are actually all protocols
lying on the same layer and there are a lot of cyclic dependencies
between them.  Later on it will be necessary to figure out a way to
decouple the different modules but for now it is simpler to put
everything in the same namespace.
@nojb

This comment has been minimized.

Contributor

nojb commented Nov 30, 2014

I would suggest to merge this now (together with mirage/mirage#319). I reverted the changes to STACKV4 so that we can merge this with a minimum of disruption (in particular, no changes are needed for conduit and any layer above that). The only visible interface changes are:

  • IPV4.set_ipv4_* functions have been renamed IPV4.set_ip_* because they are shared between IPV4 and IPV6.
  • IPV4.get_ipv4 and get_ipv4_netmask now return a list of Ipaddr.V4.t (again because this is the common semantics with IPV6.)
  • Several types that had v4 in their names (like IPV4.ipv4addr) have lost that particle.

In mirage/mirage#319 I added some basic support for configuring IPv6 for those who want to experiment with it, but reverted any change to STACKV4. I would feel better if we merged this first, and then turned the attention to how to integrate/replace the STACKV4 signature. I have been working on that, but any change is going to be fairly disruptive and more discussion is needed anyway, as opposed to the current change set which should be pretty uncontroversial.

I also have a patch ready to update mirage-skeleton with respect to the small notational changes listed above.

PS: it would also be great if I could rearrange the directory structure of mirage-tcpip in a more systematic fashion, if no one is opposed to it.

@mor1

This comment has been minimized.

Member

mor1 commented Nov 30, 2014

fwiw, i'm never opposed to rearrangement to increase systematicness (if
that's even a word ;)

On 30 November 2014 at 00:18, Nicolas Ojeda Bar notifications@github.com
wrote:

I would suggest to merge this now (together with mirage/mirage#319
mirage/mirage#319). I reverted the changes to
STACKV4 so that we can merge this with a minimum of disruption (in
particular, no changes are needed for conduit and any layer above that).
The only visible interface changes are:

  • IPV4.set_ipv4_* functions have been renamed IPV4.set_ip_* because
    they are shared between IPV4 and IPV6.
  • IPV4.get_ipv4 and get_ipv4_netmask now return a list of Ipaddr.V4.t
    (again because this is the common semantics with IPV6.)

In mirage/mirage#319 mirage/mirage#319 I added
some basic support for configuring IPv6 for those who want to experiment
with it, but reverted any change to STACKV4. I would feel better if we
merged this first, and then turned the attention to how to
integrate/replace the STACKV4 signature. I have been working on that, but
any change is going to be fairly disruptive and more discussion is needed
anyway, as opposed to the current change set which should be pretty
uncontroversial.

I also have a patch ready to update mirage-skeleton with respect to the
small notational changes listed above.

PS: it would also be great if I could rearrange the directory structure of
mirage-tcpip in a more systematic fashion, if no one is opposed to it.

Reply to this email directly or view it on GitHub
#70 (comment).

Richard Mortier
mort@cantab.net

@avsm

This comment has been minimized.

Member

avsm commented Dec 12, 2014

Am seeing bad udp checksums (previously we didnt set them at all). Causes DHCP to drop. Investigating...

11:58:16.833225 IP (tos 0x0, ttl 38, id 8120, offset 0, flags [none], proto UDP (17), length 580)
    0.0.0.0.bootpc > broadcasthost.bootps: [bad udp cksum 0xeffa -> 0xfda0!] BOOTP/DHCP, Request from 52:c2:b4:0c:cf:32 (oui Unknown), length 552, xid 0x108d58df, secs 10, Flags [none] (0x0000)

@nojb

This comment has been minimized.

Contributor

nojb commented Dec 12, 2014

Yes, we must set the UDP checksum because it is mandatory for IPv6. Strange, didn't see this in my testing. Will investigate on my side, it is probably an errant packet offset calculation. Will report back here.

@nojb

This comment has been minimized.

Contributor

nojb commented Dec 12, 2014

I think that should fix the bug - but cannot test right now. Could you test to see if bug is gone?

@nojb

This comment has been minimized.

Contributor

nojb commented Dec 12, 2014

I can confirm now that checksum calculation for UDP seems to be working. I tried a DHCP example and checksum was being calculated correctly. Either way, please let me know if it also works for you.

@avsm

This comment has been minimized.

Member

avsm commented Dec 12, 2014

I have lunch and the bug is fixed. This is great burp! :-)

@avsm avsm merged commit df16025 into mirage:master Dec 12, 2014

http://tools.ietf.org/html/rfc3810
*)
module Ipaddr = Ipaddr.V6

This comment has been minimized.

@avsm

avsm Dec 12, 2014

Member

This introduces a 4.02.0 dependency I think, due to a module alias? Needs to work on 4.01.0 as well.

This comment has been minimized.

@nojb

nojb Dec 12, 2014

Contributor

No, I do not think so. If I understand correctly the 4.02.0 module alias stuff only refines the typing of expressions such as module M = ....

This comment has been minimized.

@nojb

nojb Dec 13, 2014

Contributor

You are right, this does not compile in 4.01.0. Submitted a fix in #91.

This comment has been minimized.

@avsm

avsm Dec 13, 2014

Member

Thanks!

On 13 Dec 2014, at 14:16, Nicolas Ojeda Bar notifications@github.com wrote:

In lib/ndpv6.ml:


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment