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

usm: http2: fix index calculation #20931

Merged
merged 3 commits into from
Nov 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions pkg/network/ebpf/c/protocols/http2/decoding.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,18 @@ static __always_inline http2_stream_t *http2_fetch_stream(const http2_stream_key
}

// Similar to read_var_int, but with a small optimization of getting the current character as input argument.
static __always_inline bool read_var_int_with_given_current_char(struct __sk_buff *skb, skb_info_t *skb_info, __u8 current_char_as_number, __u8 max_number_for_bits, __u8 *out) {
static __always_inline bool read_var_int_with_given_current_char(struct __sk_buff *skb, skb_info_t *skb_info, __u64 current_char_as_number, __u64 max_number_for_bits, __u64 *out) {
current_char_as_number &= max_number_for_bits;

if (current_char_as_number < max_number_for_bits) {
*out = current_char_as_number;
return true;
}

__u8 next_char = 0;
if (bpf_skb_load_bytes(skb, skb_info->data_off, &next_char, sizeof(next_char)) >=0 && (next_char & 128) == 0) {
__u64 next_char = 0;
if (bpf_skb_load_bytes(skb, skb_info->data_off, &next_char, 1) >=0 && (next_char & 128) == 0) {
skb_info->data_off++;
*out = current_char_as_number + next_char & 127;
*out = current_char_as_number + (next_char & 127);
return true;
}

Expand All @@ -68,9 +68,9 @@ static __always_inline bool read_var_int_with_given_current_char(struct __sk_buf
// n must always be between 1 and 8.
//
// The returned remain buffer is either a smaller suffix of p, or err != nil.
static __always_inline bool read_var_int(struct __sk_buff *skb, skb_info_t *skb_info, __u8 max_number_for_bits, __u8 *out) {
__u8 current_char_as_number = 0;
if (bpf_skb_load_bytes(skb, skb_info->data_off, &current_char_as_number, sizeof(current_char_as_number)) < 0) {
static __always_inline bool read_var_int(struct __sk_buff *skb, skb_info_t *skb_info, __u64 max_number_for_bits, __u64 *out) {
__u64 current_char_as_number = 0;
if (bpf_skb_load_bytes(skb, skb_info->data_off, &current_char_as_number, 1) < 0) {
return false;
}
skb_info->data_off++;
Expand Down Expand Up @@ -119,8 +119,8 @@ READ_INTO_BUFFER(path, HTTP2_MAX_PATH_LEN, BLK_SIZE)

// parse_field_literal handling the case when the key is part of the static table and the value is a dynamic string
// which will be stored in the dynamic table.
static __always_inline bool parse_field_literal(struct __sk_buff *skb, skb_info_t *skb_info, http2_header_t *headers_to_process, __u8 index, __u64 global_dynamic_counter, __u8 *interesting_headers_counter) {
__u8 str_len = 0;
static __always_inline bool parse_field_literal(struct __sk_buff *skb, skb_info_t *skb_info, http2_header_t *headers_to_process, __u64 index, __u64 global_dynamic_counter, __u8 *interesting_headers_counter) {
__u64 str_len = 0;
if (!read_var_int(skb, skb_info, MAX_6_BITS, &str_len)) {
return false;
}
Expand Down Expand Up @@ -162,8 +162,8 @@ static __always_inline __u8 filter_relevant_headers(struct __sk_buff *skb, skb_i
const __u32 end = frame_end < skb_info->data_end + 1 ? frame_end : skb_info->data_end + 1;
bool is_literal = false;
bool is_indexed = false;
__u8 max_bits = 0;
__u8 index = 0;
__u64 max_bits = 0;
__u64 index = 0;

__u64 *global_dynamic_counter = get_dynamic_counter(tup);
if (global_dynamic_counter == NULL) {
Expand Down
53 changes: 53 additions & 0 deletions pkg/network/usm/monitor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,59 @@ func TestHTTP2(t *testing.T) {
})
}

func (s *USMHTTP2Suite) TestHTTP2ManyDifferentPaths() {
t := s.T()
cfg := networkconfig.New()
cfg.EnableHTTP2Monitoring = true

startH2CServer(t)

monitor, err := NewMonitor(cfg, nil, nil, nil)
require.NoError(t, err)
require.NoError(t, monitor.Start())
defer monitor.Stop()

// Should be bigger than the length of the http2_dynamic_table which is 1024
numberOfRequests := 1500
clients := getClientsArray(t, 1)
for i := 0; i < numberOfRequests; i++ {
for j := 0; j < 2; j++ {
req, err := clients[0].Post(fmt.Sprintf("%s/test-%d", http2SrvAddr, i+1), "application/json", bytes.NewReader([]byte("test")))
require.NoError(t, err, "could not make request")
req.Body.Close()
}
}

matches := PrintableInt(0)

seenRequests := map[string]int{}
assert.Eventuallyf(t, func() bool {
stats := monitor.GetProtocolStats()
http2Stats, ok := stats[protocols.HTTP2]
if !ok {
return false
}
http2StatsTyped := http2Stats.(map[http.Key]*http.RequestStats)
for key, stat := range http2StatsTyped {
if (key.DstPort == http2SrvPort || key.SrcPort == http2SrvPort) && key.Method == http.MethodPost && strings.HasPrefix(key.Path.Content.Get(), "/test") {
if _, ok := seenRequests[key.Path.Content.Get()]; !ok {
seenRequests[key.Path.Content.Get()] = 0
}
seenRequests[key.Path.Content.Get()] += stat.Data[200].Count
matches.Add(stat.Data[200].Count)
}
}

return matches.Load() == 2*numberOfRequests
}, time.Second*10, time.Millisecond*100, "%v != %v", &matches, 2*numberOfRequests)

for i := 0; i < numberOfRequests; i++ {
if v, ok := seenRequests[fmt.Sprintf("/test-%d", i+1)]; !ok || v != 2 {
t.Logf("path: /test-%d should have 2 occurrences but instead has %d", i+1, v)
}
}
}

func (s *USMHTTP2Suite) TestSimpleHTTP2() {
t := s.T()
cfg := networkconfig.New()
Expand Down