* [PATCH net-next,v3 12/12] qede: use ethtool_rx_flow_rule() to remove duplicated parser code
From: Pablo Neira Ayuso @ 2018-11-21 2:51 UTC (permalink / raw)
To: netdev
Cc: davem, thomas.lendacky, f.fainelli, ariel.elior, michael.chan,
santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz,
Manish.Chopra
In-Reply-To: <20181121025132.14305-1-pablo@netfilter.org>
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 <pablo@netfilter.org>
---
v3: Suggested by Jiri Pirko:
- Pass struct flow_rule *rule to all parser functions.
Moreover, do not remove qede_flow_spec_validate_unused().
drivers/net/ethernet/qlogic/qede/qede_filter.c | 271 +++++++------------------
1 file changed, 71 insertions(+), 200 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c
index ed77950f6cf9..b7562fb86e52 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c
@@ -1665,132 +1665,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,34 +1749,38 @@ static int qede_parse_actions(struct qede_dev *edev,
struct flow_action *flow_action)
{
const struct flow_action_entry *act;
- int rc = -EINVAL, num_act = 0, i;
- bool is_drop = false;
+ int i;
if (!flow_action_has_entries(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_DROP:
+ break;
+ case FLOW_ACTION_QUEUE:
+ if (ethtool_get_flow_spec_ring_vf(act->queue_index))
+ break;
- if (act->id == FLOW_ACTION_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
-qede_tc_parse_ports(struct qede_dev *edev,
- struct tc_cls_flower_offload *f,
+qede_tc_parse_ports(struct qede_dev *edev, struct flow_rule *rule,
struct qede_arfs_tuple *t)
{
- struct flow_rule *rule = tc_cls_flower_offload_flow_rule(f);
-
if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_PORTS)) {
struct flow_match_ports match;
@@ -1921,11 +1799,9 @@ qede_tc_parse_ports(struct qede_dev *edev,
}
static int
-qede_tc_parse_v6_common(struct qede_dev *edev,
- struct tc_cls_flower_offload *f,
+qede_tc_parse_v6_common(struct qede_dev *edev, struct flow_rule *rule,
struct qede_arfs_tuple *t)
{
- struct flow_rule *rule = tc_cls_flower_offload_flow_rule(f);
struct in6_addr zero_addr, addr;
memset(&zero_addr, 0, sizeof(addr));
@@ -1948,19 +1824,16 @@ qede_tc_parse_v6_common(struct qede_dev *edev,
memcpy(&t->dst_ipv6, &match.key->dst, sizeof(addr));
}
- if (qede_tc_parse_ports(edev, f, t))
+ if (qede_tc_parse_ports(edev, rule, t))
return -EINVAL;
return qede_set_v6_tuple_to_profile(edev, t, &zero_addr);
}
static int
-qede_tc_parse_v4_common(struct qede_dev *edev,
- struct tc_cls_flower_offload *f,
+qede_tc_parse_v4_common(struct qede_dev *edev, struct flow_rule *rule,
struct qede_arfs_tuple *t)
{
- struct flow_rule *rule = tc_cls_flower_offload_flow_rule(f);
-
if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_IPV4_ADDRS)) {
struct flow_match_ipv4_addrs match;
@@ -1975,62 +1848,56 @@ qede_tc_parse_v4_common(struct qede_dev *edev,
t->dst_ipv4 = match.key->dst;
}
- if (qede_tc_parse_ports(edev, f, t))
+ if (qede_tc_parse_ports(edev, rule, t))
return -EINVAL;
return qede_set_v4_tuple_to_profile(edev, t);
}
static int
-qede_tc_parse_tcp_v6(struct qede_dev *edev,
- struct tc_cls_flower_offload *f,
+qede_tc_parse_tcp_v6(struct qede_dev *edev, struct flow_rule *rule,
struct qede_arfs_tuple *tuple)
{
tuple->ip_proto = IPPROTO_TCP;
tuple->eth_proto = htons(ETH_P_IPV6);
- return qede_tc_parse_v6_common(edev, f, tuple);
+ return qede_tc_parse_v6_common(edev, rule, tuple);
}
static int
-qede_tc_parse_tcp_v4(struct qede_dev *edev,
- struct tc_cls_flower_offload *f,
+qede_tc_parse_tcp_v4(struct qede_dev *edev, struct flow_rule *rule,
struct qede_arfs_tuple *tuple)
{
tuple->ip_proto = IPPROTO_TCP;
tuple->eth_proto = htons(ETH_P_IP);
- return qede_tc_parse_v4_common(edev, f, tuple);
+ return qede_tc_parse_v4_common(edev, rule, tuple);
}
static int
-qede_tc_parse_udp_v6(struct qede_dev *edev,
- struct tc_cls_flower_offload *f,
+qede_tc_parse_udp_v6(struct qede_dev *edev, struct flow_rule *rule,
struct qede_arfs_tuple *tuple)
{
tuple->ip_proto = IPPROTO_UDP;
tuple->eth_proto = htons(ETH_P_IPV6);
- return qede_tc_parse_v6_common(edev, f, tuple);
+ return qede_tc_parse_v6_common(edev, rule, tuple);
}
static int
-qede_tc_parse_udp_v4(struct qede_dev *edev,
- struct tc_cls_flower_offload *f,
+qede_tc_parse_udp_v4(struct qede_dev *edev, struct flow_rule *rule,
struct qede_arfs_tuple *tuple)
{
tuple->ip_proto = IPPROTO_UDP;
tuple->eth_proto = htons(ETH_P_IP);
- return qede_tc_parse_v4_common(edev, f, tuple);
+ return qede_tc_parse_v4_common(edev, rule, tuple);
}
static int
qede_parse_flower_attr(struct qede_dev *edev, __be16 proto,
- struct tc_cls_flower_offload *f,
- struct qede_arfs_tuple *tuple)
+ struct flow_rule *rule, struct qede_arfs_tuple *tuple)
{
- struct flow_rule *rule = tc_cls_flower_offload_flow_rule(f);
struct flow_dissector *dissector = rule->match.dissector;
int rc = -EINVAL;
u8 ip_proto = 0;
@@ -2062,15 +1929,15 @@ qede_parse_flower_attr(struct qede_dev *edev, __be16 proto,
}
if (ip_proto == IPPROTO_TCP && proto == htons(ETH_P_IP))
- rc = qede_tc_parse_tcp_v4(edev, f, tuple);
+ rc = qede_tc_parse_tcp_v4(edev, rule, tuple);
else if (ip_proto == IPPROTO_TCP && proto == htons(ETH_P_IPV6))
- rc = qede_tc_parse_tcp_v6(edev, f, tuple);
+ rc = qede_tc_parse_tcp_v6(edev, rule, tuple);
else if (ip_proto == IPPROTO_UDP && proto == htons(ETH_P_IP))
- rc = qede_tc_parse_udp_v4(edev, f, tuple);
+ rc = qede_tc_parse_udp_v4(edev, rule, tuple);
else if (ip_proto == IPPROTO_UDP && proto == htons(ETH_P_IPV6))
- rc = qede_tc_parse_udp_v6(edev, f, tuple);
+ rc = qede_tc_parse_udp_v6(edev, rule, tuple);
else
- DP_NOTICE(edev, "Invalid tc protocol request\n");
+ DP_NOTICE(edev, "Invalid protocol request\n");
return rc;
}
@@ -2090,7 +1957,7 @@ int qede_add_tc_flower_fltr(struct qede_dev *edev, __be16 proto,
}
/* parse flower attribute and prepare filter */
- if (qede_parse_flower_attr(edev, proto, f, &t))
+ if (qede_parse_flower_attr(edev, proto, f->rule, &t))
goto unlock;
/* Validate profile mode and number of filters */
@@ -2147,16 +2014,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 +2038,54 @@ 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));
+ struct ethtool_rx_flow_rule *flow;
+ __be16 proto;
+ int err = 0;
if (qede_flow_spec_validate_unused(edev, fs))
return -EOPNOTSUPP;
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 = ethtool_rx_flow_rule_alloc(fs);
+ if (!flow)
+ return -ENOMEM;
+
+ if (qede_parse_flower_attr(edev, proto, flow->rule, t)) {
+ err = -EINVAL;
+ goto err_out;
+ }
+
+ /* Make sure location is valid and filter isn't already set */
+ err = qede_flow_spec_validate(edev, &flow->rule->action, t,
+ fs->location);
+err_out:
+ ethtool_rx_flow_rule_free(flow);
+ return err;
+
}
int qede_add_cls_rule(struct qede_dev *edev, struct ethtool_rxnfc *info)
@@ -2227,12 +2103,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
^ permalink raw reply related
* [PATCH net-next,v3 11/12] qede: place ethtool_rx_flow_spec after code after TC flower codebase
From: Pablo Neira Ayuso @ 2018-11-21 2:51 UTC (permalink / raw)
To: netdev
Cc: davem, thomas.lendacky, f.fainelli, ariel.elior, michael.chan,
santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz,
Manish.Chopra
In-Reply-To: <20181121025132.14305-1-pablo@netfilter.org>
This is a preparation patch to reuse the existing TC flower codebase
from ethtool_rx_flow_spec.
This patch is merely moving the core ethtool_rx_flow_spec parser after
tc flower offload driver code so we can skip a few forward function
declarations in the follow up patch.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v3: no changes.
drivers/net/ethernet/qlogic/qede/qede_filter.c | 264 ++++++++++++-------------
1 file changed, 132 insertions(+), 132 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c
index 833c9ec58a6e..ed77950f6cf9 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c
@@ -1791,72 +1791,6 @@ static int qede_flow_spec_to_tuple_udpv6(struct qede_dev *edev,
return 0;
}
-static int qede_flow_spec_to_tuple(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;
-
- 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);
- 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);
- default:
- DP_VERBOSE(edev, NETIF_MSG_IFUP,
- "Can't support flow of type %08x\n", fs->flow_type);
- return -EOPNOTSUPP;
- }
-
- return 0;
-}
-
-static int qede_flow_spec_validate(struct qede_dev *edev,
- struct ethtool_rx_flow_spec *fs,
- struct qede_arfs_tuple *t)
-{
- if (fs->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)) {
- DP_INFO(edev, "Location already in use\n");
- return -EINVAL;
- }
-
- /* Check if the filtering-mode could support the filter */
- if (edev->arfs->filter_count &&
- edev->arfs->mode != t->mode) {
- DP_INFO(edev,
- "flow_spec would require filtering mode %08x, but %08x is configured\n",
- t->mode, edev->arfs->filter_count);
- 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");
- 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)
@@ -1896,72 +1830,6 @@ static void qede_flow_set_destination(struct qede_dev *edev,
"Configuring N-tuple for VF 0x%02x\n", n->vfid - 1);
}
-int qede_add_cls_rule(struct qede_dev *edev, struct ethtool_rxnfc *info)
-{
- struct ethtool_rx_flow_spec *fsp = &info->fs;
- struct qede_arfs_fltr_node *n;
- struct qede_arfs_tuple t;
- int min_hlen, rc;
-
- __qede_lock(edev);
-
- if (!edev->arfs) {
- rc = -EPERM;
- goto unlock;
- }
-
- /* 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);
- if (rc)
- goto unlock;
-
- if (qede_flow_find_fltr(edev, &t)) {
- rc = -EINVAL;
- goto unlock;
- }
-
- n = kzalloc(sizeof(*n), GFP_KERNEL);
- if (!n) {
- rc = -ENOMEM;
- goto unlock;
- }
-
- min_hlen = qede_flow_get_min_header_size(&t);
- n->data = kzalloc(min_hlen, GFP_KERNEL);
- if (!n->data) {
- kfree(n);
- rc = -ENOMEM;
- goto unlock;
- }
-
- n->sw_id = fsp->location;
- set_bit(n->sw_id, edev->arfs->arfs_fltr_bmap);
- n->buf_len = min_hlen;
-
- memcpy(&n->tuple, &t, sizeof(n->tuple));
-
- qede_flow_set_destination(edev, n, fsp);
-
- /* Build a minimal header according to the flow */
- n->tuple.build_hdr(&n->tuple, n->data);
-
- rc = qede_enqueue_fltr_and_config_searcher(edev, n, 0);
- if (rc)
- goto unlock;
-
- qede_configure_arfs_fltr(edev, n, n->rxq_id, true);
- rc = qede_poll_arfs_filter_config(edev, n);
-unlock:
- __qede_unlock(edev);
-
- return rc;
-}
-
int qede_delete_flow_filter(struct qede_dev *edev, u64 cookie)
{
struct qede_arfs_fltr_node *fltr = NULL;
@@ -2277,3 +2145,135 @@ int qede_add_tc_flower_fltr(struct qede_dev *edev, __be16 proto,
__qede_unlock(edev);
return rc;
}
+
+static int qede_flow_spec_validate(struct qede_dev *edev,
+ struct ethtool_rx_flow_spec *fs,
+ struct qede_arfs_tuple *t)
+{
+ if (fs->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)) {
+ DP_INFO(edev, "Location already in use\n");
+ return -EINVAL;
+ }
+
+ /* Check if the filtering-mode could support the filter */
+ if (edev->arfs->filter_count &&
+ edev->arfs->mode != t->mode) {
+ DP_INFO(edev,
+ "flow_spec would require filtering mode %08x, but %08x is configured\n",
+ t->mode, edev->arfs->filter_count);
+ 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");
+ 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)
+{
+ memset(t, 0, sizeof(*t));
+
+ if (qede_flow_spec_validate_unused(edev, fs))
+ return -EOPNOTSUPP;
+
+ 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);
+ 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);
+ default:
+ DP_VERBOSE(edev, NETIF_MSG_IFUP,
+ "Can't support flow of type %08x\n", fs->flow_type);
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+int qede_add_cls_rule(struct qede_dev *edev, struct ethtool_rxnfc *info)
+{
+ struct ethtool_rx_flow_spec *fsp = &info->fs;
+ struct qede_arfs_fltr_node *n;
+ struct qede_arfs_tuple t;
+ int min_hlen, rc;
+
+ __qede_lock(edev);
+
+ if (!edev->arfs) {
+ rc = -EPERM;
+ goto unlock;
+ }
+
+ /* 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);
+ if (rc)
+ goto unlock;
+
+ if (qede_flow_find_fltr(edev, &t)) {
+ rc = -EINVAL;
+ goto unlock;
+ }
+
+ n = kzalloc(sizeof(*n), GFP_KERNEL);
+ if (!n) {
+ rc = -ENOMEM;
+ goto unlock;
+ }
+
+ min_hlen = qede_flow_get_min_header_size(&t);
+ n->data = kzalloc(min_hlen, GFP_KERNEL);
+ if (!n->data) {
+ kfree(n);
+ rc = -ENOMEM;
+ goto unlock;
+ }
+
+ n->sw_id = fsp->location;
+ set_bit(n->sw_id, edev->arfs->arfs_fltr_bmap);
+ n->buf_len = min_hlen;
+
+ memcpy(&n->tuple, &t, sizeof(n->tuple));
+
+ qede_flow_set_destination(edev, n, fsp);
+
+ /* Build a minimal header according to the flow */
+ n->tuple.build_hdr(&n->tuple, n->data);
+
+ rc = qede_enqueue_fltr_and_config_searcher(edev, n, 0);
+ if (rc)
+ goto unlock;
+
+ qede_configure_arfs_fltr(edev, n, n->rxq_id, true);
+ rc = qede_poll_arfs_filter_config(edev, n);
+unlock:
+ __qede_unlock(edev);
+
+ return rc;
+}
--
2.11.0
^ permalink raw reply related
* [PATCH net-next,v3 10/12] dsa: bcm_sf2: use flow_rule infrastructure
From: Pablo Neira Ayuso @ 2018-11-21 2:51 UTC (permalink / raw)
To: netdev
Cc: davem, thomas.lendacky, f.fainelli, ariel.elior, michael.chan,
santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz,
Manish.Chopra
In-Reply-To: <20181121025132.14305-1-pablo@netfilter.org>
Update this driver to use the flow_rule infrastructure, hence we can use
the same code to populate hardware IR from ethtool_rx_flow and the
cls_flower interfaces.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v3: adapt it to use new ethtool_rx_flow_rule_alloc()
drivers/net/dsa/bcm_sf2_cfp.c | 109 +++++++++++++++++++++++++++---------------
1 file changed, 71 insertions(+), 38 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2_cfp.c b/drivers/net/dsa/bcm_sf2_cfp.c
index e14663ab6dbc..3bdc65fe8408 100644
--- a/drivers/net/dsa/bcm_sf2_cfp.c
+++ b/drivers/net/dsa/bcm_sf2_cfp.c
@@ -16,6 +16,7 @@
#include <linux/netdevice.h>
#include <net/dsa.h>
#include <linux/bitmap.h>
+#include <net/flow_offload.h>
#include "bcm_sf2.h"
#include "bcm_sf2_regs.h"
@@ -257,7 +258,8 @@ static int bcm_sf2_cfp_act_pol_set(struct bcm_sf2_priv *priv,
}
static void bcm_sf2_cfp_slice_ipv4(struct bcm_sf2_priv *priv,
- struct ethtool_tcpip4_spec *v4_spec,
+ struct flow_dissector_key_ipv4_addrs *addrs,
+ struct flow_dissector_key_ports *ports,
unsigned int slice_num,
bool mask)
{
@@ -278,7 +280,7 @@ static void bcm_sf2_cfp_slice_ipv4(struct bcm_sf2_priv *priv,
* UDF_n_A6 [23:8]
* UDF_n_A5 [7:0]
*/
- reg = be16_to_cpu(v4_spec->pdst) >> 8;
+ reg = be16_to_cpu(ports->dst) >> 8;
if (mask)
offset = CORE_CFP_MASK_PORT(3);
else
@@ -289,9 +291,9 @@ static void bcm_sf2_cfp_slice_ipv4(struct bcm_sf2_priv *priv,
* UDF_n_A4 [23:8]
* UDF_n_A3 [7:0]
*/
- reg = (be16_to_cpu(v4_spec->pdst) & 0xff) << 24 |
- (u32)be16_to_cpu(v4_spec->psrc) << 8 |
- (be32_to_cpu(v4_spec->ip4dst) & 0x0000ff00) >> 8;
+ reg = (be16_to_cpu(ports->dst) & 0xff) << 24 |
+ (u32)be16_to_cpu(ports->src) << 8 |
+ (be32_to_cpu(addrs->dst) & 0x0000ff00) >> 8;
if (mask)
offset = CORE_CFP_MASK_PORT(2);
else
@@ -302,9 +304,9 @@ static void bcm_sf2_cfp_slice_ipv4(struct bcm_sf2_priv *priv,
* UDF_n_A2 [23:8]
* UDF_n_A1 [7:0]
*/
- reg = (u32)(be32_to_cpu(v4_spec->ip4dst) & 0xff) << 24 |
- (u32)(be32_to_cpu(v4_spec->ip4dst) >> 16) << 8 |
- (be32_to_cpu(v4_spec->ip4src) & 0x0000ff00) >> 8;
+ reg = (u32)(be32_to_cpu(addrs->dst) & 0xff) << 24 |
+ (u32)(be32_to_cpu(addrs->dst) >> 16) << 8 |
+ (be32_to_cpu(addrs->src) & 0x0000ff00) >> 8;
if (mask)
offset = CORE_CFP_MASK_PORT(1);
else
@@ -317,8 +319,8 @@ static void bcm_sf2_cfp_slice_ipv4(struct bcm_sf2_priv *priv,
* Slice ID [3:2]
* Slice valid [1:0]
*/
- reg = (u32)(be32_to_cpu(v4_spec->ip4src) & 0xff) << 24 |
- (u32)(be32_to_cpu(v4_spec->ip4src) >> 16) << 8 |
+ reg = (u32)(be32_to_cpu(addrs->src) & 0xff) << 24 |
+ (u32)(be32_to_cpu(addrs->src) >> 16) << 8 |
SLICE_NUM(slice_num) | SLICE_VALID;
if (mask)
offset = CORE_CFP_MASK_PORT(0);
@@ -332,9 +334,13 @@ static int bcm_sf2_cfp_ipv4_rule_set(struct bcm_sf2_priv *priv, int port,
unsigned int queue_num,
struct ethtool_rx_flow_spec *fs)
{
- struct ethtool_tcpip4_spec *v4_spec, *v4_m_spec;
const struct cfp_udf_layout *layout;
unsigned int slice_num, rule_index;
+ struct ethtool_rx_flow_rule *flow;
+ struct flow_match_ipv4_addrs ipv4;
+ struct flow_match_ports ports;
+ struct flow_match_basic basic;
+ struct flow_match_ip ip;
u8 ip_proto, ip_frag;
u8 num_udf;
u32 reg;
@@ -343,13 +349,9 @@ static int bcm_sf2_cfp_ipv4_rule_set(struct bcm_sf2_priv *priv, int port,
switch (fs->flow_type & ~FLOW_EXT) {
case TCP_V4_FLOW:
ip_proto = IPPROTO_TCP;
- v4_spec = &fs->h_u.tcp_ip4_spec;
- v4_m_spec = &fs->m_u.tcp_ip4_spec;
break;
case UDP_V4_FLOW:
ip_proto = IPPROTO_UDP;
- v4_spec = &fs->h_u.udp_ip4_spec;
- v4_m_spec = &fs->m_u.udp_ip4_spec;
break;
default:
return -EINVAL;
@@ -367,11 +369,22 @@ static int bcm_sf2_cfp_ipv4_rule_set(struct bcm_sf2_priv *priv, int port,
if (rule_index > bcm_sf2_cfp_rule_size(priv))
return -ENOSPC;
+ flow = ethtool_rx_flow_rule_alloc(fs);
+ if (!flow)
+ return -ENOMEM;
+
+ flow_rule_match_ipv4_addrs(flow->rule, &ipv4);
+ flow_rule_match_ports(flow->rule, &ports);
+ flow_rule_match_basic(flow->rule, &basic);
+ flow_rule_match_ip(flow->rule, &ip);
+
layout = &udf_tcpip4_layout;
/* We only use one UDF slice for now */
slice_num = bcm_sf2_get_slice_number(layout, 0);
- if (slice_num == UDF_NUM_SLICES)
- return -EINVAL;
+ if (slice_num == UDF_NUM_SLICES) {
+ ret = -EINVAL;
+ goto out_err_flow_rule;
+ }
num_udf = bcm_sf2_get_num_udf_slices(layout->udfs[slice_num].slices);
@@ -398,9 +411,10 @@ static int bcm_sf2_cfp_ipv4_rule_set(struct bcm_sf2_priv *priv, int port,
* Reserved [1]
* UDF_Valid[8] [0]
*/
- core_writel(priv, v4_spec->tos << IPTOS_SHIFT |
- ip_proto << IPPROTO_SHIFT | ip_frag << IP_FRAG_SHIFT |
- udf_upper_bits(num_udf),
+ core_writel(priv, ip.key->tos << IPTOS_SHIFT |
+ basic.key->n_proto << IPPROTO_SHIFT |
+ ip_frag << IP_FRAG_SHIFT |
+ udf_upper_bits(num_udf),
CORE_CFP_DATA_PORT(6));
/* Mask with the specific layout for IPv4 packets */
@@ -417,8 +431,8 @@ static int bcm_sf2_cfp_ipv4_rule_set(struct bcm_sf2_priv *priv, int port,
core_writel(priv, udf_lower_bits(num_udf) << 24, CORE_CFP_MASK_PORT(5));
/* Program the match and the mask */
- bcm_sf2_cfp_slice_ipv4(priv, v4_spec, slice_num, false);
- bcm_sf2_cfp_slice_ipv4(priv, v4_m_spec, SLICE_NUM_MASK, true);
+ bcm_sf2_cfp_slice_ipv4(priv, ipv4.key, ports.key, slice_num, false);
+ bcm_sf2_cfp_slice_ipv4(priv, ipv4.mask, ports.mask, SLICE_NUM_MASK, true);
/* Insert into TCAM now */
bcm_sf2_cfp_rule_addr_set(priv, rule_index);
@@ -426,14 +440,14 @@ static int bcm_sf2_cfp_ipv4_rule_set(struct bcm_sf2_priv *priv, int port,
ret = bcm_sf2_cfp_op(priv, OP_SEL_WRITE | TCAM_SEL);
if (ret) {
pr_err("TCAM entry at addr %d failed\n", rule_index);
- return ret;
+ goto out_err_flow_rule;
}
/* Insert into Action and policer RAMs now */
ret = bcm_sf2_cfp_act_pol_set(priv, rule_index, port_num,
queue_num, true);
if (ret)
- return ret;
+ goto out_err_flow_rule;
/* Turn on CFP for this rule now */
reg = core_readl(priv, CORE_CFP_CTL_REG);
@@ -446,6 +460,10 @@ static int bcm_sf2_cfp_ipv4_rule_set(struct bcm_sf2_priv *priv, int port,
fs->location = rule_index;
return 0;
+
+out_err_flow_rule:
+ ethtool_rx_flow_rule_free(flow);
+ return ret;
}
static void bcm_sf2_cfp_slice_ipv6(struct bcm_sf2_priv *priv,
@@ -584,6 +602,10 @@ static int bcm_sf2_cfp_ipv6_rule_set(struct bcm_sf2_priv *priv, int port,
struct ethtool_tcpip6_spec *v6_spec, *v6_m_spec;
unsigned int slice_num, rule_index[2];
const struct cfp_udf_layout *layout;
+ struct ethtool_rx_flow_rule *flow;
+ struct flow_match_ipv6_addrs ipv6;
+ struct flow_match_ports ports;
+ struct flow_match_basic basic;
u8 ip_proto, ip_frag;
int ret = 0;
u8 num_udf;
@@ -645,6 +667,15 @@ static int bcm_sf2_cfp_ipv6_rule_set(struct bcm_sf2_priv *priv, int port,
goto out_err;
}
+ flow = ethtool_rx_flow_rule_alloc(fs);
+ if (!flow) {
+ ret = -ENOMEM;
+ goto out_err;
+ }
+ flow_rule_match_ipv6_addrs(flow->rule, &ipv6);
+ flow_rule_match_ports(flow->rule, &ports);
+ flow_rule_match_basic(flow->rule, &basic);
+
/* Apply the UDF layout for this filter */
bcm_sf2_cfp_udf_set(priv, layout, slice_num);
@@ -668,7 +699,7 @@ static int bcm_sf2_cfp_ipv6_rule_set(struct bcm_sf2_priv *priv, int port,
* Reserved [1]
* UDF_Valid[8] [0]
*/
- reg = 1 << L3_FRAMING_SHIFT | ip_proto << IPPROTO_SHIFT |
+ reg = 1 << L3_FRAMING_SHIFT | basic.key->n_proto << IPPROTO_SHIFT |
ip_frag << IP_FRAG_SHIFT | udf_upper_bits(num_udf);
core_writel(priv, reg, CORE_CFP_DATA_PORT(6));
@@ -688,10 +719,10 @@ static int bcm_sf2_cfp_ipv6_rule_set(struct bcm_sf2_priv *priv, int port,
core_writel(priv, udf_lower_bits(num_udf) << 24, CORE_CFP_MASK_PORT(5));
/* Slice the IPv6 source address and port */
- bcm_sf2_cfp_slice_ipv6(priv, v6_spec->ip6src, v6_spec->psrc,
- slice_num, false);
- bcm_sf2_cfp_slice_ipv6(priv, v6_m_spec->ip6src, v6_m_spec->psrc,
- SLICE_NUM_MASK, true);
+ bcm_sf2_cfp_slice_ipv6(priv, ipv6.key->src.in6_u.u6_addr32,
+ ports.key->src, slice_num, false);
+ bcm_sf2_cfp_slice_ipv6(priv, ipv6.mask->src.in6_u.u6_addr32,
+ ports.mask->src, SLICE_NUM_MASK, true);
/* Insert into TCAM now because we need to insert a second rule */
bcm_sf2_cfp_rule_addr_set(priv, rule_index[0]);
@@ -699,20 +730,20 @@ static int bcm_sf2_cfp_ipv6_rule_set(struct bcm_sf2_priv *priv, int port,
ret = bcm_sf2_cfp_op(priv, OP_SEL_WRITE | TCAM_SEL);
if (ret) {
pr_err("TCAM entry at addr %d failed\n", rule_index[0]);
- goto out_err;
+ goto out_err_flow_rule;
}
/* Insert into Action and policer RAMs now */
ret = bcm_sf2_cfp_act_pol_set(priv, rule_index[0], port_num,
queue_num, false);
if (ret)
- goto out_err;
+ goto out_err_flow_rule;
/* Now deal with the second slice to chain this rule */
slice_num = bcm_sf2_get_slice_number(layout, slice_num + 1);
if (slice_num == UDF_NUM_SLICES) {
ret = -EINVAL;
- goto out_err;
+ goto out_err_flow_rule;
}
num_udf = bcm_sf2_get_num_udf_slices(layout->udfs[slice_num].slices);
@@ -748,10 +779,10 @@ static int bcm_sf2_cfp_ipv6_rule_set(struct bcm_sf2_priv *priv, int port,
/* Mask all */
core_writel(priv, 0, CORE_CFP_MASK_PORT(5));
- bcm_sf2_cfp_slice_ipv6(priv, v6_spec->ip6dst, v6_spec->pdst, slice_num,
- false);
- bcm_sf2_cfp_slice_ipv6(priv, v6_m_spec->ip6dst, v6_m_spec->pdst,
- SLICE_NUM_MASK, true);
+ bcm_sf2_cfp_slice_ipv6(priv, ipv6.key->dst.in6_u.u6_addr32,
+ ports.key->dst, slice_num, false);
+ bcm_sf2_cfp_slice_ipv6(priv, ipv6.mask->dst.in6_u.u6_addr32,
+ ports.key->dst, SLICE_NUM_MASK, true);
/* Insert into TCAM now */
bcm_sf2_cfp_rule_addr_set(priv, rule_index[1]);
@@ -759,7 +790,7 @@ static int bcm_sf2_cfp_ipv6_rule_set(struct bcm_sf2_priv *priv, int port,
ret = bcm_sf2_cfp_op(priv, OP_SEL_WRITE | TCAM_SEL);
if (ret) {
pr_err("TCAM entry at addr %d failed\n", rule_index[1]);
- goto out_err;
+ goto out_err_flow_rule;
}
/* Insert into Action and policer RAMs now, set chain ID to
@@ -768,7 +799,7 @@ static int bcm_sf2_cfp_ipv6_rule_set(struct bcm_sf2_priv *priv, int port,
ret = bcm_sf2_cfp_act_pol_set(priv, rule_index[1], port_num,
queue_num, true);
if (ret)
- goto out_err;
+ goto out_err_flow_rule;
/* Turn on CFP for this rule now */
reg = core_readl(priv, CORE_CFP_CTL_REG);
@@ -784,6 +815,8 @@ static int bcm_sf2_cfp_ipv6_rule_set(struct bcm_sf2_priv *priv, int port,
return ret;
+out_err_flow_rule:
+ ethtool_rx_flow_rule_free(flow);
out_err:
clear_bit(rule_index[1], priv->cfp.used);
return ret;
--
2.11.0
^ permalink raw reply related
* [PATCH net-next,v3 09/12] flow_dissector: add basic ethtool_rx_flow_spec to flow_rule structure translator
From: Pablo Neira Ayuso @ 2018-11-21 2:51 UTC (permalink / raw)
To: netdev
Cc: davem, thomas.lendacky, f.fainelli, ariel.elior, michael.chan,
santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz,
Manish.Chopra
In-Reply-To: <20181121025132.14305-1-pablo@netfilter.org>
This patch adds a function to translate the ethtool_rx_flow_spec
structure to the flow_rule representation.
This allows us to reuse code from the driver side given that both flower
and ethtool_rx_flow interfaces use the same representation.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v3: Suggested by Jiri Pirko:
- Add struct ethtool_rx_flow_rule, keep placeholder to private
dissector information.
Reported by Manish Chopra:
- Fix incorrect dissector user_keys flags.
include/linux/ethtool.h | 10 +++
net/core/ethtool.c | 189 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 199 insertions(+)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index afd9596ce636..99849e0858b2 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -400,4 +400,14 @@ struct ethtool_ops {
void (*get_ethtool_phy_stats)(struct net_device *,
struct ethtool_stats *, u64 *);
};
+
+struct ethtool_rx_flow_rule {
+ struct flow_rule *rule;
+ unsigned long priv[0];
+};
+
+struct ethtool_rx_flow_rule *
+ethtool_rx_flow_rule_alloc(const struct ethtool_rx_flow_spec *fs);
+void ethtool_rx_flow_rule_free(struct ethtool_rx_flow_rule *rule);
+
#endif /* _LINUX_ETHTOOL_H */
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index d05402868575..e679d6478371 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -28,6 +28,7 @@
#include <linux/sched/signal.h>
#include <linux/net.h>
#include <net/xdp_sock.h>
+#include <net/flow_offload.h>
/*
* Some useful ethtool_ops methods that're device independent.
@@ -2808,3 +2809,191 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
return rc;
}
+
+struct ethtool_rx_flow_key {
+ struct flow_dissector_key_basic basic;
+ union {
+ struct flow_dissector_key_ipv4_addrs ipv4;
+ struct flow_dissector_key_ipv6_addrs ipv6;
+ };
+ struct flow_dissector_key_ports tp;
+ struct flow_dissector_key_ip ip;
+} __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
+
+struct ethtool_rx_flow_match {
+ struct flow_dissector dissector;
+ struct ethtool_rx_flow_key key;
+ struct ethtool_rx_flow_key mask;
+};
+
+struct ethtool_rx_flow_rule *
+ethtool_rx_flow_rule_alloc(const struct ethtool_rx_flow_spec *fs)
+{
+ static struct in6_addr zero_addr = {};
+ struct ethtool_rx_flow_match *match;
+ struct ethtool_rx_flow_rule *flow;
+ struct flow_action_entry *act;
+
+ flow = kzalloc(sizeof(struct ethtool_rx_flow_rule) +
+ sizeof(struct ethtool_rx_flow_match), GFP_KERNEL);
+ if (!flow)
+ return NULL;
+
+ /* ethtool_rx supports only one single action per rule. */
+ flow->rule = flow_rule_alloc(1);
+ if (!flow->rule) {
+ kfree(flow);
+ return NULL;
+ }
+
+ match = (struct ethtool_rx_flow_match *)flow->priv;
+ flow->rule->match.dissector = &match->dissector;
+ flow->rule->match.mask = &match->mask;
+ flow->rule->match.key = &match->key;
+
+ match->mask.basic.n_proto = 0xffff;
+
+ switch (fs->flow_type & ~FLOW_EXT) {
+ case TCP_V4_FLOW:
+ case UDP_V4_FLOW: {
+ const struct ethtool_tcpip4_spec *v4_spec, *v4_m_spec;
+
+ match->key.basic.n_proto = htons(ETH_P_IP);
+
+ v4_spec = &fs->h_u.tcp_ip4_spec;
+ v4_m_spec = &fs->m_u.tcp_ip4_spec;
+
+ if (v4_m_spec->ip4src) {
+ match->key.ipv4.src = v4_spec->ip4src;
+ match->mask.ipv4.src = v4_m_spec->ip4src;
+ }
+ if (v4_m_spec->ip4dst) {
+ match->key.ipv4.dst = v4_spec->ip4dst;
+ match->mask.ipv4.dst = v4_m_spec->ip4dst;
+ }
+ if (v4_m_spec->ip4src ||
+ v4_m_spec->ip4dst) {
+ match->dissector.used_keys |=
+ (1 << FLOW_DISSECTOR_KEY_IPV4_ADDRS);
+ match->dissector.offset[FLOW_DISSECTOR_KEY_IPV4_ADDRS] =
+ offsetof(struct ethtool_rx_flow_key, ipv4);
+ }
+ if (v4_m_spec->psrc) {
+ match->key.tp.src = v4_spec->psrc;
+ match->mask.tp.src = v4_m_spec->psrc;
+ }
+ if (v4_m_spec->pdst) {
+ match->key.tp.dst = v4_spec->pdst;
+ match->mask.tp.dst = v4_m_spec->pdst;
+ }
+ if (v4_m_spec->psrc ||
+ v4_m_spec->pdst) {
+ match->dissector.used_keys |=
+ (1 << FLOW_DISSECTOR_KEY_PORTS);
+ match->dissector.offset[FLOW_DISSECTOR_KEY_PORTS] =
+ offsetof(struct ethtool_rx_flow_key, tp);
+ }
+ if (v4_m_spec->tos) {
+ match->key.ip.tos = v4_spec->pdst;
+ match->mask.ip.tos = v4_m_spec->pdst;
+ match->dissector.used_keys |=
+ (1 << FLOW_DISSECTOR_KEY_IP);
+ match->dissector.offset[FLOW_DISSECTOR_KEY_IP] =
+ offsetof(struct ethtool_rx_flow_key, ip);
+ }
+ }
+ break;
+ case TCP_V6_FLOW:
+ case UDP_V6_FLOW: {
+ const struct ethtool_tcpip6_spec *v6_spec, *v6_m_spec;
+
+ match->key.basic.n_proto = htons(ETH_P_IPV6);
+
+ v6_spec = &fs->h_u.tcp_ip6_spec;
+ v6_m_spec = &fs->m_u.tcp_ip6_spec;
+ if (memcmp(v6_m_spec->ip6src, &zero_addr, sizeof(zero_addr))) {
+ memcpy(&match->key.ipv6.src, v6_spec->ip6src,
+ sizeof(match->key.ipv6.src));
+ memcpy(&match->mask.ipv6.src, v6_m_spec->ip6src,
+ sizeof(match->mask.ipv6.src));
+ }
+ if (memcmp(v6_m_spec->ip6dst, &zero_addr, sizeof(zero_addr))) {
+ memcpy(&match->key.ipv6.dst, v6_spec->ip6dst,
+ sizeof(match->key.ipv6.dst));
+ memcpy(&match->mask.ipv6.dst, v6_m_spec->ip6dst,
+ sizeof(match->mask.ipv6.dst));
+ }
+ if (memcmp(v6_m_spec->ip6src, &zero_addr, sizeof(zero_addr)) ||
+ memcmp(v6_m_spec->ip6src, &zero_addr, sizeof(zero_addr))) {
+ match->dissector.used_keys |=
+ (1 << FLOW_DISSECTOR_KEY_IPV6_ADDRS);
+ match->dissector.offset[FLOW_DISSECTOR_KEY_IPV6_ADDRS] =
+ offsetof(struct ethtool_rx_flow_key, ipv6);
+ }
+ if (v6_m_spec->psrc) {
+ match->key.tp.src = v6_spec->psrc;
+ match->mask.tp.src = v6_m_spec->psrc;
+ }
+ if (v6_m_spec->pdst) {
+ match->key.tp.dst = v6_spec->pdst;
+ match->mask.tp.dst = v6_m_spec->pdst;
+ }
+ if (v6_m_spec->psrc ||
+ v6_m_spec->pdst) {
+ match->dissector.used_keys |=
+ (1 << FLOW_DISSECTOR_KEY_PORTS);
+ match->dissector.offset[FLOW_DISSECTOR_KEY_PORTS] =
+ offsetof(struct ethtool_rx_flow_key, tp);
+ }
+ if (v6_m_spec->tclass) {
+ match->key.ip.tos = v6_spec->tclass;
+ match->mask.ip.tos = v6_m_spec->tclass;
+ match->dissector.used_keys |=
+ (1 << FLOW_DISSECTOR_KEY_IP);
+ match->dissector.offset[FLOW_DISSECTOR_KEY_IP] =
+ offsetof(struct ethtool_rx_flow_key, ip);
+ }
+ }
+ break;
+ }
+
+ switch (fs->flow_type & ~FLOW_EXT) {
+ case TCP_V4_FLOW:
+ case TCP_V6_FLOW:
+ match->key.basic.ip_proto = IPPROTO_TCP;
+ break;
+ case UDP_V4_FLOW:
+ case UDP_V6_FLOW:
+ match->key.basic.ip_proto = IPPROTO_UDP;
+ break;
+ }
+ match->mask.basic.ip_proto = 0xff;
+
+ match->dissector.used_keys |= (1 << FLOW_DISSECTOR_KEY_BASIC);
+ match->dissector.offset[FLOW_DISSECTOR_KEY_BASIC] =
+ offsetof(struct ethtool_rx_flow_key, basic);
+
+ act = &flow->rule->action.entries[0];
+ switch (fs->ring_cookie) {
+ case RX_CLS_FLOW_DISC:
+ act->id = FLOW_ACTION_DROP;
+ break;
+ case RX_CLS_FLOW_WAKE:
+ act->id = FLOW_ACTION_WAKE;
+ break;
+ default:
+ act->id = FLOW_ACTION_QUEUE;
+ act->queue_index = fs->ring_cookie;
+ break;
+ }
+
+ return flow;
+}
+EXPORT_SYMBOL(ethtool_rx_flow_rule_alloc);
+
+void ethtool_rx_flow_rule_free(struct ethtool_rx_flow_rule *flow)
+{
+ kfree(flow->rule);
+ kfree(flow);
+}
+EXPORT_SYMBOL(ethtool_rx_flow_rule_free);
--
2.11.0
^ permalink raw reply related
* [PATCH net-next,v3 08/12] flow_dissector: add wake-up-on-lan and queue to flow_action
From: Pablo Neira Ayuso @ 2018-11-21 2:51 UTC (permalink / raw)
To: netdev
Cc: davem, thomas.lendacky, f.fainelli, ariel.elior, michael.chan,
santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz,
Manish.Chopra
In-Reply-To: <20181121025132.14305-1-pablo@netfilter.org>
These actions need to be added to support bcm sf2 features available
through the ethtool_rx_flow interface.
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v3: no changes.
include/net/flow_offload.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index 8c1235fb6ed2..7205abd6cac6 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -116,6 +116,8 @@ enum flow_action_id {
FLOW_ACTION_ADD,
FLOW_ACTION_CSUM,
FLOW_ACTION_MARK,
+ FLOW_ACTION_WAKE,
+ FLOW_ACTION_QUEUE,
};
/* This is mirroring enum pedit_header_type definition for easy mapping between
@@ -150,6 +152,7 @@ struct flow_action_entry {
const struct ip_tunnel_info *tunnel; /* FLOW_ACTION_TUNNEL_ENCAP */
u32 csum_flags; /* FLOW_ACTION_CSUM */
u32 mark; /* FLOW_ACTION_MARK */
+ u32 queue_index; /* FLOW_ACTION_QUEUE */
};
};
--
2.11.0
^ permalink raw reply related
* [PATCH net-next,v3 07/12] cls_flower: don't expose TC actions to drivers anymore
From: Pablo Neira Ayuso @ 2018-11-21 2:51 UTC (permalink / raw)
To: netdev
Cc: davem, thomas.lendacky, f.fainelli, ariel.elior, michael.chan,
santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz,
Manish.Chopra
In-Reply-To: <20181121025132.14305-1-pablo@netfilter.org>
Now that drivers have been converted to use the flow action
infrastructure, remove this field from the tc_cls_flower_offload
structure.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v3: no changes.
include/net/pkt_cls.h | 1 -
net/sched/cls_flower.c | 5 -----
2 files changed, 6 deletions(-)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index a08c06e383db..9bd724bfa860 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -768,7 +768,6 @@ struct tc_cls_flower_offload {
unsigned long cookie;
struct flow_rule *rule;
struct flow_stats stats;
- struct tcf_exts *exts;
u32 classid;
};
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index b88cf29aff7b..ea92228ddc12 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -392,7 +392,6 @@ static int fl_hw_replace_filter(struct tcf_proto *tp,
cls_flower.rule->match.dissector = &f->mask->dissector;
cls_flower.rule->match.mask = &f->mask->key;
cls_flower.rule->match.key = &f->mkey;
- cls_flower.exts = &f->exts;
cls_flower.classid = f->res.classid;
err = tc_setup_flow_action(&cls_flower.rule->action, &f->exts);
@@ -427,7 +426,6 @@ static void fl_hw_update_stats(struct tcf_proto *tp, struct cls_fl_filter *f)
tc_cls_common_offload_init(&cls_flower.common, tp, f->flags, NULL);
cls_flower.command = TC_CLSFLOWER_STATS;
cls_flower.cookie = (unsigned long) f;
- cls_flower.exts = &f->exts;
cls_flower.classid = f->res.classid;
tc_setup_cb_call(block, &f->exts, TC_SETUP_CLSFLOWER,
@@ -1490,7 +1488,6 @@ static int fl_reoffload(struct tcf_proto *tp, bool add, tc_setup_cb_t *cb,
cls_flower.rule->match.dissector = &mask->dissector;
cls_flower.rule->match.mask = &mask->key;
cls_flower.rule->match.key = &f->mkey;
- cls_flower.exts = &f->exts;
err = tc_setup_flow_action(&cls_flower.rule->action,
&f->exts);
@@ -1523,7 +1520,6 @@ static int fl_hw_create_tmplt(struct tcf_chain *chain,
{
struct tc_cls_flower_offload cls_flower = {};
struct tcf_block *block = chain->block;
- struct tcf_exts dummy_exts = { 0, };
cls_flower.rule = flow_rule_alloc(0);
if (!cls_flower.rule)
@@ -1535,7 +1531,6 @@ static int fl_hw_create_tmplt(struct tcf_chain *chain,
cls_flower.rule->match.dissector = &tmplt->dissector;
cls_flower.rule->match.mask = &tmplt->mask;
cls_flower.rule->match.key = &tmplt->dummy_key;
- cls_flower.exts = &dummy_exts;
/* We don't care if driver (any of them) fails to handle this
* call. It serves just as a hint for it.
--
2.11.0
^ permalink raw reply related
* [PATCH net-next,v3 05/12] cls_flower: add statistics retrieval infrastructure and use it
From: Pablo Neira Ayuso @ 2018-11-21 2:51 UTC (permalink / raw)
To: netdev
Cc: davem, thomas.lendacky, f.fainelli, ariel.elior, michael.chan,
santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz,
Manish.Chopra
In-Reply-To: <20181121025132.14305-1-pablo@netfilter.org>
This patch provides the flow_stats structure that acts as container for
tc_cls_flower_offload, then we can use to restore the statistics on the
existing TC actions. Hence, tcf_exts_stats_update() is not used from
drivers.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v3: Suggested by Jiri Pirko:
- Rename to struct flow_stats and to function flow_stats_update().
drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 4 ++--
drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 6 +++---
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 2 +-
drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c | 2 +-
drivers/net/ethernet/netronome/nfp/flower/offload.c | 5 ++---
include/net/flow_offload.h | 14 ++++++++++++++
include/net/pkt_cls.h | 1 +
net/sched/cls_flower.c | 4 ++++
8 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
index b82143d6cdde..09cd75f54eba 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
@@ -1366,8 +1366,8 @@ static int bnxt_tc_get_flow_stats(struct bnxt *bp,
lastused = flow->lastused;
spin_unlock(&flow->stats_lock);
- tcf_exts_stats_update(tc_flow_cmd->exts, stats.bytes, stats.packets,
- lastused);
+ flow_stats_update(&tc_flow_cmd->stats, stats.bytes, stats.packets,
+ lastused);
return 0;
}
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
index 39c5af5dad3d..8a2d66ee1d7b 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
@@ -807,9 +807,9 @@ int cxgb4_tc_flower_stats(struct net_device *dev,
if (ofld_stats->packet_count != packets) {
if (ofld_stats->prev_packet_count != packets)
ofld_stats->last_used = jiffies;
- tcf_exts_stats_update(cls->exts, bytes - ofld_stats->byte_count,
- packets - ofld_stats->packet_count,
- ofld_stats->last_used);
+ flow_stats_update(&cls->stats, bytes - ofld_stats->byte_count,
+ packets - ofld_stats->packet_count,
+ ofld_stats->last_used);
ofld_stats->packet_count = packets;
ofld_stats->byte_count = bytes;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 2645e5d1e790..ad53214e0ee5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -3224,7 +3224,7 @@ int mlx5e_stats_flower(struct mlx5e_priv *priv,
mlx5_fc_query_cached(counter, &bytes, &packets, &lastuse);
- tcf_exts_stats_update(f->exts, bytes, packets, lastuse);
+ flow_stats_update(&f->stats, bytes, packets, lastuse);
return 0;
}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
index e6c4c672b1ca..60900e53243b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
@@ -460,7 +460,7 @@ int mlxsw_sp_flower_stats(struct mlxsw_sp *mlxsw_sp,
if (err)
goto err_rule_get_stats;
- tcf_exts_stats_update(f->exts, bytes, packets, lastuse);
+ flow_stats_update(&f->stats, bytes, packets, lastuse);
mlxsw_sp_acl_ruleset_put(mlxsw_sp, ruleset);
return 0;
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 708331234908..524b9ae1a639 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -532,9 +532,8 @@ nfp_flower_get_stats(struct nfp_app *app, struct net_device *netdev,
ctx_id = be32_to_cpu(nfp_flow->meta.host_ctx_id);
spin_lock_bh(&priv->stats_lock);
- tcf_exts_stats_update(flow->exts, priv->stats[ctx_id].bytes,
- priv->stats[ctx_id].pkts,
- priv->stats[ctx_id].used);
+ flow_stats_update(&flow->stats, priv->stats[ctx_id].bytes,
+ priv->stats[ctx_id].pkts, priv->stats[ctx_id].used);
priv->stats[ctx_id].pkts = 0;
priv->stats[ctx_id].bytes = 0;
diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index 48aa47ba5561..8c1235fb6ed2 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -179,4 +179,18 @@ static inline bool flow_rule_match_key(const struct flow_rule *rule,
return dissector_uses_key(rule->match.dissector, key);
}
+struct flow_stats {
+ u64 pkts;
+ u64 bytes;
+ u64 lastused;
+};
+
+static inline void flow_stats_update(struct flow_stats *flow_stats,
+ u64 pkts, u64 bytes, u64 lastused)
+{
+ flow_stats->pkts = pkts;
+ flow_stats->bytes = bytes;
+ flow_stats->lastused = lastused;
+}
+
#endif /* _NET_FLOW_OFFLOAD_H */
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index abb035f84321..a08c06e383db 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -767,6 +767,7 @@ struct tc_cls_flower_offload {
enum tc_fl_command command;
unsigned long cookie;
struct flow_rule *rule;
+ struct flow_stats stats;
struct tcf_exts *exts;
u32 classid;
};
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 8898943b8ee6..b88cf29aff7b 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -432,6 +432,10 @@ static void fl_hw_update_stats(struct tcf_proto *tp, struct cls_fl_filter *f)
tc_setup_cb_call(block, &f->exts, TC_SETUP_CLSFLOWER,
&cls_flower, false);
+
+ tcf_exts_stats_update(&f->exts, cls_flower.stats.bytes,
+ cls_flower.stats.pkts,
+ cls_flower.stats.lastused);
}
static bool __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f,
--
2.11.0
^ permalink raw reply related
* [PATCH net-next,v3 04/12] cls_api: add translator to flow_action representation
From: Pablo Neira Ayuso @ 2018-11-21 2:51 UTC (permalink / raw)
To: netdev
Cc: davem, thomas.lendacky, f.fainelli, ariel.elior, michael.chan,
santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz,
Manish.Chopra
In-Reply-To: <20181121025132.14305-1-pablo@netfilter.org>
This patch implements a new function to translate from native TC action
to the new flow_action representation. Moreover, this patch also updates
cls_flower to use this new function.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v3: add tcf_exts_num_actions() and pass it to flow_rule_alloc() to calculate
the size of the array of actions.
include/net/pkt_cls.h | 5 +++
net/sched/cls_api.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++++
net/sched/cls_flower.c | 21 +++++++--
3 files changed, 139 insertions(+), 3 deletions(-)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 359876ee32be..abb035f84321 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -620,6 +620,11 @@ tcf_match_indev(struct sk_buff *skb, int ifindex)
}
#endif /* CONFIG_NET_CLS_IND */
+unsigned int tcf_exts_num_actions(struct tcf_exts *exts);
+
+int tc_setup_flow_action(struct flow_action *flow_action,
+ const struct tcf_exts *exts);
+
int tc_setup_cb_call(struct tcf_block *block, struct tcf_exts *exts,
enum tc_setup_type type, void *type_data, bool err_stop);
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index d92f44ac4c39..6f8b953dabc4 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -31,6 +31,14 @@
#include <net/netlink.h>
#include <net/pkt_sched.h>
#include <net/pkt_cls.h>
+#include <net/tc_act/tc_mirred.h>
+#include <net/tc_act/tc_vlan.h>
+#include <net/tc_act/tc_tunnel_key.h>
+#include <net/tc_act/tc_pedit.h>
+#include <net/tc_act/tc_csum.h>
+#include <net/tc_act/tc_gact.h>
+#include <net/tc_act/tc_skbedit.h>
+#include <net/tc_act/tc_mirred.h>
extern const struct nla_policy rtm_tca_policy[TCA_MAX + 1];
@@ -2567,6 +2575,114 @@ int tc_setup_cb_call(struct tcf_block *block, struct tcf_exts *exts,
}
EXPORT_SYMBOL(tc_setup_cb_call);
+int tc_setup_flow_action(struct flow_action *flow_action,
+ const struct tcf_exts *exts)
+{
+ const struct tc_action *act;
+ int i, j, k;
+
+ if (!exts)
+ return 0;
+
+ j = 0;
+ tcf_exts_for_each_action(i, act, exts) {
+ struct flow_action_entry *key;
+
+ key = &flow_action->entries[j];
+ if (is_tcf_gact_ok(act)) {
+ key->id = FLOW_ACTION_ACCEPT;
+ } else if (is_tcf_gact_shot(act)) {
+ key->id = FLOW_ACTION_DROP;
+ } else if (is_tcf_gact_trap(act)) {
+ key->id = FLOW_ACTION_TRAP;
+ } else if (is_tcf_gact_goto_chain(act)) {
+ key->id = FLOW_ACTION_GOTO;
+ key->chain_index = tcf_gact_goto_chain_index(act);
+ } else if (is_tcf_mirred_egress_redirect(act)) {
+ key->id = FLOW_ACTION_REDIRECT;
+ key->dev = tcf_mirred_dev(act);
+ } else if (is_tcf_mirred_egress_mirror(act)) {
+ key->id = FLOW_ACTION_MIRRED;
+ key->dev = tcf_mirred_dev(act);
+ } else if (is_tcf_vlan(act)) {
+ switch (tcf_vlan_action(act)) {
+ case TCA_VLAN_ACT_PUSH:
+ key->id = FLOW_ACTION_VLAN_PUSH;
+ key->vlan.vid = tcf_vlan_push_vid(act);
+ key->vlan.proto = tcf_vlan_push_proto(act);
+ key->vlan.prio = tcf_vlan_push_prio(act);
+ break;
+ case TCA_VLAN_ACT_POP:
+ key->id = FLOW_ACTION_VLAN_POP;
+ break;
+ case TCA_VLAN_ACT_MODIFY:
+ key->id = FLOW_ACTION_VLAN_MANGLE;
+ key->vlan.vid = tcf_vlan_push_vid(act);
+ key->vlan.proto = tcf_vlan_push_proto(act);
+ key->vlan.prio = tcf_vlan_push_prio(act);
+ break;
+ default:
+ goto err_out;
+ }
+ } else if (is_tcf_tunnel_set(act)) {
+ key->id = FLOW_ACTION_TUNNEL_ENCAP;
+ key->tunnel = tcf_tunnel_info(act);
+ } else if (is_tcf_tunnel_release(act)) {
+ key->id = FLOW_ACTION_TUNNEL_DECAP;
+ key->tunnel = tcf_tunnel_info(act);
+ } else if (is_tcf_pedit(act)) {
+ for (k = 0; k < tcf_pedit_nkeys(act); k++) {
+ switch (tcf_pedit_cmd(act, k)) {
+ case TCA_PEDIT_KEY_EX_CMD_SET:
+ key->id = FLOW_ACTION_MANGLE;
+ break;
+ case TCA_PEDIT_KEY_EX_CMD_ADD:
+ key->id = FLOW_ACTION_ADD;
+ break;
+ default:
+ goto err_out;
+ }
+ key->mangle.htype = tcf_pedit_htype(act, k);
+ key->mangle.mask = tcf_pedit_mask(act, k);
+ key->mangle.val = tcf_pedit_val(act, k);
+ key->mangle.offset = tcf_pedit_offset(act, k);
+ key = &flow_action->entries[++j];
+ }
+ } else if (is_tcf_csum(act)) {
+ key->id = FLOW_ACTION_CSUM;
+ key->csum_flags = tcf_csum_update_flags(act);
+ } else if (is_tcf_skbedit_mark(act)) {
+ key->id = FLOW_ACTION_MARK;
+ key->mark = tcf_skbedit_mark(act);
+ } else {
+ goto err_out;
+ }
+
+ if (!is_tcf_pedit(act))
+ j++;
+ }
+ return 0;
+err_out:
+ return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL(tc_setup_flow_action);
+
+unsigned int tcf_exts_num_actions(struct tcf_exts *exts)
+{
+ unsigned int num_acts = 0;
+ struct tc_action *act;
+ int i;
+
+ tcf_exts_for_each_action(i, act, exts) {
+ if (is_tcf_pedit(act))
+ num_acts += tcf_pedit_nkeys(act);
+ else
+ num_acts++;
+ }
+ return num_acts;
+}
+EXPORT_SYMBOL(tcf_exts_num_actions);
+
static __net_init int tcf_net_init(struct net *net)
{
struct tcf_net *tn = net_generic(net, tcf_net_id);
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index d2971fbfc3d9..8898943b8ee6 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -382,7 +382,7 @@ static int fl_hw_replace_filter(struct tcf_proto *tp,
bool skip_sw = tc_skip_sw(f->flags);
int err;
- cls_flower.rule = flow_rule_alloc();
+ cls_flower.rule = flow_rule_alloc(tcf_exts_num_actions(&f->exts));
if (!cls_flower.rule)
return -ENOMEM;
@@ -395,6 +395,12 @@ static int fl_hw_replace_filter(struct tcf_proto *tp,
cls_flower.exts = &f->exts;
cls_flower.classid = f->res.classid;
+ err = tc_setup_flow_action(&cls_flower.rule->action, &f->exts);
+ if (err) {
+ kfree(cls_flower.rule);
+ return err;
+ }
+
err = tc_setup_cb_call(block, &f->exts, TC_SETUP_CLSFLOWER,
&cls_flower, skip_sw);
kfree(cls_flower.rule);
@@ -1467,7 +1473,8 @@ static int fl_reoffload(struct tcf_proto *tp, bool add, tc_setup_cb_t *cb,
if (tc_skip_hw(f->flags))
continue;
- cls_flower.rule = flow_rule_alloc();
+ cls_flower.rule =
+ flow_rule_alloc(tcf_exts_num_actions(&f->exts));
if (!cls_flower.rule)
return -ENOMEM;
@@ -1480,6 +1487,14 @@ static int fl_reoffload(struct tcf_proto *tp, bool add, tc_setup_cb_t *cb,
cls_flower.rule->match.mask = &mask->key;
cls_flower.rule->match.key = &f->mkey;
cls_flower.exts = &f->exts;
+
+ err = tc_setup_flow_action(&cls_flower.rule->action,
+ &f->exts);
+ if (err) {
+ kfree(cls_flower.rule);
+ return err;
+ }
+
cls_flower.classid = f->res.classid;
err = cb(TC_SETUP_CLSFLOWER, &cls_flower, cb_priv);
@@ -1506,7 +1521,7 @@ static int fl_hw_create_tmplt(struct tcf_chain *chain,
struct tcf_block *block = chain->block;
struct tcf_exts dummy_exts = { 0, };
- cls_flower.rule = flow_rule_alloc();
+ cls_flower.rule = flow_rule_alloc(0);
if (!cls_flower.rule)
return -ENOMEM;
--
2.11.0
^ permalink raw reply related
* [PATCH net-next,v3 02/12] net/mlx5e: support for two independent packet edit actions
From: Pablo Neira Ayuso @ 2018-11-21 2:51 UTC (permalink / raw)
To: netdev
Cc: davem, thomas.lendacky, f.fainelli, ariel.elior, michael.chan,
santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz,
Manish.Chopra
In-Reply-To: <20181121025132.14305-1-pablo@netfilter.org>
This patch adds pedit_headers_action structure to store the result of
parsing tc pedit actions. Then, it calls alloc_tc_pedit_action() to
populate the mlx5e hardware intermediate representation once all actions
have been parsed.
This patch comes in preparation for the new flow_action infrastructure,
where each packet mangling comes in an separated action, ie. not packed
as in tc pedit.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v3: no changes.
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 81 ++++++++++++++++++-------
1 file changed, 59 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 6a22f7f22890..2645e5d1e790 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -1748,6 +1748,12 @@ struct pedit_headers {
struct udphdr udp;
};
+struct pedit_headers_action {
+ struct pedit_headers vals;
+ struct pedit_headers masks;
+ u32 pedits;
+};
+
static int pedit_header_offsets[] = {
[TCA_PEDIT_KEY_EX_HDR_TYPE_ETH] = offsetof(struct pedit_headers, eth),
[TCA_PEDIT_KEY_EX_HDR_TYPE_IP4] = offsetof(struct pedit_headers, ip4),
@@ -1759,16 +1765,15 @@ static int pedit_header_offsets[] = {
#define pedit_header(_ph, _htype) ((void *)(_ph) + pedit_header_offsets[_htype])
static int set_pedit_val(u8 hdr_type, u32 mask, u32 val, u32 offset,
- struct pedit_headers *masks,
- struct pedit_headers *vals)
+ struct pedit_headers_action *hdrs)
{
u32 *curr_pmask, *curr_pval;
if (hdr_type >= __PEDIT_HDR_TYPE_MAX)
goto out_err;
- curr_pmask = (u32 *)(pedit_header(masks, hdr_type) + offset);
- curr_pval = (u32 *)(pedit_header(vals, hdr_type) + offset);
+ curr_pmask = (u32 *)(pedit_header(&hdrs->masks, hdr_type) + offset);
+ curr_pval = (u32 *)(pedit_header(&hdrs->vals, hdr_type) + offset);
if (*curr_pmask & mask) /* disallow acting twice on the same location */
goto out_err;
@@ -1824,8 +1829,7 @@ static struct mlx5_fields fields[] = {
* max from the SW pedit action. On success, it says how many HW actions were
* actually parsed.
*/
-static int offload_pedit_fields(struct pedit_headers *masks,
- struct pedit_headers *vals,
+static int offload_pedit_fields(struct pedit_headers_action *hdrs,
struct mlx5e_tc_flow_parse_attr *parse_attr,
struct netlink_ext_ack *extack)
{
@@ -1840,10 +1844,10 @@ static int offload_pedit_fields(struct pedit_headers *masks,
__be16 mask_be16;
void *action;
- set_masks = &masks[TCA_PEDIT_KEY_EX_CMD_SET];
- add_masks = &masks[TCA_PEDIT_KEY_EX_CMD_ADD];
- set_vals = &vals[TCA_PEDIT_KEY_EX_CMD_SET];
- add_vals = &vals[TCA_PEDIT_KEY_EX_CMD_ADD];
+ set_masks = &hdrs[TCA_PEDIT_KEY_EX_CMD_SET].masks;
+ add_masks = &hdrs[TCA_PEDIT_KEY_EX_CMD_ADD].masks;
+ set_vals = &hdrs[TCA_PEDIT_KEY_EX_CMD_SET].vals;
+ add_vals = &hdrs[TCA_PEDIT_KEY_EX_CMD_ADD].vals;
action_size = MLX5_UN_SZ_BYTES(set_action_in_add_action_in_auto);
action = parse_attr->mod_hdr_actions;
@@ -1939,12 +1943,14 @@ static int offload_pedit_fields(struct pedit_headers *masks,
}
static int alloc_mod_hdr_actions(struct mlx5e_priv *priv,
- const struct tc_action *a, int namespace,
+ struct pedit_headers_action *hdrs,
+ int namespace,
struct mlx5e_tc_flow_parse_attr *parse_attr)
{
int nkeys, action_size, max_actions;
- nkeys = tcf_pedit_nkeys(a);
+ nkeys = hdrs[TCA_PEDIT_KEY_EX_CMD_SET].pedits +
+ hdrs[TCA_PEDIT_KEY_EX_CMD_ADD].pedits;
action_size = MLX5_UN_SZ_BYTES(set_action_in_add_action_in_auto);
if (namespace == MLX5_FLOW_NAMESPACE_FDB) /* FDB offloading */
@@ -1968,18 +1974,15 @@ static const struct pedit_headers zero_masks = {};
static int parse_tc_pedit_action(struct mlx5e_priv *priv,
const struct tc_action *a, int namespace,
struct mlx5e_tc_flow_parse_attr *parse_attr,
+ struct pedit_headers_action *hdrs,
struct netlink_ext_ack *extack)
{
- struct pedit_headers masks[__PEDIT_CMD_MAX], vals[__PEDIT_CMD_MAX], *cmd_masks;
int nkeys, i, err = -EOPNOTSUPP;
u32 mask, val, offset;
u8 cmd, htype;
nkeys = tcf_pedit_nkeys(a);
- memset(masks, 0, sizeof(struct pedit_headers) * __PEDIT_CMD_MAX);
- memset(vals, 0, sizeof(struct pedit_headers) * __PEDIT_CMD_MAX);
-
for (i = 0; i < nkeys; i++) {
htype = tcf_pedit_htype(a, i);
cmd = tcf_pedit_cmd(a, i);
@@ -2000,21 +2003,37 @@ static int parse_tc_pedit_action(struct mlx5e_priv *priv,
val = tcf_pedit_val(a, i);
offset = tcf_pedit_offset(a, i);
- err = set_pedit_val(htype, ~mask, val, offset, &masks[cmd], &vals[cmd]);
+ err = set_pedit_val(htype, ~mask, val, offset, &hdrs[cmd]);
if (err)
goto out_err;
+
+ hdrs[cmd].pedits++;
}
- err = alloc_mod_hdr_actions(priv, a, namespace, parse_attr);
+ return 0;
+out_err:
+ return err;
+}
+
+static int alloc_tc_pedit_action(struct mlx5e_priv *priv, int namespace,
+ struct mlx5e_tc_flow_parse_attr *parse_attr,
+ struct pedit_headers_action *hdrs,
+ struct netlink_ext_ack *extack)
+{
+ struct pedit_headers *cmd_masks;
+ int err;
+ u8 cmd;
+
+ err = alloc_mod_hdr_actions(priv, hdrs, namespace, parse_attr);
if (err)
goto out_err;
- err = offload_pedit_fields(masks, vals, parse_attr, extack);
+ err = offload_pedit_fields(hdrs, parse_attr, extack);
if (err < 0)
goto out_dealloc_parsed_actions;
for (cmd = 0; cmd < __PEDIT_CMD_MAX; cmd++) {
- cmd_masks = &masks[cmd];
+ cmd_masks = &hdrs[cmd].masks;
if (memcmp(cmd_masks, &zero_masks, sizeof(zero_masks))) {
NL_SET_ERR_MSG_MOD(extack,
"attempt to offload an unsupported field");
@@ -2156,6 +2175,7 @@ static int parse_tc_nic_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
struct mlx5e_tc_flow *flow,
struct netlink_ext_ack *extack)
{
+ struct pedit_headers_action hdrs[__PEDIT_CMD_MAX] = {};
struct mlx5_nic_flow_attr *attr = flow->nic_attr;
const struct tc_action *a;
LIST_HEAD(actions);
@@ -2178,7 +2198,7 @@ static int parse_tc_nic_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
if (is_tcf_pedit(a)) {
err = parse_tc_pedit_action(priv, a, MLX5_FLOW_NAMESPACE_KERNEL,
- parse_attr, extack);
+ parse_attr, hdrs, extack);
if (err)
return err;
@@ -2232,6 +2252,14 @@ static int parse_tc_nic_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
return -EINVAL;
}
+ if (hdrs[TCA_PEDIT_KEY_EX_CMD_SET].pedits ||
+ hdrs[TCA_PEDIT_KEY_EX_CMD_ADD].pedits) {
+ err = alloc_tc_pedit_action(priv, MLX5_FLOW_NAMESPACE_KERNEL,
+ parse_attr, hdrs, extack);
+ if (err)
+ return err;
+ }
+
attr->action = action;
if (!actions_match_supported(priv, exts, parse_attr, flow, extack))
return -EOPNOTSUPP;
@@ -2778,6 +2806,7 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
struct mlx5e_tc_flow *flow,
struct netlink_ext_ack *extack)
{
+ struct pedit_headers_action hdrs[__PEDIT_CMD_MAX] = {};
struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
struct mlx5_esw_flow_attr *attr = flow->esw_attr;
struct mlx5e_rep_priv *rpriv = priv->ppriv;
@@ -2803,7 +2832,7 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
if (is_tcf_pedit(a)) {
err = parse_tc_pedit_action(priv, a, MLX5_FLOW_NAMESPACE_FDB,
- parse_attr, extack);
+ parse_attr, hdrs, extack);
if (err)
return err;
@@ -2909,6 +2938,14 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
return -EINVAL;
}
+ if (hdrs[TCA_PEDIT_KEY_EX_CMD_SET].pedits ||
+ hdrs[TCA_PEDIT_KEY_EX_CMD_ADD].pedits) {
+ err = alloc_tc_pedit_action(priv, MLX5_FLOW_NAMESPACE_KERNEL,
+ parse_attr, hdrs, extack);
+ if (err)
+ return err;
+ }
+
attr->action = action;
if (!actions_match_supported(priv, exts, parse_attr, flow, extack))
return -EOPNOTSUPP;
--
2.11.0
^ permalink raw reply related
* [PATCH net-next,v3 03/12] flow_dissector: add flow action infrastructure
From: Pablo Neira Ayuso @ 2018-11-21 2:51 UTC (permalink / raw)
To: netdev
Cc: davem, thomas.lendacky, f.fainelli, ariel.elior, michael.chan,
santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz,
Manish.Chopra
In-Reply-To: <20181121025132.14305-1-pablo@netfilter.org>
This new infrastructure defines the nic actions that you can perform
from existing network drivers. This infrastructure allows us to avoid a
direct dependency with the native software TC action representation.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v3: Suggested by Jiri Pirko:
- Remove _key postfix and _KEY_ infix in flow_action definitions.
- Use enum flow_action_mangle_base for consistency.
- Rename key field to entries and num_keys to num_entries.
- Rename struct flow_action_key to flow_action_entry.
- Use placeholder in struct flow_action to store array of actions
from flow_rule_alloc().
include/net/flow_offload.h | 69 +++++++++++++++++++++++++++++++++++++++++++++-
net/core/flow_offload.c | 14 ++++++++--
2 files changed, 80 insertions(+), 3 deletions(-)
diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index 461c66595763..48aa47ba5561 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -100,11 +100,78 @@ void flow_rule_match_enc_keyid(const struct flow_rule *rule,
void flow_rule_match_enc_opts(const struct flow_rule *rule,
struct flow_match_enc_opts *out);
+enum flow_action_id {
+ FLOW_ACTION_ACCEPT = 0,
+ FLOW_ACTION_DROP,
+ FLOW_ACTION_TRAP,
+ FLOW_ACTION_GOTO,
+ FLOW_ACTION_REDIRECT,
+ FLOW_ACTION_MIRRED,
+ FLOW_ACTION_VLAN_PUSH,
+ FLOW_ACTION_VLAN_POP,
+ FLOW_ACTION_VLAN_MANGLE,
+ FLOW_ACTION_TUNNEL_ENCAP,
+ FLOW_ACTION_TUNNEL_DECAP,
+ FLOW_ACTION_MANGLE,
+ FLOW_ACTION_ADD,
+ FLOW_ACTION_CSUM,
+ FLOW_ACTION_MARK,
+};
+
+/* This is mirroring enum pedit_header_type definition for easy mapping between
+ * tc pedit action. Legacy TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK is mapped to
+ * FLOW_ACT_MANGLE_UNSPEC, which is supported by no driver.
+ */
+enum flow_action_mangle_base {
+ FLOW_ACT_MANGLE_UNSPEC = 0,
+ FLOW_ACT_MANGLE_HDR_TYPE_ETH,
+ FLOW_ACT_MANGLE_HDR_TYPE_IP4,
+ FLOW_ACT_MANGLE_HDR_TYPE_IP6,
+ FLOW_ACT_MANGLE_HDR_TYPE_TCP,
+ FLOW_ACT_MANGLE_HDR_TYPE_UDP,
+};
+
+struct flow_action_entry {
+ enum flow_action_id id;
+ union {
+ u32 chain_index; /* FLOW_ACTION_GOTO */
+ struct net_device *dev; /* FLOW_ACTION_REDIRECT */
+ struct { /* FLOW_ACTION_VLAN */
+ u16 vid;
+ __be16 proto;
+ u8 prio;
+ } vlan;
+ struct { /* FLOW_ACTION_PACKET_EDIT */
+ enum flow_action_mangle_base htype;
+ u32 offset;
+ u32 mask;
+ u32 val;
+ } mangle;
+ const struct ip_tunnel_info *tunnel; /* FLOW_ACTION_TUNNEL_ENCAP */
+ u32 csum_flags; /* FLOW_ACTION_CSUM */
+ u32 mark; /* FLOW_ACTION_MARK */
+ };
+};
+
+struct flow_action {
+ unsigned int num_entries;
+ struct flow_action_entry entries[0];
+};
+
+static inline bool flow_action_has_entries(const struct flow_action *action)
+{
+ return action->num_entries;
+}
+
+#define flow_action_for_each(__i, __act, __actions) \
+ for (__i = 0, __act = &(__actions)->entries[0]; __i < (__actions)->num_entries; __act = &(__actions)->entries[++__i])
+
struct flow_rule {
struct flow_match match;
+ struct flow_action action;
};
-struct flow_rule *flow_rule_alloc(void);
+struct flow_rule *flow_rule_alloc(unsigned int num_actions);
static inline bool flow_rule_match_key(const struct flow_rule *rule,
enum flow_dissector_key_id key)
diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
index 2fbf6903d2f6..c3a00eac4804 100644
--- a/net/core/flow_offload.c
+++ b/net/core/flow_offload.c
@@ -3,9 +3,19 @@
#include <linux/slab.h>
#include <net/flow_offload.h>
-struct flow_rule *flow_rule_alloc(void)
+struct flow_rule *flow_rule_alloc(unsigned int num_actions)
{
- return kzalloc(sizeof(struct flow_rule), GFP_KERNEL);
+ struct flow_rule *rule;
+
+ rule = kzalloc(sizeof(struct flow_rule) +
+ sizeof(struct flow_action_entry) * num_actions,
+ GFP_KERNEL);
+ if (!rule)
+ return NULL;
+
+ rule->action.num_entries = num_actions;
+
+ return rule;
}
EXPORT_SYMBOL(flow_rule_alloc);
--
2.11.0
^ permalink raw reply related
* [PATCH net-next,v3 00/12] add flow_rule infrastructure
From: Pablo Neira Ayuso @ 2018-11-21 2:51 UTC (permalink / raw)
To: netdev
Cc: davem, thomas.lendacky, f.fainelli, ariel.elior, michael.chan,
santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz,
Manish.Chopra
Hi,
This patchset is the third iteration [1] [2] [3] to introduce a kernel
intermediate (IR) to express ACL hardware offloads.
This round addresses feedback from Jiri Pirko:
* Add net/core/flow_offload.c and include/net/flow_offload.h.
* Add flow_rule_alloc() helper function.
* Remove _key postfix and _KEY_ infix in flow_action definitions.
* Use enum flow_action_mangle_base for consistency.
* Rename key field to entries and num_keys to num_entries.
* Rename struct flow_action_key to flow_action_entry.
* Use placeholder in struct flow_action to store array of actions
from flow_rule_alloc().
* Add tcf_exts_num_actions() and pass it to flow_rule_alloc() to
calculate the size of the array of actions.
* Rename to struct flow_stats and to function flow_stats_update().
* Add struct ethtool_rx_flow_rule, keep placeholder to private
dissector information.
* Pass struct flow_rule *rule to all parser functions in qlogic/qede
driver.
This also fixes a bug reported by Manish Chopra, in the ethtool_rx_spec
to flow_rule translator.
Making all these changes have been an exercise to review the existing
infrastructure, to understand what has been done and to propose
improvements to the _great work_ that core drivers developers have done
so far to introduce HW offloads through the existing frontend APIs. I
still have more feedback and technical ideas that I'm very much looking
forward to discuss with them in the future.
Main goal of this patchset is to avoid code duplication for driver
developers. There are no netfilter changes coming in this batch.
I would like to explore Netfilter hardware offloads in the future.
Thanks a lot for reviewing!
[1] https://lwn.net/Articles/766695/
[2] https://marc.info/?l=linux-netdev&m=154233253114506&w=2
[3] https://marc.info/?l=linux-netdev&m=154258780717036&w=2
Pablo Neira Ayuso (12):
flow_dissector: add flow_rule and flow_match structures and use them
net/mlx5e: support for two independent packet edit actions
flow_dissector: add flow action infrastructure
cls_api: add translator to flow_action representation
cls_flower: add statistics retrieval infrastructure and use it
drivers: net: use flow action infrastructure
cls_flower: don't expose TC actions to drivers anymore
flow_dissector: add wake-up-on-lan and queue to flow_action
flow_dissector: add basic ethtool_rx_flow_spec to flow_rule structure translator
dsa: bcm_sf2: use flow_rule infrastructure
qede: place ethtool_rx_flow_spec after code after TC flower codebase
qede: use ethtool_rx_flow_rule() to remove duplicated parser code
drivers/net/dsa/bcm_sf2_cfp.c | 109 +--
drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 252 +++----
.../net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 450 ++++++-------
drivers/net/ethernet/intel/i40e/i40e_main.c | 178 ++---
drivers/net/ethernet/intel/iavf/iavf_main.c | 195 +++---
drivers/net/ethernet/intel/igb/igb_main.c | 64 +-
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 743 ++++++++++-----------
drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c | 2 +-
.../net/ethernet/mellanox/mlxsw/spectrum_flower.c | 258 ++++---
drivers/net/ethernet/netronome/nfp/flower/action.c | 196 +++---
drivers/net/ethernet/netronome/nfp/flower/match.c | 417 ++++++------
.../net/ethernet/netronome/nfp/flower/offload.c | 150 ++---
drivers/net/ethernet/qlogic/qede/qede_filter.c | 560 ++++++----------
include/linux/ethtool.h | 10 +
include/net/flow_offload.h | 199 ++++++
include/net/pkt_cls.h | 18 +-
net/core/Makefile | 2 +-
net/core/ethtool.c | 189 ++++++
net/core/flow_offload.c | 153 +++++
net/sched/cls_api.c | 116 ++++
net/sched/cls_flower.c | 69 +-
21 files changed, 2339 insertions(+), 1991 deletions(-)
create mode 100644 include/net/flow_offload.h
create mode 100644 net/core/flow_offload.c
--
2.11.0
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: libbpf: retry program creation without the name
From: Quentin Monnet @ 2018-11-21 2:49 UTC (permalink / raw)
To: Stanislav Fomichev, Alexei Starovoitov
Cc: Daniel Borkmann, Stanislav Fomichev, netdev, ast, vladum
In-Reply-To: <20181120232614.kgzuoh6aj7bmo36u@mini-arch.hsd1.ca.comcast.net>
2018-11-20 15:26 UTC-0800 ~ Stanislav Fomichev <sdf@fomichev.me>
> On 11/20, Alexei Starovoitov wrote:
>> On Wed, Nov 21, 2018 at 12:18:57AM +0100, Daniel Borkmann wrote:
>>> On 11/21/2018 12:04 AM, Alexei Starovoitov wrote:
>>>> On Tue, Nov 20, 2018 at 01:19:05PM -0800, Stanislav Fomichev wrote:
>>>>> On 11/20, Alexei Starovoitov wrote:
>>>>>> On Mon, Nov 19, 2018 at 04:46:25PM -0800, Stanislav Fomichev wrote:
>>>>>>> [Recent commit 23499442c319 ("bpf: libbpf: retry map creation without
>>>>>>> the name") fixed this issue for maps, let's do the same for programs.]
>>>>>>>
>>>>>>> Since commit 88cda1c9da02 ("bpf: libbpf: Provide basic API support
>>>>>>> to specify BPF obj name"), libbpf unconditionally sets bpf_attr->name
>>>>>>> for programs. Pre v4.14 kernels don't know about programs names and
>>>>>>> return an error about unexpected non-zero data. Retry sys_bpf without
>>>>>>> a program name to cover older kernels.
>>>>>>>
>>>>>>> Signed-off-by: Stanislav Fomichev <sdf@google.com>
>>>>>>> ---
>>>>>>> tools/lib/bpf/bpf.c | 10 ++++++++++
>>>>>>> 1 file changed, 10 insertions(+)
>>>>>>>
>>>>>>> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
>>>>>>> index 961e1b9fc592..cbe9d757c646 100644
>>>>>>> --- a/tools/lib/bpf/bpf.c
>>>>>>> +++ b/tools/lib/bpf/bpf.c
>>>>>>> @@ -212,6 +212,16 @@ int bpf_load_program_xattr(const struct bpf_load_program_attr *load_attr,
>>>>>>> if (fd >= 0 || !log_buf || !log_buf_sz)
>>>>>>> return fd;
>>>>>>>
>>>>>>> + if (fd < 0 && errno == E2BIG && load_attr->name) {
>>>>>>> + /* Retry the same syscall, but without the name.
>>>>>>> + * Pre v4.14 kernels don't support prog names.
>>>>>>> + */
>>>>>>
>>>>>> I'm afraid that will put unnecessary stress on the kernel.
>>>>>> This check needs to be tighter.
>>>>>> Like E2BIG and anything in the log_buf probably means that
>>>>>> E2BIG came from the verifier and nothing to do with prog_name.
>>>>>> Asking kernel to repeat is an unnecessary work.
>>>>>>
>>>>>> In general we need to think beyond this single prog_name field.
>>>>>> There are bunch of other fields in bpf_load_program_xattr() and older kernels
>>>>>> won't support them. Are we going to zero them out one by one
>>>>>> and retry? I don't think that would be practical.
>>>>> I general, we don't want to zero anything out. However,
>>>>> for this particular problem the rationale is the following:
>>>>> In commit 88cda1c9da02 we started unconditionally setting {prog,map}->name
>>>>> from the 'higher' libbpfc layer which breaks users on the older kernels.
>>>>>
>>>>>> Also libbpf silently ignoring prog_name is not great for debugging.
>>>>>> A warning is needed.
>>>>>> But it cannot be done out of lib/bpf/bpf.c, since it's a set of syscall
>>>>>> wrappers.
>>>>>> Imo such "old kernel -> lets retry" feature should probably be done
>>>>>> at lib/bpf/libbpf.c level. inside load_program().
>>>>> For maps bpftools calls bpf_create_map_xattr directly, that's why
>>>>> for maps I did the retry on the lower level (and why for programs I initially
>>>>> thought about doing the same). However, in this case maybe asking
>>>>> user to omit 'name' argument might be a better option.
>>>>>
>>>>> For program names, I agree, we might think about doing it on the higher
>>>>> level (although I'm not sure whether we want to have different API
>>>>> expectations, i.e. bpf_create_map_xattr ignoring the name and
>>>>> bpf_load_program_xattr not ignoring the name).
>>>>>
>>>>> So given that rationale above, what do you think is the best way to
>>>>> move forward?
>>>>> 1. Same patch, but tighten the retry check inside bpf_load_program_xattr ?
>>>>> 2. Move this retry logic into load_program and have different handling
>>>>> for bpf_create_map_xattr vs bpf_load_program_xattr ?
>>>>> 3. Do 2 and move the retry check for maps from bpf_create_map_xattr
>>>>> into bpf_object__create_maps ?
>>>>>
>>>>> (I'm slightly leaning towards #3)
>>>>
>>>> me too. I think it's cleaner for maps to do it in
>>>> bpf_object__create_maps().
>>>> Originally bpf.c was envisioned to be a thin layer on top of bpf syscall.
>>>> Whereas 'smart bits' would go into libbpf.c
>>>
>>> Can't we create in bpf_object__load() a small helper bpf_object__probe_caps()
>>> which would figure this out _once_ upon start with a few things to probe for
>>> availability in the underlying kernel for maps and programs? E.g. programs
>>> it could try to inject a tiny 'r0 = 0; exit' snippet where we figure out
>>> things like prog name support etc. Given underlying kernel doesn't change, we
>>> would only try this once and it doesn't require fallback every time.
>>
>> +1. great idea!
> Sounds good, let me try to do it.
>
> It sounds more like a recent LPC proposal/idea to have some sys_bpf option
> to query BPF features. This new bpf_object__probe_caps can probably query
> that in the future if we eventually add support for it.
>
Hi,
LPC proposal indeed. I've been working on implementing this kind of
probes in bpftool. I don't probe name support for now (but I can
certainly add it), but I detect supported program types, map types,
header functions, and a couple of other parameters. The idea (initially
from Daniel) was to dump "#define" declarations that could later be
included in a header file and used for a BPF project (or alternatively,
JSON output).
I felt like bpftool was maybe a better place to do it, as the set of
probes may grow large (all types, helpers, etc). It might have
consequences on the running system: for example, if I don't raise the
rlimit in bpftool before starting the probes, a good half of them fail
because of consecutive program creation and reclaim delay for locked
memory usage.
So should we start adding probes to libbpf and should I move mine to the
lib as well, or should the one in the v3 of this series be directed to
something like bpftool instead?
Thanks,
Quentin
^ permalink raw reply
* [PATCH v2 2/2] arm64/bpf: don't allocate BPF JIT programs in module memory
From: Ard Biesheuvel @ 2018-11-21 13:17 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Ard Biesheuvel, Daniel Borkmann, Alexei Starovoitov,
Rick Edgecombe, Eric Dumazet, Jann Horn, Kees Cook, Jessica Yu,
Arnd Bergmann, Catalin Marinas, Will Deacon, Mark Rutland,
David S. Miller, linux-kernel, netdev
In-Reply-To: <20181121131733.14910-1-ard.biesheuvel@linaro.org>
The arm64 module region is a 128 MB region that is kept close to
the core kernel, in order to ensure that relative branches are
always in range. So using the same region for programs that do
not have this restriction is wasteful, and preferably avoided.
Now that the core BPF JIT code permits the alloc/free routines to
be overridden, implement them by simple vmalloc_exec()/vfree()
calls, which can be served from anywere. This also solves an
issue under KASAN, where shadow memory is needlessly allocated for
all BPF programs (which don't require KASAN shadow pages since
they are not KASAN instrumented)
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/net/bpf_jit_comp.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index a6fdaea07c63..f91b7c157841 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -940,3 +940,13 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
tmp : orig_prog);
return prog;
}
+
+void *bpf_jit_alloc_exec(unsigned long size)
+{
+ return vmalloc_exec(size);
+}
+
+void bpf_jit_free_exec(const void *addr)
+{
+ return vfree(size);
+}
--
2.17.1
^ permalink raw reply related
* [PATCH v2 1/2] bpf: add __weak hook for allocating executable memory
From: Ard Biesheuvel @ 2018-11-21 13:17 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Ard Biesheuvel, Daniel Borkmann, Alexei Starovoitov,
Rick Edgecombe, Eric Dumazet, Jann Horn, Kees Cook, Jessica Yu,
Arnd Bergmann, Catalin Marinas, Will Deacon, Mark Rutland,
David S. Miller, linux-kernel, netdev
In-Reply-To: <20181121131733.14910-1-ard.biesheuvel@linaro.org>
By default, BPF uses module_alloc() to allocate executable memory,
but this is not necessary on all arches and potentially undesirable
on some of them.
So break out the module_alloc() and module_memfree() calls into __weak
functions to allow them to be overridden in arch code.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
kernel/bpf/core.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 1a796e0799ec..f28d8a5eb6b8 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -609,6 +609,16 @@ static void bpf_jit_uncharge_modmem(u32 pages)
atomic_long_sub(pages, &bpf_jit_current);
}
+void *__weak bpf_jit_alloc_exec(unsigned long size)
+{
+ return module_alloc(size);
+}
+
+void __weak bpf_jit_free_exec(const void *addr)
+{
+ return module_memfree(size);
+}
+
struct bpf_binary_header *
bpf_jit_binary_alloc(unsigned int proglen, u8 **image_ptr,
unsigned int alignment,
@@ -626,7 +636,7 @@ bpf_jit_binary_alloc(unsigned int proglen, u8 **image_ptr,
if (bpf_jit_charge_modmem(pages))
return NULL;
- hdr = module_alloc(size);
+ hdr = bpf_jit_alloc_exec(size);
if (!hdr) {
bpf_jit_uncharge_modmem(pages);
return NULL;
@@ -650,7 +660,7 @@ void bpf_jit_binary_free(struct bpf_binary_header *hdr)
{
u32 pages = hdr->pages;
- module_memfree(hdr);
+ bpf_jit_free_exec(hdr);
bpf_jit_uncharge_modmem(pages);
}
--
2.17.1
^ permalink raw reply related
* [PATCH v2 0/2] bpf: permit JIT allocations to be served outside the module region
From: Ard Biesheuvel @ 2018-11-21 13:17 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Ard Biesheuvel, Daniel Borkmann, Alexei Starovoitov,
Rick Edgecombe, Eric Dumazet, Jann Horn, Kees Cook, Jessica Yu,
Arnd Bergmann, Catalin Marinas, Will Deacon, Mark Rutland,
David S. Miller, linux-kernel, netdev
On arm64, modules are allocated from a 128 MB window which is close to
the core kernel, so that relative direct branches are guaranteed to be
in range (except in some KASLR configurations). Also, module_alloc()
is in charge of allocating KASAN shadow memory when running with KASAN
enabled.
This means that the way BPF reuses module_alloc()/module_memfree() is
undesirable on arm64 (and potentially other architectures as well),
and so this series refactors BPF's use of those functions to permit
architectures to change this behavior.
Patch #1 breaks out the module_alloc() and module_memfree() calls into
__weak functions so they can be overridden.
Patch #4 implements the new alloc/free overrides for arm64
Changes since v1:
- Drop misguided attempt to 'fix' and refactor the free path. Instead,
just add another __weak wrapper for the invocation of module_memfree()
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Jann Horn <jannh@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: netdev@vger.kernel.org
Ard Biesheuvel (2):
bpf: add __weak hook for allocating executable memory
arm64/bpf: don't allocate BPF JIT programs in module memory
arch/arm64/net/bpf_jit_comp.c | 10 ++++++++++
kernel/bpf/core.c | 14 ++++++++++++--
2 files changed, 22 insertions(+), 2 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH bpf-next v2] libbpf: make sure bpf headers are c++ include-able
From: Alexei Starovoitov @ 2018-11-21 2:42 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: Stanislav Fomichev, netdev, ast, daniel, ys114321
In-Reply-To: <20181121015952.ta7hwo5ugaurlzcd@mini-arch.hsd1.ca.comcast.net>
On Tue, Nov 20, 2018 at 05:59:52PM -0800, Stanislav Fomichev wrote:
> On 11/20, Alexei Starovoitov wrote:
> > On Tue, Nov 20, 2018 at 04:05:55PM -0800, Stanislav Fomichev wrote:
> > > On 11/20, Alexei Starovoitov wrote:
> > > > On Tue, Nov 20, 2018 at 01:37:23PM -0800, Stanislav Fomichev wrote:
> > > > > Wrap headers in extern "C", to turn off C++ mangling.
> > > > > This simplifies including libbpf in c++ and linking against it.
> > > > >
> > > > > v2 changes:
> > > > > * do the same for btf.h
> > > > >
> > > > > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > > > > ---
> > > > > tools/lib/bpf/bpf.h | 9 +++++++++
> > > > > tools/lib/bpf/btf.h | 8 ++++++++
> > > > > tools/lib/bpf/libbpf.h | 9 +++++++++
> > > > > 3 files changed, 26 insertions(+)
> > > > >
> > > > > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> > > > > index 26a51538213c..9ea3aec82d8a 100644
> > > > > --- a/tools/lib/bpf/bpf.h
> > > > > +++ b/tools/lib/bpf/bpf.h
> > > > > @@ -27,6 +27,10 @@
> > > > > #include <stdbool.h>
> > > > > #include <stddef.h>
> > > > >
> > > > > +#ifdef __cplusplus
> > > > > +extern "C" {
> > > > > +#endif
> > > >
> > > > Acked-by: Alexei Starovoitov <ast@kernel.org>
> > > >
> > > > was wondering whether it's possible to make it testable.
> > > > HOSTCXX is available, but I don't see much of the kernel tree
> > > > using it...
> > > By testable you mean compile some dummy c++ main and link against libbpf?
> >
> > yes. something like this.
> > to make sure that it keeps being functional and no one introduces 'int new'
> > in some function argument list by accident.
> I tried something like the patch below, it does seem to work locally (building
> in the same directory, no cross-compile). Who would be the best to
> review that kind of stuff?
>
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index 425b480bda75..4c0e58628aad 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -66,7 +66,7 @@ ifndef VERBOSE
> endif
>
> FEATURE_USER = .libbpf
> -FEATURE_TESTS = libelf libelf-mmap bpf reallocarray
> +FEATURE_TESTS = libelf libelf-mmap bpf reallocarray cxx
> FEATURE_DISPLAY = libelf bpf
>
> INCLUDES = -I. -I$(srctree)/tools/include -I$(srctree)/tools/arch/$(ARCH)/include/uapi -I$(srctree)/tools/include/uapi
> @@ -148,6 +148,10 @@ LIB_FILE := $(addprefix $(OUTPUT),$(LIB_FILE))
>
> CMD_TARGETS = $(LIB_FILE)
>
> +ifeq ($(feature-cxx), 1)
> + CMD_TARGETS += $(OUTPUT)test_libbpf
> +endif
> +
> TARGETS = $(CMD_TARGETS)
>
> all: fixdep all_cmd
> @@ -175,6 +179,9 @@ $(OUTPUT)libbpf.so: $(BPF_IN)
> $(OUTPUT)libbpf.a: $(BPF_IN)
> $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
>
> +$(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a
> + $(QUIET_LINK)$(CXX) $^ -lelf -o $@
looks good to me.
pls include test_libbpf.cpp and resubmit for bpf-next.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: add read/write access to skb->tstamp from tc clsact progs
From: Alexei Starovoitov @ 2018-11-21 2:40 UTC (permalink / raw)
To: Vlad Dumitrescu
Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Eric Dumazet,
Willem de Bruijn
In-Reply-To: <20181121001848.35894-1-vladum@google.com>
On Tue, Nov 20, 2018 at 07:18:48PM -0500, Vlad Dumitrescu wrote:
> This could be used to rate limit egress traffic in concert with a qdisc
> which supports Earliest Departure Time, such as FQ.
>
> Signed-off-by: Vlad Dumitrescu <vladum@google.com>
> ---
> include/uapi/linux/bpf.h | 1 +
> net/core/filter.c | 26 +++++++++++++++++++++
> tools/include/uapi/linux/bpf.h | 1 +
> tools/testing/selftests/bpf/test_verifier.c | 4 ++++
> 4 files changed, 32 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index c1554aa074659..23e2031a43d43 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2468,6 +2468,7 @@ struct __sk_buff {
>
> __u32 data_meta;
> struct bpf_flow_keys *flow_keys;
> + __u64 tstamp;
> };
>
> struct bpf_tunnel_key {
> diff --git a/net/core/filter.c b/net/core/filter.c
> index f6ca38a7d4332..c45155c8e519c 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5573,6 +5573,10 @@ static bool bpf_skb_is_valid_access(int off, int size, enum bpf_access_type type
> if (size != sizeof(struct bpf_flow_keys *))
> return false;
> break;
> + case bpf_ctx_range(struct __sk_buff, tstamp):
> + if (size != sizeof(__u64))
> + return false;
> + break;
> default:
> /* Only narrow read access allowed for now. */
> if (type == BPF_WRITE) {
> @@ -5600,6 +5604,7 @@ static bool sk_filter_is_valid_access(int off, int size,
> case bpf_ctx_range(struct __sk_buff, data_end):
> case bpf_ctx_range(struct __sk_buff, flow_keys):
> case bpf_ctx_range_till(struct __sk_buff, family, local_port):
> + case bpf_ctx_range(struct __sk_buff, tstamp):
> return false;
> }
>
> @@ -5624,6 +5629,7 @@ static bool cg_skb_is_valid_access(int off, int size,
> case bpf_ctx_range(struct __sk_buff, tc_classid):
> case bpf_ctx_range(struct __sk_buff, data_meta):
> case bpf_ctx_range(struct __sk_buff, flow_keys):
> + case bpf_ctx_range(struct __sk_buff, tstamp):
> return false;
looks good to me.
Any particular reason you decided to disable it for cg_skb ?
It seems to me the same EDT approach will work from
cgroup-bpf skb hooks just as well and then we can have neat
way of controlling traffic per-container instead of tc-clsbpf global.
If you're already on cgroup v2 it will save you a lot of classifier
cycles, since you'd be able to group apps by cgroup
instead of relying on ip only.
^ permalink raw reply
* PATCH net : Micrel KSZ8061 PHY link failure after cable connect
From: Onnasch, Alexander (EXT) @ 2018-11-21 13:05 UTC (permalink / raw)
Cc: Onnasch, Alexander (EXT), Andrew Lunn, Florian Fainelli,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
For tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git
With Micrel KSZ8061 PHY, the link may occasionally not come up after
Ethernet cable connect. The vendor's (Microchip, former Micrel) errata
sheet 80000688A.pdf describes the problem and possible workarounds in
detail, see below.
The patch implements workaround 1, which permanently fixes the issue.
DESCRIPTION
Link-up may not occur properly when the Ethernet cable is initially
connected. This issue occurs more commonly when the cable is connected
slowly, but it may occur any time a cable is connected. This issue occurs
in the auto-negotiation circuit, and will not occur if auto-negotiation
is disabled (which requires that the two link partners be set to the
same speed and duplex).
END USER IMPLICATIONS
When this issue occurs, link is not established. Subsequent cable
plug/unplug cycles will not correct the issue.
WORK AROUND
There are four approaches to work around this issue:
1. This issue can be prevented by setting bit 15 in MMD device address 1,
register 2, prior to connecting the cable or prior to setting the
Restart Auto-Negotiation bit in register 0h.The MMD registers are
accessed via the indirect access registers Dh and Eh, or via the Micrel
EthUtil utility as shown here:
• If using the EthUtil utility (usually with a Micrel KSZ8061
Evaluation Board), type the following commands:
> address 1
> mmd 1
> iw 2 b61a
• Alternatively, write the following registers to write to the
indirect MMD register:
Write register Dh, data 0001h
Write register Eh, data 0002h
Write register Dh, data 4001h
Write register Eh, data B61Ah
2. The issue can be avoided by disabling auto-negotiation in the KSZ8061,
either by the strapping option, or by clearing bit 12 in register 0h.
Care must be taken to ensure that the KSZ8061 and the link partner
will link with the same speed and duplex. Note that the KSZ8061
defaults to full-duplex when auto-negotiation is off, but other
devices may default to half-duplex in the event of failed
auto-negotiation.
3. The issue can be avoided by connecting the cable prior to powering-up
or resetting the KSZ8061, and leaving it plugged in thereafter.
4. If the above measures are not taken and the problem occurs, link can
be recovered by setting the Restart Auto-Negotiation bit in
register 0h, or by resetting or power cycling the device. Reset may
be either hardware reset or software reset (register 0h, bit 15).
PLAN
This errata will not be corrected in a future revision.
Signed-off-by: Alexander Onnasch <alexander.onnasch@landisgyr.com>
---
drivers/net/phy/micrel.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 6c45ff6..eb85cf4 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -339,6 +339,17 @@ static int ksz8041_config_aneg(struct phy_device *phydev)
return genphy_config_aneg(phydev);
}
+static int ksz8061_config_init(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_DEVID1, 0xB61A);
+ if (ret)
+ return ret;
+
+ return kszphy_config_init(phydev);
+}
+
static int ksz9021_load_values_from_of(struct phy_device *phydev,
const struct device_node *of_node,
u16 reg,
@@ -938,7 +949,7 @@ static struct phy_driver ksphy_driver[] = {
.phy_id_mask = MICREL_PHY_ID_MASK,
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
- .config_init = kszphy_config_init,
+ .config_init = ksz8061_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
--
2.7.4
^ permalink raw reply related
* [PATCH] net: amd: add of_node_put()
From: Yangtao Li @ 2018-11-21 12:58 UTC (permalink / raw)
To: davem, yuehaibing; +Cc: netdev, linux-kernel, Yangtao Li
of_find_node_by_path() acquires a reference to the node
returned by it and that reference needs to be dropped by its caller.
bl_idle_init() doesn't do that, so fix it.
Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
drivers/net/ethernet/amd/sunlance.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/amd/sunlance.c b/drivers/net/ethernet/amd/sunlance.c
index b4fc0ed5bce8..9d4899826823 100644
--- a/drivers/net/ethernet/amd/sunlance.c
+++ b/drivers/net/ethernet/amd/sunlance.c
@@ -1419,7 +1419,7 @@ static int sparc_lance_probe_one(struct platform_device *op,
prop = of_get_property(nd, "tpe-link-test?", NULL);
if (!prop)
- goto no_link_test;
+ goto node_put;
if (strcmp(prop, "true")) {
printk(KERN_NOTICE "SunLance: warning: overriding option "
@@ -1428,6 +1428,8 @@ static int sparc_lance_probe_one(struct platform_device *op,
"to ecd@skynet.be\n");
auxio_set_lte(AUXIO_LTE_ON);
}
+node_put:
+ of_node_put(nd);
no_link_test:
lp->auto_select = 1;
lp->tpe = 0;
--
2.17.0
^ permalink raw reply related
* RE: [PATCH v1 net] lan743x: fix return value for lan743x_tx_napi_poll
From: Tristram.Ha @ 2018-11-21 2:13 UTC (permalink / raw)
To: Bryan.Whitehead; +Cc: davem, netdev, UNGLinuxDriver, andrew, f.fainelli
In-Reply-To: <27c637a7-a988-1acb-b967-70f5719306a1@gmail.com>
Slightly out of topic I am not sure why NAPI is used on the transmit side.
Originally NAPI was designed to fix the receive interrupt happening on each
receive frame problem, so on transmit side it is to avoid the transmit
done interrupt on each transmit frame? Typically hardware has a way
to trigger transmit done interrupt or not in each transmit frame.
NAPI may have other uses in newer kernels that I am not aware of.
I notice 2 problems in the driver:
1. netif_napi_add is used instead of netif_tx_napi_add.
2. In all other drivers that use netif_tx_napi_add most do not call napi_complete_done.
They all call napi_complete directly and return 0.
freescale/gianfar.c
rocker/rocker_main.c
ti/cpsw.c
virtio_net.c does use napi_complete_done but it also passes 0 as a parameter.
> -----Original Message-----
> From: Florian Fainelli <f.fainelli@gmail.com>
> Sent: Tuesday, November 20, 2018 2:12 PM
> To: Bryan Whitehead - C21958 <Bryan.Whitehead@microchip.com>;
> andrew@lunn.ch
> Cc: davem@davemloft.net; netdev@vger.kernel.org; UNGLinuxDriver
> <UNGLinuxDriver@microchip.com>
> Subject: Re: [PATCH v1 net] lan743x: fix return value for
> lan743x_tx_napi_poll
>
> On 11/20/18 1:39 PM, Bryan.Whitehead@microchip.com wrote:
> >> -----Original Message-----
> >> From: Andrew Lunn <andrew@lunn.ch>
> >> Sent: Tuesday, November 20, 2018 2:31 PM
> >> To: Bryan Whitehead - C21958 <Bryan.Whitehead@microchip.com>
> >> Cc: davem@davemloft.net; netdev@vger.kernel.org; UNGLinuxDriver
> >> <UNGLinuxDriver@microchip.com>
> >> Subject: Re: [PATCH v1 net] lan743x: fix return value for
> >> lan743x_tx_napi_poll
> >>
> >> On Tue, Nov 20, 2018 at 01:26:43PM -0500, Bryan Whitehead wrote:
> >>> It has been noticed that under stress the lan743x driver will
> >>> sometimes hang or cause a kernel panic. It has been noticed that
> >>> returning '0' instead of 'weight' fixes this issue.
> >>>
> >>> fixes: rare kernel panic under heavy traffic load.
> >>> Signed-off-by: Bryan Whitehead <Bryan.Whitehead@microchip.com>
> >>
> >> Hi Bryan
> >>
> >> This sounds like a band aid over something which is broken, not a real fix.
> >>
> >> Can you show us the stack trace from the panic?
> >>
> >> Andrew
> >
> > Andrew,
> >
> > Admittedly, my knowledge of what the kernel is doing behind the scenes is
> limited.
> >
> > But according to documentation found on
> > https://wiki.linuxfoundation.org/networking/napi
> >
> > It states the following
> > "The poll() function may also process TX completions, in which case if it
> processes
> > the entire TX ring then it should count that work as the rest of the budget.
> > Otherwise, TX completions are not counted."
> >
> > So based on that, the original driver was returning the full budget. But I was
> having
> > Issues with it. And the above documentation seems to suggest that I could
> return 0
> > As in "not counted" from above.
> >
> > I tried it, and my lock up issues disappeared.
> >
> > Regarding the kernel panic stack trace. So far its very hard to replicate that
> on the
> > latest kernel. I've seen it more frequently when back porting to older
> kernels such
> > as 4.14, and 4.9. This same fix caused those kernel panics to disappear.
> > Are you interested in seeing a stack dump from older kernels?
> >
> > In the latest kernel the issue manifests as a kernel message which states
> > "[ 945.021101] enp48s0: Budget exhausted after napi rescheduled"
> >
> > I'm not sure what that means. But it does not lock up immediately after
> seeing that
> > Message. But it usually locks up with in a minute of seeing that message.
> >
> > And the sometimes I get the following warning
> > [ 1240.425020] ------------[ cut here ]------------
> > [ 1240.426014] NETDEV WATCHDOG: enp0s25 (e1000e): transmit queue 0
> timed out
> > [ 1240.430027] WARNING: CPU: 0 PID: 0 at net/sched/sch_generic.c:461
> dev_watchdog+0x1ef/0x200
> > [ 1240.430027] Modules linked in: lan743x
> > [ 1240.430027] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G I 4.19.2 #1
> > [ 1240.430027] Hardware name: Hewlett-Packard HP Compaq dc7900
> Convertible Minitower/3032h, BIOS 786G1 v01.16 03/05/2009
> > [ 1240.430027] RIP: 0010:dev_watchdog+0x1ef/0x200
> > [ 1240.430027] Code: 00 48 63 4d e0 eb 93 4c 89 e7 c6 05 68 30 b3 00 01 e8 25
> 3d fd ff 89 d9 48 89 c2 4c 89 e6 48 c7 c7 98 92 48 ab e8 f1 28 87 ff <0f> 0b eb c0
> 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 48 c7 47 08 00
> > [ 1240.430027] RSP: 0018:ffff98490be03e90 EFLAGS: 00010282
> > [ 1240.430027] RAX: 0000000000000000 RBX: 0000000000000000 RCX:
> 0000000000000000
> > [ 1240.497168] RDX: 0000000000040400 RSI: 00000000000000f6 RDI:
> 0000000000000300
> > [ 1240.497168] RBP: ffff984908574440 R08: 0000000000000000 R09:
> 00000000000003a4
> > [ 1240.497168] R10: 0000000000000020 R11: ffffffffabc928ed R12:
> ffff984908574000
> > [ 1240.497168] R13: 0000000000000000 R14: 0000000000000000 R15:
> ffff98490be195b0
> > [ 1240.497168] FS: 0000000000000000(0000) GS:ffff98490be00000(0000)
> knlGS:0000000000000000
> > [ 1240.497168] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 1240.497168] CR2: 00007f31cd4c0000 CR3: 0000000109bca000 CR4:
> 00000000000406f0
> > [ 1240.497168] Call Trace:
> > [ 1240.497168] <IRQ>
> > [ 1240.497168] ? qdisc_reset+0xe0/0xe0
> > [ 1240.497168] call_timer_fn+0x26/0x130
> > [ 1240.497168] run_timer_softirq+0x1cd/0x400
> > [ 1240.497168] ? hpet_interrupt_handler+0x10/0x30
> > [ 1240.497168] __do_softirq+0xed/0x2aa
> > [ 1240.497168] irq_exit+0xb7/0xc0
> > [ 1240.497168] do_IRQ+0x45/0xd0
> > [ 1240.497168] common_interrupt+0xf/0xf
> > [ 1240.497168] </IRQ>
> > [ 1240.497168] RIP: 0010:cpuidle_enter_state+0xa6/0x330
> > [ 1240.497168] Code: 65 8b 3d 1d b0 4d 55 e8 58 6a 95 ff 48 89 c3 66 66 66 66
> 90 31 ff e8 59 73 95 ff 80 7c 24 0b 00 0f 85 25 02 00 00 fb 4c 29 eb <48> ba cf f7
> 53 e3 a5 9b c4 20 48 89 d8 48 c1 fb 3f 48 f7 ea b8 ff
> > [ 1240.497168] RSP: 0018:ffffffffab603e60 EFLAGS: 00000216 ORIG_RAX:
> ffffffffffffffde
> > [ 1240.497168] RAX: ffff98490be20a80 RBX: 000000000081035c RCX:
> 00000120cf178c49
> > [ 1240.497168] RDX: 00000120cf178ca0 RSI: 00000120cf178ca0 RDI:
> 0000000000000000
> > [ 1240.497168] RBP: ffff984908fbd000 R08: fffffffb58ea5f9e R09:
> 000001208e0b48df
> > [ 1240.497168] R10: 00000000000018c4 R11: 0000000000002468 R12:
> 0000000000000002
> > [ 1240.497168] R13: 00000120ce968944 R14: ffffffffab6a68a0 R15:
> ffffffffab611740
> > [ 1240.497168] do_idle+0x1da/0x230
> > [ 1240.497168] cpu_startup_entry+0x6a/0x70
> > [ 1240.497168] start_kernel+0x4a2/0x4c2
> > [ 1240.497168] secondary_startup_64+0xa4/0xb0
> > [ 1240.497168] ---[ end trace c6f3be34c214db4e ]---
> >
> > Notice the warning is referring to a different adapter. So I suspect that
> whatever happened it froze
> > All network adapters.
> >
> > If you have suggestions let me know.
>
> Did you look at the output of "perf top" or something along those lines
> to figure out if your lan743x driver is indeed responsible for that by
> not being scheduler friendly? What is likely happening is that you do
> not reclaim "weight" packets and instead keep looping into NAPI context,
> which prevents the system from making further progress.
>
> Calling napi_complete_done() for the TX path is not necessary AFAICT,
> what you really want to do is call napi_complete() and make sure you:
>
> - reclaim/free as many TX buffers as possible, without looking at the
> NAPI weight which becomes irrelevant
> - if you have been able to reclaim enough descriptors, wake-up the
> transmit queue
>
> So ignoring the NAPI weight like you do is correct, but calling
> napi_complete_done() with a 0 argument does not sound correct to me.
> --
> Florian
^ permalink raw reply
* [Patch net-next 2/2] net: dump whole skb data in netdev_rx_csum_fault()
From: Cong Wang @ 2018-11-21 2:13 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Herbert Xu, Eric Dumazet, David Miller
In-Reply-To: <20181121021309.6595-1-xiyou.wangcong@gmail.com>
Currently, we only dump a few selected skb fields in
netdev_rx_csum_fault(). It is not suffient for debugging checksum
fault. This patch introduces skb_dump() which dumps skb mac header,
network header and its whole skb->data too.
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Eric Dumazet <edumazet@google.com>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/linux/skbuff.h | 5 +++++
net/core/dev.c | 6 +-----
net/core/skbuff.c | 49 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 55 insertions(+), 5 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index afddb5c17ce5..844c0a7ff52f 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4218,5 +4218,10 @@ static inline __wsum lco_csum(struct sk_buff *skb)
return csum_partial(l4_hdr, csum_start - l4_hdr, partial);
}
+#ifdef CONFIG_BUG
+void skb_dump(const char *level, const struct sk_buff *skb, bool dump_header,
+ bool dump_mac_header, bool dump_network_header);
+#endif
+
#endif /* __KERNEL__ */
#endif /* _LINUX_SKBUFF_H */
diff --git a/net/core/dev.c b/net/core/dev.c
index f2bfd2eda7b2..dc54c89fb4b1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3097,11 +3097,7 @@ void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb)
pr_err("%s: hw csum failure\n", dev ? dev->name : "<unknown>");
if (dev)
pr_err("dev features: %pNF\n", &dev->features);
- pr_err("skb len=%u data_len=%u pkt_type=%u gso_size=%u gso_type=%u nr_frags=%u ip_summed=%u csum=%x csum_complete_sw=%d csum_valid=%d csum_level=%u\n",
- skb->len, skb->data_len, skb->pkt_type,
- skb_shinfo(skb)->gso_size, skb_shinfo(skb)->gso_type,
- skb_shinfo(skb)->nr_frags, skb->ip_summed, skb->csum,
- skb->csum_complete_sw, skb->csum_valid, skb->csum_level);
+ skb_dump(KERN_ERR, skb, true, true, true);
dump_stack();
}
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index b6ba923e7dc7..21aaef3f6a4a 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5555,3 +5555,52 @@ void skb_condense(struct sk_buff *skb)
*/
skb->truesize = SKB_TRUESIZE(skb_end_offset(skb));
}
+
+#ifdef CONFIG_BUG
+void skb_dump(const char *level, const struct sk_buff *skb, bool dump_header,
+ bool dump_mac_header, bool dump_network_header)
+{
+ struct sk_buff *frag_iter;
+ int i;
+
+ if (dump_header)
+ printk("%sskb len=%u data_len=%u pkt_type=%u gso_size=%u gso_type=%u nr_frags=%u ip_summed=%u csum=%x csum_complete_sw=%d csum_valid=%d csum_level=%u\n",
+ level, skb->len, skb->data_len, skb->pkt_type,
+ skb_shinfo(skb)->gso_size, skb_shinfo(skb)->gso_type,
+ skb_shinfo(skb)->nr_frags, skb->ip_summed, skb->csum,
+ skb->csum_complete_sw, skb->csum_valid, skb->csum_level);
+
+ if (dump_mac_header && skb_mac_header_was_set(skb))
+ print_hex_dump(level, "mac header: ", DUMP_PREFIX_OFFSET, 16, 1,
+ skb_mac_header(skb), skb_mac_header_len(skb),
+ false);
+
+ if (dump_network_header && skb_network_header_was_set(skb))
+ print_hex_dump(level, "network header: ", DUMP_PREFIX_OFFSET,
+ 16, 1, skb_network_header(skb),
+ skb_network_header_len(skb), false);
+
+ print_hex_dump(level, "skb data: ", DUMP_PREFIX_OFFSET, 16, 1,
+ skb->data, skb->len, false);
+
+ for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+ skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+ u32 p_off, p_len, copied;
+ struct page *p;
+ u8 *vaddr;
+
+ skb_frag_foreach_page(frag, frag->page_offset, skb_frag_size(frag),
+ p, p_off, p_len, copied) {
+ vaddr = kmap_atomic(p);
+ print_hex_dump(level, "skb frag: ", DUMP_PREFIX_OFFSET,
+ 16, 1, vaddr + p_off, p_len, false);
+ kunmap_atomic(vaddr);
+ }
+ }
+
+ if (skb_has_frag_list(skb))
+ printk("%sskb frags list:\n", level);
+ skb_walk_frags(skb, frag_iter)
+ skb_dump(level, frag_iter, false, false, false);
+}
+#endif
--
2.19.1
^ permalink raw reply related
* [Patch net-next 1/2] net: introduce skb_network_header_was_set()
From: Cong Wang @ 2018-11-21 2:13 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/linux/skbuff.h | 5 +++++
net/core/skbuff.c | 2 ++
2 files changed, 7 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index a2e8297a5b00..afddb5c17ce5 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2444,6 +2444,11 @@ static inline u32 skb_network_header_len(const struct sk_buff *skb)
return skb->transport_header - skb->network_header;
}
+static inline int skb_network_header_was_set(const struct sk_buff *skb)
+{
+ return skb->network_header != (typeof(skb->network_header))~0U;
+}
+
static inline u32 skb_inner_network_header_len(const struct sk_buff *skb)
{
return skb->inner_transport_header - skb->inner_network_header;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 9a8a72cefe9b..b6ba923e7dc7 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -227,6 +227,7 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
skb_reset_tail_pointer(skb);
skb->end = skb->tail + size;
skb->mac_header = (typeof(skb->mac_header))~0U;
+ skb->network_header = (typeof(skb->network_header))~0U;
skb->transport_header = (typeof(skb->transport_header))~0U;
/* make sure we initialize shinfo sequentially */
@@ -292,6 +293,7 @@ struct sk_buff *__build_skb(void *data, unsigned int frag_size)
skb_reset_tail_pointer(skb);
skb->end = skb->tail + size;
skb->mac_header = (typeof(skb->mac_header))~0U;
+ skb->network_header = (typeof(skb->network_header))~0U;
skb->transport_header = (typeof(skb->transport_header))~0U;
/* make sure we initialize shinfo sequentially */
--
2.19.1
^ permalink raw reply related
* Re: [PATCH v4 bpf-next 1/2] bpf: adding support for map in map in libbpf
From: Alexei Starovoitov @ 2018-11-21 2:11 UTC (permalink / raw)
To: Nikita V. Shirokov
Cc: Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski, netdev
In-Reply-To: <20181121013344.11006-2-tehnerd@tehnerd.com>
On Tue, Nov 20, 2018 at 05:33:43PM -0800, Nikita V. Shirokov wrote:
> idea is pretty simple. for specified map (pointed by struct bpf_map)
> we would provide descriptor of already loaded map, which is going to be
> used as a prototype for inner map. proposed workflow:
> 1) open bpf's object (bpf_object__open)
> 2) create bpf's map which is going to be used as a prototype
> 3) find (by name) map-in-map which you want to load and update w/
> descriptor of inner map w/ a new helper from this patch
> 4) load bpf program w/ bpf_object__load
>
> Signed-off-by: Nikita V. Shirokov <tehnerd@tehnerd.com>
> Acked-by: Yonghong Song <yhs@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 33 +++++++++++++++++++++++++++------
> tools/lib/bpf/libbpf.h | 2 ++
> 2 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index a01eb9584e52..0f46e8497ab8 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -163,6 +163,7 @@ struct bpf_map {
> char *name;
> size_t offset;
> int map_ifindex;
> + int inner_map_fd;
> struct bpf_map_def def;
> __u32 btf_key_type_id;
> __u32 btf_value_type_id;
> @@ -585,6 +586,14 @@ static int compare_bpf_map(const void *_a, const void *_b)
> return a->offset - b->offset;
> }
>
> +static bool bpf_map_type__is_mapinmap(enum bpf_map_type type)
there is already public api bpf_create_map_in_map()
Please use the existing naming convention.
> +{
> + if (type == BPF_MAP_TYPE_ARRAY_OF_MAPS ||
> + type == BPF_MAP_TYPE_HASH_OF_MAPS)
> + return true;
> + return false;
> +}
> +
> static int
> bpf_object__init_maps(struct bpf_object *obj, int flags)
> {
> @@ -648,13 +657,15 @@ bpf_object__init_maps(struct bpf_object *obj, int flags)
> }
> obj->nr_maps = nr_maps;
>
> - /*
> - * fill all fd with -1 so won't close incorrect
> - * fd (fd=0 is stdin) when failure (zclose won't close
> - * negative fd)).
> - */
> - for (i = 0; i < nr_maps; i++)
> + for (i = 0; i < nr_maps; i++) {
> + /*
> + * fill all fd with -1 so won't close incorrect
> + * fd (fd=0 is stdin) when failure (zclose won't close
> + * negative fd)).
> + */
> obj->maps[i].fd = -1;
> + obj->maps[i].inner_map_fd = -1;
> + }
>
> /*
> * Fill obj->maps using data in "maps" section.
> @@ -1146,6 +1157,9 @@ bpf_object__create_maps(struct bpf_object *obj)
> create_attr.btf_fd = 0;
> create_attr.btf_key_type_id = 0;
> create_attr.btf_value_type_id = 0;
> + if (bpf_map_type__is_mapinmap(def->type) &&
> + map->inner_map_fd >= 0)
> + create_attr.inner_map_fd = map->inner_map_fd;
>
> if (obj->btf && !bpf_map_find_btf_info(map, obj->btf)) {
> create_attr.btf_fd = btf__fd(obj->btf);
> @@ -2562,6 +2576,13 @@ void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex)
> map->map_ifindex = ifindex;
> }
>
> +void bpf_map__set_inner_map_fd(struct bpf_map *map, int fd)
> +{
> + if (bpf_map_type__is_mapinmap(map->def.type) &&
> + map->inner_map_fd == -1)
> + map->inner_map_fd = fd;
return an error?
> +}
> +
> static struct bpf_map *
> __bpf_map__iter(struct bpf_map *m, struct bpf_object *obj, int i)
> {
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index b1686a787102..e2132c8c84ae 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -293,6 +293,8 @@ LIBBPF_API void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex);
> LIBBPF_API int bpf_map__pin(struct bpf_map *map, const char *path);
> LIBBPF_API int bpf_map__unpin(struct bpf_map *map, const char *path);
>
> +LIBBPF_API void bpf_map__set_inner_map_fd(struct bpf_map *map, int fd);
> +
> LIBBPF_API long libbpf_get_error(const void *ptr);
>
> struct bpf_prog_load_attr {
> --
> 2.15.1
>
^ permalink raw reply
* Re: [PATCH net-next v3 00/13] virtio: support packed ring
From: Tiwei Bie @ 2018-11-21 12:42 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: jasowang, virtualization, linux-kernel, netdev, virtio-dev, wexu,
jfreimann, maxime.coquelin
In-Reply-To: <20181121071308-mutt-send-email-mst@kernel.org>
On Wed, Nov 21, 2018 at 07:20:27AM -0500, Michael S. Tsirkin wrote:
> On Wed, Nov 21, 2018 at 06:03:17PM +0800, Tiwei Bie wrote:
> > Hi,
> >
> > This patch set implements packed ring support in virtio driver.
> >
> > A performance test between pktgen (pktgen_sample03_burst_single_flow.sh)
> > and DPDK vhost (testpmd/rxonly/vhost-PMD) has been done, I saw
> > ~30% performance gain in packed ring in this case.
>
> Thanks a lot, this is very exciting!
> Dave, given the holiday, attempts to wrap up the 1.1 spec and the
> patchset size I would very much appreciate a bit more time for
> review. Say until Nov 28?
>
> > To make this patch set work with below patch set for vhost,
> > some hacks are needed to set the _F_NEXT flag in indirect
> > descriptors (this should be fixed in vhost):
> >
> > https://lkml.org/lkml/2018/7/3/33
>
> Could you pls clarify - do you mean it doesn't yet work with vhost
> because of a vhost bug, and to test it with the linked patches
> you had to hack in _F_NEXT? Because I do not see _F_NEXT
> in indirect descriptors in this patch (which is fine).
> Or did I miss it?
You didn't miss anything. :)
I think it's a small bug in vhost, which Jason may fix very
quickly, so I didn't post it. Below is the hack I used:
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cd7e755484e3..42faea7d8cf8 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -980,6 +980,7 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
unsigned int i, n, err_idx;
u16 head, id;
dma_addr_t addr;
+ int c = 0;
head = vq->packed.next_avail_idx;
desc = alloc_indirect_packed(total_sg, gfp);
@@ -1001,8 +1002,9 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
if (vring_mapping_error(vq, addr))
goto unmap_release;
- desc[i].flags = cpu_to_le16(n < out_sgs ?
- 0 : VRING_DESC_F_WRITE);
+ desc[i].flags = cpu_to_le16((n < out_sgs ?
+ 0 : VRING_DESC_F_WRITE) |
+ (++c == total_sg ? 0 : VRING_DESC_F_NEXT));
desc[i].addr = cpu_to_le64(addr);
desc[i].len = cpu_to_le32(sg->length);
i++;
--
2.14.1
>
> > v2 -> v3:
> > - Use leXX instead of virtioXX (MST);
> > - Refactor split ring first (MST);
> > - Add debug helpers (MST);
> > - Put split/packed ring specific fields in sub structures (MST);
> > - Handle normal descriptors and indirect descriptors differently (MST);
> > - Track the DMA addr/len related info in a separate structure (MST);
> > - Calculate AVAIL/USED flags only when wrap counter wraps (MST);
> > - Define a struct/union to read event structure (MST);
> > - Define a macro for wrap counter bit in uapi (MST);
> > - Define the AVAIL/USED bits as shifts instead of values (MST);
> > - s/_F_/_FLAG_/ in VRING_PACKED_EVENT_* as they are values (MST);
> > - Drop the notify workaround for QEMU's tx-timer in packed ring (MST);
> >
> > v1 -> v2:
> > - Use READ_ONCE() to read event off_wrap and flags together (Jason);
> > - Add comments related to ccw (Jason);
> >
> > RFC v6 -> v1:
> > - Avoid extra virtio_wmb() in virtqueue_enable_cb_delayed_packed()
> > when event idx is off (Jason);
> > - Fix bufs calculation in virtqueue_enable_cb_delayed_packed() (Jason);
> > - Test the state of the desc at used_idx instead of last_used_idx
> > in virtqueue_enable_cb_delayed_packed() (Jason);
> > - Save wrap counter (as part of queue state) in the return value
> > of virtqueue_enable_cb_prepare_packed();
> > - Refine the packed ring definitions in uapi;
> > - Rebase on the net-next tree;
> >
> > RFC v5 -> RFC v6:
> > - Avoid tracking addr/len/flags when DMA API isn't used (MST/Jason);
> > - Define wrap counter as bool (Jason);
> > - Use ALIGN() in vring_init_packed() (Jason);
> > - Avoid using pointer to track `next` in detach_buf_packed() (Jason);
> > - Add comments for barriers (Jason);
> > - Don't enable RING_PACKED on ccw for now (noticed by Jason);
> > - Refine the memory barrier in virtqueue_poll();
> > - Add a missing memory barrier in virtqueue_enable_cb_delayed_packed();
> > - Remove the hacks in virtqueue_enable_cb_prepare_packed();
> >
> > RFC v4 -> RFC v5:
> > - Save DMA addr, etc in desc state (Jason);
> > - Track used wrap counter;
> >
> > RFC v3 -> RFC v4:
> > - Make ID allocation support out-of-order (Jason);
> > - Various fixes for EVENT_IDX support;
> >
> > RFC v2 -> RFC v3:
> > - Split into small patches (Jason);
> > - Add helper virtqueue_use_indirect() (Jason);
> > - Just set id for the last descriptor of a list (Jason);
> > - Calculate the prev in virtqueue_add_packed() (Jason);
> > - Fix/improve desc suppression code (Jason/MST);
> > - Refine the code layout for XXX_split/packed and wrappers (MST);
> > - Fix the comments and API in uapi (MST);
> > - Remove the BUG_ON() for indirect (Jason);
> > - Some other refinements and bug fixes;
> >
> > RFC v1 -> RFC v2:
> > - Add indirect descriptor support - compile test only;
> > - Add event suppression supprt - compile test only;
> > - Move vring_packed_init() out of uapi (Jason, MST);
> > - Merge two loops into one in virtqueue_add_packed() (Jason);
> > - Split vring_unmap_one() for packed ring and split ring (Jason);
> > - Avoid using '%' operator (Jason);
> > - Rename free_head -> next_avail_idx (Jason);
> > - Add comments for virtio_wmb() in virtqueue_add_packed() (Jason);
> > - Some other refinements and bug fixes;
> >
> >
> > Tiwei Bie (13):
> > virtio: add packed ring types and macros
> > virtio_ring: add _split suffix for split ring functions
> > virtio_ring: put split ring functions together
> > virtio_ring: put split ring fields in a sub struct
> > virtio_ring: introduce debug helpers
> > virtio_ring: introduce helper for indirect feature
> > virtio_ring: allocate desc state for split ring separately
> > virtio_ring: extract split ring handling from ring creation
> > virtio_ring: cache whether we will use DMA API
> > virtio_ring: introduce packed ring support
> > virtio_ring: leverage event idx in packed ring
> > virtio_ring: disable packed ring on unsupported transports
> > virtio_ring: advertize packed ring layout
> >
> > drivers/misc/mic/vop/vop_main.c | 13 +
> > drivers/remoteproc/remoteproc_virtio.c | 13 +
> > drivers/s390/virtio/virtio_ccw.c | 14 +
> > drivers/virtio/virtio_ring.c | 1811 +++++++++++++++++++++++++-------
> > include/uapi/linux/virtio_config.h | 3 +
> > include/uapi/linux/virtio_ring.h | 52 +
> > 6 files changed, 1530 insertions(+), 376 deletions(-)
> >
> > --
> > 2.14.5
^ permalink raw reply related
* Re: [PATCH bpf-next] bpf: add read/write access to skb->tstamp from tc clsact progs
From: Willem de Bruijn @ 2018-11-21 2:01 UTC (permalink / raw)
To: Eric Dumazet
Cc: Vlad Dumitrescu, Alexei Starovoitov, Daniel Borkmann,
Network Development, Eric Dumazet, Willem de Bruijn
In-Reply-To: <8154bb99-4454-42a7-29b7-58ae8c9e57f0@gmail.com>
On Tue, Nov 20, 2018 at 8:22 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 11/20/2018 04:18 PM, Vlad Dumitrescu wrote:
> > This could be used to rate limit egress traffic in concert with a qdisc
> > which supports Earliest Departure Time, such as FQ.
> >
> > Signed-off-by: Vlad Dumitrescu <vladum@google.com>
> > ---
> > include/uapi/linux/bpf.h | 1 +
> > net/core/filter.c | 26 +++++++++++++++++++++
> > tools/include/uapi/linux/bpf.h | 1 +
> > tools/testing/selftests/bpf/test_verifier.c | 4 ++++
> > 4 files changed, 32 insertions(+)
> >
>
> Awesome, thanks Vlad
>
> Note that this also can be used to implement a delay (a la netem).
>
> Acked-by: Eric Dumazet <edumazet@google.com>
Acked-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox