netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: stefan.tomanek@wertarbyte.de
Cc: netdev@vger.kernel.org, hannes@stressinduktion.org,
	bsderandrew@gmail.com
Subject: Re: [PATCH v3] fib_rules: add .suppress operation
Date: Wed, 31 Jul 2013 15:13:54 -0700 (PDT)	[thread overview]
Message-ID: <20130731.151354.1698882304457097890.davem@davemloft.net> (raw)
In-Reply-To: <20130730074636.GC10550@zirkel.wertarbyte.de>

From: Stefan Tomanek <stefan.tomanek@wertarbyte.de>
Date: Tue, 30 Jul 2013 09:46:36 +0200

> This change adds a new operation to the fib_rules_ops struct; it allows the
> suppression of routing decisions if certain criteria are not met by its
> results.
> 
> The first implemented constraint is a minimum prefix length added to the
> structures of routing rules. If a rule is added with a minimum prefix length
>>0, only routes meeting this threshold will be considered. Any other (more
> general) routing table entries will be ignored.
> 
> When configuring a system with multiple network uplinks and default routes, it
> is often convinient to reference the main routing table multiple times - but
> omitting the default route. Using this patch and a modified "ip" utility, this
> can be achieved by using the following command sequence:
> 
>   $ ip route add table secuplink default via 10.42.23.1
> 
>   $ ip rule add pref 100            table main prefixlength 1
>   $ ip rule add pref 150 fwmark 0xA table secuplink
> 
> With this setup, packets marked 0xA will be processed by the additional routing
> table "secuplink", but only if no suitable route in the main routing table can
> be found. By using a minimal prefixlength of 1, the default route (/0) of the
> table "main" is hidden to packets processed by rule 100; packets traveling to
> destinations with more specific routing entries are processed as usual.
> 
> Signed-off-by: Stefan Tomanek <stefan.tomanek@wertarbyte.de>

I just want to mention that the more quirky crap we put into the FIB
rules layer, the harder it will every be to make a scalable data
structure for FIB rule handling.

Right now it's basically a linear walk of rules, with processing at
each level.

And every single ipv4 route lookup is going to go through this entire
process.

Anyways, there are coding style problems in your change which you'll
need to address:

> +		if (!err && ops->suppress && ops->suppress(rule, arg)) {
> +			continue;
> +		}

Since statement basic blocks should not use curly braces.

> +		if (!(arg->flags & FIB_LOOKUP_NOREF)) {
> +			fib_info_put(result->fi);
> +		}

Likewise.

  reply	other threads:[~2013-07-31 22:13 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-23 22:02 [PATCH] fib_rules: add minimum prefix length Stefan Tomanek
2013-07-23 22:38 ` Stephen Hemminger
2013-07-23 22:52   ` Stefan Tomanek
2013-07-24  2:14 ` Hannes Frederic Sowa
2013-07-24  7:57   ` Stefan Tomanek
2013-07-25 16:29   ` Stefan Tomanek
2013-07-25 17:43     ` [PATCH] fib_rules/fib6rules: " Stefan Tomanek
2013-07-25 18:17     ` [PATCH] fib_rules: " Hannes Frederic Sowa
2013-07-25 18:28       ` Stefan Tomanek
2013-07-25 21:46         ` Andrew Collins
2013-07-25 22:11           ` Stefan Tomanek
2013-07-26 10:46             ` [PATCH] fib_rules: add .suppress operation Stefan Tomanek
2013-07-26 17:05               ` Hannes Frederic Sowa
2013-07-27  6:08                 ` Hannes Frederic Sowa
2013-07-27  6:26               ` Hannes Frederic Sowa
2013-07-27  7:07               ` Hannes Frederic Sowa
2013-07-27 10:21                 ` Stefan Tomanek
2013-07-27 15:10                   ` Hannes Frederic Sowa
2013-07-28 10:36                     ` Stefan Tomanek
2013-07-30  6:52                 ` [PATCH v2 1/2] fib6_rules: make error handling consistent with IPv4 Stefan Tomanek
2013-07-30  6:53                 ` [PATCH v2 2/2] fib_rules: add .suppress operation Stefan Tomanek
2013-07-30  7:03                   ` David Miller
2013-07-30  7:23                     ` Stefan Tomanek
2013-07-30  7:34                       ` David Miller
2013-07-30  7:46                 ` [PATCH v3] " Stefan Tomanek
2013-07-31 22:13                   ` David Miller [this message]
2013-08-01  0:24                     ` Stefan Tomanek
2013-08-01  0:26                       ` David Miller
2013-08-01  0:17                 ` [PATCH v4] " Stefan Tomanek
2013-08-01  0:27                   ` David Miller
2013-08-01  1:26                     ` Stefan Tomanek
2013-08-01  5:35                       ` Hannes Frederic Sowa
2013-07-26 10:54           ` [PATCH] fib_rules: add minimum prefix length Stefan Tomanek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130731.151354.1698882304457097890.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=bsderandrew@gmail.com \
    --cc=hannes@stressinduktion.org \
    --cc=netdev@vger.kernel.org \
    --cc=stefan.tomanek@wertarbyte.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).