issue in generating checksum on a payload

[sachintiptur]

Hi All, I am trying to do following things using xdp:

1. parse in coming packets
2. if packets are ICMP or PROFINET, extract the payload(anything after vlan hdr) and calculate checksum of it
3. store checksum in a map
4. make a pkt duplication check by doing map lookup, if its duplicate delete map entry and drop the pkt.
5. if pkt received for the first time, update map with calculated cksum and pass the pkt PS: source code files are attached

Issues:

1. trying to calculate icmp or profinet payload using bpf_csum_diff() call, but facing issues in passing right parameters. What pointers and size should be passed exactly in this case?
2. when i make the csum api somehow work by tweaking pointers and size, prog runs but there is network disruption seen and losing connectivity sometime. (cksum generation is still wrong but prog is compiling and running).

vm_elim.py.txt.txt

vm_pre.txt

Your help will be much appreciated. Thanks, Sachin

[tohojo]

sachints123 [email protected] writes:

Hi All, I am trying to do following things using xdp:

1. parse in coming packets
2. if packets are ICMP or PROFINET, extract the payload(anything after vlan hdr) and calculate checksum of it
3. store checksum in a map
4. make a pkt duplication check by doing map lookup, if its duplicate delete map entry and drop the pkt.
5. if pkt received for the first time, update map with calculated cksum and pass the pkt PS: source code files are attached

Issues:

1. trying to calculate icmp or profinet payload using bpf_csum_diff() call, but facing issues in passing right parameters. What pointers and size should be passed exactly in this case?

If you're just calculating a checksum wholesale, you'll just pass NULL as from, 0 as from_size, the ptr to the data as to, the data length as to_size and 0 as seed.

2. when i make the csum api somehow work by tweaking pointers and size, prog runs but there is network disruption seen and losing connectivity sometime. (cksum generation is still wrong but prog is compiling and running).

What do you mean 'network disruption'? Depending on your driver, the network device may reconfigure itself when you're loading the XDP program which can result in packet drops for a little while; is that what you mean? Or is your program just dropping packets?

[sachintiptur]

sachints123 [email protected] writes: Hi All, I am trying to do following things using xdp: 1. parse in coming packets 2. if packets are ICMP or PROFINET, extract the payload(anything after vlan hdr) and calculate checksum of it 3. store checksum in a map 4. make a pkt duplication check by doing map lookup, if its duplicate delete map entry and drop the pkt. 5. if pkt received for the first time, update map with calculated cksum and pass the pkt PS: source code files are attached Issues: 1. trying to calculate icmp or profinet payload using bpf_csum_diff() call, but facing issues in passing right parameters. What pointers and size should be passed exactly in this case? If you're just calculating a checksum wholesale, you'll just pass NULL as from, 0 as from_size, the ptr to the data as to, the data length as to_size and 0 as seed. 2. when i make the csum api somehow work by tweaking pointers and size, prog runs but there is network disruption seen and losing connectivity sometime. (cksum generation is still wrong but prog is compiling and running). What do you mean 'network disruption'? Depending on your driver, the network device may reconfigure itself when you're loading the XDP program which can result in packet drops for a little while; is that what you mean? Or is your program just dropping packets?

1. Yes i want to calculate checksum for whole payload not the diff. I had tried before they way you suggested but i got "unbounded memory access, use 'var &= const' or 'if (var < const)'" error. Here is the code: profiHdr = (void*)(data + nh_off); //nh_off is upto vlan hdr or ip hdr depending on pkt __u32 size = (data_end - profiHdr); bpf_trace_printk("insidee echo:%u\n",size); if(profiHdr > data_end) return rc; __u64 csum,val=0; csum = bpf_csum_diff(NULL, 0, (__be32*)profiHdr, size, 0); am i doing it right?

2. Yes, some of the pkts are getting dropped, and ssh terminal responds slowly. But not all packets are dropping

[tohojo]

sachints123 [email protected] writes:

sachints123 [email protected] writes: Hi All, I am trying to do following things using xdp: 1. parse in coming packets 2. if packets are ICMP or PROFINET, extract the payload(anything after vlan hdr) and calculate checksum of it 3. store checksum in a map 4. make a pkt duplication check by doing map lookup, if its duplicate delete map entry and drop the pkt. 5. if pkt received for the first time, update map with calculated cksum and pass the pkt PS: source code files are attached Issues: 1. trying to calculate icmp or profinet payload using bpf_csum_diff() call, but facing issues in passing right parameters. What pointers and size should be passed exactly in this case? If you're just calculating a checksum wholesale, you'll just pass NULL as from, 0 as from_size, the ptr to the data as to, the data length as to_size and 0 as seed. 2. when i make the csum api somehow work by tweaking pointers and size, prog runs but there is network disruption seen and losing connectivity sometime. (cksum generation is still wrong but prog is compiling and running). What do you mean 'network disruption'? Depending on your driver, the network device may reconfigure itself when you're loading the XDP program which can result in packet drops for a little while; is that what you mean? Or is your program just dropping packets?

1. Yes i want to calculate checksum for whole payload not the diff. I had tried before they way you suggested but i got "unbounded memory access, use 'var &= const' or 'if (var < const)'" error. Here is the code: profiHdr = (void*)(data + nh_off); //nh_off is upto vlan hdr or ip hdr depending on pkt __u32 size = (data_end - profiHdr); bpf_trace_printk("insidee echo:%u\n",size); if(profiHdr > data_end) return rc; __u64 csum,val=0; csum = bpf_csum_diff(NULL, 0, (__be32*)profiHdr, size, 0); am i doing it right?

That's the verifier telling you that you can't just take the size from the packet and pass it on to the header. You need to bound the buffer size by comparing to some fixed size.

I'm not sure if it'll actually be enough to just compare it to some big size, say:

if (size < 1500) /* should always be the case in practice / csum = bpf_csum_diff(NULL, 0, (__be32)profiHdr, size, 0);

2. Yes, some of the pkts are getting dropped, and ssh terminal responds slowly. But not all packets are dropping

Presumably that's because your program logic is dropping them? Likely because the checksum calculation goes awry and your logic ends up at XDP_DROP?

[sachintiptur]

sachints123 [email protected] writes:

sachints123 @.> writes: > Hi All, I am trying to do following things using xdp: 1. parse in coming packets 2. if packets are ICMP or PROFINET, extract the payload(anything after vlan hdr) and calculate checksum of it 3. store checksum in a map 4. make a pkt duplication check by doing map lookup, if its duplicate delete map entry and drop the pkt. 5. if pkt received for the first time, update map with calculated cksum and pass the pkt PS: source code files are attached Issues: 1. trying to calculate icmp or profinet payload using bpf_csum_diff() call, but facing issues in passing right parameters. What pointers and size should be passed exactly in this case? > If you're just calculating a checksum wholesale, you'll just pass NULL as from, 0 as from_size, the ptr to the data as to, the data length as to_size and 0 as seed. > 2. when i make the csum api somehow work by tweaking pointers and size, prog runs but there is network disruption seen and losing connectivity sometime. (cksum generation is still wrong but prog is compiling and running). > What do you mean 'network disruption'? Depending on your driver, the network device may reconfigure itself when you're loading the XDP program which can result in packet drops for a little while; is that what you mean? Or is your program just dropping packets? 1. Yes i want to calculate checksum for whole payload not the diff. I had tried before they way you suggested but i got "unbounded memory access, use 'var &= const' or 'if (var < const)'" error. Here is the code: profiHdr = (void)(data + nh_off); //nh_off is upto vlan hdr or ip hdr depending on pkt __u32 size = (data_end - profiHdr); bpf_trace_printk("insidee echo:%u\n",size); if(profiHdr > data_end) return rc; __u64 csum,val=0; csum = bpf_csum_diff(NULL, 0, (__be32)profiHdr, size, 0); am i doing it right? That's the verifier telling you that you can't just take the size from the packet and pass it on to the header. You need to bound the buffer size by comparing to some fixed size. I'm not sure if it'll actually be enough to just compare it to some big size, say: if (size < 1500) / should always be the case in practice / csum = bpf_csum_diff(NULL, 0, (__be32)profiHdr, size, 0);

2. Yes, some of the pkts are getting dropped, and ssh terminal responds slowly. But not all packets are dropping Presumably that's because your program logic is dropping them? Likely because the checksum calculation goes awry and your logic ends up at XDP_DROP?

1. Now I am getting below error after putting bound check(i tried checking size < for 48, 500,1500) "invalid access to packet, off=18 size=1499, R3(id=0,off=18,r=38) R3 offset is outside of the packet"

Code: int xdp_eliminate(struct CTXTYPE *ctx) {

void* data_end = (void*)(long)ctx->data_end;
void* data = (void*)(long)ctx->data;
struct ethhdr *eth = data;
struct iphdr *iphdr;
void* profiHdr;
struct icmphdr *icmphdr;

// drop packets
int rc = RETURNCODE; // let pass XDP_PASS 
uint16_t h_proto;
uint32_t type;
uint64_t nh_off = 0;

int key = 0;
int i;
__u16 echo_reply, old_csum;

/* char* out_mac = "OUT_MAC"; char* out_mac_1 = "OUT_MAC1"; */

nh_off = sizeof(*eth);
if (data + nh_off  > data_end)
    return rc;

h_proto = eth->h_proto;

if (h_proto == htons(ETH_P_8021Q) || h_proto == htons(ETH_P_8021AD)) {
            bpf_trace_printk("insidee vlan\\n");
    struct vlan_hdr *vhdr;
    vhdr = data + nh_off;
    nh_off += sizeof(struct vlan_hdr);
    if (data + nh_off > data_end)
        return rc;
        h_proto = vhdr->h_vlan_encapsulated_proto;
}

if (h_proto == htons(ETH_P_IP)){
            bpf_trace_printk("insidee ip\\n");
    iphdr = data + nh_off;
    if(iphdr > data_end) return rc;
            h_proto = parse_ipv4(data, nh_off, data_end);
    }
	
if (h_proto == 1) {
    bpf_trace_printk("insidee icmp\\n");
    nh_off += sizeof(struct iphdr);
    icmphdr = data + nh_off;
    if(icmphdr > data_end)
       return rc;
    type = parse_icmphdr(data, nh_off, data_end);
}
 if (h_proto == htons(ETH_P_PROFINET) || type == ICMP_ECHO) {
        profiHdr = (void*)(data + nh_off);
        __u32 size = (data_end - profiHdr);
		
        bpf_trace_printk("inside echo:%u\\n",size);
		
        if(profiHdr > data_end) return rc;
	
        __u64 csum,val=0;
           
        if(size < 1500)
            csum = bpf_csum_diff(NULL, 0, (__be32*)profiHdr, size, 0);

            int *elem;
            elem = cksum.lookup(&csum);
            if(elem) {
                rc = XDP_DROP;
                cksum.delete(&csum);
            } else {
            bpf_trace_printk("map update\\n");
                rc = XDP_PASS;
                cksum.update(&csum,&val);
            }
    }
    return rc;

}

2. Yes I think, logic is dropping pkts based on cksum
3. If i reduce the size check to below 24, I get a different error "124: (85) call bpf_map_lookup_elem#1 invalid indirect read from stack off -40+0 size 8" for map lookup.
4. Is there any bug in the verifier code? I am using kernel version "5.4.0-53-generic"

[tohojo]

sachints123 [email protected] writes:

sachints123 [email protected] writes:

sachints123 @.> writes: > Hi All, I am trying to do following things using xdp: 1. parse in coming packets 2. if packets are ICMP or PROFINET, extract the payload(anything after vlan hdr) and calculate checksum of it 3. store checksum in a map 4. make a pkt duplication check by doing map lookup, if its duplicate delete map entry and drop the pkt. 5. if pkt received for the first time, update map with calculated cksum and pass the pkt PS: source code files are attached Issues: 1. trying to calculate icmp or profinet payload using bpf_csum_diff() call, but facing issues in passing right parameters. What pointers and size should be passed exactly in this case? > If you're just calculating a checksum wholesale, you'll just pass NULL as from, 0 as from_size, the ptr to the data as to, the data length as to_size and 0 as seed. > 2. when i make the csum api somehow work by tweaking pointers and size, prog runs but there is network disruption seen and losing connectivity sometime. (cksum generation is still wrong but prog is compiling and running). > What do you mean 'network disruption'? Depending on your driver, the network device may reconfigure itself when you're loading the XDP program which can result in packet drops for a little while; is that what you mean? Or is your program just dropping packets? 1. Yes i want to calculate checksum for whole payload not the diff. I had tried before they way you suggested but i got "unbounded memory access, use 'var &= const' or 'if (var < const)'" error. Here is the code: profiHdr = (void)(data + nh_off); //nh_off is upto vlan hdr or ip hdr depending on pkt __u32 size = (data_end - profiHdr); bpf_trace_printk("insidee echo:%u\n",size); if(profiHdr > data_end) return rc; __u64 csum,val=0; csum = bpf_csum_diff(NULL, 0, (__be32)profiHdr, size, 0); am i doing it right? That's the verifier telling you that you can't just take the size from the packet and pass it on to the header. You need to bound the buffer size by comparing to some fixed size. I'm not sure if it'll actually be enough to just compare it to some big size, say: if (size < 1500) / should always be the case in practice / csum = bpf_csum_diff(NULL, 0, (__be32)profiHdr, size, 0);

2. Yes, some of the pkts are getting dropped, and ssh terminal responds slowly. But not all packets are dropping Presumably that's because your program logic is dropping them? Likely because the checksum calculation goes awry and your logic ends up at XDP_DROP?

1. Now I am getting below error after putting bound check(i tried checking size < for 48, 500,1500) "invalid access to packet, off=18 size=1499, R3(id=0,off=18,r=38) R3 offset is outside of the packet"

Yeah, this is because the verifier can't prove that you're not reading beyond the end of the packet. Your code is not technically wrong, it's just that the static analysis performed by the verifier can't prove this is the case.

Code: int xdp_eliminate(struct CTXTYPE *ctx) {

void* data_end = (void*)(long)ctx->data_end;
void* data = (void*)(long)ctx->data;
struct ethhdr *eth = data;
struct iphdr *iphdr;
void* profiHdr;
struct icmphdr *icmphdr;

// drop packets int rc = RETURNCODE; // let pass XDP_PASS uint16_t h_proto; uint32_t type; uint64_t nh_off = 0;

int key = 0;
int i;
__u16 echo_reply, old_csum;

/* char* out_mac = "OUT_MAC"; char* out_mac_1 = "OUT_MAC1"; */

nh_off = sizeof(*eth); if (data + nh_off > data_end) return rc;

h_proto = eth->h_proto;

if (h_proto == htons(ETH_P_8021Q) || h_proto == htons(ETH_P_8021AD)) {
            bpf_trace_printk("insidee vlan\\n");
    struct vlan_hdr *vhdr;
    vhdr = data + nh_off;
    nh_off += sizeof(struct vlan_hdr);
    if (data + nh_off > data_end)
        return rc;
        h_proto = vhdr->h_vlan_encapsulated_proto;
}

if (h_proto == htons(ETH_P_IP)){
            bpf_trace_printk("insidee ip\\n");
    iphdr = data + nh_off;
    if(iphdr > data_end) return rc;
            h_proto = parse_ipv4(data, nh_off, data_end);
    }
  
if (h_proto == 1) {
    bpf_trace_printk("insidee icmp\\n");
    nh_off += sizeof(struct iphdr);
    icmphdr = data + nh_off;
    if(icmphdr > data_end)
       return rc;
    type = parse_icmphdr(data, nh_off, data_end);
}

if (h_proto == htons(ETH_P_PROFINET) || type == ICMP_ECHO) { profiHdr = (void*)(data + nh_off); __u32 size = (data_end - profiHdr);

        bpf_trace_printk("inside echo:%u\\n",size);
  	
        if(profiHdr > data_end) return rc;
  
        __u64 csum,val=0;
           
        if(size < 1500)

Maybe try replacing this with: if(size < 1500 && profiHdr + size <= data_end)

            csum = bpf_csum_diff(NULL, 0, (__be32*)profiHdr, size, 0);

            int *elem;
            elem = cksum.lookup(&csum);
            if(elem) {
                rc = XDP_DROP;
                cksum.delete(&csum);
            } else {
            bpf_trace_printk("map update\\n");
                rc = XDP_PASS;
                cksum.update(&csum,&val);
            }
    }
    return rc;

}

2. Yes I think, logic is dropping pkts based on cksum

You may want to test on a veth so you don't lock yourself out if you drop the wrong packets :)

3. If i reduce the size check to below 24, I get a different error "124: (85) call bpf_map_lookup_elem#1 invalid indirect read from stack off -40+0 size 8" for map lookup.

I think this is because you're using a map key of size 4 (u32) with a map that uses 8-byte (u64) keys.

4. Is there any bug in the verifier code? I am using kernel version "5.4.0-53-generic"

The verifier errs on the side of safety, so if it can't prove your code is correct it will reject it. And in this case it can't, as mentioned above... Try expanding the if() like I proposed; that may help prove that you won't read past the packet...

[sachintiptur]

sachints123 [email protected] writes:

sachints123 @.> writes: > > sachints123 @.> writes: > Hi All, I am trying to do following things using xdp: 1. parse in coming packets 2. if packets are ICMP or PROFINET, extract the payload(anything after vlan hdr) and calculate checksum of it 3. store checksum in a map 4. make a pkt duplication check by doing map lookup, if its duplicate delete map entry and drop the pkt. 5. if pkt received for the first time, update map with calculated cksum and pass the pkt PS: source code files are attached Issues: 1. trying to calculate icmp or profinet payload using bpf_csum_diff() call, but facing issues in passing right parameters. What pointers and size should be passed exactly in this case? > If you're just calculating a checksum wholesale, you'll just pass NULL as from, 0 as from_size, the ptr to the data as to, the data length as to_size and 0 as seed. > 2. when i make the csum api somehow work by tweaking pointers and size, prog runs but there is network disruption seen and losing connectivity sometime. (cksum generation is still wrong but prog is compiling and running). > What do you mean 'network disruption'? Depending on your driver, the network device may reconfigure itself when you're loading the XDP program which can result in packet drops for a little while; is that what you mean? Or is your program just dropping packets? 1. Yes i want to calculate checksum for whole payload not the diff. I had tried before they way you suggested but i got "unbounded memory access, use 'var &= const' or 'if (var < const)'" error. Here is the code: profiHdr = (void*)(data + nh_off); //nh_off is upto vlan hdr or ip hdr depending on pkt __u32 size = (data_end - profiHdr); bpf_trace_printk("insidee echo:%u\n",size); if(profiHdr > data_end) return rc; __u64 csum,val=0; csum = bpf_csum_diff(NULL, 0, (__be32*)profiHdr, size, 0); am i doing it right? > That's the verifier telling you that you can't just take the size from the packet and pass it on to the header. You need to bound the buffer size by comparing to some fixed size. I'm not sure if it'll actually be enough to just compare it to some big size, say: if (size < 1500) /* should always be the case in practice / csum = bpf_csum_diff(NULL, 0, (__be32)profiHdr, size, 0); > 2. Yes, some of the pkts are getting dropped, and ssh terminal responds slowly. But not all packets are dropping > Presumably that's because your program logic is dropping them? Likely because the checksum calculation goes awry and your logic ends up at XDP_DROP? 1. Now I am getting below error after putting bound check(i tried checking size < for 48, 500,1500) "invalid access to packet, off=18 size=1499, R3(id=0,off=18,r=38) R3 offset is outside of the packet" Yeah, this is because the verifier can't prove that you're not reading beyond the end of the packet. Your code is not technically wrong, it's just that the static analysis performed by the verifier can't prove this is the case. Code: int xdp_eliminate(struct CTXTYPE ctx) { void data_end = (void*)(long)ctx->data_end; void* data = (void*)(long)ctx->data; struct ethhdr eth = data; struct iphdr iphdr; void profiHdr; struct icmphdr icmphdr; // drop packets int rc = RETURNCODE; // let pass XDP_PASS uint16_t h_proto; uint32_t type; uint64_t nh_off = 0; int key = 0; int i; __u16 echo_reply, old_csum; / char out_mac = "OUT_MAC"; char* out_mac_1 = "OUT_MAC1"; */ nh_off = sizeof(eth); if (data + nh_off > data_end) return rc; h_proto = eth->h_proto; if (h_proto == htons(ETH_P_8021Q) || h_proto == htons(ETH_P_8021AD)) { bpf_trace_printk("insidee vlan\n"); struct vlan_hdr vhdr; vhdr = data + nh_off; nh_off += sizeof(struct vlan_hdr); if (data + nh_off > data_end) return rc; h_proto = vhdr->h_vlan_encapsulated_proto; } if (h_proto == htons(ETH_P_IP)){ bpf_trace_printk("insidee ip\n"); iphdr = data + nh_off; if(iphdr > data_end) return rc; h_proto = parse_ipv4(data, nh_off, data_end); } if (h_proto == 1) { bpf_trace_printk("insidee icmp\n"); nh_off += sizeof(struct iphdr); icmphdr = data + nh_off; if(icmphdr > data_end) return rc; type = parse_icmphdr(data, nh_off, data_end); } if (h_proto == htons(ETH_P_PROFINET) || type == ICMP_ECHO) { profiHdr = (void)(data + nh_off); __u32 size = (data_end - profiHdr); bpf_trace_printk("inside echo:%u\n",size); if(profiHdr > data_end) return rc; __u64 csum,val=0; if(size < 1500) Maybe try replacing this with: if(size < 1500 && profiHdr + size <= data_end) csum = bpf_csum_diff(NULL, 0, (__be32)profiHdr, size, 0); int *elem; elem = cksum.lookup(&csum); if(elem) { rc = XDP_DROP; cksum.delete(&csum); } else { bpf_trace_printk("map update\n"); rc = XDP_PASS; cksum.update(&csum,&val); } } return rc; } 2. Yes I think, logic is dropping pkts based on cksum You may want to test on a veth so you don't lock yourself out if you drop the wrong packets :)

3. If i reduce the size check to below 24, I get a different error "124: (85) call bpf_map_lookup_elem#1 invalid indirect read from stack off -40+0 size 8" for map lookup. I think this is because you're using a map key of size 4 (u32) with a map that uses 8-byte (u64) keys.
4. Is there any bug in the verifier code? I am using kernel version "5.4.0-53-generic" The verifier errs on the side of safety, so if it can't prove your code is correct it will reject it. And in this case it can't, as mentioned above... Try expanding the if() like I proposed; that may help prove that you won't read past the packet...

1. No luck, i tried different boundary conditions for profiHdr check, it didnt work.
2. Also the map related error is also still present, i am using u64 key and u64 val for the maps.

BPF_HASH(cksum, u64,u64);

.... uint64_t csum; //...calculate csum.... .... uint64_t *elem; uint64_t val=0; elem = cksum.lookup(&csum); if(elem) { rc = XDP_DROP; cksum.delete(&csum); } else { bpf_trace_printk("map update\n"); rc = XDP_PASS; cksum.update(&csum,&val); } .....

[sachintiptur]

2. Initializing csum = 0 cleared the map lookup issue. Still debugging why bpf_csum_diff is not accepting header pointer

[tohojo]

sachints123 [email protected] writes:

2. Initializing csum = 0 cleared the map lookup issue.

Ah, right, the return value from csum is only 32-bits wide, so the 64-bit stack variable would not be initialised completely unless you do so in the code.

Still debugging why bpf_csum_diff is not accepting header pointer

Yeah, the "just give me the whole packet" thing is hard to get past the verifier. You could try allocating a fixed-size buffer and copy packet data into that, and then passing that buffer to the helper...

[sachintiptur]

I am trying to use fixed size buffer, but memcpy is not being allowed. I tried calling memcpy directly and as well as defining inline function __builtin_memcpy as per cilium guidebut didnt work. Any other way to do it?