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.

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

PPPoE: miscellaneous improvements #140

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

laarmen
Copy link
Contributor

@laarmen laarmen commented Jun 23, 2020

In the course of some work on the PPPoE subsystem regarding the configuration reload, I made various modifications to the codebase that are only loosely related. This PR is the first of several for these changes.

These patches shouldn't change anything to the behavior of the program, and are only aimed at easing the development.

Copy link

@micron10 micron10 left a comment

Choose a reason for hiding this comment

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

With this patch not connect pppoe users with this error :

[2020-12-20 12:24:55]: debug: vlan-mon: notify 4 100 8863 0
[2020-12-20 12:24:55]: info: pppoe: create vlan vlan100 parent eth1
[2020-12-20 12:25:00]: warn: pppoe: discarding packet (unsupported version 0)
[2020-12-20 12:25:10]: warn: pppoe: discarding packet (unsupported version 0)
[2020-12-20 12:25:25]: info: pppoe: remove vlan vlan100

@laarmen
Copy link
Contributor Author

laarmen commented Dec 20, 2020

Hi, thank you for testing this, I appreciate it very much.

Could you share a bit more details about the error case? Is it on any client, or only some of them ? Are you running on amd64 or 32-bit x86?

@micron10
Copy link

micron10 commented Dec 20, 2020 via email

@micron10
Copy link

micron10 commented Dec 20, 2020 via email

@micron10
Copy link

micron10 commented Dec 21, 2020 via email

@micron10
Copy link

micron10 commented Dec 24, 2020 via email

@laarmen
Copy link
Contributor Author

laarmen commented Dec 24, 2020 via email

This way, the client code doesn't need to know the intrinsics of how
exactly the packet data is encoded within the payload, it can just
directly access the structure fields.

A small theoretical bonus is that it makes the code a bit more portable
as the size of an `int` isn't guaranteed to be 4 bytes on every
platform. Granted, it is 32-bit on all the platforms supported by
Debian, and the Linux kernel pretty much makes the same assumption, but
it doesn't cost anything.

v2: Fix the pointer arithmetic bug that rendered the whole patch
non-functional

Signed-off-by: Simon Chopin <s.chopin@alphalink.fr>

fixup
This patch is more for quality of life for development : if the headers
aren't self-contained, the tooling tends to complain about incomplete
types and undefined constants when editing the header file.

Signed-off-by: Simon Chopin <s.chopin@alphalink.fr>
@laarmen
Copy link
Contributor Author

laarmen commented Jan 11, 2021 via email

@micron10
Copy link

micron10 commented Jan 11, 2021 via email

@laarmen
Copy link
Contributor Author

laarmen commented Jan 12, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants