* [PATCH net-next] mlxsw: spectrum: acl: Use struct_size() in kzalloc()
From: Gustavo A. R. Silva @ 2019-02-25 19:01 UTC (permalink / raw)
To: Jiri Pirko, Ido Schimmel, David S. Miller
Cc: netdev, linux-kernel, Gustavo A. R. Silva
One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:
struct foo {
int stuff;
struct boo entry[];
};
size = sizeof(struct foo) + count * sizeof(struct boo);
instance = kzalloc(size, GFP_KERNEL)
Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:
instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL)
Notice that, in this case, variable alloc_size is not necessary, hence
it is removed.
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_keys.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_keys.c b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_keys.c
index df78d23b3ec3..cb3e663b1d37 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_keys.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_keys.c
@@ -236,12 +236,10 @@ mlxsw_afk_key_info_create(struct mlxsw_afk *mlxsw_afk,
struct mlxsw_afk_element_usage *elusage)
{
struct mlxsw_afk_key_info *key_info;
- size_t alloc_size;
int err;
- alloc_size = sizeof(*key_info) +
- sizeof(key_info->blocks[0]) * mlxsw_afk->max_blocks;
- key_info = kzalloc(alloc_size, GFP_KERNEL);
+ key_info = kzalloc(struct_size(key_info, blocks, mlxsw_afk->max_blocks),
+ GFP_KERNEL);
if (!key_info)
return ERR_PTR(-ENOMEM);
err = mlxsw_afk_picker(mlxsw_afk, key_info, elusage);
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v3 0/7] net: sched: pie: align PIE implementation with RFC 8033
From: Leslie Monis @ 2019-02-25 19:09 UTC (permalink / raw)
To: davem
Cc: netdev, Leslie Monis, Mohit P . Tahiliani, Dave Taht,
Jamal Hadi Salim
The current implementation of the PIE queuing discipline is according to the
IETF draft [http://tools.ietf.org/html/draft-pan-aqm-pie-00] and the paper
[PIE: A Lightweight Control Scheme to Address the Bufferbloat Problem].
However, a lot of necessary modifications and enhancements have been proposed
in RFC 8033, which have not yet been incorporated in the source code of Linux.
This patch series helps in achieving the same.
Performance tests carried out using Flent [https://flent.org/]
Changes from v2 to v3:
- Used div_u64() instead of direct division after explicit type casting as
recommended by David
Changes from v1 to v2:
- Excluded the patch setting PIE dynamically active/inactive as the test
results were unsatisfactory
- Fixed a scaling issue when adding more auto-tuning cases which caused
local variables to underflow
- Changed the long if/else chain to a loop as suggested by Stephen
- Changed the position of the accu_prob variable in the pie_vars
structure as recommended by Stephen
Mohit P. Tahiliani (7):
net: sched: pie: change value of QUEUE_THRESHOLD
net: sched: pie: change default value of pie_params->target
net: sched: pie: change default value of pie_params->tupdate
net: sched: pie: change initial value of pie_vars->burst_time
net: sched: pie: add more cases to auto-tune alpha and beta
net: sched: pie: add derandomization mechanism
net: sched: pie: update references
include/uapi/linux/pkt_sched.h | 2 +-
net/sched/sch_pie.c | 107 ++++++++++++++++++++-------------
2 files changed, 66 insertions(+), 43 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net-next v3 1/7] net: sched: pie: change value of QUEUE_THRESHOLD
From: Leslie Monis @ 2019-02-25 19:09 UTC (permalink / raw)
To: davem
Cc: netdev, Mohit P. Tahiliani, Dave Taht, Jamal Hadi Salim,
Dhaval Khandla, Hrishikesh Hiraskar, Manish Kumar B,
Sachin D . Patil, Leslie Monis
In-Reply-To: <20190225191001.26797-1-lesliemonis@gmail.com>
From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>
RFC 8033 recommends a value of 16384 bytes for the queue
threshold.
Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
Acked-by: Dave Taht <dave.taht@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/sch_pie.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index d1429371592f..7778eff6cdb7 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -31,7 +31,7 @@
#include <net/pkt_sched.h>
#include <net/inet_ecn.h>
-#define QUEUE_THRESHOLD 10000
+#define QUEUE_THRESHOLD 16384
#define DQCOUNT_INVALID -1
#define MAX_PROB 0xffffffff
#define PIE_SCALE 8
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v3 2/7] net: sched: pie: change default value of pie_params->target
From: Leslie Monis @ 2019-02-25 19:09 UTC (permalink / raw)
To: davem
Cc: netdev, Mohit P. Tahiliani, Dave Taht, Jamal Hadi Salim,
Dhaval Khandla, Hrishikesh Hiraskar, Manish Kumar B,
Sachin D . Patil, Leslie Monis
In-Reply-To: <20190225191001.26797-1-lesliemonis@gmail.com>
From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>
RFC 8033 suggests a default value of 15 milliseconds for the
target queue delay.
Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
Acked-by: Dave Taht <dave.taht@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/sch_pie.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 7778eff6cdb7..91af9bf19852 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -83,7 +83,7 @@ static void pie_params_init(struct pie_params *params)
params->beta = 20;
params->tupdate = usecs_to_jiffies(30 * USEC_PER_MSEC); /* 30 ms */
params->limit = 1000; /* default of 1000 packets */
- params->target = PSCHED_NS2TICKS(20 * NSEC_PER_MSEC); /* 20 ms */
+ params->target = PSCHED_NS2TICKS(15 * NSEC_PER_MSEC); /* 15 ms */
params->ecn = false;
params->bytemode = false;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v3 3/7] net: sched: pie: change default value of pie_params->tupdate
From: Leslie Monis @ 2019-02-25 19:09 UTC (permalink / raw)
To: davem
Cc: netdev, Mohit P. Tahiliani, Dave Taht, Jamal Hadi Salim,
Dhaval Khandla, Hrishikesh Hiraskar, Manish Kumar B,
Sachin D . Patil, Leslie Monis
In-Reply-To: <20190225191001.26797-1-lesliemonis@gmail.com>
From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>
RFC 8033 suggests a default value of 15 milliseconds for the
update interval.
Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
Acked-by: Dave Taht <dave.taht@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/sch_pie.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 91af9bf19852..702f75afc312 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -81,7 +81,7 @@ static void pie_params_init(struct pie_params *params)
{
params->alpha = 2;
params->beta = 20;
- params->tupdate = usecs_to_jiffies(30 * USEC_PER_MSEC); /* 30 ms */
+ params->tupdate = usecs_to_jiffies(15 * USEC_PER_MSEC); /* 15 ms */
params->limit = 1000; /* default of 1000 packets */
params->target = PSCHED_NS2TICKS(15 * NSEC_PER_MSEC); /* 15 ms */
params->ecn = false;
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v3 4/7] net: sched: pie: change initial value of pie_vars->burst_time
From: Leslie Monis @ 2019-02-25 19:09 UTC (permalink / raw)
To: davem
Cc: netdev, Mohit P. Tahiliani, Dave Taht, Jamal Hadi Salim,
Dhaval Khandla, Hrishikesh Hiraskar, Manish Kumar B,
Sachin D . Patil, Leslie Monis
In-Reply-To: <20190225191001.26797-1-lesliemonis@gmail.com>
From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>
RFC 8033 suggests an initial value of 150 milliseconds for
the maximum time allowed for a burst of packets.
Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
Acked-by: Dave Taht <dave.taht@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/sch_pie.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 702f75afc312..d88ab53593b3 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -92,8 +92,8 @@ static void pie_vars_init(struct pie_vars *vars)
{
vars->dq_count = DQCOUNT_INVALID;
vars->avg_dq_rate = 0;
- /* default of 100 ms in pschedtime */
- vars->burst_time = PSCHED_NS2TICKS(100 * NSEC_PER_MSEC);
+ /* default of 150 ms in pschedtime */
+ vars->burst_time = PSCHED_NS2TICKS(150 * NSEC_PER_MSEC);
}
static bool drop_early(struct Qdisc *sch, u32 packet_size)
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v3 5/7] net: sched: pie: add more cases to auto-tune alpha and beta
From: Leslie Monis @ 2019-02-25 19:09 UTC (permalink / raw)
To: davem
Cc: netdev, Mohit P. Tahiliani, Dave Taht, Jamal Hadi Salim,
Dhaval Khandla, Hrishikesh Hiraskar, Manish Kumar B,
Sachin D . Patil, Leslie Monis
In-Reply-To: <20190225191001.26797-1-lesliemonis@gmail.com>
From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>
The current implementation scales the local alpha and beta
variables in the calculate_probability function by the same
amount for all values of drop probability below 1%.
RFC 8033 suggests using additional cases for auto-tuning
alpha and beta when the drop probability is less than 1%.
In order to add more auto-tuning cases, MAX_PROB must be
scaled by u64 instead of u32 to prevent underflow when
scaling the local alpha and beta variables in the
calculate_probability function.
Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
Acked-by: Dave Taht <dave.taht@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
include/uapi/linux/pkt_sched.h | 2 +-
net/sched/sch_pie.c | 65 +++++++++++++++++-----------------
2 files changed, 33 insertions(+), 34 deletions(-)
diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index 0d18b1d1fbbc..1eb572ef3f27 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -954,7 +954,7 @@ enum {
#define TCA_PIE_MAX (__TCA_PIE_MAX - 1)
struct tc_pie_xstats {
- __u32 prob; /* current probability */
+ __u64 prob; /* current probability */
__u32 delay; /* current delay in ms */
__u32 avg_dq_rate; /* current average dq_rate in bits/pie_time */
__u32 packets_in; /* total number of packets enqueued */
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index d88ab53593b3..30f158582499 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -33,7 +33,7 @@
#define QUEUE_THRESHOLD 16384
#define DQCOUNT_INVALID -1
-#define MAX_PROB 0xffffffff
+#define MAX_PROB 0xffffffffffffffff
#define PIE_SCALE 8
/* parameters used */
@@ -49,7 +49,7 @@ struct pie_params {
/* variables used */
struct pie_vars {
- u32 prob; /* probability but scaled by u32 limit. */
+ u64 prob; /* probability but scaled by u64 limit. */
psched_time_t burst_time;
psched_time_t qdelay;
psched_time_t qdelay_old;
@@ -99,8 +99,8 @@ static void pie_vars_init(struct pie_vars *vars)
static bool drop_early(struct Qdisc *sch, u32 packet_size)
{
struct pie_sched_data *q = qdisc_priv(sch);
- u32 rnd;
- u32 local_prob = q->vars.prob;
+ u64 rnd;
+ u64 local_prob = q->vars.prob;
u32 mtu = psched_mtu(qdisc_dev(sch));
/* If there is still burst allowance left skip random early drop */
@@ -124,11 +124,11 @@ static bool drop_early(struct Qdisc *sch, u32 packet_size)
* probablity. Smaller packets will have lower drop prob in this case
*/
if (q->params.bytemode && packet_size <= mtu)
- local_prob = (local_prob / mtu) * packet_size;
+ local_prob = (u64)packet_size * div_u64(local_prob, mtu);
else
local_prob = q->vars.prob;
- rnd = prandom_u32();
+ prandom_bytes(&rnd, 8);
if (rnd < local_prob)
return true;
@@ -317,9 +317,10 @@ static void calculate_probability(struct Qdisc *sch)
u32 qlen = sch->qstats.backlog; /* queue size in bytes */
psched_time_t qdelay = 0; /* in pschedtime */
psched_time_t qdelay_old = q->vars.qdelay; /* in pschedtime */
- s32 delta = 0; /* determines the change in probability */
- u32 oldprob;
- u32 alpha, beta;
+ s64 delta = 0; /* determines the change in probability */
+ u64 oldprob;
+ u64 alpha, beta;
+ u32 power;
bool update_prob = true;
q->vars.qdelay_old = q->vars.qdelay;
@@ -339,38 +340,36 @@ static void calculate_probability(struct Qdisc *sch)
* value for alpha as 0.125. In this implementation, we use values 0-32
* passed from user space to represent this. Also, alpha and beta have
* unit of HZ and need to be scaled before they can used to update
- * probability. alpha/beta are updated locally below by 1) scaling them
- * appropriately 2) scaling down by 16 to come to 0-2 range.
- * Please see paper for details.
- *
- * We scale alpha and beta differently depending on whether we are in
- * light, medium or high dropping mode.
+ * probability. alpha/beta are updated locally below by scaling down
+ * by 16 to come to 0-2 range.
*/
- if (q->vars.prob < MAX_PROB / 100) {
- alpha =
- (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 7;
- beta =
- (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 7;
- } else if (q->vars.prob < MAX_PROB / 10) {
- alpha =
- (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 5;
- beta =
- (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 5;
- } else {
- alpha =
- (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 4;
- beta =
- (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 4;
+ alpha = ((u64)q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 4;
+ beta = ((u64)q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 4;
+
+ /* We scale alpha and beta differently depending on how heavy the
+ * congestion is. Please see RFC 8033 for details.
+ */
+ if (q->vars.prob < MAX_PROB / 10) {
+ alpha >>= 1;
+ beta >>= 1;
+
+ power = 100;
+ while (q->vars.prob < div_u64(MAX_PROB, power) &&
+ power <= 1000000) {
+ alpha >>= 2;
+ beta >>= 2;
+ power *= 10;
+ }
}
/* alpha and beta should be between 0 and 32, in multiples of 1/16 */
- delta += alpha * ((qdelay - q->params.target));
- delta += beta * ((qdelay - qdelay_old));
+ delta += alpha * (u64)(qdelay - q->params.target);
+ delta += beta * (u64)(qdelay - qdelay_old);
oldprob = q->vars.prob;
/* to ensure we increase probability in steps of no more than 2% */
- if (delta > (s32)(MAX_PROB / (100 / 2)) &&
+ if (delta > (s64)(MAX_PROB / (100 / 2)) &&
q->vars.prob >= MAX_PROB / 10)
delta = (MAX_PROB / 100) * 2;
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v3 6/7] net: sched: pie: add derandomization mechanism
From: Leslie Monis @ 2019-02-25 19:10 UTC (permalink / raw)
To: davem
Cc: netdev, Mohit P. Tahiliani, Dave Taht, Jamal Hadi Salim,
Dhaval Khandla, Hrishikesh Hiraskar, Manish Kumar B,
Sachin D . Patil, Leslie Monis
In-Reply-To: <20190225191001.26797-1-lesliemonis@gmail.com>
From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>
Random dropping of packets to achieve latency control may
introduce outlier situations where packets are dropped too
close to each other or too far from each other. This can
cause the real drop percentage to temporarily deviate from
the intended drop probability. In certain scenarios, such
as a small number of simultaneous TCP flows, these
deviations can cause significant deviations in link
utilization and queuing latency.
RFC 8033 suggests using a derandomization mechanism to avoid
these deviations.
Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
Acked-by: Dave Taht <dave.taht@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/sch_pie.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 30f158582499..916b878d3491 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -55,8 +55,10 @@ struct pie_vars {
psched_time_t qdelay_old;
u64 dq_count; /* measured in bytes */
psched_time_t dq_tstamp; /* drain rate */
+ u64 accu_prob; /* accumulated drop probability */
u32 avg_dq_rate; /* bytes per pschedtime tick,scaled */
u32 qlen_old; /* in bytes */
+ u8 accu_prob_overflows; /* overflows of accu_prob */
};
/* statistics gathering */
@@ -91,9 +93,11 @@ static void pie_params_init(struct pie_params *params)
static void pie_vars_init(struct pie_vars *vars)
{
vars->dq_count = DQCOUNT_INVALID;
+ vars->accu_prob = 0;
vars->avg_dq_rate = 0;
/* default of 150 ms in pschedtime */
vars->burst_time = PSCHED_NS2TICKS(150 * NSEC_PER_MSEC);
+ vars->accu_prob_overflows = 0;
}
static bool drop_early(struct Qdisc *sch, u32 packet_size)
@@ -128,9 +132,29 @@ static bool drop_early(struct Qdisc *sch, u32 packet_size)
else
local_prob = q->vars.prob;
+ if (local_prob == 0) {
+ q->vars.accu_prob = 0;
+ q->vars.accu_prob_overflows = 0;
+ }
+
+ if (local_prob > MAX_PROB - q->vars.accu_prob)
+ q->vars.accu_prob_overflows++;
+
+ q->vars.accu_prob += local_prob;
+
+ if (q->vars.accu_prob_overflows == 0 &&
+ q->vars.accu_prob < (MAX_PROB / 100) * 85)
+ return false;
+ if (q->vars.accu_prob_overflows == 8 &&
+ q->vars.accu_prob >= MAX_PROB / 2)
+ return true;
+
prandom_bytes(&rnd, 8);
- if (rnd < local_prob)
+ if (rnd < local_prob) {
+ q->vars.accu_prob = 0;
+ q->vars.accu_prob_overflows = 0;
return true;
+ }
return false;
}
@@ -168,6 +192,8 @@ static int pie_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
out:
q->stats.dropped++;
+ q->vars.accu_prob = 0;
+ q->vars.accu_prob_overflows = 0;
return qdisc_drop(skb, sch, to_free);
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v3 7/7] net: sched: pie: update references
From: Leslie Monis @ 2019-02-25 19:10 UTC (permalink / raw)
To: davem
Cc: netdev, Mohit P. Tahiliani, Dave Taht, Jamal Hadi Salim,
Dhaval Khandla, Hrishikesh Hiraskar, Manish Kumar B,
Sachin D . Patil, Leslie Monis
In-Reply-To: <20190225191001.26797-1-lesliemonis@gmail.com>
From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>
RFC 8033 replaces the IETF draft for PIE
Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
Acked-by: Dave Taht <dave.taht@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/sch_pie.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 916b878d3491..f8314a14a256 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -17,9 +17,7 @@
* University of Oslo, Norway.
*
* References:
- * IETF draft submission: http://tools.ietf.org/html/draft-pan-aqm-pie-00
- * IEEE Conference on High Performance Switching and Routing 2013 :
- * "PIE: A * Lightweight Control Scheme to Address the Bufferbloat Problem"
+ * RFC 8033: https://tools.ietf.org/html/rfc8034
*/
#include <linux/module.h>
--
2.17.1
^ permalink raw reply related
* [PATCH][next] Bluetooth: mgmt: Use struct_size() helper
From: Gustavo A. R. Silva @ 2019-02-25 19:11 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, David S. Miller
Cc: linux-bluetooth, netdev, linux-kernel, Gustavo A. R. Silva
Make use of the struct_size() helper instead of an open-coded version
in order to avoid any potential type mistakes, in particular in the
context in which this code is being used.
So, change the following form:
sizeof(*rp) + (sizeof(rp->entry[0]) * count);
to :
struct_size(rp, entry, count)
Notice that, in this case, variable rp_len is not necessary, hence
it is removed.
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
net/bluetooth/mgmt.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index ccce954f8146..1e2acaddcdfd 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -474,7 +474,6 @@ static int read_ext_index_list(struct sock *sk, struct hci_dev *hdev,
{
struct mgmt_rp_read_ext_index_list *rp;
struct hci_dev *d;
- size_t rp_len;
u16 count;
int err;
@@ -488,8 +487,7 @@ static int read_ext_index_list(struct sock *sk, struct hci_dev *hdev,
count++;
}
- rp_len = sizeof(*rp) + (sizeof(rp->entry[0]) * count);
- rp = kmalloc(rp_len, GFP_ATOMIC);
+ rp = kmalloc(struct_size(rp, entry, count), GFP_ATOMIC);
if (!rp) {
read_unlock(&hci_dev_list_lock);
return -ENOMEM;
@@ -525,7 +523,6 @@ static int read_ext_index_list(struct sock *sk, struct hci_dev *hdev,
}
rp->num_controllers = cpu_to_le16(count);
- rp_len = sizeof(*rp) + (sizeof(rp->entry[0]) * count);
read_unlock(&hci_dev_list_lock);
@@ -538,7 +535,8 @@ static int read_ext_index_list(struct sock *sk, struct hci_dev *hdev,
hci_sock_clear_flag(sk, HCI_MGMT_UNCONF_INDEX_EVENTS);
err = mgmt_cmd_complete(sk, MGMT_INDEX_NONE,
- MGMT_OP_READ_EXT_INDEX_LIST, 0, rp, rp_len);
+ MGMT_OP_READ_EXT_INDEX_LIST, 0, rp,
+ struct_size(rp, entry, count));
kfree(rp);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next] mlxsw: spectrum: acl: Use struct_size() in kzalloc()
From: Ido Schimmel @ 2019-02-25 19:17 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Jiri Pirko, Ido Schimmel, David S. Miller, netdev, linux-kernel
In-Reply-To: <20190225190132.GA23478@embeddedor>
On Mon, Feb 25, 2019 at 01:01:32PM -0600, Gustavo A. R. Silva wrote:
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
>
> struct foo {
> int stuff;
> struct boo entry[];
> };
>
> size = sizeof(struct foo) + count * sizeof(struct boo);
> instance = kzalloc(size, GFP_KERNEL)
>
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
>
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL)
>
> Notice that, in this case, variable alloc_size is not necessary, hence
> it is removed.
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Thanks
^ permalink raw reply
* Re: [PATCH net-next v3 2/2] net: phy: aquantia: add hwmon support
From: Florian Fainelli @ 2019-02-25 19:19 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <47178fa9-1065-87f8-bcfa-59d8764d21f8@gmail.com>
On 2/25/19 10:56 AM, Heiner Kallweit wrote:
> This adds HWMON support for the temperature sensor and the related
> alarms on the 107/108/109 chips. This patch is based on work from
> Nikita and Andrew. I added:
> - support for changing alarm thresholds via sysfs
> - move HWMON code to a separate source file to improve maintainability
> - smaller changes like using IS_REACHABLE instead of ifdef
> (avoids problems if PHY driver is built in and HWMON is a module)
>
> v2:
> - remove struct aqr_priv
> - rename header file to aquantia.h
> v3:
> - add conditional compiling of aquantia_hwmon.c
> - improve converting sensor register values to/from long
> - add helper aqr_hwmon_test_bit
>
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* RE: [net-next 05/14] i40e: fix up 32 bit timespec references
From: Keller, Jacob E @ 2019-02-25 19:20 UTC (permalink / raw)
To: Arnd Bergmann, Kirsher, Jeffrey T
Cc: David Miller, Brandeburg, Jesse, Networking, nhorman@redhat.com,
sassmann@redhat.com, jogreene@redhat.com
In-Reply-To: <CAK8P3a3fSofgDdLZ3S+MaEVz8DHnX6HO6xGiRWfCjQ614Kf4Cg@mail.gmail.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
> Behalf Of Arnd Bergmann
> Sent: Monday, February 25, 2019 6:46 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: David Miller <davem@davemloft.net>; Brandeburg, Jesse
> <jesse.brandeburg@intel.com>; Networking <netdev@vger.kernel.org>;
> nhorman@redhat.com; sassmann@redhat.com; jogreene@redhat.com
> Subject: Re: [net-next 05/14] i40e: fix up 32 bit timespec references
>
> On Wed, Jul 26, 2017 at 12:33 PM Jeff Kirsher
> <jeffrey.t.kirsher@intel.com> wrote:
> >
> > From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> >
> > As it turns out there was only a small set of errors
> > on 32 bit, and we just needed to be using the right calls
> > for dealing with timespec64 variables.
>
> I just stumbled over code added by this older patch, and can't make sense
> of the commit description here. Was this an attempt to fix a bug, or
> just a cleanup?
>
> >
> > - then = ns_to_timespec64(delta);
> > mutex_lock(&pf->tmreg_lock);
> >
> > i40e_ptp_read(pf, &now);
> > - now = timespec64_add(now, then);
> > + timespec64_add_ns(&now, delta);
> > i40e_ptp_write(pf, (const struct timespec64 *)&now);
>
> The problem I noticed here is that 'delta' is a user provided 64-bit
> number from clock_adjtime(), and timespec64_add_ns() performs uses
> a repeated addition instead of a div/mod pair. When the number
> is large, we may end up adding a single second 8 billion times,
> which may take a while even on a fast CPU.
>
It looked like the timespec64_add_ns does a div/mod pair...? Or am I mis-reading how the function is implemented? Quite probably.
Either way, the code is incorrect, because timespec64_add_ns doesn't actually work with signed values. A negative delta actually ends up resulting in a significant positive addition. Woops!
> Should the commit 0ac30ce43323 ("i40e: fix up 32 bit timespec
> references") just be reverted?
>
Yea, let's revert it.
Thanks,
Jake
> Arnd
^ permalink raw reply
* Re: [PATCH v2 bpf-next 4/4] tools/bpftool: recognize bpf_prog_info runtime and runcnt
From: Andrii Nakryiko @ 2019-02-25 19:24 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, davem, edumazet, netdev, bpf, Kernel Team
In-Reply-To: <cf8b7ffc-22f9-a095-7614-2d191ba53522@iogearbox.net>
On Mon, Feb 25, 2019 at 3:17 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 02/23/2019 06:44 PM, Alexei Starovoitov wrote:
> > $ bpftool p s
> > 1: kprobe tag a56587d488d216c9 gpl runtime 79786 runcnt 8
> > loaded_at 2019-02-22T12:22:51-0800 uid 0
> > xlated 352B not jited memlock 4096B
> >
> > $ bpftool --json --pretty p s
> > [{
> > "id": 1,
> > "type": "kprobe",
> > "tag": "a56587d488d216c9",
> > "gpl_compatible": true,
> > "run_time_ns": 79786,
> > "run_cnt": 8,
> > "loaded_at": 1550866971,
> > "uid": 0,
> > "bytes_xlated": 352,
> > "jited": false,
> > "bytes_memlock": 4096
> > }
> > ]
> >
> > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> > ---
> > tools/bpf/bpftool/Documentation/bpftool-prog.rst | 4 +++-
> > tools/bpf/bpftool/prog.c | 7 +++++++
> > 2 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > index 12bc1e2d4b46..d8b46a8f17e0 100644
> > --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > @@ -171,7 +171,7 @@ EXAMPLES
> >
> > ::
> >
> > - 10: xdp name some_prog tag 005a3d2123620c8b gpl
> > + 10: xdp name some_prog tag 005a3d2123620c8b gpl runtime 81632 runcnt 10
> > loaded_at 2017-09-29T20:11:00+0000 uid 0
> > xlated 528B jited 370B memlock 4096B map_ids 10
> >
> > @@ -184,6 +184,8 @@ EXAMPLES
> > "type": "xdp",
> > "tag": "005a3d2123620c8b",
> > "gpl_compatible": true,
> > + "run_time_ns": 81632,
> > + "run_cnt": 10,
> > "loaded_at": 1506715860,
> > "uid": 0,
> > "bytes_xlated": 528,
> > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> > index db978c8d76a8..cdb6bd424340 100644
> > --- a/tools/bpf/bpftool/prog.c
> > +++ b/tools/bpf/bpftool/prog.c
> > @@ -214,6 +214,10 @@ static void print_prog_json(struct bpf_prog_info *info, int fd)
> > info->tag[4], info->tag[5], info->tag[6], info->tag[7]);
> >
> > jsonw_bool_field(json_wtr, "gpl_compatible", info->gpl_compatible);
> > + if (info->runtime) {
> > + jsonw_uint_field(json_wtr, "run_time_ns", info->runtime);
> > + jsonw_uint_field(json_wtr, "run_cnt", info->runcnt);
>
> Didn't Andrii mean to generally name these fields run_time_ns and run_cnt, so also
> uapi aside from just changing bpftool output (why keeping these names not the same)?
Yeah, I think consistency is important. This also applies to patch #1
(bpf: enable program stats), which exposes same stats through procfs.
Ideally all those names would be consistent (for obvious reasons). As
for _ns, I realize it's a bit longer and more verbose, but having time
units as part of a name is really great for users and doesn't require
extra documentation (and extra man lookups).
>
> > + }
> >
> > print_dev_json(info->ifindex, info->netns_dev, info->netns_ino);
> >
> > @@ -277,6 +281,9 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
> > fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
> > print_dev_plain(info->ifindex, info->netns_dev, info->netns_ino);
> > printf("%s", info->gpl_compatible ? " gpl" : "");
> > + if (info->runtime)
> > + printf(" runtime %lld runcnt %lld",
> > + info->runtime, info->runcnt);
> > printf("\n");
> >
> > if (info->load_time) {
> >
>
^ permalink raw reply
* Re: INFO: task hung in rtnetlink_rcv_msg
From: Jesper Dangaard Brouer @ 2019-02-25 19:26 UTC (permalink / raw)
To: Joe Perches
Cc: Daniel Borkmann, Jakub Kicinski, syzbot, ast, christian, davem,
dsahern, hawk, idosch, john.fastabend, kafai, ktkhai,
linux-kernel, netdev, petrm, roopa, songliubraving,
syzkaller-bugs, xdp-newbies, yhs, brouer
In-Reply-To: <57807dca3bf554d0a07356ba1e53fae4d58cfb51.camel@perches.com>
On Mon, 25 Feb 2019 08:55:40 -0800
Joe Perches <joe@perches.com> wrote:
> On Mon, 2019-02-25 at 09:39 +0100, Jesper Dangaard Brouer wrote:
> > On Sat, 23 Feb 2019 01:47:00 +0100
> > Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> > > On 02/22/2019 10:45 PM, Jakub Kicinski wrote:
> > > > On Fri, 22 Feb 2019 12:14:57 -0800, Joe Perches wrote:
> > > > > On Fri, 2019-02-22 at 12:01 -0800, Jakub Kicinski wrote:
> > > > > > Hi!
> > > > > >
> > > > > > Seems like something funny is going on with get_maintainer.pl since XDP
> > > > > > entry got added. We seem to have been CCed on:
> > > > >
> > > > > I suggest removing the MAINTAINERS line with
> > > > >
> > > > > K: xdp
> > > > >
> > > > > as xdp is a pretty generic file/patch content
> > > > > regex match for the K: type
> > > > >
> > > > > $ git grep --name-only xdp | wc -l
> > > > > 236
> >
> > I'm unsure how K: works, but you grep definitely selects some wrong files.
> >
> > I tried with "xdp_":
> > git grep --name-only xdp_
> >
> > That does catch all the driver that have XDP support, which is the
> > point of the exercise (to catch drivers).
> >
> > It does contain a couple of false-positives:
> > drivers/net/ethernet/neterion/vxge/vxge-traffic.c
> > drivers/thunderbolt/tb_msgs.h
> > drivers/thunderbolt/xdomain.c
> > sound/soc/codecs/rt5670.c
> >
> > Via the pattern '[^a-z]xdp_' I'm only left with the thunderbolt
> > false-positive, as it have a data struct's called tb_xdp_*.
>
> Then it's probably better to use
>
> K: \Wxdp_
>
> and if XDP_ is also to be matched, maybe
>
> K: (?i:\Wxdp_)
>
The last one seems to be the best at matching the relevant files :-)
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH net-next 7/8] net: switchdev: Replace port attr set SDO with a notification
From: Florian Fainelli @ 2019-02-25 19:47 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev@vger.kernel.org, David S. Miller, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE,
Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <20190225094940.GA15128@splinter>
On 2/25/19 1:49 AM, Ido Schimmel wrote:
> On Sun, Feb 24, 2019 at 08:47:27AM -0800, Florian Fainelli wrote:
>> Le 2/23/19 à 2:32 AM, Ido Schimmel a écrit :
>>> On Fri, Feb 22, 2019 at 03:59:25PM -0800, Florian Fainelli wrote:
>>>> - if (attr->flags & SWITCHDEV_F_NO_RECURSE)
>>>> + if (attr & SWITCHDEV_F_DEFER)
>>>> + rc = call_switchdev_blocking_notifiers(nt, dev,
>>>> + &attr_info.info, NULL);
>>>> + else
>>>> + rc = call_switchdev_notifiers(nt, dev, &attr_info.info, NULL);
>>>
>>> I don't believe this is needed. You're calling this function from
>>> switchdev_port_attr_set_now() which is always called from process
>>> context. switchdev_port_attr_set() takes care of that. Similar to
>>> switchdev_port_obj_add().
>>
>> Except for net/bridge/br_switchdev.c when we check the bridge port's
>> flags support with PRE_BRIDGE_FLAGS. In that case we are executing from
>> the caller (atomic) context and we can't defer otherwise that trumps the
>> whole idea of being able to do a quick check and return that to the
>> caller that we cannot support specific flags. How would you recommend
>> approaching that?
>
> In this case you can invoke call_switchdev_notifiers() directly from
> br_switchdev_set_port_flag(). Eventually switchdev_port_attr_set() will
> be gone and bridge code will invoke the notifiers directly.
That can be done, but it still requires the target driver (mlxsw,
ocelot, dsa, etc.) to support attribute notification from blocking and
non-blocking context. Are you fine with that?
--
Florian
^ permalink raw reply
* Re: [PATCH v2 bpf-next 2/4] bpf: expose program stats via bpf_prog_info
From: Andrii Nakryiko @ 2019-02-25 19:47 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: davem, Daniel Borkmann, edumazet, netdev, bpf, Kernel Team
In-Reply-To: <20190223174422.663270-3-ast@kernel.org>
On Sat, Feb 23, 2019 at 9:44 AM Alexei Starovoitov <ast@kernel.org> wrote:
>
> Return bpf program runtime and runcnt via bpf_prog_info
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> include/uapi/linux/bpf.h | 2 ++
> kernel/bpf/syscall.c | 5 +++++
> 2 files changed, 7 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index bcdd2474eee7..d2cb85d85b39 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2813,6 +2813,8 @@ struct bpf_prog_info {
> __u32 jited_line_info_rec_size;
> __u32 nr_prog_tags;
> __aligned_u64 prog_tags;
> + __u64 runtime;
> + __u64 runcnt;
> } __attribute__((aligned(8)));
>
> struct bpf_map_info {
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 9a8c990c5df4..3405e0a4cf77 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2152,6 +2152,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
> struct bpf_prog_info __user *uinfo = u64_to_user_ptr(attr->info.info);
> struct bpf_prog_info info = {};
> u32 info_len = attr->info.info_len;
> + struct bpf_prog_stats stats;
> char __user *uinsns;
> u32 ulen;
> int err;
> @@ -2191,6 +2192,10 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
> if (err)
> return err;
>
> + bpf_prog_get_stats(prog, &stats);
> + info.runtime = stats.nsecs;
> + info.runcnt = stats.cnt;
> +
lgtm, but see naming discussion in other thread.
Acked-by: Andrii Nakryiko <andriin@fb.com>
> if (!capable(CAP_SYS_ADMIN)) {
> info.jited_prog_len = 0;
> info.xlated_prog_len = 0;
> --
> 2.20.0
>
^ permalink raw reply
* Re: [PATCH 1/2] bpf: context casting for tail call
From: kbuild test robot @ 2019-02-25 20:10 UTC (permalink / raw)
To: Kris Van Hees; +Cc: kbuild-all, netdev
In-Reply-To: <201902251554.x1PFsWmr017326@userv0122.oracle.com>
[-- Attachment #1: Type: text/plain, Size: 1718 bytes --]
Hi Kris,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on bpf-next/master]
[also build test ERROR on v5.0-rc8 next-20190225]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Kris-Van-Hees/bpf-context-casting-for-tail-call-and-gtrace-prog-type/20190226-034827
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: x86_64-randconfig-x016-201908 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-20) 8.2.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
kernel/bpf/core.c: In function 'bpf_prog_array_can_cast':
>> kernel/bpf/core.c:1657:8: error: 'bpf_prog_types' undeclared (first use in this function); did you mean 'bpf_prog_type'?
ops = bpf_prog_types[array->owner_prog_type];
^~~~~~~~~~~~~~
bpf_prog_type
kernel/bpf/core.c:1657:8: note: each undeclared identifier is reported only once for each function it appears in
vim +1657 kernel/bpf/core.c
1649
1650 bool bpf_prog_array_can_cast(struct bpf_array *array, const struct bpf_prog *fp)
1651 {
1652 const struct bpf_prog_ops *ops;
1653
1654 if (array->owner_jited != fp->jited)
1655 return false;
1656
> 1657 ops = bpf_prog_types[array->owner_prog_type];
1658 if (ops->can_cast)
1659 return ops->can_cast(fp->type, array->owner_prog_type);
1660
1661 return false;
1662 }
1663
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28326 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] bpf: context casting for tail call
From: kbuild test robot @ 2019-02-25 20:10 UTC (permalink / raw)
To: Kris Van Hees; +Cc: kbuild-all, netdev
In-Reply-To: <201902251554.x1PFsWmr017326@userv0122.oracle.com>
[-- Attachment #1: Type: text/plain, Size: 8559 bytes --]
Hi Kris,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on bpf-next/master]
[also build test WARNING on v5.0-rc8 next-20190225]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Kris-Van-Hees/bpf-context-casting-for-tail-call-and-gtrace-prog-type/20190226-034827
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=xtensa
All warnings (new ones prefixed by >>):
kernel//bpf/core.c: In function '___bpf_prog_run':
>> kernel//bpf/core.c:1452:13: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
BPF_R1 = (u64) ctx;
^
vim +1452 kernel//bpf/core.c
1290
1291 select_insn:
1292 goto *jumptable[insn->code];
1293
1294 /* ALU */
1295 #define ALU(OPCODE, OP) \
1296 ALU64_##OPCODE##_X: \
1297 DST = DST OP SRC; \
1298 CONT; \
1299 ALU_##OPCODE##_X: \
1300 DST = (u32) DST OP (u32) SRC; \
1301 CONT; \
1302 ALU64_##OPCODE##_K: \
1303 DST = DST OP IMM; \
1304 CONT; \
1305 ALU_##OPCODE##_K: \
1306 DST = (u32) DST OP (u32) IMM; \
1307 CONT;
1308
1309 ALU(ADD, +)
1310 ALU(SUB, -)
1311 ALU(AND, &)
1312 ALU(OR, |)
1313 ALU(LSH, <<)
1314 ALU(RSH, >>)
1315 ALU(XOR, ^)
1316 ALU(MUL, *)
1317 #undef ALU
1318 ALU_NEG:
1319 DST = (u32) -DST;
1320 CONT;
1321 ALU64_NEG:
1322 DST = -DST;
1323 CONT;
1324 ALU_MOV_X:
1325 DST = (u32) SRC;
1326 CONT;
1327 ALU_MOV_K:
1328 DST = (u32) IMM;
1329 CONT;
1330 ALU64_MOV_X:
1331 DST = SRC;
1332 CONT;
1333 ALU64_MOV_K:
1334 DST = IMM;
1335 CONT;
1336 LD_IMM_DW:
1337 DST = (u64) (u32) insn[0].imm | ((u64) (u32) insn[1].imm) << 32;
1338 insn++;
1339 CONT;
1340 ALU_ARSH_X:
1341 DST = (u64) (u32) ((*(s32 *) &DST) >> SRC);
1342 CONT;
1343 ALU_ARSH_K:
1344 DST = (u64) (u32) ((*(s32 *) &DST) >> IMM);
1345 CONT;
1346 ALU64_ARSH_X:
1347 (*(s64 *) &DST) >>= SRC;
1348 CONT;
1349 ALU64_ARSH_K:
1350 (*(s64 *) &DST) >>= IMM;
1351 CONT;
1352 ALU64_MOD_X:
1353 div64_u64_rem(DST, SRC, &AX);
1354 DST = AX;
1355 CONT;
1356 ALU_MOD_X:
1357 AX = (u32) DST;
1358 DST = do_div(AX, (u32) SRC);
1359 CONT;
1360 ALU64_MOD_K:
1361 div64_u64_rem(DST, IMM, &AX);
1362 DST = AX;
1363 CONT;
1364 ALU_MOD_K:
1365 AX = (u32) DST;
1366 DST = do_div(AX, (u32) IMM);
1367 CONT;
1368 ALU64_DIV_X:
1369 DST = div64_u64(DST, SRC);
1370 CONT;
1371 ALU_DIV_X:
1372 AX = (u32) DST;
1373 do_div(AX, (u32) SRC);
1374 DST = (u32) AX;
1375 CONT;
1376 ALU64_DIV_K:
1377 DST = div64_u64(DST, IMM);
1378 CONT;
1379 ALU_DIV_K:
1380 AX = (u32) DST;
1381 do_div(AX, (u32) IMM);
1382 DST = (u32) AX;
1383 CONT;
1384 ALU_END_TO_BE:
1385 switch (IMM) {
1386 case 16:
1387 DST = (__force u16) cpu_to_be16(DST);
1388 break;
1389 case 32:
1390 DST = (__force u32) cpu_to_be32(DST);
1391 break;
1392 case 64:
1393 DST = (__force u64) cpu_to_be64(DST);
1394 break;
1395 }
1396 CONT;
1397 ALU_END_TO_LE:
1398 switch (IMM) {
1399 case 16:
1400 DST = (__force u16) cpu_to_le16(DST);
1401 break;
1402 case 32:
1403 DST = (__force u32) cpu_to_le32(DST);
1404 break;
1405 case 64:
1406 DST = (__force u64) cpu_to_le64(DST);
1407 break;
1408 }
1409 CONT;
1410
1411 /* CALL */
1412 JMP_CALL:
1413 /* Function call scratches BPF_R1-BPF_R5 registers,
1414 * preserves BPF_R6-BPF_R9, and stores return value
1415 * into BPF_R0.
1416 */
1417 BPF_R0 = (__bpf_call_base + insn->imm)(BPF_R1, BPF_R2, BPF_R3,
1418 BPF_R4, BPF_R5);
1419 CONT;
1420
1421 JMP_CALL_ARGS:
1422 BPF_R0 = (__bpf_call_base_args + insn->imm)(BPF_R1, BPF_R2,
1423 BPF_R3, BPF_R4,
1424 BPF_R5,
1425 insn + insn->off + 1);
1426 CONT;
1427
1428 JMP_TAIL_CALL: {
1429 void *ctx = (void *) (unsigned long) BPF_R1;
1430 struct bpf_map *map = (struct bpf_map *) (unsigned long) BPF_R2;
1431 struct bpf_array *array = container_of(map, struct bpf_array, map);
1432 struct bpf_prog *prog;
1433 u32 index = BPF_R3;
1434 u32 type = BPF_R4;
1435
1436 if (unlikely(index >= array->map.max_entries))
1437 goto out;
1438 if (unlikely(tail_call_cnt > MAX_TAIL_CALL_CNT))
1439 goto out;
1440
1441 tail_call_cnt++;
1442
1443 prog = READ_ONCE(array->ptrs[index]);
1444 if (!prog)
1445 goto out;
1446 if (prog->aux->ops->cast_context) {
1447 ctx = prog->aux->ops->cast_context(type, prog->type,
1448 ctx);
1449 if (!ctx)
1450 goto out;
1451
> 1452 BPF_R1 = (u64) ctx;
1453 }
1454
1455 /* ARG1 at this point is guaranteed to point to CTX from
1456 * the verifier side due to the fact that the tail call is
1457 * handeled like a helper, that is, bpf_tail_call_proto,
1458 * where arg1_type is ARG_PTR_TO_CTX.
1459 */
1460 insn = prog->insnsi;
1461 goto select_insn;
1462 out:
1463 CONT;
1464 }
1465 JMP_JA:
1466 insn += insn->off;
1467 CONT;
1468 JMP_EXIT:
1469 return BPF_R0;
1470 /* JMP */
1471 #define COND_JMP(SIGN, OPCODE, CMP_OP) \
1472 JMP_##OPCODE##_X: \
1473 if ((SIGN##64) DST CMP_OP (SIGN##64) SRC) { \
1474 insn += insn->off; \
1475 CONT_JMP; \
1476 } \
1477 CONT; \
1478 JMP32_##OPCODE##_X: \
1479 if ((SIGN##32) DST CMP_OP (SIGN##32) SRC) { \
1480 insn += insn->off; \
1481 CONT_JMP; \
1482 } \
1483 CONT; \
1484 JMP_##OPCODE##_K: \
1485 if ((SIGN##64) DST CMP_OP (SIGN##64) IMM) { \
1486 insn += insn->off; \
1487 CONT_JMP; \
1488 } \
1489 CONT; \
1490 JMP32_##OPCODE##_K: \
1491 if ((SIGN##32) DST CMP_OP (SIGN##32) IMM) { \
1492 insn += insn->off; \
1493 CONT_JMP; \
1494 } \
1495 CONT;
1496 COND_JMP(u, JEQ, ==)
1497 COND_JMP(u, JNE, !=)
1498 COND_JMP(u, JGT, >)
1499 COND_JMP(u, JLT, <)
1500 COND_JMP(u, JGE, >=)
1501 COND_JMP(u, JLE, <=)
1502 COND_JMP(u, JSET, &)
1503 COND_JMP(s, JSGT, >)
1504 COND_JMP(s, JSLT, <)
1505 COND_JMP(s, JSGE, >=)
1506 COND_JMP(s, JSLE, <=)
1507 #undef COND_JMP
1508 /* STX and ST and LDX*/
1509 #define LDST(SIZEOP, SIZE) \
1510 STX_MEM_##SIZEOP: \
1511 *(SIZE *)(unsigned long) (DST + insn->off) = SRC; \
1512 CONT; \
1513 ST_MEM_##SIZEOP: \
1514 *(SIZE *)(unsigned long) (DST + insn->off) = IMM; \
1515 CONT; \
1516 LDX_MEM_##SIZEOP: \
1517 DST = *(SIZE *)(unsigned long) (SRC + insn->off); \
1518 CONT;
1519
1520 LDST(B, u8)
1521 LDST(H, u16)
1522 LDST(W, u32)
1523 LDST(DW, u64)
1524 #undef LDST
1525 STX_XADD_W: /* lock xadd *(u32 *)(dst_reg + off16) += src_reg */
1526 atomic_add((u32) SRC, (atomic_t *)(unsigned long)
1527 (DST + insn->off));
1528 CONT;
1529 STX_XADD_DW: /* lock xadd *(u64 *)(dst_reg + off16) += src_reg */
1530 atomic64_add((u64) SRC, (atomic64_t *)(unsigned long)
1531 (DST + insn->off));
1532 CONT;
1533
1534 default_label:
1535 /* If we ever reach this, we have a bug somewhere. Die hard here
1536 * instead of just returning 0; we could be somewhere in a subprog,
1537 * so execution could continue otherwise which we do /not/ want.
1538 *
1539 * Note, verifier whitelists all opcodes in bpf_opcode_in_insntable().
1540 */
1541 pr_warn("BPF interpreter: unknown opcode %02x\n", insn->code);
1542 BUG_ON(1);
1543 return 0;
1544 }
1545 STACK_FRAME_NON_STANDARD(___bpf_prog_run); /* jump table */
1546
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56355 bytes --]
^ permalink raw reply
* Re: [PATCH] tun: fix blocking read
From: Timur Celik @ 2019-02-25 20:12 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev, linux-kernel
In-Reply-To: <20190225.102548.247362521843809607.davem@davemloft.net>
On Mon, Feb 25, 2019 at 10:25:48AM -0800, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 25 Feb 2019 07:53:08 -0800
>
>>
>>
>> On 02/24/2019 10:12 PM, David Miller wrote:
>>> From: Timur Celik <mail@timurcelik.de>
>>> Date: Sat, 23 Feb 2019 12:53:13 +0100
>>>
>>>> This patch moves setting of the current state into the loop. Otherwise
>>>> the task may end up in a busy wait loop if none of the break conditions
>>>> are met.
>>>>
>>>> Signed-off-by: Timur Celik <mail@timurcelik.de>
>>>
>>> Applied and queued up for -stable, thanks.
>>>
>>
>> First part of the patch was matching the changelog, however
>> the second change was not really needed, or should have used
>>
>> __set_current_state(TASK_RUNNING);
>
> Timur, please followup.
Thanks for taking a look at this. Eric is right of course.
^ permalink raw reply
* [PATCH] tun: remove unnecessary memory barrier
From: Timur Celik @ 2019-02-25 20:13 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev, linux-kernel
In-Reply-To: <20190225.102548.247362521843809607.davem@davemloft.net>
Replace set_current_state with __set_current_state since no memory
barrier is needed at this point.
Signed-off-by: Timur Celik <mail@timurcelik.de>
---
drivers/net/tun.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index d291762b9e9d..53f4f37b0ffd 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2185,7 +2185,7 @@ static void *tun_ring_recv(struct tun_file *tfile, int noblock, int *err)
schedule();
}
- set_current_state(TASK_RUNNING);
+ __set_current_state(TASK_RUNNING);
remove_wait_queue(&tfile->wq.wait, &wait);
out:
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] tun: remove unnecessary memory barrier
From: Eric Dumazet @ 2019-02-25 20:15 UTC (permalink / raw)
To: Timur Celik, David Miller; +Cc: netdev, linux-kernel
In-Reply-To: <20190225201313.GA30005@timur-laptop.fritz.box>
On 02/25/2019 12:13 PM, Timur Celik wrote:
> Replace set_current_state with __set_current_state since no memory
> barrier is needed at this point.
>
> Signed-off-by: Timur Celik <mail@timurcelik.de>
> ---
> drivers/net/tun.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
SGTM, thanks.
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH net] ipvs: get sctphdr by sctphoff in sctp_csum_check
From: Julian Anastasov @ 2019-02-25 20:19 UTC (permalink / raw)
To: Xin Long
Cc: network dev, netfilter-devel, Marcelo Ricardo Leitner,
Neil Horman, pablo, Simon Horman, lvs-devel
In-Reply-To: <cd1c829cfd9ee916e57c51f6879947f0f88c1eb1.1551094063.git.lucien.xin@gmail.com>
Hello,
On Mon, 25 Feb 2019, Xin Long wrote:
> sctp_csum_check() is called by sctp_s/dnat_handler() where it calls
> skb_make_writable() to ensure sctphdr to be linearized.
>
> So there's no need to get sctphdr by calling skb_header_pointer()
> in sctp_csum_check().
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Looks good to me, thanks!
Acked-by: Julian Anastasov <ja@ssi.bg>
I guess, it is for the nf-next/net-next tree because it just
eliminates a duplicate check.
> ---
> net/netfilter/ipvs/ip_vs_proto_sctp.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> index b0cd7d0..0ecf241 100644
> --- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
> +++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> @@ -183,7 +183,7 @@ static int
> sctp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp)
> {
> unsigned int sctphoff;
> - struct sctphdr *sh, _sctph;
> + struct sctphdr *sh;
> __le32 cmp, val;
>
> #ifdef CONFIG_IP_VS_IPV6
> @@ -193,10 +193,7 @@ sctp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp)
> #endif
> sctphoff = ip_hdrlen(skb);
>
> - sh = skb_header_pointer(skb, sctphoff, sizeof(_sctph), &_sctph);
> - if (sh == NULL)
> - return 0;
> -
> + sh = (struct sctphdr *)(skb->data + sctphoff);
> cmp = sh->checksum;
> val = sctp_compute_cksum(skb, sctphoff);
>
> --
> 2.1.0
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [RFC bpf-next 0/7] net: flow_dissector: trigger BPF hook when called from eth_get_headlen
From: Stanislav Fomichev @ 2019-02-25 20:33 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Willem de Bruijn, Stanislav Fomichev, Network Development,
David Miller, Alexei Starovoitov, Daniel Borkmann, simon.horman,
Willem de Bruijn
In-Reply-To: <20190214173542.GA20651@mini-arch>
On 02/14, Stanislav Fomichev wrote:
> On 02/13, Alexei Starovoitov wrote:
> > On Wed, Feb 13, 2019 at 09:57:25PM -0800, Stanislav Fomichev wrote:
> > >
> > > > That 'stuck with __sk_buff' is what bothers me.
> > > I might have use the wrong word here. I don't think there is another
> > > option to be honest. Using __sk_buff makes flow dissector programs work
> > > with fragmented packets;
> >
> > good point. indeed real skb is essential.
> >
> > > > It's an indication that api wasn't thought through if first thing
> > > > it needs is this fake skb hack.
> > > > If bpf_flow.c is a realistic example of such flow dissector prog
> > > > it means that real skb fields are accessed.
> > > > In particular skb->vlan_proto, skb->protocol.
> > > I do manually set skb->protocol to eth->h_proto in my proposal. This is later
> > > correctly handled by bpf_flow.c: parse_eth_proto() is called on skb->protocol
> > > and we correctly handle bpf_htons(ETH_P_8021Q) there. So existing
> > > bpf_flow.c works as expected.
> > ...
> > > The goal of this patch series was to essentially make this skb/no-skb
> > > context transparent to the bpf_flow.c (i.e. no changes from the user
> > > flow programs). Adding another flow dissector for eth_get_headlen case
> > > also seems as a no go.
> >
> > The problem with this thinking is assumption that bpf_flow.c is the only program.
> I agree, it's a bad assumption, but it is sort of a reference implementation,
> I don't expect other users to do something wildly different. Hopefully :-)
>
> > Since ctx of flow_dissector prog type is 'struct __sk_buff'
> > all fields should be valid or the verifier has to reject access
> > to fields that were not set.
> > You cannot "manually set skb->protocol to eth->h_proto" in fake skb
> > and ignore the rest.
> Ugh, I did expect that we only allow a minimal set of __sk_buff fields
> to be allowed from the flow dissector program type, but that's not the
> case. We explicitly prohibit access only to
> family/ips/ports/tc_classid/tstamp/wire_len, everything else is readable :-/
> Any idea why?
> Stuff like ingress_ifindex/ifindex/hash/mark/queue_mapping, does flow dissector
> programs really need to know that?
>
> For the most part, using zero-initialized fake skb looks fine, except:
> * infindex, where we do skb->dev->ifndex (skb->dev is NULL)
> * gso_segs, where we do skb_shinfo(skb)->gso_segs (we are missing
> shinfo)
>
> So there is indeed a couple of problems.
>
> How do you feel about tightening down the access to sk_buff fields from
> the flow dissector program type? That is an API change, but I don't see why
> existing users should use those fields. Let's allow access only to
> len/data/data_end, protocol, vlan_{present,tci,proto}, cb, flow_keys,
> that should be enough to dissect the packet (I also looked at C-based
> implementation, it doesn't use anything besides that).
> We can always rollback if somebody complains about.
To revive the conversation, here is what I was thinking about (whitelist
the skb fields, not blacklist them):
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -6591,11 +6591,14 @@ static bool flow_dissector_is_valid_access(int off, int size,
case bpf_ctx_range_ptr(struct __sk_buff, flow_keys):
info->reg_type = PTR_TO_FLOW_KEYS;
break;
- case bpf_ctx_range(struct __sk_buff, tc_classid):
- case bpf_ctx_range(struct __sk_buff, data_meta):
- case bpf_ctx_range_till(struct __sk_buff, family, local_port):
- case bpf_ctx_range(struct __sk_buff, tstamp):
- case bpf_ctx_range(struct __sk_buff, wire_len):
+ case bpf_ctx_range(struct __sk_buff, len):
+ case bpf_ctx_range(struct __sk_buff, protocol):
+ case bpf_ctx_range(struct __sk_buff, vlan_present):
+ case bpf_ctx_range(struct __sk_buff, vlan_tci):
+ case bpf_ctx_range(struct __sk_buff, vlan_proto):
+ case bpf_ctx_range_till(struct __sk_buff, cb[0], cb[4]):
+ break;
+ default:
return false;
}
What do you think?
^ permalink raw reply
* Re: [PATCH 2/2] bpf: initial implementation for BPF_PROG_TYPE_GTRACE
From: kbuild test robot @ 2019-02-25 20:57 UTC (permalink / raw)
To: Kris Van Hees; +Cc: kbuild-all, netdev
In-Reply-To: <201902251554.x1PFsj2G026501@aserv0121.oracle.com>
[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]
Hi Kris,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on bpf-next/master]
[also build test ERROR on next-20190225]
[cannot apply to v5.0-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Kris-Van-Hees/bpf-context-casting-for-tail-call-and-gtrace-prog-type/20190226-034827
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: riscv-defconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=riscv
All errors (new ones prefixed by >>):
kernel/bpf/syscall.o: In function `.LANCHOR1':
>> syscall.c:(.rodata+0x3d8): undefined reference to `gtrace_prog_ops'
kernel/bpf/verifier.o: In function `.LANCHOR1':
>> verifier.c:(.rodata+0x4c0): undefined reference to `gtrace_verifier_ops'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 16821 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox