You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by ma...@apache.org on 2017/03/06 21:18:41 UTC
[05/50] incubator-mynewt-core git commit: MYNEWT-647 Changes to NMP
over OIC scheme
MYNEWT-647 Changes to NMP over OIC scheme
This commit addresses the following three issues with NMP over OIC (OMP):
1. Parts of NMP header missing from OMP responses. This causes problems
when multiplexing OMP among many target devices.
2. Parts of the NMP header are scattered in a few different places.
This works, but it makes it difficult to debug packet traces.
3. NMP errors get lost in translation. Instead of an NMP message with
an error code, the OMP server sends back a generic "Bad Request" OIC
message.
***** Current Scheme
*** Requests
The NMP op is indicated by the OIC op:
NMP read <=> OIC get
NMP write <=> OIC put
The NMP group and ID are indicated as CoAP URI Query options:
gr=<group>
id=<id>
The remaining NMP header fields, seq and flags, are not present.
The NMP payload is the entire CoAP request body. This is identical to the body
of a plain NMP request.
*** Responses
The NMP header is not present. The NMP op is indicated in the payload (see
below), but other header fields cannot be inferred.
Payload consists of a single CBOR key-value pair. For read responses, the key
is "r"; for write responses, the key is "w". The value is a second CBOR map
containing the actual NMP response fields.
Errors encountered during processing of NMP requests are reported via OIC error
responses (bad request, internal server error).
***** Proposed Scheme
*** Requests
* The OIC op is always the same: put.
* No URI Query CoAP options.
* The NMP header is included in the payload as a key-value pair (key="_h").
This pair is in the root map of the request and is a sibling of the other
request fields. The value of this pair is the big-endian eight-byte NMP
header with a length field of 0.
*** Responses
* As with requests, the NMP header is included in the payload as a
key-value pair (key="_h").
* No "r" or "w" field. The response fields are inserted into the root map
as a sibling of the "_h" pair.
* Errors encountered during processing of NMP requests are reported
identically to other NMP responses (embedded NMP response).
*** Notes
* Keys that start with an underscore are reserved to the OIC manager
protocol (e.g., "_h"). NMP requests and responses must not name any of
their fields with a leading underscore.
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/860d2d26
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/860d2d26
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/860d2d26
Branch: refs/heads/master
Commit: 860d2d266110918b4a6f923f1e18c9032d13c07d
Parents: a4df93c
Author: Christopher Collins <cc...@apache.org>
Authored: Wed Mar 1 17:41:21 2017 -0800
Committer: Christopher Collins <cc...@apache.org>
Committed: Thu Mar 2 08:15:05 2017 -0800
----------------------------------------------------------------------
mgmt/mgmt/include/mgmt/mgmt.h | 24 ++-
mgmt/mgmt/src/mgmt.c | 22 ++-
mgmt/newtmgr/include/newtmgr/newtmgr.h | 21 ---
mgmt/oicmgr/src/oicmgr.c | 238 ++++++++++++++++++----------
4 files changed, 192 insertions(+), 113 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/860d2d26/mgmt/mgmt/include/mgmt/mgmt.h
----------------------------------------------------------------------
diff --git a/mgmt/mgmt/include/mgmt/mgmt.h b/mgmt/mgmt/include/mgmt/mgmt.h
index 9b79723..3c402f2 100644
--- a/mgmt/mgmt/include/mgmt/mgmt.h
+++ b/mgmt/mgmt/include/mgmt/mgmt.h
@@ -36,6 +36,10 @@ extern "C" {
#define STR(x) #x
#endif
+#define NMGR_OP_READ (0)
+#define NMGR_OP_READ_RSP (1)
+#define NMGR_OP_WRITE (2)
+#define NMGR_OP_WRITE_RSP (3)
/* First 64 groups are reserved for system level newtmgr commands.
* Per-user commands are then defined after group 64.
@@ -63,6 +67,24 @@ extern "C" {
#define MGMT_ERR_EBADSTATE (6) /* Current state disallows command. */
#define MGMT_ERR_EPERUSER (256)
+#define NMGR_HDR_SIZE (8)
+
+struct nmgr_hdr {
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+ uint8_t nh_op:3; /* NMGR_OP_XXX */
+ uint8_t _res1:5;
+#endif
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+ uint8_t _res1:5;
+ uint8_t nh_op:3; /* NMGR_OP_XXX */
+#endif
+ uint8_t nh_flags; /* XXX reserved for future flags */
+ uint16_t nh_len; /* length of the payload */
+ uint16_t nh_group; /* NMGR_GROUP_XXX */
+ uint8_t nh_seq; /* sequence number */
+ uint8_t nh_id; /* message ID within group */
+};
+
struct mgmt_cbuf;
typedef int (*mgmt_handler_func_t)(struct mgmt_cbuf *);
@@ -85,7 +107,7 @@ struct mgmt_group {
sizeof(struct mgmt_handler));
int mgmt_group_register(struct mgmt_group *group);
-void mgmt_cbuf_setoerr(struct mgmt_cbuf *njb, int errcode);
+int mgmt_cbuf_setoerr(struct mgmt_cbuf *njb, int errcode);
const struct mgmt_handler *mgmt_find_handler(uint16_t group_id,
uint16_t handler_id);
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/860d2d26/mgmt/mgmt/src/mgmt.c
----------------------------------------------------------------------
diff --git a/mgmt/mgmt/src/mgmt.c b/mgmt/mgmt/src/mgmt.c
index 4ac232a..c1a2a1e 100644
--- a/mgmt/mgmt/src/mgmt.c
+++ b/mgmt/mgmt/src/mgmt.c
@@ -137,14 +137,20 @@ err:
return (NULL);
}
-void
+int
mgmt_cbuf_setoerr(struct mgmt_cbuf *cb, int errcode)
{
- CborEncoder *penc = &cb->encoder;
- CborError g_err = CborNoError;
- CborEncoder rsp;
- g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "rc");
- g_err |= cbor_encode_int(&rsp, errcode);
- g_err |= cbor_encoder_close_container(penc, &rsp);
+ int rc;
+
+ rc = cbor_encode_text_stringz(&cb->encoder, "rc");
+ if (rc != 0) {
+ return rc;
+ }
+
+ rc = cbor_encode_int(&cb->encoder, errcode);
+ if (rc != 0) {
+ return rc;
+ }
+
+ return 0;
}
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/860d2d26/mgmt/newtmgr/include/newtmgr/newtmgr.h
----------------------------------------------------------------------
diff --git a/mgmt/newtmgr/include/newtmgr/newtmgr.h b/mgmt/newtmgr/include/newtmgr/newtmgr.h
index 50deb15..1cf314d 100644
--- a/mgmt/newtmgr/include/newtmgr/newtmgr.h
+++ b/mgmt/newtmgr/include/newtmgr/newtmgr.h
@@ -29,27 +29,6 @@
extern "C" {
#endif
-#define NMGR_OP_READ (0)
-#define NMGR_OP_READ_RSP (1)
-#define NMGR_OP_WRITE (2)
-#define NMGR_OP_WRITE_RSP (3)
-
-struct nmgr_hdr {
-#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
- uint8_t nh_op:3; /* NMGR_OP_XXX */
- uint8_t _res1:5;
-#endif
-#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
- uint8_t _res1:5;
- uint8_t nh_op:3; /* NMGR_OP_XXX */
-#endif
- uint8_t nh_flags; /* XXX reserved for future flags */
- uint16_t nh_len; /* length of the payload */
- uint16_t nh_group; /* NMGR_GROUP_XXX */
- uint8_t nh_seq; /* sequence number */
- uint8_t nh_id; /* message ID within group */
-};
-
struct nmgr_transport;
/**
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/860d2d26/mgmt/oicmgr/src/oicmgr.c
----------------------------------------------------------------------
diff --git a/mgmt/oicmgr/src/oicmgr.c b/mgmt/oicmgr/src/oicmgr.c
index d2110ef..d7d3591 100644
--- a/mgmt/oicmgr/src/oicmgr.c
+++ b/mgmt/oicmgr/src/oicmgr.c
@@ -22,6 +22,7 @@
#include <os/os.h>
#include <os/endian.h>
+#include <defs/error.h>
#include <assert.h>
#include <string.h>
@@ -52,9 +53,6 @@ static struct omgr_state omgr_state = {
.os_event.ev_cb = omgr_event_start,
};
-static void omgr_oic_get(oc_request_t *request, oc_interface_mask_t interface);
-static void omgr_oic_put(oc_request_t *request, oc_interface_mask_t interface);
-
struct os_eventq *
mgmt_evq_get(void)
{
@@ -67,124 +65,199 @@ mgmt_evq_set(struct os_eventq *evq)
os_eventq_designate(&omgr_state.os_evq, evq, &omgr_state.os_event);
}
-static const struct mgmt_handler *
-omgr_find_handler(const char *q, int qlen)
+static int
+omgr_oic_read_hdr(struct CborValue *cv, struct nmgr_hdr *out_hdr)
{
- char id_str[8];
- int grp = -1;
- int id = -1;
- char *str;
- char *eptr;
- int slen;
-
- slen = oc_ri_get_query_value(q, qlen, "gr", &str);
- if (slen > 0 && slen < sizeof(id_str) - 1) {
- memcpy(id_str, str, slen);
- id_str[slen] = '\0';
- grp = strtoul(id_str, &eptr, 0);
- if (*eptr != '\0') {
- return NULL;
- }
+ size_t hlen;
+ int rc;
+
+ struct cbor_attr_t attrs[] = {
+ [0] = {
+ .attribute = "_h",
+ .type = CborAttrByteStringType,
+ .addr.bytestring.data = (void *)out_hdr,
+ .addr.bytestring.len = &hlen,
+ .nodefault = 1,
+ .len = sizeof *out_hdr,
+ },
+ [1] = { 0 }
+ };
+
+ rc = cbor_read_object(cv, attrs);
+ if (rc != 0 || hlen != sizeof *out_hdr) {
+ return MGMT_ERR_EINVAL;
}
- slen = oc_ri_get_query_value(q, qlen, "id", &str);
- if (slen > 0 && slen < sizeof(id_str) - 1) {
- memcpy(id_str, str, slen);
- id_str[slen] = '\0';
- id = strtoul(id_str, &eptr, 0);
- if (*eptr != '\0') {
- return NULL;
- }
+
+ out_hdr->nh_len = ntohs(out_hdr->nh_len);
+ out_hdr->nh_group = ntohs(out_hdr->nh_group);
+
+ return 0;
+}
+
+static int
+omgr_encode_nmp_hdr(struct CborEncoder *enc, struct nmgr_hdr hdr)
+{
+ int rc;
+
+ rc = cbor_encode_text_string(enc, "_h", 2);
+ if (rc != 0) {
+ return MGMT_ERR_ENOMEM;
+ }
+
+ hdr.nh_len = htons(hdr.nh_len);
+ hdr.nh_group = htons(hdr.nh_group);
+
+ /* Encode the NMP header in the response. */
+ rc = cbor_encode_byte_string(enc, (void *)&hdr, sizeof hdr);
+ if (rc != 0) {
+ return MGMT_ERR_ENOMEM;
+ }
+
+ return 0;
+}
+
+static int
+omgr_send_err_rsp(struct CborEncoder *enc, const struct nmgr_hdr *hdr,
+ int nmp_status)
+{
+ int rc;
+
+ rc = omgr_encode_nmp_hdr(enc, *hdr);
+ if (rc != 0) {
+ return rc;
+ }
+
+ rc = cbor_encode_text_stringz(enc, "rc");
+ if (rc != 0) {
+ return MGMT_ERR_ENOMEM;
+ }
+
+ rc = cbor_encode_int(enc, nmp_status);
+ if (rc != 0) {
+ return MGMT_ERR_ENOMEM;
}
- return mgmt_find_handler(grp, id);
+
+ return 0;
}
static void
-omgr_oic_op(oc_request_t *req, oc_interface_mask_t mask, int isset)
+omgr_oic_put(oc_request_t *req, oc_interface_mask_t mask)
{
struct omgr_state *o = &omgr_state;
const struct mgmt_handler *handler;
uint16_t data_off;
struct os_mbuf *m;
int rc = 0;
- extern CborEncoder g_encoder;
+ struct nmgr_hdr hdr;
+ int rsp_hdr_filled = 0;
- if (!req->query_len) {
- goto bad_req;
- }
-
- handler = omgr_find_handler(req->query, req->query_len);
- if (!handler) {
- goto bad_req;
- }
+ coap_get_payload(req->packet, &m, &data_off);
- rc = coap_get_payload(req->packet, &m, &data_off);
cbor_mbuf_reader_init(&o->os_cbuf.ob_reader, m, data_off);
-
cbor_parser_init(&o->os_cbuf.ob_reader.r, 0, &o->os_cbuf.ob_mj.parser,
&o->os_cbuf.ob_mj.it);
- /* start generating the CBOR OUTPUT */
- /* this is worth a quick note. We are encoding CBOR within CBOR, so we
- * need to use the same encoder as ocf stack. We are using their global
- * encoder g_encoder which they intialized before this function is called.
- * But we can't call their macros here as it won't use the right mape
- * encoder (ob_mj) */
+ rc = omgr_oic_read_hdr(&o->os_cbuf.ob_mj.it, &hdr);
+ if (rc != 0) {
+ rc = MGMT_ERR_EINVAL;
+ goto done;
+ }
+
+ /* Convert request header to response header to be sent. */
+ switch (hdr.nh_op) {
+ case NMGR_OP_READ:
+ hdr.nh_op = NMGR_OP_READ_RSP;
+ break;
+
+ case NMGR_OP_WRITE:
+ hdr.nh_op = NMGR_OP_WRITE_RSP;
+ break;
+
+ default:
+ goto done;
+ }
+ rsp_hdr_filled = 1;
+
+ /* Begin root map in response. */
cbor_encoder_create_map(&g_encoder, &o->os_cbuf.ob_mj.encoder,
CborIndefiniteLength);
+ handler = mgmt_find_handler(hdr.nh_group, hdr.nh_id);
+ if (handler == NULL) {
+ rc = MGMT_ERR_ENOENT;
+ goto done;
+ }
+
+ cbor_mbuf_reader_init(&o->os_cbuf.ob_reader, m, data_off);
+ cbor_parser_init(&o->os_cbuf.ob_reader.r, 0, &o->os_cbuf.ob_mj.parser,
+ &o->os_cbuf.ob_mj.it);
+
switch (mask) {
case OC_IF_BASELINE:
oc_process_baseline_interface(req->resource);
+ /* Fallthrough */
+
case OC_IF_RW:
- if (!isset) {
- cbor_encode_text_string(&root_map, "r", 1);
- if (handler->mh_read) {
- rc = handler->mh_read(&o->os_cbuf.ob_mj);
+ switch (hdr.nh_op) {
+ case NMGR_OP_READ_RSP:
+ if (handler->mh_read == NULL) {
+ rc = MGMT_ERR_ENOENT;
} else {
- goto bad_req;
+ rc = handler->mh_read(&o->os_cbuf.ob_mj);
}
- } else {
- cbor_encode_text_string(&root_map, "w", 1);
- if (handler->mh_write) {
- rc = handler->mh_write(&o->os_cbuf.ob_mj);
+ break;
+
+ case NMGR_OP_WRITE_RSP:
+ if (handler->mh_write == NULL) {
+ rc = MGMT_ERR_ENOENT;
} else {
- goto bad_req;
+ rc = handler->mh_write(&o->os_cbuf.ob_mj);
}
+ break;
+
+ default:
+ rc = MGMT_ERR_EINVAL;
+ break;
}
- if (rc) {
- goto bad_req;
+ if (rc != 0) {
+ goto done;
+ }
+
+ /* Encode the NMP header in the response. */
+ rc = omgr_encode_nmp_hdr(&o->os_cbuf.ob_mj.encoder, hdr);
+ if (rc != 0) {
+ rc = MGMT_ERR_ENOMEM;
+ goto done;
}
break;
+
default:
break;
}
- cbor_encoder_close_container(&g_encoder, &o->os_cbuf.ob_mj.encoder);
- oc_send_response(req, OC_STATUS_OK);
+ rc = 0;
- return;
-bad_req:
- /*
- * XXXX might send partially constructed response as payload
- */
- if (rc == MGMT_ERR_ENOMEM) {
- rc = OC_STATUS_INTERNAL_SERVER_ERROR;
- } else {
- rc = OC_STATUS_BAD_REQUEST;
- }
- oc_send_response(req, rc);
-}
+done:
+ if (rc != 0) {
+ if (rsp_hdr_filled) {
+ rc = omgr_send_err_rsp(&g_encoder, &hdr, rc);
+ }
+ switch (rc) {
+ case 0:
+ break;
-static void
-omgr_oic_get(oc_request_t *req, oc_interface_mask_t mask)
-{
- omgr_oic_op(req, mask, 0);
-}
+ case MGMT_ERR_ENOMEM:
+ rc = OC_STATUS_INTERNAL_SERVER_ERROR;
+ break;
-static void
-omgr_oic_put(oc_request_t *req, oc_interface_mask_t mask)
-{
- omgr_oic_op(req, mask, 1);
+ default:
+ rc = OC_STATUS_BAD_REQUEST;
+ break;
+ }
+ }
+
+ cbor_encoder_close_container(&g_encoder, &o->os_cbuf.ob_mj.encoder);
+ oc_send_response(req, rc);
}
static void
@@ -204,7 +277,6 @@ omgr_event_start(struct os_event *ev)
oc_resource_bind_resource_interface(res, mode);
oc_resource_set_default_interface(res, mode);
oc_resource_set_discoverable(res);
- oc_resource_set_request_handler(res, OC_GET, omgr_oic_get);
oc_resource_set_request_handler(res, OC_PUT, omgr_oic_put);
oc_add_resource(res);
}