All of lore.kernel.org
 help / color / mirror / Atom feed
* XDP Software Issue - Payload Matching
@ 2020-05-08 13:57 Christian Deacon
  2020-05-11 10:41 ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 10+ messages in thread
From: Christian Deacon @ 2020-05-08 13:57 UTC (permalink / raw)
  To: xdp-newbies

Hey everyone,

I apologize if this isn't the correct place to discuss an issue with XDP 
software I'm attempting to create. I'm not sure where else I can request 
help on this since it may be related to BPF/XDP itself.

I've made an XDP Firewall project on GitHub here:

https://github.com/gamemann/XDP-Firewall

I am still fairly new to C and XDP. Therefore, I'm sure many 
improvements could be made to the software. However, everything besides 
the payload matching is working correctly from what I've tested.

Basically, this program transfers filtering rules from a config file in 
the user space to the XDP program via BPF maps. The XDP program then 
performs checks against each filter specified. I'm trying to implement 
payload matching into this project and I got the user-space side working 
properly. However, when I attempt to check the payload within the XDP 
program, I keep receiving errors either when compiling (stating the BPF 
stack has been exhausted) or the following error when attaching the XDP 
program:

```
The sequence of 8193 jumps is too complex.
processed 100132 insns (limit 1000000) max_states_per_insn 4 
total_states 1279 peak_states 1279 mark_read 97
```

There is a very long BPF stack trace that I can attach if need to be. 
The following is the part of code causing this issue (it's not commented 
out on my development VM):

https://github.com/gamemann/XDP-Firewall/blob/master/src/xdpfw_kern.c#L306

If I comment out line 332 or set `found` to 1, the XDP program does not 
crash. I've tried a `goto` approach as well which is available here:

https://gist.github.com/gamemann/9f0d42c25151d0f2e1630840d04fd599

However, this causes the following error when starting the XDP program:

```
invalid read from stack off -488+0 size 8
processed 844 insns (limit 1000000) max_states_per_insn 4 total_states 
28 peak_states 28 mark_read
```

If I comment out line 27 from that Gist (`continue;`), the program runs 
properly. I've also tried moving the code into its own for loop by 
making some modifications. However, I get the same results. I'd assume 
this is some sort of BPF limitation with for loops and jumps. However, 
I'm sure there's a strong possibility I'm just not doing something right 
when attempting to match the payload.

Does anyone know what I'm trying to do is possible within BPF/XDP?

The VM I'm using to develop this project and having the above issues is 
running Ubuntu 18.04 on kernel `5.6.1-050601-generic`.

If you need additional information, please let me know.

Any help is highly appreciated and thank you for your time!

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: XDP Software Issue - Payload Matching
  2020-05-08 13:57 XDP Software Issue - Payload Matching Christian Deacon
@ 2020-05-11 10:41 ` Toke Høiland-Jørgensen
  2020-05-11 18:40   ` Christian Deacon
  0 siblings, 1 reply; 10+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-05-11 10:41 UTC (permalink / raw)
  To: Christian Deacon, xdp-newbies

Christian Deacon <gamemann@gflclan.com> writes:

> Hey everyone,
>
> I apologize if this isn't the correct place to discuss an issue with XDP 
> software I'm attempting to create. I'm not sure where else I can request 
> help on this since it may be related to BPF/XDP itself.
>
> I've made an XDP Firewall project on GitHub here:
>
> https://github.com/gamemann/XDP-Firewall
>
> I am still fairly new to C and XDP. Therefore, I'm sure many 
> improvements could be made to the software. However, everything besides 
> the payload matching is working correctly from what I've tested.
>
> Basically, this program transfers filtering rules from a config file in 
> the user space to the XDP program via BPF maps. The XDP program then 
> performs checks against each filter specified. I'm trying to implement 
> payload matching into this project and I got the user-space side working 
> properly. However, when I attempt to check the payload within the XDP 
> program, I keep receiving errors either when compiling (stating the BPF 
> stack has been exhausted) or the following error when attaching the XDP 
> program:
>
> ```
> The sequence of 8193 jumps is too complex.
> processed 100132 insns (limit 1000000) max_states_per_insn 4 
> total_states 1279 peak_states 1279 mark_read 97
> ```
>
> There is a very long BPF stack trace that I can attach if need to be. 
> The following is the part of code causing this issue (it's not commented 
> out on my development VM):
>
> https://github.com/gamemann/XDP-Firewall/blob/master/src/xdpfw_kern.c#L306
>
> If I comment out line 332 or set `found` to 1, the XDP program does not 
> crash. I've tried a `goto` approach as well which is available here:
>
> https://gist.github.com/gamemann/9f0d42c25151d0f2e1630840d04fd599
>
> However, this causes the following error when starting the XDP program:
>
> ```
> invalid read from stack off -488+0 size 8
> processed 844 insns (limit 1000000) max_states_per_insn 4 total_states 
> 28 peak_states 28 mark_read
> ```
>
> If I comment out line 27 from that Gist (`continue;`), the program runs 
> properly. I've also tried moving the code into its own for loop by 
> making some modifications. However, I get the same results. I'd assume 
> this is some sort of BPF limitation with for loops and jumps. However, 
> I'm sure there's a strong possibility I'm just not doing something right 
> when attempting to match the payload.

Yeah, you're right, the verifier caps the size of the tree of branches
it'll explore at 8192 entries (which is why your error triggers at
8193). So to get past the verifier you'll simply have to limit the
complexity of your program.

A few techniques come to mind to achieve this:

- Limit the length of the loop (you have MAX_PCKT_LENGTH at 64k, which
  is never going to be the case since XDP doesn't support jumboframes).

- Split up your program into smaller components and use tail calls or
  non-inlined functions to call through to them (though not sure how far
  per-function verification has come, so the latter may not give you
  much benefit yet). Splitting up the code also helps with not running
  the bits that are not needed in a current filter configuration, which
  is an important way to improve performance of XDP programs. See
  xdp-filter[0] for an example of how to conditionally load different
  code subsets as needed.

- Use a matching algorithm that doesn't require looping through the
  packet byte-by-byte as you're doing now. For instance, you could have
  a hash map that uses the payload you're trying to match as the key
  with an appropriate chunk size.

-Toke

[0] https://github.com/xdp-project/xdp-tools/tree/master/xdp-filter

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: XDP Software Issue - Payload Matching
  2020-05-11 10:41 ` Toke Høiland-Jørgensen
@ 2020-05-11 18:40   ` Christian Deacon
  2020-05-12 14:28     ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 10+ messages in thread
From: Christian Deacon @ 2020-05-11 18:40 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, xdp-newbies

Hey Toke,

Thank you for your response and the information!

I actually found out last night about this as well, but when setting the 
max payload match length to 1500 bytes (instead of 65535), I receive a 
new error:

```
invalid read from stack off -488+0 size 8
processed 46277 insns (limit 1000000) max_states_per_insn 4 total_states 
617 peak_states 617 mark_read 599
```

Here's the new code:

```
if (filter[i]->payloadLen > 0)
{
     uint8_t found = 1;

     // MAX_PAYLOAD_LENGTH = 1500
     for (uint16_t j = 0; j < MAX_PAYLOAD_LENGTH; j++)
     {
         if ((j + 1) > filter[i]->payloadLen)
         {
             break;
         }

         uint8_t *byte = data + sizeof(struct ethhdr) + (iph->ihl * 4) + 
l4headerLen + j;

         if (byte + 1 > (uint8_t *)data_end)
         {
             break;
         }

         if (*byte == filter[i]->payloadMatch[j])
         {
             continue;
         }

         found = 0;

         break;
     }

     if (!found)
     {
         continue;
     }
}
```

This error occurs when initializing the `byte` pointer (if I comment out 
any lines with the `byte` pointer, the XDP program runs without 
crashing). Though, to my understanding, I am doing things correctly here 
along with ensuring the `byte` pointer is within the packet's range.

I am also going to look into implementing your second suggestion! I've 
been trying to think of ways to improve performance with the software 
and initially, I planned to only have filtering rules activated after a 
certain global PPS/BPS threshold is met (specified in the config file). 
However, it sounds like your suggestion would be more efficient.

Thank you for all your help!


On 5/11/2020 5:41 AM, Toke Høiland-Jørgensen wrote:
> Christian Deacon <gamemann@gflclan.com> writes:
>
>> Hey everyone,
>>
>> I apologize if this isn't the correct place to discuss an issue with XDP
>> software I'm attempting to create. I'm not sure where else I can request
>> help on this since it may be related to BPF/XDP itself.
>>
>> I've made an XDP Firewall project on GitHub here:
>>
>> https://github.com/gamemann/XDP-Firewall
>>
>> I am still fairly new to C and XDP. Therefore, I'm sure many
>> improvements could be made to the software. However, everything besides
>> the payload matching is working correctly from what I've tested.
>>
>> Basically, this program transfers filtering rules from a config file in
>> the user space to the XDP program via BPF maps. The XDP program then
>> performs checks against each filter specified. I'm trying to implement
>> payload matching into this project and I got the user-space side working
>> properly. However, when I attempt to check the payload within the XDP
>> program, I keep receiving errors either when compiling (stating the BPF
>> stack has been exhausted) or the following error when attaching the XDP
>> program:
>>
>> ```
>> The sequence of 8193 jumps is too complex.
>> processed 100132 insns (limit 1000000) max_states_per_insn 4
>> total_states 1279 peak_states 1279 mark_read 97
>> ```
>>
>> There is a very long BPF stack trace that I can attach if need to be.
>> The following is the part of code causing this issue (it's not commented
>> out on my development VM):
>>
>> https://github.com/gamemann/XDP-Firewall/blob/master/src/xdpfw_kern.c#L306
>>
>> If I comment out line 332 or set `found` to 1, the XDP program does not
>> crash. I've tried a `goto` approach as well which is available here:
>>
>> https://gist.github.com/gamemann/9f0d42c25151d0f2e1630840d04fd599
>>
>> However, this causes the following error when starting the XDP program:
>>
>> ```
>> invalid read from stack off -488+0 size 8
>> processed 844 insns (limit 1000000) max_states_per_insn 4 total_states
>> 28 peak_states 28 mark_read
>> ```
>>
>> If I comment out line 27 from that Gist (`continue;`), the program runs
>> properly. I've also tried moving the code into its own for loop by
>> making some modifications. However, I get the same results. I'd assume
>> this is some sort of BPF limitation with for loops and jumps. However,
>> I'm sure there's a strong possibility I'm just not doing something right
>> when attempting to match the payload.
> Yeah, you're right, the verifier caps the size of the tree of branches
> it'll explore at 8192 entries (which is why your error triggers at
> 8193). So to get past the verifier you'll simply have to limit the
> complexity of your program.
>
> A few techniques come to mind to achieve this:
>
> - Limit the length of the loop (you have MAX_PCKT_LENGTH at 64k, which
>    is never going to be the case since XDP doesn't support jumboframes).
>
> - Split up your program into smaller components and use tail calls or
>    non-inlined functions to call through to them (though not sure how far
>    per-function verification has come, so the latter may not give you
>    much benefit yet). Splitting up the code also helps with not running
>    the bits that are not needed in a current filter configuration, which
>    is an important way to improve performance of XDP programs. See
>    xdp-filter[0] for an example of how to conditionally load different
>    code subsets as needed.
>
> - Use a matching algorithm that doesn't require looping through the
>    packet byte-by-byte as you're doing now. For instance, you could have
>    a hash map that uses the payload you're trying to match as the key
>    with an appropriate chunk size.
>
> -Toke
>
> [0] https://github.com/xdp-project/xdp-tools/tree/master/xdp-filter
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: XDP Software Issue - Payload Matching
  2020-05-11 18:40   ` Christian Deacon
@ 2020-05-12 14:28     ` Toke Høiland-Jørgensen
  2020-05-13 13:25       ` Christian Deacon
  0 siblings, 1 reply; 10+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-05-12 14:28 UTC (permalink / raw)
  To: Christian Deacon, xdp-newbies

Christian Deacon <gamemann@gflclan.com> writes:

> Hey Toke,
>
> Thank you for your response and the information!
>
> I actually found out last night about this as well, but when setting the 
> max payload match length to 1500 bytes (instead of 65535), I receive a 
> new error:
>
> ```
> invalid read from stack off -488+0 size 8
> processed 46277 insns (limit 1000000) max_states_per_insn 4 total_states 
> 617 peak_states 617 mark_read 599
> ```

Hmm, this sounds like the verifier can't prove that your stack variable
(presumably the 'byte' pointer) is actually safe to read from the stack.
Not sure why - maybe because the variable is declared in the inner loop
and the verifier loses track? IDK, really...

> Here's the new code:
>
> ```
> if (filter[i]->payloadLen > 0)
> {
>      uint8_t found = 1;
>
>      // MAX_PAYLOAD_LENGTH = 1500
>      for (uint16_t j = 0; j < MAX_PAYLOAD_LENGTH; j++)
>      {
>          if ((j + 1) > filter[i]->payloadLen)
>          {
>              break;
>          }
>
>          uint8_t *byte = data + sizeof(struct ethhdr) + (iph->ihl * 4) + 
> l4headerLen + j;
>
>          if (byte + 1 > (uint8_t *)data_end)
>          {
>              break;
>          }
>
>          if (*byte == filter[i]->payloadMatch[j])
>          {
>              continue;
>          }
>
>          found = 0;
>
>          break;
>      }
>
>      if (!found)
>      {
>          continue;
>      }
> }
> ```
>
> This error occurs when initializing the `byte` pointer (if I comment out 
> any lines with the `byte` pointer, the XDP program runs without 
> crashing). Though, to my understanding, I am doing things correctly here 
> along with ensuring the `byte` pointer is within the packet's range.

Well, "doing something safely" and "convincing the verifier that what
you're doing is safe" is not always the same thing ;)

> I am also going to look into implementing your second suggestion! I've 
> been trying to think of ways to improve performance with the software 
> and initially, I planned to only have filtering rules activated after a 
> certain global PPS/BPS threshold is met (specified in the config file). 
> However, it sounds like your suggestion would be more efficient.
>
> Thank you for all your help!

You're welcome! :)

-Toke

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: XDP Software Issue - Payload Matching
  2020-05-12 14:28     ` Toke Høiland-Jørgensen
@ 2020-05-13 13:25       ` Christian Deacon
  2020-05-13 14:42         ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 10+ messages in thread
From: Christian Deacon @ 2020-05-13 13:25 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, xdp-newbies

Hey Toke,

Thank you for your response!

Unfortunately, I still haven't been able to get the for loop to work 
properly. I've also noticed if I use `iph->ihl * 4` when initializing 
the `byte` pointer, it produces the following error:

```
R5 !read_ok
processed 732 insns (limit 1000000) max_states_per_insn 4 total_states 
16 peak_states 16 mark_read 10
```

It seems I need to use a static size such as `sizeof(struct iphdr)`. 
Though, not all packets would have an IP header length of 20 bytes. I've 
tried performing checks with the length as well:

```
uint8_t len = iph->ihl * 4;

if (len < 20)
{
     return XDP_DROP;
}
else if (len > 36)
{
     return XDP_DROP;
}

// Setting len to 20 or any other value works fine.
// len = 20;

uint8_t *byte = data + sizeof(struct ethhdr) + len + l4headerLen;
```

However, no luck. I'm not sure what I can do to make BPF believe this is 
safe.

I was also wondering about the following:

 > Use a matching algorithm that doesn't require looping through the 
packet byte-by-byte as you're doing now. For instance, you could have a 
hash map that uses the payload you're trying to match as the key with an 
appropriate chunk size.

Do you know of any BPF Helper/kernel functions that can hash the 
payload? I looked at the BPF Helpers function list, but wasn't able to 
find anything for XDP sadly. I would like to attempt to implement 
something like this to see if I can avoid for loops since they aren't 
working well with BPF from what I've seen.

Thank you!


On 5/12/2020 9:28 AM, Toke Høiland-Jørgensen wrote:
> Christian Deacon <gamemann@gflclan.com> writes:
>
>> Hey Toke,
>>
>> Thank you for your response and the information!
>>
>> I actually found out last night about this as well, but when setting the
>> max payload match length to 1500 bytes (instead of 65535), I receive a
>> new error:
>>
>> ```
>> invalid read from stack off -488+0 size 8
>> processed 46277 insns (limit 1000000) max_states_per_insn 4 total_states
>> 617 peak_states 617 mark_read 599
>> ```
> Hmm, this sounds like the verifier can't prove that your stack variable
> (presumably the 'byte' pointer) is actually safe to read from the stack.
> Not sure why - maybe because the variable is declared in the inner loop
> and the verifier loses track? IDK, really...
>
>> Here's the new code:
>>
>> ```
>> if (filter[i]->payloadLen > 0)
>> {
>>       uint8_t found = 1;
>>
>>       // MAX_PAYLOAD_LENGTH = 1500
>>       for (uint16_t j = 0; j < MAX_PAYLOAD_LENGTH; j++)
>>       {
>>           if ((j + 1) > filter[i]->payloadLen)
>>           {
>>               break;
>>           }
>>
>>           uint8_t *byte = data + sizeof(struct ethhdr) + (iph->ihl * 4) +
>> l4headerLen + j;
>>
>>           if (byte + 1 > (uint8_t *)data_end)
>>           {
>>               break;
>>           }
>>
>>           if (*byte == filter[i]->payloadMatch[j])
>>           {
>>               continue;
>>           }
>>
>>           found = 0;
>>
>>           break;
>>       }
>>
>>       if (!found)
>>       {
>>           continue;
>>       }
>> }
>> ```
>>
>> This error occurs when initializing the `byte` pointer (if I comment out
>> any lines with the `byte` pointer, the XDP program runs without
>> crashing). Though, to my understanding, I am doing things correctly here
>> along with ensuring the `byte` pointer is within the packet's range.
> Well, "doing something safely" and "convincing the verifier that what
> you're doing is safe" is not always the same thing ;)
>
>> I am also going to look into implementing your second suggestion! I've
>> been trying to think of ways to improve performance with the software
>> and initially, I planned to only have filtering rules activated after a
>> certain global PPS/BPS threshold is met (specified in the config file).
>> However, it sounds like your suggestion would be more efficient.
>>
>> Thank you for all your help!
> You're welcome! :)
>
> -Toke
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: XDP Software Issue - Payload Matching
  2020-05-13 13:25       ` Christian Deacon
@ 2020-05-13 14:42         ` Toke Høiland-Jørgensen
  2020-05-22 14:49           ` Christian Deacon
  0 siblings, 1 reply; 10+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-05-13 14:42 UTC (permalink / raw)
  To: Christian Deacon, xdp-newbies; +Cc: Jesper Dangaard Brouer

Christian Deacon <gamemann@gflclan.com> writes:

> Hey Toke,
>
> Thank you for your response!
>
> Unfortunately, I still haven't been able to get the for loop to work 
> properly. I've also noticed if I use `iph->ihl * 4` when initializing 
> the `byte` pointer, it produces the following error:
>
> ```
> R5 !read_ok
> processed 732 insns (limit 1000000) max_states_per_insn 4 total_states 
> 16 peak_states 16 mark_read 10
> ```
>
> It seems I need to use a static size such as `sizeof(struct iphdr)`. 
> Though, not all packets would have an IP header length of 20 bytes. I've 
> tried performing checks with the length as well:
>
> ```
> uint8_t len = iph->ihl * 4;
>
> if (len < 20)
> {
>      return XDP_DROP;
> }
> else if (len > 36)
> {
>      return XDP_DROP;
> }
>
> // Setting len to 20 or any other value works fine.
> // len = 20;
>
> uint8_t *byte = data + sizeof(struct ethhdr) + len + l4headerLen;
> ```
>
> However, no luck. I'm not sure what I can do to make BPF believe this is 
> safe.

Hmm, maybe have a look at Jesper's experiments with getting to the end
of the packet:

https://github.com/xdp-project/xdp-tutorial/pull/123
https://github.com/xdp-project/xdp-tutorial/pull/124

Not sure if he ended up concluding anything definite about what the best
technique is :)

> I was also wondering about the following:
>
>  > Use a matching algorithm that doesn't require looping through the 
> packet byte-by-byte as you're doing now. For instance, you could have a 
> hash map that uses the payload you're trying to match as the key with an 
> appropriate chunk size.
>
> Do you know of any BPF Helper/kernel functions that can hash the 
> payload? I looked at the BPF Helpers function list, but wasn't able to 
> find anything for XDP sadly. I would like to attempt to implement 
> something like this to see if I can avoid for loops since they aren't 
> working well with BPF from what I've seen.

No, there's no direct hashing helper for XDP. I just meant that you
could use the (chunk of) the payload directly as a key in a hashmap.
Something like:

struct hash_key {
       u8 payload[CHUNK_SIZE];
}

int xdp_main(ctx) {
    struct hash_key lookup_key = {};
    int *verdict;

    [...]

    memcpy(&lookup_key, ctx->data, CHUNK_SIZE);
    verdict = bpf_map_lookup_elem(&lookup_key, ...);

    if (verdict)
      do_something_with(*verdict);

}

(You'd still need to convince the verifier that the memcpy from packet
data is safe, of course).

-Toke

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: XDP Software Issue - Payload Matching
  2020-05-13 14:42         ` Toke Høiland-Jørgensen
@ 2020-05-22 14:49           ` Christian Deacon
  2020-05-22 15:12             ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 10+ messages in thread
From: Christian Deacon @ 2020-05-22 14:49 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, xdp-newbies; +Cc: Jesper Dangaard Brouer

Hey Toke,

I apologize for the delay on this. I've been working on a couple other 
XDP/BPF projects recently.

Thank you for the information you provided!

Do you know of any open-source projects/examples that uses the method 
you're suggesting to compare packet data without using for/while loops? 
I haven't tried implementing the code yet, but I'm not entirely sure how 
I am going to do so.

I will give it a shot and I'll also take a look at the pull requests 
Jesper made :)

Thank you again!


On 5/13/2020 9:42 AM, Toke Høiland-Jørgensen wrote:
> Christian Deacon <gamemann@gflclan.com> writes:
>
>> Hey Toke,
>>
>> Thank you for your response!
>>
>> Unfortunately, I still haven't been able to get the for loop to work
>> properly. I've also noticed if I use `iph->ihl * 4` when initializing
>> the `byte` pointer, it produces the following error:
>>
>> ```
>> R5 !read_ok
>> processed 732 insns (limit 1000000) max_states_per_insn 4 total_states
>> 16 peak_states 16 mark_read 10
>> ```
>>
>> It seems I need to use a static size such as `sizeof(struct iphdr)`.
>> Though, not all packets would have an IP header length of 20 bytes. I've
>> tried performing checks with the length as well:
>>
>> ```
>> uint8_t len = iph->ihl * 4;
>>
>> if (len < 20)
>> {
>>       return XDP_DROP;
>> }
>> else if (len > 36)
>> {
>>       return XDP_DROP;
>> }
>>
>> // Setting len to 20 or any other value works fine.
>> // len = 20;
>>
>> uint8_t *byte = data + sizeof(struct ethhdr) + len + l4headerLen;
>> ```
>>
>> However, no luck. I'm not sure what I can do to make BPF believe this is
>> safe.
> Hmm, maybe have a look at Jesper's experiments with getting to the end
> of the packet:
>
> https://github.com/xdp-project/xdp-tutorial/pull/123
> https://github.com/xdp-project/xdp-tutorial/pull/124
>
> Not sure if he ended up concluding anything definite about what the best
> technique is :)
>
>> I was also wondering about the following:
>>
>>   > Use a matching algorithm that doesn't require looping through the
>> packet byte-by-byte as you're doing now. For instance, you could have a
>> hash map that uses the payload you're trying to match as the key with an
>> appropriate chunk size.
>>
>> Do you know of any BPF Helper/kernel functions that can hash the
>> payload? I looked at the BPF Helpers function list, but wasn't able to
>> find anything for XDP sadly. I would like to attempt to implement
>> something like this to see if I can avoid for loops since they aren't
>> working well with BPF from what I've seen.
> No, there's no direct hashing helper for XDP. I just meant that you
> could use the (chunk of) the payload directly as a key in a hashmap.
> Something like:
>
> struct hash_key {
>         u8 payload[CHUNK_SIZE];
> }
>
> int xdp_main(ctx) {
>      struct hash_key lookup_key = {};
>      int *verdict;
>
>      [...]
>
>      memcpy(&lookup_key, ctx->data, CHUNK_SIZE);
>      verdict = bpf_map_lookup_elem(&lookup_key, ...);
>
>      if (verdict)
>        do_something_with(*verdict);
>
> }
>
> (You'd still need to convince the verifier that the memcpy from packet
> data is safe, of course).
>
> -Toke
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: XDP Software Issue - Payload Matching
  2020-05-22 14:49           ` Christian Deacon
@ 2020-05-22 15:12             ` Toke Høiland-Jørgensen
  2020-07-14 15:58               ` Christian Deacon
  0 siblings, 1 reply; 10+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-05-22 15:12 UTC (permalink / raw)
  To: Christian Deacon, xdp-newbies; +Cc: Jesper Dangaard Brouer

Christian Deacon <gamemann@gflclan.com> writes:

> Hey Toke,
>
> I apologize for the delay on this. I've been working on a couple other 
> XDP/BPF projects recently.
>
> Thank you for the information you provided!
>
> Do you know of any open-source projects/examples that uses the method 
> you're suggesting to compare packet data without using for/while loops? 
> I haven't tried implementing the code yet, but I'm not entirely sure how 
> I am going to do so.

Well, having a map that uses IP addresses as lookup key is kinda the
same, I suppose, it's just a very limited part of the payload that's
being used as the key. But other than that, no, please consider this a
completely off-the-top-of-my-head idea with no warranties, implied or
otherwise :)

-Toke

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: XDP Software Issue - Payload Matching
  2020-05-22 15:12             ` Toke Høiland-Jørgensen
@ 2020-07-14 15:58               ` Christian Deacon
  2020-07-14 20:48                 ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 10+ messages in thread
From: Christian Deacon @ 2020-07-14 15:58 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, xdp-newbies

Hey Toke,


I apologize for the long delay on this. A lot has been going on recently!


I attempted to match payload data using the packet's payload as the BPF 
map key. Unfortunately, I didn't have any success with this. I stored my 
findings here from last month:


https://github.com/gamemann/XDP-Dynamic-Payload-Matching#section-methodfour-fail


I'd assume I may be missing something here, though.


I saw another XDP mailing list thread pop up recently regarding matching 
TCP payload data. I believe this may be what they're trying to achieve 
(being able to match dynamic payload data with XDP).


I was wondering if you had any other ideas on how we can match packet 
payload data against a BPF map.


Thank you for your time!


On 5/22/2020 10:12 AM, Toke Høiland-Jørgensen wrote:
> Christian Deacon <gamemann@gflclan.com> writes:
>
>> Hey Toke,
>>
>> I apologize for the delay on this. I've been working on a couple other
>> XDP/BPF projects recently.
>>
>> Thank you for the information you provided!
>>
>> Do you know of any open-source projects/examples that uses the method
>> you're suggesting to compare packet data without using for/while loops?
>> I haven't tried implementing the code yet, but I'm not entirely sure how
>> I am going to do so.
> Well, having a map that uses IP addresses as lookup key is kinda the
> same, I suppose, it's just a very limited part of the payload that's
> being used as the key. But other than that, no, please consider this a
> completely off-the-top-of-my-head idea with no warranties, implied or
> otherwise :)
>
> -Toke
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: XDP Software Issue - Payload Matching
  2020-07-14 15:58               ` Christian Deacon
@ 2020-07-14 20:48                 ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 10+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-07-14 20:48 UTC (permalink / raw)
  To: Christian Deacon, xdp-newbies

Christian Deacon <gamemann@gflclan.com> writes:

> Hey Toke,
>
>
> I apologize for the long delay on this. A lot has been going on recently!
>
>
> I attempted to match payload data using the packet's payload as the BPF 
> map key. Unfortunately, I didn't have any success with this. I stored my 
> findings here from last month:
>
>
> https://github.com/gamemann/XDP-Dynamic-Payload-Matching#section-methodfour-fail
>
>
> I'd assume I may be missing something here, though.
>
>
> I saw another XDP mailing list thread pop up recently regarding matching 
> TCP payload data. I believe this may be what they're trying to achieve 
> (being able to match dynamic payload data with XDP).
>
>
> I was wondering if you had any other ideas on how we can match packet 
> payload data against a BPF map.

That error ("invalid stack type R2 off=-16 access_size=150") comes from
this check in the verifier:

	if (off >= 0 || off < -MAX_BPF_STACK || off + access_size > 0 ||
	    access_size < 0 || (access_size == 0 && !zero_size_allowed)) {
		if (tnum_is_const(reg->var_off)) {
			verbose(env, "invalid stack type R%d off=%d access_size=%d\n",
				regno, off, access_size);
		} else {
                ..
                }
                return -EACCESS;
        }

which I think means that you're trying to use a 10-byte value as a
lookup key for a map that has a 150-byte key, which would make the
map key read through the end of the stack.

So basically, if you change

uint8_t hashkey[10];

to

uint8_t hashkey[150];

I think it ought to work?

-Toke

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-07-14 20:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-08 13:57 XDP Software Issue - Payload Matching Christian Deacon
2020-05-11 10:41 ` Toke Høiland-Jørgensen
2020-05-11 18:40   ` Christian Deacon
2020-05-12 14:28     ` Toke Høiland-Jørgensen
2020-05-13 13:25       ` Christian Deacon
2020-05-13 14:42         ` Toke Høiland-Jørgensen
2020-05-22 14:49           ` Christian Deacon
2020-05-22 15:12             ` Toke Høiland-Jørgensen
2020-07-14 15:58               ` Christian Deacon
2020-07-14 20:48                 ` Toke Høiland-Jørgensen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.