From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Pirko Subject: Re: [patch net-next v4 03/12] net: sched: introduce chain object to uapi Date: Wed, 25 Jul 2018 08:46:45 +0200 Message-ID: <20180725064645.GA2164@nanopsycho> References: <20180723072312.4153-1-jiri@resnulli.us> <20180723072312.4153-4-jiri@resnulli.us> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Linux Kernel Network Developers , David Miller , Jamal Hadi Salim , Jakub Kicinski , Simon Horman , john.hurley@netronome.com, David Ahern , mlxsw@mellanox.com, sridhar.samudrala@intel.com To: Cong Wang Return-path: Received: from mail-wr1-f68.google.com ([209.85.221.68]:42405 "EHLO mail-wr1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728397AbeGYH71 (ORCPT ); Wed, 25 Jul 2018 03:59:27 -0400 Received: by mail-wr1-f68.google.com with SMTP id e7-v6so6242601wrs.9 for ; Tue, 24 Jul 2018 23:49:12 -0700 (PDT) Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: Wed, Jul 25, 2018 at 01:20:08AM CEST, xiyou.wangcong@gmail.com wrote: >On Tue, Jul 24, 2018 at 3:30 PM Cong Wang wrote: >> >> On Mon, Jul 23, 2018 at 12:25 AM Jiri Pirko wrote: >> > + switch (n->nlmsg_type) { >> > + case RTM_NEWCHAIN: >> > + /* In case the chain was successfully added, take a reference >> > + * to the chain. This ensures that an empty chain >> > + * does not disappear at the end of this function. >> > + */ >> > + tcf_chain_hold(chain); >> > + chain->explicitly_created = true; >> > + tc_chain_notify(chain, NULL, 0, NLM_F_CREATE | NLM_F_EXCL, >> > + RTM_NEWCHAIN, false); >> > + break; >> > + case RTM_DELCHAIN: >> > + /* Flush the chain first as the user requested chain removal. */ >> > + tcf_chain_flush(chain); >> > + /* In case the chain was successfully deleted, put a reference >> > + * to the chain previously taken during addition. >> > + */ >> > + tcf_chain_put_explicitly_created(chain); >> > + break; >> >> I don't see you send notification to user-space when deleting a chain, >> am I missing anything? > >Oh, it is hidden in tcf_chain_put(): > >void tcf_chain_put(struct tcf_chain *chain) >{ > if (--chain->refcnt == 0) { > tc_chain_notify(chain, NULL, 0, 0, RTM_DELCHAIN, false); > tc_chain_tmplt_del(chain); > tcf_chain_destroy(chain); > } >} > >So, you only send out notification when the last refcnt is gone. > >If the chain that is being deleted by a user is still used by an action, >you return 0 or -EPERM? 0 and the chain stays there until the action is removed. Hmm, do you thing that -EPERM should be returned in that case? The thing is, we have to flush the chain in order to see the action references are there. We would have to have 2 ref counters, one for filter, one for actions. What do you think?