Checksum calculation error when using PSA model

[yiding-zhou]

We are trying to use the PSA model for VXLAN encapsulation, the checksum calculation in the outer IP header is incorrect, and we have to modify the '.spec' file manually , otherwise the '.pb.bin' file cannot be loaded successfully.

The code segment that needs to be modified as below:

< mov m.Ingress_tmp h.outer_ipv4.ver_ihl < mov m.Ingress_tmp_0 h.outer_ipv4.diffserv < mov m.Ingress_tmp_1 h.outer_ipv4.total_len < mov m.Ingress_tmp_2 h.outer_ipv4.identification < mov m.Ingress_tmp_3 h.outer_ipv4.flags_offset < mov m.Ingress_tmp_4 h.outer_ipv4.ttl < mov m.Ingress_tmp_5 h.outer_ipv4.protocol < mov m.Ingress_tmp_6 h.outer_ipv4.src_ip < mov m.Ingress_tmp_7 h.outer_ipv4.dst_ip < ckadd h.cksum_state.state_0 m.Ingress_tmp < ckadd h.cksum_state.state_0 m.Ingress_tmp_0 < ckadd h.cksum_state.state_0 m.Ingress_tmp_1 < ckadd h.cksum_state.state_0 m.Ingress_tmp_2 < ckadd h.cksum_state.state_0 m.Ingress_tmp_3 < ckadd h.cksum_state.state_0 m.Ingress_tmp_4 < ckadd h.cksum_state.state_0 m.Ingress_tmp_5 < ckadd h.cksum_state.state_0 m.Ingress_tmp_6 < ckadd h.cksum_state.state_0 m.Ingress_tmp_7

-------- TO -----

  ckadd h.cksum_state.state_0 h.outer_ipv4.ver_ihl
  ckadd h.cksum_state.state_0 h.outer_ipv4.diffserv
  ckadd h.cksum_state.state_0 h.outer_ipv4.total_len
  ckadd h.cksum_state.state_0 h.outer_ipv4.identification
  ckadd h.cksum_state.state_0 h.outer_ipv4.flags_offset
  ckadd h.cksum_state.state_0 h.outer_ipv4.ttl
  ckadd h.cksum_state.state_0 h.outer_ipv4.protocol
  ckadd h.cksum_state.state_0 h.outer_ipv4.src_ip
  ckadd h.cksum_state.state_0 h.outer_ipv4.dst_ip

After this modification, the calculated checksum is also incorrect, and 0x10EF needs to be added.

  add h.outer_ipv4.hdr_checksum 0x10ef

I use ipdk (p4-ovs + dpdk)

environment: p4-dpdk-target -- commit : 3344147d9d5dd9e064ac2d98b5088426cc32e191 ovs -- commit : a0a623af26347d9fc441c35cd1126a12e75d8d97

p4c 1.2.3.1 (SHA: 6c225391b BUILD: RELEASE) commit : 6c225391b24cb3ec699d0bd87f20ac341417431c

By the way, this issue still exists on p4c's latest commit: 8ee2e65604968ed26331353dc34cbcf6402dcfe8

github doesn't support p4 and spec file, I just attach a tar file.

[usha1830]

This is a bug. ckadd instruction doesn't accept metadata fields as operand.

After this modification, the calculated checksum is also incorrect, and 0x10EF needs to be added.

Though ,I am not sure why this is required

[jfingerh]

It is sometimes necessary for checksum calculations to include TCP or UDP "pseudo headers", which do not actually appear in the packet. These are often stored in metadata fields that are then passed to the checksum calculation functions/methods.

[yiding-zhou]

@jfingerh @usha1830 thanks for your reply. Will this bug be fixed in next release?

[jfingerh]

@usha1830 Here is an example in another p4c test program using metadata and a constant as inputs to a checksum extern for v1model: https://github.com/p4lang/p4c/blob/main/testdata/p4_16_samples/checksum-l4-bmv2.p4#L263

[jfingerh]

And here is another example using PSA/PNA's incremental checksum, but still has a user-defined metadata field as a parameter: https://github.com/p4lang/p4c/blob/main/testdata/p4_16_samples/psa-example-incremental-checksum.p4#L216

[usha1830]

@jafingerhut Thanks for pointing to the example programs using metadata as input to checksum extern methods.

@yiding-zhou We will try to fix by next release. However, I would prefer to fix this in two parts: first fixing it to generate ckadd with header field input and then allowing metadata and constants as pointed out by Andy by moving them to some pseudo headers.

[mihaibudiu]

Anyone working on the dpdk backend looking at this bug?

[usha1830]

Yes @kamleshbhalui is working.

[usha1830]

@yiding-zhou Please check if this issue is resolved.

[kevin-intel]

@yiding-zhou Please check if this issue is resolved.

Hi, thanks for your work. I checked after the patch applied, and find out that we can load 'pb.bin' file successfully. But, the calculated checksum is also incorrect, and 0x10EF needs to be added.

I use ipdk (p4-ovs + dpdk)

environment: p4-dpdk-target -- commit : 3344147d9d5dd9e064ac2d98b5088426cc32e191 ovs -- commit : a0a623af26347d9fc441c35cd1126a12e75d8d97 p4c 1.2.3.1 (SHA: https://github.com/p4lang/p4c/commit/750e524a21537af676a2f2f281e78f660fd0d5c8 BUILD: RELEASE) commit : https://github.com/p4lang/p4c/commit/750e524a21537af676a2f2f281e78f660fd0d5c8

[usha1830]

Sorry for delayed reply.

But, the calculated checksum is also incorrect, and 0x10EF needs to be added.

Perhaps, this needs to be added in the P4 program, not by the compiler. @jafingerhut is that right?

[jfingerh]

@usha1830 I do not know where this number 0x10EF comes from. Is it part of the packet header contents over which the checksum is being calculated? Something else?

[usha1830]

@usha1830 I do not know where this number 0x10EF comes from. Is it part of the packet header contents over which the checksum is being calculated? Something else?

@kevin-intel @yiding-zhou

[usha1830]

@yiding-zhou @kevin-intel Further analysis of this issue uncovered an issue in the checksum calculation. The argument to ckadd instruction should be a multiple of 16-bit.

Thanks @jafingerhut and @kamalakannanr89 for pointing to this issue.

ckadd h.cksum_state.state_0 h.outer_ipv4.ver_ihl // Here the operand h.outer_ipv4.ver_ihl is 8-bit, hence 8 bits will be padded.

As a workaround, you could pass the whole header h.outer_ipv4 in the ckadd instruction. Since it aligns to 16-bit boundary ,the checksum will be correct.

@kamalakannanr89 can comment further on this requirement of aligning the ckadd operand to 16-bit.

[yiding-zhou]

@usha1830 very sorry for the late reply, calling ckadd with 16bits alignment solves the issue, thanks!