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

Can't set 48 bit field in header #53

Open
jacojoubert1 opened this issue Oct 24, 2017 · 11 comments
Open

Can't set 48 bit field in header #53

jacojoubert1 opened this issue Oct 24, 2017 · 11 comments
Labels

Comments

@jacojoubert1
Copy link

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 mihaibudiu added the bug label Oct 24, 2017
@mihaibudiu
Copy link
Contributor

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

@mihaibudiu
Copy link
Contributor

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
Copy link
Contributor

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

@jacojoubert1
Copy link
Author

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
Copy link
Contributor

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
Copy link
Contributor

If you integrate the pending PR p4lang/p4c#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
Copy link
Contributor

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

@mihaibudiu
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

I see, thanks!

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

No branches or pull requests

3 participants