[PATCH] tools/ebpf: Fix IPv4 fragmentation identify

[PATCH] tools/ebpf: Fix IPv4 fragmentation identify

[luwenpeng]

The meaning of the bit flag in ip.frag_off is Don`t fragmented
When judging IPv4 is_fragmented according to ip.frag_off,
should use !(bpf_ntohs(ip.frag_off) & 0x4000) instead of !!ip.frag_off

Signed-off-by: WenPeng Lu <lwp_1994@163.com>
---
 tools/ebpf/rss.bpf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/ebpf/rss.bpf.c b/tools/ebpf/rss.bpf.c
index 20f227e2ac..e77b764f3d 100644
--- a/tools/ebpf/rss.bpf.c
+++ b/tools/ebpf/rss.bpf.c
@@ -317,7 +317,7 @@ static inline int parse_packet(struct __sk_buff *skb,
 
         info->in_src = ip.saddr;
         info->in_dst = ip.daddr;
-        info->is_fragmented = !!ip.frag_off;
+        info->is_fragmented = !(bpf_ntohs(ip.frag_off) & 0x4000);
 
         l4_protocol = ip.protocol;
         l4_offset = ip.ihl * 4;
-- 
2.30.1 (Apple Git-130)

Re: [PATCH] tools/ebpf: Fix IPv4 fragmentation identify

[Andrew Melnichenko]

Hi all,
I don't think that checking DF flag is a case for figuring out that
the packet is a fragment of some big datagram.
For nonfragmented packets, DF may not be set.
We need to check that the fragment offset is 0.
Actually, it's a good idea to check that MF flag is not set too. So we
can find the first fragment that doesn't require steering.
So the code may look something like this:
```
info->is_fragmented = !!(bpf_ntohs(ip.frag_off) & (0x2000 | 0x1fff));
// checking MF and frag offset.
 OR
info->is_fragmented = !!(bpf_ntohs(ip.frag_off) & ~0x4000); // if we
consider that CE always 0(which is always should be)
```

On Sun, Apr 23, 2023 at 9:12 AM Yan Vugenfirer <yvugenfi@redhat.com> wrote:
>
> FYI
>
> ---------- Forwarded message ---------
> From: luwenpeng <lwp_1994@163.com>
> Date: Fri, Apr 21, 2023 at 3:21 PM
> Subject: [PATCH] tools/ebpf: Fix IPv4 fragmentation identify
> To: <qemu-devel@nongnu.org>
> Cc: <jasowang@redhat.com>, luwenpeng <lwp_1994@163.com>
>
>
> The meaning of the bit flag in ip.frag_off is Don`t fragmented
> When judging IPv4 is_fragmented according to ip.frag_off,
> should use !(bpf_ntohs(ip.frag_off) & 0x4000) instead of !!ip.frag_off
>
> Signed-off-by: WenPeng Lu <lwp_1994@163.com>
> ---
>  tools/ebpf/rss.bpf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/ebpf/rss.bpf.c b/tools/ebpf/rss.bpf.c
> index 20f227e2ac..e77b764f3d 100644
> --- a/tools/ebpf/rss.bpf.c
> +++ b/tools/ebpf/rss.bpf.c
> @@ -317,7 +317,7 @@ static inline int parse_packet(struct __sk_buff *skb,
>
>          info->in_src = ip.saddr;
>          info->in_dst = ip.daddr;
> -        info->is_fragmented = !!ip.frag_off;
> +        info->is_fragmented = !(bpf_ntohs(ip.frag_off) & 0x4000);
>
>          l4_protocol = ip.protocol;
>          l4_offset = ip.ihl * 4;
> --
> 2.30.1 (Apple Git-130)
>
>

Re: [PATCH] tools/ebpf: Fix IPv4 fragmentation identify

[Andrew Melnichenko]

Hi all,
Also, for those changes to have an effect, a new eBPF skeleton must be
generated.

On Fri, Apr 28, 2023 at 1:58 PM Andrew Melnichenko <andrew@daynix.com> wrote:
>
> Hi all,
> I don't think that checking DF flag is a case for figuring out that
> the packet is a fragment of some big datagram.
> For nonfragmented packets, DF may not be set.
> We need to check that the fragment offset is 0.
> Actually, it's a good idea to check that MF flag is not set too. So we
> can find the first fragment that doesn't require steering.
> So the code may look something like this:
> ```
> info->is_fragmented = !!(bpf_ntohs(ip.frag_off) & (0x2000 | 0x1fff));
> // checking MF and frag offset.
>  OR
> info->is_fragmented = !!(bpf_ntohs(ip.frag_off) & ~0x4000); // if we
> consider that CE always 0(which is always should be)
> ```
>
> On Sun, Apr 23, 2023 at 9:12 AM Yan Vugenfirer <yvugenfi@redhat.com> wrote:
> >
> > FYI
> >
> > ---------- Forwarded message ---------
> > From: luwenpeng <lwp_1994@163.com>
> > Date: Fri, Apr 21, 2023 at 3:21 PM
> > Subject: [PATCH] tools/ebpf: Fix IPv4 fragmentation identify
> > To: <qemu-devel@nongnu.org>
> > Cc: <jasowang@redhat.com>, luwenpeng <lwp_1994@163.com>
> >
> >
> > The meaning of the bit flag in ip.frag_off is Don`t fragmented
> > When judging IPv4 is_fragmented according to ip.frag_off,
> > should use !(bpf_ntohs(ip.frag_off) & 0x4000) instead of !!ip.frag_off
> >
> > Signed-off-by: WenPeng Lu <lwp_1994@163.com>
> > ---
> >  tools/ebpf/rss.bpf.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/ebpf/rss.bpf.c b/tools/ebpf/rss.bpf.c
> > index 20f227e2ac..e77b764f3d 100644
> > --- a/tools/ebpf/rss.bpf.c
> > +++ b/tools/ebpf/rss.bpf.c
> > @@ -317,7 +317,7 @@ static inline int parse_packet(struct __sk_buff *skb,
> >
> >          info->in_src = ip.saddr;
> >          info->in_dst = ip.daddr;
> > -        info->is_fragmented = !!ip.frag_off;
> > +        info->is_fragmented = !(bpf_ntohs(ip.frag_off) & 0x4000);
> >
> >          l4_protocol = ip.protocol;
> >          l4_offset = ip.ihl * 4;
> > --
> > 2.30.1 (Apple Git-130)
> >
> >