netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Julian Anastasov <ja@ssi.bg>
To: gregory hoggarth <gregory.hoggarth@alliedtelesis.co.nz>
Cc: netdev@vger.kernel.org
Subject: Re: Linux kernel patch: elide fib_validate_source() completely when possible - bad side effect?
Date: Wed, 22 Jan 2014 23:04:52 +0200 (EET)	[thread overview]
Message-ID: <alpine.LFD.2.11.1401222222010.1855@ja.home.ssi.bg> (raw)
In-Reply-To: <52DCF1440200005D000477FD@gwia2.atlnz.lc>


	Hello,

On Mon, 20 Jan 2014, gregory hoggarth wrote:

> >>> Julian Anastasov <ja@ssi.bg> 17/1/14 09:24 PM >>> 
> >
> >	It seems only __fib_validate_source can reject all kind
> > of broadcast addresses in saddr. ip_route_input_slow() rejects
> > only the well known broadcasts. Without rp_filter may be we
> > can at least drop attempts to send replies back to broadcast
> > addresses? For example, checking result of ip_route_output_key() in
> > icmp_reply():
> >
> >	if (rt->rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST))
> >		=> ip_rt_put()

> Thank you for your reply.
> 
> While I think your solution may work, isn't the proper approach to drop these 
> rogue packets, rather than wasting CPU and other resources processing them, 
> and then beginning to craft responses which are in turn dropped?

	The problem is that it is __fib_validate_source that
takes time for every packet. And it is a rare case to see
traffic from broadcast addresses. rp_filter set on external
interface will drop such packets. It is an option that one
can use even for internal interface, if needed.

> Also seems better to drop the initial rogue packet, as that should cover all 
> (?) different types of packets, rather than having to add small patches into 
> ICMP, TCP and any other places that may need it.

	Small check looks better compared to FIB lookup for
every received packet. Note that output route can be cached
for sockets, eg. TCP, so such small checks do not occur for
every reply packet.

> So my question really is was the original patch correct and there's something
> wrong with our device configuration, or is this an overlooked / untested side-
> effect from that patch that as a result means the patch should be re-worked?

	I don't remember someone mentioning about such
side-effect, I guess it is overlooked. IMHO, it is not a good
reason to restore the old behavior. Lets see other opinions.

Regards

--
Julian Anastasov <ja@ssi.bg>

  reply	other threads:[~2014-01-22 21:06 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-17  1:59 Linux kernel patch: elide fib_validate_source() completely when possible - bad side effect? gregory hoggarth
2014-01-17  8:24 ` Julian Anastasov
2014-01-19 20:49   ` gregory hoggarth
2014-01-22 21:04     ` Julian Anastasov [this message]
2014-01-23  2:02       ` Hannes Frederic Sowa
2014-01-23  2:06         ` Hannes Frederic Sowa
2014-01-23  2:53         ` David Miller
2014-01-23  3:03           ` Hannes Frederic Sowa
2014-01-23  9:47             ` Julian Anastasov
2014-01-23 14:20               ` Hannes Frederic Sowa

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=alpine.LFD.2.11.1401222222010.1855@ja.home.ssi.bg \
    --to=ja@ssi.bg \
    --cc=gregory.hoggarth@alliedtelesis.co.nz \
    --cc=netdev@vger.kernel.org \
    /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).