From: Isak Westin <karl.westin@loytec.com>
To: "Gix, Brian" <brian.gix@intel.com>,
"linux-bluetooth@vger.kernel.org"
<linux-bluetooth@vger.kernel.org>
Subject: RE: [PATCH BlueZ 5/6] mesh: Keep canceled SAR data for at least 10 sec
Date: Fri, 7 Oct 2022 13:33:09 +0000 [thread overview]
Message-ID: <GV0P278MB01298DB6AC0C593245E3FCFB955F9@GV0P278MB0129.CHEP278.PROD.OUTLOOK.COM> (raw)
In-Reply-To: 4c8e20487a5ee71088bf6374bae8fd55a95055b3.camel@intel.com
Hi Brian,
> Hi Isak,
>
> Have you run this patch through any runtime analysis (like valgrind
> etc) to ensure it has introduced no memory leaks?
>
> I am particularily concerned with any l_timeout_remove() calls that don't immediately set the freed timer pointer to NULL, and any new
> l_timeout_create() calls that are not preceded with a check that there is not already a valid pointer occupying the seg_timeout or msg_timeout members.
>
I tested it with valgrind and found no memory leaks. However, it is perhaps anyway a better practice to use l_timeout_modify() ?
If so, I will update the patch.
> On Thu, 2022-10-06 at 16:59 +0200, Isak Westin wrote:
> > When a SAR transmission has been completed or canceled, the recipent
> > should store the block authentication values for at least 10 seconds
> > and ignore new segments with the same values during this period. See
> > MshPRFv1.0.1 section 3.5.3.4.
> > ---
> > mesh/net.c | 30 ++++++++++++++++++++++++++++--
> > 1 file changed, 28 insertions(+), 2 deletions(-)
> >
> > diff --git a/mesh/net.c b/mesh/net.c
> > index 379a6e250..e95ae5114 100644
> > --- a/mesh/net.c
> > +++ b/mesh/net.c
> > @@ -46,6 +46,7 @@
> >
> > #define SEG_TO 2
> > #define MSG_TO 60
> > +#define SAR_DEL 10
> >
> > #define DEFAULT_TRANSMIT_COUNT 1
> > #define DEFAULT_TRANSMIT_INTERVAL 100
> > @@ -166,6 +167,7 @@ struct mesh_sar {
> > bool segmented;
> > bool frnd;
> > bool frnd_cred;
> > + bool delete;
> > uint8_t ttl;
> > uint8_t last_seg;
> > uint8_t key_aid;
> > @@ -1493,13 +1495,27 @@ static void inseg_to(struct l_timeout
> > *seg_timeout, void *user_data) static void inmsg_to(struct l_timeout
> > *msg_timeout, void *user_data) {
> > struct mesh_net *net = user_data;
> > - struct mesh_sar *sar = l_queue_remove_if(net->sar_in,
> > + struct mesh_sar *sar = l_queue_find(net->sar_in,
> > match_msg_timeout, msg_timeout);
> >
> > l_timeout_remove(msg_timeout);
> > if (!sar)
> > return;
> >
> > + if (!sar->delete) {
> > + /*
> > + * Incomplete timer expired, cancel SAR and start
> > + * delete timer
> > + */
> > + sar->delete = true;
> > + l_timeout_remove(sar->seg_timeout);
> > + sar->seg_timeout = NULL;
> > + sar->msg_timeout = l_timeout_create(SAR_DEL,
> > + inmsg_to, net, NULL);
> > + return;
> > + }
> > +
> > + l_queue_remove(net->sar_in, sar);
> > sar->msg_timeout = NULL;
> > mesh_sar_free(sar);
> > }
> > @@ -1956,7 +1972,9 @@ static bool seg_rxed(struct mesh_net *net, bool
> > frnd, uint32_t iv_index,
> > /* Re-Send ACK for full msg */
> > send_net_ack(net, sar_in, expected);
> > return true;
> > - }
> > + } else if (sar_in->delete)
> > + /* Ignore canceled */
> > + return false;
> > } else {
> > uint16_t len = MAX_SEG_TO_LEN(segN);
> >
> > @@ -1996,6 +2014,9 @@ static bool seg_rxed(struct mesh_net *net, bool
> > frnd, uint32_t iv_index,
> > sar_in->len = segN * MAX_SEG_LEN + size;
> >
> > if (sar_in->flags == expected) {
> > + /* Remove message incomplete timer */
> > + l_timeout_remove(sar_in->msg_timeout);
> > +
> > /* Got it all */
> > send_net_ack(net, sar_in, expected);
> >
> > @@ -2006,6 +2027,11 @@ static bool seg_rxed(struct mesh_net *net, bool
> > frnd, uint32_t iv_index,
> > /* Kill Inter-Seg timeout */
> > l_timeout_remove(sar_in->seg_timeout);
> > sar_in->seg_timeout = NULL;
> > +
> > + /* Start delete timer */
> > + sar_in->delete = true;
> > + sar_in->msg_timeout = l_timeout_create(SAR_DEL,
> > + inmsg_to, net, NULL);
> > return true;
> > }
> >
>
>
next prev parent reply other threads:[~2022-10-07 13:33 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-06 14:59 [PATCH BlueZ 0/6] Mesh: Fixes for PTS issues Isak Westin
2022-10-06 14:59 ` [PATCH BlueZ 1/6] mesh: Update Key Refresh flag after provision Isak Westin
2022-10-06 16:22 ` Mesh: Fixes for PTS issues bluez.test.bot
2022-10-06 14:59 ` [PATCH BlueZ 2/6] mesh: provisionee: Handle unknown PDUs Isak Westin
2022-10-06 14:59 ` [PATCH BlueZ 3/6] mesh: provisionee: Handle failed provisioning Isak Westin
2022-10-06 14:59 ` [PATCH BlueZ 4/6] mesh: provisionee: Check prov start parameters Isak Westin
2022-10-06 14:59 ` [PATCH BlueZ 5/6] mesh: Keep canceled SAR data for at least 10 sec Isak Westin
2022-10-06 20:55 ` Gix, Brian
2022-10-07 13:33 ` Isak Westin [this message]
2022-10-07 14:56 ` Gix, Brian
2022-10-11 14:13 ` Isak Westin
2022-10-06 14:59 ` [PATCH BlueZ 6/6] mesh: Fix msg cache ring buffer Isak Westin
2022-10-06 20:47 ` Gix, Brian
2022-10-07 8:02 ` Isak Westin
2022-10-07 15:08 ` Gix, Brian
2022-10-06 21:00 ` [PATCH BlueZ 0/6] Mesh: Fixes for PTS issues patchwork-bot+bluetooth
2022-10-06 21:00 ` Gix, Brian
2022-10-07 15:10 ` patchwork-bot+bluetooth
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=GV0P278MB01298DB6AC0C593245E3FCFB955F9@GV0P278MB0129.CHEP278.PROD.OUTLOOK.COM \
--to=karl.westin@loytec.com \
--cc=brian.gix@intel.com \
--cc=linux-bluetooth@vger.kernel.org \
/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).