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.

Sign up for GitHub

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

Jump to bottom

Initial PPP implementation #16217

Merged
merged 17 commits into from
Jul 29, 2019
Merged

Initial PPP implementation #16217

merged 17 commits into from
Jul 29, 2019

Conversation

jukkar
Copy link
Member

@jukkar jukkar commented May 16, 2019 •
edited

Initial implementation for Point-to-point protocol (PPP). It implements LCP, IPCP and IPV6CP modules. See these RFCs for details.

This is still work-in-progress and following things need more TLC:

  • Authentication support missing
  • LCP non-default option negotiation
  • MRU/MTU handling not yet done

Fixes #14034

@jukkar jukkar added area: Networking DNM This PR should not be merged (Do Not Merge) labels May 16, 2019
@zephyrbot
Copy link
Collaborator

zephyrbot commented May 16, 2019 •
edited

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

@jukkar
Copy link
Member Author

jukkar commented Jun 9, 2019

Resolved merge conflicts

@zephyrbot zephyrbot added area: API Changes to public APIs area: Tests Issues related to a particular existing or missing test area: Build System area: Sanitycheck Sanitycheck has been renamed to Twister labels Jun 9, 2019
@jukkar jukkar removed the DNM This PR should not be merged (Do Not Merge) label Jun 17, 2019
@jukkar jukkar changed the title [WIP] Initial PPP implementation Initial PPP implementation Jun 17, 2019
@jukkar jukkar requested a review from dbkinder as a code owner June 24, 2019 11:00
@zephyrbot zephyrbot added the area: Samples Samples label Jun 24, 2019
@jukkar
Copy link
Member Author

jukkar commented Jun 24, 2019

First version that works properly against Linux pppd with IPv4 and IPv6.

@jukkar
Copy link
Member Author

jukkar commented Jun 25, 2019

  • rebased against latest master
  • added support for sending Echo-Request and receiving Echo-Reply from net-shell

tbursztyka
tbursztyka requested changes Jun 26, 2019
View reviewed changes
Copy link
Collaborator

@tbursztyka tbursztyka left a comment

Choose a reason for hiding this comment

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

Most of it seems fine to me.

There is just (well it is not a small change) the logic of config_info_* functions and ppp_send_pkt: that has to be reworked so net_pkt is allocated first so all its API can be used instead of the net_buf one.
Whit that done, I think it will be good to go.

include/net/ppp.h Outdated Show resolved Hide resolved
subsys/net/l2/ppp/ppp_stats.c Outdated Show resolved Hide resolved
subsys/net/l2/ppp/ppp_stats.h Outdated Show resolved Hide resolved
subsys/net/l2/ppp/ppp_stats.h Outdated Show resolved Hide resolved
subsys/net/l2/ppp/fsm.c Show resolved Hide resolved
subsys/net/l2/ppp/fsm.c Show resolved Hide resolved
include/net/ppp.h Show resolved Hide resolved
drivers/net/ppp.c Outdated Show resolved Hide resolved
subsys/net/l2/ppp/Kconfig Show resolved Hide resolved
include/net/ppp.h Show resolved Hide resolved
@jukkar
Copy link
Member Author

jukkar commented Jun 27, 2019

There is just (well it is not a small change) the logic of config_info_* functions and ppp_send_pkt: that has to be reworked so net_pkt is allocated first so all its API can be used instead of the net_buf one.
Whit that done, I think it will be good to go.

This is in the TODO list (also marked in the code), but as this is a bigger change, I want to postpone this a bit and to be done as a bug fix.

@jukkar
Copy link
Member Author

jukkar commented Jun 27, 2019

Updated according to comments.

@jukkar
Copy link
Member Author

jukkar commented Jul 10, 2019

Fixed the sanitychecker errors.

@jukkar
Copy link
Member Author

jukkar commented Jul 24, 2019

Resolved merge conflicts.

@jukkar jukkar closed this Jul 24, 2019
@jukkar jukkar reopened this Jul 24, 2019
@jukkar jukkar mentioned this pull request Jul 25, 2019
Closed
jukkar added 17 commits July 25, 2019 19:16
@jukkar
net: ppp: Initial support for point-to-point protocol
1dbaf52 
This implements ppp L2 component, LCP and IPCP modules.

Fixes zephyrproject-rtos#14034

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar
net: ppp: Add IPV6CP support
4076615 
Initial version for PPP IPv6 Control Protocol.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar
cmake: Add serial port for ppp in qemu
eabc2e0 
In order to simplify ppp testing, use ppp specific serial port
when starting qemu.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar
drivers: net: ppp: Driver for point-to-point protocol
08c8546 
The ppp driver uses uart_pipe driver for data transfer.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar
net: shell: Refactor network statistics printing
1c57a48 
The traffic class printing section was too convoluted because of
many #ifdef's.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar
net: shell: Add ppp network interface support
2083d7b 
Print point-to-point network information properly.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar
tests: net: ppp: Add unit tests for PPP driver
eab2c50 
Make sure that we are able to parse incoming PPP data
correctly.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar
net: ppp: Allow delay of PPP protocol handshakes
0a3a733 
By default PPP is started immediately when the network interface
goes up. This can be problematic especially when debugging the beast
so allow user to delay the startup.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar
net: ppp: Reject unhandled protocols
f1ace73 
If we receive a protocol that we do not currently handle, then
return Protocol-Reject to peer.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar
net: if: No need to set IP addresses etc for PPP
9cbdaab 
If the network interface is point-to-point one which does
not need IP address etc, then no need to start DAD etc for
those interfaces.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar
net: pkt: Add net_pkt_write_le16() helper
4a33e7b 
We had a big endian helper but little endian one was missing.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar
net: pkt: Add net_pkt_read_le16() helper
24c18ce 
We had a big endian helper but little endian one was missing.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar
samples: net: echo-server: Add PPP overlay config file
782341a 
A sample overlay config file for PPP added.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar
net: ppp: Add proper support to receive Echo-Reply message
87d10c8 
Currently only net-shell calls net_ppp_ping() command, so make
it return the amount of time that it took to receive Echo-Reply
so the net-shell can print the round trip time value.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar
net: ppp: Handle received Discard-Request
0c8ac3d 
We must discard the received Discard-Request silently.
See RFC 1661 chapter 5.9 for details.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar
doc: net: ppp: Add PPP documentation
a3117a3 
Add information about PPP into Zephyr documentation.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar
boards: Set support for serial port networking as netif:serial-net
9166af3 
As we now have PPP support, use more generic "serial-net" string instead
of "slip" when setting what kind of networking the board supports.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
tbursztyka
tbursztyka approved these changes Jul 26, 2019
View reviewed changes
Copy link
Collaborator

@tbursztyka tbursztyka left a comment

Choose a reason for hiding this comment

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

Let's get this in. However, the logic of creation packet with options will have to be reworked, too late for this now.

drivers/net/ppp.c
LOG_DBG("Saving byte %02x", byte);
}

/* This is not very intuitive but we must allocate new buffer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove that comment. If that matters, net_pkt documentation should be improved.
It was always designed so 1st it allocates the buffer 2nd it operates on the buffer. (allocate only allocates, write only writes, read only reads etc...)

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I prefer leaving this comment here for time being. I spent hours debugging this as it was not clear why the api requires that. At least for me, it would be difficult to understand why we have the ppp->available==1 check without the comment. The documentation of net-pkt should be improved in this respect.

@jukkar jukkar merged commit 5cf60a3 into zephyrproject-rtos:master Jul 29, 2019
@jukkar jukkar deleted the ppp branch July 29, 2019 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SebastianBoe SebastianBoe SebastianBoe approved these changes

@tbursztyka tbursztyka tbursztyka approved these changes

@dbkinder dbkinder dbkinder approved these changes

@andrewboie andrewboie Awaiting requested review from andrewboie

@andyross andyross Awaiting requested review from andyross

@nashif nashif Awaiting requested review from nashif

@pfalcon pfalcon Awaiting requested review from pfalcon

@fvincenzo fvincenzo Awaiting requested review from fvincenzo

@galak galak Awaiting requested review from galak

@MaureenHelm MaureenHelm Awaiting requested review from MaureenHelm

Assignees
No one assigned
Labels
area: API Changes to public APIs area: Boards area: Build System area: Documentation area: Networking area: Samples Samples area: Sanitycheck Sanitycheck has been renamed to Twister area: Tests Issues related to a particular existing or missing test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for PPP protocol
5 participants
@jukkar @zephyrbot @SebastianBoe @tbursztyka @dbkinder