From: Vlad Buslov <vladbu@mellanox•com>
To: Ido Schimmel <idosch@idosch•org>
Cc: Vlad Buslov <vladbu@mellanox•com>,
"netdev@vger•kernel.org" <netdev@vger•kernel.org>,
"jhs@mojatatu•com" <jhs@mojatatu•com>,
"xiyou.wangcong@gmail•com" <xiyou.wangcong@gmail•com>,
"jiri@resnulli•us" <jiri@resnulli•us>,
"davem@davemloft•net" <davem@davemloft•net>,
"john.hurley@netronome•com" <john.hurley@netronome•com>
Subject: Re: [PATCH net-next] net: sched: flower: insert filter to ht before offloading it to hw
Date: Thu, 11 Apr 2019 11:28:25 +0000 [thread overview]
Message-ID: <vbfmukw4w8b.fsf@mellanox.com> (raw)
In-Reply-To: <20190411111342.GA29053@splinter>
On Thu 11 Apr 2019 at 14:13, Ido Schimmel <idosch@idosch•org> wrote:
> On Fri, Apr 05, 2019 at 08:56:26PM +0300, Vlad Buslov wrote:
>> John reports:
>>
>> Recent refactoring of fl_change aims to use the classifier spinlock to
>> avoid the need for rtnl lock. In doing so, the fl_hw_replace_filer()
>> function was moved to before the lock is taken. This can create problems
>> for drivers if duplicate filters are created (commmon in ovs tc offload
>> due to filters being triggered by user-space matches).
>>
>> Drivers registered for such filters will now receive multiple copies of
>> the same rule, each with a different cookie value. This means that the
>> drivers would need to do a full match field lookup to determine
>> duplicates, repeating work that will happen in flower __fl_lookup().
>> Currently, drivers do not expect to receive duplicate filters.
>>
>> To fix this, verify that filter with same key is not present in flower
>> classifier hash table and insert the new filter to the flower hash table
>> before offloading it to hardware. Implement helper function
>> fl_ht_insert_unique() to atomically verify/insert a filter.
>>
>> This change makes filter visible to fast path at the beginning of
>> fl_change() function, which means it can no longer be freed directly in
>> case of error. Refactor fl_change() error handling code to deallocate the
>> filter with rcu timeout.
>>
>> Fixes: 620da4860827 ("net: sched: flower: refactor fl_change")
>> Reported-by: John Hurley <john.hurley@netronome•com>
>> Signed-off-by: Vlad Buslov <vladbu@mellanox•com>
>
> Vlad,
>
> Our regression machines all hit a NULL pointer dereference [1] which I
> bisected to this patch. Created this reproducer that you can use:
>
> ip netns add ns1
> ip -n ns1 link add dev dummy1 type dummy
> tc -n ns1 qdisc add dev dummy1 clsact
> tc -n ns1 filter add dev dummy1 ingress pref 1 proto ip \
> flower skip_hw src_ip 192.0.2.1 action drop
> ip netns del ns1
>
> Can you please look into this? Thanks
Will do.
prev parent reply other threads:[~2019-04-11 11:28 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-05 17:56 [PATCH net-next] net: sched: flower: insert filter to ht before offloading it to hw Vlad Buslov
2019-04-06 5:59 ` Jiri Pirko
2019-04-08 2:34 ` David Miller
2019-04-08 22:26 ` Jakub Kicinski
2019-04-09 8:23 ` Vlad Buslov
2019-04-09 17:10 ` Jakub Kicinski
2019-04-10 14:53 ` Vlad Buslov
2019-04-10 15:48 ` Jakub Kicinski
2019-04-10 16:02 ` Vlad Buslov
2019-04-10 16:09 ` Jakub Kicinski
2019-04-10 16:26 ` Vlad Buslov
2019-04-10 17:00 ` Jakub Kicinski
2019-04-16 14:20 ` [RFC PATCH net-next] net: sched: flower: refactor reoffload for concurrent access Vlad Buslov
2019-04-16 21:49 ` Jakub Kicinski
2019-04-17 7:29 ` Vlad Buslov
2019-04-17 16:34 ` Jakub Kicinski
2019-04-17 17:01 ` Vlad Buslov
2019-04-18 16:33 ` Vlad Buslov
2019-04-18 17:46 ` Jakub Kicinski
2019-04-18 17:58 ` Vlad Buslov
2019-04-18 18:02 ` Jakub Kicinski
2019-04-18 18:13 ` Vlad Buslov
2019-04-18 18:15 ` Jakub Kicinski
2019-04-11 11:13 ` [PATCH net-next] net: sched: flower: insert filter to ht before offloading it to hw Ido Schimmel
2019-04-11 11:28 ` Vlad Buslov [this message]
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=vbfmukw4w8b.fsf@mellanox.com \
--to=vladbu@mellanox$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=idosch@idosch$(echo .)org \
--cc=jhs@mojatatu$(echo .)com \
--cc=jiri@resnulli$(echo .)us \
--cc=john.hurley@netronome$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=xiyou.wangcong@gmail$(echo .)com \
/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