From patchwork Thu May 31 06:52:53 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vlad Buslov X-Patchwork-Id: 923205 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=mellanox.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 40xJ7q39Lfz9s0W for ; Thu, 31 May 2018 16:53:11 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753987AbeEaGxI (ORCPT ); Thu, 31 May 2018 02:53:08 -0400 Received: from mail-il-dmz.mellanox.com ([193.47.165.129]:60596 "EHLO mellanox.co.il" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753913AbeEaGxH (ORCPT ); Thu, 31 May 2018 02:53:07 -0400 Received: from Internal Mail-Server by MTLPINE1 (envelope-from vladbu@mellanox.com) with ESMTPS (AES256-SHA encrypted); 31 May 2018 09:55:12 +0300 Received: from reg-r-vrt-018-180.mtr.labs.mlnx (reg-r-vrt-018-180.mtr.labs.mlnx [10.213.18.180]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id w4V6r5Z5016661; Thu, 31 May 2018 09:53:05 +0300 From: Vlad Buslov To: jiri@resnulli.us Cc: netdev@vger.kernel.org, jhs@mojatatu.com, xiyou.wangcong@gmail.com, davem@davemloft.net, Vlad Buslov Subject: [PATCH net-next v2] net: sched: split tc_ctl_tfilter into three handlers Date: Thu, 31 May 2018 09:52:53 +0300 Message-Id: <1527749573-18654-1-git-send-email-vladbu@mellanox.com> X-Mailer: git-send-email 2.7.5 In-Reply-To: <20180530202533.GE2010@nanopsycho> References: <20180530202533.GE2010@nanopsycho> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org tc_ctl_tfilter handles three netlink message types: RTM_NEWTFILTER, RTM_DELTFILTER, RTM_GETTFILTER. However, implementation of this function involves a lot of branching on specific message type because most of the code is message-specific. This significantly complicates adding new functionality and doesn't provide much benefit of code reuse. Split tc_ctl_tfilter to three standalone functions that handle filter new, delete and get requests. The only truly protocol independent part of tc_ctl_tfilter is code that looks up queue, class, and block. Refactor this code to standalone tcf_block_find function that is used by all three new handlers. Signed-off-by: Vlad Buslov --- Changes V1 -> V2: - Rebase on current net-next net/sched/cls_api.c | 438 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 293 insertions(+), 145 deletions(-) diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 76303c45db19..c06585fb2dc6 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -437,6 +437,78 @@ static struct tcf_block *tcf_block_lookup(struct net *net, u32 block_index) return idr_find(&tn->idr, block_index); } +/* Find tcf block. + * Set q, parent, cl when appropriate. + */ + +static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q, + u32 *parent, unsigned long *cl, + int ifindex, u32 block_index, + struct netlink_ext_ack *extack) +{ + struct tcf_block *block; + + if (ifindex == TCM_IFINDEX_MAGIC_BLOCK) { + block = tcf_block_lookup(net, block_index); + if (!block) { + NL_SET_ERR_MSG(extack, "Block of given index was not found"); + return ERR_PTR(-EINVAL); + } + } else { + const struct Qdisc_class_ops *cops; + struct net_device *dev; + + /* Find link */ + dev = __dev_get_by_index(net, ifindex); + if (!dev) + return ERR_PTR(-ENODEV); + + /* Find qdisc */ + if (!*parent) { + *q = dev->qdisc; + *parent = (*q)->handle; + } else { + *q = qdisc_lookup(dev, TC_H_MAJ(*parent)); + if (!*q) { + NL_SET_ERR_MSG(extack, "Parent Qdisc doesn't exists"); + return ERR_PTR(-EINVAL); + } + } + + /* Is it classful? */ + cops = (*q)->ops->cl_ops; + if (!cops) { + NL_SET_ERR_MSG(extack, "Qdisc not classful"); + return ERR_PTR(-EINVAL); + } + + if (!cops->tcf_block) { + NL_SET_ERR_MSG(extack, "Class doesn't support blocks"); + return ERR_PTR(-EOPNOTSUPP); + } + + /* Do we search for filter, attached to class? */ + if (TC_H_MIN(*parent)) { + *cl = cops->find(*q, *parent); + if (*cl == 0) { + NL_SET_ERR_MSG(extack, "Specified class doesn't exist"); + return ERR_PTR(-ENOENT); + } + } + + /* And the last stroke */ + block = cops->tcf_block(*q, *cl, extack); + if (!block) + return ERR_PTR(-EINVAL); + if (tcf_block_shared(block)) { + NL_SET_ERR_MSG(extack, "This filter block is shared. Please use the block index to manipulate the filters"); + return ERR_PTR(-EOPNOTSUPP); + } + } + + return block; +} + static struct tcf_chain *tcf_block_chain_zero(struct tcf_block *block) { return list_first_entry(&block->chain_list, struct tcf_chain, list); @@ -984,9 +1056,7 @@ static void tfilter_notify_chain(struct net *net, struct sk_buff *oskb, q, parent, 0, event, false); } -/* Add/change/delete/get a filter node */ - -static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n, +static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n, struct netlink_ext_ack *extack) { struct net *net = sock_net(skb->sk); @@ -1007,8 +1077,7 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n, int err; int tp_created; - if ((n->nlmsg_type != RTM_GETTFILTER) && - !netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) + if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) return -EPERM; replay: @@ -1026,24 +1095,13 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n, cl = 0; if (prio == 0) { - switch (n->nlmsg_type) { - case RTM_DELTFILTER: - if (protocol || t->tcm_handle || tca[TCA_KIND]) { - NL_SET_ERR_MSG(extack, "Cannot flush filters with protocol, handle or kind set"); - return -ENOENT; - } - break; - case RTM_NEWTFILTER: - /* If no priority is provided by the user, - * we allocate one. - */ - if (n->nlmsg_flags & NLM_F_CREATE) { - prio = TC_H_MAKE(0x80000000U, 0U); - prio_allocate = true; - break; - } - /* fall-through */ - default: + /* If no priority is provided by the user, + * we allocate one. + */ + if (n->nlmsg_flags & NLM_F_CREATE) { + prio = TC_H_MAKE(0x80000000U, 0U); + prio_allocate = true; + } else { NL_SET_ERR_MSG(extack, "Invalid filter command with priority of zero"); return -ENOENT; } @@ -1051,66 +1109,11 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n, /* Find head of filter chain. */ - if (t->tcm_ifindex == TCM_IFINDEX_MAGIC_BLOCK) { - block = tcf_block_lookup(net, t->tcm_block_index); - if (!block) { - NL_SET_ERR_MSG(extack, "Block of given index was not found"); - err = -EINVAL; - goto errout; - } - } else { - const struct Qdisc_class_ops *cops; - struct net_device *dev; - - /* Find link */ - dev = __dev_get_by_index(net, t->tcm_ifindex); - if (!dev) - return -ENODEV; - - /* Find qdisc */ - if (!parent) { - q = dev->qdisc; - parent = q->handle; - } else { - q = qdisc_lookup(dev, TC_H_MAJ(t->tcm_parent)); - if (!q) { - NL_SET_ERR_MSG(extack, "Parent Qdisc doesn't exists"); - return -EINVAL; - } - } - - /* Is it classful? */ - cops = q->ops->cl_ops; - if (!cops) { - NL_SET_ERR_MSG(extack, "Qdisc not classful"); - return -EINVAL; - } - - if (!cops->tcf_block) { - NL_SET_ERR_MSG(extack, "Class doesn't support blocks"); - return -EOPNOTSUPP; - } - - /* Do we search for filter, attached to class? */ - if (TC_H_MIN(parent)) { - cl = cops->find(q, parent); - if (cl == 0) { - NL_SET_ERR_MSG(extack, "Specified class doesn't exist"); - return -ENOENT; - } - } - - /* And the last stroke */ - block = cops->tcf_block(q, cl, extack); - if (!block) { - err = -EINVAL; - goto errout; - } - if (tcf_block_shared(block)) { - NL_SET_ERR_MSG(extack, "This filter block is shared. Please use the block index to manipulate the filters"); - err = -EOPNOTSUPP; - goto errout; - } + block = tcf_block_find(net, &q, &parent, &cl, + t->tcm_ifindex, t->tcm_block_index, extack); + if (IS_ERR(block)) { + err = PTR_ERR(block); + goto errout; } chain_index = tca[TCA_CHAIN] ? nla_get_u32(tca[TCA_CHAIN]) : 0; @@ -1119,19 +1122,10 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n, err = -EINVAL; goto errout; } - chain = tcf_chain_get(block, chain_index, - n->nlmsg_type == RTM_NEWTFILTER); + chain = tcf_chain_get(block, chain_index, true); if (!chain) { NL_SET_ERR_MSG(extack, "Cannot find specified filter chain"); - err = n->nlmsg_type == RTM_NEWTFILTER ? -ENOMEM : -EINVAL; - goto errout; - } - - if (n->nlmsg_type == RTM_DELTFILTER && prio == 0) { - tfilter_notify_chain(net, skb, block, q, parent, n, - chain, RTM_DELTFILTER); - tcf_chain_flush(chain); - err = 0; + err = -ENOMEM; goto errout; } @@ -1152,8 +1146,7 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n, goto errout; } - if (n->nlmsg_type != RTM_NEWTFILTER || - !(n->nlmsg_flags & NLM_F_CREATE)) { + if (!(n->nlmsg_flags & NLM_F_CREATE)) { NL_SET_ERR_MSG(extack, "Need both RTM_NEWTFILTER and NLM_F_CREATE to create a new filter"); err = -ENOENT; goto errout; @@ -1178,56 +1171,15 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n, fh = tp->ops->get(tp, t->tcm_handle); if (!fh) { - if (n->nlmsg_type == RTM_DELTFILTER && t->tcm_handle == 0) { - tcf_chain_tp_remove(chain, &chain_info, tp); - tfilter_notify(net, skb, n, tp, block, q, parent, fh, - RTM_DELTFILTER, false); - tcf_proto_destroy(tp, extack); - err = 0; - goto errout; - } - - if (n->nlmsg_type != RTM_NEWTFILTER || - !(n->nlmsg_flags & NLM_F_CREATE)) { + if (!(n->nlmsg_flags & NLM_F_CREATE)) { NL_SET_ERR_MSG(extack, "Need both RTM_NEWTFILTER and NLM_F_CREATE to create a new filter"); err = -ENOENT; goto errout; } - } else { - bool last; - - switch (n->nlmsg_type) { - case RTM_NEWTFILTER: - if (n->nlmsg_flags & NLM_F_EXCL) { - if (tp_created) - tcf_proto_destroy(tp, NULL); - NL_SET_ERR_MSG(extack, "Filter already exists"); - err = -EEXIST; - goto errout; - } - break; - case RTM_DELTFILTER: - err = tfilter_del_notify(net, skb, n, tp, block, - q, parent, fh, false, &last, - extack); - if (err) - goto errout; - if (last) { - tcf_chain_tp_remove(chain, &chain_info, tp); - tcf_proto_destroy(tp, extack); - } - goto errout; - case RTM_GETTFILTER: - err = tfilter_notify(net, skb, n, tp, block, q, parent, - fh, RTM_NEWTFILTER, true); - if (err < 0) - NL_SET_ERR_MSG(extack, "Failed to send filter notify message"); - goto errout; - default: - NL_SET_ERR_MSG(extack, "Invalid netlink message type"); - err = -EINVAL; - goto errout; - } + } else if (n->nlmsg_flags & NLM_F_EXCL) { + NL_SET_ERR_MSG(extack, "Filter already exists"); + err = -EEXIST; + goto errout; } err = tp->ops->change(net, skb, tp, cl, t->tcm_handle, tca, &fh, @@ -1252,6 +1204,202 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n, return err; } +static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n, + struct netlink_ext_ack *extack) +{ + struct net *net = sock_net(skb->sk); + struct nlattr *tca[TCA_MAX + 1]; + struct tcmsg *t; + u32 protocol; + u32 prio; + u32 parent; + u32 chain_index; + struct Qdisc *q = NULL; + struct tcf_chain_info chain_info; + struct tcf_chain *chain = NULL; + struct tcf_block *block; + struct tcf_proto *tp = NULL; + unsigned long cl = 0; + void *fh = NULL; + int err; + + if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) + return -EPERM; + + err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, NULL, extack); + if (err < 0) + return err; + + t = nlmsg_data(n); + protocol = TC_H_MIN(t->tcm_info); + prio = TC_H_MAJ(t->tcm_info); + parent = t->tcm_parent; + + if (prio == 0 && (protocol || t->tcm_handle || tca[TCA_KIND])) { + NL_SET_ERR_MSG(extack, "Cannot flush filters with protocol, handle or kind set"); + return -ENOENT; + } + + /* Find head of filter chain. */ + + block = tcf_block_find(net, &q, &parent, &cl, + t->tcm_ifindex, t->tcm_block_index, extack); + if (IS_ERR(block)) { + err = PTR_ERR(block); + goto errout; + } + + chain_index = tca[TCA_CHAIN] ? nla_get_u32(tca[TCA_CHAIN]) : 0; + if (chain_index > TC_ACT_EXT_VAL_MASK) { + NL_SET_ERR_MSG(extack, "Specified chain index exceeds upper limit"); + err = -EINVAL; + goto errout; + } + chain = tcf_chain_get(block, chain_index, false); + if (!chain) { + NL_SET_ERR_MSG(extack, "Cannot find specified filter chain"); + err = -EINVAL; + goto errout; + } + + if (prio == 0) { + tfilter_notify_chain(net, skb, block, q, parent, n, + chain, RTM_DELTFILTER); + tcf_chain_flush(chain); + err = 0; + goto errout; + } + + tp = tcf_chain_tp_find(chain, &chain_info, protocol, + prio, false); + if (!tp || IS_ERR(tp)) { + NL_SET_ERR_MSG(extack, "Filter with specified priority/protocol not found"); + err = PTR_ERR(tp); + goto errout; + } else if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], tp->ops->kind)) { + NL_SET_ERR_MSG(extack, "Specified filter kind does not match existing one"); + err = -EINVAL; + goto errout; + } + + fh = tp->ops->get(tp, t->tcm_handle); + + if (!fh) { + if (t->tcm_handle == 0) { + tcf_chain_tp_remove(chain, &chain_info, tp); + tfilter_notify(net, skb, n, tp, block, q, parent, fh, + RTM_DELTFILTER, false); + tcf_proto_destroy(tp, extack); + err = 0; + } else { + NL_SET_ERR_MSG(extack, "Specified filter handle not found"); + err = -ENOENT; + } + } else { + bool last; + + err = tfilter_del_notify(net, skb, n, tp, block, + q, parent, fh, false, &last, + extack); + if (err) + goto errout; + if (last) { + tcf_chain_tp_remove(chain, &chain_info, tp); + tcf_proto_destroy(tp, extack); + } + } + +errout: + if (chain) + tcf_chain_put(chain); + return err; +} + +static int tc_get_tfilter(struct sk_buff *skb, struct nlmsghdr *n, + struct netlink_ext_ack *extack) +{ + struct net *net = sock_net(skb->sk); + struct nlattr *tca[TCA_MAX + 1]; + struct tcmsg *t; + u32 protocol; + u32 prio; + u32 parent; + u32 chain_index; + struct Qdisc *q = NULL; + struct tcf_chain_info chain_info; + struct tcf_chain *chain = NULL; + struct tcf_block *block; + struct tcf_proto *tp = NULL; + unsigned long cl = 0; + void *fh = NULL; + int err; + + err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, NULL, extack); + if (err < 0) + return err; + + t = nlmsg_data(n); + protocol = TC_H_MIN(t->tcm_info); + prio = TC_H_MAJ(t->tcm_info); + parent = t->tcm_parent; + + if (prio == 0) { + NL_SET_ERR_MSG(extack, "Invalid filter command with priority of zero"); + return -ENOENT; + } + + /* Find head of filter chain. */ + + block = tcf_block_find(net, &q, &parent, &cl, + t->tcm_ifindex, t->tcm_block_index, extack); + if (IS_ERR(block)) { + err = PTR_ERR(block); + goto errout; + } + + chain_index = tca[TCA_CHAIN] ? nla_get_u32(tca[TCA_CHAIN]) : 0; + if (chain_index > TC_ACT_EXT_VAL_MASK) { + NL_SET_ERR_MSG(extack, "Specified chain index exceeds upper limit"); + err = -EINVAL; + goto errout; + } + chain = tcf_chain_get(block, chain_index, false); + if (!chain) { + NL_SET_ERR_MSG(extack, "Cannot find specified filter chain"); + err = -EINVAL; + goto errout; + } + + tp = tcf_chain_tp_find(chain, &chain_info, protocol, + prio, false); + if (!tp || IS_ERR(tp)) { + NL_SET_ERR_MSG(extack, "Filter with specified priority/protocol not found"); + err = PTR_ERR(tp); + goto errout; + } else if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], tp->ops->kind)) { + NL_SET_ERR_MSG(extack, "Specified filter kind does not match existing one"); + err = -EINVAL; + goto errout; + } + + fh = tp->ops->get(tp, t->tcm_handle); + + if (!fh) { + NL_SET_ERR_MSG(extack, "Specified filter handle not found"); + err = -ENOENT; + } else { + err = tfilter_notify(net, skb, n, tp, block, q, parent, + fh, RTM_NEWTFILTER, true); + if (err < 0) + NL_SET_ERR_MSG(extack, "Failed to send filter notify message"); + } + +errout: + if (chain) + tcf_chain_put(chain); + return err; +} + struct tcf_dump_args { struct tcf_walker w; struct sk_buff *skb; @@ -1634,9 +1782,9 @@ static int __init tc_filter_init(void) if (err) goto err_register_pernet_subsys; - rtnl_register(PF_UNSPEC, RTM_NEWTFILTER, tc_ctl_tfilter, NULL, 0); - rtnl_register(PF_UNSPEC, RTM_DELTFILTER, tc_ctl_tfilter, NULL, 0); - rtnl_register(PF_UNSPEC, RTM_GETTFILTER, tc_ctl_tfilter, + rtnl_register(PF_UNSPEC, RTM_NEWTFILTER, tc_new_tfilter, NULL, 0); + rtnl_register(PF_UNSPEC, RTM_DELTFILTER, tc_del_tfilter, NULL, 0); + rtnl_register(PF_UNSPEC, RTM_GETTFILTER, tc_get_tfilter, tc_dump_tfilter, 0); return 0;