Skip to content

Commit

Permalink
Revert "Helper reallocate packet (#3110)" (#3263)
Browse files Browse the repository at this point in the history
This reverts commit 13eed74.
  • Loading branch information
shankarseal authored Feb 15, 2024
1 parent 6150135 commit 66087c5
Show file tree
Hide file tree
Showing 12 changed files with 12 additions and 114 deletions.
1 change: 0 additions & 1 deletion docs/eBpfExtensions.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ Helper function IDs for different program types need not be unique.
* `return_type`: Set the appropriate value for the `ebpf_return_type_t` enum that represents the return type of the
helper function.
* `arguments`: Array of (at most) five helper function arguments of type `ebpf_argument_type_t`.
* `reallocate_packet`: Flag indicating if this helper function performs packet reallocation.

#### `ebpf_argument_type_t` Enum
This enum describes the various argument types that can be passed to an eBPF helper function. This is defined in the
Expand Down
5 changes: 0 additions & 5 deletions include/ebpf_program_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,12 @@

#include <guiddef.h>
#if !defined(NO_CRT) && !defined(_NO_CRT_STDIO_INLINE)
#include <stdbool.h>
#include <stdint.h>
#else
typedef unsigned char uint8_t;
typedef unsigned int uint32_t;
typedef unsigned long long uint64_t;
typedef unsigned short wchar_t;
#define bool _Bool
#endif

#define EBPF_MAX_PROGRAM_DESCRIPTOR_NAME_LENGTH 256
Expand All @@ -29,15 +27,12 @@ typedef struct _ebpf_program_type_descriptor
char is_privileged;
} ebpf_program_type_descriptor_t;

#define HELPER_FUNCTION_REALLOCATE_PACKET 0x1

typedef struct _ebpf_helper_function_prototype
{
uint32_t helper_id;
const char* name;
ebpf_return_type_t return_type;
ebpf_argument_type_t arguments[5];
bool reallocate_packet : 1;
} ebpf_helper_function_prototype_t;

typedef struct _ebpf_program_info
Expand Down
7 changes: 1 addition & 6 deletions libs/api_common/store_helper_internal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,8 @@ _load_helper_prototype(

// Read serialized helper prototype information.
char serialized_data[sizeof(ebpf_helper_function_prototype_t)] = {0};
bool reallocate_packet = false;
size_t expected_size = sizeof(helper_prototype->helper_id) + sizeof(helper_prototype->return_type) +
sizeof(helper_prototype->arguments) + sizeof(reallocate_packet);
sizeof(helper_prototype->arguments);

status = ebpf_read_registry_value_binary(
helper_info_key, EBPF_HELPER_DATA_PROTOTYPE, (uint8_t*)serialized_data, expected_size);
Expand All @@ -67,10 +66,6 @@ _load_helper_prototype(
memcpy(&helper_prototype->arguments, serialized_data + offset, sizeof(helper_prototype->arguments));
offset += sizeof(helper_prototype->arguments);

memcpy(&reallocate_packet, serialized_data + offset, sizeof(reallocate_packet));
helper_prototype->reallocate_packet = reallocate_packet ? HELPER_FUNCTION_REALLOCATE_PACKET : 0;
offset += sizeof(reallocate_packet);

helper_prototype->name =
cxplat_duplicate_string(ebpf_down_cast_from_wstring(std::wstring(helper_name)).c_str());
if (helper_prototype->name == nullptr) {
Expand Down
2 changes: 0 additions & 2 deletions libs/api_common/windows_helpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,5 @@ get_helper_prototype_windows(int32_t n)
verifier_prototype.argument_type[i] = raw_prototype->arguments[i];
}

verifier_prototype.reallocate_packet = raw_prototype->reallocate_packet == TRUE;

return verifier_prototype;
}
8 changes: 0 additions & 8 deletions libs/execution_context/ebpf_program.c
Original file line number Diff line number Diff line change
Expand Up @@ -2063,7 +2063,6 @@ _IRQL_requires_max_(PASSIVE_LEVEL) static ebpf_result_t _ebpf_program_compute_pr
// b. Helper name.
// c. Helper return type.
// d. Helper argument types.
// e. reallocate_packet flag (if set).

// Note:
// Order and fields being hashed is important. The order and fields being hashed must match the order and fields
Expand Down Expand Up @@ -2129,13 +2128,6 @@ _IRQL_requires_max_(PASSIVE_LEVEL) static ebpf_result_t _ebpf_program_compute_pr
goto Exit;
}
}

if (helper_function_prototype->reallocate_packet) {
result = EBPF_CRYPTOGRAPHIC_HASH_APPEND_STR(cryptographic_hash, "reallocate_packet");
if (result != EBPF_SUCCESS) {
goto Exit;
}
}
}
*hash_length = 0;
result = ebpf_cryptographic_hash_get_hash_length(cryptographic_hash, hash_length);
Expand Down
7 changes: 1 addition & 6 deletions libs/shared/ebpf_serialize.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ typedef struct _ebpf_serialized_helper_function_prototype
uint32_t helper_id;
ebpf_return_type_t return_type;
ebpf_argument_type_t arguments[5];
uint8_t reallocate_packet;
size_t name_length;
uint8_t name[1];
} ebpf_serialized_helper_function_prototype_t;
Expand Down Expand Up @@ -463,8 +462,6 @@ ebpf_serialize_program_info(
for (uint16_t index = 0; index < EBPF_COUNT_OF(helper_prototype->arguments); index++) {
serialized_helper_prototype->arguments[index] = helper_prototype->arguments[index];
}
serialized_helper_prototype->reallocate_packet =
helper_prototype->reallocate_packet ? HELPER_FUNCTION_REALLOCATE_PACKET : 0;
serialized_helper_prototype->name_length = helper_function_name_length;
// Copy the program type descriptor name buffer.
memcpy(serialized_helper_prototype->name, helper_prototype->name, helper_function_name_length);
Expand Down Expand Up @@ -630,14 +627,12 @@ ebpf_deserialize_program_info(
goto Exit;
}

// Deserialize helper prototype.
// Serialize helper prototype.
helper_prototype->helper_id = serialized_helper_prototype->helper_id;
helper_prototype->return_type = serialized_helper_prototype->return_type;
for (int i = 0; i < EBPF_COUNT_OF(helper_prototype->arguments); i++) {
helper_prototype->arguments[i] = serialized_helper_prototype->arguments[i];
}
helper_prototype->reallocate_packet =
serialized_helper_prototype->reallocate_packet == HELPER_FUNCTION_REALLOCATE_PACKET;

// Adjust remaining buffer length.
result = ebpf_safe_size_t_subtract(
Expand Down
5 changes: 1 addition & 4 deletions libs/store_helper/ebpf_store_helper.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation
// SPDX-License-Identifier: MIT

#include "ebpf_program_types.h"
#include "ebpf_registry_helper.h"
#include "ebpf_store_helper.h"
#include "ebpf_windows.h"
Expand Down Expand Up @@ -40,7 +41,6 @@ _ebpf_store_update_helper_prototype(
uint32_t offset;
ebpf_store_key_t helper_function_key = NULL;
char serialized_data[sizeof(ebpf_helper_function_prototype_t)] = {0};
const bool reallocate_packet = helper_info->reallocate_packet;

wchar_t* wide_helper_name = ebpf_get_wstring_from_string(helper_info->name);
if (wide_helper_name == NULL) {
Expand All @@ -63,9 +63,6 @@ _ebpf_store_update_helper_prototype(
memcpy(serialized_data + offset, helper_info->arguments, sizeof(helper_info->arguments));
offset += sizeof(helper_info->arguments);

memcpy(serialized_data + offset, &reallocate_packet, sizeof(reallocate_packet));
offset += sizeof(reallocate_packet);

// Save the helper prototype data.
result = ebpf_write_registry_value_binary(
helper_function_key, EBPF_HELPER_DATA_PROTOTYPE, (uint8_t*)&serialized_data[0], offset);
Expand Down
3 changes: 1 addition & 2 deletions netebpfext/net_ebpf_ext_program_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ static const ebpf_helper_function_prototype_t _xdp_test_ebpf_extension_helper_fu
{XDP_EXT_HELPER_FUNCTION_START + 1,
"bpf_xdp_adjust_head",
EBPF_RETURN_TYPE_INTEGER,
{EBPF_ARGUMENT_TYPE_PTR_TO_CTX, EBPF_ARGUMENT_TYPE_ANYTHING},
HELPER_FUNCTION_REALLOCATE_PACKET}};
{EBPF_ARGUMENT_TYPE_PTR_TO_CTX, EBPF_ARGUMENT_TYPE_ANYTHING}}};

// XDP_TEST program information.
static const ebpf_context_descriptor_t _ebpf_xdp_test_context_descriptor = {
Expand Down
23 changes: 0 additions & 23 deletions tests/end_to_end/netsh_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -380,29 +380,6 @@ TEST_CASE("show verification droppacket_unsafe.o", "[netsh][verification]")
"\n");
}

TEST_CASE("show verification xdp_adjust_head_unsafe.o", "[netsh][verification]")
{
_test_helper_netsh test_helper;
test_helper.initialize();

int result;
std::string output =
_run_netsh_command(handle_ebpf_show_verification, L"xdp_adjust_head_unsafe.o", L"xdp", nullptr, &result);
REQUIRE(result == ERROR_SUPPRESS_OUTPUT);
output = strip_paths(output);
REQUIRE(
output == "Verification failed\n"
"\n"
"Verification report:\n"
"\n"
"; ./tests/sample/unsafe/xdp_adjust_head_unsafe.c:42\n"
"; ethernet_header->Type = 0x0800;\n"
"17: Upper bound must be at most packet_size (valid_access(r1.offset+12, width=2) for write)\n"
"\n"
"1 errors\n"
"\n");
}

TEST_CASE("show verification printk_unsafe.o", "[netsh][verification]")
{
_test_helper_netsh test_helper;
Expand Down
46 changes: 0 additions & 46 deletions tests/sample/unsafe/xdp_adjust_head_unsafe.c

This file was deleted.

3 changes: 0 additions & 3 deletions tools/bpf2c/bpf2c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,6 @@ get_program_info_type_hash(const std::vector<int32_t>& actual_helper_ids, const
hash_t::append_byte_range(
byte_range, program_info->program_type_specific_helper_prototype[index].arguments[argument]);
}
if (program_info->program_type_specific_helper_prototype[index].reallocate_packet) {
hash_t::append_byte_range(byte_range, reinterpret_cast<const char*>("reallocate_packet"));
}
}
}
hash_t hash(algorithm);
Expand Down
16 changes: 8 additions & 8 deletions tools/bpf2c/bpf2c.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,13 @@
</ImportGroup>
<PropertyGroup Label="UserMacros" />
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">
<LinkIncremental>false</LinkIncremental>
<LinkIncremental>true</LinkIncremental>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='NativeOnlyDebug|x64'">
<LinkIncremental>false</LinkIncremental>
<LinkIncremental>true</LinkIncremental>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='FuzzerDebug|x64'">
<LinkIncremental>false</LinkIncremental>
<LinkIncremental>true</LinkIncremental>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'">
<LinkIncremental>false</LinkIncremental>
Expand All @@ -108,7 +108,7 @@
</ClCompile>
<Link>
<SubSystem>Console</SubSystem>
<GenerateDebugInformation>DebugFull</GenerateDebugInformation>
<GenerateDebugInformation>true</GenerateDebugInformation>
<AdditionalDependencies>$(FuzzerLibs);%(AdditionalDependencies)</AdditionalDependencies>
</Link>
<PreBuildEvent>
Expand All @@ -126,7 +126,7 @@
</ClCompile>
<Link>
<SubSystem>Console</SubSystem>
<GenerateDebugInformation>DebugFull</GenerateDebugInformation>
<GenerateDebugInformation>true</GenerateDebugInformation>
<AdditionalDependencies>$(FuzzerLibs);%(AdditionalDependencies)</AdditionalDependencies>
</Link>
<PreBuildEvent>
Expand All @@ -141,7 +141,7 @@
</ClCompile>
<Link>
<SubSystem>Console</SubSystem>
<GenerateDebugInformation>DebugFull</GenerateDebugInformation>
<GenerateDebugInformation>true</GenerateDebugInformation>
<AdditionalDependencies>$(FuzzerLibs);%(AdditionalDependencies)</AdditionalDependencies>
</Link>
<PreBuildEvent>
Expand All @@ -159,7 +159,7 @@
<SubSystem>Console</SubSystem>
<EnableCOMDATFolding>true</EnableCOMDATFolding>
<OptimizeReferences>true</OptimizeReferences>
<GenerateDebugInformation>DebugFull</GenerateDebugInformation>
<GenerateDebugInformation>true</GenerateDebugInformation>
<AdditionalDependencies>$(FuzzerLibs);%(AdditionalDependencies)</AdditionalDependencies>
</Link>
<PreBuildEvent>
Expand All @@ -177,7 +177,7 @@
<SubSystem>Console</SubSystem>
<EnableCOMDATFolding>true</EnableCOMDATFolding>
<OptimizeReferences>true</OptimizeReferences>
<GenerateDebugInformation>DebugFull</GenerateDebugInformation>
<GenerateDebugInformation>true</GenerateDebugInformation>
<AdditionalDependencies>$(FuzzerLibs);%(AdditionalDependencies)</AdditionalDependencies>
</Link>
<PreBuildEvent>
Expand Down

0 comments on commit 66087c5

Please sign in to comment.