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

Add shared library for sniffing message queue traffic #461

Merged
merged 4 commits into from
Aug 17, 2022

Conversation

krakowski
Copy link
Contributor

I noticed that sometimes IPC messages were not received in time because another process received them before ipc_multiplexer could (race condition). Therefore I implemented a small shared library, which overrides the mq_receive function in such a way, that all messages are also forwarded to a special ipc_sniff queue, which can later be processed by my previous (modified) ipc_notify program from #451 .

For this to work dispatch needs to be started with LD_PRELOAD=/tmp/sd/yi-hack/lib/ipc_sniff.so prepended.

LD_PRELOAD=/tmp/sd/yi-hack/lib/ipc_sniff.so /home/app/dispatch

After this step the ipc_notify program can be started with a custom script.

ipc_notify -c "/bin/ash /tmp/sd/yi-hack/script/echo_event.sh"

echo_event.sh

#!/bin/ash

echo "${1}"

Please note that the parameter to the script is no longer the event name (like in #451) but the raw hex string (e.g 010000000800000081000100000000005). This means, the script needs to parse the hex message. This way the binary does not have to be recompiled once someone discovers a new type of message.

@roleoroleo
Copy link
Owner

This approach is very clever and I think that it's enough to ensure that all messagges are received by the sniffer.
But the race condition between dispatch and ipc_multiplexer is not solved.
I think I should use your solution inside ipc_multiplexer to read from /ipc_dispatch and write to /ipc_dispatch_x
In this manner, I could fix all race conditions.

@krakowski
Copy link
Contributor Author

Hi @roleoroleo,

I could rewrite the ipc_notify program, so that it would accept a queue name as a parameter. This way ipc_multiplexer could subscribe to the ipc_sniff queue (and receive everything) and forward the messages to ipc_dispatch_x, which ipc_notify could subscribe to instead. What do you think?

@roleoroleo
Copy link
Owner

Or we could multiplex to ipc_dispatch_x directly in your lib and remove ipc_multiplexer.

There is a "problem" with the new mq_receive: dispatch process opens a lot of queues (/ipc_dispatch, /ipc_rmm, /ipc_cloud, etc...) and receives from /ipc_dispatch and /ipc_dispatch_worker
So /ipc_sniff contains messages from these queues. Is it possible to filter mq_receive to receive only from /ipc_dispatch?

@krakowski
Copy link
Contributor Author

I think this should be possible. Every message seems to have a prefix (at least two bytes) indicating its target. ipc_dispatch seems to use 0x01, so a simple AND should do the trick here for filtering. Are other messages not relevant (for example ipc_dispatch_worker)? ipc_rmm, for example, receives a message for every snapshot taken:

02 00 00 00 04 00 00 00 07 10 01 00 0f 00 00 00 2f 74 6d 70 2f 6d 6f 74 69 6f 6e 2e 6a 70 67
                                                 /  t  m  p  /  m  o  t  i  o  n  .  j  p  g

Integrating the librarie's code into ipc_multiplexer should be a good solution. Instead of hardcoding all message types, I would prefer to create some kind of config file which gets read and parsed into a lookup table at startup, so that the user can add new message types once they are identified. Maybe something like the following text file:

007C=MOTION_START
007D=MOTION_STOP

I could try implementing this if it looks good enough for you.

@roleoroleo
Copy link
Owner

think this should be possible. Every message seems to have a prefix (at least two bytes) indicating its target. ipc_dispatch seems to use 0x01, so a simple AND should do the trick here for filtering. Are other messages not relevant (for example ipc_dispatch_worker)? ipc_rmm, for example, receives a message for every snapshot taken:

Yes, check this repo about MID_* defines: https://github.com/roleoroleo/cmd_srv/blob/master/cmd_srv.h

I could try implementing this if it looks good enough for you.

Yes, please.
No problem to change ipc_multiplexer - mqttv4 chain.

@krakowski krakowski force-pushed the feature/ipc-notify branch from 40dd664 to f703517 Compare July 20, 2022 11:47
@krakowski
Copy link
Contributor Author

Hi,

I implemented the mentioned changes. Using a lookup table failed, because the message type is an unsigned short which would result in a huge lookup table. I switched to a simple array and a binary search for finding the message name belonging to a specific message id. The mappings can be adjusted inside /tmp/sd/yi-hack/config/ipc_multiplex.conf. The intercepting code now also only forwards messages belonging to the ipc_dispatch queue. I also adjusted the init scripts for each model so that the dispatch process gets invoked with LD_PRELOAD.

The MQTT process should receive the same messages as before on ipc_dispatch_1. I also included a warning in the config file not to change the names of messages processed by the MQTT process.

Unfortunately, I won't have time until end of next week to try it out on a device. If you have time and a spare device, you could try it out and leave some feedback 🙂

@roleoroleo
Copy link
Owner

roleoroleo commented Jul 22, 2022

Thank you.
I will try it on Sunday.

EDIT

In the meantime I have looked at the code and it is ok.
What do you think about simplifying and removing the mapping table? I could modify mqttv4 to receive the binary messages and not the mapped text messages. It would be much simpler and more standard.

@krakowski krakowski force-pushed the feature/ipc-notify branch from f0de989 to d44c941 Compare July 28, 2022 11:18
@krakowski
Copy link
Contributor Author

What do you think about simplifying and removing the mapping table? I could modify mqttv4 to receive the binary messages and not the mapped text messages. It would be much simpler and more standard.

I updated the code for this scenario. Now, the injected shared library just resends the buffer without modifying anything. The receiving side is now responsible for parsing/interpreting the message. I tried it out on a real device, and it seems to work quite well, so far.

@roleoroleo
Copy link
Owner

roleoroleo commented Jul 28, 2022

Thank you very much for your code.
Sorry, but I will be away from pc for some time.
I will update my code in a couple of weeks.

@roleoroleo roleoroleo merged commit 5fba9e1 into roleoroleo:master Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants