quattor · jrha · Nov 5, 2024 · Sep 19, 2024
diff --git a/ncm-network/src/main/perl/network.pm b/ncm-network/src/main/perl/network.pm
@@ -792,7 +792,7 @@ sub process_network
                 $iface->{$attr} = [map {"$_=$opts->{$_}"} sort keys %$opts];
                 $self->debug(1, "Replaced $attr with ", join(' ', @{$iface->{$attr}}), " for interface $ifname");
 
-                # for bonding_opts, we need linkagregation settings for nmstate.
+                # for bonding_opts, we need link aggregation settings for nmstate.
                 # this should not impact existing configs as it adds interface/$name/link_aggregation
                 if ($attr eq "bonding_opts"){
                     foreach my $opt (sort keys %$opts){
@@ -906,6 +906,18 @@ sub process_network
             }
         }
 
+        # insert ovs bridge->ports data
+        if ($iface->{ovs_bridge}) {
+            my $bridge = $iface->{ovs_bridge};
+            my $bridgeiface = $nwtree->{interfaces}->{$bridge};
+            if ($bridgeiface) {
+                $bridgeiface->{ports} = [] if !exists($bridgeiface->{ports});
+                push(@{$bridgeiface->{ports}}, $ifname);
+            } else {
+                $self->warn("Interface $ifname has OVS bridge $bridge configured, but corresponding iface entry not found");
+            }
+        }
+
     }
 
     return $nwtree;

diff --git a/ncm-network/src/main/perl/nmstate.pm b/ncm-network/src/main/perl/nmstate.pm
@@ -376,22 +376,27 @@ sub generate_nmstate_config
     my $is_ip = exists $iface->{ip} ? 1 : 0;
     my $is_vlan_eth = exists $iface->{vlan} ? 1 : 0;
     my $is_partof_bond = exists $iface->{master} ? 1 : 0;
+    my $can_ignore_bootproto = $is_partof_bond;
     my $iface_changed = 0;
 
     # create hash of interface entries that will be used by nmstate config.
-    my $ifaceconfig->{name} = $name;
+    my $ifaceconfig = {
+        name => $name,
+        'profile-name' => $name,
+    };
 
     $ifaceconfig->{mtu} = $iface->{mtu} if $iface->{mtu};
     $ifaceconfig->{'mac-address'} = $iface->{hwaddr} if $iface->{hwaddr};
-    $ifaceconfig->{'profile-name'} = $name;
 
     # this will be empty if the interface isnt a bond interface.
     # we can use this to determine if this interface is bond interface.
     my $bonded_eth = get_bonded_eth($self, $name, $net->{interfaces});
 
     my $vlan_id = $self->find_vlan_id($name, $iface->{device});
 
-    if (lc($iface->{type} || '') eq 'infiniband') {
+    my $lctype = lc($iface->{type} || '');
+
+    if ($lctype eq 'infiniband') {
         $ifaceconfig->{type} = "infiniband";
         my $ib = {};
         my $pkey = $vlan_id || 65535;
@@ -403,11 +408,22 @@ sub generate_nmstate_config
         $ib->{pkey} = "0x" . sprintf("%04x", $pkey);
         $ib->{mode} = 'datagram';  # TODO: add connected mode, but who still uses that
         $ifaceconfig->{infiniband} = $ib;
+    } elsif ($lctype eq 'ovsbridge') {
+        $can_ignore_bootproto ||= 1;
+        $ifaceconfig->{type} = "ovs-bridge";
+        $ifaceconfig->{state} = "up";
+        $ifaceconfig->{bridge}->{port} = [ map { {name => $_} } (@{$iface->{ports}}, $name) ];
+    } elsif ($lctype eq 'ovsintport') {
+        $can_ignore_bootproto ||= 1;
+        # TODO: when extending this to ovsport, deal with eg type=ovsport driver=bonding
+        #    (which is a bond interface and should not be handled here)
+        $ifaceconfig->{type} = "ovs-interface";
+        $ifaceconfig->{state} = "up";
     } elsif ($is_eth) {
         $ifaceconfig->{type} = "ethernet";
         if ($is_partof_bond) {
             # no ipv4 address for bonded eth, plus in nmstate bonded eth is controlled by controller. no config is required.
-            $ifaceconfig->{ipv4}->{enabled} = "false";
+            $ifaceconfig->{ipv4}->{enabled} = $YFALSE;
             $ifaceconfig->{state} = "up";
         }
     } elsif ($is_vlan_eth) {
@@ -420,7 +436,9 @@ sub generate_nmstate_config
         $ifaceconfig->{vlan}->{'id'} = $vlan_id;
     } elsif (@$bonded_eth) {
         # if bond device
+        $can_ignore_bootproto ||= 1;
         $ifaceconfig->{type} = "bond";
+        $ifaceconfig->{state} = "up";
         $ifaceconfig->{'link-aggregation'} = $iface->{link_aggregation};
         if ($bonded_eth){
             $ifaceconfig->{'link-aggregation'}->{port} = $bonded_eth;
@@ -481,15 +499,15 @@ sub generate_nmstate_config
         } else {
             $self->error("No ip address defined for static bootproto");
         }
-    } elsif (($eth_bootproto eq "dhcp") && (!$is_partof_bond)) {
+    } elsif (($eth_bootproto eq "dhcp") && (!$can_ignore_bootproto)) {
         # dhcp configuration
         $ifaceconfig->{state} = "up";
         $ifaceconfig->{ipv4}->{dhcp} = $YTRUE;
         $ifaceconfig->{ipv4}->{enabled} = $YTRUE;
-    } elsif (($eth_bootproto eq "none") && (!$is_partof_bond)) {
+    } elsif (($eth_bootproto eq "none") && (!$can_ignore_bootproto)) {
         # no ip on interface and is not a part of a bonded interface, assume not managed so disable eth.
-        $ifaceconfig->{ipv4}->{enabled} = "false";
-        $ifaceconfig->{ipv6}->{enabled} = "false";
+        $ifaceconfig->{ipv4}->{enabled} = $YFALSE;
+        $ifaceconfig->{ipv6}->{enabled} = $YFALSE;
         $ifaceconfig->{state} = "down";
     } elsif ($eth_bootproto eq "bootp"){
         $self->error("bootp bootproto not supported by nmstate");
@@ -553,6 +571,23 @@ sub generate_nmstate_config
     }
     # return hash construct that will match what nmstate yml needs.
     my $interface->{interfaces} = [$ifaceconfig];
+
+    # insert 2nd interface: an ovs-interface with same name (a so-called admin interafce)
+    if (($ifaceconfig->{type} || '') eq 'ovs-bridge') {
+        push(@{$interface->{interfaces}}, {
+            type => "ovs-interface",
+            state => "up",
+            name => $name,
+            'profile-name' => $name,
+            ipv4 => {
+                enabled => $YFALSE,
+            },
+            ipv6 => {
+                enabled => $YFALSE,
+            },
+        });
+    };
+
     if (scalar @$routes) {
         $interface->{routes}->{config} = $routes;
     }
@@ -567,7 +602,7 @@ sub generate_nmstate_config
     # TODO: bridge_options
     # TODO: veth, anymore?
 
-    return $interface;
+    return $interface, $ifaceconfig;
 };
 
 # Generate hash of dns-resolver config for nmstate.
@@ -662,18 +697,51 @@ sub clear_inactive_nm_connections
     }
 }
 
-sub nmstate_apply
+# return ordered list of interface keys
+#    if_updwon is a hasref in if_up / if_down format
+sub nmstate_order
 {
-    my ($self, $exifiles, $ifup, $ifdown, $nwsrv) = @_;
+    my ($self, $if_updown, $full_ifaces) = @_;
+
+    # do no use the value 0 in the score; it will break the || $default logic when the value is 0
+    my $default = 10;  # lowest / first
+    # these are nmstate types
+    my $score = {
+        bond => 20,  # slaves need to be alive
+        "ovs-interface" => 30,  # can only be ports of a bridge, this could probably be 0 as well
+        "ovs-bridge" => 40,  # needs ports alive; these can be anything with lower score
+    };
+
+    my $get_score = sub {
+        my $ifname = shift;
 
+        # devices to remove most likely have no data
+        my $guesstype = 'unknown';
+        $guesstype = 'bond' if $ifname =~ m/^bond/;
+
+        my $ifdata = $full_ifaces->{$ifname} || {};
+        return $score->{$ifdata->{type} || $guesstype} || $default;
+    };
+
+    # sort on score, and with equal score alphabetic
+    my @sorted_ifnames = sort {
+        &$get_score($a) <=> &$get_score($b) || $a cmp $b
+    } keys %$if_updown;
+
+    # re-apply the ovs-interfaces, an all-in-one config yaml would solve this
+    my @reapply = grep { &$get_score($_) == 30 } @sorted_ifnames;
+
+    return @sorted_ifnames, @reapply;
+}
+
+
+sub nmstate_apply
+{
+    my ($self, $exifiles, $ifup, $ifdown, $nwsrv, $ifaces) = @_;
 
-    my @ifaces = sort keys %$ifup;
-    my @ifaces_down = sort keys %$ifdown;
 
-    # primitive re-ordering to make sure eg bond are apply'ed last, and removed first
-    my $order_pattern = '^bond';
-    @ifaces = ((grep {$_ !~ m/$order_pattern/} @ifaces), (grep {$_ =~ m/$order_pattern/} @ifaces));
-    @ifaces_down = ((grep {$_ =~ m/$order_pattern/} @ifaces_down), (grep {$_ !~ m/$order_pattern/} @ifaces_down));
+    my @ifaces = $self->nmstate_order($ifup, $ifaces);
+    my @ifaces_down = reverse $self->nmstate_order($ifdown, $ifaces);
 
     my $action;
 
@@ -788,11 +856,13 @@ sub Configure
     $self->routing_table($nwtree->{routing_table});
 
     my $ipv6 = $nwtree->{ipv6};
+    my $nmifaces = {};
     foreach my $ifacename (sort keys %$ifaces) {
         my $iface = $ifaces->{$ifacename};
-        my $nmstate_cfg = generate_nmstate_config($self, $ifacename, $net, $ipv6, $nwtree->{routing_table}, $dgw);
+        my ($nm_cfg, $nm_iface) = generate_nmstate_config($self, $ifacename, $net, $ipv6, $nwtree->{routing_table}, $dgw);
+        $nmifaces->{$ifacename} = $nm_iface;
         my $file_name = $self->iface_filename($ifacename);
-        $exifiles->{$file_name} = $self->nmstate_file_dump($file_name, $nmstate_cfg);
+        $exifiles->{$file_name} = $self->nmstate_file_dump($file_name, $nm_cfg);
 
         $self->ethtool_opts_keeps_state($file_name, $ifacename, $iface, $exifiles);
     }
@@ -888,7 +958,7 @@ sub Configure
     # nmstatectl manages rollback too when options are misconfigured in yml config
     # This is still used to mark interfaces to apply any changes via nmstatectl
     # This will also down/delete any interface connection for which config was removed.
-    my $stopstart += $self->nmstate_apply($exifiles, $ifup, $ifdown, $nwsrv);
+    my $stopstart += $self->nmstate_apply($exifiles, $ifup, $ifdown, $nwsrv, $nmifaces);
     $init_config .= "\nPOST APPLY\n";
     $init_config .= $self->get_current_config();
 

diff --git a/ncm-network/src/test/perl/nmstate_advance.t b/ncm-network/src/test/perl/nmstate_advance.t
@@ -193,7 +193,7 @@ interfaces:
       prefix-length: 24
     dhcp: false
     enabled: true
-  mac-address: 6e:a5:1b:55:77:0d
+  mac-address: 6e:a5:1b:55:77:0e
   name: eth4
   profile-name: eth4
   state: up
@@ -254,6 +254,56 @@ routes:
     state: absent
 EOF
 
+Readonly my $OVS_BRIDGE_YML => <<EOF;
+# File generated by NCM::Component::nmstate. Do not edit
+---
+interfaces:
+- bridge:
+    port:
+    - name: bond1
+    - name: eth1000
+    - name: br100
+  name: br100
+  profile-name: br100
+  state: up
+  type: ovs-bridge
+- ipv4:
+    enabled: false
+  ipv6:
+    enabled: false
+  name: br100
+  profile-name: br100
+  state: up
+  type: ovs-interface
+routes:
+  config:
+  - next-hop-interface: br100
+    state: absent
+EOF
+
+Readonly my $OVS_INTPORT_YML => <<EOF;
+# File generated by NCM::Component::nmstate. Do not edit
+---
+interfaces:
+- ipv4:
+    address:
+    - ip: 4.3.2.1
+      prefix-length: 24
+    dhcp: false
+    enabled: true
+  name: eth1000
+  profile-name: eth1000
+  state: up
+  type: ovs-interface
+routes:
+  config:
+  - next-hop-interface: eth1000
+    state: absent
+  - destination: 0.0.0.0/0
+    next-hop-address: 4.3.2.254
+    next-hop-interface: eth1000
+EOF
+
 Readonly my $RT => <<EOF;
 #
 # reserved values
@@ -318,6 +368,13 @@ is($dummy_yml, $DUMMY_YML, "Exact dummy interface yml config");
 my $alias_yml = get_file_contents("/etc/nmstate/eth4.yml");
 is($alias_yml, $ALIAS_YML, "Exact alias interface yml config");
 
+my $ovsbryml = get_file_contents("/etc/nmstate/br100.yml");
+is($ovsbryml, $OVS_BRIDGE_YML, "Exact br100 yml config");
+
+my $ovsintyml = get_file_contents("/etc/nmstate/eth1000.yml");
+is($ovsintyml, $OVS_INTPORT_YML, "Exact eth1000 yml config");
+
+
 diag "all history commands ", explain \@Test::Quattor::command_history;
 
 # apply commands are sorted alphabetically, bond last
@@ -333,6 +390,11 @@ ok(command_history_ok([
   '/usr/bin/nmstatectl apply /etc/nmstate/ib1.12345.yml',
   '/usr/bin/nmstatectl apply /etc/nmstate/vlan0.yml',
   '/usr/bin/nmstatectl apply /etc/nmstate/bond0.yml',
+  '/usr/bin/nmstatectl apply /etc/nmstate/bond1.yml',
+  '/usr/bin/nmstatectl apply /etc/nmstate/eth1000.yml',
+  '/usr/bin/nmstatectl apply /etc/nmstate/br100.yml',
+  '/usr/bin/nmstatectl apply /etc/nmstate/eth1000.yml',  # reapplied
+  '/usr/bin/nmstatectl apply /etc/nmstate/resolv.yml',
 ], []));
 
 

diff --git a/ncm-network/src/test/resources/nmstate_advance.pan b/ncm-network/src/test/resources/nmstate_advance.pan
@@ -64,12 +64,38 @@ prefix "/system/network/vips/myvip";
 "interfaces/0" = "eth0";
 
 # create aliases interfaces
-"/hardware/cards/nic/eth4/hwaddr" = "6e:a5:1b:55:77:0d";
+"/hardware/cards/nic/eth4/hwaddr" = "6e:a5:1b:55:77:0e";
 prefix "/system/network/interfaces/eth4";
 "ip" = "4.3.2.11";
 "netmask" = "255.255.255.0";
 "broadcast" = "4.3.2.255";
 "aliases/dba/broadcast" = "4.3.2.255";
 "aliases/dba/fqdn" = "host-alias1.quattor.com";
 "aliases/dba/ip" = "4.3.2.12";
-"aliases/dba/netmask" = "255.255.255.0";
+"aliases/dba/netmask" = "255.255.255.0";
+
+# ovs construction
+"/system/network/interfaces/bond1" = dict(
+    "driver", "bonding",
+    "type", "OVSPort",
+    "ovs_bridge", "br100",
+    );
+
+"/hardware/cards/nic/eth10/hwaddr" = "6e:a5:1b:55:77:10";
+"/system/network/interfaces/eth10/master" = "bond1";
+
+"/hardware/cards/nic/eth11/hwaddr" = "6e:a5:1b:55:77:11";
+"/system/network/interfaces/eth11/master" = "bond1";
+
+"/system/network/interfaces/br100" = dict(
+    "type", "OVSBridge",
+    "bootproto", "static",
+);
+
+"/system/network/interfaces/eth1000" = dict(
+    "broadcast", '4.3.2.255',
+    "ip", '4.3.2.1',
+    "netmask", '255.255.255.0',
+    "type", "OVSIntPort",
+    "ovs_bridge", "br100",
+);