Enhance network functions #597

WeiZhang555 · 2017-09-12T07:16:01Z

Fixes part of #591
Carry #533

Mainly add "interface" command and some sub-commands to implement a first-class network interface operation.

Note that "remove interface" command rely on hyperstart PR: hyperhq/hyperstart#318

Sorry that this hasn't been rebased to latest master, but I think it will still be useful.

Add runv interface command to support CNI networks: This is initial commit, after it's done, it's supposed to be able to: * add new interface into a runv container, with specified ip, mac, mtu, and tapname. * remove an interface from runv container. * update configuration of an interface. * list all network interfaces. Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

Add interface for setting mtu. Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

This commit enables setting new name for network interface in guest VM. Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

Previous implementation includes one issue that if we add some interfaces with different new name, it will fail sometimes. This is caused by the rename machanism in Guest VM. All devices have an inital device name counting from eth0, eth1... to ethN, then we rename it to some new name such as myDev1 or eth123, we need to pass device in format eth0, eth1... too. For example, we insert first interface with device name eth0 and new name eth2, guest kernel will first find newly added device eth0, then rename it to eth2. Latter comming new nic device would go back to eth0. So for adding a new nic, we should give it same device name eth0, then rename it to something else like eth3. In a word, try this example: ``` $ runv interface add --name eth2 --ip xxx ... $ runv interface add --name eth4 ... $ runv interface add --name eth8 ... $ runv interface add --name eth3 ... ``` If it works, everything would be fine! Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

Add command `interface ls` to list all interfaces in a container. Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

This commit enables using existing tap device in host, we can use `runv interface add --tapname xxx` to specify preferred tap device. This is useful when you want more customized feature on your tapdevice and give you more control over runv. Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

Now we support some interface now Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

This commit enables interface update, now we can add ip address, delete ip address and update mtu of an interface Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

Conflicts: cli/network.go hyperstart/api/grpc/hyperstart.pb.go hyperstart/api/grpc/hyperstart.proto hyperstart/api/json/spec.go hyperstart/libhyperstart/grpc.go hyperstart/libhyperstart/hyperstart.go hyperstart/libhyperstart/json.go hyperstart/proxy/proxy.go hypervisor/events.go hypervisor/network.go hypervisor/network/network.go hypervisor/network/network_linux.go hypervisor/qemu/qemu.go hypervisor/qemu/qmp_wrapper_amd64.go hypervisor/vbox/network.go hypervisor/vm_states.go

Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

gao-feng · 2017-09-26T01:29:11Z

cli/interface.go

+		infListCommand,
+	},
+	Before: func(context *cli.Context) error {
+		return cmdPrepare(context, true, context.Bool("detach"))


Should context.Bool("detach") be changed to true?

Sure. I'll do it.

gao-feng · 2017-09-26T01:32:05Z

cli/interface.go

+		return cmdPrepare(context, true, context.Bool("detach"))
+	},
+	Action: func(context *cli.Context) error {
+		return nil


Action function should be cli.ShowSubcommandHelp(context)?

gao-feng · 2017-09-26T02:02:22Z

cli/interface.go

+	ArgsUsage: `add <container-id>`,
+	Flags: []cli.Flag{
+		cli.StringFlag{
+			Name:  "tapname",


I think hostdev may be general name, tapname is too specialized name.

gao-feng · 2017-09-26T02:02:33Z

cli/interface.go

+		}
+		defer releaseFunc()
+
+		ip := context.String("ip")


why getting ip separately here?

gao-feng · 2017-09-26T02:36:32Z

cli/interface.go

+
+		conf := &api.InterfaceDescription{
+			Id:   "-1",
+			Name: context.String("name"),


What's the value of context.String("name")? is it existed?

~~Oh, my fault, this should be context.String("tapname"),~~

I modify some logic in this upstream PR, and forgot to update tap setting logic.

Sorry, misread my own code :( My first implementation want to support updating interface name such as eth1, eth2, after I'm using interface name as the index and uniq key for each interface record, updating name turns out to be impossible.

So i need to remove this now.

gao-feng · 2017-09-26T02:44:15Z

cli/network.go

 			Bridge: fakeBridge,
 			Ip:     info.Ip,
+			Name:   info.Name,
+			Mac:    info.Mac,


So nic in vm and nic in netns will share the same mac address, I think this is ok, can you check if this will cause some problem in network test?

I'll keep tracking of this, so far we didn't see any harm

gao-feng · 2017-09-26T03:10:35Z

hypervisor/network.go


 func (nc *NetworkContext) configureInterface(index, pciAddr int, name string, inf *api.InterfaceDescription, result chan<- VmEvent) {
 	if inf.TapName == "" {
-		inf.TapName = network.NicName(nc.sandbox.Id, index)


If you want to change the tapname, please update network.NicName function, other place rely on this naming rule.

gao-feng · 2017-09-26T03:20:12Z

hypervisor/network/network_linux.go

-	if err = addToBridge(iface, master, options); err != nil {
-		glog.Errorf("cannot add %s to %s ", name, bridge)
-		return err
+	if bridge != "" {


In which case BridgeIface is null? the bridge suppose not to be null here.

From previous logic:

201 if bridge == "" { 202 bridge = BridgeIface 203 }

if we pass an empty bridge, it will fall down to BridgeIface, in my test, it's still empty which means BridgeIface is empty.

I didn't look deeply into this, in my environment, I can see there's one new runv0 bridge, and no hyper0 bridge, not sure if this is related.

gao-feng · 2017-09-26T03:28:22Z

hypervisor/qemu/network.go

 	)

+	// check if tapname exists, if true, use existing one instead of create new one
+	if device != "" {


device should not be null, and even this device is existed on host, it should be added to bridge too.

btw, use TUNSETIFF of ioctl on tun fd with existed device name will attach the fd to the tap device,
so I think we don't need this change.

I'll change it back.

Because I don't know how to use TUNSETIFF, I choose a more direct way. But it's ok to me to remove this part of logic.

gao-feng · 2017-09-26T06:24:25Z

hypervisor/network.go

+	defer nc.slotLock.Unlock()
+
+	if inf.Name != "" {
+		oldInf.NewName = inf.Name


Is it necessary to add a new field NewName?Can not save new name to DeviceName?

There's one trouble here, I add a NewName for modifying device name in Guest OS.

in struct InterfaceCreated, there are 3 fields in my codes:

HostDevice string DeviceName string NewName string

HostDevice is for tap name

In GuestOS, if we want to add a new interface named eth5, and there are two existing network interface named eth0 and eth3, when we insert a new tap device, the new added device will be named eth1 in Guest, so we should use "eth1" as DeviceName which will be used by hyperstart to search for new added network interface.

then we user eth5 as NewName, hyperstart will rename eth1 to eth5. So in guest, we have 3 nic now: eth0, eth3 and eth5, next time, newly added tap device will count from eth1 again.

That's why I need to add a NewName field.

In above example, HostDevice is tap name, DeviceName is eth1 and NewName is eth5. I can't remove NewName and just set DeviceName=eth5, we still need to know the eth1 name.

That's why it's so complicated and confusing .

Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

WeiZhang555 · 2017-09-27T08:53:44Z

ping @gao-feng Updated.

gao-feng · 2017-09-27T09:17:40Z

let's merge it now, thanks for your work!

it changed the struct definition, then reverted, but leave this file changed. Signed-off-by: Wang Xu <gnawux@gmail.com>

Enhance network functions

it changed the struct definition, then reverted, but leave this file changed. Signed-off-by: Wang Xu <gnawux@gmail.com>

WeiZhang555 added 8 commits September 12, 2017 11:53

Support setting mtu for network interface

7ff8dfb

Add interface for setting mtu. Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

Support modify network interface name

373c4d6

This commit enables setting new name for network interface in guest VM. Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

Add interface ls command

50bd8ed

Add command `interface ls` to list all interfaces in a container. Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

Add "interface rm" command

84e1f86

Now we support some interface now Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

Support update interface

a87b6c4

This commit enables interface update, now we can add ip address, delete ip address and update mtu of an interface Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

WeiZhang555 mentioned this pull request Sep 12, 2017

Add more rich network functions #533

Closed

WeiZhang555 added 2 commits September 21, 2017 14:18

Fix errors during merge

d40bd49

Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

WeiZhang555 changed the title ~~Cni network support up~~ Enhance network functions Sep 25, 2017

gao-feng reviewed Sep 26, 2017

View reviewed changes

cli/interface.go Outdated

}

defer releaseFunc()

ip := context.String("ip")

Copy link

Contributor

gao-feng Sep 26, 2017

Choose a reason for hiding this comment

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

why getting ip separately here?

gao-feng reviewed Sep 26, 2017

View reviewed changes

Fix some review comments

01239f9

Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

gao-feng merged commit 6a1c8bf into hyperhq:master Sep 27, 2017

gnawux mentioned this pull request Nov 24, 2017

grpc: Extend the network API kata-containers/agent#4

Merged

gnawux mentioned this pull request Dec 12, 2017

Problem when compiling hyperd(for xen) from source hyperhq/hyperd#678

Closed

gnawux added a commit to gnawux/runv that referenced this pull request Dec 13, 2017

fix xen build failure by hyperhq#597

3a8721b

it changed the struct definition, then reverted, but leave this file changed. Signed-off-by: Wang Xu <gnawux@gmail.com>

This was referenced Dec 13, 2017

Fix xen build #646

Merged

fix xen build hyperhq/hyperd#691

Merged

jimoosciuc pushed a commit to jimoosciuc/runv that referenced this pull request May 26, 2020

Merge pull request hyperhq#597 from WeiZhang555/cni-network-support-up

ed77d3b

Enhance network functions

jimoosciuc pushed a commit to jimoosciuc/runv that referenced this pull request May 26, 2020

fix xen build failure by hyperhq#597

80a7df5

it changed the struct definition, then reverted, but leave this file changed. Signed-off-by: Wang Xu <gnawux@gmail.com>

Enhance network functions #597

Enhance network functions #597

Uh oh!

Conversation

WeiZhang555 commented Sep 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WeiZhang555 Sep 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WeiZhang555 commented Sep 27, 2017

Uh oh!

gao-feng commented Sep 27, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

WeiZhang555 commented Sep 12, 2017 •

edited

Loading

WeiZhang555 Sep 27, 2017 •

edited

Loading