[PATCH 0/5] Correct Provisioning Database Tags Matching Behavior

[PATCH 0/5] Correct Provisioning Database Tags Matching Behavior

[Grant Erickson]

At least in North America / United States, there exist Cellular MVNOs
(particularly in the IoT / M2M veritical) that neither use mobile
virtual network operator (MVNO) service provider names (SPNs) nor use
unique home network identifiers (HNIs) (that is, mobile country code
(MCC) + mobile network code (MNC) pairs). Instead, they simply use the
HNI of the parent operator.

In addition, those MVNOs typically have two or three APN schemes:

    1. A "public" APN that is broadly used by one or more MVNOs for
       the parent operator that issues PUBLIC IP addresses and does
       NOT route through the MVNOs or parent operator data center
       infrastructure.

    2. A "private" APN that may or may NOT be broadly used by one
       or more MVNOs for the parent operator that issues PRIVATE IP
       addresses and does route through the MVNOs or parent operator
       data center infrastructure.

    3. A "private" APN that is used only by the MVNO that issues
       static or dynamic PRIVATE IP addresses, does route through the
       MVNO data center infrastructure and, from there, via a VPN to
       the customer services infrastructure.

       These are sufficiently MVNO-specific where they do not or
       should not play a role in a generic provisioning database.

Consequently, to effectively distinguish or steer a given device to
(1) or (2), a 'TagsFilter' must be used. However, as presently
implemented, a tag in 'TagsFilter' not only returns APNs matching the
tag(s) but also untagged APNs.

Assuming an APN of both (1) and (2) above, used by the Verizon KORE
Wireless MVNO, prior to this proposed change, the behavior was/is:

No tag specified:

    # ./lookup-apn 311 480
    Opening database in default location
    Searching for info for network: 311480, spn: <None>

    Name: 4G LTE Contract
    APN: vzwims
    Type: 28
    Proto: 2
    Tags: (null)

    Name: 4G LTE Contract (Public)
    APN: vzwinternet
    Type: 1
    Proto: 2
    Tags: kore-m2m-public

    Name: 4G LTE Contract (Private)
    APN: wyleslte.gw7.vzwentp
    Type: 1
    Proto: 2
    Tags: kore-m2m-private

    Name: 4G LTE Contract
    APN: vzwapp
    Type: 4
    Proto: 2
    Tags: (null)

As expected.

One specific tag specified:

    # ./lookup-apn -t "kore-m2m-public" 311 480
    Opening database in default location
    Searching for info for network: 311480, spn: <None>

    Name: 4G LTE Contract
    APN: vzwims
    Type: 28
    Proto: 2
    Tags: (null)

    Name: 4G LTE Contract (Public)
    APN: vzwinternet
    Type: 1
    Proto: 2
    Tags: kore-m2m-public

    Name: 4G LTE Contract
    APN: vzwapp
    Type: 4
    Proto: 2
    Tags: (null)

Surprise! Both 'vzwapp' and 'vzwims' are returned which were not
expected or desired.

Another specific tag specified:

    # ./lookup-apn -t "kore-m2m-private" 311 480
    Opening database in default location
    Searching for info for network: 311480, spn: <None>

    Name: 4G LTE Contract
    APN: vzwims
    Type: 28
    Proto: 2
    Tags: (null)

    Name: 4G LTE Contract (Private)
    APN: wyleslte.gw7.vzwentp
    Type: 1
    Proto: 2
    Tags: kore-m2m-private

    Name: 4G LTE Contract
    APN: vzwapp
    Type: 4
    Proto: 2
    Tags: (null)

Surprise! Both 'vzwapp' and 'vzwims' are returned which were not
expected or desired.

Two specific tags specified:

    # ./lookup-apn -t "kore-m2m-public,kore-m2m-private" 311 480
    Opening database in default location
    Searching for info for network: 311480, spn: <None>

    Name: 4G LTE Contract
    APN: vzwims
    Type: 28
    Proto: 2
    Tags: (null)

    Name: 4G LTE Contract (Public)
    APN: vzwinternet
    Type: 1
    Proto: 2
    Tags: kore-m2m-public

    Name: 4G LTE Contract (Private)
    APN: wyleslte.gw7.vzwentp
    Type: 1
    Proto: 2
    Tags: kore-m2m-private

    Name: 4G LTE Contract
    APN: vzwapp
    Type: 4
    Proto: 2
    Tags: (null)

Surprise! Both 'vzwapp' and 'vzwims' are returned which were not
expected or desired.

With this change, specifying 'TagsFilter' now follows the rule of
least astonishment and matches expectations, returning only those APNs
which match a tag in 'TagsFilter', period.

One specific tag:

    # ./lookup-apn -t "kore-m2m-public" 311 480
    Opening database in default location
    Searching for info for network: 311480, spn: <None>
    Name: 4G LTE Contract (Public)
    APN: vzwinternet
    Type: 1
    Proto: 2
    Tags: kore-m2m-public

As desired / expected.

Another specific tag:

    # ./lookup-apn -t "kore-m2m-private" 311 480
    Opening database in default location
    Searching for info for network: 311480, spn: <None>
    Name: 4G LTE Contract (Private)
    APN: wyleslte.gw7.vzwentp
    Type: 1
    Proto: 2
    Tags: kore-m2m-private

As desired / expected.

Two specific tags:

    # ./lookup-apn -t "kore-m2m-private,kore-m2m-public" 311 480
    Opening database in default location
    Searching for info for network: 311480, spn: <None>
    Name: 4G LTE Contract (Public)
    APN: vzwinternet
    Type: 1
    Proto: 2
    Tags: kore-m2m-public

    Name: 4G LTE Contract (Private)
    APN: wyleslte.gw7.vzwentp
    Type: 1
    Proto: 2
    Tags: kore-m2m-private

As desired / expected.

No tags:

    # ./lookup-apn 311 480
    Opening database in default location
    Searching for info for network: 311480, spn: <None>
    Name: 4G LTE Contract
    APN: vzwims
    Type: 28
    Proto: 2
    Tags: (null)

    Name: 4G LTE Contract (Public)
    APN: vzwinternet
    Type: 1
    Proto: 2
    Tags: kore-m2m-public

    Name: 4G LTE Contract (Private)
    APN: wyleslte.gw7.vzwentp
    Type: 1
    Proto: 2
    Tags: kore-m2m-private

    Name: 4G LTE Contract
    APN: vzwapp
    Type: 4
    Proto: 2
    Tags: (null)

As desired / expected.

Grant Erickson (5):
  provisiondb: Const-qualify '__get_string'.
  provisiondb: Const-qualify '__get_contexts'.
  provisiondb: Correct the precondition return conditions of
    'tags_match'.
  provisiondb: Document 'tags_match'.
  provisiondb: Document '__get_contexts'.

 src/provisiondb.c | 64 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 3 deletions(-)

-- 
2.45.0

[PATCH 1/5] provisiondb: Const-qualify '__get_string'.

[Grant Erickson]

Const-qualify the pdb argument of '__get_string' to make it clear to
the compiler, static analyzers, and human readers that the function
is strictly a getter with no pdb argument mutation side effects.
---
 src/provisiondb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/provisiondb.c b/src/provisiondb.c
index 6a913d8d1540..8a71978b8afe 100644
--- a/src/provisiondb.c
+++ b/src/provisiondb.c
@@ -204,7 +204,7 @@ static struct provision_data *__get_provision_data(struct node *node)
 	return ((void *) node) + sizeof(struct node);
 }
 
-static int __get_string(struct provision_db *pdb, uint64_t offset,
+static int __get_string(struct provision_db const *pdb, uint64_t offset,
 				const char **out_str)
 {
 	if (!offset) {
-- 
2.45.0

[PATCH 2/5] provisiondb: Const-qualify '__get_contexts'.

[Grant Erickson]

Const-qualify the pdb argument of '__get_contexts' to make it clear to
the compiler, static analyzers, and human readers that the function
is strictly a getter with no pdb argument mutation side effects.
---
 src/provisiondb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/provisiondb.c b/src/provisiondb.c
index 8a71978b8afe..4a7f1bb6ef6e 100644
--- a/src/provisiondb.c
+++ b/src/provisiondb.c
@@ -236,7 +236,7 @@ static bool tags_match(char **tags_filter, const char *tags)
 	return false;
 }
 
-static int __get_contexts(struct provision_db *pdb, uint64_t offset,
+static int __get_contexts(struct provision_db const *pdb, uint64_t offset,
 				char **tags_filter,
 				struct provision_db_entry **contexts,
 				size_t *n_contexts)
-- 
2.45.0

[PATCH 3/5] provisiondb: Correct the precondition return conditions of 'tags_match'.

[Grant Erickson]

Prior to this change, specifying 'TagsFilter' in
${sysconfdir}/ofono/main.conf resulted in returning APNs which have no
tags plus APNs that match a tag in 'TagsFilter'. In addition to
seemingly violating the rule of least astonishment, this did not do
what was desired, which was to downselect more than one APN to one (or
more) matching a specific tag.

With this change, specifying 'TagsFilter' now follows the rule of
least astonishment and matches expectations, returning only those APNs
which match a tag in 'TagsFilter', period.

One specific tag:

    # ./lookup-apn -t "kore-m2m-public" 311 480
    Opening database in default location
    Searching for info for network: 311480, spn: <None>
    Name: 4G LTE Contract (Public)
    APN: vzwinternet
    Type: 1
    Proto: 2
    Tags: kore-m2m-public

As desired / expected.

Another specific tag:

    # ./lookup-apn -t "kore-m2m-private" 311 480
    Opening database in default location
    Searching for info for network: 311480, spn: <None>
    Name: 4G LTE Contract (Private)
    APN: wyleslte.gw7.vzwentp
    Type: 1
    Proto: 2
    Tags: kore-m2m-private

As desired / expected.

Two specific tags:

    # ./lookup-apn -t "kore-m2m-private,kore-m2m-public" 311 480
    Opening database in default location
    Searching for info for network: 311480, spn: <None>
    Name: 4G LTE Contract (Public)
    APN: vzwinternet
    Type: 1
    Proto: 2
    Tags: kore-m2m-public

    Name: 4G LTE Contract (Private)
    APN: wyleslte.gw7.vzwentp
    Type: 1
    Proto: 2
    Tags: kore-m2m-private

As desired / expected.

No tags:

    # ./lookup-apn 311 480
    Opening database in default location
    Searching for info for network: 311480, spn: <None>
    Name: 4G LTE Contract
    APN: vzwims
    Type: 28
    Proto: 2
    Tags: (null)

    Name: 4G LTE Contract (Public)
    APN: vzwinternet
    Type: 1
    Proto: 2
    Tags: kore-m2m-public

    Name: 4G LTE Contract (Private)
    APN: wyleslte.gw7.vzwentp
    Type: 1
    Proto: 2
    Tags: kore-m2m-private

    Name: 4G LTE Contract
    APN: vzwapp
    Type: 4
    Proto: 2
    Tags: (null)

As desired / expected.
---
 src/provisiondb.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/provisiondb.c b/src/provisiondb.c
index 4a7f1bb6ef6e..753d7725103a 100644
--- a/src/provisiondb.c
+++ b/src/provisiondb.c
@@ -224,9 +224,22 @@ static bool tags_match(char **tags_filter, const char *tags)
 	_auto_(l_strv_free) char **split_tags = 0;
 	unsigned int i;
 
-	if (!tags_filter || !tags)
+	/*
+	 * If tags_filter was not specified, then the caller does not want
+	 * to match on tags, so just return true.
+	 */
+	if (!tags_filter)
 		return true;
 
+	/*
+	 * If tags_filter was specified, then the caller does want to
+	 * match on tags. Consequently, if there are no tags to match
+	 * against, then we must return false since such an entry cannot
+	 * match.
+	 */
+	if (tags_filter && !tags)
+		return false;
+
 	split_tags = l_strsplit(tags, ',');
 
 	for (i = 0; tags_filter[i]; i++)
-- 
2.45.0

[PATCH 4/5] provisiondb: Document 'tags_match'.

[Grant Erickson]

Adds documentation to the 'tags_match' function.
---
 src/provisiondb.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/src/provisiondb.c b/src/provisiondb.c
index 753d7725103a..c7d35e4ada8f 100644
--- a/src/provisiondb.c
+++ b/src/provisiondb.c
@@ -219,6 +219,26 @@ static int __get_string(struct provision_db const *pdb, uint64_t offset,
 	return 0;
 }
 
+/**
+ *  @brief
+ *    Return whether one of the tags in the specified tags array
+ *    matches one of the tags in the specified comma-separated tags
+ *    list.
+ *
+ *  @param[in]  tags_filter  An optional pointer to a string array of
+ *                           tags to match.
+ *  @param[in]  tags         An optional pointer to an immutable C
+ *                           string containing a comma-separated
+ *                           list of tags for @a tags_filter to
+ *                           match against.
+ *
+ *  @returns
+ *    True if @a tags_filter is non-null and one of its tags matches a
+ *    tag in @a tags; otherwise, false.
+ *
+ *  @private
+ *
+ */
 static bool tags_match(char **tags_filter, const char *tags)
 {
 	_auto_(l_strv_free) char **split_tags = 0;
-- 
2.45.0

[PATCH 5/5] provisiondb: Document '__get_contexts'.

[Grant Erickson]

Adds documentation to the '__get_contexts' function.
---
 src/provisiondb.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/src/provisiondb.c b/src/provisiondb.c
index c7d35e4ada8f..2d7b29fdab45 100644
--- a/src/provisiondb.c
+++ b/src/provisiondb.c
@@ -269,6 +269,31 @@ static bool tags_match(char **tags_filter, const char *tags)
 	return false;
 }
 
+/**
+ *  @brief
+ *    Get contexts from the provisioning database at the specified
+ *    offset, potentially matching the optionally-specified tags.
+ *
+ *  @param[in]   pdb          A pointer to the immutable
+ *                            provisioning database from which to
+ *                            return contexts.
+ *  @param[in]   offset       The offset in @a pdb from which to
+ *                            return contexts.
+ *  @param[in]   tags_filter  An optional pointer to a string array
+ *                            of tags to match against candidate
+ *                            contexts.
+ *  @param[out]  contexts     A pointer to mutable storage for a
+ *                            an array of retrieved contexts.
+ *  @param[out]  n_contexts   A pointer to mutable storage for the
+ *                            number of contexts pointed to by @a
+ *                            contexts.
+ *
+ *  @retval  0        If successful.
+ *  @retval  -EPROTO  If @a offset results in an overrun of @a pdb.
+ *
+ *  @private
+ *
+ */
 static int __get_contexts(struct provision_db const *pdb, uint64_t offset,
 				char **tags_filter,
 				struct provision_db_entry **contexts,
-- 
2.45.0