* Re: [Patch net] addrconf: reduce unnecessary atomic allocations
From: David Ahern @ 2018-08-22 20:09 UTC (permalink / raw)
To: Cong Wang, netdev; +Cc: David Ahern
In-Reply-To: <20180822195834.7217-1-xiyou.wangcong@gmail.com>
On 8/22/18 1:58 PM, Cong Wang wrote:
> All the 3 callers of addrconf_add_mroute() assert RTNL
> lock, they don't take any additional lock either, so
> it is safe to convert it to GFP_KERNEL.
>
> Same for sit_add_v4_addrs().
>
> Cc: David Ahern <dsahern@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
> net/ipv6/addrconf.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
Not sure how I missed the double ASSERT_RTNL() check for
sit_add_v4_addrs. Thanks for following up.
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* [Patch net] addrconf: reduce unnecessary atomic allocations
From: Cong Wang @ 2018-08-22 19:58 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David Ahern
All the 3 callers of addrconf_add_mroute() assert RTNL
lock, they don't take any additional lock either, so
it is safe to convert it to GFP_KERNEL.
Same for sit_add_v4_addrs().
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/ipv6/addrconf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 2fac4ad74867..d51a8c0b3372 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2398,7 +2398,7 @@ static void addrconf_add_mroute(struct net_device *dev)
ipv6_addr_set(&cfg.fc_dst, htonl(0xFF000000), 0, 0, 0);
- ip6_route_add(&cfg, GFP_ATOMIC, NULL);
+ ip6_route_add(&cfg, GFP_KERNEL, NULL);
}
static struct inet6_dev *addrconf_add_dev(struct net_device *dev)
@@ -3062,7 +3062,7 @@ static void sit_add_v4_addrs(struct inet6_dev *idev)
if (addr.s6_addr32[3]) {
add_addr(idev, &addr, plen, scope);
addrconf_prefix_route(&addr, plen, 0, idev->dev, 0, pflags,
- GFP_ATOMIC);
+ GFP_KERNEL);
return;
}
@@ -3087,7 +3087,7 @@ static void sit_add_v4_addrs(struct inet6_dev *idev)
add_addr(idev, &addr, plen, flag);
addrconf_prefix_route(&addr, plen, 0, idev->dev,
- 0, pflags, GFP_ATOMIC);
+ 0, pflags, GFP_KERNEL);
}
}
}
--
2.14.4
^ permalink raw reply related
* [Patch net] net_sched: fix a compiler warning for tcf_exts_for_each_action()
From: Cong Wang @ 2018-08-22 19:40 UTC (permalink / raw)
To: netdev; +Cc: sfr, Cong Wang
When CONFIG_NET_CLS_ACT=n, tcf_exts_for_each_action()
is a nop, which leaves its parameters unused. Shut up
the compiler warning by casting them to void.
Fixes: 244cd96adb5f ("net_sched: remove list_head from tc_action")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/net/pkt_cls.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index c17d51865469..9ec471ffaa5d 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -303,7 +303,7 @@ static inline void tcf_exts_put_net(struct tcf_exts *exts)
for (i = 0; i < TCA_ACT_MAX_PRIO && ((a) = (exts)->actions[i]); i++)
#else
#define tcf_exts_for_each_action(i, a, exts) \
- for (; 0; )
+ for ((void)i, (void)a; 0; )
#endif
static inline void
--
2.14.4
^ permalink raw reply related
* Re: linux-next: build warning after merge of the net tree
From: Cong Wang @ 2018-08-22 19:26 UTC (permalink / raw)
To: Stephen Rothwell
Cc: David Miller, Linux Kernel Network Developers,
Linux-Next Mailing List, LKML
In-Reply-To: <20180822080449.5887e244@canb.auug.org.au>
On Tue, Aug 21, 2018 at 3:04 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
>
> After merging the net tree, today's linux-next build (KCONFIG_NAME)
> produced this warning:
>
> drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c: In function 'tc_fill_actions':
> drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c:64:6: warning: unused variable 'i' [-Wunused-variable]
> int i;
> ^
>
> Introduced by commit
>
> 244cd96adb5f ("net_sched: remove list_head from tc_action")
I bet you have CONFIG_NET_CLS_ACT=n?
Here is a quick fix:
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index c17d51865469..9ec471ffaa5d 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -303,7 +303,7 @@ static inline void tcf_exts_put_net(struct tcf_exts *exts)
for (i = 0; i < TCA_ACT_MAX_PRIO && ((a) = (exts)->actions[i]); i++)
#else
#define tcf_exts_for_each_action(i, a, exts) \
- for (; 0; )
+ for ((void)i, (void)a; 0; )
#endif
static inline void
Interestingly, neither my compiler nor kbuild-bot's compiler doesn't
catch this.
Thanks.
^ permalink raw reply related
* [RFC PATCH v2 bpf-next 2/2] bpf/verifier: display non-spill stack slot types in print_verifier_state
From: Edward Cree @ 2018-08-22 19:02 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev
In-Reply-To: <d16ea072-61a0-8f8a-aca1-13cac09d3d14@solarflare.com>
If a stack slot does not hold a spilled register (STACK_SPILL), then each
of its eight bytes could potentially have a different slot_type. This
information can be important for debugging, and previously we either did
not print anything for the stack slot, or just printed fp-X=0 in the case
where its first byte was STACK_ZERO.
Instead, print eight characters with either 0 (STACK_ZERO), m (STACK_MISC)
or ? (STACK_INVALID) for any stack slot which is neither STACK_SPILL nor
entirely STACK_INVALID.
Signed-off-by: Edward Cree <ecree@solarflare.com>
---
kernel/bpf/verifier.c | 32 +++++++++++++++++++++++++-------
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b11d45916fff..2f4b52cf864c 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -263,6 +263,13 @@ static const char * const reg_type_str[] = {
[PTR_TO_PACKET_END] = "pkt_end",
};
+static char slot_type_char[] = {
+ [STACK_INVALID] = '?',
+ [STACK_SPILL] = 'r',
+ [STACK_MISC] = 'm',
+ [STACK_ZERO] = '0',
+};
+
static void print_liveness(struct bpf_verifier_env *env,
enum bpf_reg_liveness live)
{
@@ -349,15 +356,26 @@ static void print_verifier_state(struct bpf_verifier_env *env,
}
}
for (i = 0; i < state->allocated_stack / BPF_REG_SIZE; i++) {
- if (state->stack[i].slot_type[0] == STACK_SPILL) {
- verbose(env, " fp%d",
- (-i - 1) * BPF_REG_SIZE);
- print_liveness(env, state->stack[i].spilled_ptr.live);
+ char types_buf[BPF_REG_SIZE + 1];
+ bool valid = false;
+ int j;
+
+ for (j = 0; j < BPF_REG_SIZE; j++) {
+ if (state->stack[i].slot_type[j] != STACK_INVALID)
+ valid = true;
+ types_buf[j] = slot_type_char[
+ state->stack[i].slot_type[j]];
+ }
+ types_buf[BPF_REG_SIZE] = 0;
+ if (!valid)
+ continue;
+ verbose(env, " fp%d", (-i - 1) * BPF_REG_SIZE);
+ print_liveness(env, state->stack[i].spilled_ptr.live);
+ if (state->stack[i].slot_type[0] == STACK_SPILL)
verbose(env, "=%s",
reg_type_str[state->stack[i].spilled_ptr.type]);
- }
- if (state->stack[i].slot_type[0] == STACK_ZERO)
- verbose(env, " fp%d=0", (-i - 1) * BPF_REG_SIZE);
+ else
+ verbose(env, "=%s", types_buf);
}
verbose(env, "\n");
}
^ permalink raw reply related
* [RFC PATCH v2 bpf-next 1/2] bpf/verifier: per-register parent pointers
From: Edward Cree @ 2018-08-22 19:02 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev
In-Reply-To: <d16ea072-61a0-8f8a-aca1-13cac09d3d14@solarflare.com>
By giving each register its own liveness chain, we elide the skip_callee()
logic. Instead, each register's parent is the state it inherits from;
both check_func_call() and prepare_func_exit() automatically connect
reg states to the correct chain since when they copy the reg state across
(r1-r5 into the callee as args, and r0 out as the return value) they also
copy the parent pointer.
Signed-off-by: Edward Cree <ecree@solarflare.com>
---
include/linux/bpf_verifier.h | 8 +-
kernel/bpf/verifier.c | 184 +++++++++++--------------------------------
2 files changed, 47 insertions(+), 145 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 38b04f559ad3..b42b60a83e19 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -41,6 +41,7 @@ enum bpf_reg_liveness {
};
struct bpf_reg_state {
+ /* Ordering of fields matters. See states_equal() */
enum bpf_reg_type type;
union {
/* valid when type == PTR_TO_PACKET */
@@ -59,7 +60,6 @@ struct bpf_reg_state {
* came from, when one is tested for != NULL.
*/
u32 id;
- /* 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
@@ -76,15 +76,15 @@ 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 */
+ /* parentage chain for liveness checking */
+ struct bpf_reg_state *parent;
/* Inside the callee two registers can be both PTR_TO_STACK like
* R1=fp-8 and R2=fp-8, but one of them points to this function stack
* while another to the caller's stack. To differentiate them 'frameno'
* is used which is an index in bpf_verifier_state->frame[] array
* pointing to bpf_func_state.
- * This field must be second to last, for states_equal() reasons.
*/
u32 frameno;
- /* This field must be last, for states_equal() reasons. */
enum bpf_reg_liveness live;
};
@@ -107,7 +107,6 @@ struct bpf_stack_state {
*/
struct bpf_func_state {
struct bpf_reg_state regs[MAX_BPF_REG];
- struct bpf_verifier_state *parent;
/* index of call instruction that called into this func */
int callsite;
/* stack frame number of this function state from pov of
@@ -129,7 +128,6 @@ struct bpf_func_state {
struct bpf_verifier_state {
/* call stack tracking */
struct bpf_func_state *frame[MAX_CALL_FRAMES];
- struct bpf_verifier_state *parent;
u32 curframe;
};
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ca90679a7fe5..b11d45916fff 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -380,9 +380,9 @@ static int copy_stack_state(struct bpf_func_state *dst,
/* do_check() starts with zero-sized stack in struct bpf_verifier_state to
* make it consume minimal amount of memory. check_stack_write() access from
* the program calls into realloc_func_state() to grow the stack size.
- * Note there is a non-zero 'parent' pointer inside bpf_verifier_state
- * which this function copies over. It points to previous bpf_verifier_state
- * which is never reallocated
+ * Note there is a non-zero parent pointer inside each reg of bpf_verifier_state
+ * which this function copies over. It points to corresponding reg in previous
+ * bpf_verifier_state which is never reallocated
*/
static int realloc_func_state(struct bpf_func_state *state, int size,
bool copy_old)
@@ -466,7 +466,6 @@ static int copy_verifier_state(struct bpf_verifier_state *dst_state,
dst_state->frame[i] = NULL;
}
dst_state->curframe = src->curframe;
- dst_state->parent = src->parent;
for (i = 0; i <= src->curframe; i++) {
dst = dst_state->frame[i];
if (!dst) {
@@ -732,6 +731,7 @@ static void init_reg_state(struct bpf_verifier_env *env,
for (i = 0; i < MAX_BPF_REG; i++) {
mark_reg_not_init(env, regs, i);
regs[i].live = REG_LIVE_NONE;
+ regs[i].parent = NULL;
}
/* frame pointer */
@@ -876,74 +876,21 @@ static int check_subprogs(struct bpf_verifier_env *env)
return 0;
}
-static
-struct bpf_verifier_state *skip_callee(struct bpf_verifier_env *env,
- const struct bpf_verifier_state *state,
- struct bpf_verifier_state *parent,
- u32 regno)
-{
- struct bpf_verifier_state *tmp = NULL;
-
- /* 'parent' could be a state of caller and
- * 'state' could be a state of callee. In such case
- * parent->curframe < state->curframe
- * and it's ok for r1 - r5 registers
- *
- * 'parent' could be a callee's state after it bpf_exit-ed.
- * In such case parent->curframe > state->curframe
- * and it's ok for r0 only
- */
- if (parent->curframe == state->curframe ||
- (parent->curframe < state->curframe &&
- regno >= BPF_REG_1 && regno <= BPF_REG_5) ||
- (parent->curframe > state->curframe &&
- regno == BPF_REG_0))
- return parent;
-
- if (parent->curframe > state->curframe &&
- regno >= BPF_REG_6) {
- /* for callee saved regs we have to skip the whole chain
- * of states that belong to callee and mark as LIVE_READ
- * the registers before the call
- */
- tmp = parent;
- while (tmp && tmp->curframe != state->curframe) {
- tmp = tmp->parent;
- }
- if (!tmp)
- goto bug;
- parent = tmp;
- } else {
- goto bug;
- }
- return parent;
-bug:
- verbose(env, "verifier bug regno %d tmp %p\n", regno, tmp);
- verbose(env, "regno %d parent frame %d current frame %d\n",
- regno, parent->curframe, state->curframe);
- return NULL;
-}
-
+/* Parentage chain of this register (or stack slot) should take care of all
+ * issues like callee-saved registers, stack slot allocation time, etc.
+ */
static int mark_reg_read(struct bpf_verifier_env *env,
- const struct bpf_verifier_state *state,
- struct bpf_verifier_state *parent,
- u32 regno)
+ const struct bpf_reg_state *state,
+ struct bpf_reg_state *parent)
{
bool writes = parent == state->parent; /* Observe write marks */
- if (regno == BPF_REG_FP)
- /* We don't need to worry about FP liveness because it's read-only */
- return 0;
-
while (parent) {
/* if read wasn't screened by an earlier write ... */
- if (writes && state->frame[state->curframe]->regs[regno].live & REG_LIVE_WRITTEN)
+ if (writes && state->live & REG_LIVE_WRITTEN)
break;
- parent = skip_callee(env, state, parent, regno);
- if (!parent)
- return -EFAULT;
/* ... then we depend on parent's value */
- parent->frame[parent->curframe]->regs[regno].live |= REG_LIVE_READ;
+ parent->live |= REG_LIVE_READ;
state = parent;
parent = state->parent;
writes = true;
@@ -969,7 +916,10 @@ static int check_reg_arg(struct bpf_verifier_env *env, u32 regno,
verbose(env, "R%d !read_ok\n", regno);
return -EACCES;
}
- return mark_reg_read(env, vstate, vstate->parent, regno);
+ /* We don't need to worry about FP liveness because it's read-only */
+ if (regno != BPF_REG_FP)
+ return mark_reg_read(env, ®s[regno],
+ regs[regno].parent);
} else {
/* check whether register used as dest operand can be written to */
if (regno == BPF_REG_FP) {
@@ -1080,8 +1030,8 @@ static int check_stack_write(struct bpf_verifier_env *env,
} else {
u8 type = STACK_MISC;
- /* regular write of data into stack */
- state->stack[spi].spilled_ptr = (struct bpf_reg_state) {};
+ /* regular write of data into stack destroys any spilled ptr */
+ state->stack[spi].spilled_ptr.type = NOT_INIT;
/* only mark the slot as written if all 8 bytes were written
* otherwise read propagation may incorrectly stop too soon
@@ -1106,61 +1056,6 @@ static int check_stack_write(struct bpf_verifier_env *env,
return 0;
}
-/* registers of every function are unique and mark_reg_read() propagates
- * the liveness in the following cases:
- * - from callee into caller for R1 - R5 that were used as arguments
- * - from caller into callee for R0 that used as result of the call
- * - from caller to the same caller skipping states of the callee for R6 - R9,
- * since R6 - R9 are callee saved by implicit function prologue and
- * caller's R6 != callee's R6, so when we propagate liveness up to
- * parent states we need to skip callee states for R6 - R9.
- *
- * stack slot marking is different, since stacks of caller and callee are
- * accessible in both (since caller can pass a pointer to caller's stack to
- * callee which can pass it to another function), hence mark_stack_slot_read()
- * has to propagate the stack liveness to all parent states at given frame number.
- * Consider code:
- * f1() {
- * ptr = fp - 8;
- * *ptr = ctx;
- * call f2 {
- * .. = *ptr;
- * }
- * .. = *ptr;
- * }
- * First *ptr is reading from f1's stack and mark_stack_slot_read() has
- * to mark liveness at the f1's frame and not f2's frame.
- * Second *ptr is also reading from f1's stack and mark_stack_slot_read() has
- * to propagate liveness to f2 states at f1's frame level and further into
- * f1 states at f1's frame level until write into that stack slot
- */
-static void mark_stack_slot_read(struct bpf_verifier_env *env,
- const struct bpf_verifier_state *state,
- struct bpf_verifier_state *parent,
- int slot, int frameno)
-{
- bool writes = parent == state->parent; /* Observe write marks */
-
- while (parent) {
- if (parent->frame[frameno]->allocated_stack <= slot * BPF_REG_SIZE)
- /* since LIVE_WRITTEN mark is only done for full 8-byte
- * write the read marks are conservative and parent
- * state may not even have the stack allocated. In such case
- * end the propagation, since the loop reached beginning
- * of the function
- */
- break;
- /* if read wasn't screened by an earlier write ... */
- if (writes && state->frame[frameno]->stack[slot].spilled_ptr.live & REG_LIVE_WRITTEN)
- break;
- /* ... then we depend on parent's value */
- parent->frame[frameno]->stack[slot].spilled_ptr.live |= REG_LIVE_READ;
- state = parent;
- parent = state->parent;
- writes = true;
- }
-}
-
static int check_stack_read(struct bpf_verifier_env *env,
struct bpf_func_state *reg_state /* func where register points to */,
int off, int size, int value_regno)
@@ -1198,8 +1093,8 @@ static int check_stack_read(struct bpf_verifier_env *env,
*/
state->regs[value_regno].live |= REG_LIVE_WRITTEN;
}
- mark_stack_slot_read(env, vstate, vstate->parent, spi,
- reg_state->frameno);
+ mark_reg_read(env, ®_state->stack[spi].spilled_ptr,
+ reg_state->stack[spi].spilled_ptr.parent);
return 0;
} else {
int zeros = 0;
@@ -1215,8 +1110,8 @@ static int check_stack_read(struct bpf_verifier_env *env,
off, i, size);
return -EACCES;
}
- mark_stack_slot_read(env, vstate, vstate->parent, spi,
- reg_state->frameno);
+ mark_reg_read(env, ®_state->stack[spi].spilled_ptr,
+ reg_state->stack[spi].spilled_ptr.parent);
if (value_regno >= 0) {
if (zeros == size) {
/* any size read into register is zero extended,
@@ -1908,8 +1803,8 @@ static int check_stack_boundary(struct bpf_verifier_env *env, int regno,
/* reading any byte out of 8-byte 'spill_slot' will cause
* the whole slot to be marked as 'read'
*/
- mark_stack_slot_read(env, env->cur_state, env->cur_state->parent,
- spi, state->frameno);
+ mark_reg_read(env, &state->stack[spi].spilled_ptr,
+ state->stack[spi].spilled_ptr.parent);
}
return update_stack_depth(env, state, off);
}
@@ -2366,11 +2261,13 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
state->curframe + 1 /* frameno within this callchain */,
subprog /* subprog number within this prog */);
- /* copy r1 - r5 args that callee can access */
+ /* copy r1 - r5 args that callee can access. The copy includes parent
+ * pointers, which connects us up to the liveness chain
+ */
for (i = BPF_REG_1; i <= BPF_REG_5; i++)
callee->regs[i] = caller->regs[i];
- /* after the call regsiters r0 - r5 were scratched */
+ /* after the call registers r0 - r5 were scratched */
for (i = 0; i < CALLER_SAVED_REGS; i++) {
mark_reg_not_init(env, caller->regs, caller_saved[i]);
check_reg_arg(env, caller_saved[i], DST_OP_NO_MARK);
@@ -4370,7 +4267,7 @@ static bool regsafe(struct bpf_reg_state *rold, struct bpf_reg_state *rcur,
/* explored state didn't use this */
return true;
- equal = memcmp(rold, rcur, offsetof(struct bpf_reg_state, frameno)) == 0;
+ equal = memcmp(rold, rcur, offsetof(struct bpf_reg_state, parent)) == 0;
if (rold->type == PTR_TO_STACK)
/* two stack pointers are equal only if they're pointing to
@@ -4603,7 +4500,7 @@ static bool states_equal(struct bpf_verifier_env *env,
* equivalent state (jump target or such) we didn't arrive by the straight-line
* code, so read marks in the state must propagate to the parent regardless
* of the state's write marks. That's what 'parent == state->parent' comparison
- * in mark_reg_read() and mark_stack_slot_read() is for.
+ * in mark_reg_read() is for.
*/
static int propagate_liveness(struct bpf_verifier_env *env,
const struct bpf_verifier_state *vstate,
@@ -4624,7 +4521,8 @@ static int propagate_liveness(struct bpf_verifier_env *env,
if (vparent->frame[vparent->curframe]->regs[i].live & REG_LIVE_READ)
continue;
if (vstate->frame[vstate->curframe]->regs[i].live & REG_LIVE_READ) {
- err = mark_reg_read(env, vstate, vparent, i);
+ err = mark_reg_read(env, &vstate->frame[vstate->curframe]->regs[i],
+ &vparent->frame[vstate->curframe]->regs[i]);
if (err)
return err;
}
@@ -4639,7 +4537,8 @@ static int propagate_liveness(struct bpf_verifier_env *env,
if (parent->stack[i].spilled_ptr.live & REG_LIVE_READ)
continue;
if (state->stack[i].spilled_ptr.live & REG_LIVE_READ)
- mark_stack_slot_read(env, vstate, vparent, i, frame);
+ mark_reg_read(env, &state->stack[i].spilled_ptr,
+ &parent->stack[i].spilled_ptr);
}
}
return err;
@@ -4649,7 +4548,7 @@ 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;
- struct bpf_verifier_state *cur = env->cur_state;
+ struct bpf_verifier_state *cur = env->cur_state, *new;
int i, j, err;
sl = env->explored_states[insn_idx];
@@ -4691,16 +4590,18 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
return -ENOMEM;
/* add new state to the head of linked list */
- err = copy_verifier_state(&new_sl->state, cur);
+ new = &new_sl->state;
+ err = copy_verifier_state(new, cur);
if (err) {
- free_verifier_state(&new_sl->state, false);
+ free_verifier_state(new, false);
kfree(new_sl);
return err;
}
new_sl->next = env->explored_states[insn_idx];
env->explored_states[insn_idx] = new_sl;
/* connect new state to parentage chain */
- cur->parent = &new_sl->state;
+ for (i = 0; i < BPF_REG_FP; i++)
+ cur_regs(env)[i].parent = &new->frame[new->curframe]->regs[i];
/* clear write marks in current state: the writes we did are not writes
* our child did, so they don't screen off its reads from us.
* (There are no read marks in current state, because reads always mark
@@ -4713,9 +4614,13 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
/* all stack frames are accessible from callee, clear them all */
for (j = 0; j <= cur->curframe; j++) {
struct bpf_func_state *frame = cur->frame[j];
+ struct bpf_func_state *newframe = new->frame[j];
- for (i = 0; i < frame->allocated_stack / BPF_REG_SIZE; i++)
+ for (i = 0; i < frame->allocated_stack / BPF_REG_SIZE; i++) {
frame->stack[i].spilled_ptr.live = REG_LIVE_NONE;
+ frame->stack[i].spilled_ptr.parent =
+ &newframe->stack[i].spilled_ptr;
+ }
}
return 0;
}
@@ -4734,7 +4639,6 @@ static int do_check(struct bpf_verifier_env *env)
if (!state)
return -ENOMEM;
state->curframe = 0;
- state->parent = NULL;
state->frame[0] = kzalloc(sizeof(struct bpf_func_state), GFP_KERNEL);
if (!state->frame[0]) {
kfree(state);
^ permalink raw reply related
* [RFC PATCH v2 bpf-next 0/2] verifier liveness simplification
From: Edward Cree @ 2018-08-22 19:00 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev
The first patch is a simplification of register liveness tracking by using
a separate parentage chain for each register and stack slot, thus avoiding
the need for logic to handle callee-saved registers when applying read
marks. In the future this idea may be extended to form use-def chains.
The second patch adds information about misc/zero data on the stack to the
state dumps emitted to the log at various points; this information was
found essential in debugging the first patch, and may be useful elsewhere.
Edward Cree (2):
bpf/verifier: per-register parent pointers
bpf/verifier: display non-spill stack slot types in
print_verifier_state
include/linux/bpf_verifier.h | 8 +-
kernel/bpf/verifier.c | 216 ++++++++++++++-----------------------------
2 files changed, 72 insertions(+), 152 deletions(-)
^ permalink raw reply
* Re: [bpf PATCH 0/2] tls, sockmap, fixes for sk_wait_event
From: Alexei Starovoitov @ 2018-08-22 18:40 UTC (permalink / raw)
To: John Fastabend; +Cc: ast, daniel, davejwatson, netdev, davem
In-Reply-To: <20180822153314.27968.72499.stgit@john-Precision-Tower-5810>
On Wed, Aug 22, 2018 at 08:37:27AM -0700, John Fastabend wrote:
> I have been testing ktls and sockmap lately and noticed that neither
> was handling sk_write_space events correctly. We need to ensure
> these events are pushed down to the lower layer in all cases to
> handle the case where the lower layer sendpage call has called
> sk_wait_event and needs to be woken up. Without this I see
> occosional stalls of sndtimeo length while we wait for the
> timeout value even though space is available.
>
> Two fixes below. Thanks.
for the set
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [bpf PATCH 1/2] tls: possible hang when do_tcp_sendpages hits sndbuf is full case
From: Dave Watson @ 2018-08-22 18:22 UTC (permalink / raw)
To: John Fastabend; +Cc: ast, daniel, netdev, davem
In-Reply-To: <20180822153732.27968.41693.stgit@john-Precision-Tower-5810>
On 08/22/18 08:37 AM, John Fastabend wrote:
> Currently, the lower protocols sk_write_space handler is not called if
> TLS is sending a scatterlist via tls_push_sg. However, normally
> tls_push_sg calls do_tcp_sendpage, which may be under memory pressure,
> that in turn may trigger a wait via sk_wait_event. Typically, this
> happens when the in-flight bytes exceed the sdnbuf size. In the normal
> case when enough ACKs are received sk_write_space() will be called and
> the sk_wait_event will be woken up allowing it to send more data
> and/or return to the user.
>
> But, in the TLS case because the sk_write_space() handler does not
> wake up the events the above send will wait until the sndtimeo is
> exceeded. By default this is MAX_SCHEDULE_TIMEOUT so it look like a
> hang to the user (especially this impatient user). To fix this pass
> the sk_write_space event to the lower layers sk_write_space event
> which in the TCP case will wake any pending events.
>
> I observed the above while integrating sockmap and ktls. It
> initially appeared as test_sockmap (modified to use ktls) occasionally
> hanging. To reliably reproduce this reduce the sndbuf size and stress
> the tls layer by sending many 1B sends. This results in every byte
> needing a header and each byte individually being sent to the crypto
> layer.
>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Super, thanks!
Acked-by: Dave Watson <davejwatson@fb.com>
^ permalink raw reply
* Re: [PATCH 05/11] can: rcar: use SPDX identifier for Renesas drivers
From: Philippe Ombredanne @ 2018-08-22 18:16 UTC (permalink / raw)
To: Fabio Estevam
Cc: Wolfram Sang, Greg Kroah-Hartman, Wolfram Sang, Linux-Renesas,
Kuninori Morimoto, Wolfgang Grandegger, Marc Kleine-Budde,
David S. Miller, linux-can, netdev, linux-kernel, Thomas Gleixner
In-Reply-To: <CAOMZO5BiAyDMSR=2Y-4-1ftNuVtRS59sSJd_cdFmzX=0kgF0Bw@mail.gmail.com>
Hi Fabio,
On Wed, Aug 22, 2018 at 2:55 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Wed, Aug 22, 2018 at 3:30 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>>
>>> > +// SPDX-License-Identifier: GPL-2.0-or-later
>>>
>>> According to Documentation/process/license-rules.rst the format should
>>> be like this instead:
>>>
>>> // SPDX-License-Identifier: GPL-2.0+
>>
>> According to https://spdx.org/licenses/ it should be what I did above.
>
> Previous advice I saw was to follow the format described in
> Documentation/process/license-rules.rst
>
> Greg/Philippe,
>
> Any inputs on this matter?
>
> Thanks
IMHO we should always treat and use the
Documentation/process/license-rules.rst as the reference and not SPDX
proper who moves at its own pace and evolves its specs and license ids
independently of where we stand in the kernel.
If this is not right Doc patches are welcomed!
In this is very specific case this has been discussed on list a few
times. If I recall correctly Thomas also had an opinion on this...
So you are correct and this should be for now:
// SPDX-License-Identifier: GPL-2.0+
--
Cordially
Philippe Ombredanne
^ permalink raw reply
* [PATCH iproute2 2/3] testsuite: let make compile build the netlink helper
From: Luca Boccassi @ 2018-08-22 18:09 UTC (permalink / raw)
To: netdev; +Cc: stephen, stefan.bader
In-Reply-To: <20180822180903.26443-1-bluca@debian.org>
The generate_nlmsg binary is required but make -C testsuite compile
does not build it. Add the necessary includes and C*FLAGS to the tools
Makefile and have the compile target build it.
Signed-off-by: Luca Boccassi <bluca@debian.org>
---
testsuite/Makefile | 1 +
testsuite/tools/Makefile | 4 +++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/testsuite/Makefile b/testsuite/Makefile
index 2acd0427..5e269877 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -32,6 +32,7 @@ configure:
compile: configure
echo "Entering iproute2" && cd iproute2 && $(MAKE) && cd ..;
+ $(MAKE) -C tools
listtests:
@for t in $(TESTS); do \
diff --git a/testsuite/tools/Makefile b/testsuite/tools/Makefile
index f0ce4ee2..c936af71 100644
--- a/testsuite/tools/Makefile
+++ b/testsuite/tools/Makefile
@@ -1,6 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
+include ../../config.mk
+
generate_nlmsg: generate_nlmsg.c ../../lib/libnetlink.c
- $(CC) -o $@ $^
+ $(CC) $(CPPFLAGS) $(CFLAGS) $(LDLIBS) $(EXTRA_CFLAGS) -I../../include -include../../include/uapi/linux/netlink.h -o $@ $^
clean:
rm -f generate_nlmsg
--
2.18.0
^ permalink raw reply related
* [PATCH iproute2 3/3] testsuite: run dmesg with sudo
From: Luca Boccassi @ 2018-08-22 18:09 UTC (permalink / raw)
To: netdev; +Cc: stephen, stefan.bader
In-Reply-To: <20180822180903.26443-1-bluca@debian.org>
Some distributions like Debian nowadays restrict the dmesg command to
root-only. Run it with sudo in the testsuite.
Signed-off-by: Luca Boccassi <bluca@debian.org>
---
testsuite/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/testsuite/Makefile b/testsuite/Makefile
index 5e269877..ef45d5a7 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -79,5 +79,5 @@ endif
echo "PASS"; \
fi; \
rm "$$TMP_ERR" "$$TMP_OUT"; \
- dmesg > $(RESULTS_DIR)/$@.$$o.dmesg; \
+ sudo dmesg > $(RESULTS_DIR)/$@.$$o.dmesg; \
done
--
2.18.0
^ permalink raw reply related
* [PATCH iproute2 1/3] testsuite: remove all temp files and implement make clean
From: Luca Boccassi @ 2018-08-22 18:09 UTC (permalink / raw)
To: netdev; +Cc: stephen, stefan.bader
Some generated test files were not removed, including one executable in
the testsuite/tools directory.
Ensure make clean from the top level directory works for the testsuite
subdirs too, and that all the files are removed.
Signed-off-by: Luca Boccassi <bluca@debian.org>
---
Makefile | 2 +-
testsuite/Makefile | 3 +++
testsuite/tools/Makefile | 3 +++
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 651d2a50..ea2f797c 100644
--- a/Makefile
+++ b/Makefile
@@ -96,7 +96,7 @@ snapshot:
> include/SNAPSHOT.h
clean:
- @for i in $(SUBDIRS); \
+ @for i in $(SUBDIRS) testsuite; \
do $(MAKE) $(MFLAGS) -C $$i clean; done
clobber:
diff --git a/testsuite/Makefile b/testsuite/Makefile
index 8fcbc557..2acd0427 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -43,6 +43,9 @@ alltests: $(TESTS)
clean:
@echo "Removing $(RESULTS_DIR) dir ..."
@rm -rf $(RESULTS_DIR)
+ @rm -f iproute2/iproute2-this
+ @rm -f tests/ip/link/dev_wo_vf_rate.nl
+ $(MAKE) -C tools clean
distclean: clean
echo "Entering iproute2" && cd iproute2 && $(MAKE) distclean && cd ..;
diff --git a/testsuite/tools/Makefile b/testsuite/tools/Makefile
index f2cdc980..f0ce4ee2 100644
--- a/testsuite/tools/Makefile
+++ b/testsuite/tools/Makefile
@@ -1,3 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
generate_nlmsg: generate_nlmsg.c ../../lib/libnetlink.c
$(CC) -o $@ $^
+
+clean:
+ rm -f generate_nlmsg
--
2.18.0
^ permalink raw reply related
* [PATCH] hv_netvsc: Fix a deadlock by getting rtnl_lock earlier in netvsc_probe()
From: Dexuan Cui @ 2018-08-22 21:20 UTC (permalink / raw)
To: KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
'David S. Miller', 'netdev@vger.kernel.org'
Cc: 'devel@linuxdriverproject.org',
'linux-kernel@vger.kernel.org', 'olaf@aepfle.de',
'apw@canonical.com', 'jasowang@redhat.com',
vkuznets, 'marcelo.cerri@canonical.com', Josh Poulson
This patch fixes the race between netvsc_probe() and
rndis_set_subchannel(), which can cause a deadlock.
Fixes: 8195b1396ec8 ("hv_netvsc: fix deadlock on hotplug")
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/hyperv/netvsc_drv.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
FYI: these are the related 3 paths which show the deadlock:
#1:
Workqueue: hv_vmbus_con vmbus_onmessage_work [hv_vmbus]
Call Trace:
schedule
schedule_preempt_disabled
__mutex_lock
__device_attach
bus_probe_device
device_add
vmbus_device_register
vmbus_onoffer
vmbus_onmessage_work
process_one_work
worker_thread
kthread
ret_from_fork
#2:
schedule
schedule_preempt_disabled
__mutex_lock
netvsc_probe
vmbus_probe
really_probe
__driver_attach
bus_for_each_dev
driver_attach_async
async_run_entry_fn
process_one_work
worker_thread
kthread
ret_from_fork
#3:
Workqueue: events netvsc_subchan_work [hv_netvsc]
Call Trace:
schedule
rndis_set_subchannel
netvsc_subchan_work
process_one_work
worker_thread
kthread
ret_from_fork
Before path #1 finishes, path #2 can start to run, because just before
the "bus_probe_device(dev);" in device_add() in path #1, there is a line
"object_uevent(&dev->kobj, KOBJ_ADD);", so systemd-udevd can
immediately try to load hv_netvsc and hence path #2 can start to run.
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 1121a1ec..4fd14a0 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2206,6 +2206,16 @@ static int netvsc_probe(struct hv_device *dev,
memcpy(net->dev_addr, device_info.mac_adr, ETH_ALEN);
+ /* We must get rtnl_lock before scheduling nvdev->subchan_work,
+ * otherwise netvsc_subchan_work() can get rtnl_lock first and wait
+ * all subchannels to show up, but that may not happen because
+ * netvsc_probe() can't get rtnl_lock and as a result vmbus_onoffer()
+ * -> ... -> device_add() -> ... -> __device_attach() can't get
+ * the device lock, so all the subchannels can't be processed --
+ * finally netvsc_subchan_work() hangs for ever.
+ */
+ rtnl_lock();
+
if (nvdev->num_chn > 1)
schedule_work(&nvdev->subchan_work);
@@ -2224,7 +2234,6 @@ static int netvsc_probe(struct hv_device *dev,
else
net->max_mtu = ETH_DATA_LEN;
- rtnl_lock();
ret = register_netdevice(net);
if (ret != 0) {
pr_err("Unable to register netdev.\n");
--
2.7.4
^ permalink raw reply related
* [PATCH bpf] bpf, sockmap: fix sock hash count in alloc_sock_hash_elem
From: Daniel Borkmann @ 2018-08-22 16:24 UTC (permalink / raw)
To: alexei.starovoitov; +Cc: john.fastabend, netdev, Daniel Borkmann
When we try to allocate a new sock hash entry and the allocation
fails, then sock hash map fails to reduce the map element counter,
meaning we keep accounting this element although it was never used.
Fix it by dropping the element counter on error.
Fixes: 81110384441a ("bpf: sockmap, add hash map support")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/sockmap.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 60ceb0e..40c6ef9 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -2269,8 +2269,10 @@ static struct htab_elem *alloc_sock_hash_elem(struct bpf_htab *htab,
}
l_new = kmalloc_node(htab->elem_size, GFP_ATOMIC | __GFP_NOWARN,
htab->map.numa_node);
- if (!l_new)
+ if (!l_new) {
+ atomic_dec(&htab->count);
return ERR_PTR(-ENOMEM);
+ }
memcpy(l_new->key, key, key_size);
l_new->sk = sk;
--
2.9.5
^ permalink raw reply related
* Re: [PATCH] r8169: don't use MSI-X on RTL8106e
From: Heiner Kallweit @ 2018-08-22 19:49 UTC (permalink / raw)
To: Thomas Gleixner
Cc: David Miller, helgaas, jian-hong, nic_swsd, netdev, linux-kernel,
linux, linux-pci, marc.zyngier, hch
In-Reply-To: <alpine.DEB.2.21.1808221222260.2514@nanos.tec.linutronix.de>
On 22.08.2018 13:44, Thomas Gleixner wrote:
> On Tue, 21 Aug 2018, Heiner Kallweit wrote:
>> On 21.08.2018 21:31, David Miller wrote:
>>> From: Heiner Kallweit <hkallweit1@gmail.com>
>>> Date: Mon, 20 Aug 2018 22:46:48 +0200
>>>
>>>> I'm in contact with Realtek and according to them few chip versions
>>>> seem to clear MSI-X table entries on resume from suspend. Checking
>>>> with them how this could be fixed / worked around.
>>>> Worst case we may have to disable MSI-X in general.
>>>
>>> I worry that if the chip does this, and somehow MSI-X is enabled and
>>> an interrupt is generated, the chip will write to the cleared out
>>> MSI-X address. This will either write garbage into memory or cause
>>> a bus error and require PCI error recovery.
>>>
>>> It also looks like your test patch doesn't fix things for people who
>>> have tested it.
>>>
>> The test patch was based on the first info from Realtek which made me
>> think that the base address of the MSI-X table is cleared, what
>> obviously is not the case.
>>
>> After some further tests it seems that the solution isn't as simple
>> as storing the MSI-X table entries on suspend and restore them on
>> resume. On my system (where MSI-X works fine) MSI-X table entries
>> on resume are partially different from the ones on suspend.
>
> Which is not a surprise. Please don't try to fiddle with that at the driver
> level. The irq and PCI core code are the ones in charge and if you'd
> restore at the wrong point then hell breaks lose.
>
Instead of spending a lot of effort on a workaround which may not be
acceptable, it may be better to fall back to MSI on all affected chip
versions. For two chip versions which were reported to have this issues
we're doing this already. I asked Realtek whether they have an overview
which chip versions are affected, let's see ..
The Realtek chips provide an alternative, register-based way to access
the MSI-X table, and their Windows driver seems to use it. See here:
https://patchwork.kernel.org/patch/4149171/
But as we handle all MSI-X basics in the PCI core, this isn't an option.
> Can you please do the following:
>
> 1) Store the PCI config space at suspend time
> 2) Compare the PCI config space at resume time and print the difference
>
> Do that on a working and a non-working version of Realtek NICs.
>
> Thanks,
>
> tglx
>
>
>
^ permalink raw reply
* Re: ixgbe hangs when XDP_TX is enabled
From: Jeff Kirsher @ 2018-08-22 16:22 UTC (permalink / raw)
To: Alexander Duyck, tehnerd; +Cc: Netdev, tytus.a.wasilewski, Tymoteusz Kielan
In-Reply-To: <CAKgT0Ud7qMrH+tZJTato9bP+RFq1ejuuyRPr4rNJJ0HsmkYz5w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 9717 bytes --]
On Tue, 2018-08-21 at 11:13 -0700, Alexander Duyck wrote:
> On Tue, Aug 21, 2018 at 9:59 AM Nikita V. Shirokov <
> tehnerd@tehnerd.com> wrote:
> >
> > On Tue, Aug 21, 2018 at 08:58:15AM -0700, Alexander Duyck wrote:
> > > On Mon, Aug 20, 2018 at 12:32 PM Nikita V. Shirokov <
> > > tehnerd@tehnerd.com> wrote:
> > > >
> > > > we are getting such errors:
> > > >
> > > > [ 408.737313] ixgbe 0000:03:00.0 eth0: Detected Tx Unit Hang
> > > > (XDP)
> > > > Tx Queue <46>
> > > > TDH, TDT <0>, <2>
> > > > next_to_use <2>
> > > > next_to_clean <0>
> > > > tx_buffer_info[next_to_clean]
> > > > time_stamp <0>
> > > > jiffies <1000197c0>
> > > > [ 408.804438] ixgbe 0000:03:00.0 eth0: tx hang 1 detected on
> > > > queue 46, resetting adapter
> > > > [ 408.804440] ixgbe 0000:03:00.0 eth0: initiating reset due to
> > > > tx timeout
> > > > [ 408.817679] ixgbe 0000:03:00.0 eth0: Reset adapter
> > > > [ 408.866091] ixgbe 0000:03:00.0 eth0: TXDCTL.ENABLE for one
> > > > or more queues not cleared within the polling period
> > > > [ 409.345289] ixgbe 0000:03:00.0 eth0: detected SFP+: 3
> > > > [ 409.497232] ixgbe 0000:03:00.0 eth0: NIC Link is Up 10 Gbps,
> > > > Flow Control: RX/TX
> > > >
> > > > while running XDP prog on ixgbe nic.
> > > > right now i'm seing this on bpfnext kernel
> > > > (latest commit from Wed Aug 15 15:04:25 2018 -0700 ;
> > > > 9a76aba02a37718242d7cdc294f0a3901928aa57)
> > > >
> > > > looks like this is the same issue as reported by Brenden in
> > > > https://www.spinics.net/lists/netdev/msg439438.html
> > > >
> > > > --
> > > > Nikita V. Shirokov
> > >
> > > Could you provide some additional information about your setup.
> > > Specifically useful would be "ethtool -i", "ethtool -l", and
> > > lspci
> > > -vvv info for your device. The total number of CPUs on the system
> > > would be useful to know as well. In addition could you try
> > > reproducing
> >
> > sure:
> >
> > ethtool -l eth0
> > Channel parameters for eth0:
> > Pre-set maximums:
> > RX: 0
> > TX: 0
> > Other: 1
> > Combined: 63
> > Current hardware settings:
> > RX: 0
> > TX: 0
> > Other: 1
> > Combined: 48
> >
> > # ethtool -i eth0
> > driver: ixgbe
> > version: 5.1.0-k
> > firmware-version: 0x800006f1
> > expansion-rom-version:
> > bus-info: 0000:03:00.0
> > supports-statistics: yes
> > supports-test: yes
> > supports-eeprom-access: yes
> > supports-register-dump: yes
> > supports-priv-flags: yes
> >
> >
> > # nproc
> > 48
> >
> > lspci:
> >
> > 03:00.0 Ethernet controller: Intel Corporation 82599ES 10-Gigabit
> > SFI/SFP+ Network Connection (rev 01)
> > Subsystem: Intel Corporation Device 000d
> > Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV-
> > VGASnoop- ParErr+ Stepping- SERR+ FastB2B- DisINTx+
> > Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast
> > >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> > Latency: 0, Cache Line Size: 32 bytes
> > Interrupt: pin A routed to IRQ 30
> > NUMA node: 0
> > Region 0: Memory at c7d00000 (64-bit, non-prefetchable)
> > [size=1M]
> > Region 2: I/O ports at 6000 [size=32]
> > Region 4: Memory at c7e80000 (64-bit, non-prefetchable)
> > [size=16K]
> > Expansion ROM at c7e00000 [disabled] [size=512K]
> > Capabilities: [40] Power Management version 3
> > Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA
> > PME(D0+,D1-,D2-,D3hot+,D3cold+)
> > Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=1
> > PME-
> > Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
> > Address: 0000000000000000 Data: 0000
> > Masking: 00000000 Pending: 00000000
> > Capabilities: [70] MSI-X: Enable+ Count=64 Masked-
> > Vector table: BAR=4 offset=00000000
> > PBA: BAR=4 offset=00002000
> > Capabilities: [a0] Express (v2) Endpoint, MSI 00
> > DevCap: MaxPayload 512 bytes, PhantFunc 0, Latency
> > L0s <512ns, L1 <64us
> > ExtTag- AttnBtn- AttnInd- PwrInd- RBE+
> > FLReset+ SlotPowerLimit 0.000W
> > DevCtl: Report errors: Correctable+ Non-Fatal+
> > Fatal+ Unsupported+
> > RlxdOrd- ExtTag- PhantFunc- AuxPwr-
> > NoSnoop+ FLReset-
> > MaxPayload 256 bytes, MaxReadReq 512 bytes
> > DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq+
> > AuxPwr+ TransPend+
> > LnkCap: Port #2, Speed 5GT/s, Width x8, ASPM L0s,
> > Exit Latency L0s unlimited, L1 <8us
> > ClockPM- Surprise- LLActRep- BwNot-
> > ASPMOptComp-
> > LnkCtl: ASPM Disabled; RCB 64 bytes Disabled-
> > CommClk+
> > ExtSynch- ClockPM- AutWidDis- BWInt-
> > AutBWInt-
> > LnkSta: Speed 5GT/s, Width x8, TrErr- Train-
> > SlotClk+ DLActive- BWMgmt- ABWMgmt-
> > DevCap2: Completion Timeout: Range ABCD,
> > TimeoutDis+, LTR-, OBFF Not Supported
> > DevCtl2: Completion Timeout: 50us to 50ms,
> > TimeoutDis-, LTR-, OBFF Disabled
> > LnkCtl2: Target Link Speed: 5GT/s,
> > EnterCompliance- SpeedDis-
> > Transmit Margin: Normal Operating Range,
> > EnterModifiedCompliance- ComplianceSOS-
> > Compliance De-emphasis: -6dB
> > LnkSta2: Current De-emphasis Level: -6dB,
> > EqualizationComplete-, EqualizationPhase1-
> > EqualizationPhase2-, EqualizationPhase3-,
> > LinkEqualizationRequest-
> > Capabilities: [100 v1] Advanced Error Reporting
> > UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt-
> > UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
> > UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt-
> > UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
> > UESvrt: DLP+ SDES- TLP- FCP+ CmpltTO- CmpltAbrt-
> > UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
> > CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout-
> > NonFatalErr+
> > CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout-
> > NonFatalErr+
> > AERCap: First Error Pointer: 00, GenCap+ CGenEn-
> > ChkCap+ ChkEn-
> > Capabilities: [140 v1] Device Serial Number 90-e2-ba-ff-
> > ff-b6-b2-60
> > Capabilities: [150 v1] Alternative Routing-ID
> > Interpretation (ARI)
> > ARICap: MFVC- ACS-, Next Function: 0
> > ARICtl: MFVC- ACS-, Function Group: 0
> > Capabilities: [160 v1] Single Root I/O Virtualization (SR-
> > IOV)
> > IOVCap: Migration-, Interrupt Message Number: 000
> > IOVCtl: Enable- Migration- Interrupt- MSE-
> > ARIHierarchy+
> > IOVSta: Migration-
> > Initial VFs: 64, Total VFs: 64, Number of VFs: 0,
> > Function Dependency Link: 00
> > VF offset: 128, stride: 2, Device ID: 10ed
> > Supported Page Size: 00000553, System Page Size:
> > 00000001
> > Region 0: Memory at 00000000c7c00000 (64-bit,
> > prefetchable)
> > Region 3: Memory at 00000000c7b00000 (64-bit,
> > prefetchable)
> > VF Migration: offset: 00000000, BIR: 0
> > Kernel driver in use: ixgbe
> >
> >
> >
> >
> > workaround for now is to do the same, as Brenden did in his
> > original
> > finding: make sure that combined + xdp queues < max_tx_queues
> > (e.g. w/ combined == 14 the issue goes away).
> >
> > > the issue with one of the sample XDP programs provided with the
> > > kernel
> > > such as the xdp2 which I believe uses the XDP_TX function. We
> > > need to
> > > try and create a similar setup in our own environment for
> > > reproduction and debugging.
> >
> > will try but this could take a while, because i'm not sure that we
> > have
> > ixgbe in our test lab (and it would be hard to run such test in
> > prod)
> >
> > >
> > > Thanks.
> > >
> > > - Alex
> >
> > --
> > Nikita V. Shirokov
>
> So I have been reading the datasheet
> (
> https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/82599-10-gbe-controller-datasheet.pdf
> )
> and it looks like the assumption that Brenden came to in the earlier
> referenced link is probably correct. From what I can tell there is a
> limit of 64 queues in the base RSS mode of the device, so while it
> supports more than 64 queues you can only make use of 64 as per table
> 7-25.
>
> For now I think the workaround you are using is probably the only
> viable solution. I myself don't have time to work on resolving this,
> but I am sure on of the maintainers for ixgbe will be responding
> shortly.
I have notified the 10GbE maintainers, and we are working to reproduce
the issue currently.
>
> One possible solution we may want to look at would be to make use of
> the 32 pool/VF mode in the MTQC register. That should enable us to
> make use of all 128 queues but I am sure there would be other side
> effects such as having to set the bits in the PFVFTE register in
> order
> to enable the extra Tx queues.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 05/11] can: rcar: use SPDX identifier for Renesas drivers
From: Wolfram Sang @ 2018-08-22 19:36 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Philippe Ombredanne, Fabio Estevam, Wolfram Sang, Linux-Renesas,
Kuninori Morimoto, Wolfgang Grandegger, Marc Kleine-Budde,
David S. Miller, linux-can, netdev, linux-kernel, Thomas Gleixner
In-Reply-To: <20180822193133.GA1613@kroah.com>
[-- Attachment #1: Type: text/plain, Size: 631 bytes --]
> > IMHO we should always treat and use the
> > Documentation/process/license-rules.rst as the reference and not SPDX
> > proper who moves at its own pace and evolves its specs and license ids
> > independently of where we stand in the kernel.
> > If this is not right Doc patches are welcomed!
> > In this is very specific case this has been discussed on list a few
> > times. If I recall correctly Thomas also had an opinion on this...
> > So you are correct and this should be for now:
> > // SPDX-License-Identifier: GPL-2.0+
>
> That is correct, stick with that format/version for now please.
OK, will fix.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* ozlabs.org down?
From: Jeff Kirsher @ 2018-08-22 16:12 UTC (permalink / raw)
To: Jeremy Kerr; +Cc: patchwork, netdev
It appears the entire site is down, including patchworks. Is this
expected? And for how long?
--
Cheers,
Jeff
^ permalink raw reply
* Re: [PATCH 05/11] can: rcar: use SPDX identifier for Renesas drivers
From: Greg Kroah-Hartman @ 2018-08-22 19:31 UTC (permalink / raw)
To: Philippe Ombredanne
Cc: Fabio Estevam, Wolfram Sang, Wolfram Sang, Linux-Renesas,
Kuninori Morimoto, Wolfgang Grandegger, Marc Kleine-Budde,
David S. Miller, linux-can, netdev, linux-kernel, Thomas Gleixner
In-Reply-To: <CAOFm3uF9_rbELx0aYLDXpDnZfi2Z11V1Z4Ktw+DDJT62o2S1kQ@mail.gmail.com>
On Wed, Aug 22, 2018 at 08:16:45PM +0200, Philippe Ombredanne wrote:
> Hi Fabio,
> On Wed, Aug 22, 2018 at 2:55 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > On Wed, Aug 22, 2018 at 3:30 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >>
> >>> > +// SPDX-License-Identifier: GPL-2.0-or-later
> >>>
> >>> According to Documentation/process/license-rules.rst the format should
> >>> be like this instead:
> >>>
> >>> // SPDX-License-Identifier: GPL-2.0+
> >>
> >> According to https://spdx.org/licenses/ it should be what I did above.
> >
> > Previous advice I saw was to follow the format described in
> > Documentation/process/license-rules.rst
> >
> > Greg/Philippe,
> >
> > Any inputs on this matter?
> >
> > Thanks
>
> IMHO we should always treat and use the
> Documentation/process/license-rules.rst as the reference and not SPDX
> proper who moves at its own pace and evolves its specs and license ids
> independently of where we stand in the kernel.
> If this is not right Doc patches are welcomed!
> In this is very specific case this has been discussed on list a few
> times. If I recall correctly Thomas also had an opinion on this...
> So you are correct and this should be for now:
> // SPDX-License-Identifier: GPL-2.0+
That is correct, stick with that format/version for now please.
thanks,
greg k-h
^ permalink raw reply
* [bpf PATCH 1/2] tls: possible hang when do_tcp_sendpages hits sndbuf is full case
From: John Fastabend @ 2018-08-22 15:37 UTC (permalink / raw)
To: ast, daniel, davejwatson; +Cc: netdev, john.fastabend, davem
In-Reply-To: <20180822153314.27968.72499.stgit@john-Precision-Tower-5810>
Currently, the lower protocols sk_write_space handler is not called if
TLS is sending a scatterlist via tls_push_sg. However, normally
tls_push_sg calls do_tcp_sendpage, which may be under memory pressure,
that in turn may trigger a wait via sk_wait_event. Typically, this
happens when the in-flight bytes exceed the sdnbuf size. In the normal
case when enough ACKs are received sk_write_space() will be called and
the sk_wait_event will be woken up allowing it to send more data
and/or return to the user.
But, in the TLS case because the sk_write_space() handler does not
wake up the events the above send will wait until the sndtimeo is
exceeded. By default this is MAX_SCHEDULE_TIMEOUT so it look like a
hang to the user (especially this impatient user). To fix this pass
the sk_write_space event to the lower layers sk_write_space event
which in the TCP case will wake any pending events.
I observed the above while integrating sockmap and ktls. It
initially appeared as test_sockmap (modified to use ktls) occasionally
hanging. To reliably reproduce this reduce the sndbuf size and stress
the tls layer by sending many 1B sends. This results in every byte
needing a header and each byte individually being sent to the crypto
layer.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
net/tls/tls_main.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 93c0c22..180b664 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -213,9 +213,14 @@ static void tls_write_space(struct sock *sk)
{
struct tls_context *ctx = tls_get_ctx(sk);
- /* We are already sending pages, ignore notification */
- if (ctx->in_tcp_sendpages)
+ /* If in_tcp_sendpages call lower protocol write space handler
+ * to ensure we wake up any waiting operations there. For example
+ * if do_tcp_sendpages where to call sk_wait_event.
+ */
+ if (ctx->in_tcp_sendpages) {
+ ctx->sk_write_space(sk);
return;
+ }
if (!sk->sk_write_pending && tls_is_pending_closed_record(ctx)) {
gfp_t sk_allocation = sk->sk_allocation;
^ permalink raw reply related
* [bpf PATCH 0/2] tls, sockmap, fixes for sk_wait_event
From: John Fastabend @ 2018-08-22 15:37 UTC (permalink / raw)
To: ast, daniel, davejwatson; +Cc: netdev, john.fastabend, davem
I have been testing ktls and sockmap lately and noticed that neither
was handling sk_write_space events correctly. We need to ensure
these events are pushed down to the lower layer in all cases to
handle the case where the lower layer sendpage call has called
sk_wait_event and needs to be woken up. Without this I see
occosional stalls of sndtimeo length while we wait for the
timeout value even though space is available.
Two fixes below. Thanks.
---
John Fastabend (2):
tls: possible hang when do_tcp_sendpages hits sndbuf is full case
bpf: sockmap: write_space events need to be passed to TCP handler
kernel/bpf/sockmap.c | 3 +++
net/tls/tls_main.c | 9 +++++++--
2 files changed, 10 insertions(+), 2 deletions(-)
^ permalink raw reply
* [bpf PATCH 2/2] bpf: sockmap: write_space events need to be passed to TCP handler
From: John Fastabend @ 2018-08-22 15:37 UTC (permalink / raw)
To: ast, daniel, davejwatson; +Cc: netdev, john.fastabend, davem
In-Reply-To: <20180822153314.27968.72499.stgit@john-Precision-Tower-5810>
When sockmap code is using the stream parser it also handles the write
space events in order to handle the case where (a) verdict redirects
skb to another socket and (b) the sockmap then sends the skb but due
to memory constraints (or other EAGAIN errors) needs to do a retry.
But the initial code missed a third case where the
skb_send_sock_locked() triggers an sk_wait_event(). A typically case
would be when sndbuf size is exceeded. If this happens because we
do not pass the write_space event to the lower layers we never wake
up the event and it will wait for sndtimeo. Which as noted in ktls
fix may be rather large and look like a hang to the user.
To reproduce the best test is to reduce the sndbuf size and send
1B data chunks to stress the memory handling.
To fix this pass the event from the upper layer to the lower layer.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/sockmap.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 98e621a..1d092f3 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -1427,12 +1427,15 @@ static void smap_tx_work(struct work_struct *w)
static void smap_write_space(struct sock *sk)
{
struct smap_psock *psock;
+ void (*write_space)(struct sock *sk);
rcu_read_lock();
psock = smap_psock_sk(sk);
if (likely(psock && test_bit(SMAP_TX_RUNNING, &psock->state)))
schedule_work(&psock->tx_work);
+ write_space = psock->save_write_space;
rcu_read_unlock();
+ write_space(sk);
}
static void smap_stop_sock(struct smap_psock *psock, struct sock *sk)
^ permalink raw reply related
* Re: [PATCH] net: macb: do not disable MDIO bus at open/close time
From: Claudiu Beznea @ 2018-08-22 15:33 UTC (permalink / raw)
To: Anssi Hannula, netdev, Nicolas Ferre; +Cc: David S. Miller, Andrew Lunn
In-Reply-To: <20180820145530.7657-1-anssi.hannula@bitwise.fi>
On 20.08.2018 17:55, Anssi Hannula wrote:
> macb_reset_hw() is called from macb_close() and indirectly from
> macb_open(). macb_reset_hw() zeroes the NCR register, including the MPE
> (Management Port Enable) bit.
>
> This will prevent accessing any other PHYs for other Ethernet MACs on
> the MDIO bus, which remains registered at macb_reset_hw() time, until
> macb_init_hw() is called from macb_open() which sets the MPE bit again.
>
> I.e. currently the MDIO bus has a short disruption at open time and is
> disabled at close time until the interface is opened again.
>
> Fix that by only touching the RE and TE bits when enabling and disabling
> RX/TX.
>
> Fixes: 6c36a7074436 ("macb: Use generic PHY layer")
> Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
> ---
>
> Claudiu Beznea wrote:
>> On 10.08.2018 09:22, Anssi Hannula wrote:
>>>
>>> macb_reset_hw() is called in init path too,
>>
>> I only see it in macb_close() and macb_open() called from macb_init_hw().
>
> Yeah, macb_init_hw() is what I meant :)
>
>
> drivers/net/ethernet/cadence/macb_main.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index dc09f9a8a49b..6501e9b3785a 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2028,14 +2028,17 @@ static void macb_reset_hw(struct macb *bp)
> {
> struct macb_queue *queue;
> unsigned int q;
> + u32 ctrl = macb_readl(bp, NCR);
>
> /* Disable RX and TX (XXX: Should we halt the transmission
> * more gracefully?)
> */
> - macb_writel(bp, NCR, 0);
> + ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
>
> /* Clear the stats registers (XXX: Update stats first?) */
> - macb_writel(bp, NCR, MACB_BIT(CLRSTAT));
> + ctrl |= MACB_BIT(CLRSTAT);
> +
> + macb_writel(bp, NCR, ctrl);
>
> /* Clear all status flags */
> macb_writel(bp, TSR, -1);
> @@ -2170,6 +2173,7 @@ static void macb_init_hw(struct macb *bp)
> unsigned int q;
>
> u32 config;
> + u32 ctrl;
>
> macb_reset_hw(bp);
> macb_set_hwaddr(bp);
> @@ -2223,7 +2227,9 @@ static void macb_init_hw(struct macb *bp)
> }
>
> /* Enable TX and RX */
> - macb_writel(bp, NCR, MACB_BIT(RE) | MACB_BIT(TE) | MACB_BIT(MPE));
> + ctrl = macb_readl(bp, NCR);
> + ctrl |= MACB_BIT(RE) | MACB_BIT(TE);
> + macb_writel(bp, NCR, ctrl);
I would keep it as:
macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(RE) | MACB_BIT(TE));
> }
>
> /* The hash address register is 64 bits long and takes up two
>
^ permalink raw reply
* Re: [PATCH] strparser: remove any offset before parsing messages
From: Dave Watson @ 2018-08-22 18:38 UTC (permalink / raw)
To: Dominique Martinet
Cc: Doron Roberts-Kedes, Tom Herbert, David S. Miller, netdev,
linux-kernel
In-Reply-To: <20180822054707.GA13455@nautica>
On 08/22/18 07:47 AM, Dominique Martinet wrote:
> > > As I wrote above, I think it should not be possible, so we're not
> > > even talking about a small percentage here.
> > > The reason I didn't use skb_pull (the head-only variant) is that I'd
> > > rather have the overhead than a BUG() if I'm wrong on this...
> >
> > A printk in that section when (orig_offset + eaten > skb_headlen(head))
> > confirms that this case is not uncommon or impossible. Would have to do
> > more work to see how many hundreds of times per second, but it is not a
> > philosophical concern.
>
> Hmm, right, it does happen if I force two bigger packets in a single
> write() on my reproducer; I guess my workload didn't exhibit that
> behaviour with a 9p client...
>
> I've tried measuring that overhead as well by writing a more complex bpf
> program that would fetch the offset in the skb but for some reason I'm
> reading a 0 offset when it's not zero... well, not like there's much
> choice for this at this point anyway; I don't think we'll do this
> without pull, I'll put that on background.
For what it is worth we checked the offset in bpf, something
along the lines of
struct kcm_rx_msg { int full_len; int offset;};
static inline struct kcm_rx_msg *kcm_rx_msg(struct __sk_buff *skb)
{ return (struct kcm_rx_msg *)skb->cb;}
int decode_framing(struct __sk_buff *skb)
{ return load_word(skb, kcm_rx_msg(skb)->offset);}
Although it did puzzle me for a while figuring that out when I ran in
to it.
^ 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