Skip to content
This repository has been archived by the owner on Jan 19, 2023. It is now read-only.

changing reject to accept introduce stack increaseing #113

Open
eggqq007 opened this issue Jun 5, 2019 · 9 comments
Open

changing reject to accept introduce stack increaseing #113

eggqq007 opened this issue Jun 5, 2019 · 9 comments

Comments

@eggqq007
Copy link

eggqq007 commented Jun 5, 2019

Hi William and Mihai

I am trying to use p4x-xdp to make a simple loadbalancer. I found the stack increases if I change the "reject" to "accept" in parser. The stack increase from from 248byte to 440byte. It means I can only process one or two type of packets
I am not sure if it is llvm issue. Did you found a workaround for it?

parser Parser(packet_in packet, out Headers hd) {
    state start {
        packet.extract(hd.ethernet);
        transition select(hd.ethernet.protocol) {
            ETH_P_IPV4: parse_ipv4;
            default: accept; //change reject to accept, the stack size increase from 248 to 440
        }
    }
    state parse_ipv4 {
        packet.extract(hd.ipv4);
        transition select(hd.ipv4.protocol) {
            IPPROTO_TCP: parse_tcp;
            default: reject;
        }
    }

    state parse_tcp {
        packet.extract(hd.tcp);
        transition accept;
    }
}

the entire p4 example(
xdp_test.txt
) is uploaded. Please rename xdp_test.txt to xdp_test.p4.

The following command I used to compiled and load:

~/work/p4c_view1/p4c/extensions/p4c-xdp/p4c-xdp --target xdp -o ./xdp_test.c xdp_test.p4  && clang -g -D__KERNEL__ -D__ASM_SYSREG_H  -Wno-unused-value -Wno-pointer-sign -Wno-compare-distinct-pointer-types -Wno-gnu-variable-sized-type-not-at-end -Wno-tautological-compare -O2 -emit-llvm -c /root/work/p4_vgw/xdp_test.c -o -| llc -march=bpf -filetype=obj -o ./xdp_test.o && ip -force  link set dev ens160 xdp obj ./xdp_test.o verb

My env is :

Ubuntu 18.04

root@gzhenyu-ubuntu:~/work/p4_vgw# llc --version
LLVM (http://llvm.org/):
  LLVM version 6.0.0
  
  Optimized build.
  Default target: x86_64-pc-linux-gnu
  Host CPU: sandybridge

root@gzhenyu-ubuntu:~/work/p4_vgw# clang --version 
clang version 6.0.0-1ubuntu2 (tags/RELEASE_600/final)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
@eggqq007
Copy link
Author

eggqq007 commented Jun 5, 2019

Upload the llvm-objdump -S -no-show-raw-insn xdp_test.o files:

xdp_test_smallstack.txt
xdp_test_bigstack.txt

@eggqq007
Copy link
Author

ping......

@mihaibudiu
Copy link
Contributor

I don't know if there is much we can say about this; we do not have any control on how clang generates code.

@eggqq007
Copy link
Author

OK....I found a workaround.
I rewrite some logical in ControlBodyTranslator::compileEmitField to consume memcpy, and eliminate the write_byte. It helps reduce the stack from about 350 to 128byte. And performance is better also.

I also rewrite some logical in ControlBodyTranslator::preorder to help to judge if we should rewrite re-populate a header.

And also fix the issue that using if-else in control.("if" emit a useless ";" in if-else statement which cause compile error)

I found the counter only support 32bit which is too small, so increase it from 32bit to 64bit if using counterArray which type is hash.

@eggqq007
Copy link
Author

introducing 64bit counter: 2.7Mpps -->2.5Mpps
using memcpy and eliminating write_byte & avoid re-populate packet's header: 2.5Mpps --> 2.8Mpps

@mihaibudiu
Copy link
Contributor

Do you plan to contribute these changes to our code?

@eggqq007
Copy link
Author

It is draft code and I am afraid I don't have enough time to make it become a accepted patch recently.
I will upload my draft code diff first in case someone need it.

@williamtu
Copy link
Contributor

if you're allowed to change kernel, then you can increase MAX_BPF_STACK to a little larger value.

@eggqq007
Copy link
Author

Not only the linux stack size, the llvm-6 and llvm-7 would hit error in compiling if the stack size above 512bytes...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
3 participants