From: Mathieu Desnoyers Date: Tue, 27 Sep 2022 19:07:24 +0000 (-0400) Subject: Fix: event notification capture error handling X-Git-Url: http://git.liburcu.org/?a=commitdiff_plain;ds=sidebyside;h=736f2dd654af78b4bce81cf8ba9579370a162a38;p=lttng-modules.git Fix: event notification capture error handling When the captured fields end up taking more than 512 bytes of space for the msgpack message, the notification append capture fails. Currently, this is handled by printing a WARN_ON_ONCE() on the console, and a printk "Error appending capture to notification" warning. Considering that this kind of error is very much legitimate, spamming the console with warnings is not the way we want to handle this. Rather than print a warning on the console, reset the msgpack writer position to skip the problematic captured field entirely when it is erroneous. Signed-off-by: Mathieu Desnoyers Change-Id: I4c98dc85266dd7af5e11bbd3d73ab5118c9e03af --- diff --git a/include/lttng/msgpack.h b/include/lttng/msgpack.h index f667db6c..0d19518d 100644 --- a/include/lttng/msgpack.h +++ b/include/lttng/msgpack.h @@ -58,4 +58,7 @@ int lttng_msgpack_begin_array( struct lttng_msgpack_writer *writer, size_t count); int lttng_msgpack_end_array(struct lttng_msgpack_writer *writer); +int lttng_msgpack_save_writer_pos(struct lttng_msgpack_writer *writer, uint8_t **pos); +int lttng_msgpack_restore_writer_pos(struct lttng_msgpack_writer *writer, uint8_t *pos); + #endif /* _LTTNG_KERNEL_MSGPACK_H */ diff --git a/src/lib/msgpack/msgpack.c b/src/lib/msgpack/msgpack.c index e7ff1e39..b9bac85f 100644 --- a/src/lib/msgpack/msgpack.c +++ b/src/lib/msgpack/msgpack.c @@ -567,6 +567,18 @@ end: return ret; } +int lttng_msgpack_save_writer_pos(struct lttng_msgpack_writer *writer, uint8_t **pos) +{ + *pos = writer->write_pos; + return 0; +} + +int lttng_msgpack_restore_writer_pos(struct lttng_msgpack_writer *writer, uint8_t *pos) +{ + writer->write_pos = pos; + return 0; +} + void lttng_msgpack_writer_init(struct lttng_msgpack_writer *writer, uint8_t *buffer, size_t size) { diff --git a/src/lttng-event-notifier-notification.c b/src/lttng-event-notifier-notification.c index ae35b1f4..b8f01395 100644 --- a/src/lttng-event-notifier-notification.c +++ b/src/lttng-event-notifier-notification.c @@ -46,25 +46,21 @@ int capture_enum(struct lttng_msgpack_writer *writer, */ ret = lttng_msgpack_begin_map(writer, 2); if (ret) { - WARN_ON_ONCE(1); goto end; } ret = lttng_msgpack_write_str(writer, "type"); if (ret) { - WARN_ON_ONCE(1); goto end; } ret = lttng_msgpack_write_str(writer, "enum"); if (ret) { - WARN_ON_ONCE(1); goto end; } ret = lttng_msgpack_write_str(writer, "value"); if (ret) { - WARN_ON_ONCE(1); goto end; } @@ -72,25 +68,20 @@ int capture_enum(struct lttng_msgpack_writer *writer, case LTTNG_INTERPRETER_TYPE_SIGNED_ENUM: ret = lttng_msgpack_write_signed_integer(writer, output->u.s); if (ret) { - WARN_ON_ONCE(1); goto end; } break; case LTTNG_INTERPRETER_TYPE_UNSIGNED_ENUM: ret = lttng_msgpack_write_signed_integer(writer, output->u.u); if (ret) { - WARN_ON_ONCE(1); goto end; } break; default: - WARN_ON(1); + WARN_ON_ONCE(1); } ret = lttng_msgpack_end_map(writer); - if (ret) - WARN_ON_ONCE(1); - end: return ret; } @@ -164,7 +155,7 @@ int64_t capture_sequence_element_signed(uint8_t *ptr, break; } default: - WARN_ON(1); + WARN_ON_ONCE(1); } return value; @@ -239,7 +230,7 @@ uint64_t capture_sequence_element_unsigned(uint8_t *ptr, break; } default: - WARN_ON(1); + WARN_ON_ONCE(1); } return value; @@ -256,7 +247,6 @@ int capture_sequence(struct lttng_msgpack_writer *writer, ret = lttng_msgpack_begin_array(writer, output->u.sequence.nr_elem); if (ret) { - WARN_ON_ONCE(1); goto end; } @@ -272,7 +262,9 @@ int capture_sequence(struct lttng_msgpack_writer *writer, break; default: /* Capture of array of non-integer are not supported. */ - WARN_ON(1); + WARN_ON_ONCE(1); + ret = -1; + goto end; } signedness = integer_type->signedness; for (i = 0; i < output->u.sequence.nr_elem; i++) { @@ -280,14 +272,12 @@ int capture_sequence(struct lttng_msgpack_writer *writer, ret = lttng_msgpack_write_signed_integer(writer, capture_sequence_element_signed(ptr, integer_type)); if (ret) { - WARN_ON_ONCE(1); goto end; } } else { ret = lttng_msgpack_write_unsigned_integer(writer, capture_sequence_element_unsigned(ptr, integer_type)); if (ret) { - WARN_ON_ONCE(1); goto end; } } @@ -299,15 +289,13 @@ int capture_sequence(struct lttng_msgpack_writer *writer, * take into account that the next element might be further * away. */ - WARN_ON(integer_type->alignment > integer_type->size); + WARN_ON_ONCE(integer_type->alignment > integer_type->size); /* Size is in number of bits. */ ptr += (integer_type->size / CHAR_BIT) ; } ret = lttng_msgpack_end_array(writer); - if (ret) - WARN_ON_ONCE(1); end: return ret; } @@ -323,17 +311,9 @@ int notification_append_capture( switch (output->type) { case LTTNG_INTERPRETER_TYPE_S64: ret = lttng_msgpack_write_signed_integer(writer, output->u.s); - if (ret) { - WARN_ON_ONCE(1); - goto end; - } break; case LTTNG_INTERPRETER_TYPE_U64: ret = lttng_msgpack_write_unsigned_integer(writer, output->u.u); - if (ret) { - WARN_ON_ONCE(1); - goto end; - } break; case LTTNG_INTERPRETER_TYPE_STRING: if (output->u.str.user) { @@ -341,31 +321,18 @@ int notification_append_capture( } else { ret = lttng_msgpack_write_str(writer, output->u.str.str); } - if (ret) { - WARN_ON_ONCE(1); - goto end; - } break; case LTTNG_INTERPRETER_TYPE_SEQUENCE: ret = capture_sequence(writer, output); - if (ret) { - WARN_ON_ONCE(1); - goto end; - } break; case LTTNG_INTERPRETER_TYPE_SIGNED_ENUM: case LTTNG_INTERPRETER_TYPE_UNSIGNED_ENUM: ret = capture_enum(writer, output); - if (ret) { - WARN_ON_ONCE(1); - goto end; - } break; default: ret = -1; - WARN_ON(1); + WARN_ON_ONCE(1); } -end: return ret; } @@ -373,11 +340,7 @@ static int notification_append_empty_capture( struct lttng_event_notifier_notification *notif) { - int ret = lttng_msgpack_write_nil(¬if->writer); - if (ret) - WARN_ON_ONCE(1); - - return ret; + return lttng_msgpack_write_nil(¬if->writer); } static @@ -395,7 +358,6 @@ int notification_init(struct lttng_event_notifier_notification *notif, ret = lttng_msgpack_begin_array(writer, event_notifier->priv->num_captures); if (ret) { - WARN_ON_ONCE(1); goto end; } @@ -486,14 +448,12 @@ void lttng_event_notifier_notification_send(struct lttng_kernel_event_notifier * struct lttng_kernel_notification_ctx *notif_ctx) { struct lttng_event_notifier_notification notif = { 0 }; - int ret; if (unlikely(!READ_ONCE(event_notifier->parent.enabled))) return; - ret = notification_init(¬if, event_notifier); - if (ret) { - WARN_ON_ONCE(1); + if (notification_init(¬if, event_notifier)) { + record_error(event_notifier); goto end; } @@ -509,15 +469,23 @@ void lttng_event_notifier_notification_send(struct lttng_kernel_event_notifier * list_for_each_entry_rcu(capture_bc_runtime, &event_notifier->priv->capture_bytecode_runtime_head, node) { struct lttng_interpreter_output output; + uint8_t *save_pos; + int ret; + lttng_msgpack_save_writer_pos(¬if.writer, &save_pos); if (capture_bc_runtime->interpreter_func(capture_bc_runtime, stack_data, probe_ctx, &output) == LTTNG_KERNEL_BYTECODE_INTERPRETER_OK) ret = notification_append_capture(¬if, &output); else ret = notification_append_empty_capture(¬if); - - if (ret) - printk(KERN_WARNING "Error appending capture to notification"); + if (ret) { + /* + * On append capture error, skip the field + * capture by restoring the msgpack writer + * position. + */ + lttng_msgpack_restore_writer_pos(¬if.writer, save_pos); + } } }