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

ebpf: encode platform in type constants #1646

Closed
wants to merge 1 commit into from

Conversation

lmb
Copy link
Collaborator

@lmb lmb commented Jan 14, 2025

ebpf: encode platform in type constants

Linux and Windows share the concept of program types, map types and so on. 
For example, XDP is supported on both platforms. Unfortunately the constant 
values used by both platforms are different. On Linux, XDP has value 0x6 
while on Windows it is 0x1.

This problem extends to a CollectionSpec parsed from an ELF. Because of the 
different values a Windows PerCPUHash aliases with a Linux ProgramArray. 
This causes all sorts of mayhem that is hard to avoid without making too 
many changes to the code base.

Introduce the notion of a supported platform. External users only see this
as a string, like "linux". Internally each platform has a corresponding tag
which is used to encode the platform into a constant. This means that there
are distinct constants for each platform. Instead of overloading the XDP
constant we will introduce WindowsXDP. This will allow the ELF reader to
remain platform independent and makes generating stringers a lot easier.

The value of the platform tag is chosen so that on Linux the constant values
do not change and are identical with the value in upstream headers. On
Windows the constants have a fixed (but implementation defined) offset.

The downside of this approach is that Windows constants will all require a
prefix of some sort to distinguish them from Linux ones. This is probably 
most onerous for map types, because those tend to be created manually more 
frequently than programs, for example. We can add some behind the scenes 
translation of Linux map types to Windows ones if need be.

Another downside is that constant values are now limited to 28 bits since we
steal the top 4 bits to store the platform. The constants we're applying 
this to are all sequentially numbered from 0, so this is hopefully enough.

Signed-off-by: Lorenz Bauer <[email protected]>

@lmb lmb requested a review from ti-mo January 14, 2025 11:07
@github-actions github-actions bot added the breaking-change Changes exported API label Jan 14, 2025
@lmb lmb force-pushed the windows-platform-constants branch 2 times, most recently from 80c7a27 to 2d1f0b6 Compare January 14, 2025 11:14
asm/instruction.go Outdated Show resolved Hide resolved
Linux and Windows share the concept of program types, map types and so on.
For example, XDP is supported on both platforms. Unfortunately the constant
values used by both platforms are different. On Linux, XDP has value 0x6
while on Windows it is 0x1.

This problem extends to a CollectionSpec parsed from an ELF. Because of the
different values a Windows PerCPUHash aliases with a Linux ProgramArray.
This causes all sorts of mayhem that is hard to avoid without making too
many changes to the code base.

Introduce the notion of a supported platform. External users only see
this as a string, like "linux". Internally each platform has a
corresponding tag which is used to encode the platform into a constant.
This means that there are distinct constants for each platform.
Instead of overloading the XDP constant we will introduce WindowsXDP.
This will allow the ELF reader to remain platform independent and
makes generating stringers a lot easier.

The value of the platform tag is chosen so that on Linux the constant
values do not change and are identical with the value in upstream
headers. On Windows the constants have a fixed (but implementation
defined) offset.

The downside of this approach is that Windows constants will all require
a prefix of some sort to distinguish them from Linux ones. This is probably
most onerous for map types, because those tend to be created manually more
frequently than programs, for example. We can add some behind the scenes
translation of Linux map types to Windows ones if need be.

Another downside is that constant values are now limited to 28 bits since
we steal the top 4 bits to store the platform. The constants we're applying
this to are all sequentially numbered from 0, so this is hopefully enough.

Signed-off-by: Lorenz Bauer <[email protected]>
@lmb lmb force-pushed the windows-platform-constants branch from 2d1f0b6 to d0be3c9 Compare January 20, 2025 15:53
@lmb
Copy link
Collaborator Author

lmb commented Jan 20, 2025

@ti-mo I ended up using string instead of a new dedicated type. Usage within the library doesn't really change much. The upside is that we don't need to fiddle around with multiple package to hide LinuxTag. Downside is that the code is theoretically slower. I think it doesn't make a difference in practice.

@lmb lmb marked this pull request as ready for review January 20, 2025 17:06
@lmb lmb requested review from mmat11 and a team as code owners January 20, 2025 17:06
@lmb lmb marked this pull request as draft January 21, 2025 12:40
@lmb
Copy link
Collaborator Author

lmb commented Jan 21, 2025

Closing this for now, I discovered that adding Windows CI is pretty straight forward. #1650 is a better first PR.

@lmb lmb closed this Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Changes exported API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants