* [PATCH net-next 07/13] net: sched: red: offload a graft notification
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Drivers offloading Qdiscs should have reasonable certainty
the offloaded behaviour matches the SW path. This is impossible
if the driver does not know about all Qdiscs or when Qdiscs move
and are reused. Send a graft notification from RED. The drivers
are expected to simply stop offloading the Qdisc, if a non-standard
child is ever grafted onto it.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
include/net/pkt_cls.h | 2 ++
net/sched/sch_red.c | 17 +++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index fa31d034231d..01f2802b7aee 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -834,6 +834,7 @@ enum tc_red_command {
TC_RED_DESTROY,
TC_RED_STATS,
TC_RED_XSTATS,
+ TC_RED_GRAFT,
};
struct tc_red_qopt_offload_params {
@@ -853,6 +854,7 @@ struct tc_red_qopt_offload {
struct tc_red_qopt_offload_params set;
struct tc_qopt_offload_stats stats;
struct red_stats *xstats;
+ u32 child_handle;
};
};
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index a1d08bdd9357..4b5ca172ee2d 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -367,6 +367,21 @@ static int red_dump_class(struct Qdisc *sch, unsigned long cl,
return 0;
}
+static void red_graft_offload(struct Qdisc *sch,
+ struct Qdisc *new, struct Qdisc *old,
+ struct netlink_ext_ack *extack)
+{
+ struct tc_red_qopt_offload graft_offload = {
+ .handle = sch->handle,
+ .parent = sch->parent,
+ .child_handle = new->handle,
+ .command = TC_RED_GRAFT,
+ };
+
+ qdisc_offload_graft_helper(qdisc_dev(sch), sch, new, old,
+ TC_SETUP_QDISC_RED, &graft_offload, extack);
+}
+
static int red_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
struct Qdisc **old, struct netlink_ext_ack *extack)
{
@@ -376,6 +391,8 @@ static int red_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
new = &noop_qdisc;
*old = qdisc_replace(sch, new, &q->qdisc);
+
+ red_graft_offload(sch, new, *old, extack);
return 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 08/13] net: sched: mq: offload a graft notification
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Drivers offloading Qdiscs should have reasonable certainty
the offloaded behaviour matches the SW path. This is impossible
if the driver does not know about all Qdiscs or when Qdiscs move
and are reused. Send a graft notification from MQ.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
include/net/pkt_cls.h | 11 ++++++++++-
net/sched/sch_mq.c | 9 +++++++++
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 01f2802b7aee..5d31820b7e80 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -821,12 +821,21 @@ enum tc_mq_command {
TC_MQ_CREATE,
TC_MQ_DESTROY,
TC_MQ_STATS,
+ TC_MQ_GRAFT,
+};
+
+struct tc_mq_opt_offload_graft_params {
+ unsigned long queue;
+ u32 child_handle;
};
struct tc_mq_qopt_offload {
enum tc_mq_command command;
u32 handle;
- struct tc_qopt_offload_stats stats;
+ union {
+ struct tc_qopt_offload_stats stats;
+ struct tc_mq_opt_offload_graft_params graft_params;
+ };
};
enum tc_red_command {
diff --git a/net/sched/sch_mq.c b/net/sched/sch_mq.c
index 1db5c1bf6ddd..203659bc3906 100644
--- a/net/sched/sch_mq.c
+++ b/net/sched/sch_mq.c
@@ -193,6 +193,7 @@ static int mq_graft(struct Qdisc *sch, unsigned long cl, struct Qdisc *new,
struct Qdisc **old, struct netlink_ext_ack *extack)
{
struct netdev_queue *dev_queue = mq_queue_get(sch, cl);
+ struct tc_mq_qopt_offload graft_offload;
struct net_device *dev = qdisc_dev(sch);
if (dev->flags & IFF_UP)
@@ -203,6 +204,14 @@ static int mq_graft(struct Qdisc *sch, unsigned long cl, struct Qdisc *new,
new->flags |= TCQ_F_ONETXQUEUE | TCQ_F_NOPARENT;
if (dev->flags & IFF_UP)
dev_activate(dev);
+
+ graft_offload.handle = sch->handle;
+ graft_offload.graft_params.queue = cl - 1;
+ graft_offload.graft_params.child_handle = new ? new->handle : 0;
+ graft_offload.command = TC_MQ_GRAFT;
+
+ qdisc_offload_graft_helper(qdisc_dev(sch), sch, new, *old,
+ TC_SETUP_QDISC_MQ, &graft_offload, extack);
return 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 09/13] nfp: abm: build full Qdisc hierarchy based on graft notifications
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Using graft notifications recreate in the driver the full Qdisc
hierarchy. Keep track of how many times each Qdisc is attached
to the hierarchy to make sure we don't offload Qdiscs which are
attached multiple times (device queues can't be shared). For
graft events of Qdiscs we don't know exist make the child as
invalid/untracked.
Note that MQ Qdisc doesn't send destruction events reliably when
device is dismantled, so we need to manually clean out the
children otherwise we'd think Qdiscs which are still in use
are getting freed.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/main.h | 2 +
.../net/ethernet/netronome/nfp/abm/qdisc.c | 105 ++++++++++++++++++
2 files changed, 107 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index adffa36981e0..daca93e90099 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -78,6 +78,8 @@ enum nfp_qdisc_type {
NFP_QDISC_RED,
};
+#define NFP_QDISC_UNTRACKED ((struct nfp_qdisc *)1UL)
+
/**
* struct nfp_qdisc - tracked TC Qdisc
* @netdev: netdev on which Qdisc was created
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index 3ecb63060429..151d2dafbc76 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
/* Copyright (C) 2018 Netronome Systems, Inc. */
+#include <linux/rtnetlink.h>
#include <net/pkt_cls.h>
#include <net/pkt_sched.h>
#include <net/red.h>
@@ -12,6 +13,66 @@
#include "../nfp_port.h"
#include "main.h"
+static bool nfp_abm_qdisc_child_valid(struct nfp_qdisc *qdisc, unsigned int id)
+{
+ return qdisc->children[id] &&
+ qdisc->children[id] != NFP_QDISC_UNTRACKED;
+}
+
+static void *nfp_abm_qdisc_tree_deref_slot(void __rcu **slot)
+{
+ return rtnl_dereference(*slot);
+}
+
+static void
+nfp_abm_qdisc_unlink_children(struct nfp_qdisc *qdisc,
+ unsigned int start, unsigned int end)
+{
+ unsigned int i;
+
+ for (i = start; i < end; i++)
+ if (nfp_abm_qdisc_child_valid(qdisc, i)) {
+ qdisc->children[i]->use_cnt--;
+ qdisc->children[i] = NULL;
+ }
+}
+
+static void
+nfp_abm_qdisc_clear_mq(struct net_device *netdev, struct nfp_abm_link *alink,
+ struct nfp_qdisc *qdisc)
+{
+ struct radix_tree_iter iter;
+ unsigned int mq_refs = 0;
+ void __rcu **slot;
+
+ if (!qdisc->use_cnt)
+ return;
+ /* MQ doesn't notify well on destruction, we need special handling of
+ * MQ's children.
+ */
+ if (qdisc->type == NFP_QDISC_MQ &&
+ qdisc == alink->root_qdisc &&
+ netdev->reg_state == NETREG_UNREGISTERING)
+ return;
+
+ /* Count refs held by MQ instances and clear pointers */
+ radix_tree_for_each_slot(slot, &alink->qdiscs, &iter, 0) {
+ struct nfp_qdisc *mq = nfp_abm_qdisc_tree_deref_slot(slot);
+ unsigned int i;
+
+ if (mq->type != NFP_QDISC_MQ || mq->netdev != netdev)
+ continue;
+ for (i = 0; i < mq->num_children; i++)
+ if (mq->children[i] == qdisc) {
+ mq->children[i] = NULL;
+ mq_refs++;
+ }
+ }
+
+ WARN(qdisc->use_cnt != mq_refs, "non-zero qdisc use count: %d (- %d)\n",
+ qdisc->use_cnt, mq_refs);
+}
+
static void
nfp_abm_offload_compile_red(struct nfp_abm_link *alink,
struct nfp_red_qdisc *qdisc, unsigned int queue)
@@ -70,6 +131,7 @@ nfp_abm_qdisc_free(struct net_device *netdev, struct nfp_abm_link *alink,
if (!qdisc)
return;
+ nfp_abm_qdisc_clear_mq(netdev, alink, qdisc);
WARN_ON(radix_tree_delete(&alink->qdiscs,
TC_H_MAJ(qdisc->handle)) != qdisc);
@@ -152,12 +214,44 @@ nfp_abm_qdisc_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
if (!qdisc)
return;
+ /* We don't get TC_SETUP_ROOT_QDISC w/ MQ when netdev is unregistered */
+ if (alink->root_qdisc == qdisc)
+ qdisc->use_cnt--;
+
+ nfp_abm_qdisc_unlink_children(qdisc, 0, qdisc->num_children);
nfp_abm_qdisc_free(netdev, alink, qdisc);
if (alink->root_qdisc == qdisc)
alink->root_qdisc = NULL;
}
+static int
+nfp_abm_qdisc_graft(struct nfp_abm_link *alink, u32 handle, u32 child_handle,
+ unsigned int id)
+{
+ struct nfp_qdisc *parent, *child;
+
+ parent = nfp_abm_qdisc_find(alink, handle);
+ if (!parent)
+ return 0;
+
+ if (WARN(id >= parent->num_children,
+ "graft child out of bound %d >= %d\n",
+ id, parent->num_children))
+ return -EINVAL;
+
+ nfp_abm_qdisc_unlink_children(parent, id, id + 1);
+
+ child = nfp_abm_qdisc_find(alink, child_handle);
+ if (child)
+ child->use_cnt++;
+ else
+ child = NFP_QDISC_UNTRACKED;
+ parent->children[id] = child;
+
+ return 0;
+}
+
static void
__nfp_abm_reset_root(struct net_device *netdev, struct nfp_abm_link *alink,
u32 handle, unsigned int qs, u32 init_val)
@@ -404,6 +498,9 @@ int nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
return nfp_abm_red_stats(alink, opt);
case TC_RED_XSTATS:
return nfp_abm_red_xstats(alink, opt);
+ case TC_RED_GRAFT:
+ return nfp_abm_qdisc_graft(alink, opt->handle,
+ opt->child_handle, 0);
default:
return -EOPNOTSUPP;
}
@@ -460,6 +557,10 @@ int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
return 0;
case TC_MQ_STATS:
return nfp_abm_mq_stats(alink, opt);
+ case TC_MQ_GRAFT:
+ return nfp_abm_qdisc_graft(alink, opt->handle,
+ opt->graft_params.child_handle,
+ opt->graft_params.queue);
default:
return -EOPNOTSUPP;
}
@@ -470,7 +571,11 @@ int nfp_abm_setup_root(struct net_device *netdev, struct nfp_abm_link *alink,
{
if (opt->ingress)
return -EOPNOTSUPP;
+ if (alink->root_qdisc)
+ alink->root_qdisc->use_cnt--;
alink->root_qdisc = nfp_abm_qdisc_find(alink, opt->handle);
+ if (alink->root_qdisc)
+ alink->root_qdisc->use_cnt++;
return 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 10/13] net: sched: red: notify drivers about RED's limit parameter
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
RED qdisc's limit parameter changes the behaviour of the qdisc,
for instance if it's set to 0 qdisc will drop all the packets.
When replace operation happens and parameter is set to non-0
a new fifo qdisc will be instantiated and replace the old child
qdisc which will be destroyed.
Drivers need to know the parameter, even if they don't impose
the actual limit to be able to reliably reconstruct the Qdisc
hierarchy.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
include/net/pkt_cls.h | 1 +
net/sched/sch_red.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 5d31820b7e80..c497ada7f591 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -850,6 +850,7 @@ struct tc_red_qopt_offload_params {
u32 min;
u32 max;
u32 probability;
+ u32 limit;
bool is_ecn;
bool is_harddrop;
struct gnet_stats_queue *qstats;
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index 4b5ca172ee2d..9df9942340ea 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -166,6 +166,7 @@ static int red_offload(struct Qdisc *sch, bool enable)
opt.set.min = q->parms.qth_min >> q->parms.Wlog;
opt.set.max = q->parms.qth_max >> q->parms.Wlog;
opt.set.probability = q->parms.max_P;
+ opt.set.limit = q->limit;
opt.set.is_ecn = red_use_ecn(q);
opt.set.is_harddrop = red_use_harddrop(q);
opt.set.qstats = &sch->qstats;
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 11/13] nfp: abm: reset RED's child based on limit
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
RED qdisc will replace its child Qdisc with a new FIFO queue if
it is reconfigured and the limit parameter is not 0.
This means that when it's created with limit of 0 it will have no FIFO,
and all packets will be dropped. If it's changed and limit is specified
it will loose its existing child (implicit graft). Make sure we mark
RED Qdisc child as NFP_QDISC_UNTRACKED if its not the expected FIFO.
nfp_abm_qdisc_replace() will return 1 if Qdisc already existed.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
.../net/ethernet/netronome/nfp/abm/qdisc.c | 27 +++++++++++++++----
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index 151d2dafbc76..1b3c0b5b52bf 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -196,7 +196,7 @@ nfp_abm_qdisc_replace(struct net_device *netdev, struct nfp_abm_link *alink,
if (*qdisc) {
if (WARN_ON((*qdisc)->type != type))
return -EINVAL;
- return 0;
+ return 1;
}
*qdisc = nfp_abm_qdisc_alloc(netdev, alink, type, parent_handle, handle,
@@ -357,11 +357,24 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
i = nfp_abm_red_find(alink, opt);
existing = i >= 0;
- if (ret) {
+ if (ret < 0) {
err = ret;
goto err_destroy;
}
+ /* If limit != 0 child gets reset */
+ if (opt->set.limit) {
+ if (nfp_abm_qdisc_child_valid(qdisc, 0))
+ qdisc->children[0]->use_cnt--;
+ qdisc->children[0] = NULL;
+ } else {
+ /* Qdisc was just allocated without a limit will use noop_qdisc,
+ * i.e. a block hole.
+ */
+ if (!ret)
+ qdisc->children[0] = NFP_QDISC_UNTRACKED;
+ }
+
if (!nfp_abm_red_check_params(alink, opt)) {
err = -EINVAL;
goto err_destroy;
@@ -533,10 +546,14 @@ nfp_abm_mq_create(struct net_device *netdev, struct nfp_abm_link *alink,
struct tc_mq_qopt_offload *opt)
{
struct nfp_qdisc *qdisc;
+ int ret;
- return nfp_abm_qdisc_replace(netdev, alink, NFP_QDISC_MQ,
- TC_H_ROOT, opt->handle,
- alink->total_queues, &qdisc);
+ ret = nfp_abm_qdisc_replace(netdev, alink, NFP_QDISC_MQ,
+ TC_H_ROOT, opt->handle, alink->total_queues,
+ &qdisc);
+ if (ret < 0)
+ return ret;
+ return 0;
}
int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 12/13] nfp: abm: save RED's parameters
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Use the new driver Qdisc structure to keep track of parameters
of RED Qdiscs. This way as the Qdisc moves around in the hierarchy
we will be able to configure the HW appropriately.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/main.h | 14 ++++++++++++++
drivers/net/ethernet/netronome/nfp/abm/qdisc.c | 5 ++++-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index daca93e90099..d0d85f82bd1c 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -89,6 +89,11 @@ enum nfp_qdisc_type {
* @use_cnt: number of attachment points in the hierarchy
* @num_children: current size of the @children array
* @children: pointers to children
+ *
+ * @params_ok: parameters of this Qdisc are OK for offload
+ *
+ * @red: RED Qdisc specific parameters and state
+ * @red.threshold: ECN marking threshold
*/
struct nfp_qdisc {
struct net_device *netdev;
@@ -98,6 +103,15 @@ struct nfp_qdisc {
unsigned int use_cnt;
unsigned int num_children;
struct nfp_qdisc **children;
+
+ bool params_ok;
+
+ union {
+ /* TC_SETUP_QDISC_RED */
+ struct {
+ u32 threshold;
+ } red;
+ };
};
/**
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index 1b3c0b5b52bf..fb68038ec1da 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -375,7 +375,10 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
qdisc->children[0] = NFP_QDISC_UNTRACKED;
}
- if (!nfp_abm_red_check_params(alink, opt)) {
+ qdisc->params_ok = nfp_abm_red_check_params(alink, opt);
+ if (qdisc->params_ok) {
+ qdisc->red.threshold = opt->set.min;
+ } else {
err = -EINVAL;
goto err_destroy;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 13/13] nfp: abm: restructure Qdisc handling
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
In preparation of handling more Qdisc types switch to a different
offload strategy. We have now recreated the Qdisc hierarchy in
the driver. Every time the hierarchy changes parse it, and update
the configuration of the HW accordingly.
While at it drop the support of pretending that we can instantiate
a single queue on a multi-queue device in HW/FW. MQ is now required,
and each queue will have its own instance of RED.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/ctrl.c | 73 ---
drivers/net/ethernet/netronome/nfp/abm/main.c | 14 +-
drivers/net/ethernet/netronome/nfp/abm/main.h | 57 +-
.../net/ethernet/netronome/nfp/abm/qdisc.c | 548 ++++++++++--------
4 files changed, 340 insertions(+), 352 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
index 564427e8a6e8..1629b07f727b 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
@@ -50,27 +50,6 @@ nfp_abm_ctrl_stat(struct nfp_abm_link *alink, const struct nfp_rtsym *sym,
return 0;
}
-static int
-nfp_abm_ctrl_stat_all(struct nfp_abm_link *alink, const struct nfp_rtsym *sym,
- unsigned int stride, unsigned int offset, bool is_u64,
- u64 *res)
-{
- u64 val, sum = 0;
- unsigned int i;
- int err;
-
- for (i = 0; i < alink->vnic->max_rx_rings; i++) {
- err = nfp_abm_ctrl_stat(alink, sym, stride, offset, i,
- is_u64, &val);
- if (err)
- return err;
- sum += val;
- }
-
- *res = sum;
- return 0;
-}
-
int __nfp_abm_ctrl_set_q_lvl(struct nfp_abm *abm, unsigned int id, u32 val)
{
struct nfp_cpp *cpp = abm->app->cpp;
@@ -155,42 +134,6 @@ int nfp_abm_ctrl_read_q_stats(struct nfp_abm_link *alink, unsigned int i,
i, true, &stats->overlimits);
}
-int nfp_abm_ctrl_read_stats(struct nfp_abm_link *alink,
- struct nfp_alink_stats *stats)
-{
- u64 pkts = 0, bytes = 0;
- int i, err;
-
- for (i = 0; i < alink->vnic->max_rx_rings; i++) {
- pkts += nn_readq(alink->vnic, NFP_NET_CFG_RXR_STATS(i));
- bytes += nn_readq(alink->vnic, NFP_NET_CFG_RXR_STATS(i) + 8);
- }
- stats->tx_pkts = pkts;
- stats->tx_bytes = bytes;
-
- err = nfp_abm_ctrl_stat_all(alink, alink->abm->q_lvls,
- NFP_QLVL_STRIDE, NFP_QLVL_BLOG_BYTES,
- false, &stats->backlog_bytes);
- if (err)
- return err;
-
- err = nfp_abm_ctrl_stat_all(alink, alink->abm->q_lvls,
- NFP_QLVL_STRIDE, NFP_QLVL_BLOG_PKTS,
- false, &stats->backlog_pkts);
- if (err)
- return err;
-
- err = nfp_abm_ctrl_stat_all(alink, alink->abm->qm_stats,
- NFP_QMSTAT_STRIDE, NFP_QMSTAT_DROP,
- true, &stats->drops);
- if (err)
- return err;
-
- return nfp_abm_ctrl_stat_all(alink, alink->abm->qm_stats,
- NFP_QMSTAT_STRIDE, NFP_QMSTAT_ECN,
- true, &stats->overlimits);
-}
-
int nfp_abm_ctrl_read_q_xstats(struct nfp_abm_link *alink, unsigned int i,
struct nfp_alink_xstats *xstats)
{
@@ -207,22 +150,6 @@ int nfp_abm_ctrl_read_q_xstats(struct nfp_abm_link *alink, unsigned int i,
i, true, &xstats->ecn_marked);
}
-int nfp_abm_ctrl_read_xstats(struct nfp_abm_link *alink,
- struct nfp_alink_xstats *xstats)
-{
- int err;
-
- err = nfp_abm_ctrl_stat_all(alink, alink->abm->qm_stats,
- NFP_QMSTAT_STRIDE, NFP_QMSTAT_DROP,
- true, &xstats->pdrop);
- if (err)
- return err;
-
- return nfp_abm_ctrl_stat_all(alink, alink->abm->qm_stats,
- NFP_QMSTAT_STRIDE, NFP_QMSTAT_ECN,
- true, &xstats->ecn_marked);
-}
-
int nfp_abm_ctrl_qm_enable(struct nfp_abm *abm)
{
return nfp_mbox_cmd(abm->app->pf, NFP_MBOX_PCIE_ABM_ENABLE,
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index da5394886a78..a5732d3bd1b7 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -7,6 +7,7 @@
#include <linux/lockdep.h>
#include <linux/netdevice.h>
#include <linux/rcupdate.h>
+#include <linux/rtnetlink.h>
#include <linux/slab.h>
#include "../nfpcore/nfp.h"
@@ -310,22 +311,14 @@ nfp_abm_vnic_alloc(struct nfp_app *app, struct nfp_net *nn, unsigned int id)
alink->abm = abm;
alink->vnic = nn;
alink->id = id;
- alink->parent = TC_H_ROOT;
alink->total_queues = alink->vnic->max_rx_rings;
- alink->red_qdiscs = kvcalloc(alink->total_queues,
- sizeof(*alink->red_qdiscs),
- GFP_KERNEL);
- if (!alink->red_qdiscs) {
- err = -ENOMEM;
- goto err_free_alink;
- }
/* This is a multi-host app, make sure MAC/PHY is up, but don't
* make the MAC/PHY state follow the state of any of the ports.
*/
err = nfp_eth_set_configured(app->cpp, eth_port->index, true);
if (err < 0)
- goto err_free_qdiscs;
+ goto err_free_alink;
netif_keep_dst(nn->dp.netdev);
@@ -335,8 +328,6 @@ nfp_abm_vnic_alloc(struct nfp_app *app, struct nfp_net *nn, unsigned int id)
return 0;
-err_free_qdiscs:
- kvfree(alink->red_qdiscs);
err_free_alink:
kfree(alink);
return err;
@@ -348,7 +339,6 @@ static void nfp_abm_vnic_free(struct nfp_app *app, struct nfp_net *nn)
nfp_abm_kill_reprs(alink->abm, alink);
WARN(!radix_tree_empty(&alink->qdiscs), "left over qdiscs\n");
- kvfree(alink->red_qdiscs);
kfree(alink);
}
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index d0d85f82bd1c..240e2c8683fe 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -9,6 +9,11 @@
#include <net/devlink.h>
#include <net/pkt_cls.h>
+/* Dump of 64 PRIOs and 256 REDs seems to take 850us on Xeon v4 @ 2.20GHz;
+ * 2.5ms / 400Hz seems more than sufficient for stats resolution.
+ */
+#define NFP_ABM_STATS_REFRESH_IVAL (2500 * 1000) /* ns */
+
#define NFP_ABM_LVL_INFINITY S32_MAX
struct nfp_app;
@@ -91,9 +96,19 @@ enum nfp_qdisc_type {
* @children: pointers to children
*
* @params_ok: parameters of this Qdisc are OK for offload
+ * @offload_mark: offload refresh state - selected for offload
+ * @offloaded: Qdisc is currently offloaded to the HW
+ *
+ * @mq: MQ Qdisc specific parameters and state
+ * @mq.stats: current stats of the MQ Qdisc
+ * @mq.prev_stats: previously reported @mq.stats
*
* @red: RED Qdisc specific parameters and state
* @red.threshold: ECN marking threshold
+ * @red.stats: current stats of the RED Qdisc
+ * @red.prev_stats: previously reported @red.stats
+ * @red.xstats: extended stats for RED - current
+ * @red.prev_xstats: extended stats for RED - previously reported
*/
struct nfp_qdisc {
struct net_device *netdev;
@@ -105,29 +120,26 @@ struct nfp_qdisc {
struct nfp_qdisc **children;
bool params_ok;
+ bool offload_mark;
+ bool offloaded;
union {
+ /* NFP_QDISC_MQ */
+ struct {
+ struct nfp_alink_stats stats;
+ struct nfp_alink_stats prev_stats;
+ } mq;
/* TC_SETUP_QDISC_RED */
struct {
u32 threshold;
+ struct nfp_alink_stats stats;
+ struct nfp_alink_stats prev_stats;
+ struct nfp_alink_xstats xstats;
+ struct nfp_alink_xstats prev_xstats;
} red;
};
};
-/**
- * struct nfp_red_qdisc - representation of single RED Qdisc
- * @handle: handle of currently offloaded RED Qdisc
- * @threshold: marking threshold of this Qdisc
- * @stats: statistics from last refresh
- * @xstats: base of extended statistics
- */
-struct nfp_red_qdisc {
- u32 handle;
- u32 threshold;
- struct nfp_alink_stats stats;
- struct nfp_alink_xstats xstats;
-};
-
/**
* struct nfp_abm_link - port tuple of a ABM NIC
* @abm: back pointer to nfp_abm
@@ -135,9 +147,9 @@ struct nfp_red_qdisc {
* @id: id of the data vNIC
* @queue_base: id of base to host queue within PCIe (not QC idx)
* @total_queues: number of PF queues
- * @parent: handle of expected parent, i.e. handle of MQ, or TC_H_ROOT
- * @num_qdiscs: number of currently used qdiscs
- * @red_qdiscs: array of qdiscs
+ *
+ * @last_stats_update: ktime of last stats update
+ *
* @root_qdisc: pointer to the current root of the Qdisc hierarchy
* @qdiscs: all qdiscs recorded by major part of the handle
*/
@@ -147,13 +159,14 @@ struct nfp_abm_link {
unsigned int id;
unsigned int queue_base;
unsigned int total_queues;
- u32 parent;
- unsigned int num_qdiscs;
- struct nfp_red_qdisc *red_qdiscs;
+
+ u64 last_stats_update;
+
struct nfp_qdisc *root_qdisc;
struct radix_tree_root qdiscs;
};
+void nfp_abm_qdisc_offload_update(struct nfp_abm_link *alink);
int nfp_abm_setup_root(struct net_device *netdev, struct nfp_abm_link *alink,
struct tc_root_qopt_offload *opt);
int nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
@@ -166,12 +179,8 @@ int nfp_abm_ctrl_find_addrs(struct nfp_abm *abm);
int __nfp_abm_ctrl_set_q_lvl(struct nfp_abm *abm, unsigned int id, u32 val);
int nfp_abm_ctrl_set_q_lvl(struct nfp_abm_link *alink, unsigned int queue,
u32 val);
-int nfp_abm_ctrl_read_stats(struct nfp_abm_link *alink,
- struct nfp_alink_stats *stats);
int nfp_abm_ctrl_read_q_stats(struct nfp_abm_link *alink, unsigned int i,
struct nfp_alink_stats *stats);
-int nfp_abm_ctrl_read_xstats(struct nfp_abm_link *alink,
- struct nfp_alink_xstats *xstats);
int nfp_abm_ctrl_read_q_xstats(struct nfp_abm_link *alink, unsigned int i,
struct nfp_alink_xstats *xstats);
u64 nfp_abm_ctrl_stat_non_sto(struct nfp_abm_link *alink, unsigned int i);
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index fb68038ec1da..16c4afe3a37f 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -13,6 +13,11 @@
#include "../nfp_port.h"
#include "main.h"
+static bool nfp_abm_qdisc_is_red(struct nfp_qdisc *qdisc)
+{
+ return qdisc->type == NFP_QDISC_RED;
+}
+
static bool nfp_abm_qdisc_child_valid(struct nfp_qdisc *qdisc, unsigned int id)
{
return qdisc->children[id] &&
@@ -24,6 +29,74 @@ static void *nfp_abm_qdisc_tree_deref_slot(void __rcu **slot)
return rtnl_dereference(*slot);
}
+static void
+nfp_abm_stats_propagate(struct nfp_alink_stats *parent,
+ struct nfp_alink_stats *child)
+{
+ parent->tx_pkts += child->tx_pkts;
+ parent->tx_bytes += child->tx_bytes;
+ parent->backlog_pkts += child->backlog_pkts;
+ parent->backlog_bytes += child->backlog_bytes;
+ parent->overlimits += child->overlimits;
+ parent->drops += child->drops;
+}
+
+static void
+nfp_abm_stats_update_red(struct nfp_abm_link *alink, struct nfp_qdisc *qdisc,
+ unsigned int queue)
+{
+ struct nfp_cpp *cpp = alink->abm->app->cpp;
+ int err;
+
+ if (!qdisc->offloaded)
+ return;
+
+ err = nfp_abm_ctrl_read_q_stats(alink, queue, &qdisc->red.stats);
+ if (err)
+ nfp_err(cpp, "RED stats (%d) read failed with error %d\n",
+ queue, err);
+
+ err = nfp_abm_ctrl_read_q_xstats(alink, queue, &qdisc->red.xstats);
+ if (err)
+ nfp_err(cpp, "RED xstats (%d) read failed with error %d\n",
+ queue, err);
+}
+
+static void
+nfp_abm_stats_update_mq(struct nfp_abm_link *alink, struct nfp_qdisc *qdisc)
+{
+ unsigned int i;
+
+ if (qdisc->type != NFP_QDISC_MQ)
+ return;
+
+ for (i = 0; i < alink->total_queues; i++)
+ if (nfp_abm_qdisc_child_valid(qdisc, i))
+ nfp_abm_stats_update_red(alink, qdisc->children[i], i);
+}
+
+static void __nfp_abm_stats_update(struct nfp_abm_link *alink, u64 time_now)
+{
+ alink->last_stats_update = time_now;
+ if (alink->root_qdisc)
+ nfp_abm_stats_update_mq(alink, alink->root_qdisc);
+}
+
+static void nfp_abm_stats_update(struct nfp_abm_link *alink)
+{
+ u64 now;
+
+ /* Limit the frequency of updates - stats of non-leaf qdiscs are a sum
+ * of all their leafs, so we would read the same stat multiple times
+ * for every dump.
+ */
+ now = ktime_get();
+ if (now - alink->last_stats_update < NFP_ABM_STATS_REFRESH_IVAL)
+ return;
+
+ __nfp_abm_stats_update(alink, now);
+}
+
static void
nfp_abm_qdisc_unlink_children(struct nfp_qdisc *qdisc,
unsigned int start, unsigned int end)
@@ -38,89 +111,174 @@ nfp_abm_qdisc_unlink_children(struct nfp_qdisc *qdisc,
}
static void
-nfp_abm_qdisc_clear_mq(struct net_device *netdev, struct nfp_abm_link *alink,
- struct nfp_qdisc *qdisc)
+nfp_abm_qdisc_offload_stop(struct nfp_abm_link *alink, struct nfp_qdisc *qdisc)
{
- struct radix_tree_iter iter;
- unsigned int mq_refs = 0;
- void __rcu **slot;
+ /* Don't complain when qdisc is getting unlinked */
+ if (qdisc->use_cnt)
+ nfp_warn(alink->abm->app->cpp, "Offload of '%08x' stopped\n",
+ qdisc->handle);
- if (!qdisc->use_cnt)
+ if (!nfp_abm_qdisc_is_red(qdisc))
return;
- /* MQ doesn't notify well on destruction, we need special handling of
- * MQ's children.
+
+ qdisc->red.stats.backlog_pkts = 0;
+ qdisc->red.stats.backlog_bytes = 0;
+}
+
+static int
+__nfp_abm_stats_init(struct nfp_abm_link *alink,
+ unsigned int queue, struct nfp_alink_stats *prev_stats,
+ struct nfp_alink_xstats *prev_xstats)
+{
+ u64 backlog_pkts, backlog_bytes;
+ int err;
+
+ /* Don't touch the backlog, backlog can only be reset after it has
+ * been reported back to the tc qdisc stats.
*/
- if (qdisc->type == NFP_QDISC_MQ &&
- qdisc == alink->root_qdisc &&
- netdev->reg_state == NETREG_UNREGISTERING)
- return;
+ backlog_pkts = prev_stats->backlog_pkts;
+ backlog_bytes = prev_stats->backlog_bytes;
- /* Count refs held by MQ instances and clear pointers */
- radix_tree_for_each_slot(slot, &alink->qdiscs, &iter, 0) {
- struct nfp_qdisc *mq = nfp_abm_qdisc_tree_deref_slot(slot);
- unsigned int i;
+ err = nfp_abm_ctrl_read_q_stats(alink, queue, prev_stats);
+ if (err) {
+ nfp_err(alink->abm->app->cpp,
+ "RED stats init (%d) failed with error %d\n",
+ queue, err);
+ return err;
+ }
- if (mq->type != NFP_QDISC_MQ || mq->netdev != netdev)
- continue;
- for (i = 0; i < mq->num_children; i++)
- if (mq->children[i] == qdisc) {
- mq->children[i] = NULL;
- mq_refs++;
- }
+ err = nfp_abm_ctrl_read_q_xstats(alink, queue, prev_xstats);
+ if (err) {
+ nfp_err(alink->abm->app->cpp,
+ "RED xstats init (%d) failed with error %d\n",
+ queue, err);
+ return err;
}
- WARN(qdisc->use_cnt != mq_refs, "non-zero qdisc use count: %d (- %d)\n",
- qdisc->use_cnt, mq_refs);
+ prev_stats->backlog_pkts = backlog_pkts;
+ prev_stats->backlog_bytes = backlog_bytes;
+ return 0;
+}
+
+static int
+nfp_abm_stats_init(struct nfp_abm_link *alink, struct nfp_qdisc *qdisc,
+ unsigned int queue)
+{
+ return __nfp_abm_stats_init(alink, queue,
+ &qdisc->red.prev_stats,
+ &qdisc->red.prev_xstats);
}
static void
-nfp_abm_offload_compile_red(struct nfp_abm_link *alink,
- struct nfp_red_qdisc *qdisc, unsigned int queue)
+nfp_abm_offload_compile_red(struct nfp_abm_link *alink, struct nfp_qdisc *qdisc,
+ unsigned int queue)
{
- if (!qdisc->handle)
+ qdisc->offload_mark = qdisc->type == NFP_QDISC_RED &&
+ qdisc->params_ok &&
+ qdisc->use_cnt == 1 &&
+ !qdisc->children[0];
+
+ /* If we are starting offload init prev_stats */
+ if (qdisc->offload_mark && !qdisc->offloaded)
+ if (nfp_abm_stats_init(alink, qdisc, queue))
+ qdisc->offload_mark = false;
+
+ if (!qdisc->offload_mark)
return;
- nfp_abm_ctrl_set_q_lvl(alink, queue, qdisc->threshold);
+ nfp_abm_ctrl_set_q_lvl(alink, queue, qdisc->red.threshold);
}
-static void nfp_abm_offload_compile_one(struct nfp_abm_link *alink)
+static void
+nfp_abm_offload_compile_mq(struct nfp_abm_link *alink, struct nfp_qdisc *qdisc)
{
unsigned int i;
- bool is_mq;
- is_mq = alink->num_qdiscs > 1;
+ qdisc->offload_mark = qdisc->type == NFP_QDISC_MQ;
+ if (!qdisc->offload_mark)
+ return;
for (i = 0; i < alink->total_queues; i++) {
- struct nfp_red_qdisc *next;
+ struct nfp_qdisc *child = qdisc->children[i];
- if (is_mq && !alink->red_qdiscs[i].handle)
+ if (!nfp_abm_qdisc_child_valid(qdisc, i))
continue;
- next = is_mq ? &alink->red_qdiscs[i] : &alink->red_qdiscs[0];
- nfp_abm_offload_compile_red(alink, next, i);
+ nfp_abm_offload_compile_red(alink, child, i);
}
}
-static void nfp_abm_offload_update(struct nfp_abm *abm)
+void nfp_abm_qdisc_offload_update(struct nfp_abm_link *alink)
{
- struct nfp_abm_link *alink = NULL;
- struct nfp_pf *pf = abm->app->pf;
- struct nfp_net *nn;
+ struct nfp_abm *abm = alink->abm;
+ struct radix_tree_iter iter;
+ struct nfp_qdisc *qdisc;
+ void __rcu **slot;
size_t i;
/* Mark all thresholds as unconfigured */
- __bitmap_set(abm->threshold_undef, 0, abm->num_thresholds);
+ __bitmap_set(abm->threshold_undef,
+ alink->queue_base, alink->total_queues);
- /* Configure all offloads */
- list_for_each_entry(nn, &pf->vnics, vnic_list) {
- alink = nn->app_priv;
- nfp_abm_offload_compile_one(alink);
+ /* Clear offload marks */
+ radix_tree_for_each_slot(slot, &alink->qdiscs, &iter, 0) {
+ qdisc = nfp_abm_qdisc_tree_deref_slot(slot);
+ qdisc->offload_mark = false;
+ }
+
+ if (alink->root_qdisc)
+ nfp_abm_offload_compile_mq(alink, alink->root_qdisc);
+
+ /* Refresh offload status */
+ radix_tree_for_each_slot(slot, &alink->qdiscs, &iter, 0) {
+ qdisc = nfp_abm_qdisc_tree_deref_slot(slot);
+ if (!qdisc->offload_mark && qdisc->offloaded)
+ nfp_abm_qdisc_offload_stop(alink, qdisc);
+ qdisc->offloaded = qdisc->offload_mark;
}
/* Reset the unconfigured thresholds */
for (i = 0; i < abm->num_thresholds; i++)
if (test_bit(i, abm->threshold_undef))
__nfp_abm_ctrl_set_q_lvl(abm, i, NFP_ABM_LVL_INFINITY);
+
+ __nfp_abm_stats_update(alink, ktime_get());
+}
+
+static void
+nfp_abm_qdisc_clear_mq(struct net_device *netdev, struct nfp_abm_link *alink,
+ struct nfp_qdisc *qdisc)
+{
+ struct radix_tree_iter iter;
+ unsigned int mq_refs = 0;
+ void __rcu **slot;
+
+ if (!qdisc->use_cnt)
+ return;
+ /* MQ doesn't notify well on destruction, we need special handling of
+ * MQ's children.
+ */
+ if (qdisc->type == NFP_QDISC_MQ &&
+ qdisc == alink->root_qdisc &&
+ netdev->reg_state == NETREG_UNREGISTERING)
+ return;
+
+ /* Count refs held by MQ instances and clear pointers */
+ radix_tree_for_each_slot(slot, &alink->qdiscs, &iter, 0) {
+ struct nfp_qdisc *mq = nfp_abm_qdisc_tree_deref_slot(slot);
+ unsigned int i;
+
+ if (mq->type != NFP_QDISC_MQ || mq->netdev != netdev)
+ continue;
+ for (i = 0; i < mq->num_children; i++)
+ if (mq->children[i] == qdisc) {
+ mq->children[i] = NULL;
+ mq_refs++;
+ }
+ }
+
+ WARN(qdisc->use_cnt != mq_refs, "non-zero qdisc use count: %d (- %d)\n",
+ qdisc->use_cnt, mq_refs);
}
static void
@@ -221,8 +379,13 @@ nfp_abm_qdisc_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
nfp_abm_qdisc_unlink_children(qdisc, 0, qdisc->num_children);
nfp_abm_qdisc_free(netdev, alink, qdisc);
- if (alink->root_qdisc == qdisc)
+ if (alink->root_qdisc == qdisc) {
alink->root_qdisc = NULL;
+ /* Only root change matters, other changes are acted upon on
+ * the graft notification.
+ */
+ nfp_abm_qdisc_offload_update(alink);
+ }
}
static int
@@ -249,66 +412,73 @@ nfp_abm_qdisc_graft(struct nfp_abm_link *alink, u32 handle, u32 child_handle,
child = NFP_QDISC_UNTRACKED;
parent->children[id] = child;
+ nfp_abm_qdisc_offload_update(alink);
+
return 0;
}
static void
-__nfp_abm_reset_root(struct net_device *netdev, struct nfp_abm_link *alink,
- u32 handle, unsigned int qs, u32 init_val)
+nfp_abm_stats_calculate(struct nfp_alink_stats *new,
+ struct nfp_alink_stats *old,
+ struct gnet_stats_basic_packed *bstats,
+ struct gnet_stats_queue *qstats)
{
- memset(alink->red_qdiscs, 0,
- sizeof(*alink->red_qdiscs) * alink->num_qdiscs);
-
- alink->parent = handle;
- alink->num_qdiscs = qs;
+ _bstats_update(bstats, new->tx_bytes - old->tx_bytes,
+ new->tx_pkts - old->tx_pkts);
+ qstats->qlen += new->backlog_pkts - old->backlog_pkts;
+ qstats->backlog += new->backlog_bytes - old->backlog_bytes;
+ qstats->overlimits += new->overlimits - old->overlimits;
+ qstats->drops += new->drops - old->drops;
}
static void
-nfp_abm_reset_root(struct net_device *netdev, struct nfp_abm_link *alink,
- u32 handle, unsigned int qs)
+nfp_abm_stats_red_calculate(struct nfp_alink_xstats *new,
+ struct nfp_alink_xstats *old,
+ struct red_stats *stats)
{
- __nfp_abm_reset_root(netdev, alink, handle, qs, NFP_ABM_LVL_INFINITY);
+ stats->forced_mark += new->ecn_marked - old->ecn_marked;
+ stats->pdrop += new->pdrop - old->pdrop;
}
static int
-nfp_abm_red_find(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
+nfp_abm_red_xstats(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
{
- unsigned int i = TC_H_MIN(opt->parent) - 1;
+ struct nfp_qdisc *qdisc;
- if (opt->parent == TC_H_ROOT)
- i = 0;
- else if (TC_H_MAJ(alink->parent) == TC_H_MAJ(opt->parent))
- i = TC_H_MIN(opt->parent) - 1;
- else
- return -EOPNOTSUPP;
+ nfp_abm_stats_update(alink);
- if (i >= alink->num_qdiscs ||
- opt->handle != alink->red_qdiscs[i].handle)
+ qdisc = nfp_abm_qdisc_find(alink, opt->handle);
+ if (!qdisc || !qdisc->offloaded)
return -EOPNOTSUPP;
- return i;
+ nfp_abm_stats_red_calculate(&qdisc->red.xstats,
+ &qdisc->red.prev_xstats,
+ opt->xstats);
+ qdisc->red.prev_xstats = qdisc->red.xstats;
+ return 0;
}
-static void
-nfp_abm_red_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
- u32 handle)
+static int
+nfp_abm_red_stats(struct nfp_abm_link *alink, u32 handle,
+ struct tc_qopt_offload_stats *stats)
{
- unsigned int i;
+ struct nfp_qdisc *qdisc;
- nfp_abm_qdisc_destroy(netdev, alink, handle);
+ nfp_abm_stats_update(alink);
- for (i = 0; i < alink->num_qdiscs; i++)
- if (handle == alink->red_qdiscs[i].handle)
- break;
- if (i == alink->num_qdiscs)
- return;
+ qdisc = nfp_abm_qdisc_find(alink, handle);
+ if (!qdisc)
+ return -EOPNOTSUPP;
+ /* If the qdisc offload has stopped we may need to adjust the backlog
+ * counters back so carry on even if qdisc is not currently offloaded.
+ */
- if (alink->parent == TC_H_ROOT)
- nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 0);
- else
- memset(&alink->red_qdiscs[i], 0, sizeof(*alink->red_qdiscs));
+ nfp_abm_stats_calculate(&qdisc->red.stats,
+ &qdisc->red.prev_stats,
+ stats->bstats, stats->qstats);
+ qdisc->red.prev_stats = qdisc->red.stats;
- nfp_abm_offload_update(alink->abm);
+ return qdisc->offloaded ? 0 : -EOPNOTSUPP;
}
static bool
@@ -347,20 +517,12 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
struct tc_red_qopt_offload *opt)
{
struct nfp_qdisc *qdisc;
- bool existing;
- int i, err;
int ret;
ret = nfp_abm_qdisc_replace(netdev, alink, NFP_QDISC_RED, opt->parent,
opt->handle, 1, &qdisc);
-
- i = nfp_abm_red_find(alink, opt);
- existing = i >= 0;
-
- if (ret < 0) {
- err = ret;
- goto err_destroy;
- }
+ if (ret < 0)
+ return ret;
/* If limit != 0 child gets reset */
if (opt->set.limit) {
@@ -376,127 +538,11 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
}
qdisc->params_ok = nfp_abm_red_check_params(alink, opt);
- if (qdisc->params_ok) {
+ if (qdisc->params_ok)
qdisc->red.threshold = opt->set.min;
- } else {
- err = -EINVAL;
- goto err_destroy;
- }
-
- if (existing) {
- nfp_abm_offload_update(alink->abm);
- return 0;
- }
-
- if (opt->parent == TC_H_ROOT) {
- i = 0;
- __nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 1, opt->set.min);
- } else if (TC_H_MAJ(alink->parent) == TC_H_MAJ(opt->parent)) {
- i = TC_H_MIN(opt->parent) - 1;
- } else {
- return -EINVAL;
- }
- alink->red_qdiscs[i].handle = opt->handle;
-
- if (opt->parent == TC_H_ROOT)
- err = nfp_abm_ctrl_read_stats(alink,
- &alink->red_qdiscs[i].stats);
- else
- err = nfp_abm_ctrl_read_q_stats(alink, i,
- &alink->red_qdiscs[i].stats);
- if (err)
- goto err_destroy;
-
- if (opt->parent == TC_H_ROOT)
- err = nfp_abm_ctrl_read_xstats(alink,
- &alink->red_qdiscs[i].xstats);
- else
- err = nfp_abm_ctrl_read_q_xstats(alink, i,
- &alink->red_qdiscs[i].xstats);
- if (err)
- goto err_destroy;
-
- alink->red_qdiscs[i].threshold = opt->set.min;
- alink->red_qdiscs[i].stats.backlog_pkts = 0;
- alink->red_qdiscs[i].stats.backlog_bytes = 0;
-
- nfp_abm_offload_update(alink->abm);
-
- return 0;
-err_destroy:
- /* If the qdisc keeps on living, but we can't offload undo changes */
- if (existing) {
- opt->set.qstats->qlen -=
- alink->red_qdiscs[i].stats.backlog_pkts;
- opt->set.qstats->backlog -=
- alink->red_qdiscs[i].stats.backlog_bytes;
- }
- nfp_abm_red_destroy(netdev, alink, opt->handle);
-
- return err;
-}
-
-static void
-nfp_abm_update_stats(struct nfp_alink_stats *new, struct nfp_alink_stats *old,
- struct tc_qopt_offload_stats *stats)
-{
- _bstats_update(stats->bstats, new->tx_bytes - old->tx_bytes,
- new->tx_pkts - old->tx_pkts);
- stats->qstats->qlen += new->backlog_pkts - old->backlog_pkts;
- stats->qstats->backlog += new->backlog_bytes - old->backlog_bytes;
- stats->qstats->overlimits += new->overlimits - old->overlimits;
- stats->qstats->drops += new->drops - old->drops;
-}
-
-static int
-nfp_abm_red_stats(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
-{
- struct nfp_alink_stats *prev_stats;
- struct nfp_alink_stats stats;
- int i, err;
-
- i = nfp_abm_red_find(alink, opt);
- if (i < 0)
- return i;
- prev_stats = &alink->red_qdiscs[i].stats;
-
- if (alink->parent == TC_H_ROOT)
- err = nfp_abm_ctrl_read_stats(alink, &stats);
- else
- err = nfp_abm_ctrl_read_q_stats(alink, i, &stats);
- if (err)
- return err;
-
- nfp_abm_update_stats(&stats, prev_stats, &opt->stats);
- *prev_stats = stats;
-
- return 0;
-}
-
-static int
-nfp_abm_red_xstats(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
-{
- struct nfp_alink_xstats *prev_xstats;
- struct nfp_alink_xstats xstats;
- int i, err;
-
- i = nfp_abm_red_find(alink, opt);
- if (i < 0)
- return i;
- prev_xstats = &alink->red_qdiscs[i].xstats;
-
- if (alink->parent == TC_H_ROOT)
- err = nfp_abm_ctrl_read_xstats(alink, &xstats);
- else
- err = nfp_abm_ctrl_read_q_xstats(alink, i, &xstats);
- if (err)
- return err;
-
- opt->xstats->forced_mark += xstats.ecn_marked - prev_xstats->ecn_marked;
- opt->xstats->pdrop += xstats.pdrop - prev_xstats->pdrop;
-
- *prev_xstats = xstats;
+ if (qdisc->use_cnt == 1)
+ nfp_abm_qdisc_offload_update(alink);
return 0;
}
@@ -508,10 +554,10 @@ int nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
case TC_RED_REPLACE:
return nfp_abm_red_replace(netdev, alink, opt);
case TC_RED_DESTROY:
- nfp_abm_red_destroy(netdev, alink, opt->handle);
+ nfp_abm_qdisc_destroy(netdev, alink, opt->handle);
return 0;
case TC_RED_STATS:
- return nfp_abm_red_stats(alink, opt);
+ return nfp_abm_red_stats(alink, opt->handle, &opt->stats);
case TC_RED_XSTATS:
return nfp_abm_red_xstats(alink, opt);
case TC_RED_GRAFT:
@@ -522,28 +568,6 @@ int nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
}
}
-static int
-nfp_abm_mq_stats(struct nfp_abm_link *alink, struct tc_mq_qopt_offload *opt)
-{
- struct nfp_alink_stats stats;
- unsigned int i;
- int err;
-
- for (i = 0; i < alink->num_qdiscs; i++) {
- if (alink->red_qdiscs[i].handle == TC_H_UNSPEC)
- continue;
-
- err = nfp_abm_ctrl_read_q_stats(alink, i, &stats);
- if (err)
- return err;
-
- nfp_abm_update_stats(&stats, &alink->red_qdiscs[i].stats,
- &opt->stats);
- }
-
- return 0;
-}
-
static int
nfp_abm_mq_create(struct net_device *netdev, struct nfp_abm_link *alink,
struct tc_mq_qopt_offload *opt)
@@ -556,27 +580,63 @@ nfp_abm_mq_create(struct net_device *netdev, struct nfp_abm_link *alink,
&qdisc);
if (ret < 0)
return ret;
+
+ qdisc->params_ok = true;
+ qdisc->offloaded = true;
+ nfp_abm_qdisc_offload_update(alink);
return 0;
}
+static int
+nfp_abm_mq_stats(struct nfp_abm_link *alink, u32 handle,
+ struct tc_qopt_offload_stats *stats)
+{
+ struct nfp_qdisc *qdisc, *red;
+ unsigned int i;
+
+ qdisc = nfp_abm_qdisc_find(alink, handle);
+ if (!qdisc)
+ return -EOPNOTSUPP;
+
+ nfp_abm_stats_update(alink);
+
+ /* MQ stats are summed over the children in the core, so we need
+ * to add up the unreported child values.
+ */
+ memset(&qdisc->mq.stats, 0, sizeof(qdisc->mq.stats));
+ memset(&qdisc->mq.prev_stats, 0, sizeof(qdisc->mq.prev_stats));
+
+ for (i = 0; i < qdisc->num_children; i++) {
+ if (!nfp_abm_qdisc_child_valid(qdisc, i))
+ continue;
+
+ if (!nfp_abm_qdisc_is_red(qdisc->children[i]))
+ continue;
+ red = qdisc->children[i];
+
+ nfp_abm_stats_propagate(&qdisc->mq.stats,
+ &red->red.stats);
+ nfp_abm_stats_propagate(&qdisc->mq.prev_stats,
+ &red->red.prev_stats);
+ }
+
+ nfp_abm_stats_calculate(&qdisc->mq.stats, &qdisc->mq.prev_stats,
+ stats->bstats, stats->qstats);
+
+ return qdisc->offloaded ? 0 : -EOPNOTSUPP;
+}
+
int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
struct tc_mq_qopt_offload *opt)
{
switch (opt->command) {
case TC_MQ_CREATE:
- nfp_abm_reset_root(netdev, alink, opt->handle,
- alink->total_queues);
- nfp_abm_offload_update(alink->abm);
return nfp_abm_mq_create(netdev, alink, opt);
case TC_MQ_DESTROY:
nfp_abm_qdisc_destroy(netdev, alink, opt->handle);
- if (opt->handle == alink->parent) {
- nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 0);
- nfp_abm_offload_update(alink->abm);
- }
return 0;
case TC_MQ_STATS:
- return nfp_abm_mq_stats(alink, opt);
+ return nfp_abm_mq_stats(alink, opt->handle, &opt->stats);
case TC_MQ_GRAFT:
return nfp_abm_qdisc_graft(alink, opt->handle,
opt->graft_params.child_handle,
@@ -597,5 +657,7 @@ int nfp_abm_setup_root(struct net_device *netdev, struct nfp_abm_link *alink,
if (alink->root_qdisc)
alink->root_qdisc->use_cnt++;
+ nfp_abm_qdisc_offload_update(alink);
+
return 0;
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next v2] geneve: Use empty braces for addr6 initializer
From: Stefano Brivio @ 2018-11-13 9:02 UTC (permalink / raw)
To: Nathan Chancellor
Cc: David S. Miller, Sabrina Dubroca, netdev, linux-kernel,
Joe Perches
In-Reply-To: <20181113061146.12182-1-natechancellor@gmail.com>
On Mon, 12 Nov 2018 23:11:47 -0700
Nathan Chancellor <natechancellor@gmail.com> wrote:
> Clang warns:
>
> drivers/net/geneve.c:428:29: error: suggest braces around initialization
> of subobject [-Werror,-Wmissing-braces]
> struct in6_addr addr6 = { 0 };
> ^
> {}
>
> Most initializations of structs in the kernel seem to use this format.
>
> Fixes: a07966447f39 ("geneve: ICMP error lookup handler")
> Suggested-by: Joe Perches <joe@perches.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Also,
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
--
Stefano
^ permalink raw reply
* Hello, this is the second times am sending you this mail and you refused to reply to my email why?
From: KATIE HIGGINS @ 2018-11-12 23:09 UTC (permalink / raw)
^ permalink raw reply
* Re: [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
From: Martin Lau @ 2018-11-12 23:10 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Stanislav Fomichev, netdev@vger.kernel.org,
linux-kselftest@vger.kernel.org, ast@kernel.org,
daniel@iogearbox.net, shuah@kernel.org,
jakub.kicinski@netronome.com, quentin.monnet@netronome.com,
Roman Gushchin, jiong.wang@netronome.com,
bhole_prashant_q7@lab.ntt.co.jp, john.fastabend@gmail.com,
jbenc@redhat.com, treeze.taeung@gmail.com, Yonghong Song,
Okash Khawaja, "sandipan@linux
In-Reply-To: <20181112221011.qqqkexvr3mw5clnr@mini-arch>
On Mon, Nov 12, 2018 at 02:10:11PM -0800, Stanislav Fomichev wrote:
> On 11/12, Martin Lau wrote:
> > On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
> > [ ... ]
> > > @@ -1918,23 +2160,20 @@ void *bpf_object__priv(struct bpf_object *obj)
> > > }
> > >
> > > static struct bpf_program *
> > > -__bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> > > {
> > > - size_t idx;
> > > + ssize_t idx;
> > >
> > > if (!obj->programs)
> > > return NULL;
> > > - /* First handler */
> > > - if (prev == NULL)
> > > - return &obj->programs[0];
> > >
> > > - if (prev->obj != obj) {
> > > + if (p->obj != obj) {
> > > pr_warning("error: program handler doesn't match object\n");
> > > return NULL;
> > > }
> > >
> > > - idx = (prev - obj->programs) + 1;
> > > - if (idx >= obj->nr_programs)
> > > + idx = (p - obj->programs) + i;
> > > + if (idx >= obj->nr_programs || idx < 0)
> > > return NULL;
> > > return &obj->programs[idx];
> > > }
> > > @@ -1944,8 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > {
> > > struct bpf_program *prog = prev;
> > >
> > > + if (prev == NULL)
> > > + return obj->programs;
> > > +
> > This patch breaks the behavior introduced in
> > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > "Make bpf_program__next() skip over '.text' section if object file
> > has pseudo calls. The '.text' section is hardly a program in that
> > case, it's more of a storage for code of functions other than main."
> >
> > For example, the userspace could have been doing:
> > prog = bpf_program__next(NULL, obj);
> > bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> > bpf_object__load(obj);
> >
> > For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> > the prog returned by bpf_program__next() could be in ".text" instead of
> > the main bpf program. The next bpf_program__set_type() has
> > no effect to the main program. The following bpf_object__load()
> > will catch user in surprise with the main bpf prog in
> > the wrong BPF_PROG_TYPE.
>
> Will something like the following fix your concern? (plus, assuming the
> same for prev):
>
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> {
> struct bpf_program *prog = prev;
>
> - if (prev == NULL)
> - return obj->programs;
> + if (prev == NULL) {
> + prog = obj->programs;
> + if (!prog || !bpf_program__is_function_storage(prog, obj))
> + return prog;
> + }
>
> do {
> prog = __bpf_program__iter(prog, obj, 1);
>
> Any suggestions for a better way to do it?
I think that would work. The bpf_program__prev() will need the same
treatment though...
Here is my mostly untested fix to unblock my other dev works. It moves
the very first NULL check back to __bpf_program__iter():
>From de1c89ae1768e756825a6874268b5b1686695c93 Mon Sep 17 00:00:00 2001
From: Martin KaFai Lau <kafai@fb.com>
Date: Mon, 12 Nov 2018 14:52:39 -0800
Subject: [PATCH] bpf: libbpf: Fix bpf_program__next() API
This patch restores the behavior in
commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
such that bpf_program__next() does not return pseudo programs in ".text".
Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e827542ffa3a..a01eb9584e52 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
}
static struct bpf_program *
-__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
+__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
{
+ size_t nr_programs = obj->nr_programs;
ssize_t idx;
- if (!obj->programs)
+ if (!nr_programs)
return NULL;
+ if (!p)
+ /* Iter from the beginning */
+ return forward ? &obj->programs[0] :
+ &obj->programs[nr_programs - 1];
+
if (p->obj != obj) {
pr_warning("error: program handler doesn't match object\n");
return NULL;
}
- idx = (p - obj->programs) + i;
+ idx = (p - obj->programs) + (forward ? 1 : -1);
if (idx >= obj->nr_programs || idx < 0)
return NULL;
return &obj->programs[idx];
@@ -2216,11 +2222,8 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
{
struct bpf_program *prog = prev;
- if (prev == NULL)
- return obj->programs;
-
do {
- prog = __bpf_program__iter(prog, obj, 1);
+ prog = __bpf_program__iter(prog, obj, true);
} while (prog && bpf_program__is_function_storage(prog, obj));
return prog;
@@ -2231,14 +2234,8 @@ bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
{
struct bpf_program *prog = next;
- if (next == NULL) {
- if (!obj->nr_programs)
- return NULL;
- return obj->programs + obj->nr_programs - 1;
- }
-
do {
- prog = __bpf_program__iter(prog, obj, -1);
+ prog = __bpf_program__iter(prog, obj, false);
} while (prog && bpf_program__is_function_storage(prog, obj));
return prog;
--
2.17.1
>
> > > do {
> > > - prog = __bpf_program__next(prog, obj);
> > > + prog = __bpf_program__iter(prog, obj, 1);
> > > + } while (prog && bpf_program__is_function_storage(prog, obj));
> > > +
> > > + return prog;
> > > +}
> > > +
> > > +struct bpf_program *
> > > +bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
> > > +{
> > > + struct bpf_program *prog = next;
> > > +
> > > + if (next == NULL) {
> > > + if (!obj->nr_programs)
> > > + return NULL;
> > > + return obj->programs + obj->nr_programs - 1;
> > > + }
> > > +
> > > + do {
> > > + prog = __bpf_program__iter(prog, obj, -1);
> > > } while (prog && bpf_program__is_function_storage(prog, obj));
> > >
> > > return prog;
^ permalink raw reply related
* Re: [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
From: Stanislav Fomichev @ 2018-11-12 23:29 UTC (permalink / raw)
To: Martin Lau
Cc: Stanislav Fomichev, netdev@vger.kernel.org,
linux-kselftest@vger.kernel.org, ast@kernel.org,
daniel@iogearbox.net, shuah@kernel.org,
jakub.kicinski@netronome.com, quentin.monnet@netronome.com,
Roman Gushchin, jiong.wang@netronome.com,
bhole_prashant_q7@lab.ntt.co.jp, john.fastabend@gmail.com,
jbenc@redhat.com, treeze.taeung@gmail.com, Yonghong Song,
Okash Khawaja, "sandipan@linux
In-Reply-To: <20181112231040.l4qdresmbhcyf7vs@kafai-mbp.dhcp.thefacebook.com>
On 11/12, Martin Lau wrote:
> On Mon, Nov 12, 2018 at 02:10:11PM -0800, Stanislav Fomichev wrote:
> > On 11/12, Martin Lau wrote:
> > > On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
> > > [ ... ]
> > > > @@ -1918,23 +2160,20 @@ void *bpf_object__priv(struct bpf_object *obj)
> > > > }
> > > >
> > > > static struct bpf_program *
> > > > -__bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > > +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> > > > {
> > > > - size_t idx;
> > > > + ssize_t idx;
> > > >
> > > > if (!obj->programs)
> > > > return NULL;
> > > > - /* First handler */
> > > > - if (prev == NULL)
> > > > - return &obj->programs[0];
> > > >
> > > > - if (prev->obj != obj) {
> > > > + if (p->obj != obj) {
> > > > pr_warning("error: program handler doesn't match object\n");
> > > > return NULL;
> > > > }
> > > >
> > > > - idx = (prev - obj->programs) + 1;
> > > > - if (idx >= obj->nr_programs)
> > > > + idx = (p - obj->programs) + i;
> > > > + if (idx >= obj->nr_programs || idx < 0)
> > > > return NULL;
> > > > return &obj->programs[idx];
> > > > }
> > > > @@ -1944,8 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > > {
> > > > struct bpf_program *prog = prev;
> > > >
> > > > + if (prev == NULL)
> > > > + return obj->programs;
> > > > +
> > > This patch breaks the behavior introduced in
> > > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > > "Make bpf_program__next() skip over '.text' section if object file
> > > has pseudo calls. The '.text' section is hardly a program in that
> > > case, it's more of a storage for code of functions other than main."
> > >
> > > For example, the userspace could have been doing:
> > > prog = bpf_program__next(NULL, obj);
> > > bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> > > bpf_object__load(obj);
> > >
> > > For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> > > the prog returned by bpf_program__next() could be in ".text" instead of
> > > the main bpf program. The next bpf_program__set_type() has
> > > no effect to the main program. The following bpf_object__load()
> > > will catch user in surprise with the main bpf prog in
> > > the wrong BPF_PROG_TYPE.
> >
> > Will something like the following fix your concern? (plus, assuming the
> > same for prev):
> >
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > {
> > struct bpf_program *prog = prev;
> >
> > - if (prev == NULL)
> > - return obj->programs;
> > + if (prev == NULL) {
> > + prog = obj->programs;
> > + if (!prog || !bpf_program__is_function_storage(prog, obj))
> > + return prog;
> > + }
> >
> > do {
> > prog = __bpf_program__iter(prog, obj, 1);
> >
> > Any suggestions for a better way to do it?
> I think that would work. The bpf_program__prev() will need the same
> treatment though...
>
> Here is my mostly untested fix to unblock my other dev works. It moves
> the very first NULL check back to __bpf_program__iter():
I like your version and it works with my simple flow dissector test :-)
Thanks for spotting and fixing it!
> From de1c89ae1768e756825a6874268b5b1686695c93 Mon Sep 17 00:00:00 2001
> From: Martin KaFai Lau <kafai@fb.com>
> Date: Mon, 12 Nov 2018 14:52:39 -0800
> Subject: [PATCH] bpf: libbpf: Fix bpf_program__next() API
>
> This patch restores the behavior in
> commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> such that bpf_program__next() does not return pseudo programs in ".text".
>
> Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
> 1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index e827542ffa3a..a01eb9584e52 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
> }
>
> static struct bpf_program *
> -__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
> {
> + size_t nr_programs = obj->nr_programs;
> ssize_t idx;
>
> - if (!obj->programs)
> + if (!nr_programs)
> return NULL;
>
> + if (!p)
> + /* Iter from the beginning */
> + return forward ? &obj->programs[0] :
> + &obj->programs[nr_programs - 1];
> +
> if (p->obj != obj) {
> pr_warning("error: program handler doesn't match object\n");
> return NULL;
> }
>
> - idx = (p - obj->programs) + i;
> + idx = (p - obj->programs) + (forward ? 1 : -1);
> if (idx >= obj->nr_programs || idx < 0)
> return NULL;
> return &obj->programs[idx];
> @@ -2216,11 +2222,8 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> {
> struct bpf_program *prog = prev;
>
> - if (prev == NULL)
> - return obj->programs;
> -
> do {
> - prog = __bpf_program__iter(prog, obj, 1);
> + prog = __bpf_program__iter(prog, obj, true);
> } while (prog && bpf_program__is_function_storage(prog, obj));
>
> return prog;
> @@ -2231,14 +2234,8 @@ bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
> {
> struct bpf_program *prog = next;
>
> - if (next == NULL) {
> - if (!obj->nr_programs)
> - return NULL;
> - return obj->programs + obj->nr_programs - 1;
> - }
> -
> do {
> - prog = __bpf_program__iter(prog, obj, -1);
> + prog = __bpf_program__iter(prog, obj, false);
> } while (prog && bpf_program__is_function_storage(prog, obj));
>
> return prog;
> --
> 2.17.1
>
> >
> > > > do {
> > > > - prog = __bpf_program__next(prog, obj);
> > > > + prog = __bpf_program__iter(prog, obj, 1);
> > > > + } while (prog && bpf_program__is_function_storage(prog, obj));
> > > > +
> > > > + return prog;
> > > > +}
> > > > +
> > > > +struct bpf_program *
> > > > +bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
> > > > +{
> > > > + struct bpf_program *prog = next;
> > > > +
> > > > + if (next == NULL) {
> > > > + if (!obj->nr_programs)
> > > > + return NULL;
> > > > + return obj->programs + obj->nr_programs - 1;
> > > > + }
> > > > +
> > > > + do {
> > > > + prog = __bpf_program__iter(prog, obj, -1);
> > > > } while (prog && bpf_program__is_function_storage(prog, obj));
> > > >
> > > > return prog;
^ permalink raw reply
* [PATCH][bpf-next] bpf: fix null pointer dereference on pointer offload
From: Colin King @ 2018-11-13 9:29 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, netdev; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Pointer offload is being null checked however the following statement
dereferences the potentially null pointer offload when assigning
offload->dev_state. Fix this by only assigning it if offload is not
null.
Detected by CoverityScan, CID#1475437 ("Dereference after null check")
Fixes: 00db12c3d141 ("bpf: call verifier_prep from its callback in struct bpf_offload_dev")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
kernel/bpf/offload.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index 52c5617e3716..54cf2b9c44a4 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -130,9 +130,10 @@ int bpf_prog_offload_verifier_prep(struct bpf_prog *prog)
down_read(&bpf_devs_lock);
offload = prog->aux->offload;
- if (offload)
+ if (offload) {
ret = offload->offdev->ops->prepare(prog);
- offload->dev_state = !ret;
+ offload->dev_state = !ret;
+ }
up_read(&bpf_devs_lock);
return ret;
--
2.19.1
^ permalink raw reply related
* Re: [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
From: Martin Lau @ 2018-11-12 23:41 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Stanislav Fomichev, netdev@vger.kernel.org,
linux-kselftest@vger.kernel.org, ast@kernel.org,
daniel@iogearbox.net, shuah@kernel.org,
jakub.kicinski@netronome.com, quentin.monnet@netronome.com,
Roman Gushchin, jiong.wang@netronome.com,
bhole_prashant_q7@lab.ntt.co.jp, john.fastabend@gmail.com,
jbenc@redhat.com, treeze.taeung@gmail.com, Yonghong Song,
Okash Khawaja, "sandipan@linux
In-Reply-To: <20181112232925.74cajfncscaf7sc6@mini-arch>
On Mon, Nov 12, 2018 at 03:29:25PM -0800, Stanislav Fomichev wrote:
> On 11/12, Martin Lau wrote:
> > On Mon, Nov 12, 2018 at 02:10:11PM -0800, Stanislav Fomichev wrote:
> > > On 11/12, Martin Lau wrote:
> > > > On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
> > > > [ ... ]
> > > > > @@ -1918,23 +2160,20 @@ void *bpf_object__priv(struct bpf_object *obj)
> > > > > }
> > > > >
> > > > > static struct bpf_program *
> > > > > -__bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > > > +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> > > > > {
> > > > > - size_t idx;
> > > > > + ssize_t idx;
> > > > >
> > > > > if (!obj->programs)
> > > > > return NULL;
> > > > > - /* First handler */
> > > > > - if (prev == NULL)
> > > > > - return &obj->programs[0];
> > > > >
> > > > > - if (prev->obj != obj) {
> > > > > + if (p->obj != obj) {
> > > > > pr_warning("error: program handler doesn't match object\n");
> > > > > return NULL;
> > > > > }
> > > > >
> > > > > - idx = (prev - obj->programs) + 1;
> > > > > - if (idx >= obj->nr_programs)
> > > > > + idx = (p - obj->programs) + i;
> > > > > + if (idx >= obj->nr_programs || idx < 0)
> > > > > return NULL;
> > > > > return &obj->programs[idx];
> > > > > }
> > > > > @@ -1944,8 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > > > {
> > > > > struct bpf_program *prog = prev;
> > > > >
> > > > > + if (prev == NULL)
> > > > > + return obj->programs;
> > > > > +
> > > > This patch breaks the behavior introduced in
> > > > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > > > "Make bpf_program__next() skip over '.text' section if object file
> > > > has pseudo calls. The '.text' section is hardly a program in that
> > > > case, it's more of a storage for code of functions other than main."
> > > >
> > > > For example, the userspace could have been doing:
> > > > prog = bpf_program__next(NULL, obj);
> > > > bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> > > > bpf_object__load(obj);
> > > >
> > > > For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> > > > the prog returned by bpf_program__next() could be in ".text" instead of
> > > > the main bpf program. The next bpf_program__set_type() has
> > > > no effect to the main program. The following bpf_object__load()
> > > > will catch user in surprise with the main bpf prog in
> > > > the wrong BPF_PROG_TYPE.
> > >
> > > Will something like the following fix your concern? (plus, assuming the
> > > same for prev):
> > >
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > {
> > > struct bpf_program *prog = prev;
> > >
> > > - if (prev == NULL)
> > > - return obj->programs;
> > > + if (prev == NULL) {
> > > + prog = obj->programs;
> > > + if (!prog || !bpf_program__is_function_storage(prog, obj))
> > > + return prog;
> > > + }
> > >
> > > do {
> > > prog = __bpf_program__iter(prog, obj, 1);
> > >
> > > Any suggestions for a better way to do it?
> > I think that would work. The bpf_program__prev() will need the same
> > treatment though...
> >
> > Here is my mostly untested fix to unblock my other dev works. It moves
> > the very first NULL check back to __bpf_program__iter():
> I like your version and it works with my simple flow dissector test :-)
Great. I will send out an offical patch.
> Thanks for spotting and fixing it!
>
>
> > From de1c89ae1768e756825a6874268b5b1686695c93 Mon Sep 17 00:00:00 2001
> > From: Martin KaFai Lau <kafai@fb.com>
> > Date: Mon, 12 Nov 2018 14:52:39 -0800
> > Subject: [PATCH] bpf: libbpf: Fix bpf_program__next() API
> >
> > This patch restores the behavior in
> > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > such that bpf_program__next() does not return pseudo programs in ".text".
> >
> > Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
> > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> > ---
> > tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
> > 1 file changed, 11 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index e827542ffa3a..a01eb9584e52 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
> > }
> >
> > static struct bpf_program *
> > -__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> > +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
> > {
> > + size_t nr_programs = obj->nr_programs;
> > ssize_t idx;
> >
> > - if (!obj->programs)
> > + if (!nr_programs)
> > return NULL;
> >
> > + if (!p)
> > + /* Iter from the beginning */
> > + return forward ? &obj->programs[0] :
> > + &obj->programs[nr_programs - 1];
> > +
> > if (p->obj != obj) {
> > pr_warning("error: program handler doesn't match object\n");
> > return NULL;
> > }
> >
> > - idx = (p - obj->programs) + i;
> > + idx = (p - obj->programs) + (forward ? 1 : -1);
> > if (idx >= obj->nr_programs || idx < 0)
> > return NULL;
> > return &obj->programs[idx];
> > @@ -2216,11 +2222,8 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > {
> > struct bpf_program *prog = prev;
> >
> > - if (prev == NULL)
> > - return obj->programs;
> > -
> > do {
> > - prog = __bpf_program__iter(prog, obj, 1);
> > + prog = __bpf_program__iter(prog, obj, true);
> > } while (prog && bpf_program__is_function_storage(prog, obj));
> >
> > return prog;
> > @@ -2231,14 +2234,8 @@ bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
> > {
> > struct bpf_program *prog = next;
> >
> > - if (next == NULL) {
> > - if (!obj->nr_programs)
> > - return NULL;
> > - return obj->programs + obj->nr_programs - 1;
> > - }
> > -
> > do {
> > - prog = __bpf_program__iter(prog, obj, -1);
> > + prog = __bpf_program__iter(prog, obj, false);
> > } while (prog && bpf_program__is_function_storage(prog, obj));
> >
> > return prog;
> > --
> > 2.17.1
> >
> > >
> > > > > do {
> > > > > - prog = __bpf_program__next(prog, obj);
> > > > > + prog = __bpf_program__iter(prog, obj, 1);
> > > > > + } while (prog && bpf_program__is_function_storage(prog, obj));
> > > > > +
> > > > > + return prog;
> > > > > +}
> > > > > +
> > > > > +struct bpf_program *
> > > > > +bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
> > > > > +{
> > > > > + struct bpf_program *prog = next;
> > > > > +
> > > > > + if (next == NULL) {
> > > > > + if (!obj->nr_programs)
> > > > > + return NULL;
> > > > > + return obj->programs + obj->nr_programs - 1;
> > > > > + }
> > > > > +
> > > > > + do {
> > > > > + prog = __bpf_program__iter(prog, obj, -1);
> > > > > } while (prog && bpf_program__is_function_storage(prog, obj));
> > > > >
> > > > > return prog;
^ permalink raw reply
* [PATCH bpf-next] bpf: libbpf: Fix bpf_program__next() API
From: Martin KaFai Lau @ 2018-11-12 23:44 UTC (permalink / raw)
To: netdev
Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team,
Stanislav Fomichev, Stanislav Fomichev
This patch restores the behavior in
commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs")
such that bpf_program__next() does not return pseudo programs in ".text".
Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e827542ffa3a..a01eb9584e52 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
}
static struct bpf_program *
-__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
+__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
{
+ size_t nr_programs = obj->nr_programs;
ssize_t idx;
- if (!obj->programs)
+ if (!nr_programs)
return NULL;
+ if (!p)
+ /* Iter from the beginning */
+ return forward ? &obj->programs[0] :
+ &obj->programs[nr_programs - 1];
+
if (p->obj != obj) {
pr_warning("error: program handler doesn't match object\n");
return NULL;
}
- idx = (p - obj->programs) + i;
+ idx = (p - obj->programs) + (forward ? 1 : -1);
if (idx >= obj->nr_programs || idx < 0)
return NULL;
return &obj->programs[idx];
@@ -2216,11 +2222,8 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
{
struct bpf_program *prog = prev;
- if (prev == NULL)
- return obj->programs;
-
do {
- prog = __bpf_program__iter(prog, obj, 1);
+ prog = __bpf_program__iter(prog, obj, true);
} while (prog && bpf_program__is_function_storage(prog, obj));
return prog;
@@ -2231,14 +2234,8 @@ bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
{
struct bpf_program *prog = next;
- if (next == NULL) {
- if (!obj->nr_programs)
- return NULL;
- return obj->programs + obj->nr_programs - 1;
- }
-
do {
- prog = __bpf_program__iter(prog, obj, -1);
+ prog = __bpf_program__iter(prog, obj, false);
} while (prog && bpf_program__is_function_storage(prog, obj));
return prog;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH bpf-next] selftests/bpf: Fix uninitialized duration warning
From: Martin Lau @ 2018-11-12 23:51 UTC (permalink / raw)
To: Joe Stringer; +Cc: daniel@iogearbox.net, netdev@vger.kernel.org
In-Reply-To: <20181109181816.22622-1-joe@wand.net.nz>
On Fri, Nov 09, 2018 at 10:18:16AM -0800, Joe Stringer wrote:
> Daniel Borkmann reports:
>
> test_progs.c: In function ‘main’:
> test_progs.c:81:3: warning: ‘duration’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> printf("%s:PASS:%s %d nsec\n", __func__, tag, duration);\
> ^~~~~~
> test_progs.c:1706:8: note: ‘duration’ was declared here
> __u32 duration;
> ^~~~~~~~
>
> Signed-off-by: Joe Stringer <joe@wand.net.nz>
I can repro with my locally compiled gcc 7.3. It fixed the warning.
Acked-by: Martin KaFai Lau <kafai@fb.com>
> ---
>
> I'm actually not able to reproduce this with GCC 7.3 or 8.2, so I'll
> rely on review to establish that this patch works as intended.
> ---
> tools/testing/selftests/bpf/test_progs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 2d3c04f45530..c1e688f61061 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -1703,7 +1703,7 @@ static void test_reference_tracking()
> const char *file = "./test_sk_lookup_kern.o";
> struct bpf_object *obj;
> struct bpf_program *prog;
> - __u32 duration;
> + __u32 duration = 0;
> int err = 0;
>
> obj = bpf_object__open(file);
> --
> 2.17.1
>
^ permalink raw reply
* Re: [net-next PATCH v3] net: sched: cls_flower: Classify packets using port ranges
From: Nambiar, Amritha @ 2018-11-12 23:54 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, jakub.kicinski, sridhar.samudrala, jhs,
xiyou.wangcong
In-Reply-To: <20181110071458.GA2019@nanopsycho.orion>
On 11/9/2018 11:14 PM, Jiri Pirko wrote:
> Sat, Nov 10, 2018 at 01:11:10AM CET, amritha.nambiar@intel.com wrote:
>
> [...]
>
>> @@ -1026,8 +1122,7 @@ static void fl_init_dissector(struct flow_dissector *dissector,
>> FLOW_DISSECTOR_KEY_IPV4_ADDRS, ipv4);
>> FL_KEY_SET_IF_MASKED(mask, keys, cnt,
>> FLOW_DISSECTOR_KEY_IPV6_ADDRS, ipv6);
>> - FL_KEY_SET_IF_MASKED(mask, keys, cnt,
>> - FLOW_DISSECTOR_KEY_PORTS, tp);
>> + FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_PORTS, tp);
>
> You still need to set the key under a condition. Something like:
> if (FL_KEY_IS_MASKED(mask, tp) ||
> FL_KEY_IS_MASKED(mask, tp_min) ||
> FL_KEY_IS_MASKED(mask, tp_max)
> FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_PORTS, tp);
>
Yes, will do. Thanks!
>
>> FL_KEY_SET_IF_MASKED(mask, keys, cnt,
>> FLOW_DISSECTOR_KEY_IP, ip);
>> FL_KEY_SET_IF_MASKED(mask, keys, cnt,
>
> [...]
>
^ permalink raw reply
* [net-next PATCH v4] net: sched: cls_flower: Classify packets using port ranges
From: Amritha Nambiar @ 2018-11-13 0:15 UTC (permalink / raw)
To: netdev, davem
Cc: jakub.kicinski, amritha.nambiar, sridhar.samudrala, jhs,
xiyou.wangcong, jiri
Added support in tc flower for filtering based on port ranges.
Example:
1. Match on a port range:
-------------------------
$ tc filter add dev enp4s0 protocol ip parent ffff:\
prio 1 flower ip_proto tcp dst_port range 20-30 skip_hw\
action drop
$ tc -s filter show dev enp4s0 parent ffff:
filter protocol ip pref 1 flower chain 0
filter protocol ip pref 1 flower chain 0 handle 0x1
eth_type ipv4
ip_proto tcp
dst_port range 20-30
skip_hw
not_in_hw
action order 1: gact action drop
random type none pass val 0
index 1 ref 1 bind 1 installed 85 sec used 3 sec
Action statistics:
Sent 460 bytes 10 pkt (dropped 10, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
2. Match on IP address and port range:
--------------------------------------
$ tc filter add dev enp4s0 protocol ip parent ffff:\
prio 1 flower dst_ip 192.168.1.1 ip_proto tcp dst_port range 100-200\
skip_hw action drop
$ tc -s filter show dev enp4s0 parent ffff:
filter protocol ip pref 1 flower chain 0 handle 0x2
eth_type ipv4
ip_proto tcp
dst_ip 192.168.1.1
dst_port range 100-200
skip_hw
not_in_hw
action order 1: gact action drop
random type none pass val 0
index 2 ref 1 bind 1 installed 58 sec used 2 sec
Action statistics:
Sent 920 bytes 20 pkt (dropped 20, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
v4:
1. Added condition before setting port key.
2. Organized setting and dumping port range keys into functions
and added validation of input range.
v3:
1. Moved new fields in UAPI enum to the end of enum.
2. Removed couple of empty lines.
v2:
Addressed Jiri's comments:
1. Added separate functions for dst and src comparisons.
2. Removed endpoint enum.
3. Added new bit TCA_FLOWER_FLAGS_RANGE to decide normal/range
lookup.
4. Cleaned up fl_lookup function.
Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
---
include/uapi/linux/pkt_cls.h | 7 ++
net/sched/cls_flower.c | 155 ++++++++++++++++++++++++++++++++++++++++--
2 files changed, 156 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 401d0c1..95d0db2 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -485,6 +485,11 @@ enum {
TCA_FLOWER_IN_HW_COUNT,
+ TCA_FLOWER_KEY_PORT_SRC_MIN, /* be16 */
+ TCA_FLOWER_KEY_PORT_SRC_MAX, /* be16 */
+ TCA_FLOWER_KEY_PORT_DST_MIN, /* be16 */
+ TCA_FLOWER_KEY_PORT_DST_MAX, /* be16 */
+
__TCA_FLOWER_MAX,
};
@@ -518,6 +523,8 @@ enum {
TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
};
+#define TCA_FLOWER_MASK_FLAGS_RANGE (1 << 0) /* Range-based match */
+
/* Match-all classifier */
enum {
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index c6c3278..85e9f8e 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -55,6 +55,8 @@ struct fl_flow_key {
struct flow_dissector_key_ip ip;
struct flow_dissector_key_ip enc_ip;
struct flow_dissector_key_enc_opts enc_opts;
+ struct flow_dissector_key_ports tp_min;
+ struct flow_dissector_key_ports tp_max;
} __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
struct fl_flow_mask_range {
@@ -65,6 +67,7 @@ struct fl_flow_mask_range {
struct fl_flow_mask {
struct fl_flow_key key;
struct fl_flow_mask_range range;
+ u32 flags;
struct rhash_head ht_node;
struct rhashtable ht;
struct rhashtable_params filter_ht_params;
@@ -179,13 +182,89 @@ static void fl_clear_masked_range(struct fl_flow_key *key,
memset(fl_key_get_start(key, mask), 0, fl_mask_range(mask));
}
-static struct cls_fl_filter *fl_lookup(struct fl_flow_mask *mask,
- struct fl_flow_key *mkey)
+static bool fl_range_port_dst_cmp(struct cls_fl_filter *filter,
+ struct fl_flow_key *key,
+ struct fl_flow_key *mkey)
+{
+ __be16 min_mask, max_mask, min_val, max_val;
+
+ min_mask = htons(filter->mask->key.tp_min.dst);
+ max_mask = htons(filter->mask->key.tp_max.dst);
+ min_val = htons(filter->key.tp_min.dst);
+ max_val = htons(filter->key.tp_max.dst);
+
+ if (min_mask && max_mask) {
+ if (htons(key->tp.dst) < min_val ||
+ htons(key->tp.dst) > max_val)
+ return false;
+
+ /* skb does not have min and max values */
+ mkey->tp_min.dst = filter->mkey.tp_min.dst;
+ mkey->tp_max.dst = filter->mkey.tp_max.dst;
+ }
+ return true;
+}
+
+static bool fl_range_port_src_cmp(struct cls_fl_filter *filter,
+ struct fl_flow_key *key,
+ struct fl_flow_key *mkey)
+{
+ __be16 min_mask, max_mask, min_val, max_val;
+
+ min_mask = htons(filter->mask->key.tp_min.src);
+ max_mask = htons(filter->mask->key.tp_max.src);
+ min_val = htons(filter->key.tp_min.src);
+ max_val = htons(filter->key.tp_max.src);
+
+ if (min_mask && max_mask) {
+ if (htons(key->tp.src) < min_val ||
+ htons(key->tp.src) > max_val)
+ return false;
+
+ /* skb does not have min and max values */
+ mkey->tp_min.src = filter->mkey.tp_min.src;
+ mkey->tp_max.src = filter->mkey.tp_max.src;
+ }
+ return true;
+}
+
+static struct cls_fl_filter *__fl_lookup(struct fl_flow_mask *mask,
+ struct fl_flow_key *mkey)
{
return rhashtable_lookup_fast(&mask->ht, fl_key_get_start(mkey, mask),
mask->filter_ht_params);
}
+static struct cls_fl_filter *fl_lookup_range(struct fl_flow_mask *mask,
+ struct fl_flow_key *mkey,
+ struct fl_flow_key *key)
+{
+ struct cls_fl_filter *filter, *f;
+
+ list_for_each_entry_rcu(filter, &mask->filters, list) {
+ if (!fl_range_port_dst_cmp(filter, key, mkey))
+ continue;
+
+ if (!fl_range_port_src_cmp(filter, key, mkey))
+ continue;
+
+ f = __fl_lookup(mask, mkey);
+ if (f)
+ return f;
+ }
+ return NULL;
+}
+
+static struct cls_fl_filter *fl_lookup(struct fl_flow_mask *mask,
+ struct fl_flow_key *mkey,
+ struct fl_flow_key *key)
+{
+ if ((mask->flags & TCA_FLOWER_MASK_FLAGS_RANGE))
+ return fl_lookup_range(mask, mkey, key);
+
+ return __fl_lookup(mask, mkey);
+}
+
static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
struct tcf_result *res)
{
@@ -208,7 +287,7 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
fl_set_masked_key(&skb_mkey, &skb_key, mask);
- f = fl_lookup(mask, &skb_mkey);
+ f = fl_lookup(mask, &skb_mkey, &skb_key);
if (f && !tc_skip_sw(f->flags)) {
*res = f->res;
return tcf_exts_exec(skb, &f->exts, res);
@@ -514,6 +593,31 @@ static void fl_set_key_val(struct nlattr **tb,
memcpy(mask, nla_data(tb[mask_type]), len);
}
+static int fl_set_key_port_range(struct nlattr **tb, struct fl_flow_key *key,
+ struct fl_flow_key *mask)
+{
+ fl_set_key_val(tb, &key->tp_min.dst,
+ TCA_FLOWER_KEY_PORT_DST_MIN, &mask->tp_min.dst,
+ TCA_FLOWER_UNSPEC, sizeof(key->tp_min.dst));
+ fl_set_key_val(tb, &key->tp_max.dst,
+ TCA_FLOWER_KEY_PORT_DST_MAX, &mask->tp_max.dst,
+ TCA_FLOWER_UNSPEC, sizeof(key->tp_max.dst));
+ fl_set_key_val(tb, &key->tp_min.src,
+ TCA_FLOWER_KEY_PORT_SRC_MIN, &mask->tp_min.src,
+ TCA_FLOWER_UNSPEC, sizeof(key->tp_min.src));
+ fl_set_key_val(tb, &key->tp_max.src,
+ TCA_FLOWER_KEY_PORT_SRC_MAX, &mask->tp_max.src,
+ TCA_FLOWER_UNSPEC, sizeof(key->tp_max.src));
+
+ if ((mask->tp_min.dst && mask->tp_max.dst &&
+ htons(key->tp_max.dst) <= htons(key->tp_min.dst)) ||
+ (mask->tp_min.src && mask->tp_max.src &&
+ htons(key->tp_max.src) <= htons(key->tp_min.src)))
+ return -EINVAL;
+
+ return 0;
+}
+
static int fl_set_key_mpls(struct nlattr **tb,
struct flow_dissector_key_mpls *key_val,
struct flow_dissector_key_mpls *key_mask)
@@ -921,6 +1025,14 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
sizeof(key->arp.tha));
}
+ if (key->basic.ip_proto == IPPROTO_TCP ||
+ key->basic.ip_proto == IPPROTO_UDP ||
+ key->basic.ip_proto == IPPROTO_SCTP) {
+ ret = fl_set_key_port_range(tb, key, mask);
+ if (ret)
+ return ret;
+ }
+
if (tb[TCA_FLOWER_KEY_ENC_IPV4_SRC] ||
tb[TCA_FLOWER_KEY_ENC_IPV4_DST]) {
key->enc_control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
@@ -1038,8 +1150,9 @@ static void fl_init_dissector(struct flow_dissector *dissector,
FLOW_DISSECTOR_KEY_IPV4_ADDRS, ipv4);
FL_KEY_SET_IF_MASKED(mask, keys, cnt,
FLOW_DISSECTOR_KEY_IPV6_ADDRS, ipv6);
- FL_KEY_SET_IF_MASKED(mask, keys, cnt,
- FLOW_DISSECTOR_KEY_PORTS, tp);
+ if (FL_KEY_IS_MASKED(mask, tp) ||
+ FL_KEY_IS_MASKED(mask, tp_min) || FL_KEY_IS_MASKED(mask, tp_max))
+ FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_PORTS, tp);
FL_KEY_SET_IF_MASKED(mask, keys, cnt,
FLOW_DISSECTOR_KEY_IP, ip);
FL_KEY_SET_IF_MASKED(mask, keys, cnt,
@@ -1086,6 +1199,10 @@ static struct fl_flow_mask *fl_create_new_mask(struct cls_fl_head *head,
fl_mask_copy(newmask, mask);
+ if ((newmask->key.tp_min.dst && newmask->key.tp_max.dst) ||
+ (newmask->key.tp_min.src && newmask->key.tp_max.src))
+ newmask->flags |= TCA_FLOWER_MASK_FLAGS_RANGE;
+
err = fl_init_mask_hashtable(newmask);
if (err)
goto errout_free;
@@ -1239,7 +1356,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
goto errout_idr;
if (!tc_skip_sw(fnew->flags)) {
- if (!fold && fl_lookup(fnew->mask, &fnew->mkey)) {
+ if (!fold && __fl_lookup(fnew->mask, &fnew->mkey)) {
err = -EEXIST;
goto errout_mask;
}
@@ -1476,6 +1593,26 @@ static int fl_dump_key_val(struct sk_buff *skb,
return 0;
}
+static int fl_dump_key_port_range(struct sk_buff *skb, struct fl_flow_key *key,
+ struct fl_flow_key *mask)
+{
+ if (fl_dump_key_val(skb, &key->tp_min.dst, TCA_FLOWER_KEY_PORT_DST_MIN,
+ &mask->tp_min.dst, TCA_FLOWER_UNSPEC,
+ sizeof(key->tp_min.dst)) ||
+ fl_dump_key_val(skb, &key->tp_max.dst, TCA_FLOWER_KEY_PORT_DST_MAX,
+ &mask->tp_max.dst, TCA_FLOWER_UNSPEC,
+ sizeof(key->tp_max.dst)) ||
+ fl_dump_key_val(skb, &key->tp_min.src, TCA_FLOWER_KEY_PORT_SRC_MIN,
+ &mask->tp_min.src, TCA_FLOWER_UNSPEC,
+ sizeof(key->tp_min.src)) ||
+ fl_dump_key_val(skb, &key->tp_max.src, TCA_FLOWER_KEY_PORT_SRC_MAX,
+ &mask->tp_max.src, TCA_FLOWER_UNSPEC,
+ sizeof(key->tp_max.src)))
+ return -1;
+
+ return 0;
+}
+
static int fl_dump_key_mpls(struct sk_buff *skb,
struct flow_dissector_key_mpls *mpls_key,
struct flow_dissector_key_mpls *mpls_mask)
@@ -1812,6 +1949,12 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
sizeof(key->arp.tha))))
goto nla_put_failure;
+ if ((key->basic.ip_proto == IPPROTO_TCP ||
+ key->basic.ip_proto == IPPROTO_UDP ||
+ key->basic.ip_proto == IPPROTO_SCTP) &&
+ fl_dump_key_port_range(skb, key, mask))
+ goto nla_put_failure;
+
if (key->enc_control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS &&
(fl_dump_key_val(skb, &key->enc_ipv4.src,
TCA_FLOWER_KEY_ENC_IPV4_SRC, &mask->enc_ipv4.src,
^ permalink raw reply related
* [Patch net-next] net: get rid of __tcp_checksum_complete()
From: Cong Wang @ 2018-11-13 0:17 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang
__tcp_checksum_complete() is 100% same with __skb_checksum_complete()
and there is no other caller except tcp_checksum_complete().
So, just use __skb_checksum_complete() there.
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/net/tcp.h | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 4743836bed2e..b84b694e8b3d 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1315,15 +1315,10 @@ static inline __sum16 tcp_v4_check(int len, __be32 saddr,
return csum_tcpudp_magic(saddr,daddr,len,IPPROTO_TCP,base);
}
-static inline __sum16 __tcp_checksum_complete(struct sk_buff *skb)
-{
- return __skb_checksum_complete(skb);
-}
-
static inline bool tcp_checksum_complete(struct sk_buff *skb)
{
return !skb_csum_unnecessary(skb) &&
- __tcp_checksum_complete(skb);
+ __skb_checksum_complete(skb);
}
bool tcp_add_backlog(struct sock *sk, struct sk_buff *skb);
--
2.19.1
^ permalink raw reply related
* [PATCH net] net_sched: sch_fq: ensure maxrate fq parameter applies to EDT flows
From: Eric Dumazet @ 2018-11-13 0:17 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
When EDT conversion happened, fq lost the ability to enfore a maxrate
for all flows. It kept it for non EDT flows.
This commit restores the functionality.
Tested:
tc qd replace dev eth0 root fq maxrate 500Mbit
netperf -P0 -H host -- -O THROUGHPUT
489.75
Fixes: ab408b6dc744 ("tcp: switch tcp and sch_fq to new earliest departure time model")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/sched/sch_fq.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 4b1af706896c07e5a0fe6d542dfcd530acdcf8f5..25a7cf6d380fd1ef5610a43a06dea488121b8206 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -469,22 +469,29 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
goto begin;
}
prefetch(&skb->end);
- f->credit -= qdisc_pkt_len(skb);
+ plen = qdisc_pkt_len(skb);
+ f->credit -= plen;
- if (ktime_to_ns(skb->tstamp) || !q->rate_enable)
+ if (!q->rate_enable)
goto out;
rate = q->flow_max_rate;
- if (skb->sk)
- rate = min(skb->sk->sk_pacing_rate, rate);
- if (rate <= q->low_rate_threshold) {
- f->credit = 0;
- plen = qdisc_pkt_len(skb);
- } else {
- plen = max(qdisc_pkt_len(skb), q->quantum);
- if (f->credit > 0)
- goto out;
+ /* If EDT time was provided for this skb, we need to
+ * update f->time_next_packet only if this qdisc enforces
+ * a flow max rate.
+ */
+ if (!skb->tstamp) {
+ if (skb->sk)
+ rate = min(skb->sk->sk_pacing_rate, rate);
+
+ if (rate <= q->low_rate_threshold) {
+ f->credit = 0;
+ } else {
+ plen = max(plen, q->quantum);
+ if (f->credit > 0)
+ goto out;
+ }
}
if (rate != ~0UL) {
u64 len = (u64)plen * NSEC_PER_SEC;
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related
* [PATCH net-next 0/5] net: hns3: Add support of hardware GRO to HNS3 Driver
From: Salil Mehta @ 2018-11-13 10:13 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
linux-kernel, linux-rdma, linuxarm
This patch-set adds support of hardware assisted GRO feature to
HNS3 driver on Rev B(=0x21) platform. Current hardware only
supports TCP/IPv{4|6} flows.
Peng Li (5):
net: hns3: Enable HW GRO for Rev B(=0x21) HNS3 hardware
net: hns3: Add handling of GRO Pkts not fully RX'ed in NAPI poll
net: hns3: Add support for ethtool -K to enable/disable HW GRO
net: hns3: Add skb chain when num of RX buf exceeds MAX_SKB_FRAGS
net: hns3: Adds GRO params to SKB for the stack
drivers/net/ethernet/hisilicon/hns3/hnae3.h | 7 +
.../net/ethernet/hisilicon/hns3/hns3_enet.c | 289 ++++++++++++++----
.../net/ethernet/hisilicon/hns3/hns3_enet.h | 17 +-
.../hisilicon/hns3/hns3pf/hclge_cmd.h | 7 +
.../hisilicon/hns3/hns3pf/hclge_main.c | 45 +++
.../hisilicon/hns3/hns3vf/hclgevf_cmd.h | 8 +
.../hisilicon/hns3/hns3vf/hclgevf_main.c | 45 +++
7 files changed, 351 insertions(+), 67 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net-next 1/5] net: hns3: Enable HW GRO for Rev B(=0x21) HNS3 hardware
From: Salil Mehta @ 2018-11-13 10:13 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
linux-kernel, linux-rdma, linuxarm
In-Reply-To: <20181113101307.6020-1-salil.mehta@huawei.com>
From: Peng Li <lipeng321@huawei.com>
HNS3 hardware Revision B(=0x21) supports Hardware GRO feature. This
patch enables this feature in the HNS3 PF/VF driver.
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hnae3.h | 4 ++
.../net/ethernet/hisilicon/hns3/hns3_enet.c | 4 +-
.../hisilicon/hns3/hns3pf/hclge_cmd.h | 7 ++++
.../hisilicon/hns3/hns3pf/hclge_main.c | 36 ++++++++++++++++++
.../hisilicon/hns3/hns3vf/hclgevf_cmd.h | 8 ++++
.../hisilicon/hns3/hns3vf/hclgevf_main.c | 37 +++++++++++++++++++
6 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index f69d39f17bdd..21d934b7a2a3 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -52,6 +52,7 @@
#define HNAE3_UNIC_CLIENT_INITED_B 0x4
#define HNAE3_ROCE_CLIENT_INITED_B 0x5
#define HNAE3_DEV_SUPPORT_FD_B 0x6
+#define HNAE3_DEV_SUPPORT_GRO_B 0x7
#define HNAE3_DEV_SUPPORT_ROCE_DCB_BITS (BIT(HNAE3_DEV_SUPPORT_DCB_B) |\
BIT(HNAE3_DEV_SUPPORT_ROCE_B))
@@ -65,6 +66,9 @@
#define hnae3_dev_fd_supported(hdev) \
hnae3_get_bit((hdev)->ae_dev->flag, HNAE3_DEV_SUPPORT_FD_B)
+#define hnae3_dev_gro_supported(hdev) \
+ hnae3_get_bit((hdev)->ae_dev->flag, HNAE3_DEV_SUPPORT_GRO_B)
+
#define ring_ptr_move_fw(ring, p) \
((ring)->p = ((ring)->p + 1) % (ring)->desc_num)
#define ring_ptr_move_bw(ring, p) \
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 8d07ec668d5d..a510ddfd45a5 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -1714,8 +1714,10 @@ static void hns3_disable_sriov(struct pci_dev *pdev)
static void hns3_get_dev_capability(struct pci_dev *pdev,
struct hnae3_ae_dev *ae_dev)
{
- if (pdev->revision >= 0x21)
+ if (pdev->revision >= 0x21) {
hnae3_set_bit(ae_dev->flag, HNAE3_DEV_SUPPORT_FD_B, 1);
+ hnae3_set_bit(ae_dev->flag, HNAE3_DEV_SUPPORT_GRO_B, 1);
+ }
}
/* hns3_probe - Device initialization routine
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
index 872cd4bdd70d..aef044d08b11 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
@@ -152,6 +152,7 @@ enum hclge_opcode_type {
/* TSO command */
HCLGE_OPC_TSO_GENERIC_CONFIG = 0x0C01,
+ HCLGE_OPC_GRO_GENERIC_CONFIG = 0x0C10,
/* RSS commands */
HCLGE_OPC_RSS_GENERIC_CONFIG = 0x0D01,
@@ -758,6 +759,12 @@ struct hclge_cfg_tso_status_cmd {
u8 rsv[20];
};
+#define HCLGE_GRO_EN_B 0
+struct hclge_cfg_gro_status_cmd {
+ __le16 gro_en;
+ u8 rsv[22];
+};
+
#define HCLGE_TSO_MSS_MIN 256
#define HCLGE_TSO_MSS_MAX 9668
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 43bfc730a62d..d02712446d50 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -921,6 +921,28 @@ static int hclge_config_tso(struct hclge_dev *hdev, int tso_mss_min,
return hclge_cmd_send(&hdev->hw, &desc, 1);
}
+static int hclge_config_gro(struct hclge_dev *hdev, bool en)
+{
+ struct hclge_cfg_gro_status_cmd *req;
+ struct hclge_desc desc;
+ int ret;
+
+ if (!hnae3_dev_gro_supported(hdev))
+ return 0;
+
+ hclge_cmd_setup_basic_desc(&desc, HCLGE_OPC_GRO_GENERIC_CONFIG, false);
+ req = (struct hclge_cfg_gro_status_cmd *)desc.data;
+
+ req->gro_en = cpu_to_le16(en ? 1 : 0);
+
+ ret = hclge_cmd_send(&hdev->hw, &desc, 1);
+ if (ret)
+ dev_err(&hdev->pdev->dev,
+ "GRO hardware config cmd failed, ret = %d\n", ret);
+
+ return ret;
+}
+
static int hclge_alloc_tqps(struct hclge_dev *hdev)
{
struct hclge_tqp *tqp;
@@ -7090,6 +7112,13 @@ static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev)
goto err_mdiobus_unreg;
}
+ ret = hclge_config_gro(hdev, true);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "Failed to enable GRO in hardware, ret =%d\n", ret);
+ goto err_mdiobus_unreg;
+ }
+
ret = hclge_init_vlan_config(hdev);
if (ret) {
dev_err(&pdev->dev, "VLAN init fail, ret =%d\n", ret);
@@ -7221,6 +7250,13 @@ static int hclge_reset_ae_dev(struct hnae3_ae_dev *ae_dev)
return ret;
}
+ ret = hclge_config_gro(hdev, true);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "Failed to enable GRO in hardware, ret =%d\n", ret);
+ return ret;
+ }
+
ret = hclge_init_vlan_config(hdev);
if (ret) {
dev_err(&pdev->dev, "VLAN init fail, ret =%d\n", ret);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.h b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.h
index 090541de0c7d..47030b42341f 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.h
@@ -87,6 +87,8 @@ enum hclgevf_opcode_type {
HCLGEVF_OPC_QUERY_TX_STATUS = 0x0B03,
HCLGEVF_OPC_QUERY_RX_STATUS = 0x0B13,
HCLGEVF_OPC_CFG_COM_TQP_QUEUE = 0x0B20,
+ /* GRO command */
+ HCLGEVF_OPC_GRO_GENERIC_CONFIG = 0x0C10,
/* RSS cmd */
HCLGEVF_OPC_RSS_GENERIC_CONFIG = 0x0D01,
HCLGEVF_OPC_RSS_INPUT_TUPLE = 0x0D02,
@@ -149,6 +151,12 @@ struct hclgevf_query_res_cmd {
__le16 rsv[7];
};
+#define HCLGEVF_GRO_EN_B 0
+struct hclgevf_cfg_gro_status_cmd {
+ __le16 gro_en;
+ u8 rsv[22];
+};
+
#define HCLGEVF_RSS_DEFAULT_OUTPORT_B 4
#define HCLGEVF_RSS_HASH_KEY_OFFSET_B 4
#define HCLGEVF_RSS_HASH_KEY_NUM 16
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 6b4d1477055f..3f256414fbe4 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -1655,6 +1655,29 @@ static int hclgevf_init_roce_base_info(struct hclgevf_dev *hdev)
return 0;
}
+static int hclgevf_config_gro(struct hclgevf_dev *hdev, bool en)
+{
+ struct hclgevf_cfg_gro_status_cmd *req;
+ struct hclgevf_desc desc;
+ int ret;
+
+ if (!hnae3_dev_gro_supported(hdev))
+ return 0;
+
+ hclgevf_cmd_setup_basic_desc(&desc, HCLGEVF_OPC_GRO_GENERIC_CONFIG,
+ false);
+ req = (struct hclgevf_cfg_gro_status_cmd *)desc.data;
+
+ req->gro_en = cpu_to_le16(en ? 1 : 0);
+
+ ret = hclgevf_cmd_send(&hdev->hw, &desc, 1);
+ if (ret)
+ dev_err(&hdev->pdev->dev,
+ "VF GRO hardware config cmd failed, ret = %d.\n", ret);
+
+ return ret;
+}
+
static int hclgevf_rss_init_hw(struct hclgevf_dev *hdev)
{
struct hclgevf_rss_cfg *rss_cfg = &hdev->rss_cfg;
@@ -2122,6 +2145,13 @@ static int hclgevf_reset_hdev(struct hclgevf_dev *hdev)
return ret;
}
+ ret = hclgevf_config_gro(hdev, true);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "failed to enable VF GRO in hardware, ret =%d\n", ret);
+ return ret;
+ }
+
ret = hclgevf_init_vlan_config(hdev);
if (ret) {
dev_err(&hdev->pdev->dev,
@@ -2199,6 +2229,13 @@ static int hclgevf_init_hdev(struct hclgevf_dev *hdev)
goto err_config;
}
+ ret = hclgevf_config_gro(hdev, true);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "Failed to enable VF GRO in hardware, ret =%d\n", ret);
+ goto err_config;
+ }
+
/* Initialize RSS for this VF */
ret = hclgevf_rss_init_hw(hdev);
if (ret) {
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 2/5] net: hns3: Add handling of GRO Pkts not fully RX'ed in NAPI poll
From: Salil Mehta @ 2018-11-13 10:13 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
linux-kernel, linux-rdma, linuxarm
In-Reply-To: <20181113101307.6020-1-salil.mehta@huawei.com>
From: Peng Li <lipeng321@huawei.com>
The "FE bit" in the description means the last description for
a packets. When HW GRO enable, HW write data to ring every
packet/buffer, there is greater probability that driver handle
with the describtion but HW still not set the "FE bit".
When driver handle the packet and HW still not set "FE bit",
driver stores skb and bd_num in rx ring, and continue to use the
skb and bd_num in next napi.
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
.../net/ethernet/hisilicon/hns3/hns3_enet.c | 196 ++++++++++++------
.../net/ethernet/hisilicon/hns3/hns3_enet.h | 6 +
2 files changed, 140 insertions(+), 62 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index a510ddfd45a5..d8c5e1198670 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -2383,6 +2383,90 @@ static bool hns3_parse_vlan_tag(struct hns3_enet_ring *ring,
}
}
+static int hns3_alloc_skb(struct hns3_enet_ring *ring, int length,
+ unsigned char *va)
+{
+#define HNS3_NEED_ADD_FRAG 1
+ struct hns3_desc_cb *desc_cb = &ring->desc_cb[ring->next_to_clean];
+ struct net_device *netdev = ring->tqp->handle->kinfo.netdev;
+ struct sk_buff *skb;
+
+ ring->skb = napi_alloc_skb(&ring->tqp_vector->napi, HNS3_RX_HEAD_SIZE);
+ skb = ring->skb;
+ if (unlikely(!skb)) {
+ netdev_err(netdev, "alloc rx skb fail\n");
+
+ u64_stats_update_begin(&ring->syncp);
+ ring->stats.sw_err_cnt++;
+ u64_stats_update_end(&ring->syncp);
+
+ return -ENOMEM;
+ }
+
+ prefetchw(skb->data);
+
+ ring->pending_buf = 1;
+ if (length <= HNS3_RX_HEAD_SIZE) {
+ memcpy(__skb_put(skb, length), va, ALIGN(length, sizeof(long)));
+
+ /* We can reuse buffer as-is, just make sure it is local */
+ if (likely(page_to_nid(desc_cb->priv) == numa_node_id()))
+ desc_cb->reuse_flag = 1;
+ else /* This page cannot be reused so discard it */
+ put_page(desc_cb->priv);
+
+ ring_ptr_move_fw(ring, next_to_clean);
+ return 0;
+ }
+ u64_stats_update_begin(&ring->syncp);
+ ring->stats.seg_pkt_cnt++;
+ u64_stats_update_end(&ring->syncp);
+
+ ring->pull_len = eth_get_headlen(va, HNS3_RX_HEAD_SIZE);
+ __skb_put(skb, ring->pull_len);
+ hns3_nic_reuse_page(skb, 0, ring, ring->pull_len,
+ desc_cb);
+ ring_ptr_move_fw(ring, next_to_clean);
+
+ return HNS3_NEED_ADD_FRAG;
+}
+
+static int hns3_add_frag(struct hns3_enet_ring *ring, struct hns3_desc *desc,
+ struct sk_buff **out_skb, bool pending)
+{
+ struct sk_buff *skb = *out_skb;
+ struct hns3_desc_cb *desc_cb;
+ struct hns3_desc *pre_desc;
+ u32 bd_base_info;
+ int pre_bd;
+
+ /* if there is pending bd, the SW param next_to_clean has moved
+ * to next and the next is NULL
+ */
+ if (pending) {
+ pre_bd = (ring->next_to_clean - 1 + ring->desc_num) %
+ ring->desc_num;
+ pre_desc = &ring->desc[pre_bd];
+ bd_base_info = le32_to_cpu(pre_desc->rx.bd_base_info);
+ } else {
+ bd_base_info = le32_to_cpu(desc->rx.bd_base_info);
+ }
+
+ while (!hnae3_get_bit(bd_base_info, HNS3_RXD_FE_B)) {
+ desc = &ring->desc[ring->next_to_clean];
+ desc_cb = &ring->desc_cb[ring->next_to_clean];
+ bd_base_info = le32_to_cpu(desc->rx.bd_base_info);
+ if (!hnae3_get_bit(bd_base_info, HNS3_RXD_VLD_B))
+ return -ENXIO;
+
+ hns3_nic_reuse_page(skb, ring->pending_buf, ring, 0, desc_cb);
+ ring_ptr_move_fw(ring, next_to_clean);
+ ring->pending_buf++;
+ }
+
+ return 0;
+}
+
static void hns3_set_rx_skb_rss_type(struct hns3_enet_ring *ring,
struct sk_buff *skb)
{
@@ -2399,18 +2483,16 @@ static void hns3_set_rx_skb_rss_type(struct hns3_enet_ring *ring,
}
static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
- struct sk_buff **out_skb, int *out_bnum)
+ struct sk_buff **out_skb)
{
struct net_device *netdev = ring->tqp->handle->kinfo.netdev;
+ struct sk_buff *skb = ring->skb;
struct hns3_desc_cb *desc_cb;
struct hns3_desc *desc;
- struct sk_buff *skb;
- unsigned char *va;
u32 bd_base_info;
- int pull_len;
u32 l234info;
int length;
- int bnum;
+ int ret;
desc = &ring->desc[ring->next_to_clean];
desc_cb = &ring->desc_cb[ring->next_to_clean];
@@ -2422,9 +2504,10 @@ static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
/* Check valid BD */
if (unlikely(!hnae3_get_bit(bd_base_info, HNS3_RXD_VLD_B)))
- return -EFAULT;
+ return -ENXIO;
- va = (unsigned char *)desc_cb->buf + desc_cb->page_offset;
+ if (!skb)
+ ring->va = (unsigned char *)desc_cb->buf + desc_cb->page_offset;
/* Prefetch first cache line of first page
* Idea is to cache few bytes of the header of the packet. Our L1 Cache
@@ -2433,62 +2516,42 @@ static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
* lines. In such a case, single fetch would suffice to cache in the
* relevant part of the header.
*/
- prefetch(va);
+ prefetch(ring->va);
#if L1_CACHE_BYTES < 128
- prefetch(va + L1_CACHE_BYTES);
+ prefetch(ring->va + L1_CACHE_BYTES);
#endif
- skb = *out_skb = napi_alloc_skb(&ring->tqp_vector->napi,
- HNS3_RX_HEAD_SIZE);
- if (unlikely(!skb)) {
- netdev_err(netdev, "alloc rx skb fail\n");
+ if (!skb) {
+ ret = hns3_alloc_skb(ring, length, ring->va);
+ *out_skb = skb = ring->skb;
- u64_stats_update_begin(&ring->syncp);
- ring->stats.sw_err_cnt++;
- u64_stats_update_end(&ring->syncp);
-
- return -ENOMEM;
- }
-
- prefetchw(skb->data);
-
- bnum = 1;
- if (length <= HNS3_RX_HEAD_SIZE) {
- memcpy(__skb_put(skb, length), va, ALIGN(length, sizeof(long)));
-
- /* We can reuse buffer as-is, just make sure it is local */
- if (likely(page_to_nid(desc_cb->priv) == numa_node_id()))
- desc_cb->reuse_flag = 1;
- else /* This page cannot be reused so discard it */
- put_page(desc_cb->priv);
+ if (ret < 0) /* alloc buffer fail */
+ return ret;
+ if (ret > 0) { /* need add frag */
+ ret = hns3_add_frag(ring, desc, &skb, false);
+ if (ret)
+ return ret;
- ring_ptr_move_fw(ring, next_to_clean);
+ /* As the head data may be changed when GRO enable, copy
+ * the head data in after other data rx completed
+ */
+ memcpy(skb->data, ring->va,
+ ALIGN(ring->pull_len, sizeof(long)));
+ }
} else {
- u64_stats_update_begin(&ring->syncp);
- ring->stats.seg_pkt_cnt++;
- u64_stats_update_end(&ring->syncp);
-
- pull_len = eth_get_headlen(va, HNS3_RX_HEAD_SIZE);
-
- memcpy(__skb_put(skb, pull_len), va,
- ALIGN(pull_len, sizeof(long)));
-
- hns3_nic_reuse_page(skb, 0, ring, pull_len, desc_cb);
- ring_ptr_move_fw(ring, next_to_clean);
+ ret = hns3_add_frag(ring, desc, &skb, true);
+ if (ret)
+ return ret;
- while (!hnae3_get_bit(bd_base_info, HNS3_RXD_FE_B)) {
- desc = &ring->desc[ring->next_to_clean];
- desc_cb = &ring->desc_cb[ring->next_to_clean];
- bd_base_info = le32_to_cpu(desc->rx.bd_base_info);
- hns3_nic_reuse_page(skb, bnum, ring, 0, desc_cb);
- ring_ptr_move_fw(ring, next_to_clean);
- bnum++;
- }
+ /* As the head data may be changed when GRO enable, copy
+ * the head data in after other data rx completed
+ */
+ memcpy(skb->data, ring->va,
+ ALIGN(ring->pull_len, sizeof(long)));
}
- *out_bnum = bnum;
-
l234info = le32_to_cpu(desc->rx.l234_info);
+ bd_base_info = le32_to_cpu(desc->rx.bd_base_info);
/* Based on hw strategy, the tag offloaded will be stored at
* ot_vlan_tag in two layer tag case, and stored at vlan_tag
@@ -2539,6 +2602,7 @@ static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
ring->tqp_vector->rx_group.total_bytes += skb->len;
hns3_rx_checksum(ring, skb, desc);
+ *out_skb = skb;
hns3_set_rx_skb_rss_type(ring, skb);
return 0;
@@ -2551,9 +2615,9 @@ int hns3_clean_rx_ring(
#define RCB_NOF_ALLOC_RX_BUFF_ONCE 16
struct net_device *netdev = ring->tqp->handle->kinfo.netdev;
int recv_pkts, recv_bds, clean_count, err;
- int unused_count = hns3_desc_unused(ring);
- struct sk_buff *skb = NULL;
- int num, bnum = 0;
+ int unused_count = hns3_desc_unused(ring) - ring->pending_buf;
+ struct sk_buff *skb = ring->skb;
+ int num;
num = readl_relaxed(ring->tqp->io_base + HNS3_RING_RX_RING_FBDNUM_REG);
rmb(); /* Make sure num taken effect before the other data is touched */
@@ -2567,24 +2631,32 @@ int hns3_clean_rx_ring(
hns3_nic_alloc_rx_buffers(ring,
clean_count + unused_count);
clean_count = 0;
- unused_count = hns3_desc_unused(ring);
+ unused_count = hns3_desc_unused(ring) -
+ ring->pending_buf;
}
/* Poll one pkt */
- err = hns3_handle_rx_bd(ring, &skb, &bnum);
+ err = hns3_handle_rx_bd(ring, &skb);
if (unlikely(!skb)) /* This fault cannot be repaired */
goto out;
- recv_bds += bnum;
- clean_count += bnum;
- if (unlikely(err)) { /* Do jump the err */
- recv_pkts++;
+ if (err == -ENXIO) { /* Do not get FE for the packet */
+ goto out;
+ } else if (unlikely(err)) { /* Do jump the err */
+ recv_bds += ring->pending_buf;
+ clean_count += ring->pending_buf;
+ ring->skb = NULL;
+ ring->pending_buf = 0;
continue;
}
/* Do update ip stack process */
skb->protocol = eth_type_trans(skb, netdev);
rx_fn(ring, skb);
+ recv_bds += ring->pending_buf;
+ clean_count += ring->pending_buf;
+ ring->skb = NULL;
+ ring->pending_buf = 0;
recv_pkts++;
}
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index 10ff18af3cc7..d8c0998127be 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -401,11 +401,17 @@ struct hns3_enet_ring {
*/
int next_to_clean;
+ int pull_len; /* head length for current packet */
+ unsigned char *va; /* first buffer address for current packet */
+
u32 flag; /* ring attribute */
int irq_init_flag;
int numa_node;
cpumask_t affinity_mask;
+
+ int pending_buf;
+ struct sk_buff *skb;
};
struct hns_queue;
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 3/5] net: hns3: Add support for ethtool -K to enable/disable HW GRO
From: Salil Mehta @ 2018-11-13 10:13 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
linux-kernel, linux-rdma, linuxarm
In-Reply-To: <20181113101307.6020-1-salil.mehta@huawei.com>
From: Peng Li <lipeng321@huawei.com>
This patch adds support of ethtool -K to enable/disable
hardware GRO in HNS3 PF/VF driver.
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hnae3.h | 3 +++
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 13 ++++++++++++-
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 9 +++++++++
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 8 ++++++++
4 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index 21d934b7a2a3..1746172ffbf7 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -305,6 +305,8 @@ struct hnae3_ae_dev {
* Set vlan filter config of vf
* enable_hw_strip_rxvtag()
* Enable/disable hardware strip vlan tag of packets received
+ * set_gro_en
+ * Enable/disable HW GRO
*/
struct hnae3_ae_ops {
int (*init_ae_dev)(struct hnae3_ae_dev *ae_dev);
@@ -449,6 +451,7 @@ struct hnae3_ae_ops {
bool (*get_hw_reset_stat)(struct hnae3_handle *handle);
bool (*ae_dev_resetting)(struct hnae3_handle *handle);
unsigned long (*ae_dev_reset_cnt)(struct hnae3_handle *handle);
+ int (*set_gro_en)(struct hnae3_handle *handle, int enable);
};
struct hnae3_dcb_ops {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index d8c5e1198670..860067898471 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -1345,6 +1345,15 @@ static int hns3_nic_set_features(struct net_device *netdev,
priv->ops.maybe_stop_tx = hns3_nic_maybe_stop_tx;
}
+ if (changed & (NETIF_F_GRO_HW) && h->ae_algo->ops->set_gro_en) {
+ if (features & NETIF_F_GRO_HW)
+ ret = h->ae_algo->ops->set_gro_en(h, true);
+ else
+ ret = h->ae_algo->ops->set_gro_en(h, false);
+ if (ret)
+ return ret;
+ }
+
if ((changed & NETIF_F_HW_VLAN_CTAG_FILTER) &&
h->ae_algo->ops->enable_vlan_filter) {
if (features & NETIF_F_HW_VLAN_CTAG_FILTER)
@@ -1929,7 +1938,9 @@ static void hns3_set_default_feature(struct net_device *netdev)
NETIF_F_GSO_UDP_TUNNEL_CSUM | NETIF_F_SCTP_CRC;
if (pdev->revision >= 0x21) {
- netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+ netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER |
+ NETIF_F_GRO_HW;
+ netdev->features |= NETIF_F_GRO_HW;
if (!(h->flags & HNAE3_SUPPORT_VF)) {
netdev->hw_features |= NETIF_F_NTUPLE;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index d02712446d50..ed25d262064a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -7673,6 +7673,14 @@ static void hclge_get_link_mode(struct hnae3_handle *handle,
}
}
+static int hclge_gro_en(struct hnae3_handle *handle, int enable)
+{
+ struct hclge_vport *vport = hclge_get_vport(handle);
+ struct hclge_dev *hdev = vport->back;
+
+ return hclge_config_gro(hdev, enable);
+}
+
static const struct hnae3_ae_ops hclge_ops = {
.init_ae_dev = hclge_init_ae_dev,
.uninit_ae_dev = hclge_uninit_ae_dev,
@@ -7744,6 +7752,7 @@ static const struct hnae3_ae_ops hclge_ops = {
.get_hw_reset_stat = hclge_get_hw_reset_stat,
.ae_dev_resetting = hclge_ae_dev_resetting,
.ae_dev_reset_cnt = hclge_ae_dev_reset_cnt,
+ .set_gro_en = hclge_gro_en,
};
static struct hnae3_ae_algo ae_algo = {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 3f256414fbe4..e51f8f7d52f1 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -2374,6 +2374,13 @@ void hclgevf_update_speed_duplex(struct hclgevf_dev *hdev, u32 speed,
hdev->hw.mac.duplex = duplex;
}
+static int hclgevf_gro_en(struct hnae3_handle *handle, int enable)
+{
+ struct hclgevf_dev *hdev = hclgevf_ae_get_hdev(handle);
+
+ return hclgevf_config_gro(hdev, enable);
+}
+
static void hclgevf_get_media_type(struct hnae3_handle *handle,
u8 *media_type)
{
@@ -2448,6 +2455,7 @@ static const struct hnae3_ae_ops hclgevf_ops = {
.get_hw_reset_stat = hclgevf_get_hw_reset_stat,
.ae_dev_resetting = hclgevf_ae_dev_resetting,
.ae_dev_reset_cnt = hclgevf_ae_dev_reset_cnt,
+ .set_gro_en = hclgevf_gro_en,
};
static struct hnae3_ae_algo ae_algovf = {
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 4/5] net: hns3: Add skb chain when num of RX buf exceeds MAX_SKB_FRAGS
From: Salil Mehta @ 2018-11-13 10:13 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
linux-kernel, linux-rdma, linuxarm
In-Reply-To: <20181113101307.6020-1-salil.mehta@huawei.com>
From: Peng Li <lipeng321@huawei.com>
MAX_SKB_FRAGS in protocol stack is defined as:
MAX_SKB_FRAGS is 17 when PAGE_SIZE is 4K. If HW enable GRO, it may
merge small packets and the rx buffer may be more than
MAX_SKB_FRAGS. So driver will add skb chain when RX buffer num.
more than MAX_SKB_FRAGS.
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
.../net/ethernet/hisilicon/hns3/hns3_enet.c | 37 ++++++++++++++++++-
.../net/ethernet/hisilicon/hns3/hns3_enet.h | 2 +
2 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 860067898471..7776089b6bc2 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -2361,6 +2361,9 @@ static void hns3_rx_checksum(struct hns3_enet_ring *ring, struct sk_buff *skb,
static void hns3_rx_skb(struct hns3_enet_ring *ring, struct sk_buff *skb)
{
+ if (skb_has_frag_list(skb))
+ napi_gro_flush(&ring->tqp_vector->napi, false);
+
napi_gro_receive(&ring->tqp_vector->napi, skb);
}
@@ -2417,6 +2420,8 @@ static int hns3_alloc_skb(struct hns3_enet_ring *ring, int length,
prefetchw(skb->data);
ring->pending_buf = 1;
+ ring->frag_num = 0;
+ ring->tail_skb = NULL;
if (length <= HNS3_RX_HEAD_SIZE) {
memcpy(__skb_put(skb, length), va, ALIGN(length, sizeof(long)));
@@ -2435,7 +2440,7 @@ static int hns3_alloc_skb(struct hns3_enet_ring *ring, int length,
ring->pull_len = eth_get_headlen(va, HNS3_RX_HEAD_SIZE);
__skb_put(skb, ring->pull_len);
- hns3_nic_reuse_page(skb, 0, ring, ring->pull_len,
+ hns3_nic_reuse_page(skb, ring->frag_num++, ring, ring->pull_len,
desc_cb);
ring_ptr_move_fw(ring, next_to_clean);
@@ -2446,6 +2451,8 @@ static int hns3_add_frag(struct hns3_enet_ring *ring, struct hns3_desc *desc,
struct sk_buff **out_skb, bool pending)
{
struct sk_buff *skb = *out_skb;
+ struct sk_buff *head_skb = *out_skb;
+ struct sk_buff *new_skb;
struct hns3_desc_cb *desc_cb;
struct hns3_desc *pre_desc;
u32 bd_base_info;
@@ -2470,7 +2477,33 @@ static int hns3_add_frag(struct hns3_enet_ring *ring, struct hns3_desc *desc,
if (!hnae3_get_bit(bd_base_info, HNS3_RXD_VLD_B))
return -ENXIO;
- hns3_nic_reuse_page(skb, ring->pending_buf, ring, 0, desc_cb);
+ if (unlikely(ring->frag_num >= MAX_SKB_FRAGS)) {
+ new_skb = napi_alloc_skb(&ring->tqp_vector->napi,
+ HNS3_RX_HEAD_SIZE);
+ if (unlikely(!new_skb)) {
+ netdev_err(ring->tqp->handle->kinfo.netdev,
+ "alloc rx skb frag fail\n");
+ return -ENXIO;
+ }
+ ring->frag_num = 0;
+
+ if (ring->tail_skb) {
+ ring->tail_skb->next = new_skb;
+ ring->tail_skb = new_skb;
+ } else {
+ skb_shinfo(skb)->frag_list = new_skb;
+ ring->tail_skb = new_skb;
+ }
+ }
+
+ if (ring->tail_skb) {
+ head_skb->truesize += hnae3_buf_size(ring);
+ head_skb->data_len += le16_to_cpu(desc->rx.size);
+ head_skb->len += le16_to_cpu(desc->rx.size);
+ skb = ring->tail_skb;
+ }
+
+ hns3_nic_reuse_page(skb, ring->frag_num++, ring, 0, desc_cb);
ring_ptr_move_fw(ring, next_to_clean);
ring->pending_buf++;
}
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index d8c0998127be..8e56b7e44978 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -402,6 +402,7 @@ struct hns3_enet_ring {
int next_to_clean;
int pull_len; /* head length for current packet */
+ u32 frag_num;
unsigned char *va; /* first buffer address for current packet */
u32 flag; /* ring attribute */
@@ -412,6 +413,7 @@ struct hns3_enet_ring {
int pending_buf;
struct sk_buff *skb;
+ struct sk_buff *tail_skb;
};
struct hns_queue;
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 5/5] net: hns3: Adds GRO params to SKB for the stack
From: Salil Mehta @ 2018-11-13 10:13 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
linux-kernel, linux-rdma, linuxarm
In-Reply-To: <20181113101307.6020-1-salil.mehta@huawei.com>
From: Peng Li <lipeng321@huawei.com>
When HW GRO enable, protocol stack will not do GRO again,
driver should add gro param to the skb for the protocol
stack..
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
.../net/ethernet/hisilicon/hns3/hns3_enet.c | 43 +++++++++++++++++++
.../net/ethernet/hisilicon/hns3/hns3_enet.h | 9 ++--
2 files changed, 49 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 7776089b6bc2..22220af92aa9 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -15,6 +15,7 @@
#include <linux/vermagic.h>
#include <net/gre.h>
#include <net/pkt_cls.h>
+#include <net/tcp.h>
#include <net/vxlan.h>
#include "hnae3.h"
@@ -2318,6 +2319,12 @@ static void hns3_rx_checksum(struct hns3_enet_ring *ring, struct sk_buff *skb,
if (!(netdev->features & NETIF_F_RXCSUM))
return;
+ /* We MUST enable hardware checksum before enabling hardware GRO */
+ if (skb_shinfo(skb)->gso_size) {
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+ return;
+ }
+
/* check if hardware has done checksum */
if (!hnae3_get_bit(bd_base_info, HNS3_RXD_L3L4P_B))
return;
@@ -2511,6 +2518,39 @@ static int hns3_add_frag(struct hns3_enet_ring *ring, struct hns3_desc *desc,
return 0;
}
+static void hns3_set_gro_param(struct sk_buff *skb, u32 l234info,
+ u32 bd_base_info)
+{
+ u16 gro_count;
+ u32 l3_type;
+
+ gro_count = hnae3_get_field(l234info, HNS3_RXD_GRO_COUNT_M,
+ HNS3_RXD_GRO_COUNT_S);
+ /* if there is no HW GRO, do not set gro params */
+ if (!gro_count)
+ return;
+
+ /* tcp_gro_complete() will copy NAPI_GRO_CB(skb)->count
+ * to skb_shinfo(skb)->gso_segs
+ */
+ NAPI_GRO_CB(skb)->count = gro_count;
+
+ l3_type = hnae3_get_field(l234info, HNS3_RXD_L3ID_M,
+ HNS3_RXD_L3ID_S);
+ if (l3_type == HNS3_L3_TYPE_IPV4)
+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
+ else if (l3_type == HNS3_L3_TYPE_IPV6)
+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
+ else
+ return;
+
+ skb_shinfo(skb)->gso_size = hnae3_get_field(bd_base_info,
+ HNS3_RXD_GRO_SIZE_M,
+ HNS3_RXD_GRO_SIZE_S);
+ if (skb_shinfo(skb)->gso_size)
+ tcp_gro_complete(skb);
+}
+
static void hns3_set_rx_skb_rss_type(struct hns3_enet_ring *ring,
struct sk_buff *skb)
{
@@ -2645,6 +2685,9 @@ static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
ring->tqp_vector->rx_group.total_bytes += skb->len;
+ /* This is needed in order to enable forwarding support */
+ hns3_set_gro_param(skb, l234info, bd_base_info);
+
hns3_rx_checksum(ring, skb, desc);
*out_skb = skb;
hns3_set_rx_skb_rss_type(ring, skb);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index 8e56b7e44978..3365c9596983 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -109,6 +109,10 @@ enum hns3_nic_state {
#define HNS3_RXD_DOI_B 21
#define HNS3_RXD_OL3E_B 22
#define HNS3_RXD_OL4E_B 23
+#define HNS3_RXD_GRO_COUNT_S 24
+#define HNS3_RXD_GRO_COUNT_M (0x3f << HNS3_RXD_GRO_COUNT_S)
+#define HNS3_RXD_GRO_FIXID_B 30
+#define HNS3_RXD_GRO_ECN_B 31
#define HNS3_RXD_ODMAC_S 0
#define HNS3_RXD_ODMAC_M (0x3 << HNS3_RXD_ODMAC_S)
@@ -135,9 +139,8 @@ enum hns3_nic_state {
#define HNS3_RXD_TSIND_S 12
#define HNS3_RXD_TSIND_M (0x7 << HNS3_RXD_TSIND_S)
#define HNS3_RXD_LKBK_B 15
-#define HNS3_RXD_HDL_S 16
-#define HNS3_RXD_HDL_M (0x7ff << HNS3_RXD_HDL_S)
-#define HNS3_RXD_HSIND_B 31
+#define HNS3_RXD_GRO_SIZE_S 16
+#define HNS3_RXD_GRO_SIZE_M (0x3ff << HNS3_RXD_GRO_SIZE_S)
#define HNS3_TXD_L3T_S 0
#define HNS3_TXD_L3T_M (0x3 << HNS3_TXD_L3T_S)
--
2.17.1
^ permalink raw reply related
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