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

[gec] fix compilation of secure telemetry module gec_dl #2646

Merged
merged 8 commits into from
Jan 16, 2021

Conversation

gautierhattenberger
Copy link
Member

It compiles, but I'm not 100% sure of the modifications since it is not working, unless it is only a configuration error.

@gautierhattenberger
Copy link
Member Author

also add the support of RNG to ChibiOS arch

@podhrmic
Copy link
Member

OK, I got it working in NPS! Please make sure you are following the instructions on the wiki.

TLDR;

The main trick I had to try was to stop and redo the secure link process, after all the other processes started. Also, it takes a few seconds to get the key exchange to finish (not sure what is the holdup. If that doesn't help, reset the Simulator and then the secure link.

More info

After fetching my commit, please do make clean; make to get rid of old Cargo.lock files.

I am using Minion RP 3 airframe with these changes (you probably just need to change telemetry to transparent_gec):

paparazzi$ git diff
diff --git a/conf/airframes/AGGIEAIR/aggieair_conf.xml b/conf/airframes/AGGIEAIR/aggieair_conf.xml
index 0cf01fafe..14af7a597 100644
--- a/conf/airframes/AGGIEAIR/aggieair_conf.xml
+++ b/conf/airframes/AGGIEAIR/aggieair_conf.xml
@@ -95,7 +95,7 @@
    telemetry="telemetry/AGGIEAIR/aggieair_fixedwing.xml"
    flight_plan="flight_plans/AGGIEAIR/BasicTuning_Launcher.xml"
    settings="settings/fixedwing_basic.xml settings/nps.xml"
-   settings_modules="modules/battery_monitor.xml modules/lidar_sf11.xml modules/nav_skid_landing.xml modules/nav_survey_poly_osam.xml modules/gps.xml modules/nav_basic_fw.xml modules/guidance_basic_fw.xml modules/stabilization_attitude_fw.xml"
+   settings_modules="modules/battery_monitor.xml modules/gps.xml modules/guidance_basic_fw.xml modules/lidar_sf11.xml modules/nav_basic_fw.xml modules/nav_skid_landing.xml modules/nav_survey_poly_osam.xml modules/stabilization_attitude_fw.xml"
    gui_color="#00009e93ffff"
   />
   <aircraft
diff --git a/conf/airframes/AGGIEAIR/aggieair_minion_rp3_lia.xml b/conf/airframes/AGGIEAIR/aggieair_minion_rp3_lia.xml
index a93fdc4ec..f704ef748 100644
--- a/conf/airframes/AGGIEAIR/aggieair_minion_rp3_lia.xml
+++ b/conf/airframes/AGGIEAIR/aggieair_minion_rp3_lia.xml
@@ -39,7 +39,7 @@ AggieAir RP3 Minion
     <module name="control"/>
     <module name="navigation"/>
 
-    <module name="telemetry"   type="transparent">
+    <module name="telemetry"   type="transparent_gec">
       <configure name="MODEM_PORT"        value="UART3"/>
       <configure name="MODEM_BAUD"        value="B57600"/>
     </module>

I tested with both Pprzlink 1 and pprzlink 2

You will see the following:

     Running `sw/ext/rustlink/target/release/rustlink --udp -a 1 -n Minion_RP3 -c -v 2.0`
Value for ivy_bus: 127.255.255.255:2010
Value for ping_period: 1000
Value for status_period: 5000
Value for baudrate: 9600
Value for port: "/dev/ttyUSB0"
Use UDP: true
Use UDP broadcast: false
Value for remote_addr: 127.0.0.1
Value for pprzlink version: v2.0
Global message class is set to Telemetry
Sender id: ground_dl
Link name: 
Value for aircraft name: Some("Minion_RP3")
Value for aircraft ID: 1
Enable GEC: true
Opening /home/mpodhradsky/Workspace/Paparazzi/paparazzi/var/aircrafts/Minion_RP3/ap/generated/keys_gcs.h
Opening /home/mpodhradsky/Workspace/Paparazzi/paparazzi/var/aircrafts/Minion_RP3/nps/generated/keys_gcs.h
Starting encrypted datalink...
returning msg1, waiting msg2
message successfully decrypted
processed msg2
returning msg3, going ok

Screenshot from 2021-01-14 09-31-30

@podhrmic
Copy link
Member

The rust warnings are annoying, but apparently they don't affect the functionality.

@gautierhattenberger
Copy link
Member Author

@podhrmic I removed the mutex as I think it is not relevant here and makes conflict with uart mutex lock/unlock sequence.
It works with NPS, however with real board, it stops with stack overflow. It seems that the algo is consuming a lot of resources. Any idea what would be the correct setting for the AP stack ?

@gautierhattenberger
Copy link
Member Author

another question: is the hacl-c lib reentrant or not ? If no, I guess that explains the usage of the mutex...
but still, their is an issue with the lock sequence where we have:

  • uart lock (with check space)
  • gec lock (with start msg)
  • uart unlock (with send msg)
  • gec unlock (after send msg)

we should move one of the lock or unlock

@podhrmic
Copy link
Member

@gautierhattenberger I think changing the mutex sequence to:

  1. uart lock (with check space)
  2. gec lock (with start msg)
  3. gec unlock (after send msg)
  4. uart unlock (with send msg)

makes more sense. Not sure about re-entrancy, but I suspect hacl-c is not reentrant.

About the resources - is there an easy way to measure stack usage and/or increase stack size?

@gautierhattenberger
Copy link
Member Author

I have recently integrated a shell module and I just repaired the free_stack function, I will try that

@podhrmic
Copy link
Member

I don't see any huge array in https://github.com/paparazzi/hacl-c - but it is quite possible that all the intermediate computations take some stack space.

@gautierhattenberger
Copy link
Member Author

I don't see any huge array in https://github.com/paparazzi/hacl-c - but it is quite possible that all the intermediate computations take some stack space.

see https://github.com/paparazzi/hacl-c/blob/master/Hacl_SHA2_512.c#L316

@podhrmic
Copy link
Member

I guess that would do if the stack is small enough ( 169*64/8=~1kB buffer)

thread stack should have around 2 Kb free
both AP and FBW in case of fixedwing
@podhrmic
Copy link
Member

@gautierhattenberger Did you have to increase the stack size as well?

@gautierhattenberger
Copy link
Member Author

Yes, it needs at least 2kB free, so AP (FW) and Main (Rotorcraft) are usually fine, but FBW needs to be increased when using this module. I don't want to have this hard-coded, so I guess it should be specified in the documentation.

@podhrmic
Copy link
Member

Are you concerned about older boards with smaller memory? I am assuming that would be the downside of having a larger default stack size.

@gautierhattenberger
Copy link
Member Author

Basically, yes, I'd rather not change default settings. If more people are using this and it becomes a problem, we can reconsider this and adjust default stack size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants