Bugfix 8289: Vti mark values should be implicit

Vti tunnel uses fwmark from the kernel skbuff. This value is now internally
allocated instead of getting it from the configuration.

Also fixed 8286 where configuration was allowing both a tunnel and VTI between
the same vpn src/dst.

5 files changed, 178 insertions, 131 deletions

diff --git a/Makefile.am b/Makefile.am
index be8155b..21788a5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -21,6 +21,7 @@ sbin_cfgvti = src/cfgvti.c
 
 bin_sudo_users_SCRIPTS = scripts/vyatta-ipsec-dhcp.pl
 share_perl5_DATA = lib/Vyatta/VPN/Util.pm
+share_perl5_DATA += scripts/vtiIntf.pm
 
 logrotate_DATA = etc/logrotate.d/vyatta-ipsec-logs
 
diff --git a/scripts/vpn-config.pl b/scripts/vpn-config.pl
index 2abf3bb..7419b44 100755
--- a/scripts/vpn-config.pl
+++ b/scripts/vpn-config.pl
@@ -38,6 +38,7 @@ use Vyatta::VPN::Util;
 use Getopt::Long;
 use Vyatta::Misc;
 use NetAddr::IP;
+use Vyatta::VPN::vtiIntf;
 
 my $config_file;
 my $secrets_file;
@@ -401,7 +402,7 @@ if ( $vcVPN->exists('ipsec') ) {
       . " remote-users configured\n";
   }
   my $prev_peer = "";
-  my %marks = ();
+  vtiIntf::discoverVtiIntfs();
   foreach my $peer (@peers) {
     my $peer_ike_group =
       $vcVPN->returnValue("ipsec site-to-site peer $peer ike-group");
@@ -510,7 +511,16 @@ if ( $vcVPN->exists('ipsec') ) {
                     "$vpn_cfg_err No tunnels configured for peer \"$peer\".  At least"
                     . " one tunnel required per peer.\n");
       }
+    } else {
+      #
+      # Check if this is VTI
+      #
+      if ($vcVPN->exists("ipsec site-to-site peer $peer vti") ) {
+            vpn_die(["vpn", "ipsec", "site-to-site","peer",$peer,"vti"],
+                    "$vpn_cfg_err Both Vti and tunnel(s) configured for peer \"$peer\n");
+      }
     }
+
     foreach my $tunnel (@tunnels) {
 
       my $needs_passthrough = 'false';
@@ -1137,19 +1147,12 @@ if ( $vcVPN->exists('ipsec') ) {
       # and up/down script hook.
       #
       if ($isVti) {
-          my $mark = $vcVPN->returnValue("ipsec site-to-site peer $peer vti mark");
+          my $mark = vtiIntf::isVtimarkpresent($peer, $lip);
           if (!defined($mark) || $mark eq '') {
-            vpn_die(["vpn","ipsec","site-to-site","peer",$peer,"vti","mark"],
+            vpn_die(["vpn","ipsec","site-to-site","peer",$peer,"vti"],
                 "$vpn_cfg_err No mark specified for peer \"$peer\" vti\n");
           } else {
-              if (defined($marks{ $mark })) {
-                vpn_die(["vpn","ipsec","site-to-site","peer",$peer,"vti","mark"],
-                    "$vpn_cfg_err vti mark $mark already used.\n");
-              } else {
-                  $marks{ $mark } = 1;
-		  $mark += 0x90000000;
                   $genout .= "\tmark=$mark\n";
-              }
           }
           # up/down script hook.
           my $tunName = $vcVPN->returnValue("ipsec site-to-site peer $peer vti bind");
@@ -1550,5 +1553,4 @@ EOS
   close $dhcp_hook;
 }
 
-
 # end of file
diff --git a/scripts/vtiIntf.pm b/scripts/vtiIntf.pm
new file mode 100755
index 0000000..4844873
--- /dev/null
+++ b/scripts/vtiIntf.pm
@@ -0,0 +1,143 @@
+#!/usr/bin/perl -w
+#
+# module to find and store exisiting vti tunnels.
+
+# **** License ****
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# This code was originally developed by Vyatta, Inc.
+# Portions created by Vyatta are Copyright (C) 2012 Vyatta, Inc.
+# All Rights Reserved.
+#
+# Authors: Saurabh Mohan
+# Date: 2012
+# Description: Find and store exisiting vti tunnels
+#
+# **** End License ****
+#
+#
+
+package vtiIntf;
+
+use strict;
+
+# Collect set of existing Vti's.
+my %existingVtiName = ();
+my %existingVtiMark = ();
+my @VtiMarks;
+my $vtiMarkBase = 0x90000000;
+my $maxMarks = 2048;
+
+sub discoverVtiIntfs {
+    my @currentVtis = `/sbin/ip tunnel | grep "^vti"`;
+    if (@currentVtis != 0) {
+    	my ($remote, $local, $name, $mark);
+    	my $key;
+    	foreach my $curVti (@currentVtis) {
+    		($remote, $local, $name, $mark) = parseVtiTun($curVti);
+    		$key = "remote $remote local $local";
+    		$existingVtiName{$key} = $name;
+    		$existingVtiMark{$key} = $mark;
+    		$VtiMarks[$mark-$vtiMarkBase] = 1;
+    	}
+    }
+}
+
+#
+# Api takes as input the o/p of 'ip tunnel show' and
+#  returns a list with {remote,local,name,mark}
+# Example input:
+# vti2: ip/ip  remote 12.0.0.2  local 12.0.0.1  ttl inherit  nopmtudisc key 15
+# 
+sub parseVtiTun {
+	my ($tunop) = @_;
+	my ($tunName, $remote, $local, $mark);
+	if ($tunop =~ m/(^vti.*): .*/) {
+		$tunName = $1;
+	}
+	if ($tunop =~ m/remote ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/) {
+		$remote = $1;
+	}
+	if ($tunop =~ m/local ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/) {
+		$local = $1;
+	}
+	if ($tunop =~ m/key ([0-9\.]+)/) {
+		$mark = $1;
+	}
+	return($remote, $local, $tunName, $mark);
+}
+
+sub extractRemoteLocal {
+	my ($key) = @_;
+	my ($remote, $local);
+	if ($key =~ m/remote ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/) {
+		$remote = $1;
+	}
+	if ($key =~ m/local ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/) {
+		$local = $1;
+	}
+	return($remote, $local);
+}
+
+sub isVtinamepresent {
+	my ($remote, $local) = @_;
+	my $key = "remote $remote local $local";
+
+	if (exists $existingVtiName{$key} ) {
+		return $existingVtiName{$key};
+	}
+	return ""; 
+}
+
+#
+# Pass a referenct to the existing Vti names.
+#
+sub getVtiNames {
+	return (\%existingVtiName);
+}
+
+sub deleteVtinamepresent {
+	my ($remote, $local) = @_;
+	my $key = "remote $remote local $local";
+
+	if (exists $existingVtiName{$key} ) {
+		delete $existingVtiName{$key};
+	}
+}
+
+sub isVtimarkpresent {
+	my ($remote, $local) = @_;
+	my $key = "remote $remote local $local";
+
+	if (exists $existingVtiMark{$key} ) {
+		return $existingVtiMark{$key};
+	}
+	return ""; 
+}
+
+sub allocVtiMark {
+	for my $cmark (1 .. ($maxMarks-1)) {
+		if (! defined($VtiMarks[$cmark])) {
+			$VtiMarks[$cmark] = 1;
+			return $cmark + $vtiMarkBase ;
+		}
+	}
+	return 0;
+}
+
+sub freeVtiMark {
+	my ($freeMark) = @_;
+	if ($freeMark > 0 && $freeMark < $maxMarks) {
+		$VtiMarks[$freeMark] = 0;
+	}
+	return 0;
+}
+
+1;
diff --git a/scripts/vyatta-vti-config.pl b/scripts/vyatta-vti-config.pl
index 5c1de52..28ae548 100755
--- a/scripts/vyatta-vti-config.pl
+++ b/scripts/vyatta-vti-config.pl
@@ -35,6 +35,7 @@ use strict;
 use lib "/opt/vyatta/share/perl5";
 
 use Getopt::Long;
+use Vyatta::VPN::vtiIntf;
 
 
 my $vti_cfg_err = "VPN VTI configuration error:";
@@ -86,22 +87,7 @@ if ($checkref ne '' ) {
 # Following code is to configure the vti.
 #
 
-# Collect set of existing Vti's.
-my %existingVtiName = ();
-my %existingVtiMark = ();
-my $vtiMarkBase = 0x90000000;
-
-my @currentVtis = `/sbin/ip tunnel | grep "^vti"`;
-if (@currentVtis != 0) {
-	my ($remote, $local, $name, $mark);
-	my $key;
-	foreach my $curVti (@currentVtis) {
-		($remote, $local, $name, $mark) = parseVtiTun($curVti);
-		$key = "remote $remote local $local";
-		$existingVtiName{$key} = $name;
-		$existingVtiMark{$key} = $mark-$vtiMarkBase;
-	}
-}
+vtiIntf::discoverVtiIntfs();
 
 #
 # Prepare Vyatta::Config object
@@ -123,7 +109,6 @@ if (!$vcVPN->exists('ipsec site-to-site') ) {
     exit $result;
 }
 
-    my %marks = ();
     my %binds = ();
     my %vtiVpns = ();
     my @peers = $vcVPN->listNodes('ipsec site-to-site peer');
@@ -134,8 +119,8 @@ if (!$vcVPN->exists('ipsec site-to-site') ) {
         #
         # we have the vti configured.
         #
+        my $mark;
         my $lip = $vcVPN->returnValue("ipsec site-to-site peer $peer local-address");
-        my $mark = $vcVPN->returnValue("ipsec site-to-site peer $peer vti mark");
         my $tunName = $vcVPN->returnValue("ipsec site-to-site peer $peer vti bind");
         my $change = 0;
 
@@ -167,22 +152,6 @@ if (!$vcVPN->exists('ipsec site-to-site') ) {
             $binds{ $tunName } = 1;
         }
 
-        # Check mark is valid.
-        if (!defined($mark)) {
-            print STDERR "$vti_cfg_err mark not defined.\n";
-            exit -1;
-        }
-        if ($mark eq "") {
-            print STDERR "$vti_cfg_err Invalid mark \"$mark\".\n";
-            exit -1;
-        }
-        if (exists $marks{ $mark }) {
-                vti_die(["vpn","ipsec","site-to-site","peer",$peer,"vti","mark"],
-                    "vti mark $mark already used.\n");
-        } else {
-            $marks{ $mark } = 1;
-        }
-
         $gencmds .= "# For peer $peer local $lip.\n";
         #
         # Get the tunnel parameters.
@@ -200,27 +169,30 @@ if (!$vcVPN->exists('ipsec site-to-site') ) {
         my $description = $vcIntf->returnValue("vti $tunName description");
 
         # Check if the tunnel exists already.
-        my $vtiPresent = isVtinamepresent($peer, $lip);
+        my $vtiPresent = vtiIntf::isVtinamepresent($peer, $lip);
         if (defined($vtiPresent) && !($vtiPresent eq "")) {
             if ($vtiPresent ne $tunName) {
                 # Binding changed.
-    			$gencmds .= "sudo /sbin/ip link delete $vtiPresent &> /dev/null\n";
+		my $currMark = vtiIntf::isVtimarkpresent($peer, $lip);
+		$gencmds .= "sudo /sbin/ip link delete $vtiPresent &> /dev/null\n";
+		$gencmds .= iptableDelMark($peer, $lip, $currMark);
                 $change = 1;
             }
         }
 
-        my $existingMark = isVtimarkpresent($peer, $lip);
+        my $existingMark = vtiIntf::isVtimarkpresent($peer, $lip);
         if (defined($existingMark) && !($existingMark eq "")) {
-            if ($existingMark ne $mark) {
-                # Mark changed.
-                $gencmds .= iptableDelMark($peer, $lip, $existingMark);
-                $change = 1;
-            }
+	    $mark = $existingMark;
         } else {
+	    $mark = vtiIntf::allocVtiMark();
+	    if ($mark == 0) {
+                vti_die(["vpn","ipsec","site-to-site","peer",$peer,"vti"],
+                    "vti failed to create (not able to allocate a mark)\n");
+	    }
             $change = 1;
         }
 
-        deleteVtinamepresent($peer, $lip);
+        vtiIntf::deleteVtinamepresent($peer, $lip);
         if ($change eq 0) {
             next;
         }
@@ -229,7 +201,7 @@ if (!$vcVPN->exists('ipsec site-to-site') ) {
         # Set the configuration into the output string.
         #
         # By default we delete the tunnel...
-        my $genmark = $mark + $vtiMarkBase;
+        my $genmark = $mark;
         $gencmds .= "sudo /sbin/ip link delete $tunName &> /dev/null\n";
         $gencmds .= "sudo /opt/vyatta/sbin/cfgvti add name $tunName key $genmark remote $peer local $lip\n";
         foreach my $tunIP (@tunIPs) {
@@ -285,77 +257,10 @@ sub vti_check_reference {
     return 0;
 }
 
-#
-# Api takes as input the o/p of 'ip tunnel show' and
-#  returns a list with {remote,local,name,mark}
-# Example input:
-# vti2: ip/ip  remote 12.0.0.2  local 12.0.0.1  ttl inherit  nopmtudisc key 15
-# 
-sub parseVtiTun {
-	my ($tunop) = @_;
-	my ($tunName, $remote, $local, $mark);
-	if ($tunop =~ m/(^vti.*): .*/) {
-		$tunName = $1;
-	}
-	if ($tunop =~ m/remote ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/) {
-		$remote = $1;
-	}
-	if ($tunop =~ m/local ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/) {
-		$local = $1;
-	}
-	if ($tunop =~ m/key ([0-9\.]+)/) {
-		$mark = $1;
-	}
-	return($remote, $local, $tunName, $mark);
-}
-
-sub extractRemoteLocal {
-	my ($key) = @_;
-	my ($remote, $local);
-	if ($key =~ m/remote ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/) {
-		$remote = $1;
-	}
-	if ($key =~ m/local ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/) {
-		$local = $1;
-	}
-	return($remote, $local);
-}
-
-sub isVtinamepresent {
-	my ($remote, $local) = @_;
-	my $key = "remote $remote local $local";
-
-	if (exists $existingVtiName{$key} ) {
-		return $existingVtiName{$key};
-	}
-	return ""; 
-}
-
-sub deleteVtinamepresent {
-	my ($remote, $local) = @_;
-	my $key = "remote $remote local $local";
-
-	if (exists $existingVtiName{$key} ) {
-		delete $existingVtiName{$key};
-	}
-}
-
-sub isVtimarkpresent {
-	my ($remote, $local) = @_;
-	my $key = "remote $remote local $local";
-
-	if (exists $existingVtiMark{$key} ) {
-		return $existingVtiMark{$key};
-	}
-	return ""; 
-}
-
 sub iptableDelMark {
 	my ($remote, $local, $mark) = @_;
 	my $opcmd="";
 
-	$mark += $vtiMarkBase;
-
 	$opcmd .= "sudo iptables -t mangle -D PREROUTING -s $remote -d $local -p esp -j MARK --set-mark $mark\n";
 	$opcmd .= "sudo iptables -t mangle -D PREROUTING -s $remote -d $local -p udp --dport 4500 -j MARK --set-mark $mark\n";
 	return $opcmd;
@@ -365,8 +270,6 @@ sub iptableAddMark {
 	my ($remote, $local, $mark) = @_;
 	my $opcmd="";
 
-	$mark += $vtiMarkBase;
-
 	$opcmd .= "sudo iptables -t mangle -A PREROUTING -s $remote -d $local -p esp -j MARK --set-mark $mark\n";
 	$opcmd .= "sudo iptables -t mangle -A PREROUTING -s $remote -d $local -p udp --dport 4500 -j MARK --set-mark $mark\n";
 	return $opcmd;
@@ -375,12 +278,14 @@ sub iptableAddMark {
 sub cleanupVtiNotConfigured {
     # for all remaining entries in the Vtinamepresent hash
     # remove them from the system.
-    while (my ($tunKey, $presentVtiName) =  each(%existingVtiName) ) {
-        my ($remote, $local) = extractRemoteLocal($tunKey);
-        my $existingMark = isVtimarkpresent($remote, $local);
+    my $localVtiNames = vtiIntf::getVtiNames();
+    while (my ($tunKey, $presentVtiName) =  each(%$localVtiNames) ) {
+        my ($remote, $local) = vtiIntf::extractRemoteLocal($tunKey);
+        my $existingMark = vtiIntf::isVtimarkpresent($remote, $local);
         $gencmds .= "# For peer $remote local $local.\n";
         $gencmds .= "sudo /sbin/ip link delete $presentVtiName &> /dev/null\n";
         $gencmds .= iptableDelMark($remote, $local, $existingMark);
+        vtiIntf::freeVtiMark($existingMark);
     }
 }
 
diff --git a/templates/vpn/ipsec/site-to-site/peer/node.tag/vti/mark/node.def b/templates/vpn/ipsec/site-to-site/peer/node.tag/vti/mark/node.def
deleted file mode 100644
index 0ccee30..0000000
--- a/templates/vpn/ipsec/site-to-site/peer/node.tag/vti/mark/node.def
+++ /dev/null
@@ -1,4 +0,0 @@
-type: u32
-syntax:expression: $VAR(@) >= 0 && $VAR(@) <= 2047
-help: Mark associated with the secure tunnel interface [REQUIRED]
-val_help: u32:0-2047;