<p>Harald Welte has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/13606">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">ggsn.c: Refactor PCO processing during PDP activation<br><br>The existing PCO processing is implemented in a rater convoluted<br>way.  We scan the list of PCO elements several times for different<br>PCO protocols.  Let's change to a straight-forward model where we<br>simply do one iteration over the list of PCO elements and generate<br>responses step by step.<br><br>Change-Id: I4a7d09279b6b259e2b95f1f51159b16838b2d94c<br>---<br>M ggsn/ggsn.c<br>1 file changed, 96 insertions(+), 85 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-ggsn refs/changes/06/13606/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/ggsn/ggsn.c b/ggsn/ggsn.c</span><br><span>index eac7e16..c460b98 100644</span><br><span>--- a/ggsn/ggsn.c</span><br><span>+++ b/ggsn/ggsn.c</span><br><span>@@ -470,22 +470,6 @@</span><br><span>         uint8_t data[0];</span><br><span> } __attribute__((packed));</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* determine if PCO contains given protocol */</span><br><span style="color: hsl(0, 100%, 40%);">-static const struct pco_element *pco_contains_proto(const struct ul255_t *pco, size_t offset,</span><br><span style="color: hsl(0, 100%, 40%);">-                                               uint16_t prot, size_t prot_minlen)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- const uint8_t *cur = pco->v + 1 /*length*/ + offset;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* iterate over PCO and check if protocol contained */</span><br><span style="color: hsl(0, 100%, 40%);">-  while (cur + sizeof(struct pco_element) <= pco->v + pco->l) {</span><br><span style="color: hsl(0, 100%, 40%);">-          const struct pco_element *elem = (const struct pco_element *)cur;</span><br><span style="color: hsl(0, 100%, 40%);">-               if (ntohs(elem->protocol_id) == prot && elem->length >= prot_minlen)</span><br><span style="color: hsl(0, 100%, 40%);">-                   return elem;</span><br><span style="color: hsl(0, 100%, 40%);">-            cur += elem->length + sizeof(struct pco_element);</span><br><span style="color: hsl(0, 100%, 40%);">-    }</span><br><span style="color: hsl(0, 100%, 40%);">-       return NULL;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> /*! Get the peer of pdp based on IP version used.</span><br><span>  *  \param[in] pdp PDP context to select the peer from.</span><br><span>  *  \param[in] v4v6 IP version to select. Valid values are 4 and 6.</span><br><span>@@ -510,103 +494,130 @@</span><br><span>       return NULL;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* construct an IPCP PCO response from request*/</span><br><span style="color: hsl(0, 100%, 40%);">-static void build_ipcp_pco(const struct apn_ctx *apn, struct pdp_t *pdp, struct msgb *msg)</span><br><span style="color: hsl(120, 100%, 40%);">+static void process_pco_element_ipcp(const struct pco_element *pco_elem, struct msgb *resp,</span><br><span style="color: hsl(120, 100%, 40%);">+                               const struct apn_ctx *apn, struct pdp_t *pdp)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+       struct ippoolm_t *peer_v4 = pdp_get_peer_ipv(pdp, false);</span><br><span>    const struct in46_addr *dns1 = &apn->v4.cfg.dns[0];</span><br><span>   const struct in46_addr *dns2 = &apn->v4.cfg.dns[1];</span><br><span style="color: hsl(0, 100%, 40%);">-      const struct pco_element *pco_ipcp;</span><br><span style="color: hsl(120, 100%, 40%);">+   uint8_t *start = resp->tail;</span><br><span>      const uint8_t *ipcp;</span><br><span>         uint16_t ipcp_len;</span><br><span>   uint8_t *len1, *len2;</span><br><span>        unsigned int len_appended;</span><br><span>   ptrdiff_t consumed;</span><br><span style="color: hsl(0, 100%, 40%);">-     size_t remain, offset = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+    size_t remain;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-      /* pco_contains_proto() returns a potentially unaligned pointer into pco_req->v (see OS#3194) */</span><br><span style="color: hsl(0, 100%, 40%);">-     pco_ipcp = pco_contains_proto(&pdp->pco_req, offset, PCO_P_IPCP, sizeof(struct ipcp_hdr));</span><br><span style="color: hsl(0, 100%, 40%);">-       while (pco_ipcp) {</span><br><span style="color: hsl(0, 100%, 40%);">-              uint8_t *start = msg->tail;</span><br><span style="color: hsl(120, 100%, 40%);">+        if (!peer_v4)</span><br><span style="color: hsl(120, 100%, 40%);">+         return;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-             ipcp = pco_ipcp->data;</span><br><span style="color: hsl(0, 100%, 40%);">-               consumed = (ipcp - &pdp->pco_req.v[0]);</span><br><span style="color: hsl(0, 100%, 40%);">-          remain = sizeof(pdp->pco_req.v) - consumed;</span><br><span style="color: hsl(0, 100%, 40%);">-          ipcp_len = osmo_load16be(ipcp + 2); /* 1=code + 1=id */</span><br><span style="color: hsl(0, 100%, 40%);">-         if (remain < 0 || remain < ipcp_len)</span><br><span style="color: hsl(0, 100%, 40%);">-                      return;</span><br><span style="color: hsl(120, 100%, 40%);">+       ipcp = pco_elem->data;</span><br><span style="color: hsl(120, 100%, 40%);">+     consumed = (ipcp - &pdp->pco_req.v[0]);</span><br><span style="color: hsl(120, 100%, 40%);">+        remain = sizeof(pdp->pco_req.v) - consumed;</span><br><span style="color: hsl(120, 100%, 40%);">+        ipcp_len = osmo_load16be(ipcp + 2); /* 1=code + 1=id */</span><br><span style="color: hsl(120, 100%, 40%);">+       if (remain < 0 || remain < ipcp_len)</span><br><span style="color: hsl(120, 100%, 40%);">+            return;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-             /* Three byte T16L header */</span><br><span style="color: hsl(0, 100%, 40%);">-            msgb_put_u16(msg, 0x8021);      /* IPCP */</span><br><span style="color: hsl(0, 100%, 40%);">-              len1 = msgb_put(msg, 1);        /* Length of contents: delay */</span><br><span style="color: hsl(120, 100%, 40%);">+       /* Three byte T16L header */</span><br><span style="color: hsl(120, 100%, 40%);">+  msgb_put_u16(resp, 0x8021);     /* IPCP */</span><br><span style="color: hsl(120, 100%, 40%);">+    len1 = msgb_put(resp, 1);       /* Length of contents: delay */</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-             msgb_put_u8(msg, 0x02);         /* ACK */</span><br><span style="color: hsl(0, 100%, 40%);">-               msgb_put_u8(msg, ipcp[1]);      /* ID: Needs to match request */</span><br><span style="color: hsl(0, 100%, 40%);">-                msgb_put_u8(msg, 0x00);         /* Length MSB */</span><br><span style="color: hsl(0, 100%, 40%);">-                len2 = msgb_put(msg, 1);        /* Length LSB: delay */</span><br><span style="color: hsl(120, 100%, 40%);">+       msgb_put_u8(resp, 0x02);        /* ACK */</span><br><span style="color: hsl(120, 100%, 40%);">+     msgb_put_u8(resp, ipcp[1]);     /* ID: Needs to match request */</span><br><span style="color: hsl(120, 100%, 40%);">+      msgb_put_u8(resp, 0x00);        /* Length MSB */</span><br><span style="color: hsl(120, 100%, 40%);">+      len2 = msgb_put(resp, 1);       /* Length LSB: delay */</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-             if (dns1->len == 4 && ipcp_contains_option(ipcp, ipcp_len, IPCP_OPT_PRIMARY_DNS, 4)) {</span><br><span style="color: hsl(0, 100%, 40%);">-                       msgb_put_u8(msg, 0x81);         /* DNS1 Tag */</span><br><span style="color: hsl(0, 100%, 40%);">-                  msgb_put_u8(msg, 2 + dns1->len);/* DNS1 Length, incl. TL */</span><br><span style="color: hsl(0, 100%, 40%);">-                  msgb_put_u32(msg, ntohl(dns1->v4.s_addr));</span><br><span style="color: hsl(0, 100%, 40%);">-           }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-               if (dns2->len == 4 && ipcp_contains_option(ipcp, ipcp_len, IPCP_OPT_SECONDARY_DNS, 4)) {</span><br><span style="color: hsl(0, 100%, 40%);">-                     msgb_put_u8(msg, 0x83);         /* DNS2 Tag */</span><br><span style="color: hsl(0, 100%, 40%);">-                  msgb_put_u8(msg, 2 + dns2->len);/* DNS2 Length, incl. TL */</span><br><span style="color: hsl(0, 100%, 40%);">-                  msgb_put_u32(msg, ntohl(dns2->v4.s_addr));</span><br><span style="color: hsl(0, 100%, 40%);">-           }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-               /* patch in length values */</span><br><span style="color: hsl(0, 100%, 40%);">-            len_appended = msg->tail - start;</span><br><span style="color: hsl(0, 100%, 40%);">-            *len1 = len_appended - 3;</span><br><span style="color: hsl(0, 100%, 40%);">-               *len2 = len_appended - 3;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-               offset += sizeof(pco_ipcp) + pco_ipcp->length;</span><br><span style="color: hsl(0, 100%, 40%);">-               pco_ipcp = pco_contains_proto(&pdp->pco_req, offset, PCO_P_IPCP, sizeof(struct ipcp_hdr));</span><br><span style="color: hsl(120, 100%, 40%);">+     if (dns1->len == 4 && ipcp_contains_option(ipcp, ipcp_len, IPCP_OPT_PRIMARY_DNS, 4)) {</span><br><span style="color: hsl(120, 100%, 40%);">+             msgb_put_u8(resp, 0x81);                /* DNS1 Tag */</span><br><span style="color: hsl(120, 100%, 40%);">+                msgb_put_u8(resp, 2 + dns1->len);    /* DNS1 Length, incl. TL */</span><br><span style="color: hsl(120, 100%, 40%);">+           msgb_put_u32(resp, ntohl(dns1->v4.s_addr));</span><br><span>       }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (dns2->len == 4 && ipcp_contains_option(ipcp, ipcp_len, IPCP_OPT_SECONDARY_DNS, 4)) {</span><br><span style="color: hsl(120, 100%, 40%);">+           msgb_put_u8(resp, 0x83);                /* DNS2 Tag */</span><br><span style="color: hsl(120, 100%, 40%);">+                msgb_put_u8(resp, 2 + dns2->len);    /* DNS2 Length, incl. TL */</span><br><span style="color: hsl(120, 100%, 40%);">+           msgb_put_u32(resp, ntohl(dns2->v4.s_addr));</span><br><span style="color: hsl(120, 100%, 40%);">+        }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   /* patch in length values */</span><br><span style="color: hsl(120, 100%, 40%);">+  len_appended = resp->tail - start;</span><br><span style="color: hsl(120, 100%, 40%);">+ *len1 = len_appended - 3;</span><br><span style="color: hsl(120, 100%, 40%);">+     *len2 = len_appended - 3;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+static void process_pco_element_dns_ipv6(const struct pco_element *pco_elem, struct msgb *resp,</span><br><span style="color: hsl(120, 100%, 40%);">+                                        const struct apn_ctx *apn, struct pdp_t *pdp)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+     unsigned int i;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+     for (i = 0; i < ARRAY_SIZE(apn->v6.cfg.dns); i++) {</span><br><span style="color: hsl(120, 100%, 40%);">+             const struct in46_addr *i46a = &apn->v6.cfg.dns[i];</span><br><span style="color: hsl(120, 100%, 40%);">+            if (i46a->len != 16)</span><br><span style="color: hsl(120, 100%, 40%);">+                       continue;</span><br><span style="color: hsl(120, 100%, 40%);">+             msgb_t16lv_put(resp, PCO_P_DNS_IPv6_ADDR, i46a->len, i46a->v6.s6_addr);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+static void process_pco_element_dns_ipv4(const struct pco_element *pco_elem, struct msgb *resp,</span><br><span style="color: hsl(120, 100%, 40%);">+                                        const struct apn_ctx *apn, struct pdp_t *pdp)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+     unsigned int i;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+     for (i = 0; i < ARRAY_SIZE(apn->v4.cfg.dns); i++) {</span><br><span style="color: hsl(120, 100%, 40%);">+             const struct in46_addr *i46a = &apn->v4.cfg.dns[i];</span><br><span style="color: hsl(120, 100%, 40%);">+            if (i46a->len != 4)</span><br><span style="color: hsl(120, 100%, 40%);">+                        continue;</span><br><span style="color: hsl(120, 100%, 40%);">+             msgb_t16lv_put(resp, PCO_P_DNS_IPv4_ADDR, i46a->len, (uint8_t *)&i46a->v4);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+static void process_pco_element(const struct pco_element *pco_elem, struct msgb *resp,</span><br><span style="color: hsl(120, 100%, 40%);">+                                const struct apn_ctx *apn, struct pdp_t *pdp)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+      switch (ntohs(pco_elem->protocol_id)) {</span><br><span style="color: hsl(120, 100%, 40%);">+    case PCO_P_IPCP:</span><br><span style="color: hsl(120, 100%, 40%);">+              process_pco_element_ipcp(pco_elem, resp, apn, pdp);</span><br><span style="color: hsl(120, 100%, 40%);">+           break;</span><br><span style="color: hsl(120, 100%, 40%);">+        case PCO_P_DNS_IPv6_ADDR:</span><br><span style="color: hsl(120, 100%, 40%);">+             process_pco_element_dns_ipv6(pco_elem, resp, apn, pdp);</span><br><span style="color: hsl(120, 100%, 40%);">+               break;</span><br><span style="color: hsl(120, 100%, 40%);">+        case PCO_P_DNS_IPv4_ADDR:</span><br><span style="color: hsl(120, 100%, 40%);">+             process_pco_element_dns_ipv4(pco_elem, resp, apn, pdp);</span><br><span style="color: hsl(120, 100%, 40%);">+               break;</span><br><span style="color: hsl(120, 100%, 40%);">+        default:</span><br><span style="color: hsl(120, 100%, 40%);">+              break;</span><br><span style="color: hsl(120, 100%, 40%);">+        }</span><br><span> }</span><br><span> </span><br><span> /* process one PCO request from a MS/UE, putting together the proper responses */</span><br><span> static void process_pco(const struct apn_ctx *apn, struct pdp_t *pdp)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-   struct msgb *msg = msgb_alloc(256, "PCO");</span><br><span style="color: hsl(0, 100%, 40%);">-    struct ippoolm_t *peer_v4 = pdp_get_peer_ipv(pdp, false);</span><br><span style="color: hsl(0, 100%, 40%);">-       unsigned int i;</span><br><span style="color: hsl(120, 100%, 40%);">+       struct msgb *resp = msgb_alloc(256, "PCO.resp");</span><br><span style="color: hsl(120, 100%, 40%);">+    const struct ul255_t *pco = &pdp->pco_req;</span><br><span style="color: hsl(120, 100%, 40%);">+     const struct pco_element *pco_elem;</span><br><span style="color: hsl(120, 100%, 40%);">+   const uint8_t *cur;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- OSMO_ASSERT(msg);</span><br><span style="color: hsl(0, 100%, 40%);">-       msgb_put_u8(msg, 0x80); /* ext-bit + configuration protocol byte */</span><br><span style="color: hsl(120, 100%, 40%);">+   /* build the header of the PCO response */</span><br><span style="color: hsl(120, 100%, 40%);">+    OSMO_ASSERT(resp);</span><br><span style="color: hsl(120, 100%, 40%);">+    msgb_put_u8(resp, 0x80); /* ext-bit + configuration protocol byte */</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-        if (peer_v4)</span><br><span style="color: hsl(0, 100%, 40%);">-            build_ipcp_pco(apn, pdp, msg);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-  if (pco_contains_proto(&pdp->pco_req, 0, PCO_P_DNS_IPv6_ADDR, 0)) {</span><br><span style="color: hsl(0, 100%, 40%);">-              for (i = 0; i < ARRAY_SIZE(apn->v6.cfg.dns); i++) {</span><br><span style="color: hsl(0, 100%, 40%);">-                       const struct in46_addr *i46a = &apn->v6.cfg.dns[i];</span><br><span style="color: hsl(0, 100%, 40%);">-                      if (i46a->len != 16)</span><br><span style="color: hsl(0, 100%, 40%);">-                         continue;</span><br><span style="color: hsl(0, 100%, 40%);">-                       msgb_t16lv_put(msg, PCO_P_DNS_IPv6_ADDR, i46a->len, i46a->v6.s6_addr);</span><br><span style="color: hsl(0, 100%, 40%);">-            }</span><br><span style="color: hsl(120, 100%, 40%);">+     /* iterate over the PCO elements in the request; call process_pco_element() for each */</span><br><span style="color: hsl(120, 100%, 40%);">+       for (cur = pco->v + 1, pco_elem = (const struct pco_element *) cur;</span><br><span style="color: hsl(120, 100%, 40%);">+             cur + sizeof(struct pco_element) <= pco->v + pco->l;</span><br><span style="color: hsl(120, 100%, 40%);">+         cur += pco_elem->length + sizeof(*pco_elem), pco_elem = (const struct pco_element *) cur) {</span><br><span style="color: hsl(120, 100%, 40%);">+           process_pco_element(pco_elem, resp, apn, pdp);</span><br><span>       }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   if (pco_contains_proto(&pdp->pco_req, 0, PCO_P_DNS_IPv4_ADDR, 0)) {</span><br><span style="color: hsl(0, 100%, 40%);">-              for (i = 0; i < ARRAY_SIZE(apn->v4.cfg.dns); i++) {</span><br><span style="color: hsl(0, 100%, 40%);">-                       const struct in46_addr *i46a = &apn->v4.cfg.dns[i];</span><br><span style="color: hsl(0, 100%, 40%);">-                      if (i46a->len != 4)</span><br><span style="color: hsl(0, 100%, 40%);">-                          continue;</span><br><span style="color: hsl(0, 100%, 40%);">-                       msgb_t16lv_put(msg, PCO_P_DNS_IPv4_ADDR, i46a->len, (uint8_t *)&i46a->v4);</span><br><span style="color: hsl(0, 100%, 40%);">-            }</span><br><span style="color: hsl(0, 100%, 40%);">-       }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-       if (msgb_length(msg) > 1) {</span><br><span style="color: hsl(0, 100%, 40%);">-          memcpy(pdp->pco_neg.v, msgb_data(msg), msgb_length(msg));</span><br><span style="color: hsl(0, 100%, 40%);">-            pdp->pco_neg.l = msgb_length(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* copy the PCO response msgb and copy its contents over to the PDP context */</span><br><span style="color: hsl(120, 100%, 40%);">+        if (msgb_length(resp) > 1) {</span><br><span style="color: hsl(120, 100%, 40%);">+               memcpy(pdp->pco_neg.v, msgb_data(resp), msgb_length(resp));</span><br><span style="color: hsl(120, 100%, 40%);">+                pdp->pco_neg.l = msgb_length(resp);</span><br><span>       } else</span><br><span>               pdp->pco_neg.l = 0;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-  msgb_free(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+       msgb_free(resp);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static bool apn_supports_ipv4(const struct apn_ctx *apn)</span><br><span> {</span><br><span>         if (apn->v4.cfg.static_prefix.addr.len  || apn->v4.cfg.dynamic_prefix.addr.len)</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/13606">change 13606</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/13606"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-ggsn </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I4a7d09279b6b259e2b95f1f51159b16838b2d94c </div>
<div style="display:none"> Gerrit-Change-Number: 13606 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Harald Welte <laforge@gnumonks.org> </div>