Merge "svd: Make privileged SVD operations idempotent"

neutron/agent/linux/svd.py

@@ -18,13 +18,10 @@ import errno
 from pyroute2.netlink import exceptions as netlink_exc
 
 from neutron._i18n import _
+from neutron.privileged.agent.linux import ip_lib as priv_ip_lib
 from neutron.privileged.agent.linux import svd as privileged_svd
 
 
-class SvdDeviceAlreadyExists(Exception):
-    pass
-
-
 class SvdPortInUse(Exception):
     pass
 
@@ -33,14 +30,6 @@ class SvdDevsNotFound(Exception):
     pass
 
 
-class SvdSviNotFound(Exception):
-    pass
-
-
-class SvdNotFound(Exception):
-    pass
-
-
 class SvdNetlinkError(Exception):
     pass
 
@@ -53,6 +42,9 @@ class Svd:
 
     When a VNI is mapped to a VLAN, the caller-provided VLAN interface
     name is created.
+
+    All operations are idempotent: create/add succeed when resources
+    already exist; delete/remove succeed when resources are already gone.
     """
 
     def __init__(self, br_evpn, vxlan_evpn):
@@ -65,10 +57,6 @@ class Svd:
                 self.br_evpn, self.vxlan_evpn,
                 local_ip, mac, dstport, br_mtu)
         except netlink_exc.NetlinkError as e:
-            if e.code == errno.EEXIST:
-                raise SvdDeviceAlreadyExists(
-                    _("SVD %(br)s/%(vx)s device(s) already exist(s)") %
-                    {'br': self.br_evpn, 'vx': self.vxlan_evpn})
             if e.code == errno.EADDRINUSE:
                 raise SvdPortInUse(
                     _("UDP port %(port)d already in use") %
@@ -80,10 +68,6 @@ class Svd:
     def delete(self):
         try:
             privileged_svd.delete_svd(self.br_evpn, self.vxlan_evpn)
-        except IndexError:
-            raise SvdNotFound(
-                _("SVD %(br)s/%(vx)s not found") %
-                {'br': self.br_evpn, 'vx': self.vxlan_evpn})
         except netlink_exc.NetlinkError as e:
             raise SvdNetlinkError(
                 _("Failed to delete SVD %(br)s/%(vx)s: %(err)s") %
@@ -94,7 +78,7 @@ class Svd:
             privileged_svd.add_vni(
                 self.br_evpn, self.vxlan_evpn,
                 svi_name, lo_name, vni, vid, vrf_name, mac, br_mtu)
-        except IndexError:
+        except priv_ip_lib.NetworkInterfaceNotFound:
             raise SvdDevsNotFound(
                 _("SVD %(br)s/%(vx)s or VRF %(vrf)s not found") %
                 {'br': self.br_evpn, 'vx': self.vxlan_evpn,
@@ -111,11 +95,10 @@ class Svd:
             privileged_svd.del_vni(
                 self.br_evpn, self.vxlan_evpn,
                 svi_name, lo_name, vni, vid)
-        except IndexError:
-            raise SvdSviNotFound(
-                _("SVI for VNI %(vni)d not found on SVD %(br)s/%(vx)s") %
-                {'vni': vni, 'br': self.br_evpn,
-                 'vx': self.vxlan_evpn})
+        except priv_ip_lib.NetworkInterfaceNotFound:
+            raise SvdDevsNotFound(
+                _("SVD %(br)s/%(vx)s not found") %
+                {'br': self.br_evpn, 'vx': self.vxlan_evpn})
         except netlink_exc.NetlinkError as e:
             raise SvdNetlinkError(
                 _("Failed to delete VNI %(vni)d from SVD %(br)s/%(vx)s:"

neutron/agent/ovn/extensions/evpn/__init__.py

@@ -84,9 +84,6 @@ class EVPNAgentExtension(ovn_ext_mgr.OVNAgentExtension):
                                 dstport=dstport, br_mtu=self.cfg.br_mtu)
                 LOG.debug("EVPN SVD using child vxlan port %d", dstport)
                 return
-            except linux_svd.SvdDeviceAlreadyExists:
-                LOG.warning("SVD already exists, reusing")
-                return
             except linux_svd.SvdPortInUse:
                 LOG.debug("UDP port %d in use, trying next", dstport)
         raise RuntimeError(_("Failed to create SVD after %d attempts: "

neutron/agent/ovn/extensions/evpn/svd.py

@@ -30,7 +30,6 @@ class EvpnSvd(svd.Svd):
     def __init__(self, br_evpn, vxlan_evpn, index=0):
         super().__init__(br_evpn, vxlan_evpn)
         self._index = index
-        self._svi_names = {}
 
     def add_vni(self, vni, vid, vrf_name, mac, br_mtu):
         svi_name = evpn_const.EVPN_VLAN_IFNAME_PATTERN % {
@@ -38,10 +37,10 @@ class EvpnSvd(svd.Svd):
         lo_name = evpn_const.EVPN_AD_IFNAME % {
             'vni': vni}
         super().add_vni(svi_name, lo_name, vni, vid, vrf_name, mac, br_mtu)
-        self._svi_names[vni] = svi_name
 
     def del_vni(self, vni, vid):
-        svi_name = self._svi_names.pop(vni)
+        svi_name = evpn_const.EVPN_VLAN_IFNAME_PATTERN % {
+            'index': self._index, 'vid': vid}
         lo_name = evpn_const.EVPN_AD_IFNAME % {
             'vni': vni}
         super().del_vni(svi_name, lo_name, vni, vid)

neutron/privileged/agent/linux/svd.py

@@ -15,10 +15,12 @@
 
 """Low-level functions to create Single VxLAN Device and VNI:VLAN mapping"""
 
+import errno
 
 from oslo_log import log
 from pyroute2 import config as pyroute2_config
 from pyroute2 import netlink
+from pyroute2.netlink import exceptions as netlink_exc
 from pyroute2.netlink.rtnl import ifinfmsg
 from pyroute2.netlink.rtnl.ifinfmsg.plugins import vxlan
 
@@ -118,25 +120,39 @@ def _make_bridge_vni_msg(vxlan_idx, vni):
 
 
 def _bridge_add_vni(ipr, vxlan_idx, vni):
-    """Add a VNI filter entry on a vxlan device.
+    """Add a VNI filter entry or skip if already present.
 
     Equivalent to:
         bridge vni add dev <vxlan> vni <vni>
     """
     msg = _make_bridge_vni_msg(vxlan_idx, vni)
-    ipr.nlm_request(msg, msg_type=RTM_NEWTUNNEL,
-                    msg_flags=BRIDGE_ADD_VNI_MSG_FLAGS)
+    try:
+        ipr.nlm_request(msg, msg_type=RTM_NEWTUNNEL,
+                        msg_flags=BRIDGE_ADD_VNI_MSG_FLAGS)
+    except netlink_exc.NetlinkError as e:
+        if e.code == errno.EEXIST:
+            LOG.debug("Bridge VNI %d on ifindex %d already present, "
+                      "skipping", vni, vxlan_idx)
+            return
+        raise
 
 
 def _bridge_del_vni(ipr, vxlan_idx, vni):
-    """Delete a VNI filter entry on a vxlan device.
+    """Delete a VNI filter entry or skip if already absent.
 
     Equivalent to:
         bridge vni del dev <vxlan> vni <vni>
     """
     msg = _make_bridge_vni_msg(vxlan_idx, vni)
-    ipr.nlm_request(msg, msg_type=RTM_DELTUNNEL,
-                    msg_flags=BRIDGE_DEL_VNI_MSG_FLAGS)
+    try:
+        ipr.nlm_request(msg, msg_type=RTM_DELTUNNEL,
+                        msg_flags=BRIDGE_DEL_VNI_MSG_FLAGS)
+    except netlink_exc.NetlinkError as e:
+        if e.code == errno.ENOENT:
+            LOG.debug("Bridge VNI %d on ifindex %d already absent, "
+                      "skipping", vni, vxlan_idx)
+            return
+        raise
 
 
 def _set_addrgenmode_none(ipr, idx):
@@ -167,66 +183,100 @@ def _set_addrgenmode_none(ipr, idx):
 # End Workarounds for features missing in pyroute2 0.8.x.
 
 
-def _cleanup_partial_svd(ipr, br_evpn, vxlan_evpn):
-    """Remove partially created SVD devices after a failed create."""
-    for ifname in (vxlan_evpn, br_evpn):
-        idx = ipr.link_lookup(ifname=ifname)
-        if idx:
-            ipr.link(nl_const.IP_LINK_DEL, index=idx[0])
+def _add_link(ipr, ifname, **kwargs):
+    """Add link or skip if exists.  Returns the ifindex of the device."""
+    try:
+        ipr.link(nl_const.IP_LINK_ADD, ifname=ifname, **kwargs)
+    except netlink_exc.NetlinkError as e:
+        if e.code == errno.EEXIST:
+            LOG.debug("Link %s already exists, skipping add", ifname)
+            return ipr.link_lookup(ifname=ifname)[0]
+        raise
+    return ipr.link_lookup(ifname=ifname)[0]
+
+
+def _del_link(ipr, ifname):
+    """Delete link or skip if absent."""
+    idx = ipr.link_lookup(ifname=ifname)
+    if not idx:
+        LOG.debug("Link %s already absent, skipping delete", ifname)
+        return
+    ipr.link(nl_const.IP_LINK_DEL, index=idx[0])
+
+
+def _add_vlan_filter(ipr, index, label, **kwargs):
+    """Add vlan_filter or skip if already present."""
+    try:
+        ipr.vlan_filter(nl_const.IP_LINK_ADD, index=index, **kwargs)
+    except netlink_exc.NetlinkError as e:
+        if e.code == errno.EEXIST:
+            LOG.debug("VLAN filter on %s already present, skipping", label)
+            return
+        raise
+
+
+def _del_vlan_filter(ipr, index, label, **kwargs):
+    """Delete vlan_filter or skip if absent."""
+    try:
+        ipr.vlan_filter(nl_const.IP_LINK_DEL, index=index, **kwargs)
+    except netlink_exc.NetlinkError as e:
+        if e.code == errno.ENOENT:
+            LOG.debug("VLAN filter on %s already absent, skipping", label)
+            return
+        raise
+
+
+def _link_idx(ipr, ifname):
+    """Return ifindex for ifname or raise NetworkInterfaceNotFound."""
+    idx = ipr.link_lookup(ifname=ifname)
+    if not idx:
+        raise priv_ip_lib.NetworkInterfaceNotFound(
+            device=ifname, namespace=None)
+    return idx[0]
 
 
 @privileged.default.entrypoint
 def create_svd(br_evpn, vxlan_evpn, local_ip, mac, dstport, br_mtu):
-    """Create a shared Single VxLAN Device (SVD)
+    """Create a shared Single VxLAN Device (SVD).
 
-    A shared SVD consist of a vlan-aware Linux bridge and a vlan-aware VxLAN
+    A shared SVD consists of a vlan-aware Linux bridge and a vlan-aware
+    VxLAN.  Idempotent: if devices already exist, their configuration
+    is re-applied to match the desired state.
     """
     with priv_ip_lib.get_iproute(None) as ipr:
 
-        try:
-            # Equivalent to:
-            # ip link add <vxlan_evpn> type vxlan \
-            #   dstport <dstport> local <local_ip> \
-            #   nolearning external vnifilter
-            ipr.link(nl_const.IP_LINK_ADD, ifname=vxlan_evpn, kind='vxlan',
-                     vxlan_port=dstport,
-                     vxlan_local=local_ip,
-                     vxlan_learning=0,
-                     vxlan_collect_metadata=1,
-                     vxlan_vnifilter=1)
-            vxlan_idx = ipr.link_lookup(ifname=vxlan_evpn)[0]
+        # ip link add <vxlan_evpn> vxlan ...
+        vxlan_idx = _add_link(
+            ipr, vxlan_evpn, kind='vxlan',
+            vxlan_port=dstport,
+            vxlan_local=local_ip,
+            vxlan_learning=0,
+            vxlan_collect_metadata=1,
+            vxlan_vnifilter=1)
 
-            # Equivalent to:
-            # ip link add <br_evpn> type bridge vlan_filtering 1 \
-            #   vlan_default_pvid 0
-            # ip link set <br_evpn> address <mac>
-            # ip link set <br_evpn> up
-            ipr.link(nl_const.IP_LINK_ADD, ifname=br_evpn, kind='bridge',
-                     br_vlan_filtering=1, br_vlan_default_pvid=0)
-            br_idx = ipr.link_lookup(ifname=br_evpn)[0]
-            ipr.link(nl_const.IP_LINK_SET, index=br_idx, address=mac,
-                     state='up')
+        # ip link add <br_evpn> type bridge vlan_filtering 1 ...
+        br_idx = _add_link(
+            ipr, br_evpn, kind='bridge',
+            br_vlan_filtering=1, br_vlan_default_pvid=0)
 
-            # Equivalent to:
-            # ip link set <vxlan_evpn> address <mac> master <br_evpn>
-            # ip link set <vxlan_evpn> up
-            # bridge link set dev <vxlan_evpn> vlan_tunnel on neigh_suppress
-            #   on learning off
-            ipr.link(nl_const.IP_LINK_SET, index=vxlan_idx, address=mac,
-                     master=br_idx, state='up')
-            ipr.brport(nl_const.IP_LINK_SET, index=vxlan_idx,
-                       vlan_tunnel=1, neigh_suppress=1, learning=0)
+        # ip link set <br_evpn> address <mac> up
+        ipr.link(nl_const.IP_LINK_SET, index=br_idx, address=mac,
+                 state='up')
 
-            # Equivalent to:
-            # ip link set <br_evpn> mtu 1500 addrgenmode none
-            # ip link set <vxlan_evpn> addrgenmode none
-            ipr.link(nl_const.IP_LINK_SET, index=br_idx,
-                     mtu=br_mtu)
-            _set_addrgenmode_none(ipr, br_idx)
-            _set_addrgenmode_none(ipr, vxlan_idx)
-        except Exception:
-            _cleanup_partial_svd(ipr, br_evpn, vxlan_evpn)
-            raise
+        # ip link set <vxlan_evpn> address <mac> master <br_evpn> up
+        # bridge link set dev <vxlan_evpn> vlan_tunnel on neigh_suppress
+        #   on learning off
+        ipr.link(nl_const.IP_LINK_SET, index=vxlan_idx, address=mac,
+                 master=br_idx, state='up')
+        ipr.brport(nl_const.IP_LINK_SET, index=vxlan_idx,
+                   vlan_tunnel=1, neigh_suppress=1, learning=0)
+
+        # ip link set <br_evpn> mtu <br_mtu> addrgenmode none
+        # ip link set <vxlan_evpn> addrgenmode none
+        ipr.link(nl_const.IP_LINK_SET, index=br_idx,
+                 mtu=br_mtu)
+        _set_addrgenmode_none(ipr, br_idx)
+        _set_addrgenmode_none(ipr, vxlan_idx)
 
     LOG.debug("Created SVD: bridge %s, vxlan %s (local_ip %s, dstport %d)",
               br_evpn, vxlan_evpn, local_ip, dstport)
@@ -234,11 +284,10 @@ def create_svd(br_evpn, vxlan_evpn, local_ip, mac, dstport, br_mtu):
 
 @privileged.default.entrypoint
 def delete_svd(br_evpn, vxlan_evpn):
+    """Delete a shared SVD, already-absent devices are skipped."""
     with priv_ip_lib.get_iproute(None) as ipr:
-        vxlan_idx = ipr.link_lookup(ifname=vxlan_evpn)[0]
-        br_idx = ipr.link_lookup(ifname=br_evpn)[0]
-        ipr.link(nl_const.IP_LINK_DEL, index=vxlan_idx)
-        ipr.link(nl_const.IP_LINK_DEL, index=br_idx)
+        _del_link(ipr, vxlan_evpn)
+        _del_link(ipr, br_evpn)
     LOG.debug("Deleted SVD: bridge %s, vxlan %s",
               br_evpn, vxlan_evpn)
 
@@ -246,47 +295,39 @@ def delete_svd(br_evpn, vxlan_evpn):
 @privileged.default.entrypoint
 def add_vni(br_evpn, vxlan_evpn, svi_name, lo_name, vni, vid, vrf_name, mac,
             br_mtu):
+    """Map a VNI to the SVD, existing sub-resources are skipped."""
     with priv_ip_lib.get_iproute(None) as ipr:
-        br_idx = ipr.link_lookup(ifname=br_evpn)[0]
-        vxlan_idx = ipr.link_lookup(ifname=vxlan_evpn)[0]
-        vrf_idx = ipr.link_lookup(ifname=vrf_name)[0]
+        br_idx = _link_idx(ipr, br_evpn)
+        vxlan_idx = _link_idx(ipr, vxlan_evpn)
+        vrf_idx = _link_idx(ipr, vrf_name)
 
-        # Equivalent to:
         # bridge vlan add dev <br_evpn> vid <vid> self
-        # bridge vlan add dev <vxlan_evpn> vid <vid>
-        # bridge vlan add dev <vxlan_evpn> vid <vid> \
-        #   tunnel_info id <vni>
-        ipr.vlan_filter(nl_const.IP_LINK_ADD, index=br_idx,
-                        vlan_info={'vid': vid},
-                        vlan_flags='self')
-        ipr.vlan_filter(nl_const.IP_LINK_ADD, index=vxlan_idx,
-                        vlan_info={'vid': vid},
-                        vlan_tunnel_info={'vid': vid, 'id': vni})
+        _add_vlan_filter(
+            ipr, br_idx, br_evpn,
+            vlan_info={'vid': vid}, vlan_flags='self')
+        # bridge vlan add dev <vxlan_evpn> vid <vid> tunnel_info id <vni>
+        _add_vlan_filter(
+            ipr, vxlan_idx, vxlan_evpn,
+            vlan_info={'vid': vid},
+            vlan_tunnel_info={'vid': vid, 'id': vni})
 
-        # Equivalent to:
         # bridge vni add dev <vxlan_evpn> vni <vni>
         _bridge_add_vni(ipr, vxlan_idx, vni)
 
-        # Equivalent to:
         # ip link add <svi_name> link <br_evpn> type vlan id <vid>
-        # ip link set <svi_name> master <vrf_name>
-        # ip link set <svi_name> addr <mac> addrgenmode none
-        # ip link set <svi_name> up
-        ipr.link(nl_const.IP_LINK_ADD, ifname=svi_name, kind='vlan',
-                 link=br_idx, vlan_id=vid)
-        svi_idx = ipr.link_lookup(ifname=svi_name)[0]
+        svi_idx = _add_link(
+            ipr, svi_name, kind='vlan', link=br_idx, vlan_id=vid)
+        # ip link set <svi_name> master <vrf_name> addr <mac> up
         ipr.link(nl_const.IP_LINK_SET, index=svi_idx,
                  master=vrf_idx, address=mac,
                  mtu=br_mtu, state='up')
         _set_addrgenmode_none(ipr, svi_idx)
 
-        # Equivalent to:
         # ip link add <lo_name> type dummy
-        # ip link set <lo_name> master <br_name>
-        # ip link set <lo_name> up
-        ipr.link(nl_const.IP_LINK_ADD, ifname=lo_name, kind='dummy')
-        lo_idx = ipr.link_lookup(ifname=lo_name)[0]
-        ipr.link(nl_const.IP_LINK_SET, index=lo_idx, master=br_idx, state='up')
+        lo_idx = _add_link(ipr, lo_name, kind='dummy')
+        # ip link set <lo_name> master <br_evpn> up
+        ipr.link(nl_const.IP_LINK_SET, index=lo_idx, master=br_idx,
+                 state='up')
 
     LOG.debug("SVD %s/%s: added VLAN %d -> VNI %d, SVI %s, lo %s",
               br_evpn, vxlan_evpn, vid, vni, svi_name, lo_name)
@@ -294,33 +335,28 @@ def add_vni(br_evpn, vxlan_evpn, svi_name, lo_name, vni, vid, vrf_name, mac,
 
 @privileged.default.entrypoint
 def del_vni(br_evpn, vxlan_evpn, svi_name, lo_name, vni, vid):
+    """Remove a VNI mapping from the SVD, missing resources are skipped."""
     with priv_ip_lib.get_iproute(None) as ipr:
-        br_idx = ipr.link_lookup(ifname=br_evpn)[0]
-        vxlan_idx = ipr.link_lookup(ifname=vxlan_evpn)[0]
+        br_idx = _link_idx(ipr, br_evpn)
+        vxlan_idx = _link_idx(ipr, vxlan_evpn)
 
-        # Equivalent to:
         # ip link del <lo_name>
-        lo_idx = ipr.link_lookup(ifname=lo_name)[0]
-        ipr.link(nl_const.IP_LINK_DEL, index=lo_idx)
-
-        # Equivalent to:
+        _del_link(ipr, lo_name)
         # ip link del <svi_name>
-        svi_idx = ipr.link_lookup(ifname=svi_name)[0]
-        ipr.link(nl_const.IP_LINK_DEL, index=svi_idx)
+        _del_link(ipr, svi_name)
 
-        # Equivalent to:
         # bridge vni del dev <vxlan_evpn> vni <vni>
-        # bridge vlan del dev <vxlan_evpn> vid <vid>
-        # bridge vlan del dev <vxlan_evpn> vid <vid> \
-        #   tunnel_info id <vni>
-        # bridge vlan del dev <br_evpn> vid <vid> self
         _bridge_del_vni(ipr, vxlan_idx, vni)
-        ipr.vlan_filter(nl_const.IP_LINK_DEL, index=vxlan_idx,
-                        vlan_info={'vid': vid},
-                        vlan_tunnel_info={'vid': vid, 'id': vni})
-        ipr.vlan_filter(nl_const.IP_LINK_DEL, index=br_idx,
-                        vlan_info={'vid': vid},
-                        vlan_flags='self')
+
+        # bridge vlan del dev <vxlan_evpn> vid <vid> tunnel_info id <vni>
+        _del_vlan_filter(
+            ipr, vxlan_idx, vxlan_evpn,
+            vlan_info={'vid': vid},
+            vlan_tunnel_info={'vid': vid, 'id': vni})
+        # bridge vlan del dev <br_evpn> vid <vid> self
+        _del_vlan_filter(
+            ipr, br_idx, br_evpn,
+            vlan_info={'vid': vid}, vlan_flags='self')
 
     LOG.debug("SVD %s/%s: removed VLAN %d -> VNI %d",
               br_evpn, vxlan_evpn, vid, vni)

neutron/tests/functional/agent/linux/test_svd.py

@@ -104,12 +104,63 @@ class TestSvdFunctional(base.BaseNetlinkTestCase):
         self.assertIn('external', vx_output)
         self.assertIn('addrgenmode none', vx_output)
 
-    def test_create_svd_device_exists(self):
+    def test_create_svd_idempotent(self):
+        svd = self._create_svd()
+        self.assertTrue(ip_lib.device_exists(self._br))
+        self.assertTrue(ip_lib.device_exists(self._vx))
+
+        svd.create(local_ip=self.LOCAL_IP, mac=self.MAC,
+                   dstport=self.DSTPORT,
+                   br_mtu=self.BR_MTU)
+        self.assertTrue(ip_lib.device_exists(self._br))
+        self.assertTrue(ip_lib.device_exists(self._vx))
+
+    def test_create_svd_reapplies_mtu(self):
+        """Re-create SVD with a different MTU.
+
+        Verifies that unconditional SET operations update the bridge
+        MTU when create_svd is called again with a new value.
+        """
         self._create_svd()
+        br_output = agent_utils.execute(
+            ['ip', '-d', 'link', 'show', self._br],
+            run_as_root=True, privsep_exec=True)
+        self.assertIn('mtu 1500', br_output)
+
         brvxlan = linux_svd.Svd(br_evpn=self._br, vxlan_evpn=self._vx)
-        self.assertRaises(linux_svd.SvdDeviceAlreadyExists, brvxlan.create,
-                          local_ip=self.LOCAL_IP, mac=self.MAC,
-                          dstport=self.DSTPORT, br_mtu=self.BR_MTU)
+        brvxlan.create(local_ip=self.LOCAL_IP, mac=self.MAC,
+                       dstport=self.DSTPORT,
+                       br_mtu=9000)
+
+        br_output = agent_utils.execute(
+            ['ip', '-d', 'link', 'show', self._br],
+            run_as_root=True, privsep_exec=True)
+        self.assertIn('mtu 9000', br_output)
+
+    def test_create_svd_idempotent_preserves_brport_flags(self):
+        """Re-create SVD and verify brport flags survive.
+
+        Checks that neigh_suppress and vlan_tunnel remain set on
+        the vxlan bridge port after an idempotent re-create.
+        """
+        self._create_svd()
+
+        brport_output = self._bridge_cmd(
+            '-d', 'link', 'show', 'dev', self._vx)
+        self.assertIn('neigh_suppress on', brport_output)
+        self.assertIn('vlan_tunnel on', brport_output)
+        self.assertIn('learning off', brport_output)
+
+        brvxlan = linux_svd.Svd(br_evpn=self._br, vxlan_evpn=self._vx)
+        brvxlan.create(local_ip=self.LOCAL_IP, mac=self.MAC,
+                       dstport=self.DSTPORT,
+                       br_mtu=self.BR_MTU)
+
+        brport_output = self._bridge_cmd(
+            '-d', 'link', 'show', 'dev', self._vx)
+        self.assertIn('neigh_suppress on', brport_output)
+        self.assertIn('vlan_tunnel on', brport_output)
+        self.assertIn('learning off', brport_output)
 
     def test_delete_svd(self):
         svd = self._create_svd()
@@ -119,9 +170,37 @@ class TestSvdFunctional(base.BaseNetlinkTestCase):
         self.assertFalse(ip_lib.device_exists(self._br))
         self.assertFalse(ip_lib.device_exists(self._vx))
 
-    def test_delete_svd_not_found(self):
-        brvxlan = linux_svd.Svd(br_evpn=self._br, vxlan_evpn=self._vx)
-        self.assertRaises(linux_svd.SvdNotFound, brvxlan.delete)
+    def test_delete_svd_idempotent(self):
+        svd = self._create_svd()
+
+        svd.delete()
+        self.assertFalse(ip_lib.device_exists(self._br))
+        self.assertFalse(ip_lib.device_exists(self._vx))
+
+        svd.delete()
+        self.assertFalse(ip_lib.device_exists(self._br))
+        self.assertFalse(ip_lib.device_exists(self._vx))
+
+    def test_delete_svd_with_mapped_vnis(self):
+        """Delete SVD without removing VNIs first.
+
+        Verifies that deleting the bridge removes the SVI (a VLAN
+        sub-interface)
+        """
+        svd = self._create_svd()
+        vni = self._vni()
+        vid = self._vid()
+        svi_name = self._svi_name(vid)
+        lo_name = evpn_const.EVPN_AD_IFNAME % {'vni': vni}
+        svd.add_vni(svi_name, lo_name, vni, vid, self._vrf, self.SVI_MAC,
+                    self.BR_MTU)
+        self.addCleanup(self._safe_delete, lo_name)
+
+        svd.delete()
+
+        self.assertFalse(ip_lib.device_exists(self._br))
+        self.assertFalse(ip_lib.device_exists(self._vx))
+        self.assertFalse(ip_lib.device_exists(svi_name))
 
     def test_add_vni(self):
         svd = self._create_svd()
@@ -158,7 +237,7 @@ class TestSvdFunctional(base.BaseNetlinkTestCase):
         vni_output = self._bridge_cmd('vni', 'show', 'dev', self._vx)
         self.assertNotIn(str(vni), vni_output)
 
-    def test_add_vni_netlink_error(self):
+    def test_add_vni_idempotent(self):
         svd = self._create_svd()
         vni = self._vni()
         vid = self._vid()
@@ -166,11 +245,19 @@ class TestSvdFunctional(base.BaseNetlinkTestCase):
         lo_name = evpn_const.EVPN_AD_IFNAME % {'vni': vni}
         svd.add_vni(svi_name, lo_name, vni, vid, self._vrf, self.SVI_MAC,
                     self.BR_MTU)
+        self.assertTrue(ip_lib.device_exists(svi_name))
+        self.assertTrue(ip_lib.device_exists(lo_name))
+        vni_output = self._bridge_cmd('vni', 'show', 'dev', self._vx)
+        self.assertIn(str(vni), vni_output)
         self.addCleanup(svd.del_vni, svi_name, lo_name, vni, vid)
-        # Add the same VNI a second time to trigger NetlinkError
-        self.assertRaises(linux_svd.SvdNetlinkError, svd.add_vni,
-                          svi_name, lo_name, vni, vid, self._vrf, self.SVI_MAC,
-                          self.BR_MTU)
+
+        svd.add_vni(svi_name, lo_name, vni, vid, self._vrf, self.SVI_MAC,
+                    self.BR_MTU)
+
+        self.assertTrue(ip_lib.device_exists(svi_name))
+        self.assertTrue(ip_lib.device_exists(lo_name))
+        vni_output = self._bridge_cmd('vni', 'show', 'dev', self._vx)
+        self.assertIn(str(vni), vni_output)
 
     def test_del_vni(self):
         svd = self._create_svd()
@@ -189,13 +276,37 @@ class TestSvdFunctional(base.BaseNetlinkTestCase):
         vni_output = self._bridge_cmd('vni', 'show', 'dev', self._vx)
         self.assertNotIn(str(vni), vni_output)
 
-    def test_del_vni_svi_not_found(self):
+    def test_del_vni_idempotent(self):
         svd = self._create_svd()
+        vni = self._vni()
+        vid = self._vid()
+        svi_name = self._svi_name(vid)
+        lo_name = evpn_const.EVPN_AD_IFNAME % {'vni': vni}
+        svd.add_vni(svi_name, lo_name, vni, vid, self._vrf, self.SVI_MAC,
+                    self.BR_MTU)
 
-        self.assertRaises(linux_svd.SvdSviNotFound, svd.del_vni,
-                          self._svi_name(self._vid()),
-                          evpn_const.EVPN_AD_IFNAME % {'vni': self._vni()},
-                          self._vni(), self._vid())
+        svd.del_vni(svi_name, lo_name, vni, vid)
+        self.assertFalse(ip_lib.device_exists(svi_name))
+        self.assertFalse(ip_lib.device_exists(lo_name))
+        vni_output = self._bridge_cmd('vni', 'show', 'dev', self._vx)
+        self.assertNotIn(str(vni), vni_output)
+
+        svd.del_vni(svi_name, lo_name, vni, vid)
+        self.assertFalse(ip_lib.device_exists(svi_name))
+        self.assertFalse(ip_lib.device_exists(lo_name))
+        vni_output = self._bridge_cmd('vni', 'show', 'dev', self._vx)
+        self.assertNotIn(str(vni), vni_output)
+
+    def test_del_vni_svd_not_found(self):
+        brvxlan = linux_svd.Svd(br_evpn=self._br, vxlan_evpn=self._vx)
+        vni = self._vni()
+        vid = self._vid()
+        svi_name = self._svi_name(vid)
+        lo_name = evpn_const.EVPN_AD_IFNAME % {'vni': vni}
+        self.assertRaises(linux_svd.SvdDevsNotFound, brvxlan.del_vni,
+                          svi_name, lo_name, vni, vid)
+        self.assertFalse(ip_lib.device_exists(self._br))
+        self.assertFalse(ip_lib.device_exists(self._vx))
 
     def test_add_multiple_vnis(self):
         svd = self._create_svd()
@@ -247,3 +358,90 @@ class TestSvdFunctional(base.BaseNetlinkTestCase):
         self.assertIn('state UP', link_output)
 
         svd.del_vni(svi_name, lo_name, vni, vid)
+
+    def test_create_svd_partial_recovery(self):
+        """Create only the bridge, then call create_svd.
+
+        Verifies that create_svd completes the vxlan setup when the
+        bridge already exists (crash-recovery scenario).
+        """
+        privileged.create_interface(self._br, None, 'bridge',
+                                    br_vlan_filtering=1,
+                                    br_vlan_default_pvid=0)
+        self.addCleanup(self._safe_delete, self._br)
+        self.addCleanup(self._safe_delete, self._vx)
+        self.assertTrue(ip_lib.device_exists(self._br))
+        self.assertFalse(ip_lib.device_exists(self._vx))
+
+        brvxlan = linux_svd.Svd(br_evpn=self._br, vxlan_evpn=self._vx)
+        brvxlan.create(local_ip=self.LOCAL_IP, mac=self.MAC,
+                       dstport=self.DSTPORT,
+                       br_mtu=self.BR_MTU)
+
+        self.assertTrue(ip_lib.device_exists(self._br))
+        self.assertTrue(ip_lib.device_exists(self._vx))
+
+        br_output = agent_utils.execute(
+            ['ip', '-d', 'link', 'show', self._br],
+            run_as_root=True, privsep_exec=True)
+        self.assertIn('mtu 1500', br_output)
+
+    def test_del_vni_partial_recovery(self):
+        """Delete loopback and SVI manually, then call del_vni.
+
+        Verifies that del_vni cleans up the remaining VLAN filters and
+        VNI filter when the link devices are already gone
+        (crash-recovery scenario).
+        """
+        svd = self._create_svd()
+        vni = self._vni()
+        vid = self._vid()
+        svi_name = self._svi_name(vid)
+        lo_name = evpn_const.EVPN_AD_IFNAME % {'vni': vni}
+        svd.add_vni(svi_name, lo_name, vni, vid, self._vrf, self.SVI_MAC,
+                    self.BR_MTU)
+
+        agent_utils.execute(
+            ['ip', 'link', 'del', lo_name],
+            run_as_root=True, privsep_exec=True)
+        agent_utils.execute(
+            ['ip', 'link', 'del', svi_name],
+            run_as_root=True, privsep_exec=True)
+        self.assertFalse(ip_lib.device_exists(lo_name))
+        self.assertFalse(ip_lib.device_exists(svi_name))
+
+        svd.del_vni(svi_name, lo_name, vni, vid)
+
+        vni_output = self._bridge_cmd('vni', 'show', 'dev', self._vx)
+        self.assertNotIn(str(vni), vni_output)
+        br_vlans = self._bridge_cmd('vlan', 'show', 'dev', self._br)
+        self.assertNotRegex(br_vlans, r'\b%s\b' % vid)
+
+    def test_add_vni_partial_recovery(self):
+        """Create the SVI manually, then call add_vni.
+
+        Verifies that add_vni completes the remaining resources (VLAN
+        filters, VNI filter, loopback) when the SVI already exists.
+        """
+        svd = self._create_svd()
+        vni = self._vni()
+        vid = self._vid()
+        svi_name = self._svi_name(vid)
+        lo_name = evpn_const.EVPN_AD_IFNAME % {'vni': vni}
+
+        agent_utils.execute(
+            ['ip', 'link', 'add', svi_name, 'link', self._br,
+             'type', 'vlan', 'id', str(vid)],
+            run_as_root=True, privsep_exec=True)
+        self.addCleanup(self._safe_delete, svi_name)
+        self.assertTrue(ip_lib.device_exists(svi_name))
+        self.assertFalse(ip_lib.device_exists(lo_name))
+
+        svd.add_vni(svi_name, lo_name, vni, vid, self._vrf, self.SVI_MAC,
+                    self.BR_MTU)
+        self.addCleanup(svd.del_vni, svi_name, lo_name, vni, vid)
+
+        self.assertTrue(ip_lib.device_exists(svi_name))
+        self.assertTrue(ip_lib.device_exists(lo_name))
+        vni_output = self._bridge_cmd('vni', 'show', 'dev', self._vx)
+        self.assertIn(str(vni), vni_output)

neutron/tests/functional/agent/ovn/extensions/test_evpn.py

@@ -214,6 +214,39 @@ class TestFsmSvdIntegration(base.BaseNetlinkTestCase):
         self.assertEqual(fsm.Evpn.WAITING_FOR_ROUTER, evpn.state)
         self.assertFalse(ip_lib.device_exists(svi_name))
 
+    def test_evpn_svd_del_vni_idempotent_without_prior_add(self):
+        """del_vni on EvpnSvd succeeds when VNI was never added."""
+        vni = 4000
+        vid = 40
+        svi_name = evpn_const.EVPN_VLAN_IFNAME_PATTERN % {
+            'index': 0, 'vid': vid}
+        lo_name = evpn_const.EVPN_AD_IFNAME % {'vni': vni}
+
+        self.svd.del_vni(vni, vid)
+
+        self.assertFalse(ip_lib.device_exists(svi_name))
+        self.assertFalse(ip_lib.device_exists(lo_name))
+
+    def test_evpn_svd_del_vni_twice_idempotent(self):
+        """del_vni called twice on EvpnSvd does not raise."""
+        vni = 5000
+        vid = 50
+        svi_name = evpn_const.EVPN_VLAN_IFNAME_PATTERN % {
+            'index': 0, 'vid': vid}
+        lo_name = evpn_const.EVPN_AD_IFNAME % {'vni': vni}
+
+        self.svd.add_vni(vni, vid, self._vrf, self.SVI_MAC,
+                         evpn_const.EVPN_BR_MTU)
+        self.assertTrue(ip_lib.device_exists(svi_name))
+
+        self.svd.del_vni(vni, vid)
+        self.assertFalse(ip_lib.device_exists(svi_name))
+        self.assertFalse(ip_lib.device_exists(lo_name))
+
+        self.svd.del_vni(vni, vid)
+        self.assertFalse(ip_lib.device_exists(svi_name))
+        self.assertFalse(ip_lib.device_exists(lo_name))
+
 
 class TestGetFreeUdpPort(functional_base.BaseLoggingTestCase):
 

neutron/tests/unit/agent/ovn/extensions/test_evpn.py

@@ -113,12 +113,10 @@ class TestCreateSvdWithFreePort(base.BaseTestCase):
         self.ext._create_svd_with_free_port()
         self.assertEqual(3, self.ext.svd.create.call_count)
 
-    def test_reuses_existing_svd(self):
+    def test_idempotent_create_existing_svd(self):
         mock.patch.object(
             evpn_ext.EVPNAgentExtension, '_get_free_udp_port',
             return_value=49152).start()
-        self.ext.svd.create.side_effect = linux_svd.SvdDeviceAlreadyExists(
-            "already exists")
         self.ext._create_svd_with_free_port()
         self.ext.svd.create.assert_called_once()