MitchDrage commented on code in PR #14240:
URL: https://github.com/apache/cloudstack/pull/14240#discussion_r4162113075


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java:
##########
@@ -458,6 +458,14 @@ public boolean isExistingBridge(String bridgeName) {
 
     @Override
     public void deleteBr(NicTO nic) {
+        if (Networks.BroadcastDomainType.getSchemeValue(nic.getBroadcastUri()) 
== Networks.BroadcastDomainType.Vxlan) {
+            // VXLAN bridges are named after the VNI alone, so no physical 
interface lookup is needed
+            String vxlanId = 
Networks.BroadcastDomainType.getValue(nic.getBroadcastUri());
+            if (vxlanId != null) {
+                deleteVnetBr(generateVxnetBrName(null, vxlanId), true);

Review Comment:
   Here's my proposed solution:
   
   ```
   +            protected String getVxlanPif(String vxlanId) {
   +                return Script.runSimpleBashScript("ip -d link show vxlan" + 
vxlanId + " | grep -o 'dev [^ ]*' | cut -d' ' -f2");
   +            }
       
                String scriptPath = null;
                if (cmdout != null && cmdout.contains("vxlan")) {
                    scriptPath = _modifyVxlanPath;
   +                // Read the pif from the VXLAN device.
   +                String vxlanPif = getVxlanPif(vNetId);
   +                if (vxlanPif != null) {
   +                    pName = vxlanPif;
   +                }
                } else {
                    scriptPath = _modifyVlanPath;
                }
   ```
   
   The interface is read from the VXLAN device with ip -d link show vxlan<vni> 
| grep -o 'dev [^ ]*' | cut -d' ' -f2.
   
   For a device created by the stock modifyvxlan.sh (from a test with a dummy 
interface):
   
   
   `vxlan id 5000 group 239.0.19.136 dev dummy0 srcport 0 0 dstport 8472 ttl 10 
...`
   the lookup returns dummy0, which is passed as -p, so the multicast route is 
removed.
   
   For a device created by modifyvxlan-evpn.sh (from a live host):
   
   
   `vxlan id 133284 local 10.254.1.101 srcport 0 0 dstport 4789 ttl auto ageing 
300 nolearning`
   there's no dev, so the lookup returns nothing and the existing value is 
passed unchanged. The EVPN script doesn't use -p on delete.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to