From: "Gix, Brian" <brian.gix@intel.com>
To: "michal.lowas-rzechonek@silvair.com"
<michal.lowas-rzechonek@silvair.com>,
"linux-bluetooth@vger.kernel.org"
<linux-bluetooth@vger.kernel.org>
Cc: "Stotland, Inga" <inga.stotland@intel.com>
Subject: Re: [PATCH BlueZ v2 1/3] mesh: Split APP_IDX_DEV into APP_IDX_DEV_LOCAL and APP_IDX_DEV_REMOTE
Date: Tue, 2 Jul 2019 18:15:13 +0000 [thread overview]
Message-ID: <1562091311.23933.6.camel@intel.com> (raw)
In-Reply-To: <20190702090731.30852-2-michal.lowas-rzechonek@silvair.com>
Hi Michał,
This patch fails check patch, plus other issues:
WARNING:LONG_LINE: line over 80 characters
#79: FILE: mesh/model.c:310:
+ if (fwd->idx != APP_IDX_DEV_LOCAL && !has_binding(mod->bindings, fwd->idx))
WARNING:LONG_LINE: line over 80 characters
#119: FILE: mesh/model.c:1386:
+ l_queue_push_head(mod->bindings, L_UINT_TO_PTR(APP_IDX_DEV_LOCAL));
WARNING:LONG_LINE: line over 80 characters
#125: FILE: mesh/model.c:1391:
+ l_queue_push_head(mod->bindings, L_UINT_TO_PTR(APP_IDX_DEV_LOCAL));
total: 0 errors, 3 warnings, 125 lines checked
On Tue, 2019-07-02 at 11:07 +0200, Michał Lowas-Rzechonek wrote:
> This is needed to distinguish incoming messages encrypted using a device
> key: if the key is local, the message can be forwarded to internal
> models. If the key is a known remote one, it will be forwarded to the
> application via DevKeyMessageReceived() API.
> ---
> mesh/cfgmod-server.c | 15 ++++++++-------
> mesh/model.c | 20 +++++++++++---------
> mesh/net.h | 10 ++++++----
> 3 files changed, 25 insertions(+), 20 deletions(-)
>
> diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
> index 634ac006b..a849b5e99 100644
> --- a/mesh/cfgmod-server.c
> +++ b/mesh/cfgmod-server.c
> @@ -69,7 +69,8 @@ static void send_pub_status(struct mesh_node *node, uint16_t src, uint16_t dst,
> n += 2;
> }
>
> - mesh_model_send(node, dst, src, APP_IDX_DEV, DEFAULT_TTL, msg, n);
> + mesh_model_send(node, dst, src, APP_IDX_DEV_LOCAL, DEFAULT_TTL,
> + msg, n);
> }
>
> static bool config_pub_get(struct mesh_node *node, uint16_t src, uint16_t dst,
> @@ -254,7 +255,7 @@ static void send_sub_status(struct mesh_node *node, uint16_t src, uint16_t dst,
> n += 2;
> }
>
> - mesh_model_send(node, dst, src, APP_IDX_DEV, DEFAULT_TTL, msg, n);
> + mesh_model_send(node, dst, src, APP_IDX_DEV_LOCAL, DEFAULT_TTL, msg, n);
> }
>
> static bool config_sub_get(struct mesh_node *node, uint16_t src, uint16_t dst,
> @@ -312,7 +313,7 @@ static bool config_sub_get(struct mesh_node *node, uint16_t src, uint16_t dst,
>
> *msg_status = (uint8_t) status;
>
> - mesh_model_send(node, dst, src, APP_IDX_DEV, DEFAULT_TTL, msg, n);
> + mesh_model_send(node, dst, src, APP_IDX_DEV_LOCAL, DEFAULT_TTL, msg, n);
> return true;
> }
>
> @@ -487,7 +488,7 @@ static void send_model_app_status(struct mesh_node *node, uint16_t src,
> l_put_le16(id, msg + n);
> n += 2;
>
> - mesh_model_send(node, dst, src, APP_IDX_DEV, DEFAULT_TTL, msg, n);
> + mesh_model_send(node, dst, src, APP_IDX_DEV_LOCAL, DEFAULT_TTL, msg, n);
> }
>
> static void model_app_list(struct mesh_node *node, uint16_t src, uint16_t dst,
> @@ -540,7 +541,7 @@ static void model_app_list(struct mesh_node *node, uint16_t src, uint16_t dst,
>
> if (result >= 0) {
> *status = result;
> - mesh_model_send(node, dst, src, APP_IDX_DEV, DEFAULT_TTL,
> + mesh_model_send(node, dst, src, APP_IDX_DEV_LOCAL, DEFAULT_TTL,
> msg, n);
> }
>
> @@ -758,7 +759,7 @@ static bool cfg_srv_pkt(uint16_t src, uint32_t dst,
> uint16_t interval;
> uint16_t n;
>
> - if (idx != APP_IDX_DEV)
> + if (idx != APP_IDX_DEV_LOCAL)
> return false;
>
> if (mesh_model_opcode_get(pkt, size, &opcode, &n)) {
> @@ -1259,7 +1260,7 @@ static bool cfg_srv_pkt(uint16_t src, uint32_t dst,
> if (n) {
> /* print_packet("App Tx", long_msg ? long_msg : msg, n); */
> mesh_model_send(node, unicast, src,
> - APP_IDX_DEV, DEFAULT_TTL,
> + APP_IDX_DEV_LOCAL, DEFAULT_TTL,
> long_msg ? long_msg : msg, n);
> }
> l_free(long_msg);
> diff --git a/mesh/model.c b/mesh/model.c
> index 7401dcecb..e09dbd364 100644
> --- a/mesh/model.c
> +++ b/mesh/model.c
> @@ -306,7 +306,8 @@ static void forward_model(void *a, void *b)
> bool result;
>
> l_debug("model %8.8x with idx %3.3x", mod->id, fwd->idx);
> - if (fwd->idx != APP_IDX_DEV && !has_binding(mod->bindings, fwd->idx))
> +
> + if (fwd->idx != APP_IDX_DEV_LOCAL && !has_binding(mod->bindings, fwd->idx))
> return;
>
> dst = fwd->dst;
> @@ -356,15 +357,16 @@ static int dev_packet_decrypt(struct mesh_node *node, const uint8_t *data,
> uint16_t dst, uint8_t key_id, uint32_t seq,
> uint32_t iv_idx, uint8_t *out)
> {
> - const uint8_t *dev_key;
> + uint8_t dev_key[16];
> + const uint8_t *key;
You have renamed dev_key --> key, and no longer use dev_key that I can see here... which will cause it's own
build warning:
mesh/model.c: In function ‘dev_packet_decrypt’:
mesh/model.c:360:10: error: unused variable ‘dev_key’ [-Werror=unused-variable]
uint8_t dev_key[16];
^~~~~~~
cc1: all warnings being treated as errors
>
> - dev_key = node_get_device_key(node);
> - if (!dev_key)
> + key = node_get_device_key(node);
> + if (!key)
> return false;
>
> if (mesh_crypto_payload_decrypt(NULL, 0, data, size, szmict, src,
> - dst, key_id, seq, iv_idx, out, dev_key))
> - return APP_IDX_DEV;
> + dst, key_id, seq, iv_idx, out, key))
> + return APP_IDX_DEV_LOCAL;
>
> return -1;
> }
> @@ -952,7 +954,7 @@ bool mesh_model_send(struct mesh_node *node, uint16_t src, uint16_t target,
> if (IS_UNASSIGNED(target))
> return false;
>
> - if (app_idx == APP_IDX_DEV) {
> + if (app_idx == APP_IDX_DEV_LOCAL) {
> key = node_get_device_key(node);
> if (!key)
> return false;
> @@ -1381,12 +1383,12 @@ struct mesh_model *mesh_model_setup(struct mesh_node *node, uint8_t ele_idx,
> if (ele_idx != PRIMARY_ELE_IDX)
> return NULL;
>
> - l_queue_push_head(mod->bindings, L_UINT_TO_PTR(APP_IDX_DEV));
> + l_queue_push_head(mod->bindings, L_UINT_TO_PTR(APP_IDX_DEV_LOCAL));
> return mod;
> }
>
> if (db_mod->id == CONFIG_CLI_MODEL) {
> - l_queue_push_head(mod->bindings, L_UINT_TO_PTR(APP_IDX_DEV));
> + l_queue_push_head(mod->bindings, L_UINT_TO_PTR(APP_IDX_DEV_LOCAL));
> return mod;
> }
>
> diff --git a/mesh/net.h b/mesh/net.h
> index f19024766..834712d8d 100644
> --- a/mesh/net.h
> +++ b/mesh/net.h
> @@ -37,10 +37,12 @@ struct mesh_node;
> #define SEQ_MASK 0xffffff
>
> #define CREDFLAG_MASK 0x1000
> -#define APP_IDX_MASK 0x0fff
> -#define APP_IDX_DEV 0x7fff
> -#define APP_IDX_ANY 0x8000
> -#define APP_IDX_NET 0xffff
> +
> +#define APP_IDX_MASK 0x0fff
> +#define APP_IDX_DEV_REMOTE 0x6fff
> +#define APP_IDX_DEV_LOCAL 0x7fff
> +#define APP_IDX_ANY 0x8000
> +#define APP_IDX_NET 0xffff
Since this patch-set needs to be re-spun, Inga was hoping you could also remove the unused cruft APP_IDX_NET
altogether...
>
> #define NET_IDX_INVALID 0xffff
> #define NET_NID_INVALID 0xff
next prev parent reply other threads:[~2019-07-02 18:15 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-02 9:07 [PATCH BlueZ v2 0/3] Add possibility to use remote device keys Michał Lowas-Rzechonek
2019-07-02 9:07 ` [PATCH BlueZ v2 1/3] mesh: Split APP_IDX_DEV into APP_IDX_DEV_LOCAL and APP_IDX_DEV_REMOTE Michał Lowas-Rzechonek
2019-07-02 18:15 ` Gix, Brian [this message]
2019-07-02 9:07 ` [PATCH BlueZ v2 2/3] mesh: Implement DevKeySend() method on Node interface Michał Lowas-Rzechonek
2019-07-02 9:07 ` [PATCH BlueZ v2 3/3] mesh: Handle messages encrypted with a remote device key Michał Lowas-Rzechonek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1562091311.23933.6.camel@intel.com \
--to=brian.gix@intel.com \
--cc=inga.stotland@intel.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=michal.lowas-rzechonek@silvair.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).