Page MenuHomeFreeBSD
Differential D23075 Diff 66564 sys/net/route_temporal.c

Changeset View

Standalone View

View Options

sys/net/route_temporal.c

  • This file was added.
PropertyOld ValueNew Value
svn:eol-stylenullnative
\ No newline at end of property
svn:keywordsnullFreeBSD=%H
\ No newline at end of property
svn:mime-typenulltext/plain
\ No newline at end of property
/*-
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD
*
* Copyright (c) 2020 Alexander V. Chernikov
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*
* $FreeBSD$
*/
bzUnsubmitted
Not Done
Inline Actions

Normally this would be a __FBSDID(..); in the code after the cdefs include and not the comment.

bz: Normally this would be a __FBSDID(..); in the code after the cdefs include and not the comment.
melifaroAuthorUnsubmitted
Done
Inline Actions

Copied this pattern from the files with newer copyright :-)
Will change back to __FBSDID()

melifaro: Copied this pattern from the files with newer copyright :-) Will change back to `__FBSDID()`
/*
* This file contains code responsible for expiring temporal routes
* (typically, redirect-originated) from the route tables.
*/
#include <sys/cdefs.h>
#include <sys/param.h>
#include <sys/systm.h>
#include <sys/socket.h>
#include <sys/kernel.h>
#include <sys/lock.h>
#include <sys/rmlock.h>
#include <net/if.h>
#include <net/route.h>
#include <net/route_var.h>
#include <net/vnet.h>
/*
* Callback returning 1 for the expired routes.
* Updates time of the next nearest route expiration
* as a side effect.
*/
bzUnsubmitted
Not Done
Inline Actions

extra space to remove .. but the 2nd sentence can also be one line easily.

bz: extra space to remove .. but the 2nd sentence can also be one line easily.
static int
expire_route(const struct rtentry *rt, void *arg)
{
time_t *next_callout = (time_t *)arg;
bzUnsubmitted
Done
Inline Actions

No assignments in variable declarations.

bz: No assignments in variable declarations.
if (rt->rt_expire == 0)
return (0);
if (rt->rt_expire <= time_uptime) {
printf("expiring %p, ts %lu\n", rt, rt->rt_expire);
return (1);
bzUnsubmitted
Done
Inline Actions

Is this is debug printf which should go? Otherwise at least at a %s: func please.

bz: Is this is debug printf which should go? Otherwise at least at a %s: __func__ please.
}
/*
* Update next_callout to determine the next ts to
* run the callback at.
*/
if (*next_callout == 0 || *next_callout > rt->rt_expire)
bzUnsubmitted
Not Done
Inline Actions

Here the next_callout = (time_t *)arg; could happen; which means we'd also not do it for previous returns and save some work.

bz: Here the next_callout = (time_t *)arg; could happen; which means we'd also not do it for…
melifaroAuthorUnsubmitted
Done
Inline Actions

Sorry, not sure if I get it correctly :-(. Can you please rephrase the statement?

melifaro: Sorry, not sure if I get it correctly :-(. Can you please rephrase the statement?
bzUnsubmitted
Done
Inline Actions

If you do not do the assignment at variable declaration time, it could be moved down to here:

next_callout = (time_t *)arg;

as this is the first use of next_callout in this function. So there is no need to assing arg to it any sooner. If (rt->rt_expire == 0) is true we'd return and an early assignment at variable declaration time would have been wasted CPU cycles (and probably register space).

bz: If you do not do the assignment at variable declaration time, it could be moved down to here…
*next_callout = rt->rt_expire;
return (0);
}
/*
* Per-rnh callout function traversing the tree and deleting
* expired routes. Calculates next callout run by looking at
* the rt_expire time for the remaining temporal routes.
*/
static void
expire_callout(void *arg)
{
struct rib_head *rnh = (struct rib_head *)arg;
time_t next_expire = 0;
bzUnsubmitted
Done
Inline Actions

Please move the assignments down before first use.

bz: Please move the assignments down before first use.
int ticks;
CURVNET_SET(rnh->rib_vnet);
rib_walk_del(rnh->rib_fibnum, rnh->rib_family, expire_route,
(void *)&next_expire, 1);
RIB_WLOCK(rnh);
if (next_expire > 0) {
ticks = (next_expire - time_uptime) * hz;
if (ticks < 0)
ticks = 0;
bzUnsubmitted
Not Done
Inline Actions

Isn't this a problem after roughly 24 days?

bz: Isn't this a problem after roughly 24 days?
melifaroAuthorUnsubmitted
Done
Inline Actions

That's a good point, thanks for catching this!
I was mostly thinking of temporal routes as redirects with reasonably limited lifetime, however user is able to set up arbitrary expiration time. I'll change the handling here to callout_reset_sbt() to avoid using ticks at all or move to int64_t.

melifaro: That's a good point, thanks for catching this! I was mostly thinking of temporal routes as…
callout_reset(&rnh->expire_callout, ticks, expire_callout, rnh);
rnh->next_expire = next_expire;
} else {
/*
* Before resetting next_expire, check that tmproutes_update()
* has not kicked in and scheduled another invocation.
*/
if (callout_pending(&rnh->expire_callout) == 0)
rnh->next_expire = 0;
}
RIB_WUNLOCK(rnh);
CURVNET_RESTORE();
}
/*
* Function responsible for updating the time of the next calllout
* w.r.t. new temporal routes insertion.
*
* Called by the routing code upon adding new temporal route
* to the tree. RIB_WLOCK must be held.
*/
void
tmproutes_update(struct rib_head *rnh, struct rtentry *rt)
{
int ticks;
RIB_WLOCK_ASSERT(rnh);
if (rnh->next_expire == 0 || rnh->next_expire > rt->rt_expire) {
/*
* Callback is not scheduled, is executing,
* or is scheduled for a later time than we need.
*
* Schedule the one for the current @rt expiration time.
*/
ticks = (rt->rt_expire - time_uptime) * hz;
if (ticks < 0)
ticks = 0;
callout_reset(&rnh->expire_callout, ticks, expire_callout, rnh);
bzUnsubmitted
Not Done
Inline Actions

Same ~24 days issue as above?

bz: Same ~24 days issue as above?
rnh->next_expire = rt->rt_expire;
}
}
void
tmproutes_init(struct rib_head *rh)
{
callout_init(&rh->expire_callout, 1);
}
void
tmproutes_destroy(struct rib_head *rh)
{
callout_drain(&rh->expire_callout);
}