p4c-xdp icon indicating copy to clipboard operation
p4c-xdp copied to clipboard

changing reject to accept introduce stack increaseing

Open eggqq007 opened this issue 5 years ago • 9 comments

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 avatar Jun 05 '19 00:06 eggqq007

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

xdp_test_smallstack.txt xdp_test_bigstack.txt

eggqq007 avatar Jun 05 '19 01:06 eggqq007

ping......

eggqq007 avatar Jun 12 '19 00:06 eggqq007

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

mihaibudiu avatar Jun 13 '19 09:06 mihaibudiu

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 avatar Jun 15 '19 09:06 eggqq007

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

eggqq007 avatar Jun 15 '19 09:06 eggqq007

Do you plan to contribute these changes to our code?

mihaibudiu avatar Jun 17 '19 17:06 mihaibudiu

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.

eggqq007 avatar Jun 19 '19 00:06 eggqq007

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

williamtu avatar Jun 19 '19 20:06 williamtu

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

eggqq007 avatar Jun 20 '19 02:06 eggqq007