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

What is the meaning of MCTP_DEFAULT_CLOCK_GETTIME? #17

Open
dcrowell77 opened this issue Jan 3, 2025 · 3 comments
Open

What is the meaning of MCTP_DEFAULT_CLOCK_GETTIME? #17

dcrowell77 opened this issue Jan 3, 2025 · 3 comments

Comments

@dcrowell77
Copy link

I see that MCTP_DEFAULT_CLOCK_GETTIME showed up in a recent update. I couldn't find any comments describing the meaning of this precompiler variable to determine if we should be setting it or not in our environment. (Currently it leads to some compile failures due to a nested set of includes from time.h)

@JaylynRen
Copy link

Get the some issue when compiling.

@mkj
Copy link
Contributor

mkj commented Jan 14, 2025

Some background, the clock handling was added to handle tag allocation and expiry inside libmctp. mctp_message_tx_request() allocates a tag from the pool of free or expired tags, so needs to know the current time for a 6 second expiry. The new i2c bus handler also uses mctp_now() or least-recently-used expiry in a neighbour table (if needed that could just use a counter instead).

On POSIXish platforms, MCTP_DEFAULT_CLOCK_GETTIME will be set by default (core-internal.h) to use clock_gettime() for mctp_now().

On other platforms, the intent is that it will be built with -DMCTP_DEFAULT_CLOCK_GETTIME=0. If the application is using any mctp_now() functionality such as mctp_message_tx_request(), it needs to call mctp_set_now_op() to provide a time callback. Otherwise that can be omitted. What failure are you seeing with time.h?

It might make sense to have libmctp on non-posix platforms only expose clock-requiring functions after the application opts in, otherwise #ifdef those away. I'm not sure how much future libmctp functionality might also want to use mctp_now() though.

@dcrowell77
Copy link
Author

What failure are you seeing with time.h?

Hostboot is pretty far from a POSIXish platform, we play some games with standard libraries. In this case, there is a chain of dependencies that ends up pulling a C++ header into the libmctp C code, which obviously doesn't go well. We haven't had time to dig in yet, but I wanted to get a sense of direction before we did.

mctp_message_tx_request() seems to be a new interface since we just refreshed our copy (less than 2 months ago...) so we'll have to evaluate how we might or might not use that interface.

needs to know the current time for a 6 second expiry

Is there some specific reason for the 6 second number? We may have platform-specific reasons to use something else.

Thanks for the info.

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

No branches or pull requests

3 participants