vmware-archive / p4c-xdp

Backend for the P4 compiler targeting XDP
Apache License 2.0
171 stars 26 forks source link

Can't set 48 bit field in header #53

Open jacojoubert1 opened 6 years ago

jacojoubert1 commented 6 years ago

Trying to set 48 bit fields fields causes an error as shown below. Setting 32 bit fields does work.

It seems like the function memcpy tries to use the value to which the field must be set as an address: 18838586676582 = 0x112233445566 (the value to which the MAC address should be set) memcpy(&hd.outer_eth.destination, &18838586676582, 6);

For some reason I also can't use setValid(); inside an action. When using it directly inside the apply{} block it works.

P4 Code:

action l2l3_lookup(macAddr_t dmac, macAddr_t smac, ipAddr_t dip, ipAddr_t sip) {
        hd.outer_eth.destination = dmac;
        hd.outer_eth.source = smac;

        hd.outer_ipv4.dstAddr = dip;
        hd.outer_ipv4.srcAddr = sip;
    }

Errors:

p4c-xdp --Werror -I ../p4include --target xdp -o xdp_vlan_to_vxlan.c xdp_vlan_to_vxlan.p4;
clang \
    -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 -g -c xdp_vlan_to_vxlan.c -o -| llc -march=bpf -filetype=obj -o xdp_vlan_to_vxlan.o
xdp_vlan_to_vxlan.c:213:43: error: no member named 'setValid' in 'struct ipv4_t'
                            hd.outer_ipv4.setValid();
                            ~~~~~~~~~~~~~ ^
xdp_vlan_to_vxlan.c:233:47: error: cannot take the address of an rvalue of type
      'long'
            memcpy(&hd.outer_eth.destination, &18838586676582, 6);
                                              ^~~~~~~~~~~~~~~
xdp_vlan_to_vxlan.c:234:42: error: cannot take the address of an rvalue of type
      'long'
            memcpy(&hd.outer_eth.source, &37603585123959, 6);
                                         ^~~~~~~~~~~~~~~
3 errors generated.
sudo ./bpfloader xdp_vlan_to_vxlan.o || true
mihaibudiu commented 6 years ago

Can you please post a complete P4 program which we can add to the testsuite?

mihaibudiu commented 6 years ago

I thought I have fixed the setValid a while back. Are you using the latest version of the code? I could not reproduce the problem with setValid in an action.

mihaibudiu commented 6 years ago

I have pushed a PR that tries to fix this, but my testing is not very good. https://github.com/p4lang/p4c/pull/987 Can you see whether it works?

jacojoubert1 commented 6 years ago

The p4 I'm trying to compile is attached. I'm using the docker image provided to build and did a git pull to make sure I have the latest changes. Still got the errors above.

Also, does p4c-xdp support 'const entries' yet? I commented it out for now as I get syntax errors trying to implement it.

xdp_vlan_to_vxlan.p4.txt

mihaibudiu commented 6 years ago

Indeed, this back-end is much behind the other ones. There is no support for const entries yet. But please file issues for every one of these.

mihaibudiu commented 6 years ago

If you integrate the pending PR https://github.com/p4lang/p4c/pull/987 into the ebpf back-end, the code that is generated looks reasonable (but I didn't test it); hopefully someone will review the PR so we can merge it. I don't see any problems with isValid in actions. Please pull the latest versions for both p4c and p4c-xdp. I can also send you the output file by email so we can compare notes.

mihaibudiu commented 6 years ago

BTW: I filed this issue https://github.com/p4lang/p4c/issues/991 about table initializers. I have a bit of a back-log this week, but I can hopefully get to it next week.

mihaibudiu commented 6 years ago

We have merged the PR with support for up to 64-bit constants into the ebpf compiler. Can you see whether it solves this issue?

williamtu commented 6 years ago

I uncomment your xdp_vlan_to_vxlan.p4.txt and both setValid() and the const entries work OK on 4.14 kernel. Although I again hit some verifier issue, but it's not related to this.

mihaibudiu commented 6 years ago

We are not generating code for the initializers for tables. That's still TBD. It should be part of the ebpf compiler, though, and reused by xdp. We should probably generate that as part of the control-plane setup in the header file. I still have a back-log, and I am not sure I will get to it this week either.

williamtu commented 6 years ago

I see, thanks!