From: "Duyck, Alexander H" <alexander.h.duyck@intel•com>
To: "netdev@vger•kernel.org" <netdev@vger•kernel.org>,
"jiri@resnulli•us" <jiri@resnulli•us>
Cc: "davem@davemloft•net" <davem@davemloft•net>,
"idosch@mellanox•com" <idosch@mellanox•com>,
"eladr@mellanox•com" <eladr@mellanox•com>,
"mlxsw@mellanox•com" <mlxsw@mellanox•com>,
"kaber@trash•net" <kaber@trash•net>
Subject: Re: [patch net-next 1/7] ipv4: fib: Only flush FIB aliases belonging to currently flushed table
Date: Thu, 9 Feb 2017 19:00:51 +0000 [thread overview]
Message-ID: <1486666849.7829.67.camel@intel.com> (raw)
In-Reply-To: <1486632524-5588-2-git-send-email-jiri@resnulli.us>
On Thu, 2017-02-09 at 10:28 +0100, Jiri Pirko wrote:
> From: Ido Schimmel <idosch@mellanox•com>
>
> In case the MAIN table is flushed and its trie is shared with the LOCAL
> table, then we might be flushing FIB aliases belonging to the latter.
> This can lead to FIB_ENTRY_DEL notifications sent with the wrong table
> ID.
>
> The above doesn't affect current listeners, as the table ID is ignored
> during entry deletion, but this will change later in the patchset.
>
> When flushing a particular table, skip any aliases belonging to a
> different one.
>
> Signed-off-by: Ido Schimmel <idosch@mellanox•com>
> Signed-off-by: Jiri Pirko <jiri@mellanox•com>
> CC: Alexander Duyck <alexander.h.duyck@intel•com>
> CC: Patrick McHardy <kaber@trash•net>
> ---
> net/ipv4/fib_trie.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
> index 2919d1a..5ef4596 100644
> --- a/net/ipv4/fib_trie.c
> +++ b/net/ipv4/fib_trie.c
> @@ -1963,7 +1963,8 @@ int fib_table_flush(struct net *net, struct fib_table *tb)
> hlist_for_each_entry_safe(fa, tmp, &n->leaf, fa_list) {
> struct fib_info *fi = fa->fa_info;
>
> - if (!fi || !(fi->fib_flags & RTNH_F_DEAD)) {
> + if (!fi || !(fi->fib_flags & RTNH_F_DEAD) ||
> + tb->tb_id != fa->tb_id) {
> slen = fa->fa_slen;
> continue;
> }
One change I might make if you end up having to do a v2 would be to
test for the table ID first. It can end up saving a few cycles in the
whole flushing process since the table ID is in the fib alias instead
of having to dereference the fib info.
That being said, I am just being a bit nit-picky so the code itself is
functionally correct and there is nothing here that should cause any
issues.
Reviewed-by: Alexander Duyck <alexander.h.duyck@intel•com>
next prev parent reply other threads:[~2017-02-09 19:01 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-09 9:28 [patch net-next 0/7] mlxsw: Identical routes handling Jiri Pirko
2017-02-09 9:28 ` [patch net-next 1/7] ipv4: fib: Only flush FIB aliases belonging to currently flushed table Jiri Pirko
2017-02-09 19:00 ` Duyck, Alexander H [this message]
2017-02-09 9:28 ` [patch net-next 2/7] ipv4: fib: Send deletion notification with actual FIB alias type Jiri Pirko
2017-02-09 9:28 ` [patch net-next 3/7] ipv4: fib: Send notification before deleting FIB alias Jiri Pirko
2017-02-09 9:28 ` [patch net-next 4/7] ipv4: fib: Add events for FIB replace and append Jiri Pirko
2017-02-09 9:28 ` [patch net-next 5/7] mlxsw: spectrum_router: Correctly handle identical routes Jiri Pirko
2017-02-09 9:28 ` [patch net-next 6/7] mlxsw: spectrum_router: Add support for route append Jiri Pirko
2017-02-09 9:28 ` [patch net-next 7/7] mlxsw: spectrum_router: Add support for route replace Jiri Pirko
2017-02-10 16:34 ` [patch net-next 0/7] mlxsw: Identical routes handling David Miller
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=1486666849.7829.67.camel@intel.com \
--to=alexander.h.duyck@intel$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=eladr@mellanox$(echo .)com \
--cc=idosch@mellanox$(echo .)com \
--cc=jiri@resnulli$(echo .)us \
--cc=kaber@trash$(echo .)net \
--cc=mlxsw@mellanox$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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