Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

google_iot_device: Make it run with BOARD=qemu_x86 #2

Open
pfalcon opened this issue Aug 15, 2019 · 3 comments
Open

google_iot_device: Make it run with BOARD=qemu_x86 #2

pfalcon opened this issue Aug 15, 2019 · 3 comments
Labels
bug Something isn't working

Comments

@pfalcon
Copy link

pfalcon commented Aug 15, 2019

From https://github.com/atigyi/zephyr/blob/google_iot_device_sdk_integration/samples/net/google_iot_device/README.rst:

Only tested on native_posix board. This is where Google IoT looks for value from Zephyr community: the board support.

Indeed, "native_posix" is quite a peculiar "board". It's not "native Zephyr", it's a simulation of some of Zephyr functionality. Quite leaky simulation also, so if you're not careful, you can easily pick up some functions from the underlying POSIX (e.g., Linux) system, which aren't actually available in Zephyr.

This is actually what happened in the previous branch I worked with (https://github.com/galak/iot-device-sdk-embedded-c/tree/zephyr), and I fixed it to run on a "real" Zephyr (still a qemu one). My work on that is in https://github.com/pfalcon/iot-device-sdk-embedded-c

Proposed plan: I'll review this repo's branch regarding that situation, and if needed, will prepare a PR with changes required for qemu_x86.

@pfalcon pfalcon added the bug Something isn't working label Aug 15, 2019
atigyi pushed a commit that referenced this issue Aug 29, 2019
Currently, the free block bitmap is roughly 4 times larger than it
needs to, wasting memory.

Let's assume maxsz = 128, minsz = 8 and n_max = 40.

Z_MPOOL_LVLS(128, 8) returns 3. The block size for level #0 is 128,
the block size for level #1 is 128/4 = 32, and the block size for
level #2 is 32/4 = 8. Hence levels 0, 1, and 2 for a total of 3 levels.
So far so good.

Now let's look at Z_MPOOL_LBIT_WORDS(). We get:

Z_MPOOL_LBIT_WORDS_UNCLAMPED(40, 0) = ((40 << 0) + 31) / 32 = 2
Z_MPOOL_LBIT_WORDS_UNCLAMPED(40, 1) = ((40 << 2) + 31) / 32 = 5
Z_MPOOL_LBIT_WORDS_UNCLAMPED(40, 2) = ((40 << 4) + 31) / 32 = 20

None of those are < 2 so Z_MPOOL_LBIT_WORDS() takes the results from
Z_MPOOL_LBIT_WORDS_UNCLAMPED().

Finally, let's look at _MPOOL_BITS_SIZE(. It sums all possible levels
with Z_MPOOL_LBIT_BYTES() which is:

  #define Z_MPOOL_LBIT_BYTES(maxsz, minsz, l, n_max)    \
        (Z_MPOOL_LVLS((maxsz), (minsz)) >= (l) ?        \
         4 * Z_MPOOL_LBIT_WORDS((n_max), l) : 0)

Or given what we already have:

Z_MPOOL_LBIT_BYTES(128, 8, 0, 40) = (3 >= 0) ? 4 * 2  : 0 = 8
Z_MPOOL_LBIT_BYTES(128, 8, 1, 40) = (3 >= 1) ? 4 * 5  : 0 = 20
Z_MPOOL_LBIT_BYTES(128, 8, 2, 40) = (3 >= 2) ? 4 * 20 : 0 = 80
Z_MPOOL_LBIT_BYTES(128, 8, 3, 40) = (3 >= 3) ? 4 * ??

Wait... we're missing this one:

Z_MPOOL_LBIT_WORDS_UNCLAMPED(40, 3) = ((40 << 6) + 31) / 32 = 80

then:

Z_MPOOL_LBIT_BYTES(128, 8, 3, 40) = (3 >= 3) ? 4 * 80 : 0 = 320

Further levels yeld (3 >= 4), (3 >= 5), etc. so they're all false and
produce 0.

So this means that we're statically allocating 428 bytes to the bitmap
when clearly only the first 3 Z_MPOOL_LBIT_BYTES() results for the
corresponding 3 levels that we have should be summed e.g. only
108 bytes.

Here the code logic gets confused between level numbers and the number
levels, hence the extra allocation which happens to be exponential.

Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
@pfalcon
Copy link
Author

pfalcon commented Sep 4, 2019

Some updates:

I found that bsp/iotc_bsp_io_net_zephyr.c may call poll() with uninitialized memory. This seems to pass "harmless" with a host system (Linux with native_posix), but leads to error with Zephyr. Maybe we'll need to fix that ;-), later, but so far I have a draft fix for bsp/iotc_bsp_io_net_zephyr.c, needs cleanup. Beyond that, integration of mbedTLS entropy subsystem with Zephyr entropy subsystem. So far, I just recollected the essence of the issue and forward-ported a workaround from my previous port. It will need to be fixed properly. With that, I finally proceed to actual connection, which fails. Certainly, it's a matter of lack of real time source in Zephyr. I had a kind of solution in my old port, hopefully will have time to work to improve it to a fully automagic state.

@pfalcon
Copy link
Author

pfalcon commented Sep 6, 2019

Ok, so with #8 and #9 paving the road to run on native Zephyr targets (thanks for merging), I squashed some issues and forward-ported my older changes, and got the sample app working on BOARD=qemu_x86 !

But I should say right away that there're a lot (well, relatively) clean up to do before we could say "ship it". I mean, if following the schedule "any changes will looks like belonging on Zephyr side, should be done on Zephyr side (instead of adhocly worked around on the SDK port side)". And that's the schedule I'm trying to follow.

Note that I'll be less available next few weeks. Before that, I hope to push current WIP changes in my branch, https://github.com/pfalcon/zephyr/commits/google_iot_device_sdk_integration-pfalcon , file more TODO tickets here, and send an interim progress report via email, so we could plan for next steps.

@pfalcon
Copy link
Author

pfalcon commented Sep 12, 2019

Ok, so changes on samples/net/google_iot_device side, required to get it running on qemu_x86, is in my branch: https://github.com/pfalcon/zephyr/commits/google_iot_device_sdk_integration-pfalcon .

Those are not complete however, because they should be accompanied with changes on Zephyr side. I'm currently working in upstream to get them there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant