Port PPP to the new stack

[axeld]

Port over Waldemar's PPP code to the new networking stack.

[wkornewald]

I should at least watch this task...

[mshlyn]

porting ppp to new netstack

[mshlyn]

pppoe configuration file

[mshlyn]

Usage: 1, merge the patch and compile 2, put ppp1 template configuration file to /boot/home/config/settings/kernel/drivers/ptpnet/ppp1 and change the settings. 3, In terminal type: ifconfig ppp1 up

What is done: 1, port mbuf to net_buffer 2, basic pppoe support

Todo: 1, KPPPManager optimization, now only support basic function 2, autoconnect 3, DNS server update to resolv.conf

done in latest patch

4, spelling, code style, debug information cleanup.

Problem: 1, All lock should be recheckd! 2, Files 'haiku/src/add-ons/kernel/network/ppp/ipcp/{port_before.h, port_after.h, bittypes.h, config.h} are copied from haiku/src/kits/network/libbind/ dir. There should be better solution.

The 2nd one is solved by using simplified inet_addr.c located src/system/kernel/util/. It has no header dependency as the old one located in libbind.

[siarzhuk]

Replying to mshlyn:

Usage: 1, merge the patch and compile 2, put ppp1 template configuration file to /boot/home/config/settings/kernel/drivers/ptpnet/ppp1 and change the settings. 3, In terminal type: ifconfig ppp1 up

I think it is a good idea to ask for testing help in the haiku-development mailing list too. BTW, are you using any Source Code Management systems for your work? Lot of Haiku development branches, hosted on github, for example, send commit notifications into haiku-development mail list. IMO, it is a good way to stay in the light and receive immediate advices from our developers. ;-)

And thank you very much for this work! It was surprising delight seeing it.

[mshlyn]

Replying to siarzhuk:

Replying to mshlyn:

Usage: 1, merge the patch and compile 2, put ppp1 template configuration file to /boot/home/config/settings/kernel/drivers/ptpnet/ppp1 and change the settings. 3, In terminal type: ifconfig ppp1 up

I think it is a good idea to ask for testing help in the haiku-development mailing list too. BTW, are you using any Source Code Management systems for your work? Lot of Haiku development branches, hosted on github, for example, send commit notifications into haiku-development mail list. IMO, it is a good way to stay in the light and receive immediate advices from our developers. ;-)

This is the first time I submit patch. There is much for me to learn. I use git on my local PC. Due to bad plan,there were lots experimental code in my git repo. That is why I only submitted pure diff here. I can upload the format-patch if it helps. As for haiku branch, I only know the official one. Do you have any suggest? I would like to receive immediate advice from the developers.

And thank you very much for this work! It was surprising delight seeing it.

My pleasure if it helps a bit. And thanks very much for you good advice!

[mshlyn]

git format-patch origin/master Output

[siarzhuk]

Replying to mshlyn:

As for haiku branch, I only know the official one. Do you have any suggest? I would like to receive immediate advice from the developers.

If you asking about forked branches of Haiku - there are no such ones at the moment, AFAIK. Most of known repositories are just development branches - like your one. And all mentioned "experiments" are normal development workflow, in my opinion. ;-)

BTW, Trac is targeted mainly for problem discussions so you are welcome to ​http://www.freelists.org/list/haiku-development :-)

[mmadia]

Would it be worthwhile for mshlyn to rewrite his git history, so as to remove/consolidate the experimental changes? The archive attachment contains 37 patches. :-D

[axeld]

Sorry for the long delay, mshlyn! Your work is much appreciated, and also thanks for providing it in git format-patch format!

Your patch has a few issues so that I would not like to submit it as is:

• Why do you copy files like asm_defs.h, net_stack_driver.h, and directories.h? If you need them, they are already in the tree. You may need to alter the Jamfile to make visible. For example, to add private kernel headers, you can use something like UsePrivateKernelHeaders.
• Your patch could use some general cleanup: things like commenting out includes, etc., debug output like the changes to , or the changes made to build/jam/ReleaseBuildProfiles, src/add-ons/kernel/network/datalink_protocols/ethernet_frame/ethernet_frame.cpp, and src/add-ons/kernel/network/devices/ethernet/ethernet.cpp should not be part of it.
• Why is KPPPManager.cpp in its own directory? Besides that, we always use lower case for directory names (there are a few exceptions, but those should be fixed instead of introducing new ones).
• A bit more documentation would be great. For example, what is Interface::ChangeAddress() doing exactly, and why is it needed? I mean, why don't you just set the address like ifconfig would do?
• What does "src for Y470" mean, and why is it just empty?
• The get_interface_address_by_name you introduce deviates from the usual argument ordering (domain comes first).
• Why do you need those functions, anyway?

And also, there is a ton of coding style issues - I mention a few of those that I came across:

• There is a space between 'if', 'while', 'switch', etc. and '('
• Names like 'nd' should be avoided -- use domain instead.
• Space before and after operators (ie. name == NULL instead of name==NULL).
• We use camel case for variables, ie. pppInterfaceAddress instead of ppp_interface_address.
• Two lines between functions.
• Includes are grouped by origin, the groups are ordered most generic to most specific, and the includes within the groups are ordered alphabetically.
• The '{' goes to the end of the line, and is preceded by a space (like src/add-ons/kernel/network/ppp/shared/libkernelppp/settings_tools.cpp).
• If you always exit an 'if' block with 'return', don't use 'else' when it's not needed.
• Space after ''
• Space after ','
• Don't use superfluous parenthesis as in pppoe_input().

I hope you are willing to continue working on this?

[mshlyn]

Update patch as axeld suggested

[mshlyn]

Fixed lots of issues pointed out by axeld, including style issues. What is left:

1, Integrate PPPManager into libkernelppp 2, pppconfig, ppp_up are still not working

[phoudoin]

Thanks, I will give it a look any time soon.

[mshlyn]

'git pull --rebase' to the latest master

[luroh]

phoudoin: ping?

[umccullough]

Would be nice to see some of this merged soon so it doesn't rot :)

[luroh]

Bitrot has set in, patch 0042 no longer applies.

[jessicah]

Replaces patch 0042 of the patchset

[jessicah]

It at least compiles again. Am not sure if the use of volatile integers should be replaced with just int32 or not. The use of atomic_add family of functions require casting to int32* to compile. Probably better to switch to int32 and use atomic getters instead?

[pdziepak]

Volatile integers are not needed here. Unfortunately, this patch series in current form is very difficult to review as the majority of the patches are fixes to the previous ones and definitely should not be merged. However, I have noticed that IPCP::fID is accessed both using normal and atomic operations and that doesn't look good. Actually, I am not even sure that increment of fID really needs to be atomic (but I may be wrong).

[waddlesplash]

Hmm, so if I understand correctly:

1. all teh patchez should be combined into logical chunks
2. Volaitle ints should be removed
3. Atomic-only operations should be used on IPCP::fID

...and it should be tested on an actual dialup line. Right?

[luroh]

Moving PPP related tickets out of R1 milestone (Prop #2).

[waddlesplash]

The patchset now lives at ​https://github.com/waddlesplash/haiku/tree/pppp; I've rebased it against master and will work on it as time allows.

[waddlesplash]

Entire patchset merged in hrev49969.

[waddlesplash]

deassigning various things from me

[kallisti5]

Since this was all merged i'm going to close this one. Any additional issues should be tracked in new tickets.