ipsec: Split the SA add_del API into an separate add and del

Type: improvement

the rationale being that the del only requires the SA's ID, so it's a
bit mean to require the client to fill out all the other information as
well.

Signed-off-by: Neale Ranns <neale@graphiant.com>
Change-Id: Ibbc20405e74d6a0e1a3797465ead5271f15888e4

3 files changed, 100 insertions, 56 deletions

diff --git a/src/vnet/ipsec/ipsec.api b/src/vnet/ipsec/ipsec.api
index 8d4580a2c28..be45c3e2401 100644
--- a/src/vnet/ipsec/ipsec.api
+++ b/src/vnet/ipsec/ipsec.api
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-option version = "5.0.1";
+option version = "5.0.2";
 
 import "vnet/ipsec/ipsec_types.api";
 import "vnet/interface_types.api";
@@ -211,6 +211,18 @@ define ipsec_sad_entry_add_del_v3
   bool is_add;
   vl_api_ipsec_sad_entry_v3_t entry;
 };
+define ipsec_sad_entry_add
+{
+  u32 client_index;
+  u32 context;
+  vl_api_ipsec_sad_entry_v3_t entry;
+};
+autoreply define ipsec_sad_entry_del
+{
+  u32 client_index;
+  u32 context;
+  u32 id;
+};
 
 define ipsec_sad_entry_add_del_reply
 {
@@ -231,6 +243,12 @@ define ipsec_sad_entry_add_del_v3_reply
   i32 retval;
   u32 stat_index;
 };
+define ipsec_sad_entry_add_reply
+{
+  u32 context;
+  i32 retval;
+  u32 stat_index;
+};
 
 /** \brief Add or Update Protection for a tunnel with IPSEC
 
diff --git a/src/vnet/ipsec/ipsec_api.c b/src/vnet/ipsec/ipsec_api.c
index a0c2768318f..73f4474a604 100644
--- a/src/vnet/ipsec/ipsec_api.c
+++ b/src/vnet/ipsec/ipsec_api.c
@@ -291,6 +291,11 @@ static void vl_api_ipsec_sad_entry_add_del_t_handler
   int rv;
 
   id = ntohl (mp->entry.sad_id);
+  if (!mp->is_add)
+    {
+      rv = ipsec_sa_unlock_id (id);
+      goto out;
+    }
   spi = ntohl (mp->entry.spi);
 
   rv = ipsec_proto_decode (mp->entry.protocol, &proto);
@@ -316,13 +321,10 @@ static void vl_api_ipsec_sad_entry_add_del_t_handler
   ip_address_decode2 (&mp->entry.tunnel_src, &tun.t_src);
   ip_address_decode2 (&mp->entry.tunnel_dst, &tun.t_dst);
 
-  if (mp->is_add)
-    rv = ipsec_sa_add_and_lock (
-      id, spi, proto, crypto_alg, &crypto_key, integ_alg, &integ_key, flags,
-      mp->entry.salt, htons (mp->entry.udp_src_port),
-      htons (mp->entry.udp_dst_port), &tun, &sa_index);
-  else
-    rv = ipsec_sa_unlock_id (id);
+  rv = ipsec_sa_add_and_lock (id, spi, proto, crypto_alg, &crypto_key,
+			      integ_alg, &integ_key, flags, mp->entry.salt,
+			      htons (mp->entry.udp_src_port),
+			      htons (mp->entry.udp_dst_port), &tun, &sa_index);
 
 out:
   /* *INDENT-OFF* */
@@ -355,6 +357,12 @@ static void vl_api_ipsec_sad_entry_add_del_v2_t_handler
   };
 
   id = ntohl (mp->entry.sad_id);
+  if (!mp->is_add)
+    {
+      rv = ipsec_sa_unlock_id (id);
+      goto out;
+    }
+
   spi = ntohl (mp->entry.spi);
 
   rv = ipsec_proto_decode (mp->entry.protocol, &proto);
@@ -387,13 +395,10 @@ static void vl_api_ipsec_sad_entry_add_del_v2_t_handler
   ip_address_decode2 (&mp->entry.tunnel_src, &tun.t_src);
   ip_address_decode2 (&mp->entry.tunnel_dst, &tun.t_dst);
 
-  if (mp->is_add)
     rv = ipsec_sa_add_and_lock (
       id, spi, proto, crypto_alg, &crypto_key, integ_alg, &integ_key, flags,
       mp->entry.salt, htons (mp->entry.udp_src_port),
       htons (mp->entry.udp_dst_port), &tun, &sa_index);
-  else
-    rv = ipsec_sa_unlock_id (id);
 
 out:
   /* *INDENT-OFF* */
@@ -404,65 +409,104 @@ out:
   /* *INDENT-ON* */
 }
 
-static void
-vl_api_ipsec_sad_entry_add_del_v3_t_handler (
-  vl_api_ipsec_sad_entry_add_del_v3_t *mp)
+static int
+ipsec_sad_entry_add_v3 (const vl_api_ipsec_sad_entry_v3_t *entry,
+			u32 *sa_index)
 {
-  vl_api_ipsec_sad_entry_add_del_v3_reply_t *rmp;
   ipsec_key_t crypto_key, integ_key;
   ipsec_crypto_alg_t crypto_alg;
   ipsec_integ_alg_t integ_alg;
   ipsec_protocol_t proto;
   ipsec_sa_flags_t flags;
-  u32 id, spi, sa_index = ~0;
+  u32 id, spi;
   tunnel_t tun;
   int rv;
 
-  id = ntohl (mp->entry.sad_id);
-  spi = ntohl (mp->entry.spi);
+  id = ntohl (entry->sad_id);
+  spi = ntohl (entry->spi);
 
-  rv = ipsec_proto_decode (mp->entry.protocol, &proto);
+  rv = ipsec_proto_decode (entry->protocol, &proto);
 
   if (rv)
-    goto out;
+    return (rv);
 
-  rv = ipsec_crypto_algo_decode (mp->entry.crypto_algorithm, &crypto_alg);
+  rv = ipsec_crypto_algo_decode (entry->crypto_algorithm, &crypto_alg);
 
   if (rv)
-    goto out;
+    return (rv);
 
-  rv = ipsec_integ_algo_decode (mp->entry.integrity_algorithm, &integ_alg);
+  rv = ipsec_integ_algo_decode (entry->integrity_algorithm, &integ_alg);
 
   if (rv)
-    goto out;
+    return (rv);
 
-  flags = ipsec_sa_flags_decode (mp->entry.flags);
+  flags = ipsec_sa_flags_decode (entry->flags);
 
   if (flags & IPSEC_SA_FLAG_IS_TUNNEL)
     {
-      rv = tunnel_decode (&mp->entry.tunnel, &tun);
+      rv = tunnel_decode (&entry->tunnel, &tun);
 
       if (rv)
-	goto out;
+	return (rv);
     }
 
-  ipsec_key_decode (&mp->entry.crypto_key, &crypto_key);
-  ipsec_key_decode (&mp->entry.integrity_key, &integ_key);
+  ipsec_key_decode (&entry->crypto_key, &crypto_key);
+  ipsec_key_decode (&entry->integrity_key, &integ_key);
 
-  if (mp->is_add)
-    rv = ipsec_sa_add_and_lock (
-      id, spi, proto, crypto_alg, &crypto_key, integ_alg, &integ_key, flags,
-      mp->entry.salt, htons (mp->entry.udp_src_port),
-      htons (mp->entry.udp_dst_port), &tun, &sa_index);
+  return ipsec_sa_add_and_lock (id, spi, proto, crypto_alg, &crypto_key,
+				integ_alg, &integ_key, flags, entry->salt,
+				htons (entry->udp_src_port),
+				htons (entry->udp_dst_port), &tun, sa_index);
+}
+
+static void
+vl_api_ipsec_sad_entry_add_del_v3_t_handler (
+  vl_api_ipsec_sad_entry_add_del_v3_t *mp)
+{
+  vl_api_ipsec_sad_entry_add_del_v3_reply_t *rmp;
+  u32 id, sa_index = ~0;
+  int rv;
+
+  id = ntohl (mp->entry.sad_id);
+
+  if (!mp->is_add)
+    {
+      rv = ipsec_sa_unlock_id (id);
+    }
   else
-    rv = ipsec_sa_unlock_id (id);
+    {
+      rv = ipsec_sad_entry_add_v3 (&mp->entry, &sa_index);
+    }
 
-out:
   REPLY_MACRO2 (VL_API_IPSEC_SAD_ENTRY_ADD_DEL_V3_REPLY,
 		{ rmp->stat_index = htonl (sa_index); });
 }
 
 static void
+vl_api_ipsec_sad_entry_del_t_handler (vl_api_ipsec_sad_entry_del_t *mp)
+{
+  vl_api_ipsec_sad_entry_del_reply_t *rmp;
+  int rv;
+
+  rv = ipsec_sa_unlock_id (ntohl (mp->id));
+
+  REPLY_MACRO (VL_API_IPSEC_SAD_ENTRY_DEL_REPLY);
+}
+
+static void
+vl_api_ipsec_sad_entry_add_t_handler (vl_api_ipsec_sad_entry_add_t *mp)
+{
+  vl_api_ipsec_sad_entry_add_reply_t *rmp;
+  u32 sa_index = ~0;
+  int rv;
+
+  rv = ipsec_sad_entry_add_v3 (&mp->entry, &sa_index);
+
+  REPLY_MACRO2 (VL_API_IPSEC_SAD_ENTRY_ADD_REPLY,
+		{ rmp->stat_index = htonl (sa_index); });
+}
+
+static void
 send_ipsec_spds_details (ipsec_spd_t * spd, vl_api_registration_t * reg,
 			 u32 context)
 {
diff --git a/test/vpp_ipsec.py b/test/vpp_ipsec.py
index 2bf7eda4c23..57e5f02696c 100644
--- a/test/vpp_ipsec.py
+++ b/test/vpp_ipsec.py
@@ -267,31 +267,13 @@ class VppIpsecSA(VppObject):
             entry['udp_src_port'] = self.udp_src
         if self.udp_dst:
             entry['udp_dst_port'] = self.udp_dst
-        r = self.test.vapi.ipsec_sad_entry_add_del_v3(is_add=1, entry=entry)
+        r = self.test.vapi.ipsec_sad_entry_add(entry=entry)
         self.stat_index = r.stat_index
         self.test.registry.register(self, self.test.logger)
         return self
 
     def remove_vpp_config(self):
-        r = self.test.vapi.ipsec_sad_entry_add_del_v3(
-            is_add=0,
-            entry={
-                'sad_id': self.id,
-                'spi': self.spi,
-                'integrity_algorithm': self.integ_alg,
-                'integrity_key': {
-                    'length': len(self.integ_key),
-                    'data': self.integ_key,
-                },
-                'crypto_algorithm': self.crypto_alg,
-                'crypto_key': {
-                    'data': self.crypto_key,
-                    'length': len(self.crypto_key),
-                },
-                'protocol': self.proto,
-                'tunnel': self.tunnel_encode(),
-                'salt': self.salt
-            })
+        self.test.vapi.ipsec_sad_entry_del(id=self.id)
 
     def object_id(self):
         return "ipsec-sa-%d" % self.id