ppp
ppp copied to clipboard
ppp-project
We should avoid code duplication between rp-pppoe and this project
The pppd/plugins/pppoe directory contains a substantial amount of code from the rp-pppoe project, and unfortunately that code has diverged from upstream.
I think we should synchronize the projects so the code isn't duplicated, and possibly remove the pppoe plugin from the ppp project once rp-pppoe builds and installs properly alongside ppp.
Created this issue in response to https://github.com/ppp-project/ppp/pull/379#issuecomment-1372472906
Just my thoughts:
rp-pppoe I believe was the original implementation.
MOST users will never need the servers, the bridge mechanisms and most of the other tools provided by the rp-pppoe package, only the pppoe module as per current available in ppp.
From a perspective of "if rp-pppoe requires additional options in pppoe.so module" it makes sense for me to package the ppp module with rp-pppoe.
I do, however, think that unlikely. The way I would go personally if this was up to me and I controlled both packages (I'm not responsible for either, but believe I've got insight into both) is to put the pppoe module in ppp package as it's more involved with the ppp internals than pppoe-server (for example). Further to this, rp-pppoe (and specifically pppoe-server) cannot stand without ppp (even if the module is
I don't know if there is much sense to keep the pppoe userspace implementation around any more.
One other option which may be considered but which has a bit higher administrative overhead is a git submodule for the pppoe.so file ppp module specifically which can be imported into both projects (even though rp-pppoe cannot stand without ppp), or just an outright separate repository for that.
I think that rp-pppoe can be in the "ppp-project" organization near ppp.
I think the first step should be to examine how the C source files have diverged between the ppp project and upstream rp-pppoe and reconcile those divergences. Once that's done, we can look at what functions in the common C files are needed by both the PPPoE client and server software and try to minimize those. One example is parsePacket, which is used by both the client and server.
I'm not sure if it makes sense to have a library in the ppp project against which the pppoe-server links, or just duplicate the common code if there's only a small amount of it.
My personal preference would be to remove all the PPPoE stuff from the ppp project and just keep it in rp-pppoe, but I don't think that's going to fly :) I also don't think moving all of rp-pppoe under the ppp project makes sense; the server code is really different from everything else.
But anyway... resolving the divergence of the source should be the first step.
Oh, and I'm OK with getting rid of the userspace PPPoE code, but that means it's irrevocably tied to Linux. I know at some point, user-mode rp-pppoe used to work on BSD and even Solaris, but I suspect that has long bit-rotted.
Oh, and I'm OK with getting rid of the userspace PPPoE code, but that means it's irrevocably tied to Linux. I know at some point, user-mode rp-pppoe used to work on BSD and even Solaris, but I suspect that has long bit-rotted.
ppp itself doesn't work on BSD any more, but it does on Solaris. I accidentally tested user-mode and it works, but most likely won't perform as well, so until it breaks proper I suggest keeping it is probably the best option.
Could you also please clarify what you mean with "I'm not sure if it makes sense to have a library in the ppp project against which the pppoe-server links" - do you mean to say there is code that goes into both the pppoe module (ie, the ppp plugin) as well as the other pppoe-* binaries? This seems to be accurate upon actually looking into more detail, specifically the discovery portion of the pppoe plugin, and specifically parsePacket() seems to relate. This is a reasonably simple function from the looks of it, the rest of the common.c file looks mostly distinct functions.
discovery.c at least has diverged quite far. And here are the interesting commits:
commit dbfeebc9adcf76a50c1d4e9035d5d481914edb43
Author: Michal Ostrowski <[email protected]>
Date: Fri Dec 14 02:55:20 2001 +0000
Switch to using RoaringPenguin-based PPPOE plugin
commit cdbb124cea5feeb4468ad3f2e5b5d17de583da6d
Author: Paul Mackerras <[email protected]>
Date: Thu Jul 26 20:03:32 2001 +0000
pppoe plugin stuff - Michal Ostrowski's version, lightly hacked
The first change after these were then on the last day of 2020 in order to rename from rp-pppoe to pppoe, and then there were a bunch of changes.
Unfortunately I don't have full VCS history for the current rp-pppoe version, so can't really comment if much has happened on that front?
Yes; I meant having a library in the ppp project that contains the functions common to pppoe-server and the pppoe plugin. As you say, there are not many of them and they are pretty simple, so maybe duplicating those is not a big deal. Probably would be a good idea to split common.c into two files... one that contains functions only used by the client and one that contains functions also used by the server.
As I may have mentioned, I can't make the full PPPoE git history available because in the past I worked on some proprietary code for a customer, and that's in the git history. :(
When I speak about "ppp-project" is to have the code here: https://github.com/ppp-project/rp-pppoe.
https://github.com/ppp-project/ppp and https://github.com/ppp-project/rp-pppoe are two repositories at a same place to improve the code...
@dfskoll: Since I have proposed you to put the code on GitHub, new contributions have been done...
What is the advantage of having under ppp-project rather than under dfskoll? It seems like the only difference is who controls the project, and at this point, I prefer to keep control.
I think @dfskoll is right in that the first step is to review the code differences between the two. And then perhaps if possible consolidation of said code. I don't think there should be any difference between these two, and that kernel mode vs user-mode is really just a feature of said plugin (dependent on how you chose to configure and compile it).
Second to that, I am not a big fan of putting the code of (rp-)pppoe directly in the code tree of pppd. It's a bit of a cluster ** with automake/autoconf and having to generate multiple config.h files. It feels like the pppoe plugin (as well as several other plugins) should be standalone projects. Perhaps under the ppp-project umbrella, or some other entity.
Typically, any project can create and distribute a plugin for pppd, but I think the (rp-)pppoe module is different in that it can be used for both client and server. And people who would like to use this as a client, is not interested in the server piece of it. Establishing a repository in ppp-project/rp-ppope-plugin or something similar would require
- Automake/autoconf changes
- Separate release management
- Packaging and adoption of said by RedHat and Debian guys
- Extra scrutiny by said distributors
Also, it should be possible with packaging of rp-pppoe project to configure a dependency on the rp-pppoe-plugin being installed.
Ultimately, I don't think maintaining two repositories for rp-pppoe-plugin (or forking @dfskoll is a nice thing) is inherently difficult. It also confuses users, issue tracking, maintenance, etc.
@Neustradamus if you / Paul were to make a https://github.com/ppp-project/rp-pppoe-plugin (doesn't exist yet), would it be possible to assign "ownership" of the project (or joint ownership) to @dfskoll
Note the name difference here, just saying rp-pppoe implies the entire project from @dfskoll. I think it somehow should be named with "plugin" somehow to make distinction that it only provides the plugin.
@dfskoll is rp-pppoe (server) and utilities distributed as a package per Debian or Ubuntu? If not, you should probably file a intent to package or request to package for this software with Debian and cook up a package recipe for this.
The Debian package is called pppoe and includes the server, relay, etc.
See https://packages.debian.org/bullseye/amd64/pppoe/filelist
It does not include the pppoe.so plugin, which is part of the ppp package:
https://packages.debian.org/bullseye/amd64/ppp/filelist
Yeah, I just found it by apt-get source pppoe, gets me rp-pppoe-3.12.
So rp-pppoe.so file (which supports kernel mode?) isn't distributed on Debian?
So rp-pppoe.so file (which supports kernel mode) isn't distributed on Debian?
It is, but in the ppp package, not the pppoe package.
So users like @jkroonza needs to go out of their way to get rp-pppoe sources and compile the plugin? Yeah, I see that by the control file require ppp >= 2.3.10-1.
I think they can use the plugin that's included with ppp and ignore the one included with (rp-)pppoe. But @jkroonza can confirm.
Yeah, I am curious now. I was under the impression it had to do with performance...
The plugin included with ppp and the one in the rp-pppoe sources are very similar; they should have identical performance since their code only comes into play for PPPoE discovery packets. The kernel completely handles PPPoE session packets.
@dfskoll that is good to know. I'd like to hear it from @jkroonza to why he needed the one from rp-pppoe package (or better yet trying to understand why anyone would want the one from rp-pppoe package). Again, this is some of the pain / confusion of forking a project ...
I did cleanup of pppoe.so plugin in ppp repository, removed and changed code better fit in ppp repository. I'm not sure if such changes are useful / suitable for rp-pppoe server project. Other changes are mostly fixups for the client code.
As jkroonza said, most users do not need server part of pppoe. I think that rp-pppoe server code should work with pppoe.so plugin from ppp repository.
So what about putting all client code with pppoe.so plugin into ppp project and all server code (without pppoe.so plugin) into rp-pppoe? Or is there is too many common code which makes sense to somehow share between ppp and rp-pppoe projects?
That's what I'm getting at. You changed the pppoe.so code. But so did I, upstream, independently. We should try to merge the code together because some of the upstream changes might be good to have in ppp/plugins/pppoe.
Eventually, when the changes are merged, I guess it makes sense to have all of the plugin code live only in the ppp project, and have rp-pppoe concentrate on supplying the server and relay implementations. To the extent that there's common code between the rp-pppoe server and the pppoe plugin, there will be code duplication. I have to look and see how much duplicated code there really is.
One of the things I remember being fixed in the ppp/pppoe version is compiler warnings (that still exist in the rp-pppoe project when compiling with the plugin). And yes, I do recall @pali fixing a bunch of things there as well. My two cents is that there should be only one single source of the truth, and it should be able to handle server and client mode, user and kernel mode version in the same plugin. Also, I would love to see a wiki enabled on the GitHub project with some documentation to how to configure either of these.
And yes, there will be some duplication of code. That's just a fact when you split a project in two.
I don't see any compiler warnings with the latest rp-pppoe git HEAD. This is on Debian 11.
$ make rp-pppoe.so
gcc -DPLUGIN=1 '-DRP_VERSION="3.15"' -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent -I/usr/include -c -o plugin/plugin.o -fPIC plugin.c
gcc -DPLUGIN=1 -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -I/usr/include -c -o plugin/discovery.o -fPIC discovery.c
gcc -DPLUGIN=1 -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -I/usr/include -c -o plugin/if.o -fPIC if.c
gcc -DPLUGIN=1 -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -I/usr/include -c -o plugin/common.o -fPIC common.c
gcc -DPLUGIN=1 -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -I/usr/include -c -o plugin/debug.o -fPIC debug.c
ar -rc plugin/libplugin.a plugin/discovery.o plugin/if.o plugin/common.o plugin/debug.o
gcc -o rp-pppoe.so -shared plugin/plugin.o plugin/libplugin.a
No warnings with just a plain make that builds everything, either.
@dfskoll
This maybe because I am on a different distribution than what you are running. Currently, I am on a Ubuntu Cloud / Minimal (uvt-kvm create) using Ubuntu 22.04 Jammy Jellyfish, install gcc and build-essentials and I have the following (using git repository). I don't have such warnings in the logs when compiling pppd project.
gcc -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/usr/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -c -o pppoe.o pppoe.c
gcc -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/usr/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -c -o if.o if.c
gcc -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/usr/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -c -o debug.o debug.c
gcc -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/usr/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -c -o common.o common.c
common.c: In function ‘dropPrivs’:
common.c:230:9: warning: ignoring return value of ‘setegid’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
230 | setegid(getgid());
| ^~~~~~~~~~~~~~~~~
common.c:231:9: warning: ignoring return value of ‘seteuid’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
231 | seteuid(getuid());
| ^~~~~~~~~~~~~~~~~
gcc -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/usr/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -c -o ppp.o ppp.c
gcc -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/usr/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -c -o discovery.o discovery.c
discovery.c: In function ‘sendPADR’:
discovery.c:575:8: warning: array subscript ‘PPPoETag {aka struct PPPoETagStruct}[0]’ is partly outside array bounds of ‘PPPoEPacket[1]’ {aka ‘struct PPPoEPacketStruct[1]’} [-Warray-bounds]
575 | svc->type = TAG_SERVICE_NAME;
| ^~
discovery.c:554:17: note: while referencing ‘packet’
554 | PPPoEPacket packet;
| ^~~~~~
discovery.c:576:8: warning: array subscript ‘PPPoETag {aka struct PPPoETagStruct}[0]’ is partly outside array bounds of ‘PPPoEPacket[1]’ {aka ‘struct PPPoEPacketStruct[1]’} [-Warray-bounds]
576 | svc->length = htons(namelen);
| ^~
discovery.c:554:17: note: while referencing ‘packet’
554 | PPPoEPacket packet;
| ^~~~~~
discovery.c: In function ‘sendPADI’:
discovery.c:359:12: warning: array subscript ‘PPPoETag {aka struct PPPoETagStruct}[0]’ is partly outside array bounds of ‘PPPoEPacket[1]’ {aka ‘struct PPPoEPacketStruct[1]’} [-Warray-bounds]
359 | svc->type = TAG_SERVICE_NAME;
| ^~
discovery.c:332:17: note: while referencing ‘packet’
332 | PPPoEPacket packet;
| ^~~~~~
discovery.c:360:12: warning: array subscript ‘PPPoETag {aka struct PPPoETagStruct}[0]’ is partly outside array bounds of ‘PPPoEPacket[1]’ {aka ‘struct PPPoEPacketStruct[1]’} [-Warray-bounds]
360 | svc->length = htons(namelen);
| ^~
discovery.c:332:17: note: while referencing ‘packet’
332 | PPPoEPacket packet;
| ^~~~~~
gcc -o pppoe pppoe.o if.o debug.o common.o ppp.o discovery.o
gcc -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/usr/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -c -o pppoe-server.o pppoe-server.c
pppoe-server.c: In function ‘main’:
pppoe-server.c:1659:9: warning: ignoring return value of ‘fread’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
1659 | fread(&x, 1, sizeof(x), fp);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
pppoe-server.c:1661:9: warning: ignoring return value of ‘fread’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
1661 | fread(&CookieSeed, 1, SEED_LEN, fp);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pppoe-server.c:1838:9: warning: ignoring return value of ‘chdir’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
1838 | chdir("/");
| ^~~~~~~~~~
pppoe-server.c:1878:9: warning: ignoring return value of ‘ftruncate’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
1878 | ftruncate(LockFD, 0);
| ^~~~~~~~~~~~~~~~~~~~
pppoe-server.c:1880:9: warning: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
1880 | write(LockFD, buf, strlen(buf));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pppoe-server.c:1892:9: warning: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
1892 | write(KidPipe[1], "X", 1);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
gcc -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/usr/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -c -o md5.o md5.c
cc -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/usr/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent -c -o control_socket.o control_socket.c
cd libevent && make DEFINES=""
make[1]: Entering directory '/home/ubuntu/projects/rp-pppoe/src/libevent'
gcc -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes -I.. -c -o event.o event.c
gcc -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes -I.. -c -o event_tcp.o event_tcp.c
gcc -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes -I.. -c -o hash.o hash.c
gcc -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes -I.. -c -o event_sig.o event_sig.c
event_sig.c: In function ‘sig_handler’:
event_sig.c:146:5: warning: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
146 | write(Pipe[1], &sig, 1);
| ^~~~~~~~~~~~~~~~~~~~~~~
rm -f libevent.a
ar -cq libevent.a event.o event_tcp.o hash.o event_sig.o
ranlib libevent.a
make[1]: Leaving directory '/home/ubuntu/projects/rp-pppoe/src/libevent'
gcc -o pppoe-server pppoe-server.o if.o debug.o common.o md5.o control_socket.o libevent/libevent.a -Llibevent -levent
gcc -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/usr/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -c -o pppoe-sniff.o pppoe-sniff.c
gcc -o pppoe-sniff pppoe-sniff.o if.o common.o debug.o
gcc -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/usr/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -c -o relay.o relay.c
relay.c: In function ‘main’:
relay.c:343:9: warning: ignoring return value of ‘chdir’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
343 | chdir("/");
| ^~~~~~~~~~
relay.c: In function ‘relayLoop’:
relay.c:802:13: warning: ignoring return value of ‘read’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
802 | read(CleanPipe[0], &dummy, 1);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
relay.c: In function ‘alarmHandler’:
relay.c:1528:9: warning: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
1528 | write(CleanPipe[1], "", 1);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
gcc -o pppoe-relay relay.o if.o debug.o common.o
gcc -DPLUGIN=1 '-DRP_VERSION="3.15"' -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/usr/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent -I/usr/include -c -o plugin/plugin.o -fPIC plugin.c
plugin.c:67:12: warning: ‘seen_devnam’ defined but not used [-Wunused-variable]
67 | static int seen_devnam[2] = {0, 0};
| ^~~~~~~~~~~
gcc -DPLUGIN=1 -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/usr/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -I/usr/include -c -o plugin/discovery.o -fPIC discovery.c
discovery.c: In function ‘sendPADR’:
discovery.c:575:8: warning: array subscript ‘PPPoETag {aka struct PPPoETagStruct}[0]’ is partly outside array bounds of ‘PPPoEPacket[1]’ {aka ‘struct PPPoEPacketStruct[1]’} [-Warray-bounds]
575 | svc->type = TAG_SERVICE_NAME;
| ^~
discovery.c:554:17: note: while referencing ‘packet’
554 | PPPoEPacket packet;
| ^~~~~~
discovery.c:576:8: warning: array subscript ‘PPPoETag {aka struct PPPoETagStruct}[0]’ is partly outside array bounds of ‘PPPoEPacket[1]’ {aka ‘struct PPPoEPacketStruct[1]’} [-Warray-bounds]
576 | svc->length = htons(namelen);
| ^~
discovery.c:554:17: note: while referencing ‘packet’
554 | PPPoEPacket packet;
| ^~~~~~
discovery.c: In function ‘sendPADI’:
discovery.c:359:12: warning: array subscript ‘PPPoETag {aka struct PPPoETagStruct}[0]’ is partly outside array bounds of ‘PPPoEPacket[1]’ {aka ‘struct PPPoEPacketStruct[1]’} [-Warray-bounds]
359 | svc->type = TAG_SERVICE_NAME;
| ^~
discovery.c:332:17: note: while referencing ‘packet’
332 | PPPoEPacket packet;
| ^~~~~~
discovery.c:360:12: warning: array subscript ‘PPPoETag {aka struct PPPoETagStruct}[0]’ is partly outside array bounds of ‘PPPoEPacket[1]’ {aka ‘struct PPPoEPacketStruct[1]’} [-Warray-bounds]
360 | svc->length = htons(namelen);
| ^~
discovery.c:332:17: note: while referencing ‘packet’
332 | PPPoEPacket packet;
| ^~~~~~
gcc -DPLUGIN=1 -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/usr/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -I/usr/include -c -o plugin/if.o -fPIC if.c
gcc -DPLUGIN=1 -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/usr/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -I/usr/include -c -o plugin/common.o -fPIC common.c
common.c: In function ‘dropPrivs’:
common.c:230:9: warning: ignoring return value of ‘setegid’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
230 | setegid(getgid());
| ^~~~~~~~~~~~~~~~~
common.c:231:9: warning: ignoring return value of ‘seteuid’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
231 | seteuid(getuid());
| ^~~~~~~~~~~~~~~~~
gcc -DPLUGIN=1 -g -O2 -fno-strict-aliasing -Wall -Wstrict-prototypes '-DPPPOE_PATH="/usr/sbin/pppoe"' '-DPPPD_PATH="/usr/sbin/pppd"' '-DPLUGIN_PATH="/etc/ppp/plugins/rp-pppoe.so"' '-DPPPOE_SERVER_OPTIONS="/etc/ppp/pppoe-server-options"' -Ilibevent '-DRP_VERSION="3.15"' -I/usr/include -c -o plugin/debug.o -fPIC debug.c
ar -rc plugin/libplugin.a plugin/discovery.o plugin/if.o plugin/common.o plugin/debug.o
gcc -o rp-pppoe.so -shared plugin/plugin.o plugin/libplugin.a
Type 'make install' as root to install the software.
Ugh, wow. :slightly_smiling_face:
OK, when I have time, I will look at the fixes done in the ppp tree and merge them into rp-pppoe (and will probably need to make additional fixes of my own for code that's not in the ppp tree.)
I'll set up an Ubuntu LXC container at some point... but $DAY_JOB is becoming a bit demanding, so no ETA on this.
Regards,
Dianne.
@dfskoll that is good to know. I'd like to hear it from @jkroonza to why he needed the one from rp-pppoe package (or better yet trying to understand why anyone would want the one from rp-pppoe package). Again, this is some of the pain / confusion of forking a project ...
They are in my experience inter-changeable. On Gentoo we've been renaming rp-pppoe from ppp package to pppoe for a while now to avoid the naming conflict, and just installed both, user can choose which he prefers to use.
The plugin is only required for kernel mode. user-mode uses pty option which runs a binary specifically encapsulating and decapsulating the pppoe traffic in userspace compared to kernel mode. This is inefficient and won't scale, but it's sufficient in some cases (like running a small number of connections).
My recommendation remains:
Plugin in ppp project (for the pure and simple reason that it's convenient for the majority of pppoe users, just install ppp and you've got pppoe client support - majority of pppoe users).
Everything else in rp-pppoe package.
Gentoo originally had rp-pppoe merged predominantly for client support (early 2000's). I highly doubt the server components would have worked reliably until the work I did in the last couple of years.
OK. I think the way forward is to clean up the code so the plugin in the ppp project is up-to-date with the changes that have been made in rp-pppoe; it will then become the One True PPPoE implementation.
On the rp-pppoe side, I think we should get rid of the user-space program pppoe since if ppp ships with the plugin, there's no reason for it. Probably also clean things up to get rid of the DLPI code, the tkpppoe GUI and the pppoe-connect, etc scripts that (I think) nobody uses any more.
rp-pppoe will then become just a relay and server implementation, dependent on the plugin that ships with the ppp project. Does that make sense?
@jkroonza I am slightly confused by your response. Yes, if you use pppd via a pty option to start the pppoe negotiation; I would understand it, but if you run it directly:
/usr/sbin/pppd user [email protected] password secret debug maxfail 0 persist connect true plugin pppoe.so bond0.3 nodetach
Wouldn't that start the pppoe via the Ethernet interface directly using a AF_PPPOX socket? Let there be known, I don't know the data path of these packets by just reviewing the code. Looks like rp-pppoe in addition has capability to use BPF filters or AF_PACKET / RAW sockets to grab packets it is interested in. While the rp-pppoe.so does have additional code in if.c (1100 lines vs 250 lines in pppd/pppoe's if.c), I don't see them being so much different.
Could someone please enlighten me? It seems like using kernel mode should be possible using either version of the plugin, and if not; there should be possible to update the pppd/pppoe plugin to do so.
@dfskoll You may want to consider the change history in pppd/pppoe to figure out what have changed and to why. Looks like @pali helped fixing a bunch of stuff here too.