evpn: Remove child_vxlan_port config option

The vxlan port number was just used as a UDP port for the SVD.
The port number is not used anywhere in the datapath and doesn't
need to be statically configured from the dynamic range of ports.

This patch lets kernel pick any available port by binding to a socket.
Then it closes the socket and returns the used port for the vxlan
device.

There is a very small time window between the temporary socket is closed
and the port is set to the vxlan interface, so there is a loop that
tries the mechanism ten times, always letting the kernel to pick a new
unused port. Then it bails out and agent needs to be restarted.

Closes-Bug: #2158130
Assisted-By: Claude Opus 4.6

Change-Id: I2bebc47fe4c435539476e591b684ce552da8651d
Signed-off-by: Jakub Libosvar <jlibosva@redhat.com>

neutron/agent/linux/svd.py

@@ -29,6 +29,10 @@ class SvdDeviceAlreadyExists(Exception):
     pass
 
 
+class SvdPortInUse(Exception):
+    pass
+
+
 class SvdDevsNotFound(Exception):
     pass
 
@@ -73,6 +77,10 @@ class Svd:
                 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") %
+                    {'port': dstport})
             raise SvdNetlinkError(
                 _("Failed to add SVD %(br)s/%(vx)s: %(err)s") %
                 {'br': self.br_evpn, 'vx': self.vxlan_evpn, 'err': e})

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

@@ -14,12 +14,14 @@
 #    under the License.
 
 import dataclasses
+import socket
 
 from neutron_lib.utils import net as net_lib
 from oslo_config import cfg
 from oslo_log import log
 from pyroute2.netlink import rtnl
 
+from neutron._i18n import _
 from neutron.agent.linux import nl_constants as nl_const
 from neutron.agent.linux import nl_dispatcher
 from neutron.agent.linux import svd as linux_svd
@@ -39,7 +41,6 @@ CONF = cfg.CONF
 @dataclasses.dataclass(frozen=True)
 class EvpnConfig:
     local_ip: str
-    dstport: int
     vxlan_parent: str
     mac: str
     br_mtu: int
@@ -51,21 +52,45 @@ class EVPNAgentExtension(ovn_ext_mgr.OVNAgentExtension):
         self._evpn_fsm = fsm.EvpnFSM()
         self.nl_dispatcher = None
 
+    @staticmethod
+    def _get_free_udp_port():
+        """Return a UDP port that is not in use on the system."""
+        with socket.socket(socket.AF_INET, socket.SOCK_DGRAM) as s:
+            s.bind(('', 0))
+            port = s.getsockname()[1]
+            LOG.debug("Found free UDP port %d", port)
+            return port
+
     def _get_evpn_config(self):
         ext_ids = self.agent_api.ovs_idl.db_get(
             'Open_vSwitch', '.', 'external_ids').execute()
         local_ip = ext_ids['ovn-evpn-local-ip']
         vxlan_port = ext_ids['ovn-evpn-vxlan-ports']
         vxlan_parent = 'vxlan_sys_%s' % vxlan_port
-        dstport = CONF.ovn_evpn.child_vxlan_port
         mac = net_lib.get_random_mac(CONF.base_mac.split(':'))
-        self.cfg = EvpnConfig(local_ip=local_ip, dstport=dstport,
+        self.cfg = EvpnConfig(local_ip=local_ip,
                               vxlan_parent=vxlan_parent, mac=mac,
                               br_mtu=evpn_const.EVPN_BR_MTU)
-        LOG.debug("EVPN config: local_ip %s vxlan_parent %s "
-                  "child vxlan port %d SVD MAC %s",
-                  self.cfg.local_ip, self.cfg.vxlan_parent,
-                  self.cfg.dstport, self.cfg.mac)
+        LOG.debug("EVPN config: local_ip %s vxlan_parent %s SVD MAC %s",
+                  self.cfg.local_ip, self.cfg.vxlan_parent, self.cfg.mac)
+
+    def _create_svd_with_free_port(self, max_attempts=10):
+        for attempt in range(max_attempts):
+            dstport = self._get_free_udp_port()
+            try:
+                self.svd.create(local_ip=self.cfg.local_ip,
+                                mac=self.cfg.mac,
+                                vxlan_parent=self.cfg.vxlan_parent,
+                                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: "
+                             "no available UDP port") % max_attempts)
 
     def start(self):
         self._get_evpn_config()
@@ -74,13 +99,7 @@ class EVPNAgentExtension(ovn_ext_mgr.OVNAgentExtension):
         br_evpn = '%s%d' % (evpn_const.EVPN_LB_NAME_PREFIX, 0)
         vxlan_evpn = '%s%d' % (evpn_const.EVPN_VXLAN_IFNAME, 0)
         self.svd = svd.EvpnSvd(br_evpn=br_evpn, vxlan_evpn=vxlan_evpn)
-        try:
-            self.svd.create(local_ip=self.cfg.local_ip,
-                            mac=self.cfg.mac,
-                            vxlan_parent=self.cfg.vxlan_parent,
-                            dstport=self.cfg.dstport, br_mtu=self.cfg.br_mtu)
-        except linux_svd.SvdDeviceAlreadyExists:
-            LOG.warning("SVD already exists, reusing")
+        self._create_svd_with_free_port()
         driver = fsm_frr_driver.FsmFrrVtyshDriver(
             peer_interface=CONF.ovn_evpn.bgp_local_interface,
             bgp_router_id=self.cfg.local_ip)

neutron/conf/agent/ovn/evpn/config.py

@@ -26,10 +26,6 @@ EVPN_OPTS = [
         'bgp_local_interface',
         help=_('The local interface name (e.g. eth2) on which to establish '
                'BGP peer session')),
-    cfg.IntOpt(
-        'child_vxlan_port',
-        default=49152,
-        help=_('UDP port for the child VxLAN device used by EVPN')),
     cfg.StrOpt(
         'frr_vty_socket',
         default='/run/frr',

neutron/privileged/agent/linux/svd.py

@@ -167,6 +167,14 @@ 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])
+
+
 @privileged.default.entrypoint
 def create_svd(br_evpn, vxlan_evpn, local_ip, mac, vxlan_parent, dstport,
                br_mtu):
@@ -177,47 +185,51 @@ def create_svd(br_evpn, vxlan_evpn, local_ip, mac, vxlan_parent, dstport,
     with priv_ip_lib.get_iproute(None) as ipr:
         vxlan_parent_idx = ipr.link_lookup(ifname=vxlan_parent)[0]
 
-        # Equivalent to:
-        # ip link add <vxlan_evpn> vxlan \
-        #   dev <vxlan_parent> dstport <dstport> local <local_ip> \
-        #   no learning external vnifilter
-        ipr.link(nl_const.IP_LINK_ADD, ifname=vxlan_evpn, kind='vxlan',
-                 vxlan_link=vxlan_parent_idx,
-                 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]
+        try:
+            # Equivalent to:
+            # ip link add <vxlan_evpn> vxlan \
+            #   dev <vxlan_parent> dstport <dstport> local <local_ip> \
+            #   no learning external vnifilter
+            ipr.link(nl_const.IP_LINK_ADD, ifname=vxlan_evpn, kind='vxlan',
+                     vxlan_link=vxlan_parent_idx,
+                     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]
 
-        # 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')
+            # 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')
 
-        # 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)
+            # 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)
 
-        # 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)
+            # 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
 
     LOG.debug("Created SVD: bridge %s, vxlan %s (parent %s, "
               "local_ip %s, dstport %d)",

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

@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import socket
 from unittest import mock
 
 from oslo_utils import uuidutils
@@ -132,7 +133,6 @@ class TestFsmSvdIntegration(base.BaseNetlinkTestCase):
         ip_lib.IPDevice(self._parent).addr.add(self.LOCAL_IP + '/32')
         self.addCleanup(self._safe_delete, self._parent)
         self.cfg = evpn.EvpnConfig(local_ip=self.LOCAL_IP,
-                                   dstport=self.DSTPORT,
                                    vxlan_parent=self._parent,
                                    mac=self.SVD_MAC,
                                    br_mtu=evpn_const.EVPN_BR_MTU)
@@ -204,3 +204,16 @@ class TestFsmSvdIntegration(base.BaseNetlinkTestCase):
         evpn = self._evpn_fsm.instances[self._vrf]
         self.assertEqual(fsm.Evpn.WAITING_FOR_ROUTER, evpn.state)
         self.assertFalse(ip_lib.device_exists(svi_name))
+
+
+class TestGetFreeUdpPort(functional_base.BaseLoggingTestCase):
+
+    def test_returns_valid_port(self):
+        port = evpn.EVPNAgentExtension._get_free_udp_port()
+        self.assertGreater(port, 0)
+        self.assertLessEqual(port, 65535)
+
+    def test_returned_port_is_bindable(self):
+        port = evpn.EVPNAgentExtension._get_free_udp_port()
+        with socket.socket(socket.AF_INET, socket.SOCK_DGRAM) as s:
+            s.bind(('', port))

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

@@ -15,10 +15,10 @@
 
 from unittest import mock
 
-from oslo_config import cfg
 from pyroute2.iproute import ipmock
 from pyroute2.netlink.rtnl import ifinfmsg
 
+from neutron.agent.linux import svd as linux_svd
 from neutron.agent.ovn.extensions import evpn as evpn_ext
 from neutron.agent.ovn.extensions.evpn import exceptions as evpn_exc
 from neutron.agent.ovn.extensions.evpn import fsm
@@ -53,8 +53,6 @@ class TestEVPNAgentExtension(base.BaseTestCase):
     def setUp(self):
         super().setUp()
         evpn_conf.register_opts()
-        cfg.CONF.set_override('child_vxlan_port', self.DSTPORT,
-                              group='ovn_evpn')
         self.ext = evpn_ext.EVPNAgentExtension()
         self.ext.agent_api = mock.Mock()
         self.ext.agent_api.ovs_idl.db_get.return_value.execute.return_value = {
@@ -69,9 +67,71 @@ class TestEVPNAgentExtension(base.BaseTestCase):
                                   '.NetlinkDispatcher').start()
         mock.patch.object(evpn_ext.net_lib, 'get_random_mac',
                           return_value=self.MAC).start()
+        mock.patch.object(evpn_ext.EVPNAgentExtension,
+                          '_get_free_udp_port',
+                          return_value=self.DSTPORT).start()
         self.addCleanup(mock.patch.stopall)
 
 
+class TestCreateSvdWithFreePort(base.BaseTestCase):
+
+    LOCAL_IP = '10.10.10.10'
+    VXLAN_PORT = '4789'
+    MAC = 'fa:16:3e:aa:bb:cc'
+
+    def setUp(self):
+        super().setUp()
+        evpn_conf.register_opts()
+        self.ext = evpn_ext.EVPNAgentExtension()
+        self.ext.agent_api = mock.Mock()
+        self.ext.agent_api.ovs_idl.db_get.return_value.execute.return_value = {
+            'ovn-evpn-local-ip': self.LOCAL_IP,
+            'ovn-evpn-vxlan-ports': self.VXLAN_PORT,
+        }
+        mock.patch.object(evpn_ext.net_lib, 'get_random_mac',
+                          return_value=self.MAC).start()
+        self.ext._get_evpn_config()
+        self.ext.svd = mock.Mock()
+        self.addCleanup(mock.patch.stopall)
+
+    def test_creates_svd_on_first_try(self):
+        mock.patch.object(
+            evpn_ext.EVPNAgentExtension, '_get_free_udp_port',
+            return_value=49152).start()
+        self.ext._create_svd_with_free_port()
+        self.ext.svd.create.assert_called_once()
+
+    def test_retries_on_port_in_use(self):
+        mock.patch.object(
+            evpn_ext.EVPNAgentExtension, '_get_free_udp_port',
+            side_effect=[49152, 49153, 49154]).start()
+        self.ext.svd.create.side_effect = [
+            linux_svd.SvdPortInUse("port 49152 in use"),
+            linux_svd.SvdPortInUse("port 49153 in use"),
+            None,
+        ]
+        self.ext._create_svd_with_free_port()
+        self.assertEqual(3, self.ext.svd.create.call_count)
+
+    def test_reuses_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()
+
+    def test_raises_after_max_attempts(self):
+        mock.patch.object(
+            evpn_ext.EVPNAgentExtension, '_get_free_udp_port',
+            side_effect=list(range(49152, 49162))).start()
+        self.ext.svd.create.side_effect = linux_svd.SvdPortInUse("in use")
+        self.assertRaises(
+            RuntimeError, self.ext._create_svd_with_free_port)
+        self.assertEqual(10, self.ext.svd.create.call_count)
+
+
 class TestVrfHandler(base.BaseTestCase):
 
     def setUp(self):