You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by cc...@apache.org on 2016/12/13 03:06:27 UTC
[1/6] incubator-mynewt-core git commit: newtmgr transports - free
mbuf on failure.
Repository: incubator-mynewt-core
Updated Branches:
refs/heads/develop 52cfd9692 -> 33bc18ef5
newtmgr transports - free mbuf on failure.
The semantics of the various newtmgr transports was not consistent. The
UART transport's output function always consumed the supplied mbuf, whereas
the BLE and shell transport functions only consume the mbuf on success.
Now, all transport output functions always consume the supplied mbuf.
Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/5afe6f08
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/5afe6f08
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/5afe6f08
Branch: refs/heads/develop
Commit: 5afe6f08630ca4171b8956676707419a18c56ae1
Parents: d8bfe7f
Author: Christopher Collins <cc...@apache.org>
Authored: Mon Dec 12 15:15:13 2016 -0800
Committer: Christopher Collins <cc...@apache.org>
Committed: Mon Dec 12 18:54:25 2016 -0800
----------------------------------------------------------------------
mgmt/newtmgr/transport/ble/src/newtmgr_ble.c | 1 +
mgmt/newtmgr/transport/nmgr_shell/src/nmgr_shell.c | 1 +
2 files changed, 2 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/5afe6f08/mgmt/newtmgr/transport/ble/src/newtmgr_ble.c
----------------------------------------------------------------------
diff --git a/mgmt/newtmgr/transport/ble/src/newtmgr_ble.c b/mgmt/newtmgr/transport/ble/src/newtmgr_ble.c
index 72c8a24..395e866 100644
--- a/mgmt/newtmgr/transport/ble/src/newtmgr_ble.c
+++ b/mgmt/newtmgr/transport/ble/src/newtmgr_ble.c
@@ -197,6 +197,7 @@ nmgr_ble_out(struct nmgr_transport *nt, struct os_mbuf *om)
return (0);
err:
+ os_mbuf_free_chain(om);
return (rc);
}
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/5afe6f08/mgmt/newtmgr/transport/nmgr_shell/src/nmgr_shell.c
----------------------------------------------------------------------
diff --git a/mgmt/newtmgr/transport/nmgr_shell/src/nmgr_shell.c b/mgmt/newtmgr/transport/nmgr_shell/src/nmgr_shell.c
index 3ae170b..c57e9b1 100644
--- a/mgmt/newtmgr/transport/nmgr_shell/src/nmgr_shell.c
+++ b/mgmt/newtmgr/transport/nmgr_shell/src/nmgr_shell.c
@@ -44,6 +44,7 @@ nmgr_shell_out(struct nmgr_transport *nt, struct os_mbuf *m)
return (0);
err:
+ os_mbuf_free_chain(m);
return (rc);
}
[5/6] incubator-mynewt-core git commit: mbuf - Function to trim empty
buffers from front
Posted by cc...@apache.org.
mbuf - Function to trim empty buffers from front
/**
* Removes and frees empty mbufs from the front of a chain. If the
* chain contains a packet header, it is preserved.
*
* @param om The mbuf chain to trim.
*
* @return The head of the trimmed mbuf chain.
*/
Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/84456e5e
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/84456e5e
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/84456e5e
Branch: refs/heads/develop
Commit: 84456e5e041c3c54431163508a4b2a425aac16a1
Parents: 5afe6f0
Author: Christopher Collins <cc...@apache.org>
Authored: Mon Dec 12 19:00:13 2016 -0800
Committer: Christopher Collins <cc...@apache.org>
Committed: Mon Dec 12 19:00:13 2016 -0800
----------------------------------------------------------------------
kernel/os/include/os/os_mbuf.h | 1 +
kernel/os/src/os_mbuf.c | 53 +++++++++++++++++++++++++++++++++++++
2 files changed, 54 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/84456e5e/kernel/os/include/os/os_mbuf.h
----------------------------------------------------------------------
diff --git a/kernel/os/include/os/os_mbuf.h b/kernel/os/include/os/os_mbuf.h
index 495f4b4..4709caf 100644
--- a/kernel/os/include/os/os_mbuf.h
+++ b/kernel/os/include/os/os_mbuf.h
@@ -297,6 +297,7 @@ int os_mbuf_copyinto(struct os_mbuf *om, int off, const void *src, int len);
void os_mbuf_concat(struct os_mbuf *first, struct os_mbuf *second);
void *os_mbuf_extend(struct os_mbuf *om, uint16_t len);
struct os_mbuf *os_mbuf_pullup(struct os_mbuf *om, uint16_t len);
+struct os_mbuf *os_mbuf_trim_front(struct os_mbuf *om);
#ifdef __cplusplus
}
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/84456e5e/kernel/os/src/os_mbuf.c
----------------------------------------------------------------------
diff --git a/kernel/os/src/os_mbuf.c b/kernel/os/src/os_mbuf.c
index 9f6e0b1..99775e4 100644
--- a/kernel/os/src/os_mbuf.c
+++ b/kernel/os/src/os_mbuf.c
@@ -1278,6 +1278,59 @@ bad:
}
/**
+ * Removes and frees empty mbufs from the front of a chain. If the chain
+ * contains a packet header, it is preserved.
+ *
+ * @param om The mbuf chain to trim.
+ *
+ * @return The head of the trimmed mbuf chain.
+ */
+struct os_mbuf *
+os_mbuf_trim_front(struct os_mbuf *om)
+{
+ struct os_mbuf *next;
+ struct os_mbuf *cur;
+
+ /* Abort early if there is nothing to trim. */
+ if (om->om_len != 0) {
+ return om;
+ }
+
+ /* Starting with the second mbuf in the chain, continue removing and
+ * freeing mbufs until an non-empty one is encountered.
+ */
+ cur = SLIST_NEXT(om, om_next);
+ while (cur != NULL && cur->om_len == 0) {
+ next = SLIST_NEXT(cur, om_next);
+
+ SLIST_NEXT(om, om_next) = next;
+ os_mbuf_free(cur);
+
+ cur = next;
+ }
+
+ if (cur == NULL) {
+ /* All buffers after the first have been freed. */
+ return om;
+ }
+
+ /* Try to remove the first mbuf in the chain. If this buffer contains a
+ * packet header, make sure the second buffer can accommodate it.
+ */
+ if (OS_MBUF_LEADINGSPACE(cur) >= om->om_pkthdr_len) {
+ /* Second buffer has room; copy packet header. */
+ cur->om_pkthdr_len = om->om_pkthdr_len;
+ memcpy(OS_MBUF_PKTHDR(cur), OS_MBUF_PKTHDR(om), om->om_pkthdr_len);
+
+ /* Free first buffer. */
+ os_mbuf_free(om);
+ om = cur;
+ }
+
+ return om;
+}
+
+/**
* @} OSMbuf
* @} OSKernel
*/
[2/6] incubator-mynewt-core git commit: newtmgr - Fix unlikely mbuf
leak.
Posted by cc...@apache.org.
newtmgr - Fix unlikely mbuf leak.
Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/a7d10ec6
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/a7d10ec6
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/a7d10ec6
Branch: refs/heads/develop
Commit: a7d10ec666ba5b2f4890673c8b64950dc9091187
Parents: 52cfd96
Author: Christopher Collins <cc...@apache.org>
Authored: Mon Dec 12 15:06:10 2016 -0800
Committer: Christopher Collins <cc...@apache.org>
Committed: Mon Dec 12 18:54:25 2016 -0800
----------------------------------------------------------------------
mgmt/newtmgr/src/newtmgr.c | 1 +
1 file changed, 1 insertion(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/a7d10ec6/mgmt/newtmgr/src/newtmgr.c
----------------------------------------------------------------------
diff --git a/mgmt/newtmgr/src/newtmgr.c b/mgmt/newtmgr/src/newtmgr.c
index b57d0a3..b3c4289 100644
--- a/mgmt/newtmgr/src/newtmgr.c
+++ b/mgmt/newtmgr/src/newtmgr.c
@@ -98,6 +98,7 @@ nmgr_send_err_rsp(struct nmgr_transport *nt, struct os_mbuf *m,
{
hdr = nmgr_init_rsp(m, hdr);
if (!hdr) {
+ os_mbuf_free_chain(m);
return;
}
[4/6] incubator-mynewt-core git commit: newtmgr - Add some comments.
Posted by cc...@apache.org.
newtmgr - Add some comments.
Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/9c977ef8
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/9c977ef8
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/9c977ef8
Branch: refs/heads/develop
Commit: 9c977ef871ae55e2ce45fd700fa270b5e914b2e3
Parents: a7d10ec
Author: Christopher Collins <cc...@apache.org>
Authored: Mon Dec 12 15:06:36 2016 -0800
Committer: Christopher Collins <cc...@apache.org>
Committed: Mon Dec 12 18:54:25 2016 -0800
----------------------------------------------------------------------
mgmt/newtmgr/include/newtmgr/newtmgr.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/9c977ef8/mgmt/newtmgr/include/newtmgr/newtmgr.h
----------------------------------------------------------------------
diff --git a/mgmt/newtmgr/include/newtmgr/newtmgr.h b/mgmt/newtmgr/include/newtmgr/newtmgr.h
index 3c3081a..69a2d6b 100644
--- a/mgmt/newtmgr/include/newtmgr/newtmgr.h
+++ b/mgmt/newtmgr/include/newtmgr/newtmgr.h
@@ -43,8 +43,20 @@ struct nmgr_hdr {
};
struct nmgr_transport;
+
+/**
+ * Transmit function. The supplied mbuf is always consumed, regardless of
+ * return code.
+ */
typedef int (*nmgr_transport_out_func_t)(struct nmgr_transport *nt,
struct os_mbuf *m);
+
+/**
+ * MTU query function. The supplied mbuf should contain a request received
+ * from the peer whose MTU is being queried. This function takes an mbuf
+ * parameter because some transports store connection-specific information in
+ * the mbuf user header (e.g., the BLE transport stores the connection handle).
+ */
typedef uint16_t (*nmgr_transport_get_mtu_func_t)(struct os_mbuf *m);
struct nmgr_transport {
[6/6] incubator-mynewt-core git commit: newtmgr - Free mbufs as
fragments are txed.
Posted by cc...@apache.org.
newtmgr - Free mbufs as fragments are txed.
Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/33bc18ef
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/33bc18ef
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/33bc18ef
Branch: refs/heads/develop
Commit: 33bc18ef543538c7e9eb506a4aea8ad97e466da2
Parents: 84456e5
Author: Christopher Collins <cc...@apache.org>
Authored: Mon Dec 12 19:00:59 2016 -0800
Committer: Christopher Collins <cc...@apache.org>
Committed: Mon Dec 12 19:00:59 2016 -0800
----------------------------------------------------------------------
mgmt/newtmgr/src/newtmgr.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/33bc18ef/mgmt/newtmgr/src/newtmgr.c
----------------------------------------------------------------------
diff --git a/mgmt/newtmgr/src/newtmgr.c b/mgmt/newtmgr/src/newtmgr.c
index 2a2c3cb..ab62f0a 100644
--- a/mgmt/newtmgr/src/newtmgr.c
+++ b/mgmt/newtmgr/src/newtmgr.c
@@ -161,10 +161,7 @@ nmgr_rsp_split_frag(struct os_mbuf **om, uint16_t mtu,
goto err;
}
os_mbuf_adj(*om, mtu);
-
- /* XXX: We should try to free buffers from the response chain if
- * possible.
- */
+ *om = os_mbuf_trim_front(*om);
/* More fragments to follow. */
*out_frag = frag;
[3/6] incubator-mynewt-core git commit: newtmgr - Don't allocate mbuf
for final rsp frag.
Posted by cc...@apache.org.
newtmgr - Don't allocate mbuf for final rsp frag.
Previously, newtmgr would allocate an additional mbuf for each outgoing
fragment.
Now, newtmgr allocates an mbuf for each fragment except the last. For
the final fragment, the response mbuf itself gets passed to the
transport output function.
Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/d8bfe7fe
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/d8bfe7fe
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/d8bfe7fe
Branch: refs/heads/develop
Commit: d8bfe7fe0531c4dde70726a2600d02b592806f43
Parents: 9c977ef
Author: Christopher Collins <cc...@apache.org>
Authored: Mon Dec 12 15:13:16 2016 -0800
Committer: Christopher Collins <cc...@apache.org>
Committed: Mon Dec 12 18:54:25 2016 -0800
----------------------------------------------------------------------
mgmt/newtmgr/src/newtmgr.c | 119 +++++++++++++++++++++++++---------------
1 file changed, 76 insertions(+), 43 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/d8bfe7fe/mgmt/newtmgr/src/newtmgr.c
----------------------------------------------------------------------
diff --git a/mgmt/newtmgr/src/newtmgr.c b/mgmt/newtmgr/src/newtmgr.c
index b3c4289..2a2c3cb 100644
--- a/mgmt/newtmgr/src/newtmgr.c
+++ b/mgmt/newtmgr/src/newtmgr.c
@@ -111,73 +111,95 @@ nmgr_send_err_rsp(struct nmgr_transport *nt, struct os_mbuf *m,
nt->nt_output(nt, nmgr_task_cbuf.n_out_m);
}
+/**
+ * Splits an appropriately-sized fragment off from the front of an outgoing
+ * newtmgr response packet, if necessary. If the packet size is within the
+ * transport MTU, no splitting is performed. The fragment data is removed from
+ * the data packet mbuf.
+ *
+ * @param om The newtmgr response packet. If this
+ * constitutes a single fragment, it gets set
+ * to NULL on success.
+ * @param out_frag On success, this points to the fragment to
+ * send. If the entire packet can fit within
+ * a single fragment, this will point to the
+ * newtmgr response packet itself ('om').
+ *
+ * @return 0 on success;
+ * BLE host core return code on error.
+ */
static int
-nmgr_send_rspfrag(struct nmgr_transport *nt, struct os_mbuf *rsp,
- struct os_mbuf *req, uint16_t len)
+nmgr_rsp_split_frag(struct os_mbuf **om, uint16_t mtu,
+ struct os_mbuf **out_frag)
{
- struct os_mbuf *rspfrag;
+ struct os_mbuf *frag;
int rc;
- rspfrag = NULL;
+ /* Assume failure. */
+ *out_frag = NULL;
+
+ if (OS_MBUF_PKTLEN(*om) <= mtu) {
+ /* Final fragment. */
+ *out_frag = *om;
+ *om = NULL;
+ return 0;
+ }
- rspfrag = os_msys_get_pkthdr(len, OS_MBUF_USRHDR_LEN(req));
- if (!rspfrag) {
+ frag = os_msys_get_pkthdr(mtu, OS_MBUF_USRHDR_LEN(*om));
+ if (frag == NULL) {
rc = MGMT_ERR_ENOMEM;
goto err;
}
- /* Copy the request packet header into the response. */
- memcpy(OS_MBUF_USRHDR(rspfrag), OS_MBUF_USRHDR(req),
- OS_MBUF_USRHDR_LEN(req));
+ /* Copy the user header from the response into the fragment mbuf. */
+ memcpy(OS_MBUF_USRHDR(frag), OS_MBUF_USRHDR(*om), OS_MBUF_USRHDR_LEN(*om));
- if (os_mbuf_appendfrom(rspfrag, rsp, 0, len)) {
+ /* Move data from the front of the packet into the fragment mbuf. */
+ rc = os_mbuf_appendfrom(frag, *om, 0, mtu);
+ if (rc != 0) {
rc = MGMT_ERR_ENOMEM;
goto err;
}
+ os_mbuf_adj(*om, mtu);
- nt->nt_output(nt, rspfrag);
+ /* XXX: We should try to free buffers from the response chain if
+ * possible.
+ */
+
+ /* More fragments to follow. */
+ *out_frag = frag;
+ return 0;
- return MGMT_ERR_EOK;
err:
- if (rspfrag) {
- os_mbuf_free_chain(rspfrag);
- }
+ os_mbuf_free_chain(frag);
return rc;
}
+/**
+ * Sends a newtmgr response, fragmenting it as needed. The supplied response
+ * mbuf is consumed on success and in some failure cases. If the mbuf is
+ * consumed, the supplied pointer is set to NULL.
+ */
static int
-nmgr_rsp_fragment(struct nmgr_transport *nt, struct nmgr_hdr *rsp_hdr,
- struct os_mbuf *rsp, struct os_mbuf *req)
+nmgr_rsp_tx(struct nmgr_transport *nt, struct os_mbuf **rsp, uint16_t mtu)
{
- uint16_t mtu;
- uint16_t rsplen;
- uint16_t len ;
+ struct os_mbuf *frag;
int rc;
- len = 0;
-
- mtu = nt->nt_get_mtu(req);
-
- do {
- len = OS_MBUF_PKTLEN(rsp);
- if (len >= mtu) {
- rsplen = mtu;
- } else {
- rsplen = len;
+ while (rsp != NULL) {
+ rc = nmgr_rsp_split_frag(rsp, mtu, &frag);
+ if (rc != 0) {
+ return rc;
}
- rc = nmgr_send_rspfrag(nt, rsp, req, rsplen);
- if (rc) {
- goto err;
+ rc = nt->nt_output(nt, frag);
+ if (rc != 0) {
+ /* Output function already freed mbuf. */
+ return MGMT_ERR_EUNKNOWN;
}
-
- os_mbuf_adj(rsp, rsplen);
-
- } while (OS_MBUF_PKTLEN(rsp));
+ }
return MGMT_ERR_EOK;
-err:
- return rc;
}
static void
@@ -189,6 +211,7 @@ nmgr_handle_req(struct nmgr_transport *nt, struct os_mbuf *req)
struct nmgr_hdr hdr;
int off;
uint16_t len;
+ uint16_t mtu;
int rc;
rsp_hdr = NULL;
@@ -204,6 +227,8 @@ nmgr_handle_req(struct nmgr_transport *nt, struct os_mbuf *req)
goto err;
}
+ mtu = nt->nt_get_mtu(req);
+
/* Copy the request packet header into the response. */
memcpy(OS_MBUF_USRHDR(rsp), OS_MBUF_USRHDR(req), OS_MBUF_USRHDR_LEN(req));
@@ -260,13 +285,21 @@ nmgr_handle_req(struct nmgr_transport *nt, struct os_mbuf *req)
rsp_hdr->nh_len +=
cbor_encode_bytes_written(&nmgr_task_cbuf.n_b.encoder);
- off += sizeof(hdr) + OS_ALIGN(hdr.nh_len, 4);
-
rsp_hdr->nh_len = htons(rsp_hdr->nh_len);
- rc = nmgr_rsp_fragment(nt, rsp_hdr, rsp, req);
+
+ rc = nmgr_rsp_tx(nt, &rsp, mtu);
if (rc) {
- goto err;
+ /* If the entire mbuf was consumed by the transport, don't attempt
+ * to send an error response.
+ */
+ if (rsp == NULL) {
+ goto err_norsp;
+ } else {
+ goto err;
+ }
}
+
+ off += sizeof(hdr) + OS_ALIGN(hdr.nh_len, 4);
}
os_mbuf_free_chain(rsp);