* Re: [PATCH] liquidio: fix duplicated code for different branches
From: David Miller @ 2017-08-14 17:58 UTC (permalink / raw)
To: gustavo
Cc: derek.chickles, satananda.burla, felix.manlunas, raghu.vatsavayi,
netdev, linux-kernel
In-Reply-To: <20170813013855.GA6686@embeddedgus>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Sat, 12 Aug 2017 20:38:55 -0500
> Refactor code in order to avoid identical code for different branches.
>
> This issue was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied.
^ permalink raw reply
* Re: [PATCH] qlge: fix duplicated code for different branches
From: David Miller @ 2017-08-14 17:58 UTC (permalink / raw)
To: gustavo
Cc: harish.patil, manish.chopra, Dept-GELinuxNICDev, netdev,
linux-kernel
In-Reply-To: <20170813015840.GA8443@embeddedgus>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Sat, 12 Aug 2017 20:58:40 -0500
> Refactor code in order to avoid identical code for different branches.
>
> This issue was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] liquidio: update debug console logging mechanism
From: David Miller @ 2017-08-14 17:57 UTC (permalink / raw)
To: felix.manlunas
Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
ricardo.farrington
In-Reply-To: <20170812014314.GA1012@felix-thinkpad.cavium.com>
From: Felix Manlunas <felix.manlunas@cavium.com>
Date: Fri, 11 Aug 2017 18:43:14 -0700
> From: Rick Farrington <ricardo.farrington@cavium.com>
>
> - remove logging dependency upon global func octeon_console_debug_enabled()
> - abstract debug console logging using console structure (via function ptr)
> to allow for more flexible logging
>
> Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
> Signed-off-by: Raghu Vatsavayi <raghu.vatsavayi@cavium.com>
> Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
Applied, thanks.
^ permalink raw reply
* [PATCH net-next] bpf/verifier: track liveness for pruning
From: Edward Cree @ 2017-08-14 17:55 UTC (permalink / raw)
To: davem, Alexei Starovoitov, Alexei Starovoitov, Daniel Borkmann
Cc: netdev, linux-kernel, iovisor-dev
State of a register doesn't matter if it wasn't read in reaching an exit;
a write screens off all reads downstream of it from all explored_states
upstream of it.
This allows us to prune many more branches; here are some processed insn
counts for some Cilium programs:
Program before after
bpf_lb_opt_-DLB_L3.o 6515 3361
bpf_lb_opt_-DLB_L4.o 8976 5176
bpf_lb_opt_-DUNKNOWN.o 2960 1137
bpf_lxc_opt_-DDROP_ALL.o 95412 48537
bpf_lxc_opt_-DUNKNOWN.o 141706 79048
bpf_netdev.o 24251 17995
bpf_overlay.o 10999 9385
The runtime is also improved; here are 'time' results in ms:
Program before after
bpf_lb_opt_-DLB_L3.o 24 6
bpf_lb_opt_-DLB_L4.o 26 11
bpf_lb_opt_-DUNKNOWN.o 11 2
bpf_lxc_opt_-DDROP_ALL.o 1288 152
bpf_lxc_opt_-DUNKNOWN.o 1768 257
bpf_netdev.o 62 31
bpf_overlay.o 15 13
Signed-off-by: Edward Cree <ecree@solarflare.com>
---
include/linux/bpf_verifier.h | 11 ++-
kernel/bpf/verifier.c | 181 +++++++++++++++++++++++++++++++++----------
2 files changed, 151 insertions(+), 41 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index c61c3033..91d07ef 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -21,6 +21,12 @@
*/
#define BPF_MAX_VAR_SIZ INT_MAX
+enum bpf_reg_liveness {
+ REG_LIVE_NONE = 0, /* reg hasn't been read or written this branch */
+ REG_LIVE_READ, /* reg was read, so we're sensitive to initial value */
+ REG_LIVE_WRITTEN, /* reg was written first, screening off later reads */
+};
+
struct bpf_reg_state {
enum bpf_reg_type type;
union {
@@ -40,7 +46,7 @@ struct bpf_reg_state {
* came from, when one is tested for != NULL.
*/
u32 id;
- /* These five fields must be last. See states_equal() */
+ /* Ordering of fields matters. See states_equal() */
/* For scalar types (SCALAR_VALUE), this represents our knowledge of
* the actual value.
* For pointer types, this represents the variable part of the offset
@@ -57,6 +63,8 @@ struct bpf_reg_state {
s64 smax_value; /* maximum possible (s64)value */
u64 umin_value; /* minimum possible (u64)value */
u64 umax_value; /* maximum possible (u64)value */
+ /* This field must be last, for states_equal() reasons. */
+ enum bpf_reg_liveness live;
};
enum bpf_stack_slot_type {
@@ -74,6 +82,7 @@ struct bpf_verifier_state {
struct bpf_reg_state regs[MAX_BPF_REG];
u8 stack_slot_type[MAX_BPF_STACK];
struct bpf_reg_state spilled_regs[MAX_BPF_STACK / BPF_REG_SIZE];
+ struct bpf_verifier_state *parent;
};
/* linked list of verifier states used to prune search */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ecc590e..dcfe6ab 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -629,8 +629,10 @@ static void init_reg_state(struct bpf_reg_state *regs)
{
int i;
- for (i = 0; i < MAX_BPF_REG; i++)
+ for (i = 0; i < MAX_BPF_REG; i++) {
mark_reg_not_init(regs, i);
+ regs[i].live = REG_LIVE_NONE;
+ }
/* frame pointer */
regs[BPF_REG_FP].type = PTR_TO_STACK;
@@ -647,9 +649,26 @@ enum reg_arg_type {
DST_OP_NO_MARK /* same as above, check only, don't mark */
};
-static int check_reg_arg(struct bpf_reg_state *regs, u32 regno,
+static void mark_reg_read(const struct bpf_verifier_state *state, u32 regno)
+{
+ struct bpf_verifier_state *parent = state->parent;
+
+ while (parent) {
+ /* if read wasn't screened by an earlier write ... */
+ if (state->regs[regno].live & REG_LIVE_WRITTEN)
+ break;
+ /* ... then we depend on parent's value */
+ parent->regs[regno].live |= REG_LIVE_READ;
+ state = parent;
+ parent = state->parent;
+ }
+}
+
+static int check_reg_arg(struct bpf_verifier_env *env, u32 regno,
enum reg_arg_type t)
{
+ struct bpf_reg_state *regs = env->cur_state.regs;
+
if (regno >= MAX_BPF_REG) {
verbose("R%d is invalid\n", regno);
return -EINVAL;
@@ -661,12 +680,14 @@ static int check_reg_arg(struct bpf_reg_state *regs, u32 regno,
verbose("R%d !read_ok\n", regno);
return -EACCES;
}
+ mark_reg_read(&env->cur_state, regno);
} else {
/* check whether register used as dest operand can be written to */
if (regno == BPF_REG_FP) {
verbose("frame pointer is read only\n");
return -EACCES;
}
+ regs[regno].live |= REG_LIVE_WRITTEN;
if (t == DST_OP)
mark_reg_unknown(regs, regno);
}
@@ -695,7 +716,7 @@ static bool is_spillable_regtype(enum bpf_reg_type type)
static int check_stack_write(struct bpf_verifier_state *state, int off,
int size, int value_regno)
{
- int i;
+ int i, spi = (MAX_BPF_STACK + off) / BPF_REG_SIZE;
/* caller checked that off % size == 0 and -MAX_BPF_STACK <= off < 0,
* so it's aligned access and [off, off + size) are within stack limits
*/
@@ -710,15 +731,14 @@ static int check_stack_write(struct bpf_verifier_state *state, int off,
}
/* save register state */
- state->spilled_regs[(MAX_BPF_STACK + off) / BPF_REG_SIZE] =
- state->regs[value_regno];
+ state->spilled_regs[spi] = state->regs[value_regno];
+ state->spilled_regs[spi].live |= REG_LIVE_WRITTEN;
for (i = 0; i < BPF_REG_SIZE; i++)
state->stack_slot_type[MAX_BPF_STACK + off + i] = STACK_SPILL;
} else {
/* regular write of data into stack */
- state->spilled_regs[(MAX_BPF_STACK + off) / BPF_REG_SIZE] =
- (struct bpf_reg_state) {};
+ state->spilled_regs[spi] = (struct bpf_reg_state) {};
for (i = 0; i < size; i++)
state->stack_slot_type[MAX_BPF_STACK + off + i] = STACK_MISC;
@@ -726,11 +746,26 @@ static int check_stack_write(struct bpf_verifier_state *state, int off,
return 0;
}
+static void mark_stack_slot_read(const struct bpf_verifier_state *state, int slot)
+{
+ struct bpf_verifier_state *parent = state->parent;
+
+ while (parent) {
+ /* if read wasn't screened by an earlier write ... */
+ if (state->spilled_regs[slot].live & REG_LIVE_WRITTEN)
+ break;
+ /* ... then we depend on parent's value */
+ parent->spilled_regs[slot].live |= REG_LIVE_READ;
+ state = parent;
+ parent = state->parent;
+ }
+}
+
static int check_stack_read(struct bpf_verifier_state *state, int off, int size,
int value_regno)
{
u8 *slot_type;
- int i;
+ int i, spi;
slot_type = &state->stack_slot_type[MAX_BPF_STACK + off];
@@ -746,10 +781,13 @@ static int check_stack_read(struct bpf_verifier_state *state, int off, int size,
}
}
- if (value_regno >= 0)
+ spi = (MAX_BPF_STACK + off) / BPF_REG_SIZE;
+
+ if (value_regno >= 0) {
/* restore register state from stack */
- state->regs[value_regno] =
- state->spilled_regs[(MAX_BPF_STACK + off) / BPF_REG_SIZE];
+ state->regs[value_regno] = state->spilled_regs[spi];
+ mark_stack_slot_read(state, spi);
+ }
return 0;
} else {
for (i = 0; i < size; i++) {
@@ -1167,7 +1205,6 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
static int check_xadd(struct bpf_verifier_env *env, int insn_idx, struct bpf_insn *insn)
{
- struct bpf_reg_state *regs = env->cur_state.regs;
int err;
if ((BPF_SIZE(insn->code) != BPF_W && BPF_SIZE(insn->code) != BPF_DW) ||
@@ -1177,12 +1214,12 @@ static int check_xadd(struct bpf_verifier_env *env, int insn_idx, struct bpf_ins
}
/* check src1 operand */
- err = check_reg_arg(regs, insn->src_reg, SRC_OP);
+ err = check_reg_arg(env, insn->src_reg, SRC_OP);
if (err)
return err;
/* check src2 operand */
- err = check_reg_arg(regs, insn->dst_reg, SRC_OP);
+ err = check_reg_arg(env, insn->dst_reg, SRC_OP);
if (err)
return err;
@@ -1297,10 +1334,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
if (arg_type == ARG_DONTCARE)
return 0;
- if (type == NOT_INIT) {
- verbose("R%d !read_ok\n", regno);
- return -EACCES;
- }
+ err = check_reg_arg(env, regno, SRC_OP);
+ if (err)
+ return err;
if (arg_type == ARG_ANYTHING) {
if (is_pointer_value(env, regno)) {
@@ -1639,10 +1675,13 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx)
}
/* reset caller saved regs */
- for (i = 0; i < CALLER_SAVED_REGS; i++)
+ for (i = 0; i < CALLER_SAVED_REGS; i++) {
mark_reg_not_init(regs, caller_saved[i]);
+ check_reg_arg(env, i, DST_OP_NO_MARK);
+ }
/* update return register */
+ check_reg_arg(env, BPF_REG_0, DST_OP_NO_MARK);
if (fn->ret_type == RET_INTEGER) {
/* sets type to SCALAR_VALUE */
mark_reg_unknown(regs, BPF_REG_0);
@@ -2250,7 +2289,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
}
/* check src operand */
- err = check_reg_arg(regs, insn->dst_reg, SRC_OP);
+ err = check_reg_arg(env, insn->dst_reg, SRC_OP);
if (err)
return err;
@@ -2261,7 +2300,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
}
/* check dest operand */
- err = check_reg_arg(regs, insn->dst_reg, DST_OP);
+ err = check_reg_arg(env, insn->dst_reg, DST_OP);
if (err)
return err;
@@ -2274,7 +2313,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
}
/* check src operand */
- err = check_reg_arg(regs, insn->src_reg, SRC_OP);
+ err = check_reg_arg(env, insn->src_reg, SRC_OP);
if (err)
return err;
} else {
@@ -2285,7 +2324,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
}
/* check dest operand */
- err = check_reg_arg(regs, insn->dst_reg, DST_OP);
+ err = check_reg_arg(env, insn->dst_reg, DST_OP);
if (err)
return err;
@@ -2328,7 +2367,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
return -EINVAL;
}
/* check src1 operand */
- err = check_reg_arg(regs, insn->src_reg, SRC_OP);
+ err = check_reg_arg(env, insn->src_reg, SRC_OP);
if (err)
return err;
} else {
@@ -2339,7 +2378,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
}
/* check src2 operand */
- err = check_reg_arg(regs, insn->dst_reg, SRC_OP);
+ err = check_reg_arg(env, insn->dst_reg, SRC_OP);
if (err)
return err;
@@ -2360,7 +2399,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
}
/* check dest operand */
- err = check_reg_arg(regs, insn->dst_reg, DST_OP_NO_MARK);
+ err = check_reg_arg(env, insn->dst_reg, DST_OP_NO_MARK);
if (err)
return err;
@@ -2717,7 +2756,7 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
}
/* check src1 operand */
- err = check_reg_arg(regs, insn->src_reg, SRC_OP);
+ err = check_reg_arg(env, insn->src_reg, SRC_OP);
if (err)
return err;
@@ -2734,7 +2773,7 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
}
/* check src2 operand */
- err = check_reg_arg(regs, insn->dst_reg, SRC_OP);
+ err = check_reg_arg(env, insn->dst_reg, SRC_OP);
if (err)
return err;
@@ -2851,7 +2890,7 @@ static int check_ld_imm(struct bpf_verifier_env *env, struct bpf_insn *insn)
return -EINVAL;
}
- err = check_reg_arg(regs, insn->dst_reg, DST_OP);
+ err = check_reg_arg(env, insn->dst_reg, DST_OP);
if (err)
return err;
@@ -2917,7 +2956,7 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
}
/* check whether implicit source operand (register R6) is readable */
- err = check_reg_arg(regs, BPF_REG_6, SRC_OP);
+ err = check_reg_arg(env, BPF_REG_6, SRC_OP);
if (err)
return err;
@@ -2928,7 +2967,7 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
if (mode == BPF_IND) {
/* check explicit source operand */
- err = check_reg_arg(regs, insn->src_reg, SRC_OP);
+ err = check_reg_arg(env, insn->src_reg, SRC_OP);
if (err)
return err;
}
@@ -3194,7 +3233,11 @@ static bool regsafe(struct bpf_reg_state *rold,
struct bpf_reg_state *rcur,
bool varlen_map_access, struct idpair *idmap)
{
- if (memcmp(rold, rcur, sizeof(*rold)) == 0)
+ if (!(rold->live & REG_LIVE_READ))
+ /* explored state didn't use this */
+ return true;
+
+ if (memcmp(rold, rcur, offsetof(struct bpf_reg_state, live)) == 0)
return true;
if (rold->type == NOT_INIT)
@@ -3372,10 +3415,56 @@ static bool states_equal(struct bpf_verifier_env *env,
return ret;
}
+static bool do_propagate_liveness(const struct bpf_verifier_state *state,
+ struct bpf_verifier_state *parent)
+{
+ bool touched = false; /* any changes made? */
+ int i;
+
+ if (!parent)
+ return touched;
+ /* Propagate read liveness of registers... */
+ BUILD_BUG_ON(BPF_REG_FP + 1 != MAX_BPF_REG);
+ /* We don't need to worry about FP liveness because it's read-only */
+ for (i = 0; i < BPF_REG_FP; i++) {
+ if (parent->regs[i].live & REG_LIVE_READ)
+ continue;
+ if (state->regs[i].live == REG_LIVE_READ) {
+ parent->regs[i].live |= REG_LIVE_READ;
+ touched = true;
+ }
+ }
+ /* ... and stack slots */
+ for (i = 0; i < MAX_BPF_STACK / BPF_REG_SIZE; i++) {
+ if (parent->stack_slot_type[i * BPF_REG_SIZE] != STACK_SPILL)
+ continue;
+ if (state->stack_slot_type[i * BPF_REG_SIZE] != STACK_SPILL)
+ continue;
+ if (parent->spilled_regs[i].live & REG_LIVE_READ)
+ continue;
+ if (state->spilled_regs[i].live == REG_LIVE_READ) {
+ parent->regs[i].live |= REG_LIVE_READ;
+ touched = true;
+ }
+ }
+ return touched;
+}
+
+static void propagate_liveness(const struct bpf_verifier_state *state,
+ struct bpf_verifier_state *parent)
+{
+ while (do_propagate_liveness(state, parent)) {
+ /* Something changed, so we need to feed those changes onward */
+ state = parent;
+ parent = state->parent;
+ }
+}
+
static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
{
struct bpf_verifier_state_list *new_sl;
struct bpf_verifier_state_list *sl;
+ int i;
sl = env->explored_states[insn_idx];
if (!sl)
@@ -3385,11 +3474,14 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
return 0;
while (sl != STATE_LIST_MARK) {
- if (states_equal(env, &sl->state, &env->cur_state))
+ if (states_equal(env, &sl->state, &env->cur_state)) {
/* reached equivalent register/stack state,
- * prune the search
+ * prune the search.
+ * Registers read by the continuation are read by us.
*/
+ propagate_liveness(&sl->state, &env->cur_state);
return 1;
+ }
sl = sl->next;
}
@@ -3407,6 +3499,14 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
memcpy(&new_sl->state, &env->cur_state, sizeof(env->cur_state));
new_sl->next = env->explored_states[insn_idx];
env->explored_states[insn_idx] = new_sl;
+ /* connect new state to parentage chain */
+ env->cur_state.parent = &new_sl->state;
+ /* clear liveness marks in current state */
+ for (i = 0; i < BPF_REG_FP; i++)
+ env->cur_state.regs[i].live = REG_LIVE_NONE;
+ for (i = 0; i < MAX_BPF_STACK / BPF_REG_SIZE; i++)
+ if (env->cur_state.stack_slot_type[i * BPF_REG_SIZE] == STACK_SPILL)
+ env->cur_state.spilled_regs[i].live = REG_LIVE_NONE;
return 0;
}
@@ -3430,6 +3530,7 @@ static int do_check(struct bpf_verifier_env *env)
bool do_print_state = false;
init_reg_state(regs);
+ state->parent = NULL;
insn_idx = 0;
env->varlen_map_value_access = false;
for (;;) {
@@ -3500,11 +3601,11 @@ static int do_check(struct bpf_verifier_env *env)
/* check for reserved fields is already done */
/* check src operand */
- err = check_reg_arg(regs, insn->src_reg, SRC_OP);
+ err = check_reg_arg(env, insn->src_reg, SRC_OP);
if (err)
return err;
- err = check_reg_arg(regs, insn->dst_reg, DST_OP_NO_MARK);
+ err = check_reg_arg(env, insn->dst_reg, DST_OP_NO_MARK);
if (err)
return err;
@@ -3554,11 +3655,11 @@ static int do_check(struct bpf_verifier_env *env)
}
/* check src1 operand */
- err = check_reg_arg(regs, insn->src_reg, SRC_OP);
+ err = check_reg_arg(env, insn->src_reg, SRC_OP);
if (err)
return err;
/* check src2 operand */
- err = check_reg_arg(regs, insn->dst_reg, SRC_OP);
+ err = check_reg_arg(env, insn->dst_reg, SRC_OP);
if (err)
return err;
@@ -3589,7 +3690,7 @@ static int do_check(struct bpf_verifier_env *env)
return -EINVAL;
}
/* check src operand */
- err = check_reg_arg(regs, insn->dst_reg, SRC_OP);
+ err = check_reg_arg(env, insn->dst_reg, SRC_OP);
if (err)
return err;
@@ -3643,7 +3744,7 @@ static int do_check(struct bpf_verifier_env *env)
* of bpf_exit, which means that program wrote
* something into it earlier
*/
- err = check_reg_arg(regs, BPF_REG_0, SRC_OP);
+ err = check_reg_arg(env, BPF_REG_0, SRC_OP);
if (err)
return err;
^ permalink raw reply related
* Re: [PATCH net] datagram: When peeking datagrams with offset < 0 don't skip empty skbs
From: Thiago Macieira @ 2017-08-14 17:02 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Matthew Dawson, Paolo Abeni, Network Development
In-Reply-To: <CAF=yD-+k79=fykYs5x5-7AvE85+JPXv=NcfDy8X-4AGP8P85Pw@mail.gmail.com>
On Monday, 14 August 2017 09:33:48 PDT Willem de Bruijn wrote:
> > But here's a question: if the peek offset is equal to the length, should
> > the reading return an empty datagram? This would indicate to the caller
> > that there was a datagram there, which was skipped over.
>
> In the general case, no, it should read at the offset, which is the next
> skb.
I beg to differ. In this particular case, we are talking about datagrams. If
it were stream sockets, I would agree with you: just skip to the next. But in
datagrams, the same way you do return zero-sized ones, I would return an empty
one if you peeked at or past the end.
> Since we only need to change no-offset semantics to fix this bug,
> I would not change this behavior, which is also expected by some
> applications by now.
Do applications using SOCK_DGRAM rely on the behaviour of skipping over
datagrams that are too short?
> > That's how we deal with empty datagrams anyway.
>
> What is? With no-offset and a zero payload skb at the head, peek
> or recv returns 0, right?
Right.
--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
^ permalink raw reply
* [PATCH net] ipv6: release rt6->rt6i_idev properly during ifdown
From: Wei Wang @ 2017-08-14 17:44 UTC (permalink / raw)
To: David Miller, netdev; +Cc: Martin KaFai Lau, David Ahern, Wei Wang
From: Wei Wang <weiwan@google.com>
When a dst is created by addrconf_dst_alloc() for a host route or an
anycast route, dst->dev points to loopback dev while rt6->rt6i_idev
points to a real device.
When the real device goes down, the current cleanup code only checks for
dst->dev and assumes rt6->rt6i_idev->dev is the same. This causes the
refcount leak on the real device in the above situation.
This patch makes sure to always release the refcount taken on
rt6->rt6i_idev during dst_dev_put().
Fixes: 587fea741134 ("ipv6: mark DST_NOGC and remove the operation of
dst_free()")
Reported-by: John Stultz <john.stultz@linaro.org>
Tested-by: John Stultz <john.stultz@linaro.org>
Tested-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Wei Wang <weiwan@google.com>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
net/ipv6/route.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 4d30c96a819d..8d53abd96181 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -417,14 +417,11 @@ static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
struct net_device *loopback_dev =
dev_net(dev)->loopback_dev;
- if (dev != loopback_dev) {
- if (idev && idev->dev == dev) {
- struct inet6_dev *loopback_idev =
- in6_dev_get(loopback_dev);
- if (loopback_idev) {
- rt->rt6i_idev = loopback_idev;
- in6_dev_put(idev);
- }
+ if (idev && idev->dev != loopback_dev) {
+ struct inet6_dev *loopback_idev = in6_dev_get(loopback_dev);
+ if (loopback_idev) {
+ rt->rt6i_idev = loopback_idev;
+ in6_dev_put(idev);
}
}
}
--
2.14.0.434.g98096fd7a8-goog
^ permalink raw reply related
* Re: [PATCH v2] netfilter: nf_nat_h323: fix logical-not-parentheses warning
From: Nick Desaulniers @ 2017-08-14 17:36 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Matthias Kaehlcke, Lorenzo Colitti, Nick Desaulniers,
Jozsef Kadlecsik, Florian Westphal, David S. Miller,
Alexey Kuznetsov, Hideaki YOSHIFUJI, netfilter-devel, coreteam,
netdev, linux-kernel
In-Reply-To: <20170811181607.33878-1-ndesaulniers@google.com>
Minor nit for the commit message that can get fixed up when being merged:
On Fri, Aug 11, 2017 at 11:16 AM, Nick Desaulniers
<ndesaulniers@google.com> wrote:
> if (x)
> return
> ...
>
> rather than:
>
> if (!x == 0)
should remove the `!`, ex:
if (x == 0)
> ...
> else
> return
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* Re: [iproute PATCH 51/51] lib/bpf: Check return value of write()
From: Phil Sutter @ 2017-08-14 17:25 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Stephen Hemminger, netdev
In-Reply-To: <59916AB3.1010001@iogearbox.net>
On Mon, Aug 14, 2017 at 11:17:39AM +0200, Daniel Borkmann wrote:
> On 08/12/2017 02:05 PM, Phil Sutter wrote:
> > This is merely to silence the compiler warning. If write to stderr
> > failed, assume that printing an error message will fail as well so don't
> > even try.
> >
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> > lib/bpf.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/bpf.c b/lib/bpf.c
> > index 1dcb261dc915f..825e071cea572 100644
> > --- a/lib/bpf.c
> > +++ b/lib/bpf.c
> > @@ -591,7 +591,8 @@ int bpf_trace_pipe(void)
> >
> > ret = read(fd, buff, sizeof(buff) - 1);
> > if (ret > 0) {
> > - write(2, buff, ret);
> > + if (write(STDERR_FILENO, buff, ret) != ret)
> > + return -1;
>
> Quite unlikely to fail, but we should probably bark loudly
> here instead of just returning -1. Perhaps assert() would
> suit better.
Well, according to assert(3), it will print to stderr before aborting
the program. So if writing STDERR_FILENO failed, I guess it won't
provide much more detail to the user, either. If bpf_trace_pipe()
returns non-zero, parse_bpf() prints an error message to stderr and
returns -1. Ultimately tc will return non-zero. With stderr unfit for
writing into, I doubt there's anything left we could do besides that.
But I really think we shouldn't make such a fuss about it - writing to
stderr either always works or we're in trouble everywhere. This patch
was merely to shut gcc up, so no need to waste much energy on a scenario
which won't happen anyway.
Thanks, Phil
^ permalink raw reply
* Re: [PATCH v10 3/5] PCI: Disable Relaxed Ordering Attributes for AMD A1100
From: Raj, Ashok @ 2017-08-14 17:19 UTC (permalink / raw)
To: Ding Tianhong
Cc: leedom, bhelgaas, helgaas, werner, ganeshgr, asit.k.mallick,
patrick.j.cramer, Suravee.Suthikulpanit, Bob.Shaw, l.stach, amira,
gabriele.paoloni, David.Laight, jeffrey.t.kirsher,
catalin.marinas, will.deacon, mark.rutland, robin.murphy, davem,
alexander.duyck, linux-arm-kernel, netdev, linux-pci,
linux-kernel, linuxarm, ashok.raj
In-Reply-To: <1502725499-11276-4-git-send-email-dingtianhong@huawei.com>
On Mon, Aug 14, 2017 at 11:44:57PM +0800, Ding Tianhong wrote:
> Casey reported that the AMD ARM A1100 SoC has a bug in its PCIe
> Root Port where Upstream Transaction Layer Packets with the Relaxed
> Ordering Attribute clear are allowed to bypass earlier TLPs with
> Relaxed Ordering set, it would cause Data Corruption, so we need
> to disable Relaxed Ordering Attribute when Upstream TLPs to the
> Root Port.
>
> Signed-off-by: Casey Leedom <leedom@chelsio.com>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Acked-by: Ashok Raj <ashok.raj@intel.com>
I can't ack this patch :-).. must be someone from AMD. Please remove my
signature from this.
> ---
> drivers/pci/quirks.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 1272f7e..1407604 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -4089,6 +4089,22 @@ static void quirk_relaxedordering_disable(struct pci_dev *dev)
> quirk_relaxedordering_disable);
>
> /*
> + * The AMD ARM A1100 (AKA "SEATTLE") SoC has a bug in its PCIe Root Complex
> + * where Upstream Transaction Layer Packets with the Relaxed Ordering
> + * Attribute clear are allowed to bypass earlier TLPs with Relaxed Ordering
> + * set. This is a violation of the PCIe 3.0 Transaction Ordering Rules
> + * outlined in Section 2.4.1 (PCI Express(r) Base Specification Revision 3.0
> + * November 10, 2010). As a result, on this platform we can't use Relaxed
> + * Ordering for Upstream TLPs.
> + */
> +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AMD, 0x1a00, PCI_CLASS_NOT_DEFINED, 8,
> + quirk_relaxedordering_disable);
> +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AMD, 0x1a01, PCI_CLASS_NOT_DEFINED, 8,
> + quirk_relaxedordering_disable);
> +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AMD, 0x1a02, PCI_CLASS_NOT_DEFINED, 8,
> + quirk_relaxedordering_disable);
> +
> +/*
> * Per PCIe r3.0, sec 2.2.9, "Completion headers must supply the same
> * values for the Attribute as were supplied in the header of the
> * corresponding Request, except as explicitly allowed when IDO is used."
> --
> 1.8.3.1
>
>
^ permalink raw reply
* [PATCH net] af_key: do not use GFP_KERNEL in atomic contexts
From: Eric Dumazet @ 2017-08-14 17:16 UTC (permalink / raw)
To: David Miller; +Cc: netdev, David Ahern
From: Eric Dumazet <edumazet@google.com>
pfkey_broadcast() might be called from non process contexts,
we can not use GFP_KERNEL in these cases [1].
This patch partially reverts commit ba51b6be38c1 ("net: Fix RCU splat in
af_key"), only keeping the GFP_ATOMIC forcing under rcu_read_lock()
section.
[1] : syzkaller reported :
in_atomic(): 1, irqs_disabled(): 0, pid: 2932, name: syzkaller183439
3 locks held by syzkaller183439/2932:
#0: (&net->xfrm.xfrm_cfg_mutex){+.+.+.}, at: [<ffffffff83b43888>] pfkey_sendmsg+0x4c8/0x9f0 net/key/af_key.c:3649
#1: (&pfk->dump_lock){+.+.+.}, at: [<ffffffff83b467f6>] pfkey_do_dump+0x76/0x3f0 net/key/af_key.c:293
#2: (&(&net->xfrm.xfrm_policy_lock)->rlock){+...+.}, at: [<ffffffff83957632>] spin_lock_bh include/linux/spinlock.h:304 [inline]
#2: (&(&net->xfrm.xfrm_policy_lock)->rlock){+...+.}, at: [<ffffffff83957632>] xfrm_policy_walk+0x192/0xa30 net/xfrm/xfrm_policy.c:1028
CPU: 0 PID: 2932 Comm: syzkaller183439 Not tainted 4.13.0-rc4+ #24
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:16 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:52
___might_sleep+0x2b2/0x470 kernel/sched/core.c:5994
__might_sleep+0x95/0x190 kernel/sched/core.c:5947
slab_pre_alloc_hook mm/slab.h:416 [inline]
slab_alloc mm/slab.c:3383 [inline]
kmem_cache_alloc+0x24b/0x6e0 mm/slab.c:3559
skb_clone+0x1a0/0x400 net/core/skbuff.c:1037
pfkey_broadcast_one+0x4b2/0x6f0 net/key/af_key.c:207
pfkey_broadcast+0x4ba/0x770 net/key/af_key.c:281
dump_sp+0x3d6/0x500 net/key/af_key.c:2685
xfrm_policy_walk+0x2f1/0xa30 net/xfrm/xfrm_policy.c:1042
pfkey_dump_sp+0x42/0x50 net/key/af_key.c:2695
pfkey_do_dump+0xaa/0x3f0 net/key/af_key.c:299
pfkey_spddump+0x1a0/0x210 net/key/af_key.c:2722
pfkey_process+0x606/0x710 net/key/af_key.c:2814
pfkey_sendmsg+0x4d6/0x9f0 net/key/af_key.c:3650
sock_sendmsg_nosec net/socket.c:633 [inline]
sock_sendmsg+0xca/0x110 net/socket.c:643
___sys_sendmsg+0x755/0x890 net/socket.c:2035
__sys_sendmsg+0xe5/0x210 net/socket.c:2069
SYSC_sendmsg net/socket.c:2080 [inline]
SyS_sendmsg+0x2d/0x50 net/socket.c:2076
entry_SYSCALL_64_fastpath+0x1f/0xbe
RIP: 0033:0x445d79
RSP: 002b:00007f32447c1dc8 EFLAGS: 00000202 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000445d79
RDX: 0000000000000000 RSI: 000000002023dfc8 RDI: 0000000000000008
RBP: 0000000000000086 R08: 00007f32447c2700 R09: 00007f32447c2700
R10: 00007f32447c2700 R11: 0000000000000202 R12: 0000000000000000
R13: 00007ffe33edec4f R14: 00007f32447c29c0 R15: 0000000000000000
Fixes: ba51b6be38c1 ("net: Fix RCU splat in af_key")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Cc: David Ahern <dsa@cumulusnetworks.com>
---
net/key/af_key.c | 48 ++++++++++++++++++++++++---------------------
1 file changed, 26 insertions(+), 22 deletions(-)
diff --git a/net/key/af_key.c b/net/key/af_key.c
index ca9d3ae665e7..98f4d8211b9a 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -228,7 +228,7 @@ static int pfkey_broadcast_one(struct sk_buff *skb, struct sk_buff **skb2,
#define BROADCAST_ONE 1
#define BROADCAST_REGISTERED 2
#define BROADCAST_PROMISC_ONLY 4
-static int pfkey_broadcast(struct sk_buff *skb,
+static int pfkey_broadcast(struct sk_buff *skb, gfp_t allocation,
int broadcast_flags, struct sock *one_sk,
struct net *net)
{
@@ -278,7 +278,7 @@ static int pfkey_broadcast(struct sk_buff *skb,
rcu_read_unlock();
if (one_sk != NULL)
- err = pfkey_broadcast_one(skb, &skb2, GFP_KERNEL, one_sk);
+ err = pfkey_broadcast_one(skb, &skb2, allocation, one_sk);
kfree_skb(skb2);
kfree_skb(skb);
@@ -311,7 +311,7 @@ static int pfkey_do_dump(struct pfkey_sock *pfk)
hdr = (struct sadb_msg *) pfk->dump.skb->data;
hdr->sadb_msg_seq = 0;
hdr->sadb_msg_errno = rc;
- pfkey_broadcast(pfk->dump.skb, BROADCAST_ONE,
+ pfkey_broadcast(pfk->dump.skb, GFP_ATOMIC, BROADCAST_ONE,
&pfk->sk, sock_net(&pfk->sk));
pfk->dump.skb = NULL;
}
@@ -355,7 +355,7 @@ static int pfkey_error(const struct sadb_msg *orig, int err, struct sock *sk)
hdr->sadb_msg_len = (sizeof(struct sadb_msg) /
sizeof(uint64_t));
- pfkey_broadcast(skb, BROADCAST_ONE, sk, sock_net(sk));
+ pfkey_broadcast(skb, GFP_KERNEL, BROADCAST_ONE, sk, sock_net(sk));
return 0;
}
@@ -1389,7 +1389,7 @@ static int pfkey_getspi(struct sock *sk, struct sk_buff *skb, const struct sadb_
xfrm_state_put(x);
- pfkey_broadcast(resp_skb, BROADCAST_ONE, sk, net);
+ pfkey_broadcast(resp_skb, GFP_KERNEL, BROADCAST_ONE, sk, net);
return 0;
}
@@ -1476,7 +1476,7 @@ static int key_notify_sa(struct xfrm_state *x, const struct km_event *c)
hdr->sadb_msg_seq = c->seq;
hdr->sadb_msg_pid = c->portid;
- pfkey_broadcast(skb, BROADCAST_ALL, NULL, xs_net(x));
+ pfkey_broadcast(skb, GFP_ATOMIC, BROADCAST_ALL, NULL, xs_net(x));
return 0;
}
@@ -1589,7 +1589,7 @@ static int pfkey_get(struct sock *sk, struct sk_buff *skb, const struct sadb_msg
out_hdr->sadb_msg_reserved = 0;
out_hdr->sadb_msg_seq = hdr->sadb_msg_seq;
out_hdr->sadb_msg_pid = hdr->sadb_msg_pid;
- pfkey_broadcast(out_skb, BROADCAST_ONE, sk, sock_net(sk));
+ pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ONE, sk, sock_net(sk));
return 0;
}
@@ -1694,8 +1694,8 @@ static int pfkey_register(struct sock *sk, struct sk_buff *skb, const struct sad
return -ENOBUFS;
}
- pfkey_broadcast(supp_skb, BROADCAST_REGISTERED, sk, sock_net(sk));
-
+ pfkey_broadcast(supp_skb, GFP_KERNEL, BROADCAST_REGISTERED, sk,
+ sock_net(sk));
return 0;
}
@@ -1712,7 +1712,8 @@ static int unicast_flush_resp(struct sock *sk, const struct sadb_msg *ihdr)
hdr->sadb_msg_errno = (uint8_t) 0;
hdr->sadb_msg_len = (sizeof(struct sadb_msg) / sizeof(uint64_t));
- return pfkey_broadcast(skb, BROADCAST_ONE, sk, sock_net(sk));
+ return pfkey_broadcast(skb, GFP_ATOMIC, BROADCAST_ONE, sk,
+ sock_net(sk));
}
static int key_notify_sa_flush(const struct km_event *c)
@@ -1733,7 +1734,7 @@ static int key_notify_sa_flush(const struct km_event *c)
hdr->sadb_msg_len = (sizeof(struct sadb_msg) / sizeof(uint64_t));
hdr->sadb_msg_reserved = 0;
- pfkey_broadcast(skb, BROADCAST_ALL, NULL, c->net);
+ pfkey_broadcast(skb, GFP_ATOMIC, BROADCAST_ALL, NULL, c->net);
return 0;
}
@@ -1790,7 +1791,7 @@ static int dump_sa(struct xfrm_state *x, int count, void *ptr)
out_hdr->sadb_msg_pid = pfk->dump.msg_portid;
if (pfk->dump.skb)
- pfkey_broadcast(pfk->dump.skb, BROADCAST_ONE,
+ pfkey_broadcast(pfk->dump.skb, GFP_ATOMIC, BROADCAST_ONE,
&pfk->sk, sock_net(&pfk->sk));
pfk->dump.skb = out_skb;
@@ -1878,7 +1879,7 @@ static int pfkey_promisc(struct sock *sk, struct sk_buff *skb, const struct sadb
new_hdr->sadb_msg_errno = 0;
}
- pfkey_broadcast(skb, BROADCAST_ALL, NULL, sock_net(sk));
+ pfkey_broadcast(skb, GFP_KERNEL, BROADCAST_ALL, NULL, sock_net(sk));
return 0;
}
@@ -2206,7 +2207,7 @@ static int key_notify_policy(struct xfrm_policy *xp, int dir, const struct km_ev
out_hdr->sadb_msg_errno = 0;
out_hdr->sadb_msg_seq = c->seq;
out_hdr->sadb_msg_pid = c->portid;
- pfkey_broadcast(out_skb, BROADCAST_ALL, NULL, xp_net(xp));
+ pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ALL, NULL, xp_net(xp));
return 0;
}
@@ -2426,7 +2427,7 @@ static int key_pol_get_resp(struct sock *sk, struct xfrm_policy *xp, const struc
out_hdr->sadb_msg_errno = 0;
out_hdr->sadb_msg_seq = hdr->sadb_msg_seq;
out_hdr->sadb_msg_pid = hdr->sadb_msg_pid;
- pfkey_broadcast(out_skb, BROADCAST_ONE, sk, xp_net(xp));
+ pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ONE, sk, xp_net(xp));
err = 0;
out:
@@ -2682,7 +2683,7 @@ static int dump_sp(struct xfrm_policy *xp, int dir, int count, void *ptr)
out_hdr->sadb_msg_pid = pfk->dump.msg_portid;
if (pfk->dump.skb)
- pfkey_broadcast(pfk->dump.skb, BROADCAST_ONE,
+ pfkey_broadcast(pfk->dump.skb, GFP_ATOMIC, BROADCAST_ONE,
&pfk->sk, sock_net(&pfk->sk));
pfk->dump.skb = out_skb;
@@ -2739,7 +2740,7 @@ static int key_notify_policy_flush(const struct km_event *c)
hdr->sadb_msg_satype = SADB_SATYPE_UNSPEC;
hdr->sadb_msg_len = (sizeof(struct sadb_msg) / sizeof(uint64_t));
hdr->sadb_msg_reserved = 0;
- pfkey_broadcast(skb_out, BROADCAST_ALL, NULL, c->net);
+ pfkey_broadcast(skb_out, GFP_ATOMIC, BROADCAST_ALL, NULL, c->net);
return 0;
}
@@ -2803,7 +2804,7 @@ static int pfkey_process(struct sock *sk, struct sk_buff *skb, const struct sadb
void *ext_hdrs[SADB_EXT_MAX];
int err;
- pfkey_broadcast(skb_clone(skb, GFP_KERNEL),
+ pfkey_broadcast(skb_clone(skb, GFP_KERNEL), GFP_KERNEL,
BROADCAST_PROMISC_ONLY, NULL, sock_net(sk));
memset(ext_hdrs, 0, sizeof(ext_hdrs));
@@ -3024,7 +3025,8 @@ static int key_notify_sa_expire(struct xfrm_state *x, const struct km_event *c)
out_hdr->sadb_msg_seq = 0;
out_hdr->sadb_msg_pid = 0;
- pfkey_broadcast(out_skb, BROADCAST_REGISTERED, NULL, xs_net(x));
+ pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_REGISTERED, NULL,
+ xs_net(x));
return 0;
}
@@ -3212,7 +3214,8 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
xfrm_ctx->ctx_len);
}
- return pfkey_broadcast(skb, BROADCAST_REGISTERED, NULL, xs_net(x));
+ return pfkey_broadcast(skb, GFP_ATOMIC, BROADCAST_REGISTERED, NULL,
+ xs_net(x));
}
static struct xfrm_policy *pfkey_compile_policy(struct sock *sk, int opt,
@@ -3408,7 +3411,8 @@ static int pfkey_send_new_mapping(struct xfrm_state *x, xfrm_address_t *ipaddr,
n_port->sadb_x_nat_t_port_port = sport;
n_port->sadb_x_nat_t_port_reserved = 0;
- return pfkey_broadcast(skb, BROADCAST_REGISTERED, NULL, xs_net(x));
+ return pfkey_broadcast(skb, GFP_ATOMIC, BROADCAST_REGISTERED, NULL,
+ xs_net(x));
}
#ifdef CONFIG_NET_KEY_MIGRATE
@@ -3599,7 +3603,7 @@ static int pfkey_send_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
}
/* broadcast migrate message to sockets */
- pfkey_broadcast(skb, BROADCAST_ALL, NULL, &init_net);
+ pfkey_broadcast(skb, GFP_ATOMIC, BROADCAST_ALL, NULL, &init_net);
return 0;
^ permalink raw reply related
* Re: Kernel 4.13.0-rc4-next-20170811 - IP Routing / Forwarding performance vs Core/RSS number / HT on
From: Paolo Abeni @ 2017-08-14 16:57 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Paweł Staszewski
Cc: Linux Kernel Network Developers, Alexander Duyck, Eric Dumazet
In-Reply-To: <20170814181957.5be27906@redhat.com>
On Mon, 2017-08-14 at 18:19 +0200, Jesper Dangaard Brouer wrote:
> The output (extracted below) didn't show who called 'do_raw_spin_lock',
> BUT it showed another interesting thing. The kernel code
> __dev_queue_xmit() in might create route dst-cache problem for itself(?),
> as it will first call skb_dst_force() and then skb_dst_drop() when the
> packet is transmitted on a VLAN.
>
> static int __dev_queue_xmit(struct sk_buff *skb, void *accel_priv)
> {
> [...]
> /* If device/qdisc don't need skb->dst, release it right now while
> * its hot in this cpu cache.
> */
> if (dev->priv_flags & IFF_XMIT_DST_RELEASE)
> skb_dst_drop(skb);
> else
> skb_dst_force(skb);
I think that the high impact of the above code in this specific test is
mostly due to the following:
- ingress packets with different RSS rx hash lands on different CPUs
- but they use the same dst entry, since the destination IPs belong to
the same subnet
- the dst refcnt cacheline is contented between all the CPUs
Perhaps we can inprove the situation setting the IFF_XMIT_DST_RELEASE
flag for vlan if the underlaying device does not have (relevant)
classifier attached? (and clearing it as needed)
Paolo
^ permalink raw reply
* Re: [PATCH net] datagram: When peeking datagrams with offset < 0 don't skip empty skbs
From: Willem de Bruijn @ 2017-08-14 16:33 UTC (permalink / raw)
To: Thiago Macieira; +Cc: Matthew Dawson, Paolo Abeni, Network Development
In-Reply-To: <2828157.5Eig1SAfYW@tjmaciei-mobl1>
On Mon, Aug 14, 2017 at 12:06 PM, Thiago Macieira
<thiago.macieira@intel.com> wrote:
> On Monday, 14 August 2017 08:03:50 PDT Willem de Bruijn wrote:
>> > I'm actually surprised that only unix sockets can have negative values.
>> > Is
>> > there a reason for that? I had assumed that sk_set_peek_off would allow
>> > negative values as the code already has to support negative values due to
>> > what the initial value is.
>>
>> A negative initial value indicates that PEEK_OFF is disabled. It only
>> makes sense to peek from a positive offset from the start of the data.
>
> But here's a question: if the peek offset is equal to the length, should the
> reading return an empty datagram? This would indicate to the caller that there
> was a datagram there, which was skipped over.
In the general case, no, it should read at the offset, which is the next skb.
Zero length packets are a special case. This did come up before and
we chose to signal their existence in the queue by returning 0 for each
once, even in the offset-enabled mode.
Since we only need to change no-offset semantics to fix this bug,
I would not change this behavior, which is also expected by some
applications by now.
>
> That's how we deal with empty datagrams anyway.
What is? With no-offset and a zero payload skb at the head, peek
or recv returns 0, right?
^ permalink raw reply
* Re: Kernel 4.13.0-rc4-next-20170811 - IP Routing / Forwarding performance vs Core/RSS number / HT on
From: Eric Dumazet @ 2017-08-14 16:33 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Paweł Staszewski, Linux Kernel Network Developers,
Alexander Duyck
In-Reply-To: <20170814181957.5be27906@redhat.com>
On Mon, 2017-08-14 at 18:19 +0200, Jesper Dangaard Brouer wrote:
> The output (extracted below) didn't show who called 'do_raw_spin_lock',
> BUT it showed another interesting thing. The kernel code
> __dev_queue_xmit() in might create route dst-cache problem for itself(?),
> as it will first call skb_dst_force() and then skb_dst_drop() when the
> packet is transmitted on a VLAN.
>
> static int __dev_queue_xmit(struct sk_buff *skb, void *accel_priv)
> {
> [...]
> /* If device/qdisc don't need skb->dst, release it right now while
> * its hot in this cpu cache.
> */
> if (dev->priv_flags & IFF_XMIT_DST_RELEASE)
> skb_dst_drop(skb);
> else
> skb_dst_force(skb);
This is explained in this commit changelog.
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=93f154b594fe47e4a7e5358b309add449a046cd3
^ permalink raw reply
* [net 1/1] tipc: avoid inheriting msg_non_seq flag when message is returned
From: Jon Maloy @ 2017-08-14 16:28 UTC (permalink / raw)
To: davem, netdev; +Cc: parthasarathy.bhuvaragan, ying.xue, tipc-discussion
In the function msg_reverse(), we reverse the header while trying to
reuse the original buffer whenever possible. Those rejected/returned
messages are always transmitted as unicast, but the msg_non_seq field
is not explicitly set to zero as it should be.
We have seen cases where multicast senders set the message type to
"NOT dest_droppable", meaning that a multicast message shorter than
one MTU will be returned, e.g., during receive buffer overflow, by
reusing the original buffer. This has the effect that even the
'msg_non_seq' field is inadvertently inherited by the rejected message,
although it is now sent as a unicast message. This again leads the
receiving unicast link endpoint to steer the packet toward the broadcast
link receive function, where it is dropped. The affected unicast link is
thereafter (after 100 failed retransmissions) declared 'stale' and
reset.
We fix this by unconditionally setting the 'msg_non_seq' flag to zero
for all rejected/returned messages.
Reported-by: Canh Duc Luu <canh.d.luu@dektech.com.au>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/msg.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index ab30876..dcd90e6 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -513,6 +513,7 @@ bool tipc_msg_reverse(u32 own_node, struct sk_buff **skb, int err)
/* Now reverse the concerned fields */
msg_set_errcode(hdr, err);
+ msg_set_non_seq(hdr, 0);
msg_set_origport(hdr, msg_destport(&ohdr));
msg_set_destport(hdr, msg_origport(&ohdr));
msg_set_destnode(hdr, msg_prevnode(&ohdr));
--
2.1.4
^ permalink raw reply related
* RE: [PATCH net-next] liquidio: update debug console logging mechanism
From: Ricardo Farrington @ 2017-08-14 16:28 UTC (permalink / raw)
To: Leon Romanovsky, Manlunas, Felix
Cc: davem@davemloft.net, netdev@vger.kernel.org, Vatsavayi, Raghu,
Chickles, Derek, Burla, Satananda
In-Reply-To: <20170813055549.GO24282@mtr-leonro.local>
Hi Leon - the code to which this patch applies handles a data stream from our card's firmware to the host (over PCI).
There are device-specific registers which are accessed to transfer log data from our firmware to the host.
I don't think there is kernel code that we could use to perform this; this is not general purpose host driver logging.
Rick
-----Original Message-----
From: Leon Romanovsky [mailto:leon@kernel.org]
Sent: Saturday, August 12, 2017 10:56 PM
To: Manlunas, Felix <Felix.Manlunas@cavium.com>
Cc: davem@davemloft.net; netdev@vger.kernel.org; Vatsavayi, Raghu <Raghu.Vatsavayi@cavium.com>; Chickles, Derek <Derek.Chickles@cavium.com>; Burla, Satananda <Satananda.Burla@cavium.com>; Ricardo Farrington <Ricardo.Farrington@cavium.com>
Subject: Re: [PATCH net-next] liquidio: update debug console logging mechanism
On Fri, Aug 11, 2017 at 06:43:14PM -0700, Felix Manlunas wrote:
> From: Rick Farrington <ricardo.farrington@cavium.com>
>
> - remove logging dependency upon global func
> octeon_console_debug_enabled()
> - abstract debug console logging using console structure (via function ptr)
> to allow for more flexible logging
>
> Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
> Signed-off-by: Raghu Vatsavayi <raghu.vatsavayi@cavium.com>
> Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
> ---
> drivers/net/ethernet/cavium/liquidio/lio_main.c | 44 +++++++++++++++++-
> .../net/ethernet/cavium/liquidio/octeon_console.c | 54 ++++++++++++++--------
> .../net/ethernet/cavium/liquidio/octeon_device.h | 17 +++++--
> 3 files changed, 90 insertions(+), 25 deletions(-)
>
I'm probably missing something important, but why do you need your custom console implementation if kernel is full of such built-in options?
Thanks
^ permalink raw reply
* Re: Kernel 4.13.0-rc4-next-20170811 - IP Routing / Forwarding performance vs Core/RSS number / HT on
From: Jesper Dangaard Brouer @ 2017-08-14 16:19 UTC (permalink / raw)
To: Paweł Staszewski
Cc: Linux Kernel Network Developers, brouer, Alexander Duyck
In-Reply-To: <ce33b7b3-ae00-b6f0-e82a-6df3d5a5e995@itcare.pl>
On Sun, 13 Aug 2017 18:58:58 +0200 Paweł Staszewski <pstaszewski@itcare.pl> wrote:
> To show some difference below comparision vlan/no-vlan traffic
>
> 10Mpps forwarded traffic vith no-vlan vs 6.9Mpps with vlan
I'm trying to reproduce in my testlab (with ixgbe). I do see, a
performance reduction of about 10-19% when I forward out a VLAN
interface. This is larger than I expected, but still lower than what
you reported 30-40% slowdown.
[...]
> >>> perf top:
> >>>
> >>> PerfTop: 77835 irqs/sec kernel:99.7%
> >>> ---------------------------------------------
> >>>
> >>> 16.32% [kernel] [k] skb_dst_force
> >>> 16.30% [kernel] [k] dst_release
> >>> 15.11% [kernel] [k] rt_cache_valid
> >>> 12.62% [kernel] [k] ipv4_mtu
> >> It seems a little strange that these 4 functions are on the top
I don't see these in my test.
> >>
> >>> 5.60% [kernel] [k] do_raw_spin_lock
> >> Why is calling/taking this lock? (Use perf call-graph recording).
> > can be hard to paste it here:)
> > attached file
The attached was very big. Please don't attach so big file on mailing
lists. Next time plase share them via e.g. pastebin. The output was a
capture from your terminal, which made the output more difficult to
read. Hint: You can/could use perf --stdio and place it in a file
instead.
The output (extracted below) didn't show who called 'do_raw_spin_lock',
BUT it showed another interesting thing. The kernel code
__dev_queue_xmit() in might create route dst-cache problem for itself(?),
as it will first call skb_dst_force() and then skb_dst_drop() when the
packet is transmitted on a VLAN.
static int __dev_queue_xmit(struct sk_buff *skb, void *accel_priv)
{
[...]
/* If device/qdisc don't need skb->dst, release it right now while
* its hot in this cpu cache.
*/
if (dev->priv_flags & IFF_XMIT_DST_RELEASE)
skb_dst_drop(skb);
else
skb_dst_force(skb);
- -
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
Extracted part of attached perf output:
--5.37%--ip_rcv_finish
|
|--4.02%--ip_forward
| |
| --3.92%--ip_forward_finish
| |
| --3.91%--ip_output
| |
| --3.90%--ip_finish_output
| |
| --3.88%--ip_finish_output2
| |
| --2.77%--neigh_connected_output
| |
| --2.74%--dev_queue_xmit
| |
| --2.73%--__dev_queue_xmit
| |
| |--1.66%--dev_hard_start_xmit
| | |
| | --1.64%--vlan_dev_hard_start_xmit
| | |
| | --1.63%--dev_queue_xmit
| | |
| | --1.62%--__dev_queue_xmit
| | |
| | |--0.99%--skb_dst_drop.isra.77
| | | |
| | | --0.99%--dst_release
| | |
| | --0.55%--sch_direct_xmit
| |
| --0.99%--skb_dst_force
|
--1.29%--ip_route_input_noref
|
--1.29%--ip_route_input_rcu
|
--1.05%--rt_cache_valid
^ permalink raw reply
* RE: [PATCH net-next 2/3] Add LAN743X to Kconfig and Makefile (fwd)
From: Bryan.Whitehead @ 2017-08-14 16:16 UTC (permalink / raw)
To: julia.lawall; +Cc: netdev, davem, UNGLinuxDriver, kbuild-all
In-Reply-To: <alpine.DEB.2.20.1708141217280.3368@hadrien>
> I don't think netdev_priv can return NULL, so lines 6641 to 6646 could just be
> dropped.
>
> julia
>
Julia,
Thanks for the feedback.
I will make changes and submit again later.
Bryan
^ permalink raw reply
* RE: [PATCH net-next 1/3] Add LAN743X driver
From: Bryan.Whitehead @ 2017-08-14 16:15 UTC (permalink / raw)
To: stephen; +Cc: netdev, davem, UNGLinuxDriver
In-Reply-To: <20170811160532.6a35b3b2@xeon-e3>
>
> The statistics code here is confused.
> You are already counting rx_packets in software in napi_poll Then you get
> values from MAC. One or the other?
> There are two copies of stats, one in netdev and other in your mac structure.
>
> Also what about byte and error counts?
>
> If possible implement 64 bit get_stats64 instead.
Stephen,
Thanks for the feedback.
I will work on your suggestions, and submit again later.
Bryan
^ permalink raw reply
* RE: [PATCH net-next 1/3] Add LAN743X driver
From: Bryan.Whitehead @ 2017-08-14 16:13 UTC (permalink / raw)
To: andrew, davem; +Cc: netdev, UNGLinuxDriver
In-Reply-To: <20170811223445.GC4671@lunn.ch>
> Could you split it up a bit. Take the PTP support out for the moment, and
> submit it later once the core driver is accepted. The same for any other
> optional bits.
>
> Andrew
Andrew,
thanks for the feedback.
I will work on them and submit again later.
Bryan
^ permalink raw reply
* RE: [PATCH net-next 1/3] Add LAN743X driver
From: Bryan.Whitehead @ 2017-08-14 16:12 UTC (permalink / raw)
To: davem; +Cc: netdev, UNGLinuxDriver
In-Reply-To: <20170811.151205.2131155817966352627.davem@davemloft.net>
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Friday, August 11, 2017 6:12 PM
> To: Bryan Whitehead - C21958
> Cc: netdev@vger.kernel.org; UNGLinuxDriver
> Subject: Re: [PATCH net-next 1/3] Add LAN743X driver
>
> From: <Bryan.Whitehead@microchip.com>
> Date: Fri, 11 Aug 2017 19:47:57 +0000
>
> > +static int lan743x_pci_init(struct lan743x_adapter *adapter,
> > + struct pci_dev *pdev)
> > +{
> > + int ret = -ENODEV;
> > + int bars = 0;
> > + struct lan743x_pci *pci = &adapter->pci;
>
> Please always order local variable declarations from longest to shortest line
> (reverse christmas tree format).
>
> > + pci_set_master(pdev);
> > +
> > +clean_up:
> > + if (ret) {
>
> It is more intuitive to structure this like:
>
> return 0;
>
> clean_up:
> ...
>
> > +static u8 __iomem *lan743x_pci_get_bar_address(struct lan743x_adapter
> *adapter,
> > + int bar_index)
> > +{
> > + u8 __iomem *result = NULL;
> > + struct lan743x_pci *pci = &adapter->pci;
>
> Reverse christmas tree ordering please.
>
> > +static int lan743x_csr_light_reset(struct lan743x_adapter *adapter) {
> > + int result = -EIO;
> > + u32 data;
> > + unsigned long timeout;
>
> Likewise.
>
> > +static inline void lan743x_csr_write(
> > + struct lan743x_adapter *adapter, int offset, u32 data)
>
> Don't do the argument formatting like this please, it looks terrible.
>
> This:
>
> static inline void lan743x_csr_write(struct lan743x_adapter *adapter,
> int offset, u32 data)
>
> works much better.
>
> > +static void lan743x_intr_union_isr(void *context, u32 int_sts) {
> > + struct lan743x_adapter *adapter = (struct lan743x_adapter
> *)context;
>
> Casts from void pointers are never necessary, that's the whole point of void
> pointers. Please remove this cast.
>
> > +static irqreturn_t lan743x_vector_isr(int irq, void *ptr) {
> > + irqreturn_t result = IRQ_NONE;
> > + struct lan743x_vector *vector = (struct lan743x_vector *)ptr;
> > + struct lan743x_adapter *adapter = NULL;
> > + u32 int_sts;
> > + u32 mask;
>
> Reverse christmas tree ordering please.
>
> > +static int lan743x_intr_open(struct lan743x_adapter *adapter) {
> > + int ret = -ENODEV;
> > + struct lan743x_intr *intr = &adapter->intr;
> > + int index = 0;
>
> Likewise.
>
> > +static int lan743x_dp_wait_till_not_busy(struct lan743x_adapter
> > +*adapter) {
> > + int i;
> > + u32 dp_sel = 0;
>
> Likewise.
>
> > +static int lan743x_dp_write(struct lan743x_adapter *adapter,
> > + u32 select, u32 addr, u32 length, u32 *buf) {
> > + struct lan743x_dp *dp = &adapter->dp;
> > + int ret = -EIO;
> > + int i;
> > + u32 dp_sel;
>
> Likewise.
>
> > +#ifdef CONFIG_PTP_1588_CLOCK
> > +static int lan743x_gpio_reserve_ptp_output(struct lan743x_adapter
> *adapter,
> > + int bit, int ptp_channel)
> > +{
> > + struct lan743x_gpio *gpio = &adapter->gpio;
> > + int ret = -EBUSY;
> > + unsigned long irq_flags = 0;
> > + int bit_mask = BIT(bit);
>
> Likewise.
>
> > +#ifdef CONFIG_PTP_1588_CLOCK
> > +static int lan743x_ptpci_adjfreq(struct ptp_clock_info *ptpci, s32
> > +delta_ppb) {
> > + u32 u32_delta = 0;
> > + u64 u64_delta = 0;
> > + u32 lan743x_rate_adj = 0;
> > + bool positive = true;
> > + struct lan743x_ptp *ptp = LAN743X_PTPCI_TO_PTP;
> > + struct lan743x_adapter *adapter = LAN743X_PTP_TO_ADAPTER;
>
> Likewise.
>
> > +#ifdef CONFIG_PTP_1588_CLOCK
> > +static int lan743x_ptpci_adjtime(struct ptp_clock_info *ptpci, s64
> > +delta) {
> > + struct lan743x_ptp *ptp = LAN743X_PTPCI_TO_PTP;
> > + struct lan743x_adapter *adapter = LAN743X_PTP_TO_ADAPTER;
>
> Likewise.
>
> > +#ifdef CONFIG_PTP_1588_CLOCK
> > +static int lan743x_ptpci_gettime64(struct ptp_clock_info *ptpci,
> > + struct timespec64 *ts)
> > +{
> > + struct lan743x_ptp *ptp = LAN743X_PTPCI_TO_PTP;
> > + struct lan743x_adapter *adapter = LAN743X_PTP_TO_ADAPTER;
>
> Likewise.
>
> > +#ifdef CONFIG_PTP_1588_CLOCK
> > +static int lan743x_ptpci_settime64(struct ptp_clock_info *ptpci,
> > + const struct timespec64 *ts)
> > +{
> > + struct lan743x_ptp *ptp = LAN743X_PTPCI_TO_PTP;
> > + struct lan743x_adapter *adapter = LAN743X_PTP_TO_ADAPTER;
>
> Likewise.
>
> Also, these X_TO_Y macros are terrible. If you aren an accessor like that,
> make it a nice inline function declared in a header file with lowercase letter
> which actually does type checking on the pointer variable which is
> dereferenced.
>
> > + if (ts) {
> > + u32 seconds = 0;
> > + u32 nano_seconds = 0;
>
> Reverse christmas tree ordering please.
>
> > +#ifdef CONFIG_PTP_1588_CLOCK
> > +static int lan743x_ptp_enable_pps(struct lan743x_adapter *adapter) {
> > + struct lan743x_ptp *ptp = &adapter->ptp;
> > + int result = -ENODEV;
> > + u32 current_seconds = 0;
> > + u32 target_seconds = 0;
> > + u32 general_config = 0;
>
> Likewise.
>
> > +static void lan743x_ptp_isr(void *context) {
> > + int enable_flag = 1;
> > + u32 ptp_int_sts = 0;
> > + struct lan743x_adapter *adapter = (struct lan743x_adapter
> *)context;
> > + struct lan743x_ptp *ptp = NULL;
>
> Likewise. And again please remove the void pointer cast.
>
> > +#ifdef CONFIG_PTP_1588_CLOCK
> > +static int lan743x_ptp_reserve_event_ch(struct lan743x_adapter
> > +*adapter) {
> > + struct lan743x_ptp *ptp = &adapter->ptp;
> > + int index = 0;
> > + int result = -ENODEV;
>
> Likewise.
>
> > +#ifdef CONFIG_PTP_1588_CLOCK
> > +static void lan743x_ptp_clock_step(struct lan743x_adapter *adapter,
> > + s64 time_step_ns)
> > +{
> > + struct lan743x_ptp *ptp = &adapter->ptp;
> > + u64 abs_time_step_ns = 0;
> > + s32 seconds = 0;
> > + u32 nano_seconds = 0;
>
> Likewise.
>
> This thing is huge, I'm not reviewing any more of this enormous submission.
David,
Thanks for the feedback.
I will work on your suggestions, and submit again later.
Bryan
^ permalink raw reply
* Re: [PATCH net-next 2/3] Add LAN743X to Kconfig and Makefile
From: kbuild test robot @ 2017-08-14 16:10 UTC (permalink / raw)
To: Bryan.Whitehead; +Cc: kbuild-all, netdev, davem, UNGLinuxDriver
In-Reply-To: <90A7E81AE28BAE4CBDDB3B35F187D264406F6049@CHN-SV-EXMX02.mchp-main.com>
[-- Attachment #1: Type: text/plain, Size: 4000 bytes --]
Hi Bryan,
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Bryan-Whitehead-microchip-com/Add-LAN743X-driver/20170814-141247
config: i386-allyesconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
drivers/net//ethernet/microchip/lan743x.c: In function 'lan743x_ptpci_settime64':
>> drivers/net//ethernet/microchip/lan743x.c:1117:83: warning: format '%ld' expects argument of type 'long int', but argument 4 has type 'time64_t {aka const long long int}' [-Wformat=]
NETIF_WARNING(adapter, drv, adapter->netdev,
^
vim +1117 drivers/net//ethernet/microchip/lan743x.c
a33f5600 Bryan Whitehead 2017-08-11 1103
a33f5600 Bryan Whitehead 2017-08-11 1104 #ifdef CONFIG_PTP_1588_CLOCK
a33f5600 Bryan Whitehead 2017-08-11 1105 static int lan743x_ptpci_settime64(struct ptp_clock_info *ptpci,
a33f5600 Bryan Whitehead 2017-08-11 1106 const struct timespec64 *ts)
a33f5600 Bryan Whitehead 2017-08-11 1107 {
a33f5600 Bryan Whitehead 2017-08-11 1108 struct lan743x_ptp *ptp = LAN743X_PTPCI_TO_PTP;
a33f5600 Bryan Whitehead 2017-08-11 1109 struct lan743x_adapter *adapter = LAN743X_PTP_TO_ADAPTER;
a33f5600 Bryan Whitehead 2017-08-11 1110
a33f5600 Bryan Whitehead 2017-08-11 1111 if (ts) {
a33f5600 Bryan Whitehead 2017-08-11 1112 u32 seconds = 0;
a33f5600 Bryan Whitehead 2017-08-11 1113 u32 nano_seconds = 0;
a33f5600 Bryan Whitehead 2017-08-11 1114
a33f5600 Bryan Whitehead 2017-08-11 1115 if ((ts->tv_sec > 0xFFFFFFFFLL) ||
a33f5600 Bryan Whitehead 2017-08-11 1116 (ts->tv_sec < 0)) {
a33f5600 Bryan Whitehead 2017-08-11 @1117 NETIF_WARNING(adapter, drv, adapter->netdev,
a33f5600 Bryan Whitehead 2017-08-11 1118 "ts->tv_sec out of range, %ld",
a33f5600 Bryan Whitehead 2017-08-11 1119 ts->tv_sec);
a33f5600 Bryan Whitehead 2017-08-11 1120 return -EINVAL;
a33f5600 Bryan Whitehead 2017-08-11 1121 }
a33f5600 Bryan Whitehead 2017-08-11 1122 if ((ts->tv_nsec >= 1000000000L) ||
a33f5600 Bryan Whitehead 2017-08-11 1123 (ts->tv_nsec < 0)) {
a33f5600 Bryan Whitehead 2017-08-11 1124 NETIF_WARNING(adapter, drv, adapter->netdev,
a33f5600 Bryan Whitehead 2017-08-11 1125 "ts->tv_nsec out of range, %ld",
a33f5600 Bryan Whitehead 2017-08-11 1126 ts->tv_nsec);
a33f5600 Bryan Whitehead 2017-08-11 1127 return -EINVAL;
a33f5600 Bryan Whitehead 2017-08-11 1128 }
a33f5600 Bryan Whitehead 2017-08-11 1129 seconds = ts->tv_sec;
a33f5600 Bryan Whitehead 2017-08-11 1130 nano_seconds = ts->tv_nsec;
a33f5600 Bryan Whitehead 2017-08-11 1131 NETIF_INFO(adapter, drv, adapter->netdev,
a33f5600 Bryan Whitehead 2017-08-11 1132 "settime = %u.%09u", seconds, nano_seconds);
a33f5600 Bryan Whitehead 2017-08-11 1133 lan743x_ptp_clock_set(adapter, seconds, nano_seconds, 0);
a33f5600 Bryan Whitehead 2017-08-11 1134 } else {
a33f5600 Bryan Whitehead 2017-08-11 1135 NETIF_WARNING(adapter, drv, adapter->netdev, "ts == NULL");
a33f5600 Bryan Whitehead 2017-08-11 1136 return -EINVAL;
a33f5600 Bryan Whitehead 2017-08-11 1137 }
a33f5600 Bryan Whitehead 2017-08-11 1138 return 0;
a33f5600 Bryan Whitehead 2017-08-11 1139 }
a33f5600 Bryan Whitehead 2017-08-11 1140 #endif /*CONFIG_PTP_1588_CLOCK */
a33f5600 Bryan Whitehead 2017-08-11 1141
:::::: The code at line 1117 was first introduced by commit
:::::: a33f5600a21257dc396a7278c7c5ff74ac2f7844 Add LAN743X driver
:::::: TO: Bryan Whitehead <Bryan.Whitehead@microchip.com>
:::::: CC: 0day robot <fengguang.wu@intel.com>
---
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: 60452 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v2] openvswitch: enable NSH support
From: Eric Garver @ 2017-08-14 16:09 UTC (permalink / raw)
To: Yi Yang
Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
jbenc-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <1502371275-52446-1-git-send-email-yi.y.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
On Thu, Aug 10, 2017 at 09:21:15PM +0800, Yi Yang wrote:
> OVS master and 2.8 branch has merged NSH userspace
> patch series, this patch is to enable NSH support
> in kernel data path in order that OVS can support
> NSH in 2.8 release in compat mode by porting this.
>
> Signed-off-by: Yi Yang <yi.y.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> drivers/net/vxlan.c | 7 ++
> include/net/nsh.h | 126 ++++++++++++++++++++++++++++++
> include/uapi/linux/openvswitch.h | 33 ++++++++
> net/openvswitch/actions.c | 165 +++++++++++++++++++++++++++++++++++++++
> net/openvswitch/flow.c | 41 ++++++++++
> net/openvswitch/flow.h | 1 +
> net/openvswitch/flow_netlink.c | 54 ++++++++++++-
> 7 files changed, 426 insertions(+), 1 deletion(-)
> create mode 100644 include/net/nsh.h
Hi Yi,
In general I'd like to echo Jiri's comments on the netlink attributes.
I'd like to see the metadata separate.
I have a few other comments below.
Thanks.
Eric.
[..]
> diff --git a/include/net/nsh.h b/include/net/nsh.h
> new file mode 100644
> index 0000000..96477a1
> --- /dev/null
> +++ b/include/net/nsh.h
> @@ -0,0 +1,126 @@
> +#ifndef __NET_NSH_H
> +#define __NET_NSH_H 1
> +
> +
> +/*
> + * Network Service Header:
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * |Ver|O|C|R|R|R|R|R|R| Length | MD Type | Next Proto |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * | Service Path ID | Service Index |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * | |
> + * ~ Mandatory/Optional Context Header ~
> + * | |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * Ver = The version field is used to ensure backward compatibility
> + * going forward with future NSH updates. It MUST be set to 0x0
> + * by the sender, in this first revision of NSH.
> + *
> + * O = OAM. when set to 0x1 indicates that this packet is an operations
> + * and management (OAM) packet. The receiving SFF and SFs nodes
> + * MUST examine the payload and take appropriate action.
> + *
> + * C = context. Indicates that a critical metadata TLV is present.
> + *
> + * Length : total length, in 4-byte words, of NSH including the Base
> + * Header, the Service Path Header and the optional variable
> + * TLVs.
> + * MD Type: indicates the format of NSH beyond the mandatory Base Header
> + * and the Service Path Header.
> + *
> + * Next Protocol: indicates the protocol type of the original packet. A
> + * new IANA registry will be created for protocol type.
> + *
> + * Service Path Identifier (SPI): identifies a service path.
> + * Participating nodes MUST use this identifier for Service
> + * Function Path selection.
> + *
> + * Service Index (SI): provides location within the SFP.
> + *
> + * [0] https://tools.ietf.org/html/draft-ietf-sfc-nsh-13
> + */
> +
> +/**
> + * struct nsh_md1_ctx - Keeps track of NSH context data
> + * @nshc<1-4>: NSH Contexts.
> + */
> +struct nsh_md1_ctx {
> + __be32 c[4];
> +};
> +
> +struct nsh_md2_tlv {
> + __be16 md_class;
> + u8 type;
> + u8 length;
> + u8 md_value[];
> +};
> +
> +struct nsh_hdr {
> + __be16 ver_flags_len;
> + u8 md_type;
> + u8 next_proto;
> + __be32 path_hdr;
> + union {
> + struct nsh_md1_ctx md1;
> + struct nsh_md2_tlv md2[0];
> + };
> +};
> +
> +/* Masking NSH header fields. */
> +#define NSH_VER_MASK 0xc000
> +#define NSH_VER_SHIFT 14
> +#define NSH_FLAGS_MASK 0x3fc0
> +#define NSH_FLAGS_SHIFT 6
> +#define NSH_LEN_MASK 0x003f
> +#define NSH_LEN_SHIFT 0
> +
> +#define NSH_SPI_MASK 0xffffff00
> +#define NSH_SPI_SHIFT 8
> +#define NSH_SI_MASK 0x000000ff
> +#define NSH_SI_SHIFT 0
> +
> +#define NSH_DST_PORT 4790 /* UDP Port for NSH on VXLAN. */
> +#define ETH_P_NSH 0x894F /* Ethertype for NSH. */
> +
> +/* NSH Base Header Next Protocol. */
> +#define NSH_P_IPV4 0x01
> +#define NSH_P_IPV6 0x02
> +#define NSH_P_ETHERNET 0x03
> +#define NSH_P_NSH 0x04
> +#define NSH_P_MPLS 0x05
> +
> +/* MD Type Registry. */
> +#define NSH_M_TYPE1 0x01
> +#define NSH_M_TYPE2 0x02
> +#define NSH_M_EXP1 0xFE
> +#define NSH_M_EXP2 0xFF
> +
> +/* NSH Metadata Length. */
> +#define NSH_M_TYPE1_MDLEN 16
> +
> +/* NSH Base Header Length */
> +#define NSH_BASE_HDR_LEN 8
> +
> +/* NSH MD Type 1 header Length. */
> +#define NSH_M_TYPE1_LEN 24
> +
> +static inline u16
> +nsh_hdr_len(const struct nsh_hdr *nsh)
> +{
> + return 4 * (ntohs(nsh->ver_flags_len) & NSH_LEN_MASK) >> NSH_LEN_SHIFT;
This is doing the multiplication before the shift. It works only because
the shift is 0.
> +}
> +
> +static inline struct nsh_md1_ctx *
> +nsh_md1_ctx(struct nsh_hdr *nsh)
> +{
> + return &nsh->md1;
> +}
> +
> +static inline struct nsh_md2_tlv *
> +nsh_md2_ctx(struct nsh_hdr *nsh)
> +{
> + return nsh->md2;
> +}
> +
> +#endif /* __NET_NSH_H */
> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
> index 156ee4c..5a1c94b 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -333,6 +333,7 @@ enum ovs_key_attr {
> OVS_KEY_ATTR_CT_LABELS, /* 16-octet connection tracking label */
> OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4, /* struct ovs_key_ct_tuple_ipv4 */
> OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV6, /* struct ovs_key_ct_tuple_ipv6 */
> + OVS_KEY_ATTR_NSH, /* struct ovs_key_nsh */
>
> #ifdef __KERNEL__
> OVS_KEY_ATTR_TUNNEL_INFO, /* struct ip_tunnel_info */
> @@ -491,6 +492,15 @@ struct ovs_key_ct_tuple_ipv6 {
> __u8 ipv6_proto;
> };
>
> +struct ovs_key_nsh {
> + __u8 flags;
> + __u8 mdtype;
> + __u8 np;
> + __u8 pad;
> + __be32 path_hdr;
> + __be32 c[4];
> +};
> +
> /**
> * enum ovs_flow_attr - attributes for %OVS_FLOW_* commands.
> * @OVS_FLOW_ATTR_KEY: Nested %OVS_KEY_ATTR_* attributes specifying the flow
> @@ -769,6 +779,25 @@ struct ovs_action_push_eth {
> struct ovs_key_ethernet addresses;
> };
>
> +#define OVS_PUSH_NSH_MAX_MD_LEN 248
> +/*
> + * struct ovs_action_push_nsh - %OVS_ACTION_ATTR_PUSH_NSH
> + * @flags: NSH header flags.
> + * @mdtype: NSH metadata type.
> + * @mdlen: Length of NSH metadata in bytes.
> + * @np: NSH next_protocol: Inner packet type.
> + * @path_hdr: NSH service path id and service index.
> + * @metadata: NSH metadata for MD type 1 or 2
> + */
> +struct ovs_action_push_nsh {
> + __u8 flags;
> + __u8 mdtype;
> + __u8 mdlen;
> + __u8 np;
> + __be32 path_hdr;
> + __u8 metadata[];
> +};
> +
> /**
> * enum ovs_action_attr - Action types.
> *
> @@ -806,6 +835,8 @@ struct ovs_action_push_eth {
> * packet.
> * @OVS_ACTION_ATTR_POP_ETH: Pop the outermost Ethernet header off the
> * packet.
> + * @OVS_ACTION_ATTR_PUSH_NSH: push NSH header to the packet.
> + * @OVS_ACTION_ATTR_POP_NSH: pop the outermost NSH header off the packet.
> *
> * Only a single header can be set with a single %OVS_ACTION_ATTR_SET. Not all
> * fields within a header are modifiable, e.g. the IPv4 protocol and fragment
> @@ -835,6 +866,8 @@ enum ovs_action_attr {
> OVS_ACTION_ATTR_TRUNC, /* u32 struct ovs_action_trunc. */
> OVS_ACTION_ATTR_PUSH_ETH, /* struct ovs_action_push_eth. */
> OVS_ACTION_ATTR_POP_ETH, /* No argument. */
> + OVS_ACTION_ATTR_PUSH_NSH, /* struct ovs_action_push_nsh. */
> + OVS_ACTION_ATTR_POP_NSH, /* No argument. */
>
> __OVS_ACTION_ATTR_MAX, /* Nothing past this will be accepted
> * from userspace. */
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index e461067..50f80bc 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -38,6 +38,7 @@
> #include <net/dsfield.h>
> #include <net/mpls.h>
> #include <net/sctp/checksum.h>
> +#include <net/nsh.h>
>
> #include "datapath.h"
> #include "flow.h"
> @@ -380,6 +381,114 @@ static int push_eth(struct sk_buff *skb, struct sw_flow_key *key,
> return 0;
> }
>
> +static int push_nsh(struct sk_buff *skb, struct sw_flow_key *key,
> + const struct ovs_action_push_nsh *oapn)
> +{
> + struct nsh_hdr *nsh;
> + size_t length = NSH_BASE_HDR_LEN + oapn->mdlen;
> + u8 next_proto;
> +
> + if (key->mac_proto == MAC_PROTO_ETHERNET) {
> + next_proto = NSH_P_ETHERNET;
> + } else {
> + switch (ntohs(skb->protocol)) {
> + case ETH_P_IP:
> + next_proto = NSH_P_IPV4;
> + break;
> + case ETH_P_IPV6:
> + next_proto = NSH_P_IPV6;
> + break;
> + case ETH_P_NSH:
> + next_proto = NSH_P_NSH;
> + break;
> + default:
> + return -ENOTSUPP;
> + }
> + }
> +
>
I believe you need to validate that oapn->mdlen is a multiple of 4.
> + /* Add the NSH header */
> + if (skb_cow_head(skb, length) < 0)
> + return -ENOMEM;
> +
> + skb_push(skb, length);
> + nsh = (struct nsh_hdr *)(skb->data);
> + nsh->ver_flags_len = htons((oapn->flags << NSH_FLAGS_SHIFT) |
> + (length >> 2));
> + nsh->next_proto = next_proto;
> + nsh->path_hdr = oapn->path_hdr;
> + nsh->md_type = oapn->mdtype;
> + switch (nsh->md_type) {
> + case NSH_M_TYPE1:
> + nsh->md1 = *(struct nsh_md1_ctx *)oapn->metadata;
> + break;
> + case NSH_M_TYPE2: {
> + /* The MD2 metadata in oapn is already padded to 4 bytes. */
> + size_t len = DIV_ROUND_UP(oapn->mdlen, 4) * 4;
> +
> + memcpy(nsh->md2, oapn->metadata, len);
I don't see any validation of oapn->mdlen. Normally this happens in
__ovs_nla_copy_actions(). It will be made easier if you add a separate
MD attribute as Jiri has suggested.
> + break;
> + }
> + default:
> + return -ENOTSUPP;
> + }
> +
> + if (!skb->inner_protocol)
> + skb_set_inner_protocol(skb, skb->protocol);
> +
> + skb->protocol = htons(ETH_P_NSH);
> + key->eth.type = htons(ETH_P_NSH);
> + skb_reset_mac_header(skb);
> + skb_reset_mac_len(skb);
> +
> + /* safe right before invalidate_flow_key */
> + key->mac_proto = MAC_PROTO_NONE;
> + invalidate_flow_key(key);
> + return 0;
> +}
> +
[..]
^ permalink raw reply
* Re: [PATCH net] datagram: When peeking datagrams with offset < 0 don't skip empty skbs
From: Thiago Macieira @ 2017-08-14 16:06 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Matthew Dawson, Paolo Abeni, Network Development
In-Reply-To: <CAF=yD-L9CpW-DRtKfWWWKcNKcPSWvXOkTGZoKBq6=fQMrcKVWQ@mail.gmail.com>
On Monday, 14 August 2017 08:03:50 PDT Willem de Bruijn wrote:
> > I'm actually surprised that only unix sockets can have negative values.
> > Is
> > there a reason for that? I had assumed that sk_set_peek_off would allow
> > negative values as the code already has to support negative values due to
> > what the initial value is.
>
> A negative initial value indicates that PEEK_OFF is disabled. It only
> makes sense to peek from a positive offset from the start of the data.
But here's a question: if the peek offset is equal to the length, should the
reading return an empty datagram? This would indicate to the caller that there
was a datagram there, which was skipped over.
That's how we deal with empty datagrams anyway.
--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
^ permalink raw reply
* [PATCH net] tcp: ulp: avoid module refcnt leak in tcp_set_ulp
From: Sabrina Dubroca @ 2017-08-14 16:04 UTC (permalink / raw)
To: netdev
Cc: Sabrina Dubroca, Dave Watson, Boris Pismenny, Tom Herbert,
Hannes Frederic Sowa
__tcp_ulp_find_autoload returns tcp_ulp_ops after taking a reference on
the module. Then, if ->init fails, tcp_set_ulp propagates the error but
nothing releases that reference.
Fixes: 734942cc4ea6 ("tcp: ULP infrastructure")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
Tom, the generalized ULP version has the same problem in ulp_set().
net/ipv4/tcp_ulp.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/net/ipv4/tcp_ulp.c b/net/ipv4/tcp_ulp.c
index 2417f55374c5..6bb9e14c710a 100644
--- a/net/ipv4/tcp_ulp.c
+++ b/net/ipv4/tcp_ulp.c
@@ -122,14 +122,14 @@ int tcp_set_ulp(struct sock *sk, const char *name)
ulp_ops = __tcp_ulp_find_autoload(name);
if (!ulp_ops)
- err = -ENOENT;
- else
- err = ulp_ops->init(sk);
+ return -ENOENT;
- if (err)
- goto out;
+ err = ulp_ops->init(sk);
+ if (err) {
+ module_put(ulp_ops->owner);
+ return err;
+ }
icsk->icsk_ulp_ops = ulp_ops;
- out:
- return err;
+ return 0;
}
--
2.14.0
^ permalink raw reply related
* Re: [PATCH net-next V2 3/3] tap: XDP support
From: Michael S. Tsirkin @ 2017-08-14 16:01 UTC (permalink / raw)
To: Jason Wang; +Cc: Jakub Kicinski, davem, netdev, linux-kernel, Daniel Borkmann
In-Reply-To: <8876b3d1-699c-d033-e855-34b24a709c81@redhat.com>
On Sat, Aug 12, 2017 at 10:48:49AM +0800, Jason Wang wrote:
>
>
> On 2017年08月12日 07:12, Jakub Kicinski wrote:
> > On Fri, 11 Aug 2017 19:41:18 +0800, Jason Wang wrote:
> > > This patch tries to implement XDP for tun. The implementation was
> > > split into two parts:
> > >
> > > - fast path: small and no gso packet. We try to do XDP at page level
> > > before build_skb(). For XDP_TX, since creating/destroying queues
> > > were completely under control of userspace, it was implemented
> > > through generic XDP helper after skb has been built. This could be
> > > optimized in the future.
> > > - slow path: big or gso packet. We try to do it after skb was created
> > > through generic XDP helpers.
> > >
> > > Test were done through pktgen with small packets.
> > >
> > > xdp1 test shows ~41.1% improvement:
> > >
> > > Before: ~1.7Mpps
> > > After: ~2.3Mpps
> > >
> > > xdp_redirect to ixgbe shows ~60% improvement:
> > >
> > > Before: ~0.8Mpps
> > > After: ~1.38Mpps
> > >
> > > Suggested-by: Michael S. Tsirkin <mst@redhat.com>
> > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > Looks OK to me now :)
> >
> > Out of curiosity, you say the build_skb() is for "small packets", and it
> > seems you are always reserving the 256B regardless of XDP being
> > installed. Does this have no performance impact on non-XDP case?
>
> Have a test, only less than 1% were noticed which I think could be ignored.
>
> Thanks
What did you test btw? The biggest issue would be with something like
UDP with short packets.
--
MST
^ 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