pkg: ndn-riot: initial import of ndn-riot as a package #5914

cgundogan · 2016-10-06T14:51:42Z

This PR imports ndn-riot [1] as a pkg.
Furthermore, I included a tests application that I adopted from [2].

@cawka I hope you don't mind that I included your changes proposed in #5648.

Can be tested by navigating into tests/ndn-ping and invoke the usual make all term PORT=tap0/1

[1] https://github.com/named-data-iot/ndn-riot
[2] https://github.com/named-data-iot/ndn-riot-examples/tree/master/ndn-ping

mfrey · 2016-10-06T15:04:27Z

Build fails for me with (I'm a bit puzzled since the next statement is a assert using the variable)

/home/frey/Desktop/Projekte/RIOT/tests/ndn-ping/bin/pkg/native/ndn-riot/pit.c: 
In function ‘ndn_pit_add’:
/home/frey/Desktop/Projekte/RIOT/tests/ndn-ping/bin/pkg/native/ndn-riot/pit.c:104:13: 
error: unused variable ‘r’ [-Werror=unused-variable]
         int r = ndn_interest_get_name(&entry->shared_pi->block, &pn);

OlegHahm · 2016-10-07T09:26:19Z

Looking forward to test this.

OlegHahm · 2016-10-07T09:31:39Z

@mfrey, maybe you're building (the package) with NDEBUG set?

mfrey · 2016-10-07T09:41:28Z

@OlegHahm I haven't set anything actually. The variable is only used in the assert statement. If you don't build with NEDBUG this is an issue nontheless. Isn't it? Anyway, @cgundogan fixed it by a cast to void. I'll give it a try later.

OlegHahm · 2016-10-07T10:05:02Z

The variable is only used in the assert statement. If you don't build with NEDBUG this is an issue nontheless. Isn't it?

Sure. Just wanted to find an explanation.

wentaoshang · 2016-10-07T15:34:18Z

There is also a unittest suite for encoding & decoding APIs that has not been merged into ndn-riot repo yet:

https://github.com/wentaoshang/RIOT/tree/ndn/tests/unittests/tests-ndn_encoding

I can try to make a separate PR for this if you think it can be included.

cgundogan · 2016-10-07T15:45:20Z

if you think it can be included.

nice, definitely!

OlegHahm · 2016-10-13T22:00:08Z

Doesn't ndn-riot require malloc/free? In that case, the application should use the tlsf package.

OlegHahm · 2016-10-13T22:08:15Z

Until #5943 is fixed, the applications should also blacklist boards without hwrng.

OlegHahm · 2016-10-13T22:10:23Z

@cgundogan, with named-data-iot/ndn-riot#2 being merged, could you please update the revision of the package?

cgundogan · 2016-10-19T06:02:31Z

@OlegHahm sure, sorry for the delay. I amended the new commit hash to point to current master of https://github.com/named-data-iot/

wentaoshang · 2016-11-04T16:47:00Z

Sorry I lost track of this issue. Wonder what is the current status? Anything needed to be fixed from NDN-RIOT side? Honestly I'm not very familiar with the Github collaboration procedure but I can try my best to help.

Also I have some concern about the requirement for hardware RNG for micro-ecc, because the two platforms (SAMR21 and IoT-Lab M3) I'm using for NDN-RIOT right now do not have this feature. What I did is to patch micro-ecc (named-data-iot@8b9f0b1) and use deterministic ECDSA signing with pre-generated keys (RFC6979). Any comment on this idea?

cgundogan · 2016-11-29T17:31:23Z

@OlegHahm ping?

@wentaoshang I have no experience with the micro-ecc package, but the diff in your commit (named-data-iot@8b9f0b1) is not very apparent to me. What exactly did you change?

tfar · 2016-11-29T17:42:26Z

@wentaoshang so you're using your own deterministic ECDSA implementation or is the one you're using part of MicroECC? The patch you're referencing, named-data-iot/RIOT@8b9f0b1 , removes the RIOT hardware random generator integration. Does MicroECC behave differently based on whether g_rng_function is defined or not; or why was it removed? I think RIOT's hwrng_read is always available, regardless of hardware support.

wentaoshang · 2016-11-29T19:49:37Z

Sorry I forgot to mention this: I'm using a signing function called uECC_sign_deterministic in MicroECC https://github.com/kmackay/micro-ecc/blob/master/uECC.c#L1377 . For that function to work, g_rng_function has to be unset (hence the removal of old patch 0002). The side effect is that it'll turn off hardware RNG for all platforms, which is bad. I tried to figure out some preprocessor tricks to fix that but failed...

tfar · 2016-11-29T20:00:03Z

@wentaoshang So instead of removing the default implementation, can't you call void uECC_set_rng(uECC_RNG_Function rng_function) at https://github.com/kmackay/micro-ecc/blob/master/uECC.c#L190 in your user code to reset the random number generator callback?

wentaoshang · 2016-11-29T20:04:50Z

That could also work... Ideally I want to be able to switch automatically between deterministic/nondeterministic signing (maybe at compile time?), based on whether HWRNG is available. But what you proposed sounds a good solution for now.

tfar · 2016-11-29T20:09:50Z

The setter/getter functions for the random generator callback are part of the public API https://github.com/kmackay/micro-ecc/blob/master/uECC.h#L127 . So it seems what you want to achieve is possible without completely removing support for hardware random number generator in MciroECC.

wentaoshang · 2016-11-29T20:41:06Z

I agree. I'll try to figure out how to update ndn-riot code. I remember that without named-data-iot/RIOT@8b9f0b1 the code could not compile for platforms with no HWRNG because hwrng_read was not defined. Not sure if the problem has been fixed in more recent releases.

kaspar030 · 2018-05-23T12:01:05Z

I can't reproduce the error of Murdock, which is weird (it's not a compilation error, but a directory that is missing). Any idea to help me ? @kaspar030 ?

Try to do build the package in "pkg/ndn-riot/Makefile" (instead of adding "DIRS += BINDIRBASE/..." in "pkg/ndn-riot/Makefile.include". See how other packages do it, usually sth like "$(MAKE)" -C $(PKG_BUILDDIR) edit: in the "all:" target.

astralien3000 · 2018-05-23T13:03:49Z

Thank you @kaspar030, the problem was indeed the missing rule for compiling the package. I still don't understand why it works on my computer an not for Murdock, but well, at least it is fixed.

aabadie

@astralien3000, can you squash ? The last commits should be squashed to the first one of this PR, IIUC.

Wrong option selected.

cgundogan · 2018-05-23T13:33:27Z

pkg/ndn-riot/Makefile

@@ -0,0 +1,11 @@
+PKG_NAME=ndn-riot
+PKG_URL=https://github.com/astralien3000/ndn-riot


What is the reason not to take the official repo's url?

New errors appeared since the last time I tried the code. I will need to do a pull request to the official repos, but for the moment, I try with mine. Of course this should not be merged for the moment.

Just in case, I can quickly merge pull requests in named-data-iot/ndn-riot repo

astralien3000 · 2018-05-23T14:18:14Z

@kaspar030, well actually adding what you said did not help. I don't know why it worked once, but I got the same error again for 3 builds of Murdock... Maybe it's a race condition like @gebart said ? But I can't reproduce the error even with -j9...

kaspar030 · 2018-05-23T14:33:05Z

pkg/ndn-riot/Makefile.include

@@ -0,0 +1,3 @@
+INCLUDES += -I$(BINDIRBASE)/pkg/$(BOARD)
+DIRS += $(BINDIRBASE)/pkg/$(BOARD)/ndn-riot


this line needs to go

kaspar030 · 2018-05-23T15:07:21Z

pkg/ndn-riot/Makefile.include

-INCLUDES += -I$(BINDIRBASE)
+is this file taken in account ????
+
+INCLUDES += -I$(PKGDIRBASE)


Ah, I think I see what is going on. You've been using $BINDIRBASE/pkg/$(BOARD), which locally is the same as $PKGDIRBASE. But murdock overrides $BINDIR and $PKGDIRBASE (but not $BINDIRBASE, thus the wrong path is set.

I wonder why this didn't pop up earlier. ;)

So, INCLUDES += -I$(PKGDIRBASE) (as it is now) should work.

astralien3000 · 2018-05-23T17:21:04Z

Ok ! Almost there ! I just need to wait for the PR of ndn-riot, and we can move. Maybe tomorrow.

astralien3000 · 2018-05-24T08:25:34Z

Woo ! Murdock is ok (taking apart the static test, because squash is needed). I rolled back to the official ndn-riot repos. Everything is ready from my side, may I squash ? Also, someone else should test the last changes I made.

astralien3000 · 2018-05-24T12:09:00Z

@cgundogan, @mfrey, can one of you test ndn-ping ?

adjih · 2018-05-24T14:52:42Z

Just tested with samr21-xpro: it is working fine with client & server on two different boards.

aabadie · 2018-05-24T15:00:11Z

sys/include/net/ethertype.h

@@ -34,7 +34,8 @@ extern "C" {
 #define ETHERTYPE_RESERVED      (0x0000)    /**< Reserved */
 #define ETHERTYPE_IPV4          (0x0800)    /**< Internet protocol version 4 */
 #define ETHERTYPE_ARP           (0x0806)    /**< Address resolution protocol */
-#define ETHERTYPE_NDN           (0x0801)    /**< Parc CCNX */


Could this change have an impact on other named data network (CCN-lite) implementations ?

I don't think so. CCN-lite for RIOT uses only NDN for now (-> it does not support the other formats of CCN-lite).

Actually, this should be changed to

#define ETHERTYPE_NDN 0x8624

This is the value we are using in other places. This, probably, was just a quick hack.

I'm slightly confused by the PARC CCNX comment.

I just tested the ccn-lite-relay example, it behave the same as on master.

Actually this line is the one deleted, ETHERTYPE_NDN is defined as 0x8624 in this PR (see this). But ETHERTYPE_CCNX is still defined as 0x0801. What do we do about these lines ? They are ok for me.

According to history, Oleg added an ETHERTYPE_NDN in 2015, and then Cenk did a commit in 2016 "ethertype: use correct type for NDN and CCNx", that introduced another name for ETHERTYPE_CCNX and a new proper value for ETHERTYPE_NDN.

I think this reflects properly the fact that NDN and CCNx became different protocols.

@OlegHahm

@OlegHahm is ok

astralien3000

No further question from anyone. ACK

cgundogan added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: pkg Area: External package ports labels Oct 6, 2016

cgundogan assigned mfrey, OlegHahm and wentaoshang Oct 6, 2016

cgundogan mentioned this pull request Oct 6, 2016

pit: use unused variable r named-data-iot/ndn-riot#2

Merged

OlegHahm mentioned this pull request Oct 13, 2016

pkg: micro-ecc: missing dependency to periph/random #5943

Closed

cgundogan force-pushed the pr/ndn-riot_as_a_pkg branch 2 times, most recently from a4da9bb to 8ea42ac Compare October 19, 2016 06:01

cgundogan force-pushed the pr/ndn-riot_as_a_pkg branch from 8ea42ac to f27c93b Compare November 29, 2016 17:25

wentaoshang mentioned this pull request Dec 13, 2016

pkg/micro-ecc: support boards without hwrng feature #6204

Merged

aabadie previously requested changes May 23, 2018

View reviewed changes

cgundogan commented May 23, 2018

View reviewed changes

astralien3000 force-pushed the pr/ndn-riot_as_a_pkg branch from 04a2500 to 10cb7a7 Compare May 23, 2018 13:35

kaspar030 reviewed May 23, 2018

View reviewed changes

astralien3000 mentioned this pull request May 23, 2018

Adding non-blocking function to ndn_app API named-data-iot/ndn-riot#5

Open

astralien3000 force-pushed the pr/ndn-riot_as_a_pkg branch from 72b39dd to e40633e Compare May 23, 2018 17:18

Cenk Gündoğan added 6 commits May 24, 2018 10:58

pkg: ndn-riot: initial import of ndn-riot pkg

39ccd64

auto_init: add ndn-riot

1fb4789

ethertype: use correct type for NDN and CCNx

05d4c23

gnrc: add nettype handling for NDN

d005566

gnrc_pktdump: add hex dump for NDN

f64265d

examples: ndn-riot: import of ndn-ping application

dbd049a

astralien3000 force-pushed the pr/ndn-riot_as_a_pkg branch from 55327d1 to dbd049a Compare May 24, 2018 09:18

aabadie reviewed May 24, 2018

View reviewed changes

astralien3000 approved these changes May 24, 2018

View reviewed changes

astralien3000 merged commit 260bc37 into RIOT-OS:master May 24, 2018

cgundogan deleted the pr/ndn-riot_as_a_pkg branch May 24, 2018 16:29

cladmi modified the milestone: Release 2018.07 Jul 10, 2018

		@@ -0,0 +1,11 @@
		PKG_NAME=ndn-riot
		PKG_URL=https://github.com/astralien3000/ndn-riot

		@@ -0,0 +1,3 @@
		INCLUDES += -I$(BINDIRBASE)/pkg/$(BOARD)
		DIRS += $(BINDIRBASE)/pkg/$(BOARD)/ndn-riot

pkg: ndn-riot: initial import of ndn-riot as a package #5914

pkg: ndn-riot: initial import of ndn-riot as a package #5914

Conversation

cgundogan commented Oct 6, 2016

mfrey commented Oct 6, 2016 • edited Loading

OlegHahm commented Oct 7, 2016

OlegHahm commented Oct 7, 2016

mfrey commented Oct 7, 2016

OlegHahm commented Oct 7, 2016

wentaoshang commented Oct 7, 2016 • edited Loading

cgundogan commented Oct 7, 2016

OlegHahm commented Oct 13, 2016

OlegHahm commented Oct 13, 2016

OlegHahm commented Oct 13, 2016

cgundogan commented Oct 19, 2016

wentaoshang commented Nov 4, 2016

cgundogan commented Nov 29, 2016

tfar commented Nov 29, 2016

wentaoshang commented Nov 29, 2016

tfar commented Nov 29, 2016

wentaoshang commented Nov 29, 2016

tfar commented Nov 29, 2016

wentaoshang commented Nov 29, 2016

kaspar030 commented May 23, 2018 • edited Loading

astralien3000 commented May 23, 2018

aabadie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astralien3000 commented May 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astralien3000 commented May 23, 2018

astralien3000 commented May 24, 2018

astralien3000 commented May 24, 2018

adjih commented May 24, 2018

Choose a reason for hiding this comment

mfrey May 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mfrey May 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astralien3000 May 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astralien3000 left a comment • edited Loading

Choose a reason for hiding this comment

mfrey commented Oct 6, 2016 •

edited

Loading

wentaoshang commented Oct 7, 2016 •

edited

Loading

kaspar030 commented May 23, 2018 •

edited

Loading

mfrey May 24, 2018 •

edited

Loading

mfrey May 24, 2018 •

edited

Loading

astralien3000 May 24, 2018 •

edited

Loading

astralien3000 left a comment •

edited

Loading