Fix: Handle barectf 2 configurations with any configured byte order
Context:
In barectf 2 (v2), the ‘byte-order’ property allows users to configure
the byte order with which traces are written (completely independently
of the traced architecture’s native byte order).
In barectf 3 (v3), the v2 property ‘byte-order’ was removed and the v3
property ‘native-byte-order’ is used instead. Currently, barectf 3
requires that the configured trace byte order match the byte order of
the traced architecture (i.e. the native byte order). This new
requirement promotes efficient tracing as it allows memcpy() to be used
for trace writing. Conversely, writing the trace in a different byte
order as the traced architecture requires at least one additional CPU
operation (byte swap) per written data point.
Issue:
Because of this use of memcpy() in trace writing (see implementation
section below), barectf 3 does not correctly handle barectf 2
configurations where the tracer's configured byte order does not match
the system's native byte order.
For example, consider a barectf 2 configuration file configured to
produce traces with a little-endian byte order
(byte-order: little-endian). If a barectf 3 tracer generated with this
configuration is used on a big-endian system, the traces produced will
appear corrupted.
Because the barectf 3 tracer copies data as-is with memcpy() (without
checking or swapping byte order), the byte order of the trace data
produced will not match the expected byte order (described in the
trace's metadata file) so the trace reader (e.g. babeltrace) will not be
able to read it.
Implementation:
The difference between barectf 2 (v2.3.1) and 3 byte order behavior:
* barectf 2: Always uses the bt_bitfield_write functions when writing
trace data.
* barectf 3: Typically uses memcpy() when writing trace data (unless
the data is not byte-aligned or has a length which is not a multiple
of 8 bits)
In practice, the bt_bitfield_write functions are defined identically in
barectf 2 and barectf 3.
Note: Although, barectf 2 does define the bt_bitfield_write
functions with different type arguments depending on the configured
byte order, in practice, these functions are _always_ called with
the type unsigned char. So, there is no functional difference
between how bt_bitfield_write_le and bt_bitfield_write_be are called
regardless of the configured byte order. This means that we can get
the same barectf 2 byte order behaviour in barectf 3 without
adjusting the bt_bitfield_write functions based on the
configuration.
Changes:
Handle barectf 2 configurations with any configured byte order by adding
an optional property ‘trace-byte-order’ (equivalent of the barectf 2
‘byte-order’ property). When this property is set, the tracer will
write the trace data in the configured byte order regardless of the
traced architecture’s native byte order (like barectf 2).
These changes are transparent for existing users of the barectf Python
library and no changes to their code are required.
The configuration property ‘trace-byte-order’ is implemented with a new
trace type class ‘TraceTypeWithUnknownNativeByteOrder’ both to avoid
breaking changes to the existing ‘TraceType’ class and to encourage use
of the ‘TraceType’ class over the new trace type.
The ‘trace-byte-order’ property is only intended to allow compatibility
with barectf 2 configurations. Because tracers configurated with
‘trace-byte-order’ will always use the bt_bitfield_write functions,
intentional use of this property in barectf 3 is unrecommended as it
could result in a less efficient tracer if the C compiler used is not
good at optimizing.
Note: With a C compiler that optimizes well, the bt_bitfield_write
functions will optimize down to either:
a. 1 store instruction (equivalent to memcpy()), or
b. 1 byte swap and 1 store (if the config byte order is different from the architecture’s native byte order).
This raises the question: Why not just always use the
bt_bitfield_write functions if they optimize so well and can be
equivalent to a memcpy()? Because barectf is designed for use in
embedded situations where toolchains are highly varied and it is not
a given that the C compiler used is highly optimizing, it felt safer
to still use memcpy() when the architecture and configured trace
byte orders match as memcpy() is guaranteed to result in a quick
write operation.
Testing:
Add the following test cases:
* Configuration:
* Fail if no byte order property is defined (byte-order-no.yaml)
* Fail if both ‘native-byte-order’ and ‘trace-byte-order’ are
defined (byte-order-yes-two-properties.yaml)
* Pass if only ‘trace-byte-order’ is defined (byte-order-yes.yaml)
* Tracing:
* Barectf 2 configuration with ‘byte-order’ big endian (assuming a
little endian test system) (trace-byte-order-big-endian.yaml)
The configuration test files are adapted from the effective
configuration generated from the barectf test config two-packets.yaml.
For the tracing test, the configuration was adapted from the barectf 2
wiki and the test source was adapted from the barectf test source
two-packets.c.
Philippe: Document the user-facing changes in `trace-type-obj.adoc`.
Note: This is in preparation for adding additional passing
configurations. At the moment the only passing configuration is
config.yaml (tests/config/yaml/2/configs/pass/everything/config.yaml)
and this file is already tested via test_pass_everything.py.
tests: Fix: Exclude test include YAML files from tests
These partial YAML files are included as part of other test cases and
are not intended to be run as separate tests.
The tests run based on these partial files were still listed as passing
because currently the test system only checks that a test fails
configuration, but does not compare the reason for that failure with an
expected reason. Since all the included files are invalid partial
configurations, configuration generation would necessarily fail, and so
the test would "pass".
by giving them a specific file extension (inc.yaml).
Note: This is in preparation for filtering partial YAML files that are
not intended to be run as a separate test (e.g. because they will not
generate a valid configuration alone).
This fixes a dependency issue regarding Pylint (depending on a specific
version of astroid for Python 3.6, depending on wrapt 1.13 which makes
`poetry install` fail).
Signed-off-by: Philippe Proulx <email address hidden>
Change-Id: I987d2c6cb26647c261b270013449b4d8d3684fb7
Fix: Make traces with no data stream type ID readable by babeltrace
Currently, when the barectf configuration property
‘data-stream-type-id-field-type’ is false, barectf produces traces that
babeltrace (and Trace Compass) cannot parse.
To resolve this, remove all mentions of the data stream type ID from the
metadata file when there is no stream type ID field type feature.
Apparent cause:
When data stream type IDs are disabled, the barectf metadata file will
specify that the one data stream type has an ID of 0. This superfluous
information appears to be confusing to babeltrace even when the packet
header is explicitly empty (i.e. there is no mention of the stream type
ID in the actual trace data). This case falls into a grey area in the
CTF 1.8 specifcation (see below) and could arguably be a bug in
babeltrace since all the information needed to parse the trace
unambiguously is present in the metadata file. For example, the CTF
reader yactfr successfully reads such a trace. The stream type ID
information that barectf includes is superfluous, but not contradictory
of any data present in the trace.
However, add this fix because:
a. Two common CTF trace format readers (babeltrace and
Trace Compass) are unable to read these traces, and
b. It is an easy fix in barectf.
CTF 1.8 specification:
This metadata issue is in a grey area of the CTF specification. The
specification says, “[The data stream type ID] field is optional if
there is only one [data stream type] in the metadata” and, “The [data
stream type ID] can be left out if there is only one [data stream type]
in the trace.” However, it does not say the data stream type ID _must_
be left out of the metadata if there is only one data stream type.
Implementation:
The condition added to the metadata Jinja template simply checks that
‘data-stream-type-id-field-type’ is not None. This works when
‘data-stream-type-id-field-type’ is set to false in the configuration,
because the data stream type ID field type will be set to None when the
trace type is created in config_parse_v3.py:
dst_id_ft = self._feature_ft(...)
Testing:
The test does not check that babeltrace can read the trace, but it does
test that the lines mentioning the stream id are not present in the
generated metadata. Removing these lines has been manually verified to
be sufficient to allow babeltrace to read the trace.
The test source is adapted from the barectf test two-packets.c
In the original test, there was a space between '-' and '23' which is
invalid YAML syntax. However, this space was intentionally removed in
commit 688f203 (tests/config/fail/yaml/invalid.yaml: fix invalid YAML
(list/neg. value), May 2020)
It is not entirely clear what the intent of this test is, given the
space was intentionally removed, but it seems likely that the goal was
to test that when a YAML file contains invalid YAML syntax, the
configuration parsing will fail.
Without the space '-23', the test fails with this message:
Configuration: Cannot create configuration from YAML file
Configuration object:
`metadata` property:
`trace` property:
`byte-order` property: {'le': -23} is not of type 'string' (from schema
`config/2/config`)
With the space '- 23', the test fails with this message:
Configuration: Cannot create configuration from YAML file
YAML loader: Cannot load file: sequence entries are not allowed here in
"invalid.yaml", line 32, column 11
Replace space in '- 23' so that the test fails because of invalid YAML
syntax.
