bug 7722: open vswitch - refine code; add normal flow makes vswitch work as

normal L2/L3, otherwise the output packet cannot find right output port

api/src/com/cloud/network/ovs/OvsCreateGreTunnelAnswer.java

@@ -14,13 +14,13 @@ public class OvsCreateGreTunnelAnswer extends Answer {
 	}
 	
 	public OvsCreateGreTunnelAnswer(Command cmd, boolean success,
-			String details, String hostIp, String remoteIp, String bridge,
-			String key) {
+			String details, String hostIp, String bridge) {
 		super(cmd, success, details);
+		OvsCreateGreTunnelCommand c = (OvsCreateGreTunnelCommand)cmd;
 		this.hostIp = hostIp;
-		this.remoteIp = remoteIp;
 		this.bridge = bridge;
-		this.key = key;
+		this.remoteIp = c.getRemoteIp();
+		this.key = c.getKey();
 	}
 	
 	public String getHostIp() {

core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java

@@ -534,9 +534,8 @@ public abstract class CitrixResourceBase implements ServerResource {
     
     private Network setupvSwitchNetwork(Connection conn) {
 		try {
-			Network vswitchNw = null;
-			
 			if (_host.vswitchNetwork == null) {
+				Network vswitchNw = null;
 				Network.Record rec = new Network.Record();
 				String nwName = Networks.BroadcastScheme.VSwitch.toString();
 				Set<Network> networks = Network.getByNameLabel(conn, nwName);
@@ -551,14 +550,9 @@ public abstract class CitrixResourceBase implements ServerResource {
 
 				enableXenServerNetwork(conn, vswitchNw, "vswitch",
 						"vswicth network");
-				_host.vswitchNetwork = vswitchNw.getUuid(conn);
-			} else {
-				vswitchNw = Network.getByUuid(conn, _host.vswitchNetwork);
-				enableXenServerNetwork(conn, vswitchNw, "vswitch",
-						"vswicth network");
-			}
-			
-			return vswitchNw;
+				_host.vswitchNetwork = vswitchNw;
+			} 
+			return _host.vswitchNetwork;
 		} catch (Exception e) {
 			e.printStackTrace();
 		}
@@ -3860,7 +3854,7 @@ public abstract class CitrixResourceBase implements ServerResource {
     		for (String log: logs){
             	String [] info = log.split(",");
             	if (info.length != 5) {
-            		s_logger.warn("Wrong element number in ovs log");
+            		s_logger.warn("Wrong element number in ovs log(" + log +")");
             		continue;
             	}
             	
@@ -3916,7 +3910,6 @@ public abstract class CitrixResourceBase implements ServerResource {
     	Connection conn = getConnection();
     	String bridge = "unkonwn";	
     	try {
-    		//TODO: we may store vswtich network to _host
     		Network nw = setupvSwitchNetwork(conn);
     		bridge = nw.getBridge(conn);
     		
@@ -3924,17 +3917,16 @@ public abstract class CitrixResourceBase implements ServerResource {
 					"op", "createGRE", "bridge", bridge,
 					"remoteIP", cmd.getRemoteIp(), "greKey", cmd.getKey());
 			if (result.equalsIgnoreCase("SUCCESS") || result.equalsIgnoreCase("TUNNEL_EXISTED")) {
-				return new OvsCreateGreTunnelAnswer(cmd, true, result);
+				return new OvsCreateGreTunnelAnswer(cmd, true, result, _host.ip, bridge);
 			} else {
 				return new OvsCreateGreTunnelAnswer(cmd, false, result,
-						_host.ip, cmd.getRemoteIp(), bridge, cmd.getKey());
+						_host.ip, bridge);
 			}
     	} catch (Exception e) {
     		e.printStackTrace();
     	}
     	
-		return new OvsCreateGreTunnelAnswer(cmd, false, "EXCEPTION", _host.ip,
-				cmd.getRemoteIp(), bridge, cmd.getKey());
+		return new OvsCreateGreTunnelAnswer(cmd, false, "EXCEPTION", _host.ip, bridge);
     }
     
     private Answer execute(SecurityIngressRulesCmd cmd) {
@@ -5713,7 +5705,7 @@ public abstract class CitrixResourceBase implements ServerResource {
         public String publicNetwork;
         public String privateNetwork;
         public String linkLocalNetwork;
-        public String vswitchNetwork;
+        public Network vswitchNetwork;
         public String storageNetwork1;
         public String storageNetwork2;
         public String guestNetwork;

scripts/vm/hypervisor/xenserver/vlanRemapUtils.py

@@ -349,6 +349,10 @@ def delDropFlow(vlan):
 	delFlow = ["ovs-ofctl del-flows %s" % bridge, '"%s"' % param]
 	doCmd(delFlow)
 
+def formatNormalFlow():
+	flow = "priority=0 idle_timeout=0 hard_timeout=0 actions=normal"
+	return flow
+
 def formatDHCPFlow(bridge, inPort, vlan, ports):
 	outputs = ''
 	for i in ports:
@@ -436,6 +440,12 @@ def createFlow (bridge, vifName, mac, remap):
 				addflow = ["ovs-ofctl add-flow", param]
 				doCmd (addflow)
 
+	# add normal flow make switch work as L2/L3 switch
+	flow = formatNormalFlow()
+	param = bridge + ' "%s"' % flow
+	addflow = ["ovs-ofctl add-flow", param]
+	doCmd (addflow)
+
 	result = errors["SUCCESS"]
 	return 0
 ######################## End Flow creation utils ##########################
@@ -538,6 +548,12 @@ def doDeleteFlow(bridge, ofports, macs, remap):
 			addflow = ["ovs-ofctl add-flow", param]
 			doCmd (addflow)
 
+	# add normal flow make switch work as L2/L3 switch
+	flow = formatNormalFlow()
+	param = bridge + ' "%s"' % flow
+	addflow = ["ovs-ofctl add-flow", param]
+	doCmd (addflow)
+
 def checkArgNum(num):
 	if len (sys.argv) < num:
 		result = errors["ERR_ARGS_NUM"]

scripts/vm/hypervisor/xenserver/vmops

@@ -682,10 +682,13 @@ def ovs_get_vm_log(session, args):
             ovs_handle_rebooted_vm(session, name)
             if name.startswith('i-'):
                 info = ovs_get_common_info_from_log(name)
+                if info == None:
+                    util.SMlog("ovs_get_common_info_from_log return None for %s" % name)
+                    continue
                 result.append(info)
     except Exception, e:
         util.SMlog(e.__str__())
-        util.SMlog("OVs failed to get rule logs, better luck next time!")
+        util.SMlog("OVs failed to get logs, better luck next time!")
 
     return ";".join(result)
 
@@ -757,7 +760,7 @@ def ovs_get_domid_vifrs_hostuuid(session, vm_name):
 
     except:
         util.SMlog("### Failed to get domid or vif list for vm  ##" + vm_name)
-        return (-1, [])
+        return (-1, [], "-1")
 
 def ovs_set_tag_and_flow(session, args):
     bridge = args.pop('bridge')

server/src/com/cloud/network/element/OvsElement.java

@@ -1,103 +0,0 @@
-package com.cloud.network.element;
-
-import java.util.List;
-import java.util.Map;
-
-import javax.ejb.Local;
-
-import com.cloud.deploy.DeployDestination;
-import com.cloud.exception.ConcurrentOperationException;
-import com.cloud.exception.InsufficientCapacityException;
-import com.cloud.exception.ResourceUnavailableException;
-import com.cloud.network.Network;
-import com.cloud.network.Networks;
-import com.cloud.network.PublicIpAddress;
-import com.cloud.network.Network.Capability;
-import com.cloud.network.Network.Provider;
-import com.cloud.network.Network.Service;
-import com.cloud.network.Networks.BroadcastDomainType;
-import com.cloud.network.ovs.OvsNetworkManager;
-import com.cloud.network.rules.FirewallRule;
-import com.cloud.offering.NetworkOffering;
-import com.cloud.utils.component.AdapterBase;
-import com.cloud.utils.component.Inject;
-import com.cloud.vm.NicProfile;
-import com.cloud.vm.ReservationContext;
-import com.cloud.vm.VirtualMachine;
-import com.cloud.vm.VirtualMachineProfile;
-
-@Local(value=NetworkElement.class)
-public class OvsElement extends AdapterBase implements NetworkElement {
-	@Inject OvsNetworkManager _ovsNetworkMgr;
-	
-	@Override
-	public Map<Service, Map<Capability, String>> getCapabilities() {
-		// TODO Auto-generated method stub
-		return null;
-	}
-
-	@Override
-	public Provider getProvider() {
-		// TODO Auto-generated method stub
-		return null;
-	}
-
-	@Override
-	public boolean implement(Network network, NetworkOffering offering,
-			DeployDestination dest, ReservationContext context)
-			throws ConcurrentOperationException, ResourceUnavailableException,
-			InsufficientCapacityException {
-		// TODO Auto-generated method stub
-		return true;
-	}
-
-	@Override
-	public boolean prepare(Network network, NicProfile nic,
-			VirtualMachineProfile<? extends VirtualMachine> vm,
-			DeployDestination dest, ReservationContext context)
-			throws ConcurrentOperationException, ResourceUnavailableException,
-			InsufficientCapacityException {
-		VirtualMachine instance = vm.getVirtualMachine();
-		
-		if (network.getTrafficType() != Networks.TrafficType.Guest ||
-			instance.getType() == VirtualMachine.Type.DomainRouter) {
-			return true;
-		}	
-		
-		//_ovsNetworkMgr.CheckAndUpdateDhcpFlow(network, vm.getVirtualMachine());
-		return true;
-	}
-
-	@Override
-	public boolean release(Network network, NicProfile nic,
-			VirtualMachineProfile<? extends VirtualMachine> vm,
-			ReservationContext context) throws ConcurrentOperationException,
-			ResourceUnavailableException {
-		// TODO Auto-generated method stub
-		return true;
-	}
-
-	@Override
-	public boolean shutdown(Network network, ReservationContext context)
-			throws ConcurrentOperationException, ResourceUnavailableException {
-		// TODO Auto-generated method stub
-		return true;
-	}
-
-	@Override
-	public boolean applyIps(Network network,
-			List<? extends PublicIpAddress> ipAddress)
-			throws ResourceUnavailableException {
-		// TODO Auto-generated method stub
-		return true;
-	}
-
-	@Override
-	public boolean applyRules(Network network,
-			List<? extends FirewallRule> rules)
-			throws ResourceUnavailableException {
-		// TODO Auto-generated method stub
-		return true;
-	}
-
-}

server/src/com/cloud/network/ovs/OvsNetworkManager.java

@@ -34,8 +34,6 @@ public interface OvsNetworkManager extends Manager {
 			VirtualMachineProfile<DomainRouterVO> profile,
 			DeployDestination dest);
 
-	public void CheckAndUpdateDhcpFlow(Network nw, VirtualMachine vm);
-
 	public void handleVmStateTransition(VMInstanceVO userVm, State vmState);
 
 	public void RouterCheckAndCreateTunnel(Commands cmds,

server/src/com/cloud/network/ovs/OvsNetworkManagerImpl.java

@@ -377,7 +377,7 @@ public class OvsNetworkManagerImpl implements OvsNetworkManager {
 	
 	//FIXME: if router has record in database but not start, this will hang 10 secs due to host
 	//plugin cannot found vif for router.
-	public void CheckAndUpdateDhcpFlow(VMInstanceVO instance) {
+	protected void CheckAndUpdateDhcpFlow(VMInstanceVO instance) {
 		if (!_isEnabled) {
 			return;
 		}
@@ -397,48 +397,15 @@ public class OvsNetworkManagerImpl implements OvsNetworkManager {
 			String tag = Long.toString(_vlanMappingDao.findByAccountIdAndHostId(router.getAccountId(),
 							router.getHostId()).getVlan());
 			VmFlowLogVO log = _flowLogDao.findOrNewByVmId(instance.getId(), instance.getName());
-			_agentMgr.send(router.getHostId(), new OvsSetTagAndFlowCommand(
-					router.getName(), tag, vlans, Long.toString(log.getLogsequence()), instance.getId()));
 			s_logger.debug("ask router " + router.getName() + " on host "
 					+ router.getHostId() + " update vlan map to " + vlans);
+			_agentMgr.send(router.getHostId(), new OvsSetTagAndFlowCommand(
+					router.getName(), tag, vlans, Long.toString(log.getLogsequence()), instance.getId()));	
 		} catch (Exception e) {
 			e.printStackTrace();
 		}
 	}
 	
-	//FIXME: if at this router is not start, this will hang 10 secs due to host
-	//plugin cannot found vif for router.
-	@Override
-	public void CheckAndUpdateDhcpFlow(Network nw, VirtualMachine vm) {
-		if (!_isEnabled) {
-			return;
-		}
-		
-		DomainRouterVO router = _routerDao.findByNetworkConfiguration(nw.getId());
-		if (router == null) {
-			return;
-		}
-		
-		long accountId = nw.getAccountId();
-		if (!_vlanMappingDirtyDao.isDirty(accountId)) {
-			return;
-		}
-		
-		try {
-			String vlans = getVlanMapping(accountId);
-			String tag = Long.toString(_vlanMappingDao.findByAccountIdAndHostId(router.getAccountId(),
-							router.getHostId()).getVlan());
-			VmFlowLogVO log = _flowLogDao.findOrNewByVmId(vm.getId(), vm.getName());
-			_agentMgr.send(router.getHostId(), new OvsSetTagAndFlowCommand(
-					router.getName(), tag, vlans, Long.toString(log.getLogsequence()), vm.getId()));
-			s_logger.debug("ask router " + router.getName() + " on host "
-					+ router.getHostId() + " update vlan map to " + vlans);
-		} catch (Exception e) {
-			e.printStackTrace();
-		}
-	}
-
-	//TODO: handle router
 	@DB
 	@Override
 	public void scheduleFlowUpdateToHosts(Set<Long> affectedVms, boolean updateSeqno, Long delayMs) {
@@ -458,12 +425,15 @@ public class OvsNetworkManagerImpl implements OvsNetworkManager {
 			txn.start();
 			VmFlowLogVO log = null;
 			OvsWorkVO work = null;
-			UserVm vm = null;
+			VirtualMachine vm = null;
 			try {
 				vm = _userVMDao.acquireInLockTable(vmId);
 				if (vm == null) {
-					s_logger.warn("Ovs failed to acquire lock on vm id " + vmId);
-					continue;
+					vm = _routerDao.acquireInLockTable(vmId);
+					if (vm == null) {
+						s_logger.warn("Ovs failed to acquire lock on vm id " + vmId);
+						continue;
+					}
 				}
 				log = _flowLogDao.findOrNewByVmId(vmId, vm.getName());
 		
@@ -492,7 +462,7 @@ public class OvsNetworkManagerImpl implements OvsNetworkManager {
 		}
 	}
 	
-	protected Set<Long> getAffectedVms(VMInstanceVO instance) {
+	protected Set<Long> getAffectedVms(VMInstanceVO instance, boolean tellRouter) {
 		long accountId = instance.getAccountId();
 		if (!_vlanMappingDirtyDao.isDirty(accountId)) {
 			s_logger.debug("OVSAFFECTED: no VM affected by " + instance.getName());
@@ -505,7 +475,7 @@ public class OvsNetworkManagerImpl implements OvsNetworkManager {
 			affectedVms.add(new Long(vm.getId()));
 		}
 		
-		if (instance.getType() != VirtualMachine.Type.DomainRouter) {
+		if (tellRouter && instance.getType() != VirtualMachine.Type.DomainRouter) {
 			DomainRouterVO router = _routerDao.findBy(accountId, instance.getDataCenterId());
 			if (router != null) {
 				affectedVms.add(new Long(router.getId()));
@@ -514,8 +484,8 @@ public class OvsNetworkManagerImpl implements OvsNetworkManager {
 		return affectedVms;
 	}
 	
-	protected void handleVmStateChange(VMInstanceVO instance) {
-		Set<Long> affectedVms = getAffectedVms(instance);
+	protected void handleVmStateChange(VMInstanceVO instance, boolean tellRouter) {
+		Set<Long> affectedVms = getAffectedVms(instance, tellRouter);
 		scheduleFlowUpdateToHosts(affectedVms, true, null);
 		_vlanMappingDirtyDao.clean(instance.getAccountId());
 		s_logger.debug("OVSDIRTY:Clean dirty for account " + instance.getAccountId());
@@ -569,12 +539,12 @@ public class OvsNetworkManagerImpl implements OvsNetworkManager {
 		case Unknown:
 			return;
 		case Running:
-			handleVmStateChange(instance);
+			handleVmStateChange(instance, false);
 			break;
 		case Stopping:
 		case Stopped:
 			checkAndRemove(instance);
-			handleVmStateChange(instance);
+			handleVmStateChange(instance, true);
 			break;
 		}