From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pablo Neira Ayuso Subject: [PATCH net-next 12/12] qede: use ethtool_rx_flow_rule() to remove duplicated parser code Date: Mon, 19 Nov 2018 01:15:19 +0100 Message-ID: <20181119001519.12124-13-pablo@netfilter.org> References: <20181119001519.12124-1-pablo@netfilter.org> Cc: davem@davemloft.net, thomas.lendacky@amd.com, f.fainelli@gmail.com, ariel.elior@cavium.com, michael.chan@broadcom.com, santosh@chelsio.com, madalin.bucur@nxp.com, yisen.zhuang@huawei.com, salil.mehta@huawei.com, jeffrey.t.kirsher@intel.com, tariqt@mellanox.com, saeedm@mellanox.com, jiri@mellanox.com, idosch@mellanox.com, jakub.kicinski@netronome.com, peppe.cavallaro@st.com, grygorii.strashko@ti.com, andrew@lunn.ch, vivien.didelot@savoirfairelinux.com, alexandre.torgue@st.com, joabreu@synopsys.com, linux-net-drivers@solarflare.com, ganeshgr@chelsio.com, ogerlitz@mellanox.com To: netdev@vger.kernel.org Return-path: Received: from mail.us.es ([193.147.175.20]:58266 "EHLO mail.us.es" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727482AbeKSKjr (ORCPT ); Mon, 19 Nov 2018 05:39:47 -0500 Received: from antivirus1-rhel7.int (unknown [192.168.2.11]) by mail.us.es (Postfix) with ESMTP id 2E78417AE3A for ; Mon, 19 Nov 2018 01:17:55 +0100 (CET) Received: from antivirus1-rhel7.int (localhost [127.0.0.1]) by antivirus1-rhel7.int (Postfix) with ESMTP id 1BBB8DA7A0 for ; Mon, 19 Nov 2018 01:17:55 +0100 (CET) In-Reply-To: <20181119001519.12124-1-pablo@netfilter.org> Sender: netdev-owner@vger.kernel.org List-ID: The qede driver supports for ethtool_rx_flow_spec and flower, both codebases look very similar. This patch uses the ethtool_rx_flow_rule() infrastructure to remove the duplicated ethtool_rx_flow_spec parser and consolidate ACL offload support around the flow_rule infrastructure. Furthermore, more code can be consolidated by merging qede_add_cls_rule() and qede_add_tc_flower_fltr(), these two functions also look very similar. This driver currently provides simple ACL support, such as 5-tuple matching, drop policy and queue to CPU. Drivers that support more features can benefit from this infrastructure to save even more redundant codebase. Signed-off-by: Pablo Neira Ayuso --- Note that, after this patch, qede_add_cls_rule() and qede_add_tc_flower_fltr() can be also consolidated since their code is redundant. drivers/net/ethernet/qlogic/qede/qede_filter.c | 246 ++++++------------------- 1 file changed, 53 insertions(+), 193 deletions(-) diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c index aca302c3261b..f82b26ba8f80 100644 --- a/drivers/net/ethernet/qlogic/qede/qede_filter.c +++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c @@ -1578,30 +1578,6 @@ static void qede_flow_build_ipv6_hdr(struct qede_arfs_tuple *t, ports[1] = t->dst_port; } -/* Validate fields which are set and not accepted by the driver */ -static int qede_flow_spec_validate_unused(struct qede_dev *edev, - struct ethtool_rx_flow_spec *fs) -{ - if (fs->flow_type & FLOW_MAC_EXT) { - DP_INFO(edev, "Don't support MAC extensions\n"); - return -EOPNOTSUPP; - } - - if ((fs->flow_type & FLOW_EXT) && - (fs->h_ext.vlan_etype || fs->h_ext.vlan_tci)) { - DP_INFO(edev, "Don't support vlan-based classification\n"); - return -EOPNOTSUPP; - } - - if ((fs->flow_type & FLOW_EXT) && - (fs->h_ext.data[0] || fs->h_ext.data[1])) { - DP_INFO(edev, "Don't support user defined data\n"); - return -EOPNOTSUPP; - } - - return 0; -} - static int qede_set_v4_tuple_to_profile(struct qede_dev *edev, struct qede_arfs_tuple *t) { @@ -1665,132 +1641,6 @@ static int qede_set_v6_tuple_to_profile(struct qede_dev *edev, return 0; } -static int qede_flow_spec_to_tuple_ipv4_common(struct qede_dev *edev, - struct qede_arfs_tuple *t, - struct ethtool_rx_flow_spec *fs) -{ - if ((fs->h_u.tcp_ip4_spec.ip4src & - fs->m_u.tcp_ip4_spec.ip4src) != fs->h_u.tcp_ip4_spec.ip4src) { - DP_INFO(edev, "Don't support IP-masks\n"); - return -EOPNOTSUPP; - } - - if ((fs->h_u.tcp_ip4_spec.ip4dst & - fs->m_u.tcp_ip4_spec.ip4dst) != fs->h_u.tcp_ip4_spec.ip4dst) { - DP_INFO(edev, "Don't support IP-masks\n"); - return -EOPNOTSUPP; - } - - if ((fs->h_u.tcp_ip4_spec.psrc & - fs->m_u.tcp_ip4_spec.psrc) != fs->h_u.tcp_ip4_spec.psrc) { - DP_INFO(edev, "Don't support port-masks\n"); - return -EOPNOTSUPP; - } - - if ((fs->h_u.tcp_ip4_spec.pdst & - fs->m_u.tcp_ip4_spec.pdst) != fs->h_u.tcp_ip4_spec.pdst) { - DP_INFO(edev, "Don't support port-masks\n"); - return -EOPNOTSUPP; - } - - if (fs->h_u.tcp_ip4_spec.tos) { - DP_INFO(edev, "Don't support tos\n"); - return -EOPNOTSUPP; - } - - t->eth_proto = htons(ETH_P_IP); - t->src_ipv4 = fs->h_u.tcp_ip4_spec.ip4src; - t->dst_ipv4 = fs->h_u.tcp_ip4_spec.ip4dst; - t->src_port = fs->h_u.tcp_ip4_spec.psrc; - t->dst_port = fs->h_u.tcp_ip4_spec.pdst; - - return qede_set_v4_tuple_to_profile(edev, t); -} - -static int qede_flow_spec_to_tuple_tcpv4(struct qede_dev *edev, - struct qede_arfs_tuple *t, - struct ethtool_rx_flow_spec *fs) -{ - t->ip_proto = IPPROTO_TCP; - - if (qede_flow_spec_to_tuple_ipv4_common(edev, t, fs)) - return -EINVAL; - - return 0; -} - -static int qede_flow_spec_to_tuple_udpv4(struct qede_dev *edev, - struct qede_arfs_tuple *t, - struct ethtool_rx_flow_spec *fs) -{ - t->ip_proto = IPPROTO_UDP; - - if (qede_flow_spec_to_tuple_ipv4_common(edev, t, fs)) - return -EINVAL; - - return 0; -} - -static int qede_flow_spec_to_tuple_ipv6_common(struct qede_dev *edev, - struct qede_arfs_tuple *t, - struct ethtool_rx_flow_spec *fs) -{ - struct in6_addr zero_addr; - - memset(&zero_addr, 0, sizeof(zero_addr)); - - if ((fs->h_u.tcp_ip6_spec.psrc & - fs->m_u.tcp_ip6_spec.psrc) != fs->h_u.tcp_ip6_spec.psrc) { - DP_INFO(edev, "Don't support port-masks\n"); - return -EOPNOTSUPP; - } - - if ((fs->h_u.tcp_ip6_spec.pdst & - fs->m_u.tcp_ip6_spec.pdst) != fs->h_u.tcp_ip6_spec.pdst) { - DP_INFO(edev, "Don't support port-masks\n"); - return -EOPNOTSUPP; - } - - if (fs->h_u.tcp_ip6_spec.tclass) { - DP_INFO(edev, "Don't support tclass\n"); - return -EOPNOTSUPP; - } - - t->eth_proto = htons(ETH_P_IPV6); - memcpy(&t->src_ipv6, &fs->h_u.tcp_ip6_spec.ip6src, - sizeof(struct in6_addr)); - memcpy(&t->dst_ipv6, &fs->h_u.tcp_ip6_spec.ip6dst, - sizeof(struct in6_addr)); - t->src_port = fs->h_u.tcp_ip6_spec.psrc; - t->dst_port = fs->h_u.tcp_ip6_spec.pdst; - - return qede_set_v6_tuple_to_profile(edev, t, &zero_addr); -} - -static int qede_flow_spec_to_tuple_tcpv6(struct qede_dev *edev, - struct qede_arfs_tuple *t, - struct ethtool_rx_flow_spec *fs) -{ - t->ip_proto = IPPROTO_TCP; - - if (qede_flow_spec_to_tuple_ipv6_common(edev, t, fs)) - return -EINVAL; - - return 0; -} - -static int qede_flow_spec_to_tuple_udpv6(struct qede_dev *edev, - struct qede_arfs_tuple *t, - struct ethtool_rx_flow_spec *fs) -{ - t->ip_proto = IPPROTO_UDP; - - if (qede_flow_spec_to_tuple_ipv6_common(edev, t, fs)) - return -EINVAL; - - return 0; -} - /* Must be called while qede lock is held */ static struct qede_arfs_fltr_node * qede_flow_find_fltr(struct qede_dev *edev, struct qede_arfs_tuple *t) @@ -1875,25 +1725,32 @@ static int qede_parse_actions(struct qede_dev *edev, struct flow_action *flow_action) { const struct flow_action_key *act; - int rc = -EINVAL, num_act = 0, i; - bool is_drop = false; + int i; if (!flow_action_has_keys(flow_action)) { - DP_NOTICE(edev, "No tc actions received\n"); - return rc; + DP_NOTICE(edev, "No actions received\n"); + return -EINVAL; } flow_action_for_each(i, act, flow_action) { - num_act++; + switch (act->id) { + case FLOW_ACTION_KEY_DROP: + break; + case FLOW_ACTION_KEY_QUEUE: + if (ethtool_get_flow_spec_ring_vf(act->queue_index)) + break; - if (act->id == FLOW_ACTION_KEY_DROP) - is_drop = true; + if (act->queue_index >= QEDE_RSS_COUNT(edev)) { + DP_INFO(edev, "Queue out-of-bounds\n"); + return -EINVAL; + } + break; + default: + return -EINVAL; + } } - if (num_act == 1 && is_drop) - return 0; - - return rc; + return 0; } static int @@ -2147,16 +2004,17 @@ int qede_add_tc_flower_fltr(struct qede_dev *edev, __be16 proto, } static int qede_flow_spec_validate(struct qede_dev *edev, - struct ethtool_rx_flow_spec *fs, - struct qede_arfs_tuple *t) + struct flow_action *flow_action, + struct qede_arfs_tuple *t, + __u32 location) { - if (fs->location >= QEDE_RFS_MAX_FLTR) { + if (location >= QEDE_RFS_MAX_FLTR) { DP_INFO(edev, "Location out-of-bounds\n"); return -EINVAL; } /* Check location isn't already in use */ - if (test_bit(fs->location, edev->arfs->arfs_fltr_bmap)) { + if (test_bit(location, edev->arfs->arfs_fltr_bmap)) { DP_INFO(edev, "Location already in use\n"); return -EINVAL; } @@ -2170,46 +2028,53 @@ static int qede_flow_spec_validate(struct qede_dev *edev, return -EINVAL; } - /* If drop requested then no need to validate other data */ - if (fs->ring_cookie == RX_CLS_FLOW_DISC) - return 0; - - if (ethtool_get_flow_spec_ring_vf(fs->ring_cookie)) - return 0; - - if (fs->ring_cookie >= QEDE_RSS_COUNT(edev)) { - DP_INFO(edev, "Queue out-of-bounds\n"); + if (qede_parse_actions(edev, flow_action)) return -EINVAL; - } return 0; } -static int qede_flow_spec_to_tuple(struct qede_dev *edev, - struct qede_arfs_tuple *t, - struct ethtool_rx_flow_spec *fs) +static int qede_flow_spec_to_rule(struct qede_dev *edev, + struct qede_arfs_tuple *t, + struct ethtool_rx_flow_spec *fs) { - memset(t, 0, sizeof(*t)); - - if (qede_flow_spec_validate_unused(edev, fs)) - return -EOPNOTSUPP; + struct tc_cls_flower_offload f = {}; + struct flow_rule *flow_rule; + __be16 proto; + int err = 0; switch ((fs->flow_type & ~FLOW_EXT)) { case TCP_V4_FLOW: - return qede_flow_spec_to_tuple_tcpv4(edev, t, fs); case UDP_V4_FLOW: - return qede_flow_spec_to_tuple_udpv4(edev, t, fs); + proto = htons(ETH_P_IP); + break; case TCP_V6_FLOW: - return qede_flow_spec_to_tuple_tcpv6(edev, t, fs); case UDP_V6_FLOW: - return qede_flow_spec_to_tuple_udpv6(edev, t, fs); + proto = htons(ETH_P_IPV6); + break; default: DP_VERBOSE(edev, NETIF_MSG_IFUP, "Can't support flow of type %08x\n", fs->flow_type); return -EOPNOTSUPP; } - return 0; + flow_rule = ethtool_rx_flow_rule(fs); + if (!flow_rule) + return -ENOMEM; + + f.rule = *flow_rule; + + if (qede_parse_flower_attr(edev, proto, &f, t)) { + err = -EINVAL; + goto err_out; + } + + /* Make sure location is valid and filter isn't already set */ + err = qede_flow_spec_validate(edev, &f.rule.action, t, fs->location); +err_out: + ethtool_rx_flow_rule_free(flow_rule); + return err; + } int qede_add_cls_rule(struct qede_dev *edev, struct ethtool_rxnfc *info) @@ -2227,12 +2092,7 @@ int qede_add_cls_rule(struct qede_dev *edev, struct ethtool_rxnfc *info) } /* Translate the flow specification into something fittign our DB */ - rc = qede_flow_spec_to_tuple(edev, &t, fsp); - if (rc) - goto unlock; - - /* Make sure location is valid and filter isn't already set */ - rc = qede_flow_spec_validate(edev, fsp, &t); + rc = qede_flow_spec_to_rule(edev, &t, fsp); if (rc) goto unlock; -- 2.11.0