Re: [PATCH net] Panic when tc_lookup_action_n finds a partially initialized action.

2016-10-11 Thread Cong Wang
On Tue, Oct 11, 2016 at 2:28 AM, Krister Johansen wrote: > On Wed, Oct 05, 2016 at 11:01:38AM -0700, Cong Wang wrote: >> Does the attached patch make any sense now? Our pernet init doesn't >> rely on act_base, so even we have some race, the worst case is after >> we

Re: [PATCH net] Panic when tc_lookup_action_n finds a partially initialized action.

2016-10-11 Thread Cong Wang
On Sat, Oct 8, 2016 at 11:13 PM, Krister Johansen wrote: > Hi Cong, > Thanks for the follow-up. > > On Thu, Oct 06, 2016 at 12:01:15PM -0700, Cong Wang wrote: >> On Wed, Oct 5, 2016 at 11:11 PM, Krister Johansen >> > pernet_operations pointer. The code in

Re: [PATCH net] Panic when tc_lookup_action_n finds a partially initialized action.

2016-10-11 Thread Krister Johansen
On Wed, Oct 05, 2016 at 11:01:38AM -0700, Cong Wang wrote: > Does the attached patch make any sense now? Our pernet init doesn't > rely on act_base, so even we have some race, the worst case is after > we initialize the pernet netns for an action but its ops still not > visible, which seems fine

Re: [PATCH net] Panic when tc_lookup_action_n finds a partially initialized action.

2016-10-09 Thread Krister Johansen
Hi Cong, Thanks for the follow-up. On Thu, Oct 06, 2016 at 12:01:15PM -0700, Cong Wang wrote: > On Wed, Oct 5, 2016 at 11:11 PM, Krister Johansen > > pernet_operations pointer. The code in register_pernet_subsys() makes > > no attempt to check for duplicates. If we add a pointer that's already

Re: [PATCH net] Panic when tc_lookup_action_n finds a partially initialized action.

2016-10-06 Thread Cong Wang
On Wed, Oct 5, 2016 at 11:11 PM, Krister Johansen wrote: > > I'm not sure. The reason I didn't take this approach from the outset is > that all of TC's callers of tcf_register_action pass a pointer to a > static structure as their *ops argument. The existence of code

Re: [PATCH net] Panic when tc_lookup_action_n finds a partially initialized action.

2016-10-06 Thread Krister Johansen
On Wed, Oct 05, 2016 at 11:01:38AM -0700, Cong Wang wrote: > On Tue, Oct 4, 2016 at 11:52 PM, Krister Johansen > wrote: > > On Mon, Oct 03, 2016 at 11:22:33AM -0700, Cong Wang wrote: > >> Please try the attached patch. I also convert the read path to RCU > >> to avoid a

Re: [PATCH net] Panic when tc_lookup_action_n finds a partially initialized action.

2016-10-05 Thread Cong Wang
On Wed, Oct 5, 2016 at 11:01 AM, Cong Wang wrote: > On Tue, Oct 4, 2016 at 11:52 PM, Krister Johansen > wrote: >> On Mon, Oct 03, 2016 at 11:22:33AM -0700, Cong Wang wrote: >>> Please try the attached patch. I also convert the read path to RCU

Re: [PATCH net] Panic when tc_lookup_action_n finds a partially initialized action.

2016-10-05 Thread Cong Wang
On Tue, Oct 4, 2016 at 11:52 PM, Krister Johansen wrote: > On Mon, Oct 03, 2016 at 11:22:33AM -0700, Cong Wang wrote: >> Please try the attached patch. I also convert the read path to RCU >> to avoid a possible deadlock. A quick test shows no lockdep splat. > > I tried

Re: [PATCH net] Panic when tc_lookup_action_n finds a partially initialized action.

2016-10-05 Thread Krister Johansen
On Mon, Oct 03, 2016 at 11:22:33AM -0700, Cong Wang wrote: > Please try the attached patch. I also convert the read path to RCU > to avoid a possible deadlock. A quick test shows no lockdep splat. I tried this patch, but it doesn't solve the problem. I got a panic on my very first try: SYSTEM

Re: [PATCH net] Panic when tc_lookup_action_n finds a partially initialized action.

2016-10-04 Thread Krister Johansen
Hi Cong, Thanks for the feedback. On Mon, Oct 03, 2016 at 11:22:33AM -0700, Cong Wang wrote: > On Sat, Oct 1, 2016 at 8:13 PM, Krister Johansen > wrote: > > A tc_action_ops structure is visibile as soon as it is placed in the > > act_base list. When tcf_regsiter_action

Re: [PATCH net] Panic when tc_lookup_action_n finds a partially initialized action.

2016-10-04 Thread Krister Johansen
On Sun, Oct 02, 2016 at 09:18:06PM -0400, Jamal Hadi Salim wrote: > On 16-10-01 11:13 PM, Krister Johansen wrote: > >A tc_action_ops structure is visibile as soon as it is placed in the > >act_base list. When tcf_regsiter_action adds an item to this list and > >drops act_mod_lock, registration is

Re: [PATCH net] Panic when tc_lookup_action_n finds a partially initialized action.

2016-10-03 Thread Cong Wang
On Sat, Oct 1, 2016 at 8:13 PM, Krister Johansen wrote: > A tc_action_ops structure is visibile as soon as it is placed in the > act_base list. When tcf_regsiter_action adds an item to this list and > drops act_mod_lock, registration is not complete until >

Re: [PATCH net] Panic when tc_lookup_action_n finds a partially initialized action.

2016-10-02 Thread Jamal Hadi Salim
On 16-10-01 11:13 PM, Krister Johansen wrote: A tc_action_ops structure is visibile as soon as it is placed in the act_base list. When tcf_regsiter_action adds an item to this list and drops act_mod_lock, registration is not complete until register_pernet_subsys() finishes. If two threads

[PATCH net] Panic when tc_lookup_action_n finds a partially initialized action.

2016-10-01 Thread Krister Johansen
A tc_action_ops structure is visibile as soon as it is placed in the act_base list. When tcf_regsiter_action adds an item to this list and drops act_mod_lock, registration is not complete until register_pernet_subsys() finishes. If two threads attempt to modify a tc action in a way that triggers