Re: [PATCH bpf-next v2 2/2] selftests/bpf: Export send_recv_data helper

[Martin KaFai Lau <martin.lau@linux.dev>]

On 4/8/24 8:51 PM, Geliang Tang wrote:
>> +static void *send_recv_server(void *arg)
>> +{
>> +	struct send_recv_arg *a = (struct send_recv_arg *)arg;
>> +	ssize_t nr_sent = 0, bytes = 0;
>> +	char batch[1500];
>> +	int err = 0, fd;
>> +
>> +	fd = accept(a->fd, NULL, NULL);
>> +	while (fd == -1) {
>> +		if (errno == EINTR)
>> +			continue;
>> +		err = -errno;
>> +		goto done;
>> +	}
>> +
>> +	if (settimeo(fd, 0)) {
>> +		err = -errno;
>> +		goto done;
>> +	}
>> +
>> +	while (bytes < a->bytes && !READ_ONCE(a->stop)) {
>> +		nr_sent = send(fd, &batch,
>> +			       MIN(a->bytes - bytes, sizeof(batch)),
>> 0);
>> +		if (nr_sent == -1 && errno == EINTR)
>> +			continue;
>> +		if (nr_sent == -1) {
>> +			err = -errno;
>> +			break;
>> +		}
>> +		bytes += nr_sent;
>> +	}
>> +
>> +	ASSERT_EQ(bytes, a->bytes, "send");
>> +
>> +done:
>> +	if (fd >= 0)
>> +		close(fd);
>> +	if (err) {
>> +		WRITE_ONCE(a->stop, 1);
>> +		return ERR_PTR(err);
>> +	}
>> +	return NULL;
>> +}
>> +
>> +void send_recv_data(int lfd, int fd, uint32_t total_bytes)
>> +{
>> +	ssize_t nr_recv = 0, bytes = 0;
>> +	struct send_recv_arg arg = {
>> +		.fd	= lfd,
>> +		.bytes	= total_bytes,
>> +		.stop	= 0,
>> +	};
>> +	pthread_t srv_thread;
>> +	void *thread_ret;
>> +	char batch[1500];
>> +	int err;
>> +
>> +	err = pthread_create(&srv_thread, NULL, send_recv_server,
>> (void *)&arg);
>> +	if (!ASSERT_OK(err, "pthread_create"))
>> +		return;
>> +
>> +	/* recv total_bytes */
>> +	while (bytes < total_bytes && !READ_ONCE(arg.stop)) {
>> +		nr_recv = recv(fd, &batch,
>> +			       MIN(total_bytes - bytes,
>> sizeof(batch)), 0);
>> +		if (nr_recv == -1 && errno == EINTR)
>> +			continue;
>> +		if (nr_recv == -1)
>> +			break;
>> +		bytes += nr_recv;
>> +	}
>> +
>> +	ASSERT_EQ(bytes, total_bytes, "recv");
> I think we should avoid using ASSERT_* in network_helpers.c, but I'm
> not sure. What do you think?

There is log_err which is used by other helpers in network_helpers.c. May be use 
log_err instead and return int instead of void here. The caller can decide if it 
expects error or not and uses ASSERT accordingly.

pw-bot: cr