Skip to content
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.

Sign up for GitHub

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

Jump to bottom

allow special hostsubnets to force a vnid on egress packets #11817

Merged
merged 1 commit into from Nov 8, 2016
Merged

allow special hostsubnets to force a vnid on egress packets #11817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/sdn/api/plugin.go
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@ const (
EgressBandwidthAnnotation = "kubernetes.io/egress-bandwidth"
AssignMacvlanAnnotation = "pod.network.openshift.io/assign-macvlan"
AssignHostSubnetAnnotation = "pod.network.openshift.io/assign-subnet"
FixedVnidHost = "pod.network.openshift.io/fixed-vnid-host"
)

func IsOpenShiftNetworkPlugin(pluginName string) bool {
Expand Down
9 changes: 7 additions & 2 deletions pkg/sdn/plugin/controller.go
View file
Open in desktop
Expand Up @@ -396,8 +396,13 @@ func (plugin *OsdnNode) AddHostSubnetRules(subnet *osapi.HostSubnet) error {
otx := plugin.ovs.NewTransaction()

otx.AddFlow("table=1, priority=100, tun_src=%s, actions=goto_table:5", subnet.HostIP)
otx.AddFlow("table=8, priority=100, arp, nw_dst=%s, actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:%s->tun_dst,output:1", subnet.Subnet, subnet.HostIP)
otx.AddFlow("table=8, priority=100, ip, nw_dst=%s, actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:%s->tun_dst,output:1", subnet.Subnet, subnet.HostIP)
if vnid, ok := subnet.Annotations[osapi.FixedVnidHost]; ok {
otx.AddFlow("table=8, priority=100, arp, nw_dst=%s, actions=load:%s->NXM_NX_TUN_ID[0..31],set_field:%s->tun_dst,output:1", subnet.Subnet, vnid, subnet.HostIP)
otx.AddFlow("table=8, priority=100, ip, nw_dst=%s, actions=load:%s->NXM_NX_TUN_ID[0..31],set_field:%s->tun_dst,output:1", subnet.Subnet, vnid, subnet.HostIP)
} else {
otx.AddFlow("table=8, priority=100, arp, nw_dst=%s, actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:%s->tun_dst,output:1", subnet.Subnet, subnet.HostIP)
otx.AddFlow("table=8, priority=100, ip, nw_dst=%s, actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:%s->tun_dst,output:1", subnet.Subnet, subnet.HostIP)
}

err := otx.EndTransaction()
if err != nil {
Expand Down
19 changes: 15 additions & 4 deletions pkg/sdn/plugin/subnets.go
View file
Open in desktop
Expand Up @@ -3,6 +3,7 @@ package plugin
import (
"fmt"
"net"
"strconv"

log "github.com/golang/glog"

Expand Down Expand Up @@ -45,7 +46,7 @@ func (master *OsdnMaster) SubnetStartMaster(clusterNetwork *net.IPNet, hostSubne
return nil
}

func (master *OsdnMaster) addNode(nodeName string, nodeIP string) error {
func (master *OsdnMaster) addNode(nodeName string, nodeIP string, hsAnnotations map[string]string) error {
// Validate node IP before proceeding
if err := master.networkInfo.validateNodeIP(nodeIP); err != nil {
return err
Expand Down Expand Up @@ -76,7 +77,7 @@ func (master *OsdnMaster) addNode(nodeName string, nodeIP string) error {

sub = &osapi.HostSubnet{
TypeMeta: kapiunversioned.TypeMeta{Kind: "HostSubnet"},
ObjectMeta: kapi.ObjectMeta{Name: nodeName},
ObjectMeta: kapi.ObjectMeta{Name: nodeName, Annotations: hsAnnotations},
Host: nodeName,
HostIP: nodeIP,
Subnet: sn.String(),
Expand Down Expand Up @@ -174,7 +175,7 @@ func (master *OsdnMaster) watchNodes() {
// Node status is frequently updated by kubelet, so log only if the above condition is not met
log.V(5).Infof("Watch %s event for Node %q", delta.Type, name)

err = master.addNode(name, nodeIP)
err = master.addNode(name, nodeIP, nil)
if err != nil {
return fmt.Errorf("error creating subnet for node %s, ip %s: %v", name, nodeIP, err)
}
Expand Down Expand Up @@ -220,7 +221,17 @@ func (master *OsdnMaster) watchSubnets() {
log.Errorf("Error in deleting annotated subnet from master, name: %s, ip %s: %v", name, hostIP, err)
return nil
}
err = master.addNode(name, hostIP)
var hsAnnotations map[string]string
if vnid, ok := hs.Annotations[osapi.FixedVnidHost]; ok {
Copy link

Choose a reason for hiding this comment

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

This check is only valid for multi-tenant sdn plugin, should ignore for subnet plugin (osapi.IsOpenShiftMultitenantNetworkPlugin)

If we know that F5 with multi-tenant sdn plugin needs this hack, may be enforce osapi.FixedVnidHost to be present when osapi.AssignHostSubnetAnnotation is passed?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say it's fine to do this in both multitenant and single-tenant. Especially if we only support 0 for the fixed vnid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's probably useful for single tenant also because the tunnel id at the f5 side is configured by the admin separately. This code will allow us to comply to that setting.

vnidInt, err := strconv.Atoi(vnid)
if err == nil && vnidInt >= 0 && uint32(vnidInt) <= osapi.MaxVNID {
Copy link

Choose a reason for hiding this comment

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

err == nil && uint32(vnidInt) >= osapi.MinVNID && uint32(vnidInt) <= osapi.MaxVNID ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

MinVnid will not catch 0

hsAnnotations = make(map[string]string)
hsAnnotations[osapi.FixedVnidHost] = strconv.Itoa(vnidInt)
Copy link

Choose a reason for hiding this comment

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

Check hard-coded vnid is in the valid range (0 to 2^24-1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, fixing..

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason to allow arbitrary VNIDs? Without corresponding translation on the incoming side, this isn't very useful unless vnid==0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There may be a choice to have the f5 for a particular project only. Then we will have the facility to change the value per deployment.

} else {
log.Errorf("Vnid %s is an invalid value for annotation %s. Annotation will be ignored.", vnid, osapi.FixedVnidHost)
}
}
err = master.addNode(name, hostIP, hsAnnotations)
if err != nil {
log.Errorf("Error creating subnet for node %s, ip %s: %v", name, hostIP, err)
return nil
Expand Down