gnrc/ipv6nib: remove the need for evtimer-minutes

[kfessel]

Contribution description

remove the only use of evtimer_now_min()

Testing procedure

not yet sure

"test/gnrc_ipv6_nib*"

should not fail

Issues/PRs references

#17357 deprecated the need for min to avoid ztimer_now64

[miri64]

This can easily lead to an integer overflow. The valid lifetime of a border router is supposed to run on a scale of weeks, so having it run on milliseconds seems not that wise for that scale.

[miri64]

Maybe evimer_now_min() could use ztimer_sec instead?

[kfessel]

16 bit minutes * 60 * 1000 -> log2(2**16 *60 * 1000)=31.87267488027061 (this is save and it is allready stated in nib that it is save (where the timeout is put into the evtimer set)) (needs to use overflow substraction to get rest valid time ( afaik it is defined behavior for C))

[kfessel]

Maybe evimer_now_min() could use ztimer_sec instead?

this is our would increase the ztimer footgun potential (trough evtimer, therefore hidden (even worse from my POV) (you wrote that yourself in a PR that tried to do that very thing, #16697))

[miri64]

16 bit minutes * 60 * 1000 -> log2(2**16 *60 * 1000)=31.87267488027061 (this is save and it is allready stated in nib that it is save (where the timeout is put into the evtimer set)) (needs to use overflow substraction to get rest valid time ( afaik it is defined behavior for C))

Not really sure, what you are calculating there, but you are right, that there is no overflow with a 32-bit ms timestamp converted to a 16-bit min timestamp:

(2³² - 1) / 1000 / 60 > (2¹⁶ - 1)

is true. Thanks for pointing that out.

Number space is large enough to map the different timescales.

[miri64]

Not really sure, what you are calculating there,

Aaahh you were calculating the bits required to map a 16-bit minute timestamp to a ms timestamp :D.

[kfessel]

a first test using modified code (diverting from the 10000 default) and wireshark on native using ZEP with the gnrc_networking and the boarder router example seem to behave as expected but i am still not sure, how to show that it is working -> i am going to change the name of relevat structure memebers that this PR is changing its unit on

Make the compiler / linker will reveal all user of them.

[kfessel]

@miri64 : is this what should be written here ?

Suggested change

	uint32_t now = evtimer_now_msec();
	/* Some thing seems strage here does it expire if valid_until in 0 or not*/
	uint32_t now = evtimer_now_msec();
	uint32_t abr_valid_until = abr->valid_until_ms;
	if(abr_valid_until == 0){
	printf("%s v%" PRIu32 " not expires %" PRIu32 "min\n",
	ipv6_addr_to_str(addr_str, &abr->addr, sizeof(addr_str)),
	abr->version,
	SIXLOWPAN_ND_OPT_ABR_LTIME_DEFAULT);
	} else {
	uint32_t abr_expires_min = (abr_valid_until - now) / (SEC_PER_MIN * MS_PER_SEC);
	printf("%s v%" PRIu32 " expires %" PRIu32 "min\n",
	ipv6_addr_to_str(addr_str, &abr->addr, sizeof(addr_str)),
	abr->version,
	abr_expires_min);
	}

[benpicco]

You can squash directly

[kfessel]

This line needs to be removed i think there is something strange in the code (not of this print but oft he whole thing this print is just the tip of it, the strangeness is not modified by this PR in anyway it is just this line making a notice)

(there are mechanisms that take no input as 10000 sec but do not set the valid date in 10000 secs but set it to 0, than this prints 10000, even though some time might have passed)

[kfessel]

@benpicco are you ok with leaving that comment line in? (merging it?)

[kfessel]

I squash without the comment marked in #17411 (comment)

This PR does change anything regarding these concerns (someone more knowledgeable might have a look at it later)

I moved my concerns to #17512 to clean this PR up for merge