From: Szymon Janc <szymon.janc@codecoup.pl>
To: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH] monitor: Add support for limiting btsnoop files size
Date: Thu, 04 Jan 2018 15:33:37 +0100 [thread overview]
Message-ID: <1751555.yVb2H0TeuM@ix> (raw)
In-Reply-To: <CABBYNZKp+TdvRab+vw02s6J8Bu+GJpZAz8GGhS=4dTJXrAXg+g@mail.gmail.com>
Hi Luiz,
On Thursday, 4 January 2018 15:23:34 CET Luiz Augusto von Dentz wrote:
> Hi Szymon,
>
> On Thu, Jan 4, 2018 at 12:07 PM, Szymon Janc <szymon.janc@codecoup.pl>
wrote:
> > This adds new option -l (--limit) which allows to limit maximum
> > size of writen btsnoop file. After limit is reached new btsnoop
> > file will be created. To distinguish file postfix in for of
> > _DATE_TIME is added to path provided with -w option. For convenience
> > KkMm modifiers are also supported.
> >
> > This is especially useful on unattended systems for tiding btmon
> > with log rotation.
> >
> > Example result of running btmon -w test.bin -l 10k
> > test.log_2018-01-04_14:55:06_694638
> > test.log_2018-01-04_14:56:15_933158
> > test.log_2018-01-04_14:57:20_741201
> > test.log_2018-01-04_14:58:13_280835
> > test.log_2018-01-04_14:59:02_183569
> > test.log_2018-01-04_15:00:05_352733
> > test.log_2018-01-04_15:00:54_475147
> > test.log_2018-01-04_15:01:57_183352
>
> While this could be convenient do you plan to support reading from all
> the files or the user has to open them separately? It might be
> possible to do this even on demand when the log reach the end asks the
> user if he wants to continue to the next file.
Hmm this shouldn't be hard to achieve but instead of asking user I'd simply
allow to pass multiple files to -r switch. In particular user could do
`btmon -r test_log*`. How that sounds to you?
>
> > ---
> >
> > monitor/control.c | 55
> > ++++++++++++++++++++++++++++++++++++++++++++++++++-- monitor/control.h
> > | 2 +-
> > monitor/main.c | 35 +++++++++++++++++++++++++++++++--
> > src/shared/btsnoop.c | 17 ----------------
> > src/shared/btsnoop.h | 17 ++++++++++++++++
> > tools/btsnoop.c | 17 ----------------
> > 6 files changed, 104 insertions(+), 39 deletions(-)
> >
> > diff --git a/monitor/control.c b/monitor/control.c
> > index 1cd79ca5d..6b69f3575 100644
> > --- a/monitor/control.c
> > +++ b/monitor/control.c
> > @@ -32,6 +32,7 @@
> >
> > #include <unistd.h>
> > #include <stdlib.h>
> > #include <string.h>
> >
> > +#include <time.h>
> >
> > #include <sys/time.h>
> > #include <sys/socket.h>
> > #include <sys/un.h>
> >
> > @@ -59,6 +60,10 @@ static struct btsnoop *btsnoop_file = NULL;
> >
> > static bool hcidump_fallback = false;
> > static bool decode_control = true;
> >
> > +static size_t writer_limit = 0;
> > +static size_t writer_size = BTSNOOP_HDR_SIZE;
> > +static const char *writer_path = NULL;
> > +
> >
> > struct control_data {
> >
> > uint16_t channel;
> > int fd;
> >
> > @@ -908,6 +913,47 @@ void control_message(uint16_t opcode, const void
> > *data, uint16_t size)>
> > }
> >
> > }
> >
> > +static const char *get_real_path(const char *path)
> > +{
> > + static char real_path[FILENAME_MAX];
> > + struct timeval tv;
> > + struct tm tm;
> > +
> > + if (!writer_limit)
> > + return path;
> > +
> > + memset(&tv, 0, sizeof(tv));
> > +
> > + gettimeofday(&tv, NULL);
> > + localtime_r(&tv.tv_sec, &tm);
> > +
> > + snprintf(real_path, FILENAME_MAX,
> > + "%s_%04d-%02d-%02d_%02d:%02d:%02d_%06lu", path,
> > + tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
> > + tm.tm_hour, tm.tm_min, tm.tm_sec, tv.tv_usec);
> > +
> > + real_path[FILENAME_MAX - 1] = '\0';
> > +
> > + return real_path;
> > +}
> > +
> > +static void rotate_btsnoop_file(ssize_t pktlen)
> > +{
> > + if (!writer_limit)
> > + return;
> > +
> > + writer_size += BTSNOOP_PKT_SIZE + pktlen;
> > +
> > + if (writer_size <= writer_limit)
> > + return;
> > +
> > + writer_size = BTSNOOP_HDR_SIZE + BTSNOOP_PKT_SIZE + pktlen;
> > +
> > + btsnoop_unref(btsnoop_file);
> > + btsnoop_file = btsnoop_create(get_real_path(writer_path),
> > +
> > BTSNOOP_FORMAT_MONITOR); +}
> > +
> >
> > static void data_callback(int fd, uint32_t events, void *user_data)
> > {
> >
> > struct control_data *data = user_data;
> >
> > @@ -974,6 +1020,8 @@ static void data_callback(int fd, uint32_t events,
> > void *user_data)>
> > data->buf,
> > pktlen);
> >
> > break;
> >
> > case HCI_CHANNEL_MONITOR:
> > + rotate_btsnoop_file(pktlen);
> > +
> >
> > btsnoop_write_hci(btsnoop_file, tv, index, opcode,
> > 0,
> >
> > data->buf,
> > pktlen);
> >
> > ellisys_inject_hci(tv, index, opcode,
> >
> > @@ -1371,10 +1419,13 @@ int control_tty(const char *path, unsigned int
> > speed)>
> > return 0;
> >
> > }
> >
> > -bool control_writer(const char *path)
> > +bool control_writer(const char *path, size_t limit)
> >
> > {
> >
> > - btsnoop_file = btsnoop_create(path, BTSNOOP_FORMAT_MONITOR);
> > + writer_path = path;
> > + writer_limit = limit;
> >
> > + btsnoop_file = btsnoop_create(get_real_path(writer_path),
> > +
> > BTSNOOP_FORMAT_MONITOR);>
> > return !!btsnoop_file;
> >
> > }
> >
> > diff --git a/monitor/control.h b/monitor/control.h
> > index 630a852e4..dec19d1d4 100644
> > --- a/monitor/control.h
> > +++ b/monitor/control.h
> > @@ -24,7 +24,7 @@
> >
> > #include <stdint.h>
> >
> > -bool control_writer(const char *path);
> > +bool control_writer(const char *path, size_t limit);
> >
> > void control_reader(const char *path);
> > void control_server(const char *path);
> > int control_tty(const char *path, unsigned int speed);
> >
> > diff --git a/monitor/main.c b/monitor/main.c
> > index 3e61a4661..3755d01ae 100644
> > --- a/monitor/main.c
> > +++ b/monitor/main.c
> > @@ -31,6 +31,7 @@
> >
> > #include <stdlib.h>
> > #include <string.h>
> > #include <getopt.h>
> >
> > +#include <limits.h>
> >
> > #include <sys/un.h>
> >
> > #include "src/shared/mainloop.h"
> >
> > @@ -61,6 +62,7 @@ static void usage(void)
> >
> > printf("options:\n"
> >
> > "\t-r, --read <file> Read traces in btsnoop format\n"
> > "\t-w, --write <file> Save traces in btsnoop format\n"
> >
> > + "\t-l, --limit <bytes> Rotate btsnoop files\n"
> >
> > "\t-a, --analyze <file> Analyze traces in btsnoop
> > format\n"
> > "\t-s, --server <socket> Start monitor server socket\n"
> > "\t-p, --priority <level> Show only priority or lower\n"
> >
> > @@ -80,6 +82,7 @@ static const struct option main_options[] = {
> >
> > { "tty-speed", required_argument, NULL, 'B' },
> > { "read", required_argument, NULL, 'r' },
> > { "write", required_argument, NULL, 'w' },
> >
> > + { "limit", required_argument, NULL, 'l' },
> >
> > { "analyze", required_argument, NULL, 'a' },
> > { "server", required_argument, NULL, 's' },
> > { "priority",required_argument, NULL, 'p' },
> >
> > @@ -108,6 +111,8 @@ int main(int argc, char *argv[])
> >
> > const char *str;
> > int exit_status;
> > sigset_t mask;
> >
> > + size_t writer_limit = 0;
> > + char *endptr;
> >
> > mainloop_init();
> >
> > @@ -117,7 +122,7 @@ int main(int argc, char *argv[])
> >
> > int opt;
> > struct sockaddr_un addr;
> >
> > - opt = getopt_long(argc, argv, "d:r:w:a:s:p:i:tTSAE:vh",
> > + opt = getopt_long(argc, argv, "d:r:w:l:a:s:p:i:tTSAE:vh",
> >
> > main_options, NULL);
> >
> > if (opt < 0)
> >
> > break;
> >
> > @@ -139,6 +144,32 @@ int main(int argc, char *argv[])
> >
> > case 'w':
> > writer_path = optarg;
> > break;
> >
> > + case 'l':
> > + str = optarg;
> > + writer_limit = strtoul(str, &endptr, 10);
> > +
> > + if (writer_limit == ULONG_MAX) {
> > + fprintf(stderr, "Invalid limit value\n");
> > + return EXIT_FAILURE;
> > + }
> > +
> > + if (*endptr != '\0') {
> > + if (*endptr == 'K' || *endptr == 'k') {
> > + writer_limit *= 1024;
> > + } else if (*endptr == 'M' || *endptr ==
> > 'm') { + writer_limit *= 1024 *
> > 1024; + } else {
> > + fprintf(stderr, "Invalid limit
> > value\n"); + return EXIT_FAILURE;
> > + }
> > + }
> > +
> > + /* avoid creating too small files */
> > + if (writer_limit < 1024) {
> > + fprintf(stderr, "Too small limit
> > value\n");
> > + return EXIT_FAILURE;
> > + }
> > + break;
> >
> > case 'a':
> > analyze_path = optarg;
> > break;
> >
> > @@ -232,7 +263,7 @@ int main(int argc, char *argv[])
> >
> > return EXIT_SUCCESS;
> >
> > }
> >
> > - if (writer_path && !control_writer(writer_path)) {
> > + if (writer_path && !control_writer(writer_path, writer_limit)) {
> >
> > printf("Failed to open '%s'\n", writer_path);
> > return EXIT_FAILURE;
> >
> > }
> >
> > diff --git a/src/shared/btsnoop.c b/src/shared/btsnoop.c
> > index e20d1b382..17e3faf2a 100644
> > --- a/src/shared/btsnoop.c
> > +++ b/src/shared/btsnoop.c
> > @@ -35,23 +35,6 @@
> >
> > #include "src/shared/btsnoop.h"
> >
> > -struct btsnoop_hdr {
> > - uint8_t id[8]; /* Identification Pattern */
> > - uint32_t version; /* Version Number = 1 */
> > - uint32_t type; /* Datalink Type */
> > -} __attribute__ ((packed));
> > -#define BTSNOOP_HDR_SIZE (sizeof(struct btsnoop_hdr))
> > -
> > -struct btsnoop_pkt {
> > - uint32_t size; /* Original Length */
> > - uint32_t len; /* Included Length */
> > - uint32_t flags; /* Packet Flags */
> > - uint32_t drops; /* Cumulative Drops */
> > - uint64_t ts; /* Timestamp microseconds */
> > - uint8_t data[0]; /* Packet Data */
> > -} __attribute__ ((packed));
> > -#define BTSNOOP_PKT_SIZE (sizeof(struct btsnoop_pkt))
> > -
> >
> > static const uint8_t btsnoop_id[] = { 0x62, 0x74, 0x73, 0x6e,
> >
> > 0x6f, 0x6f, 0x70, 0x00 };
> >
> > diff --git a/src/shared/btsnoop.h b/src/shared/btsnoop.h
> > index 3df8998a3..011aa1d68 100644
> > --- a/src/shared/btsnoop.h
> > +++ b/src/shared/btsnoop.h
> > @@ -91,6 +91,23 @@ struct btsnoop_opcode_index_info {
> >
> > #define BTSNOOP_PRIORITY_INFO 6
> > #define BTSNOOP_PRIORITY_DEBUG 7
> >
> > +struct btsnoop_hdr {
> > + uint8_t id[8]; /* Identification Pattern */
> > + uint32_t version; /* Version Number = 1 */
> > + uint32_t type; /* Datalink Type */
> > +} __attribute__ ((packed));
> > +#define BTSNOOP_HDR_SIZE (sizeof(struct btsnoop_hdr))
> > +
> > +struct btsnoop_pkt {
> > + uint32_t size; /* Original Length */
> > + uint32_t len; /* Included Length */
> > + uint32_t flags; /* Packet Flags */
> > + uint32_t drops; /* Cumulative Drops */
> > + uint64_t ts; /* Timestamp microseconds */
> > + uint8_t data[0]; /* Packet Data */
> > +} __attribute__ ((packed));
> > +#define BTSNOOP_PKT_SIZE (sizeof(struct btsnoop_pkt))
> > +
> >
> > struct btsnoop_opcode_user_logging {
> >
> > uint8_t priority;
> > uint8_t ident_len;
> >
> > diff --git a/tools/btsnoop.c b/tools/btsnoop.c
> > index 3eb8082dd..465938961 100644
> > --- a/tools/btsnoop.c
> > +++ b/tools/btsnoop.c
> > @@ -41,23 +41,6 @@
> >
> > #include "src/shared/btsnoop.h"
> >
> > -struct btsnoop_hdr {
> > - uint8_t id[8]; /* Identification Pattern */
> > - uint32_t version; /* Version Number = 1 */
> > - uint32_t type; /* Datalink Type */
> > -} __attribute__ ((packed));
> > -#define BTSNOOP_HDR_SIZE (sizeof(struct btsnoop_hdr))
> > -
> > -struct btsnoop_pkt {
> > - uint32_t size; /* Original Length */
> > - uint32_t len; /* Included Length */
> > - uint32_t flags; /* Packet Flags */
> > - uint32_t drops; /* Cumulative Drops */
> > - uint64_t ts; /* Timestamp microseconds */
> > - uint8_t data[0]; /* Packet Data */
> > -} __attribute__ ((packed));
> > -#define BTSNOOP_PKT_SIZE (sizeof(struct btsnoop_pkt))
> > -
> >
> > static const uint8_t btsnoop_id[] = { 0x62, 0x74, 0x73, 0x6e,
> >
> > 0x6f, 0x6f, 0x70, 0x00 };
> >
> > --
> > 2.14.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> > in the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
--
pozdrawiam
Szymon Janc
next prev parent reply other threads:[~2018-01-04 14:33 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-04 14:07 [PATCH] monitor: Add support for limiting btsnoop files size Szymon Janc
2018-01-04 14:23 ` Luiz Augusto von Dentz
2018-01-04 14:33 ` Szymon Janc [this message]
2018-01-05 14:47 ` Marcel Holtmann
2018-01-05 15:11 ` Szymon Janc
2018-01-05 15:23 ` Marcel Holtmann
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=1751555.yVb2H0TeuM@ix \
--to=szymon.janc@codecoup.pl \
--cc=linux-bluetooth@vger.kernel.org \
--cc=luiz.dentz@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.