From: Martin Willi <martin@strongswan.org>
Date: Fri, 16 Feb 2024 09:42:43 +0000 (+0100)
Subject: child-sa: Move unique mark allocation to a separate helper function
X-Git-Tag: android-2.5.0~9^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dde40bcb9eedcb8b3d42c49aa29f98833ba3f720;p=thirdparty%2Fstrongswan.git

child-sa: Move unique mark allocation to a separate helper function

This aligns the code with unique interface ID allocation, which uses a helper
function for the same purpose and mechanic as well.
---

diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c
index aeb46ed432..97ee88acbc 100644
--- a/src/libcharon/sa/child_sa.c
+++ b/src/libcharon/sa/child_sa.c
@@ -2038,7 +2038,7 @@ child_sa_t *child_sa_create(host_t *me, host_t *other, child_cfg_t *config,
 							child_sa_create_t *data)
 {
 	private_child_sa_t *this;
-	static refcount_t unique_id = 0, unique_mark = 0;
+	static refcount_t unique_id = 0;
 
 	INIT(this,
 		.public = {
@@ -2127,27 +2127,7 @@ child_sa_t *child_sa_create(host_t *me, host_t *other, child_cfg_t *config,
 	}
 
 	allocate_unique_if_ids(&this->if_id_in, &this->if_id_out);
-
-	if (MARK_IS_UNIQUE(this->mark_in.value) ||
-		MARK_IS_UNIQUE(this->mark_out.value))
-	{
-		refcount_t mark = 0;
-		bool unique_dir = this->mark_in.value == MARK_UNIQUE_DIR ||
-						  this->mark_out.value == MARK_UNIQUE_DIR;
-
-		if (!unique_dir)
-		{
-			mark = ref_get(&unique_mark);
-		}
-		if (MARK_IS_UNIQUE(this->mark_in.value))
-		{
-			this->mark_in.value = unique_dir ? ref_get(&unique_mark) : mark;
-		}
-		if (MARK_IS_UNIQUE(this->mark_out.value))
-		{
-			this->mark_out.value = unique_dir ? ref_get(&unique_mark) : mark;
-		}
-	}
+	allocate_unique_marks(&this->mark_in.value, &this->mark_out.value);
 
 	if (!this->reqid)
 	{
diff --git a/src/libstrongswan/ipsec/ipsec_types.c b/src/libstrongswan/ipsec/ipsec_types.c
index bd8a4a52cb..6f10adf703 100644
--- a/src/libstrongswan/ipsec/ipsec_types.c
+++ b/src/libstrongswan/ipsec/ipsec_types.c
@@ -150,6 +150,34 @@ bool mark_from_string(const char *value, mark_op_t ops, mark_t *mark)
 	return TRUE;
 }
 
+/*
+ * Described in header
+ */
+void allocate_unique_marks(uint32_t *in, uint32_t *out)
+{
+	static refcount_t unique_mark = 0;
+
+	if (MARK_IS_UNIQUE(*in) || MARK_IS_UNIQUE(*out))
+	{
+		refcount_t mark = 0;
+		bool unique_dir = *in == MARK_UNIQUE_DIR ||
+						  *out == MARK_UNIQUE_DIR;
+
+		if (!unique_dir)
+		{
+			mark = ref_get(&unique_mark);
+		}
+		if (MARK_IS_UNIQUE(*in))
+		{
+			*in = unique_dir ? ref_get(&unique_mark) : mark;
+		}
+		if (MARK_IS_UNIQUE(*out))
+		{
+			*out = unique_dir ? ref_get(&unique_mark) : mark;
+		}
+	}
+}
+
 /*
  * Described in header
  */
diff --git a/src/libstrongswan/ipsec/ipsec_types.h b/src/libstrongswan/ipsec/ipsec_types.h
index 5320c46abc..b3a26ce8ee 100644
--- a/src/libstrongswan/ipsec/ipsec_types.h
+++ b/src/libstrongswan/ipsec/ipsec_types.h
@@ -242,6 +242,18 @@ enum mark_op_t {
  */
 bool mark_from_string(const char *value, mark_op_t ops, mark_t *mark);
 
+/**
+ * Allocate up to two unique marks depending on the given values.
+ *
+ * If the given values are MARK_UNIQUE, the values get replaced by a single
+ * unique mark. If the given values are MARK_UNIQUE_DIR, the values get
+ * replaced by a single unique mark for each direction.
+ *
+ * @param[out] in	inbound interface ID
+ * @param[out] out	outbound interface ID
+ */
+void allocate_unique_marks(uint32_t *in, uint32_t *out);
+
 /**
  * Special interface ID values to allocate a unique ID for each CHILD_SA/dir
  */
diff --git a/src/libstrongswan/tests/suites/test_utils.c b/src/libstrongswan/tests/suites/test_utils.c
index 4662fa8867..bbaca6debb 100644
--- a/src/libstrongswan/tests/suites/test_utils.c
+++ b/src/libstrongswan/tests/suites/test_utils.c
@@ -1124,6 +1124,41 @@ START_TEST(test_mark_from_string)
 }
 END_TEST
 
+/*******************************************************************************
+ * allocate_unique_marks
+ */
+
+static struct {
+	uint32_t in;
+	uint32_t out;
+	uint32_t exp_in;
+	uint32_t exp_out;
+} unique_mark_data[] = {
+	{0, 0, 0, 0 },
+	{42, 42, 42, 42 },
+	{42, 1337, 42, 1337 },
+	/* each call increases the internal counter by 1 or 2*/
+	{MARK_UNIQUE, 42, 1, 42 },
+	{42, MARK_UNIQUE, 42, 2 },
+	{MARK_UNIQUE_DIR, 42, 3, 42 },
+	{42, MARK_UNIQUE_DIR, 42, 4 },
+	{MARK_UNIQUE, MARK_UNIQUE, 5, 5 },
+	{MARK_UNIQUE_DIR, MARK_UNIQUE, 6, 7 },
+	{MARK_UNIQUE, MARK_UNIQUE_DIR, 8, 9 },
+	{MARK_UNIQUE_DIR, MARK_UNIQUE_DIR, 10, 11 },
+};
+
+START_TEST(test_allocate_unique_marks)
+{
+	uint32_t mark_in = unique_mark_data[_i].in,
+			 mark_out = unique_mark_data[_i].out;
+
+	allocate_unique_marks(&mark_in, &mark_out);
+	ck_assert_int_eq(mark_in, unique_mark_data[_i].exp_in);
+	ck_assert_int_eq(mark_out, unique_mark_data[_i].exp_out);
+}
+END_TEST
+
 /*******************************************************************************
  * if_id_from_string
  */
@@ -1364,6 +1399,10 @@ Suite *utils_suite_create()
 	tcase_add_loop_test(tc, test_mark_from_string, 0, countof(mark_data));
 	suite_add_tcase(s, tc);
 
+	tc = tcase_create("allocate_unique_marks");
+	tcase_add_loop_test(tc, test_allocate_unique_marks, 0, countof(unique_mark_data));
+	suite_add_tcase(s, tc);
+
 	tc = tcase_create("if_id_from_string");
 	tcase_add_loop_test(tc, test_if_id_from_string, 0, countof(if_id_data));
 	suite_add_tcase(s, tc);