Skip to content
This repository has been archived by the owner on Feb 24, 2020. It is now read-only.

networking: refactor and split KVM logic #3733

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

squeed
Copy link
Contributor

@squeed squeed commented Jun 29, 2017

This hopefully makes #3695 easier. It also enables cni v0.3 and ipv6.

@sstabellini

@squeed squeed changed the title Kvm net split Networking: split out KVM code. Jun 29, 2017
@lucab
Copy link
Member

lucab commented Jun 29, 2017

Travis is failing due to license header missing on the two new files.

@sstabellini
Copy link

sstabellini commented Jun 29, 2017 via email

@squeed
Copy link
Contributor Author

squeed commented Jun 30, 2017

@sstabellini Thanks for the feedback; I've made that method public. PTAL.

It's not that I dislike xen, it's just that it's a good test-case for making the networking modular :-).

@sstabellini
Copy link

sstabellini commented Jun 30, 2017

It works now, without any additional changes on my side. Thank you!

One thing though: I don't see XenDelNetwork being called at all. For example, I don't think it gets called when I do `rkt stop'.

@lucab lucab changed the title Networking: split out KVM code. networking: refactor and split KVM logic Jul 18, 2017
@lucab
Copy link
Member

lucab commented Jul 18, 2017

@squeed is away for some days, so I'm moving this to the next milestone. Also assigning @sstabellini for a review once this is fixed, to double-check that everything he needs is in place.

@sstabellini
Copy link

I'll do, thanks!

anet.Runtime.IP4.Routes,
cnitypes.Route{Dst: *defaultNet, GW: anet.Runtime.IP4.Gateway},
)
config.IsGw = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the answer for above question is no - this is unnecessary line, and this if could be merged with next one.

}
ifName := link.Attrs().Name
n.runtime.IfName = ifName
if config.IsGw {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't there be anet.Conf.IsDefaultGateway?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you might be right, good catch.

@squeed
Copy link
Contributor Author

squeed commented Jul 26, 2017

@sstabellini The network is torn down (and thus XenDelNetwork called) at rkt gc time. Hopefully that clears that up.

@jellonek
Copy link
Contributor

jellonek commented Aug 3, 2017

Any progress in this PR?

@jellonek
Copy link
Contributor

jellonek commented Aug 9, 2017

@sstabellini ping.

@squeed
Copy link
Contributor Author

squeed commented Aug 9, 2017

Hi there,
Sorry, I've been really swamped with other work. Sadly, my changes also have a nasty merge conflict. I'm working on resolving the merge conflict, but it will be some time. If you have some spare time, merging in master would be a great help.

@lucab lucab modified the milestones: 1.29.0, 1.30.0 Sep 21, 2017
@iaguis iaguis modified the milestones: 1.30.0, v1.31.0 Apr 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants