Commit d24a6ad4 authored by Maria Matejka's avatar Maria Matejka Committed by Maria Matejka
Browse files

Split route data structure to storage (ro) / manipulation (rw) structures.

This should help a lot with keeping the route machinery and allocations
clean. This commit also changes behaviour of rte_update() with cached
rta. Newly, calling rte_update() keeps the number of rta references.
parent a9d468c5
Loading
Loading
Loading
Loading
+6 −6
Original line number Diff line number Diff line
@@ -519,14 +519,14 @@
    {
      STATIC_ATTR;
      ACCESS_RTE;
      struct rta *rta = (*fs->rte)->attrs;
      struct rta *rta = fs->rte->attrs;

      switch (sa.sa_code)
      {
      case SA_FROM:	RESULT(sa.f_type, ip, rta->from); break;
      case SA_GW:	RESULT(sa.f_type, ip, rta->nh.gw); break;
      case SA_NET:	RESULT(sa.f_type, net, (*fs->rte)->net->n.addr); break;
      case SA_PROTO:	RESULT(sa.f_type, s, (*fs->rte)->src->proto->name); break;
      case SA_NET:	RESULT(sa.f_type, net, fs->rte->net); break;
      case SA_PROTO:	RESULT(sa.f_type, s, fs->rte->src->proto->name); break;
      case SA_SOURCE:	RESULT(sa.f_type, i, rta->source); break;
      case SA_SCOPE:	RESULT(sa.f_type, i, rta->scope); break;
      case SA_DEST:	RESULT(sa.f_type, i, rta->dest); break;
@@ -549,7 +549,7 @@

    f_rta_cow(fs);
    {
      struct rta *rta = (*fs->rte)->attrs;
      struct rta *rta = fs->rte->attrs;

      switch (sa.sa_code)
      {
@@ -561,7 +561,7 @@
	{
	  ip_addr ip = v1.val.ip;
	  struct iface *ifa = ipa_is_link_local(ip) ? rta->nh.iface : NULL;
	  neighbor *n = neigh_find((*fs->rte)->src->proto, ip, ifa, 0);
	  neighbor *n = neigh_find(fs->rte->src->proto, ip, ifa, 0);
	  if (!n || (n->scope == SCOPE_HOST))
	    runtime( "Invalid gw address" );

@@ -1194,7 +1194,7 @@
    struct rtable *table = rtc->table;
    ACCESS_RTE;
    ACCESS_EATTRS;
    const net_addr *net = (*fs->rte)->net->n.addr;
    const net_addr *net = fs->rte->net;

    /* We ignore temporary attributes, probably not a problem here */
    /* 0x02 is a value of BA_AS_PATH, we don't want to include BGP headers */
+13 −72
Original line number Diff line number Diff line
@@ -76,10 +76,7 @@ struct filter_state {
  struct filter_stack *stack;

  /* The route we are processing. This may be NULL to indicate no route available. */
  struct rte **rte;

  /* The old rta to be freed after filters are done. */
  struct rta *old_rta;
  struct rte *rte;

  /* Cached pointer to ea_list */
  struct ea_list **eattrs;
@@ -87,11 +84,11 @@ struct filter_state {
  /* Linpool for adata allocation */
  struct linpool *pool;

  /* Buffer for log output */
  struct buffer buf;

  /* Filter execution flags */
  int flags;

  /* Buffer for log output */
  struct buffer buf;
};

_Thread_local static struct filter_state filter_state;
@@ -101,15 +98,7 @@ void (*bt_assert_hook)(int result, const struct f_line_item *assert);

static inline void f_cache_eattrs(struct filter_state *fs)
{
  fs->eattrs = &((*fs->rte)->attrs->eattrs);
}

static inline void f_rte_cow(struct filter_state *fs)
{
  if (!((*fs->rte)->flags & REF_COW))
    return;

  *fs->rte = rte_cow(*fs->rte);
  fs->eattrs = &(fs->rte->attrs->eattrs);
}

/*
@@ -118,22 +107,16 @@ static inline void f_rte_cow(struct filter_state *fs)
static void
f_rta_cow(struct filter_state *fs)
{
  if (!rta_is_cached((*fs->rte)->attrs))
  if (!rta_is_cached(fs->rte->attrs))
    return;

  /* Prepare to modify rte */
  f_rte_cow(fs);

  /* Store old rta to free it later, it stores reference from rte_cow() */
  fs->old_rta = (*fs->rte)->attrs;

  /*
   * Get shallow copy of rta. Fields eattrs and nexthops of rta are shared
   * with fs->old_rta (they will be copied when the cached rta will be obtained
   * at the end of f_run()), also the lock of hostentry is inherited (we
   * suppose hostentry is not changed by filters).
   */
  (*fs->rte)->attrs = rta_do_cow((*fs->rte)->attrs, fs->pool);
  fs->rte->attrs = rta_do_cow(fs->rte->attrs, fs->pool);

  /* Re-cache the ea_list */
  f_cache_eattrs(fs);
@@ -241,29 +224,15 @@ interpret(struct filter_state *fs, const struct f_line *line, struct f_val *val)
/**
 * f_run - run a filter for a route
 * @filter: filter to run
 * @rte: route being filtered, may be modified
 * @rte: route being filtered; must be writable
 * @tmp_pool: all filter allocations go from this pool
 * @flags: flags
 *
 * If filter needs to modify the route, there are several
 * posibilities. @rte might be read-only (with REF_COW flag), in that
 * case rw copy is obtained by rte_cow() and @rte is replaced. If
 * @rte is originally rw, it may be directly modified (and it is never
 * copied).
 *
 * The returned rte may reuse the (possibly cached, cloned) rta, or
 * (if rta was modified) contains a modified uncached rta, which
 * uses parts allocated from @tmp_pool and parts shared from original
 * rta. There is one exception - if @rte is rw but contains a cached
 * rta and that is modified, rta in returned rte is also cached.
 *
 * Ownership of cached rtas is consistent with rte, i.e.
 * if a new rte is returned, it has its own clone of cached rta
 * (and cached rta of read-only source rte is intact), if rte is
 * modified in place, old cached rta is possibly freed.
 * If filter needs to modify the attributes, it allocates a local
 * shallow copy of them on @tmp_pool.
 */
enum filter_return
f_run(const struct filter *filter, struct rte **rte, struct linpool *tmp_pool, int flags)
f_run(const struct filter *filter, struct rte *rte, struct linpool *tmp_pool, int flags)
{
  if (filter == FILTER_ACCEPT)
    return F_ACCEPT;
@@ -271,7 +240,6 @@ f_run(const struct filter *filter, struct rte **rte, struct linpool *tmp_pool, i
  if (filter == FILTER_REJECT)
    return F_REJECT;

  int rte_cow = ((*rte)->flags & REF_COW);
  DBG( "Running filter `%s'...", filter->name );

  /* Initialize the filter state */
@@ -287,32 +255,6 @@ f_run(const struct filter *filter, struct rte **rte, struct linpool *tmp_pool, i
  /* Run the interpreter itself */
  enum filter_return fret = interpret(&filter_state, filter->root, NULL);

  if (filter_state.old_rta) {
    /*
     * Cached rta was modified and filter_state->rte contains now an uncached one,
     * sharing some part with the cached one. The cached rta should
     * be freed (if rte was originally COW, filter_state->old_rta is a clone
     * obtained during rte_cow()).
     *
     * This also implements the exception mentioned in f_run()
     * description. The reason for this is that rta reuses parts of
     * filter_state->old_rta, and these may be freed during rta_free(filter_state->old_rta).
     * This is not the problem if rte was COW, because original rte
     * also holds the same rta.
     */
    if (!rte_cow) {
      /* Cache the new attrs */
      (*filter_state.rte)->attrs = rta_lookup((*filter_state.rte)->attrs);

      /* Drop cached ea_list pointer */
      filter_state.eattrs = NULL;
    }

    /* Uncache the old attrs and drop the pointer as it is invalid now. */
    rta_free(filter_state.old_rta);
    filter_state.old_rta = NULL;
  }

  /* Process the filter output, log it and return */
  if (fret < F_ACCEPT) {
    if (!(filter_state.flags & FF_SILENT))
@@ -337,7 +279,7 @@ f_run(const struct filter *filter, struct rte **rte, struct linpool *tmp_pool, i
 */

enum filter_return
f_eval_rte(const struct f_line *expr, struct rte **rte, struct linpool *tmp_pool)
f_eval_rte(const struct f_line *expr, struct rte *rte, struct linpool *tmp_pool)
{
  filter_state = (struct filter_state) {
    .stack = &filter_stack,
@@ -347,8 +289,7 @@ f_eval_rte(const struct f_line *expr, struct rte **rte, struct linpool *tmp_pool

  LOG_BUFFER_INIT(filter_state.buf);

  ASSERT(!((*rte)->flags & REF_COW));
  ASSERT(!rta_is_cached((*rte)->attrs));
  ASSERT(!rta_is_cached(rte->attrs));

  return interpret(&filter_state, expr, NULL);
}
+2 −2
Original line number Diff line number Diff line
@@ -51,8 +51,8 @@ struct filter {

struct rte;

enum filter_return f_run(const struct filter *filter, struct rte **rte, struct linpool *tmp_pool, int flags);
enum filter_return f_eval_rte(const struct f_line *expr, struct rte **rte, struct linpool *tmp_pool);
enum filter_return f_run(const struct filter *filter, struct rte *rte, struct linpool *tmp_pool, int flags);
enum filter_return f_eval_rte(const struct f_line *expr, struct rte *rte, struct linpool *tmp_pool);
uint f_eval_int(const struct f_line *expr);
enum filter_return f_eval_buf(const struct f_line *expr, struct linpool *tmp_pool, buffer *buf);

+1 −1
Original line number Diff line number Diff line
@@ -72,7 +72,7 @@ static inline int u64_cmp(u64 i1, u64 i2)
#define NORET __attribute__((noreturn))
#define UNUSED __attribute__((unused))
#define PACKED __attribute__((packed))
#define NONNULL(...) __attribute__((nonnull((__VA_ARGS__))))
#define NONNULL(...) __attribute__((nonnull(__VA_ARGS__)))
#define USE_RESULT __attribute__((warn_unused_result))

#ifndef HAVE_THREAD_LOCAL
+1 −1
Original line number Diff line number Diff line
@@ -892,8 +892,8 @@ channel_reconfigure(struct channel *c, struct channel_config *cf)
  c->out_limit = cf->out_limit;

  // c->ra_mode = cf->ra_mode;
  c->merge_limit = cf->merge_limit;
  c->preference = cf->preference;
  c->merge_limit = cf->merge_limit;
  c->debug = cf->debug;
  c->in_keep_filtered = cf->in_keep_filtered;
  c->rpki_reload = cf->rpki_reload;
Loading