[PATCH pve-network v2 09/16] sdn: zones: attach the dhcp responder on tap plug, detach on unplug

[Hannes Laimer <h.laimer@proxmox.com>]

Program attach is per guest interface and the plug is the one moment
every such interface passes through on this node, covering guest
start, hotplug and incoming migration. The responder attaches before
the interface joins the bridge, so a guest already running cannot slip
its first request past it. The plug hands over the MAC of the guest
NIC, which the responder answers the interface for. Attach failures
only warn, a guest start must not depend on the responder. An unplug
reaches the backends the same way, with the interface alone, since its
zone is gone with it. A tap plugged onto a plain bridge is reported to
the backends as well, so a responder still attached to it lets go.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/PVE/Network/SDN/Zones.pm        |  31 ++++++-
 src/test/run_test_vnets_blackbox.pl | 129 ++++++++++++++++++++++++++++
 2 files changed, 156 insertions(+), 4 deletions(-)

diff --git a/src/PVE/Network/SDN/Zones.pm b/src/PVE/Network/SDN/Zones.pm
index f668303..cda8176 100644
--- a/src/PVE/Network/SDN/Zones.pm
+++ b/src/PVE/Network/SDN/Zones.pm
@@ -323,16 +323,18 @@ sub veth_create {
 }
 
 sub tap_plug {
-    my ($iface, $bridge, $tag, $firewall, $trunks, $rate) = @_;
+    my ($iface, $bridge, $tag, $firewall, $trunks, $rate, $opts) = @_;
 
     my $vnet = PVE::Network::SDN::Vnets::get_vnet($bridge, 1);
     if (!$vnet) { # fallback for classic bridge
         my $interfaces_config = PVE::INotify::read_file('interfaces');
-        my $opts = {};
-        $opts->{learning} = 0
+        my $bridge_opts = {};
+        $bridge_opts->{learning} = 0
             if $interfaces_config->{ifaces}->{$bridge}
             && $interfaces_config->{ifaces}->{$bridge}->{'bridge-disable-mac-learning'};
-        PVE::Network::tap_plug($iface, $bridge, $tag, $firewall, $trunks, $rate, $opts);
+        # the dhcp backends hear of an interface moving onto a plain bridge as well
+        PVE::Network::SDN::Dhcp::tap_plug($bridge, $iface, $opts->{mac});
+        PVE::Network::tap_plug($iface, $bridge, $tag, $firewall, $trunks, $rate, $bridge_opts);
         return;
     }
 
@@ -343,9 +345,30 @@ sub tap_plug {
         if $plugin_config->{nodes} && !defined($plugin_config->{nodes}->{$nodename});
 
     my $plugin = PVE::Network::SDN::Zones::Plugin->lookup($plugin_config->{type});
+
+    # the responder attaches before the interface joins the bridge, so a running guest cannot
+    # slip its first request past it. Loaded through Vnets, a use here would close a
+    # load-order cycle
+    PVE::Network::SDN::Dhcp::tap_plug($bridge, $iface, $opts->{mac});
     $plugin->tap_plug($plugin_config, $vnet, $tag, $iface, $bridge, $firewall, $trunks, $rate);
 }
 
+# the interface is gone or leaving, the dhcp backends drop what they hold
+# for it before the bridge side is cleaned up
+sub tap_unplug {
+    my ($iface) = @_;
+
+    PVE::Network::SDN::Dhcp::tap_unplug($iface);
+    PVE::Network::tap_unplug($iface);
+}
+
+sub veth_delete {
+    my ($veth) = @_;
+
+    PVE::Network::SDN::Dhcp::tap_unplug($veth);
+    PVE::Network::veth_delete($veth);
+}
+
 sub add_bridge_fdb {
     my ($iface, $macaddr, $bridge) = @_;
 
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 2af4880..064a96c 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -1770,6 +1770,135 @@ sub test_ebpf_backend_edge_cases {
 }
 
 run_test(\&test_ebpf_backend);
+
+sub test_zone_tap_plug {
+    my $test_name = (split(/::/, (caller(0))[3]))[-1];
+    my $zoneid = "TESTZONE";
+    my $vnetid = "testvnet";
+    my $mac = "da:65:8f:18:9b:6f";
+
+    # the zone plug hands the MAC on to the dhcp backends, a plug onto a
+    # plain bridge tells them where the interface went
+    create_zone({
+        type => "simple",
+        dhcp => "ebpf",
+        ipam => "pve",
+        zone => $zoneid,
+    });
+    create_vnet({
+        type => "vnet",
+        zone => $zoneid,
+        vnet => $vnetid,
+    });
+    create_subnet({
+        type => "subnet",
+        vnet => $vnetid,
+        subnet => "10.0.0.0/24",
+        gateway => "10.0.0.1",
+        'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.200"],
+    });
+    create_ip({
+        zone => $zoneid,
+        vnet => $vnetid,
+        mac => $mac,
+        ip => "10.0.0.100",
+    });
+    my $bridged = [];
+    my $unplugged = [];
+    $mocked_sdn_zones_super_plugin->mock(
+        tap_plug => sub {
+            push @$bridged, $_[5];
+            push $test_state->{order}->@*, 'bridge';
+        },
+    );
+    my $mocked_pve_network = Test::MockModule->new('PVE::Network');
+    $mocked_pve_network->mock(
+        tap_plug => sub {
+            push @$bridged, $_[1];
+            push $test_state->{order}->@*, 'bridge';
+        },
+        tap_unplug => sub { push @$unplugged, $_[0]; },
+        veth_delete => sub { push @$unplugged, $_[0]; },
+    );
+    $mocked_pve_inotify->mock(read_file => sub { return { ifaces => {} }; });
+    take_ebpf_calls();
+    take_order();
+
+    PVE::Network::SDN::Zones::tap_plug('tap999i0', $vnetid, undef, 0, undef, undef,
+        { mac => $mac });
+    eq_or_diff($bridged, [$vnetid], "$test_name: the zone plugin plugs the interface");
+    eq_or_diff(
+        take_ebpf_calls(),
+        [{ method => 'attach', args => ['tap999i0', $vnetid, $ebpf_record->("10.0.0.100")] }],
+        "$test_name: and the dhcp backend attaches it with the MAC's record",
+    );
+    is_deeply(
+        take_order(),
+        ['draw', 'refresh', 'write', 'bridge'],
+        "$test_name: the responder attaches before the interface joins the bridge",
+    );
+
+    @$bridged = ();
+    PVE::Network::SDN::Zones::tap_plug('tap999i0', 'vmbr0', undef, 0, undef, undef,
+        { mac => $mac });
+    eq_or_diff($bridged, ['vmbr0'], "$test_name: a plain bridge is plugged the classic way");
+    eq_or_diff(
+        take_ebpf_calls(),
+        [{ method => 'detach', args => ['tap999i0', { vnet => undef }] }],
+        "$test_name: and the dhcp backends hear where the interface went",
+    );
+
+    # a vnet of another backend is a place the ebpf backend hears of as well
+    create_zone({
+        type => "simple",
+        dhcp => "dnsmasq",
+        ipam => "pve",
+        zone => "DNSZONE",
+    });
+    create_vnet({
+        type => "vnet",
+        zone => "DNSZONE",
+        vnet => "dnsvnet",
+    });
+    @$bridged = ();
+    PVE::Network::SDN::Zones::tap_plug(
+        'tap999i0',
+        'dnsvnet',
+        undef,
+        0,
+        undef,
+        undef,
+        { mac => $mac },
+    );
+    eq_or_diff(
+        $bridged,
+        ['dnsvnet'],
+        "$test_name: a vnet of another backend is plugged by its zone",
+    );
+    eq_or_diff(
+        take_ebpf_calls(),
+        [{ method => 'detach', args => ['tap999i0', { vnet => 'dnsvnet' }] }],
+        "$test_name: and the ebpf backend hears which vnet the interface went to",
+    );
+
+    # the zone unplug and the veth delete report the interface gone before
+    # the classic ones run
+    PVE::Network::SDN::Zones::tap_unplug('tap999i0');
+    PVE::Network::SDN::Zones::veth_delete('veth999i0');
+    eq_or_diff($unplugged, ['tap999i0', 'veth999i0'], "$test_name: the classic unplugs run");
+    eq_or_diff(
+        take_ebpf_calls(),
+        [
+            { method => 'detach', args => ['tap999i0', undef] },
+            { method => 'detach', args => ['veth999i0', undef] },
+        ],
+        "$test_name: and the dhcp backends hear the interfaces are gone",
+    );
+    $mocked_sdn_zones_super_plugin->unmock('tap_plug');
+    $mocked_pve_inotify->unmock('read_file');
+}
+
+run_test(\&test_zone_tap_plug);
 run_test(\&test_ebpf_backend_edge_cases);
 
 sub test_ebpf_mac_case {
-- 
2.47.3