Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

single-thread API: add usrsctp_get_timeout() #349

Closed
ibc opened this issue Aug 13, 2019 · 4 comments
Closed

single-thread API: add usrsctp_get_timeout() #349

ibc opened this issue Aug 13, 2019 · 4 comments
Assignees

Comments

@ibc
Copy link

ibc commented Aug 13, 2019

PR #339 makes usrsctp work in single thread environments by forcing the application call usrsctp_handle_timers() periodically. While it's good enough (it does work), the optimized way of doing it would be by having an API like DTLSv1_get_timeout() and DTLSv1_handle_timeout in OpenSSL and BoringSSL.

usrsctp already has usrsctp_handle_timers() so the only missing API would be usrsctp_get_timeout().

This is, the application provides an input to the lib (via usrsctp_socket(), usrsctp_close(), usrsctp_connect(), usrsctp_conninput(), etc) and retrieves from the lib (via usrsctp_get_timeout()) the time that must elapse before the lib (usrsctp) would need to do something (like sending a retransmission, a chunk or whatever). And then the app would set its own timer with that expiration value and call usrsctp_handle_timers() when it expires. This would be much more efficient that the current approach in which the app must call usrsctp_handle_timers() every N ms just in case the lib has something to do at that time.

As @tuexen said:

usrsctp_get_timeout() should give the app the number of milliseconds until the next timer runs off. It should walk the (unordered) list of timers to compute that value. It would even be more efficient if the timer list would be ordered, but then starting a timer would be more complicated.

IMPORTANT: It should be very clear and well documented when the app should call usrsctp_get_timeout(), this is: the app should call usrsctp_get_timeout() and prepare a new timer after calling which usrsctp functions and after which usrsctp events.

@milahu
Copy link

milahu commented Feb 10, 2023

why was this closed?

fixed by #591

@ibc
Copy link
Author

ibc commented Feb 10, 2023

What do you mean? This issue was closed because it was done here: #339

@milahu
Copy link

milahu commented Feb 10, 2023

#339 adds

static unsigned get_tick_count(void)
{
#ifdef _WIN32
	return GetTickCount();
#else
	struct timeval te;
	gettimeofday(&te, NULL); // get current time
	unsigned milliseconds = te.tv_sec*1000LL + te.tv_usec/1000; // calculate milliseconds
	return milliseconds;
#endif
}

but that looks very different from #591

int
usrsctp_get_timeout(void)
{
	return sctp_get_next_tick();
}

int
sctp_get_next_tick(void)
{
	int ret;
	sctp_os_timer_t *c;

	SCTP_TIMERQ_LOCK();
	c = TAILQ_FIRST(&SCTP_BASE_INFO(callqueue));
	if (c) {
		uint32_t min_delta = UINT32_MAX;
		while (c) {
			uint32_t delta = c->c_time - ticks;
			min_delta = (delta < min_delta) ? delta : min_delta;
			c = TAILQ_NEXT(c, tqe);
		}
		ret = min_delta;
	} else {
		ret = -1;
	}
	SCTP_TIMERQ_UNLOCK();

	return ret;
}

usrsctp_get_timeout is used in frida-core

@ibc
Copy link
Author

ibc commented Feb 13, 2023

@milahu do you mean that PR #591 (not merged and unfortunately obviously ignored) is not a replacement but an addition and improvement to make single thread behavior way better than before?

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

No branches or pull requests

3 participants