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

XDP header declarations belong in XDP-for-Windows repo, not eBPF-for-Windows #3736

Closed
mtfriesen opened this issue Jul 29, 2024 · 3 comments · Fixed by #3793
Closed

XDP header declarations belong in XDP-for-Windows repo, not eBPF-for-Windows #3736

mtfriesen opened this issue Jul 29, 2024 · 3 comments · Fixed by #3793
Assignees
Labels
bug Something isn't working triaged Discussed in a triage meeting
Milestone

Comments

@mtfriesen
Copy link
Collaborator

Describe the bug

The ebpf_nethooks.h header file defines XDP structs hooks, but those should be defined at https://github.com/microsoft/xdp-for-windows/

typedef struct xdp_md_
{
    void* data;               ///< Pointer to start of packet data.
    void* data_end;           ///< Pointer to end of packet data.
    uint64_t data_meta;       ///< Packet metadata.
    uint32_t ingress_ifindex; ///< Ingress interface index.

    /* size: 26, cachelines: 1, members: 4 */
    /* last cacheline: 26 bytes */
} xdp_md_t;

typedef enum _xdp_action
{
    XDP_PASS = 1, ///< Allow the packet to pass.
    XDP_DROP,     ///< Drop the packet.
    XDP_TX        ///< Bounce the received packet back out the same NIC it arrived on.
} xdp_action_t;

/**
 * @brief Handle an incoming packet as early as possible.
 *
 * Program type: \ref EBPF_PROGRAM_TYPE_XDP_TEST
 *
 * @param[in] context Packet metadata.
 * @retval XDP_PASS Allow the packet to pass.
 * @retval XDP_DROP Drop the packet.
 * @retval XDP_TX Bounce the received packet back out the same NIC it arrived on.
 */
typedef xdp_action_t
xdp_hook_t(xdp_md_t* context);

// XDP_TEST helper functions.
#define XDP_EXT_HELPER_FN_BASE 0xFFFF

#ifndef __doxygen
#define EBPF_HELPER(return_type, name, args) typedef return_type(*const name##_t) args
#endif

typedef enum
{
    BPF_FUNC_xdp_adjust_head = XDP_EXT_HELPER_FN_BASE + 1,
} ebpf_nethook_helper_id_t;

/**
 * @brief Adjust XDP_TEST context data pointer.
 *
 * @param[in] ctx XDP_TEST context.
 * @param[in] delta Number of bytes to move the data pointer by.
 *
 * @retval 0 The operation was successful.
 * @retval <0 A failure occurred.
 */
EBPF_HELPER(int, bpf_xdp_adjust_head, (xdp_md_t * ctx, int delta));
#ifndef __doxygen
#define bpf_xdp_adjust_head ((bpf_xdp_adjust_head_t)BPF_FUNC_xdp_adjust_head)
#endif

These should all be removed or mangled such that they are clearly for testing only.

OS information

N/A.

Steps taken to reproduce bug

Include ebpf_nethooks.h from the eBPF repo, and find xdp_md_t is already predefined, along with colliding XDP helper functions.

Expected behavior

eBPF should not define structs or helpers implemented in external extensions.

Actual outcome

eBPF defines XDP structs and helpers implemented in external extensions.

Additional details

No response

@mtfriesen mtfriesen added the bug Something isn't working label Jul 29, 2024
@shankarseal shankarseal added this to the 2408 milestone Aug 5, 2024
@shankarseal shankarseal added the triaged Discussed in a triage meeting label Aug 5, 2024
@shankarseal
Copy link
Collaborator

@mtfriesen I am assigning this bug to you for now. Please let me know if you want this to be re-assigned.

If you fix this issue, then please make sure that xdp-test prog-type is not broken, by making a copy of xdp_md_test_t struct and update all the sample code.

@mtfriesen
Copy link
Collaborator Author

@shankarseal please reassign - I will make the corresponding changes in the XDP repo. Thanks!

@mtfriesen mtfriesen assigned shankarseal and unassigned mtfriesen Aug 5, 2024
@shpalani
Copy link
Collaborator

shpalani commented Aug 5, 2024

Also fix the documentation, with the new changes in https://github.com/microsoft/ebpf-for-windows/blob/main/docs/tutorial.md

If you prefer, I can make the changes.

@shankarseal shankarseal assigned shpalani and unassigned shankarseal Aug 26, 2024
@shpalani shpalani linked a pull request Aug 27, 2024 that will close this issue
@shankarseal shankarseal modified the milestones: 2408, 2409 Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triaged Discussed in a triage meeting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants