Re: [PATCH 0/1] tidbits: net-ping: add ping tidbit using raw socket

Am 05.07.26 um 18:49 schrieb Philippe Gerum:
> Hannes Diethelm <[email protected]> writes:
> 
>> Am 05.07.26 um 17:01 schrieb Philippe Gerum:
>>> Hannes Diethelm <[email protected]> writes:
>>>
>>>> This patch is the result from experimenting with evl networking. I use it to do basic
>>>> connection tests from the out of band network and check the timing.
>>>>
>>>> It is a bit complex due to evl doesn't support IPPROTO_ICMP sockets.
>>>>
>>>> Using the in-band network stack, you can use: sockfd = socket(AF_INET, SOCK_DGRAM, IPPROTO_ICMP);
>>>> and then only send the icmp part of the message. However, such a support is not really needed,
>>>> it works fine without.
>>>>
>>> This said, we have an ICMP protocol module in the evl stack since
>>> recently, which only responds to ICMP_ECHO requests ATM, directly from
>>> kernel space. Extending IPPROTO_ICMP by implementing the oob_sendmsg()
>>> handler for it there has become an option.
>>>
>>
>> That would make the ping tool way simpler. I can also wait until this is available and
>> create a new patch. It's mostly done just to figure out that IPPROTO_ICMP is not available.
> 
> Ok, I believe having IPPROTO_ICMP would be the best approach. I'll
> let you know when this is available.
> 

Meanwhile, I changed to ms and corrected the "%zu". Attached 4 variants:
oob-net-ping.c - The variant from the patch, improved
oob-net-ping-nonraw.c - OOB with IPPROTO_ICMP (Not tested due to obvious reasons)
ping-raw.c - POSIX raw
ping-nonraw.c - POSIX with IPPROTO_ICMP

This might help you debug the IPPROTO_ICMP implementation.

While sometimes having issues with EVL, I created also POSIX variants
so see if it is my issue.

I can create also a patch if you prefer for one of the two OOB variants.

There where issues with -O3 in ip_checksum(). With -O0, it works.

The only difference I see between working and faulty is that with 16bit-aligned (raw)
it fails and with 32bit-aligned (nonraw) it works. -O3 debugging doesn't really work, in assembly
its nearly unreadable with -O3 and as soon as I add a printf() in the while loop,
the checksum is good. I have no clue might be UB or a compiler bug. Took me some time to debug.
I have gcc (Debian 14.2.0-19) 14.2.0

Of course, as soon as I create a test program with only ip_checksum() and exactly the
same data, it works also, just combined with the code, it breaks down.

This version creates faulty check-sums:
static uint16_t ip_checksum(void *buf, int len)
{
	uint16_t *p = buf;	/* buf is assumed to be 16bit-aligned. */
	uint32_t sum = 0;
	int count = len;

	while (count > 1) {
		sum += *p++;
		count -= sizeof(*p);
	}

	if (count > 0)
		sum += *(uint8_t *)p;

	while (sum >> 16)
		sum = (sum & 0xffff) + (sum >> 16);

	return ~sum & 0xffff;
}

While this works:
static uint16_t ip_checksum(void *buf, int len)
{
	void *p = buf;
	uint32_t sum = 0;
	int count = len;

	while (count > 1) {
		sum += (uint16_t)(*(uint8_t *)p) + ((uint16_t)(*(uint8_t *)(p + 1)) << 8);
		p += 2;
		count -= 2;
	}

	if (count > 0)
		sum += *(uint8_t *)p;

	while (sum >> 16)
		sum = (sum & 0xffff) + (sum >> 16);

	return ~sum & 0xffff;
}