gril: per message hex dump #151
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In function `received_data` in gril/grilio.c, when we have a ring_buffer of size 8192, and its in position mark at 8192, out position mark at 8188, ring_buffer_avail_no_wrap will always return zero because it just has to wrap to write any new data. So, the function received_data in gril/grilio.c will always have toread = 0 and never go through do-while loop body, and jump over all the following if-conditions because none of their conditions are evaluated to true. function received_data will then return true and be dispatched again by gio almost immediately. This patch tries to fill ring buffer space without wrap _and_ then tries to fill space with wrap.
struct ril_s::read_so_far is never assigned a non-zero value outside the while-loop in function `new_bytes` in gril/gril.c.
GRil reads incoming bytes into ril_msg structure in function `read_fixed_record` in gril/gril.c. It's mostly working except it makes some assumptions that are not always true. 1. The `read_fixed_record` function calls `memmove` to copy data from ring_buffer, which follows that the whole parcel must not be wrapped. This is usually true because whenever a parcel is read, the ring_buffer rewind its in/out position markers to the beginning. However, if somehow rild transmits massive amount of parcels, or due to various reasons that the gio thread is not waked up in time and a lot of incoming data queued in kernel, then a parcel that spans across the boundary becomes very likely. This is actually the question raised in the comment: /* TODO: need to better understand how wrap works! */ With https://bugs.launchpad.net/ubuntu/+source/ofono/+bug/1408228, the GrilIO part is immune from this problem but the `read_fixed_record` is still affected. 2. One should really avoid using ring_buffer APIs that involve its internal states. Ring buffer wrapping should not be a concern to other components whenever possible. This patch rewrites function new_bytes and read_fixed_record so that we only utilize two ring_buffer APIs ring_buffer_read and ring_buffer_len to do all the job. Buffer wrapping is no longer a problem for incoming parcel dispatching.
…il-new-bytes' into HEAD
With OFONO_RIL_HEX_TRACE set and exported, gril begins to dump raw stream in hex. However, one such trace dump may contain several RIL messages or even an incomplete one. This patch moves such dump from grilio to gril so that hex dumps are only printed after being parsed as a complete RIL message.
NotKit
referenced
this pull request
in ubports/ofono
Mar 10, 2019
Request for pulling LTE configuration bug fix to main branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With OFONO_RIL_HEX_TRACE set and exported, gril begins to dump raw stream in hex. However, one such trace dump may contain several RIL messages or even an incomplete one. This patch moves such dump from grilio to gril so that hex dumps are only printed after being parsed as a complete RIL message.
This is for https://bugs.launchpad.net/ubuntu/+source/ofono/+bug/1331183 .