Skip to content

Conversation

@jukkar
Copy link
Member

@jukkar jukkar commented Aug 2, 2017

This is a simple RPL based border router sample application. This can do routing between for example 802.15.4 and ethernet and act as a root node for RPL network. The router provides currently a very simple web UI for seeing the current nodes. Also new commands are added to net-shell to give better information about the RPL network.

@jukkar jukkar added the net label Aug 2, 2017
@jukkar jukkar self-assigned this Aug 2, 2017
@jukkar jukkar requested a review from rveerama1 August 2, 2017 13:32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Currently/By default/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest adding the name of the configuration file:

   in the project configuration file :file:`samples/net/rpl_border_router/prj.conf`:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems there's a reference to this full name few lines below.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

connection to the integrated web

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't there be some commands here with the building instructions?

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some suggested edits

@jukkar
Copy link
Member Author

jukkar commented Aug 7, 2017

Changes:

  • Documentation fixes and enhancements.
  • net-shell output fixes
  • DAO-ACK support
  • removed BT enablers as those are not used/needed atm
  • removed ARM qemu from compilation test as there is not enough memory there for this sample

@jukkar jukkar force-pushed the border-router branch 2 times, most recently from 91afd02 to df4e87e Compare August 7, 2017 12:39
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this sample is not intended for qemu, why are we whitelisting it here? why do we have a configuration for quark se but we are not building it? Also, the frdm board does not have 15.4.. I suggest removing the whitelist and adding:

depends_on: ieee802154

and make this more generic, we are going back to adding board specific configurations, something we need to stop doing.

Copy link
Member Author

@jukkar jukkar Aug 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left the qemu_x86 there so it gets compile tested in qemu. Probably this is not needed if we are not running it in qemu. As this is still very much Work In Progress, I am still figuring out how and what hw to run this sample.
I will add the depends_on line as suggested.

@jukkar
Copy link
Member Author

jukkar commented Aug 9, 2017

Updating the patches.

@jukkar jukkar force-pushed the border-router branch 2 times, most recently from aa4f1f2 to b026ab8 Compare August 10, 2017 14:17
@jukkar
Copy link
Member Author

jukkar commented Aug 10, 2017

New version with RPL fixes.

@jukkar
Copy link
Member Author

jukkar commented Aug 11, 2017

Various bug fixes added, I will send these also in separate PR.

@jukkar jukkar mentioned this pull request Aug 11, 2017
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to put the description of the command before the example rather than after on all of these examples. For example,

 The **br repair** command will cause the RPL network to re-configure itself.

 .. code-block:: console

    shell> br repair
    [rpl-br/shell] [INF] br_repair: Starting global repair...

Copy link
Contributor

@dbkinder dbkinder Aug 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the description before the example (and fix the misspelling of "configuration").

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the description before the example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the description before the example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/like/such as/

"like" is used for comparison ("run like the wind"), and "such as" is use for examples

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, we have that QEMU emulation for 15.4, any hard reason why it can't be used?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need more than one node in the network. The qemu emulation for 15.4 is one node only solution. This could be done in simulated way with emul8 but that is some future work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you do not have a management interface

Here, I'm, a user, is perplexed - do I have a management interface or not? Did you mean to write something like "If you disabled management interface in the config file, than IPv4 support can also be disabled" ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Management interface in this context refers to network interface that provides web support (==management operations) towards host computer. I obviously need to elaborate wording here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please do :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems there's a reference to this full name few lines below.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope it would be possible to access web over 15.4 6lowpan from host too? Maybe slow, but that should work?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that should work just fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested ":" at the end of the phrase.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance to have more distinctive shell module name? In network terminology, "br" would have much higher hit for "bridge" than "border router".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to keep the command short here thus "br" was used. Note that this command is only meant/used/available for this border router application so user probably understand what the command is for.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably understand what the command is for.

Probably. I'd approach it from the other side though: "can it cause confusion or not?" Arguably it can. If short prefix is desired, I'd suggest "brt".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/to/so/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/to/so/

Likely, more copies below.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use non-default channel? Will lead to confusion.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some channel used in testing, can be changed to default.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Looking at what magic this app does that it requires #1330 and whether that can be addressed by #119 pretty easily.)

So, I won't be asking "what happens if payload is static const payload[1000000]" like I do in the mailing list RFC, let me just ask "what happens if payload_len is larger than free fragment space available, e.g. payload_len is 3K, but there're only 1.5K of free data buffers." You answer would be "But I allocated 8K of buffers above!" And that's cheating - the only way such paradigm would work is by overallocating buffers to back it.

I don't know where this "browser DOM" approach of "keep everything in memory" comes from. And the problem here is clearly in the API design. It's not natural for an embedded HTTP API to require a user to allocate a "network packet" and then keep adding data to it without limit. The natural API is http_send_data(context *ctx, const char *buf, int len), and let that call decide what to do with that data: accumulate data into infinite buffer (typical "design pattern" for desktop/server apps nowadays), send even the smallest chunk of data right away (the opposite extreme), or buffer it per the natural size of buffer (which is single packet) to minimize copies, when it fills, send it, and repeat with the reminder of data - the approach I advocate in my RFC.

So, it looks like we have a case when "interesting" design of the HTTP API starting to affect design of the IP stack (like requiring #1330).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note that this is just a sample application that does things in certain way. Nothing prevents developer to send data in certain size chunks here. I just did not implement it here as #1330 was doing it for me automatically.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sent updated HTTP API patches in #4243
That PR differs slightly from the corresponding patches in this PR, for example the old HTTP API is deprecated, and client API support is added.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note that this is just a sample application that does things in certain way. Nothing prevents developer to send data in certain size chunks here. I just did not implement it here as #1330 was doing it for me automatically.

But #1330 is no longer an application patch, it's IP stack patch. If such patches go in, and such sample applications are in the mainline, it sends a strong message how things should be done in Zephyr, nobody simply will look for doing in another way. That's why it's important to have good generic APIs right away and resist desire to push workaround-style solutions down the stack.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sent updated HTTP API patches in #4243

Thanks, will have a look.

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggested edits

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add:

 cd samples/net/http_server

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a cd to the right directory for this sample

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please do :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

either "is no board" or "are no boards"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does "compile test" mean? you can compile but not run the sample? or did you mean to say "compile and test"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

provides an

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using a

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from your browser

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be samples/net/ws_server ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should echo-server be websocket-server here?

@galak
Copy link
Contributor

galak commented Nov 9, 2017

What's the status for this, close it out? Still aimed for 1.10?

@jukkar jukkar removed this from the v1.10.0 milestone Nov 10, 2017
@jukkar jukkar requested a review from SebastianBoe as a code owner November 16, 2017 13:46
@jukkar
Copy link
Member Author

jukkar commented Nov 16, 2017

Update:

  • port to Cmake
  • use latest HTTP and websocket APIs

nagineni pushed a commit to nagineni/zephyr that referenced this pull request Nov 20, 2017
 - OCF and DGRAM can now use the same network/BLE config API's

Signed-off-by: James Prestwood <[email protected]>
jukkar added 11 commits January 9, 2018 11:00
Instead of trying to send a packet that is larger than MSS,
split it into suitable pieces and queue the pieces individually.

Jira: ZEP-1998

Signed-off-by: Jukka Rissanen <[email protected]>
This commit creates a websocket library that can be used by
applications. The websocket library implements currently only
server role and it uses services provided by net-app API.
The library supports TLS if enabled in configuration file.

This also adds websocket calls to HTTP app server if websocket
connection is established.

Signed-off-by: Jukka Rissanen <[email protected]>
This is a http(s) server that supports also websocket.
It sends back any data sent to it over a websocket.

Signed-off-by: Jukka Rissanen <[email protected]>
This tests websocket by creating a websocket support http server
and sending data to it and verifying the returned data is the same.

Signed-off-by: Jukka Rissanen <[email protected]>
Add console driver that allows console session to be transferred
over a websocket connection.

Signed-off-by: Jukka Rissanen <[email protected]>
This sample application implements a web service that provides
zephyr console over websocket.

Signed-off-by: Jukka Rissanen <[email protected]>
Introduce CONFIG_NET_ROUTING option that allows the IP stack
to route IPv6 packets between multiple network interfaces.
No support for IPv4 routing is implemented by this commit.

Signed-off-by: Jukka Rissanen <[email protected]>
As this is very specialized info which is not normally needed,
do not print it by default.

Signed-off-by: Jukka Rissanen <[email protected]>
Doing neighbor discovery in RPL network is not necessary and
that can be disabled in RPL nodes. Unfortunately the border
router needs to have ND enabled as it has also non-RPL network
interfaces in use. So in this case, mark RPL node as always
reachable.

Signed-off-by: Jukka Rissanen <[email protected]>
This is needed in routing if we are acting as border router.

Signed-off-by: Jukka Rissanen <[email protected]>
This is a very simple RPL border router sample application.
It provides HTTP(S) and net-shell interface for getting admin
information about neighbors and routes.

Signed-off-by: Jukka Rissanen <[email protected]>
@nashif
Copy link
Member

nashif commented Jan 30, 2018

what is the status of this PR? Can it be closed? is it going to be updated and made ready for merge?

@jukkar
Copy link
Member Author

jukkar commented Jan 31, 2018

Ravi is currently working with these commits in #5035 so current plan is to close this PR after his set is updated (probably tomorrow).

@rveerama1
Copy link
Contributor

rveerama1 commented Jan 31, 2018

I have separated RPL BR sample independent patches and created another #5924. #5035 PR still contains all the patches. I will rebase and update #5035 once #5924 is merged.

@jukkar jukkar closed this Feb 5, 2018
@ghost ghost removed the In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on label Feb 5, 2018
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.

7 participants