* [PATCHv2 bpf-next 05/11] bpf: Macrofy stack state copy
From: Joe Stringer @ 2018-09-21 17:10 UTC (permalink / raw)
To: ast
Cc: netdev, daniel, john.fastabend, tgraf, kafai, nitin.hande,
mauricio.vasquez
In-Reply-To: <20180921171043.20823-1-joe@wand.net.nz>
An upcoming commit will need very similar copy/realloc boilerplate, so
refactor the existing stack copy/realloc functions into macros to
simplify it.
Signed-off-by: Joe Stringer <joe@wand.net.nz>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
kernel/bpf/verifier.c | 106 ++++++++++++++++++++++++------------------
1 file changed, 60 insertions(+), 46 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1fee63d82290..311340360aa3 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -388,60 +388,74 @@ static void print_verifier_state(struct bpf_verifier_env *env,
verbose(env, "\n");
}
-static int copy_stack_state(struct bpf_func_state *dst,
- const struct bpf_func_state *src)
-{
- if (!src->stack)
- return 0;
- if (WARN_ON_ONCE(dst->allocated_stack < src->allocated_stack)) {
- /* internal bug, make state invalid to reject the program */
- memset(dst, 0, sizeof(*dst));
- return -EFAULT;
- }
- memcpy(dst->stack, src->stack,
- sizeof(*src->stack) * (src->allocated_stack / BPF_REG_SIZE));
- return 0;
-}
+#define COPY_STATE_FN(NAME, COUNT, FIELD, SIZE) \
+static int copy_##NAME##_state(struct bpf_func_state *dst, \
+ const struct bpf_func_state *src) \
+{ \
+ if (!src->FIELD) \
+ return 0; \
+ if (WARN_ON_ONCE(dst->COUNT < src->COUNT)) { \
+ /* internal bug, make state invalid to reject the program */ \
+ memset(dst, 0, sizeof(*dst)); \
+ return -EFAULT; \
+ } \
+ memcpy(dst->FIELD, src->FIELD, \
+ sizeof(*src->FIELD) * (src->COUNT / SIZE)); \
+ return 0; \
+}
+/* copy_stack_state() */
+COPY_STATE_FN(stack, allocated_stack, stack, BPF_REG_SIZE)
+#undef COPY_STATE_FN
+
+#define REALLOC_STATE_FN(NAME, COUNT, FIELD, SIZE) \
+static int realloc_##NAME##_state(struct bpf_func_state *state, int size, \
+ bool copy_old) \
+{ \
+ u32 old_size = state->COUNT; \
+ struct bpf_##NAME##_state *new_##FIELD; \
+ int slot = size / SIZE; \
+ \
+ if (size <= old_size || !size) { \
+ if (copy_old) \
+ return 0; \
+ state->COUNT = slot * SIZE; \
+ if (!size && old_size) { \
+ kfree(state->FIELD); \
+ state->FIELD = NULL; \
+ } \
+ return 0; \
+ } \
+ new_##FIELD = kmalloc_array(slot, sizeof(struct bpf_##NAME##_state), \
+ GFP_KERNEL); \
+ if (!new_##FIELD) \
+ return -ENOMEM; \
+ if (copy_old) { \
+ if (state->FIELD) \
+ memcpy(new_##FIELD, state->FIELD, \
+ sizeof(*new_##FIELD) * (old_size / SIZE)); \
+ memset(new_##FIELD + old_size / SIZE, 0, \
+ sizeof(*new_##FIELD) * (size - old_size) / SIZE); \
+ } \
+ state->COUNT = slot * SIZE; \
+ kfree(state->FIELD); \
+ state->FIELD = new_##FIELD; \
+ return 0; \
+}
+/* realloc_stack_state() */
+REALLOC_STATE_FN(stack, allocated_stack, stack, BPF_REG_SIZE)
+#undef REALLOC_STATE_FN
/* 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 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
+ * Note there is a non-zero 'parent' pointer inside bpf_verifier_state
+ * which realloc_stack_state() copies over. It points to previous
+ * bpf_verifier_state which is never reallocated.
*/
static int realloc_func_state(struct bpf_func_state *state, int size,
bool copy_old)
{
- u32 old_size = state->allocated_stack;
- struct bpf_stack_state *new_stack;
- int slot = size / BPF_REG_SIZE;
-
- if (size <= old_size || !size) {
- if (copy_old)
- return 0;
- state->allocated_stack = slot * BPF_REG_SIZE;
- if (!size && old_size) {
- kfree(state->stack);
- state->stack = NULL;
- }
- return 0;
- }
- new_stack = kmalloc_array(slot, sizeof(struct bpf_stack_state),
- GFP_KERNEL);
- if (!new_stack)
- return -ENOMEM;
- if (copy_old) {
- if (state->stack)
- memcpy(new_stack, state->stack,
- sizeof(*new_stack) * (old_size / BPF_REG_SIZE));
- memset(new_stack + old_size / BPF_REG_SIZE, 0,
- sizeof(*new_stack) * (size - old_size) / BPF_REG_SIZE);
- }
- state->allocated_stack = slot * BPF_REG_SIZE;
- kfree(state->stack);
- state->stack = new_stack;
- return 0;
+ return realloc_stack_state(state, size, copy_old);
}
static void free_func_state(struct bpf_func_state *state)
--
2.17.1
^ permalink raw reply related
* [PATCHv2 bpf-next 04/11] bpf: Add PTR_TO_SOCKET verifier type
From: Joe Stringer @ 2018-09-21 17:10 UTC (permalink / raw)
To: ast
Cc: netdev, daniel, john.fastabend, tgraf, kafai, nitin.hande,
mauricio.vasquez
In-Reply-To: <20180921171043.20823-1-joe@wand.net.nz>
Teach the verifier a little bit about a new type of pointer, a
PTR_TO_SOCKET. This pointer type is accessed from BPF through the
'struct bpf_sock' structure.
Signed-off-by: Joe Stringer <joe@wand.net.nz>
---
v2: Reuse reg_type_mismatch() in more places
Reduce the number of passes at convert_ctx_access()
---
include/linux/bpf.h | 17 +++++
include/linux/bpf_verifier.h | 2 +
kernel/bpf/verifier.c | 120 +++++++++++++++++++++++++++++++----
net/core/filter.c | 30 +++++----
4 files changed, 143 insertions(+), 26 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 988a00797bcd..daeb0d343d9c 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -154,6 +154,7 @@ enum bpf_arg_type {
ARG_PTR_TO_CTX, /* pointer to context */
ARG_ANYTHING, /* any (initialized) argument is ok */
+ ARG_PTR_TO_SOCKET, /* pointer to bpf_sock */
};
/* type of values returned from helper functions */
@@ -162,6 +163,7 @@ enum bpf_return_type {
RET_VOID, /* function doesn't return anything */
RET_PTR_TO_MAP_VALUE, /* returns a pointer to map elem value */
RET_PTR_TO_MAP_VALUE_OR_NULL, /* returns a pointer to map elem value or NULL */
+ RET_PTR_TO_SOCKET_OR_NULL, /* returns a pointer to a socket or NULL */
};
/* eBPF function prototype used by verifier to allow BPF_CALLs from eBPF programs
@@ -213,6 +215,8 @@ enum bpf_reg_type {
PTR_TO_PACKET, /* reg points to skb->data */
PTR_TO_PACKET_END, /* skb->data + headlen */
PTR_TO_FLOW_KEYS, /* reg points to bpf_flow_keys */
+ PTR_TO_SOCKET, /* reg points to struct bpf_sock */
+ PTR_TO_SOCKET_OR_NULL, /* reg points to struct bpf_sock or NULL */
};
/* The information passed from prog-specific *_is_valid_access
@@ -335,6 +339,11 @@ const struct bpf_func_proto *bpf_get_trace_printk_proto(void);
typedef unsigned long (*bpf_ctx_copy_t)(void *dst, const void *src,
unsigned long off, unsigned long len);
+typedef u32 (*bpf_convert_ctx_access_t)(enum bpf_access_type type,
+ const struct bpf_insn *src,
+ struct bpf_insn *dst,
+ struct bpf_prog *prog,
+ u32 *target_size);
u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
void *ctx, u64 ctx_size, bpf_ctx_copy_t ctx_copy);
@@ -828,4 +837,12 @@ extern const struct bpf_func_proto bpf_get_local_storage_proto;
void bpf_user_rnd_init_once(void);
u64 bpf_user_rnd_u32(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
+bool bpf_sock_is_valid_access(int off, int size, enum bpf_access_type type,
+ struct bpf_insn_access_aux *info);
+u32 bpf_sock_convert_ctx_access(enum bpf_access_type type,
+ const struct bpf_insn *si,
+ struct bpf_insn *insn_buf,
+ struct bpf_prog *prog,
+ u32 *target_size);
+
#endif /* _LINUX_BPF_H */
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index af262b97f586..23a2b17bfd75 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -58,6 +58,8 @@ struct bpf_reg_state {
* offset, so they can share range knowledge.
* For PTR_TO_MAP_VALUE_OR_NULL this is used to share which map value we
* came from, when one is tested for != NULL.
+ * For PTR_TO_SOCKET this is used to share which pointers retain the
+ * same reference to the socket, to determine proper reference freeing.
*/
u32 id;
/* For scalar types (SCALAR_VALUE), this represents our knowledge of
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 7dccb18ede03..1fee63d82290 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -80,8 +80,8 @@ static const struct bpf_verifier_ops * const bpf_verifier_ops[] = {
* (like pointer plus pointer becomes SCALAR_VALUE type)
*
* When verifier sees load or store instructions the type of base register
- * can be: PTR_TO_MAP_VALUE, PTR_TO_CTX, PTR_TO_STACK. These are three pointer
- * types recognized by check_mem_access() function.
+ * can be: PTR_TO_MAP_VALUE, PTR_TO_CTX, PTR_TO_STACK, PTR_TO_SOCKET. These are
+ * four pointer types recognized by check_mem_access() function.
*
* PTR_TO_MAP_VALUE means that this register is pointing to 'map element value'
* and the range of [ptr, ptr + map's value_size) is accessible.
@@ -267,6 +267,8 @@ static const char * const reg_type_str[] = {
[PTR_TO_PACKET_META] = "pkt_meta",
[PTR_TO_PACKET_END] = "pkt_end",
[PTR_TO_FLOW_KEYS] = "flow_keys",
+ [PTR_TO_SOCKET] = "sock",
+ [PTR_TO_SOCKET_OR_NULL] = "sock_or_null",
};
static char slot_type_char[] = {
@@ -973,6 +975,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type)
case PTR_TO_PACKET_END:
case PTR_TO_FLOW_KEYS:
case CONST_PTR_TO_MAP:
+ case PTR_TO_SOCKET:
+ case PTR_TO_SOCKET_OR_NULL:
return true;
default:
return false;
@@ -1341,6 +1345,28 @@ static int check_flow_keys_access(struct bpf_verifier_env *env, int off,
return 0;
}
+static int check_sock_access(struct bpf_verifier_env *env, u32 regno, int off,
+ int size, enum bpf_access_type t)
+{
+ struct bpf_reg_state *regs = cur_regs(env);
+ struct bpf_reg_state *reg = ®s[regno];
+ struct bpf_insn_access_aux info;
+
+ if (reg->smin_value < 0) {
+ verbose(env, "R%d min value is negative, either use unsigned index or do a if (index >=0) check.\n",
+ regno);
+ return -EACCES;
+ }
+
+ if (!bpf_sock_is_valid_access(off, size, t, &info)) {
+ verbose(env, "invalid bpf_sock_ops access off=%d size=%d\n",
+ off, size);
+ return -EACCES;
+ }
+
+ return 0;
+}
+
static bool __is_pointer_value(bool allow_ptr_leaks,
const struct bpf_reg_state *reg)
{
@@ -1459,6 +1485,9 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
*/
strict = true;
break;
+ case PTR_TO_SOCKET:
+ pointer_desc = "sock ";
+ break;
default:
break;
}
@@ -1726,6 +1755,14 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
err = check_flow_keys_access(env, off, size);
if (!err && t == BPF_READ && value_regno >= 0)
mark_reg_unknown(env, regs, value_regno);
+ } else if (reg->type == PTR_TO_SOCKET) {
+ if (t == BPF_WRITE) {
+ verbose(env, "cannot write into socket\n");
+ return -EACCES;
+ }
+ err = check_sock_access(env, regno, off, size, t);
+ if (!err && value_regno >= 0)
+ mark_reg_unknown(env, regs, value_regno);
} else {
verbose(env, "R%d invalid mem access '%s'\n", regno,
reg_type_str[reg->type]);
@@ -1949,6 +1986,10 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
err = check_ctx_reg(env, reg, regno);
if (err < 0)
return err;
+ } else if (arg_type == ARG_PTR_TO_SOCKET) {
+ expected_type = PTR_TO_SOCKET;
+ if (type != expected_type)
+ goto err_type;
} else if (arg_type_is_mem_ptr(arg_type)) {
expected_type = PTR_TO_STACK;
/* One exception here. In case function allows for NULL to be
@@ -2542,6 +2583,10 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
}
regs[BPF_REG_0].map_ptr = meta.map_ptr;
regs[BPF_REG_0].id = ++env->id_gen;
+ } else if (fn->ret_type == RET_PTR_TO_SOCKET_OR_NULL) {
+ mark_reg_known_zero(env, regs, BPF_REG_0);
+ regs[BPF_REG_0].type = PTR_TO_SOCKET_OR_NULL;
+ regs[BPF_REG_0].id = ++env->id_gen;
} else {
verbose(env, "unknown return type %d of func %s#%d\n",
fn->ret_type, func_id_name(func_id), func_id);
@@ -2679,6 +2724,8 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
return -EACCES;
case CONST_PTR_TO_MAP:
case PTR_TO_PACKET_END:
+ case PTR_TO_SOCKET:
+ case PTR_TO_SOCKET_OR_NULL:
verbose(env, "R%d pointer arithmetic on %s prohibited\n",
dst, reg_type_str[ptr_reg->type]);
return -EACCES;
@@ -3626,6 +3673,8 @@ static void mark_ptr_or_null_reg(struct bpf_reg_state *reg, u32 id,
} else {
reg->type = PTR_TO_MAP_VALUE;
}
+ } else if (reg->type == PTR_TO_SOCKET_OR_NULL) {
+ reg->type = PTR_TO_SOCKET;
}
/* We don't need id from this point onwards anymore, thus we
* should better reset it, so that state pruning has chances
@@ -4401,6 +4450,8 @@ static bool regsafe(struct bpf_reg_state *rold, struct bpf_reg_state *rcur,
case CONST_PTR_TO_MAP:
case PTR_TO_PACKET_END:
case PTR_TO_FLOW_KEYS:
+ case PTR_TO_SOCKET:
+ case PTR_TO_SOCKET_OR_NULL:
/* Only valid matches are exact, which memcmp() above
* would have accepted
*/
@@ -4678,6 +4729,37 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
return 0;
}
+/* Return true if it's OK to have the same insn return a different type. */
+static bool reg_type_mismatch_ok(enum bpf_reg_type type)
+{
+ switch (type) {
+ case PTR_TO_CTX:
+ case PTR_TO_SOCKET:
+ case PTR_TO_SOCKET_OR_NULL:
+ return false;
+ default:
+ return true;
+ }
+}
+
+/* If an instruction was previously used with particular pointer types, then we
+ * need to be careful to avoid cases such as the below, where it may be ok
+ * for one branch accessing the pointer, but not ok for the other branch:
+ *
+ * R1 = sock_ptr
+ * goto X;
+ * ...
+ * R1 = some_other_valid_ptr;
+ * goto X;
+ * ...
+ * R2 = *(u32 *)(R1 + 0);
+ */
+static bool reg_type_mismatch(enum bpf_reg_type src, enum bpf_reg_type prev)
+{
+ return src != prev && (!reg_type_mismatch_ok(src) ||
+ !reg_type_mismatch_ok(prev));
+}
+
static int do_check(struct bpf_verifier_env *env)
{
struct bpf_verifier_state *state;
@@ -4810,9 +4892,7 @@ static int do_check(struct bpf_verifier_env *env)
*/
*prev_src_type = src_reg_type;
- } else if (src_reg_type != *prev_src_type &&
- (src_reg_type == PTR_TO_CTX ||
- *prev_src_type == PTR_TO_CTX)) {
+ } else if (reg_type_mismatch(src_reg_type, *prev_src_type)) {
/* ABuser program is trying to use the same insn
* dst_reg = *(u32*) (src_reg + off)
* with different pointer types:
@@ -4857,9 +4937,7 @@ static int do_check(struct bpf_verifier_env *env)
if (*prev_dst_type == NOT_INIT) {
*prev_dst_type = dst_reg_type;
- } else if (dst_reg_type != *prev_dst_type &&
- (dst_reg_type == PTR_TO_CTX ||
- *prev_dst_type == PTR_TO_CTX)) {
+ } else if (reg_type_mismatch(dst_reg_type, *prev_dst_type)) {
verbose(env, "same insn cannot be used with different pointers\n");
return -EINVAL;
}
@@ -5276,8 +5354,10 @@ static void sanitize_dead_code(struct bpf_verifier_env *env)
}
}
-/* convert load instructions that access fields of 'struct __sk_buff'
- * into sequence of instructions that access fields of 'struct sk_buff'
+/* convert load instructions that access fields of a context type into a
+ * sequence of instructions that access fields of the underlying structure:
+ * struct __sk_buff -> struct sk_buff
+ * struct bpf_sock_ops -> struct sock
*/
static int convert_ctx_accesses(struct bpf_verifier_env *env)
{
@@ -5306,12 +5386,14 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
}
}
- if (!ops->convert_ctx_access || bpf_prog_is_dev_bound(env->prog->aux))
+ if (bpf_prog_is_dev_bound(env->prog->aux))
return 0;
insn = env->prog->insnsi + delta;
for (i = 0; i < insn_cnt; i++, insn++) {
+ bpf_convert_ctx_access_t convert_ctx_access;
+
if (insn->code == (BPF_LDX | BPF_MEM | BPF_B) ||
insn->code == (BPF_LDX | BPF_MEM | BPF_H) ||
insn->code == (BPF_LDX | BPF_MEM | BPF_W) ||
@@ -5353,8 +5435,18 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
continue;
}
- if (env->insn_aux_data[i + delta].ptr_type != PTR_TO_CTX)
+ switch (env->insn_aux_data[i + delta].ptr_type) {
+ case PTR_TO_CTX:
+ if (!ops->convert_ctx_access)
+ continue;
+ convert_ctx_access = ops->convert_ctx_access;
+ break;
+ case PTR_TO_SOCKET:
+ convert_ctx_access = bpf_sock_convert_ctx_access;
+ break;
+ default:
continue;
+ }
ctx_field_size = env->insn_aux_data[i + delta].ctx_field_size;
size = BPF_LDST_BYTES(insn);
@@ -5386,8 +5478,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
}
target_size = 0;
- cnt = ops->convert_ctx_access(type, insn, insn_buf, env->prog,
- &target_size);
+ cnt = convert_ctx_access(type, insn, insn_buf, env->prog,
+ &target_size);
if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf) ||
(ctx_field_size && !target_size)) {
verbose(env, "bpf verifier is misconfigured\n");
diff --git a/net/core/filter.c b/net/core/filter.c
index 9cc76f134ddb..43f81883f31d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5393,23 +5393,29 @@ static bool __sock_filter_check_size(int off, int size,
return size == size_default;
}
-static bool sock_filter_is_valid_access(int off, int size,
- enum bpf_access_type type,
- const struct bpf_prog *prog,
- struct bpf_insn_access_aux *info)
+bool bpf_sock_is_valid_access(int off, int size, enum bpf_access_type type,
+ struct bpf_insn_access_aux *info)
{
if (off < 0 || off >= sizeof(struct bpf_sock))
return false;
if (off % size != 0)
return false;
- if (!__sock_filter_check_attach_type(off, type,
- prog->expected_attach_type))
- return false;
if (!__sock_filter_check_size(off, size, info))
return false;
return true;
}
+static bool sock_filter_is_valid_access(int off, int size,
+ enum bpf_access_type type,
+ const struct bpf_prog *prog,
+ struct bpf_insn_access_aux *info)
+{
+ if (!__sock_filter_check_attach_type(off, type,
+ prog->expected_attach_type))
+ return false;
+ return bpf_sock_is_valid_access(off, size, type, info);
+}
+
static int bpf_unclone_prologue(struct bpf_insn *insn_buf, bool direct_write,
const struct bpf_prog *prog, int drop_verdict)
{
@@ -6121,10 +6127,10 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type type,
return insn - insn_buf;
}
-static u32 sock_filter_convert_ctx_access(enum bpf_access_type type,
- const struct bpf_insn *si,
- struct bpf_insn *insn_buf,
- struct bpf_prog *prog, u32 *target_size)
+u32 bpf_sock_convert_ctx_access(enum bpf_access_type type,
+ const struct bpf_insn *si,
+ struct bpf_insn *insn_buf,
+ struct bpf_prog *prog, u32 *target_size)
{
struct bpf_insn *insn = insn_buf;
int off;
@@ -7036,7 +7042,7 @@ const struct bpf_prog_ops lwt_seg6local_prog_ops = {
const struct bpf_verifier_ops cg_sock_verifier_ops = {
.get_func_proto = sock_filter_func_proto,
.is_valid_access = sock_filter_is_valid_access,
- .convert_ctx_access = sock_filter_convert_ctx_access,
+ .convert_ctx_access = bpf_sock_convert_ctx_access,
};
const struct bpf_prog_ops cg_sock_prog_ops = {
--
2.17.1
^ permalink raw reply related
* [PATCHv2 bpf-next 03/11] bpf: Generalize ptr_or_null regs check
From: Joe Stringer @ 2018-09-21 17:10 UTC (permalink / raw)
To: ast
Cc: netdev, daniel, john.fastabend, tgraf, kafai, nitin.hande,
mauricio.vasquez
In-Reply-To: <20180921171043.20823-1-joe@wand.net.nz>
This check will be reused by an upcoming commit for conditional jump
checks for sockets. Refactor it a bit to simplify the later commit.
Signed-off-by: Joe Stringer <joe@wand.net.nz>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
kernel/bpf/verifier.c | 43 +++++++++++++++++++++++++------------------
1 file changed, 25 insertions(+), 18 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a889398ba43d..7dccb18ede03 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -249,6 +249,11 @@ static bool type_is_pkt_pointer(enum bpf_reg_type type)
type == PTR_TO_PACKET_META;
}
+static bool reg_type_may_be_null(enum bpf_reg_type type)
+{
+ return type == PTR_TO_MAP_VALUE_OR_NULL;
+}
+
/* string representation of 'enum bpf_reg_type' */
static const char * const reg_type_str[] = {
[NOT_INIT] = "?",
@@ -3598,12 +3603,10 @@ static void reg_combine_min_max(struct bpf_reg_state *true_src,
}
}
-static void mark_map_reg(struct bpf_reg_state *regs, u32 regno, u32 id,
- bool is_null)
+static void mark_ptr_or_null_reg(struct bpf_reg_state *reg, u32 id,
+ bool is_null)
{
- struct bpf_reg_state *reg = ®s[regno];
-
- if (reg->type == PTR_TO_MAP_VALUE_OR_NULL && reg->id == id) {
+ if (reg_type_may_be_null(reg->type) && reg->id == id) {
/* Old offset (both fixed and variable parts) should
* have been known-zero, because we don't allow pointer
* arithmetic on pointers that might be NULL.
@@ -3616,11 +3619,13 @@ static void mark_map_reg(struct bpf_reg_state *regs, u32 regno, u32 id,
}
if (is_null) {
reg->type = SCALAR_VALUE;
- } else if (reg->map_ptr->inner_map_meta) {
- reg->type = CONST_PTR_TO_MAP;
- reg->map_ptr = reg->map_ptr->inner_map_meta;
- } else {
- reg->type = PTR_TO_MAP_VALUE;
+ } else if (reg->type == PTR_TO_MAP_VALUE_OR_NULL) {
+ if (reg->map_ptr->inner_map_meta) {
+ reg->type = CONST_PTR_TO_MAP;
+ reg->map_ptr = reg->map_ptr->inner_map_meta;
+ } else {
+ reg->type = PTR_TO_MAP_VALUE;
+ }
}
/* We don't need id from this point onwards anymore, thus we
* should better reset it, so that state pruning has chances
@@ -3633,8 +3638,8 @@ static void mark_map_reg(struct bpf_reg_state *regs, u32 regno, u32 id,
/* The logic is similar to find_good_pkt_pointers(), both could eventually
* be folded together at some point.
*/
-static void mark_map_regs(struct bpf_verifier_state *vstate, u32 regno,
- bool is_null)
+static void mark_ptr_or_null_regs(struct bpf_verifier_state *vstate, u32 regno,
+ bool is_null)
{
struct bpf_func_state *state = vstate->frame[vstate->curframe];
struct bpf_reg_state *reg, *regs = state->regs;
@@ -3642,14 +3647,14 @@ static void mark_map_regs(struct bpf_verifier_state *vstate, u32 regno,
int i, j;
for (i = 0; i < MAX_BPF_REG; i++)
- mark_map_reg(regs, i, id, is_null);
+ mark_ptr_or_null_reg(®s[i], id, is_null);
for (j = 0; j <= vstate->curframe; j++) {
state = vstate->frame[j];
for_each_spilled_reg(i, state, reg) {
if (!reg)
continue;
- mark_map_reg(&state->stack[i].spilled_ptr, 0, id, is_null);
+ mark_ptr_or_null_reg(reg, id, is_null);
}
}
}
@@ -3851,12 +3856,14 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
/* detect if R == 0 where R is returned from bpf_map_lookup_elem() */
if (BPF_SRC(insn->code) == BPF_K &&
insn->imm == 0 && (opcode == BPF_JEQ || opcode == BPF_JNE) &&
- dst_reg->type == PTR_TO_MAP_VALUE_OR_NULL) {
- /* Mark all identical map registers in each branch as either
+ reg_type_may_be_null(dst_reg->type)) {
+ /* Mark all identical registers in each branch as either
* safe or unknown depending R == 0 or R != 0 conditional.
*/
- mark_map_regs(this_branch, insn->dst_reg, opcode == BPF_JNE);
- mark_map_regs(other_branch, insn->dst_reg, opcode == BPF_JEQ);
+ mark_ptr_or_null_regs(this_branch, insn->dst_reg,
+ opcode == BPF_JNE);
+ mark_ptr_or_null_regs(other_branch, insn->dst_reg,
+ opcode == BPF_JEQ);
} else if (!try_match_pkt_pointers(insn, dst_reg, ®s[insn->src_reg],
this_branch, other_branch) &&
is_pointer_value(env, insn->dst_reg)) {
--
2.17.1
^ permalink raw reply related
* [PATCHv2 bpf-next 02/11] bpf: Simplify ptr_min_max_vals adjustment
From: Joe Stringer @ 2018-09-21 17:10 UTC (permalink / raw)
To: ast
Cc: netdev, daniel, john.fastabend, tgraf, kafai, nitin.hande,
mauricio.vasquez
In-Reply-To: <20180921171043.20823-1-joe@wand.net.nz>
An upcoming commit will add another two pointer types that need very
similar behaviour, so generalise this function now.
Signed-off-by: Joe Stringer <joe@wand.net.nz>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
kernel/bpf/verifier.c | 22 ++++++++++-----------
tools/testing/selftests/bpf/test_verifier.c | 14 ++++++-------
2 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 62ce45d9c558..a889398ba43d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2667,20 +2667,18 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
return -EACCES;
}
- if (ptr_reg->type == PTR_TO_MAP_VALUE_OR_NULL) {
- verbose(env, "R%d pointer arithmetic on PTR_TO_MAP_VALUE_OR_NULL prohibited, null-check it first\n",
- dst);
- return -EACCES;
- }
- if (ptr_reg->type == CONST_PTR_TO_MAP) {
- verbose(env, "R%d pointer arithmetic on CONST_PTR_TO_MAP prohibited\n",
- dst);
+ switch (ptr_reg->type) {
+ case PTR_TO_MAP_VALUE_OR_NULL:
+ verbose(env, "R%d pointer arithmetic on %s prohibited, null-check it first\n",
+ dst, reg_type_str[ptr_reg->type]);
return -EACCES;
- }
- if (ptr_reg->type == PTR_TO_PACKET_END) {
- verbose(env, "R%d pointer arithmetic on PTR_TO_PACKET_END prohibited\n",
- dst);
+ case CONST_PTR_TO_MAP:
+ case PTR_TO_PACKET_END:
+ verbose(env, "R%d pointer arithmetic on %s prohibited\n",
+ dst, reg_type_str[ptr_reg->type]);
return -EACCES;
+ default:
+ break;
}
/* In case of 'scalar += pointer', dst_reg inherits pointer type and id.
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 67c412d19c09..ceb55a9f3da9 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -3637,7 +3637,7 @@ static struct bpf_test tests[] = {
BPF_MOV64_IMM(BPF_REG_0, 0),
BPF_EXIT_INSN(),
},
- .errstr = "R3 pointer arithmetic on PTR_TO_PACKET_END",
+ .errstr = "R3 pointer arithmetic on pkt_end",
.result = REJECT,
.prog_type = BPF_PROG_TYPE_SCHED_CLS,
},
@@ -4780,7 +4780,7 @@ static struct bpf_test tests[] = {
BPF_EXIT_INSN(),
},
.fixup_map1 = { 4 },
- .errstr = "R4 pointer arithmetic on PTR_TO_MAP_VALUE_OR_NULL",
+ .errstr = "R4 pointer arithmetic on map_value_or_null",
.result = REJECT,
.prog_type = BPF_PROG_TYPE_SCHED_CLS
},
@@ -4801,7 +4801,7 @@ static struct bpf_test tests[] = {
BPF_EXIT_INSN(),
},
.fixup_map1 = { 4 },
- .errstr = "R4 pointer arithmetic on PTR_TO_MAP_VALUE_OR_NULL",
+ .errstr = "R4 pointer arithmetic on map_value_or_null",
.result = REJECT,
.prog_type = BPF_PROG_TYPE_SCHED_CLS
},
@@ -4822,7 +4822,7 @@ static struct bpf_test tests[] = {
BPF_EXIT_INSN(),
},
.fixup_map1 = { 4 },
- .errstr = "R4 pointer arithmetic on PTR_TO_MAP_VALUE_OR_NULL",
+ .errstr = "R4 pointer arithmetic on map_value_or_null",
.result = REJECT,
.prog_type = BPF_PROG_TYPE_SCHED_CLS
},
@@ -7137,7 +7137,7 @@ static struct bpf_test tests[] = {
BPF_EXIT_INSN(),
},
.fixup_map_in_map = { 3 },
- .errstr = "R1 pointer arithmetic on CONST_PTR_TO_MAP prohibited",
+ .errstr = "R1 pointer arithmetic on map_ptr prohibited",
.result = REJECT,
},
{
@@ -8811,7 +8811,7 @@ static struct bpf_test tests[] = {
BPF_MOV64_IMM(BPF_REG_0, 0),
BPF_EXIT_INSN(),
},
- .errstr = "R3 pointer arithmetic on PTR_TO_PACKET_END",
+ .errstr = "R3 pointer arithmetic on pkt_end",
.result = REJECT,
.prog_type = BPF_PROG_TYPE_XDP,
},
@@ -8830,7 +8830,7 @@ static struct bpf_test tests[] = {
BPF_MOV64_IMM(BPF_REG_0, 0),
BPF_EXIT_INSN(),
},
- .errstr = "R3 pointer arithmetic on PTR_TO_PACKET_END",
+ .errstr = "R3 pointer arithmetic on pkt_end",
.result = REJECT,
.prog_type = BPF_PROG_TYPE_XDP,
},
--
2.17.1
^ permalink raw reply related
* [PATCHv2 bpf-next 01/11] bpf: Add iterator for spilled registers
From: Joe Stringer @ 2018-09-21 17:10 UTC (permalink / raw)
To: ast
Cc: netdev, daniel, john.fastabend, tgraf, kafai, nitin.hande,
mauricio.vasquez
In-Reply-To: <20180921171043.20823-1-joe@wand.net.nz>
Add this iterator for spilled registers, it concentrates the details of
how to get the current frame's spilled registers into a single macro
while clarifying the intention of the code which is calling the macro.
Signed-off-by: Joe Stringer <joe@wand.net.nz>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
include/linux/bpf_verifier.h | 11 +++++++++++
kernel/bpf/verifier.c | 16 +++++++---------
2 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index b42b60a83e19..af262b97f586 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -131,6 +131,17 @@ struct bpf_verifier_state {
u32 curframe;
};
+#define __get_spilled_reg(slot, frame) \
+ (((slot < frame->allocated_stack / BPF_REG_SIZE) && \
+ (frame->stack[slot].slot_type[0] == STACK_SPILL)) \
+ ? &frame->stack[slot].spilled_ptr : NULL)
+
+/* Iterate over 'frame', setting 'reg' to either NULL or a spilled register. */
+#define for_each_spilled_reg(iter, frame, reg) \
+ for (iter = 0, reg = __get_spilled_reg(iter, frame); \
+ iter < frame->allocated_stack / BPF_REG_SIZE; \
+ iter++, reg = __get_spilled_reg(iter, frame))
+
/* linked list of verifier states used to prune search */
struct bpf_verifier_state_list {
struct bpf_verifier_state state;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 8ccbff4fff93..62ce45d9c558 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2250,10 +2250,9 @@ static void __clear_all_pkt_pointers(struct bpf_verifier_env *env,
if (reg_is_pkt_pointer_any(®s[i]))
mark_reg_unknown(env, regs, i);
- for (i = 0; i < state->allocated_stack / BPF_REG_SIZE; i++) {
- if (state->stack[i].slot_type[0] != STACK_SPILL)
+ for_each_spilled_reg(i, state, reg) {
+ if (!reg)
continue;
- reg = &state->stack[i].spilled_ptr;
if (reg_is_pkt_pointer_any(reg))
__mark_reg_unknown(reg);
}
@@ -3393,10 +3392,9 @@ static void find_good_pkt_pointers(struct bpf_verifier_state *vstate,
for (j = 0; j <= vstate->curframe; j++) {
state = vstate->frame[j];
- for (i = 0; i < state->allocated_stack / BPF_REG_SIZE; i++) {
- if (state->stack[i].slot_type[0] != STACK_SPILL)
+ for_each_spilled_reg(i, state, reg) {
+ if (!reg)
continue;
- reg = &state->stack[i].spilled_ptr;
if (reg->type == type && reg->id == dst_reg->id)
reg->range = max(reg->range, new_range);
}
@@ -3641,7 +3639,7 @@ static void mark_map_regs(struct bpf_verifier_state *vstate, u32 regno,
bool is_null)
{
struct bpf_func_state *state = vstate->frame[vstate->curframe];
- struct bpf_reg_state *regs = state->regs;
+ struct bpf_reg_state *reg, *regs = state->regs;
u32 id = regs[regno].id;
int i, j;
@@ -3650,8 +3648,8 @@ static void mark_map_regs(struct bpf_verifier_state *vstate, u32 regno,
for (j = 0; j <= vstate->curframe; j++) {
state = vstate->frame[j];
- for (i = 0; i < state->allocated_stack / BPF_REG_SIZE; i++) {
- if (state->stack[i].slot_type[0] != STACK_SPILL)
+ for_each_spilled_reg(i, state, reg) {
+ if (!reg)
continue;
mark_map_reg(&state->stack[i].spilled_ptr, 0, id, is_null);
}
--
2.17.1
^ permalink raw reply related
* [PATCHv2 bpf-next 00/11] Add socket lookup support
From: Joe Stringer @ 2018-09-21 17:10 UTC (permalink / raw)
To: ast
Cc: netdev, daniel, john.fastabend, tgraf, kafai, nitin.hande,
mauricio.vasquez
This series proposes a new helper for the BPF API which allows BPF programs to
perform lookups for sockets in a network namespace. This would allow programs
to determine early on in processing whether the stack is expecting to receive
the packet, and perform some action (eg drop, forward somewhere) based on this
information.
The series is structured roughly into:
* Misc refactor
* Add the socket pointer type
* Add reference tracking to ensure that socket references are freed
* Extend the BPF API to add sk_lookup_xxx() / sk_release() functions
* Add tests/documentation
The helper proposed in this series includes a parameter for a tuple which must
be filled in by the caller to determine the socket to look up. The simplest
case would be filling with the contents of the packet, ie mapping the packet's
5-tuple into the parameter. In common cases, it may alternatively be useful to
reverse the direction of the tuple and perform a lookup, to find the socket
that initiates this connection; and if the BPF program ever performs a form of
IP address translation, it may further be useful to be able to look up
arbitrary tuples that are not based upon the packet, but instead based on state
held in BPF maps or hardcoded in the BPF program.
Currently, access into the socket's fields are limited to those which are
otherwise already accessible, and are restricted to read-only access.
Changes since v1:
* Limit netns_id field to 32 bits
* Reuse reg_type_mismatch() in more places
* Reduce the number of passes at convert_ctx_access()
* Replace ptr_id defensive coding when releasing reference state with an
internal error (-EFAULT)
* Rework 'struct bpf_sock_tuple' to allow passing a packet pointer
* Allow direct packet access from helper
* Fix compile error with CONFIG_IPV6 enabled
* Improve commit messages
Changes since RFC:
* Split up sk_lookup() into sk_lookup_tcp(), sk_lookup_udp().
* Only take references on the socket when necessary.
* Make sk_release() only free the socket reference in this case.
* Fix some runtime reference leaks:
* Disallow BPF_LD_[ABS|IND] instructions while holding a reference.
* Disallow bpf_tail_call() while holding a reference.
* Prevent the same instruction being used for reference and other
pointer type.
* Simplify locating copies of a reference during helper calls by caching
the pointer id from the caller.
* Fix kbuild compilation warnings with particular configs.
* Improve code comments describing the new verifier pieces.
* Testing courtesy of Nitin
* Rebase
This tree is also available at:
https://github.com/joestringer/linux/commits/submit/sk-lookup-v2
Joe Stringer (11):
bpf: Add iterator for spilled registers
bpf: Simplify ptr_min_max_vals adjustment
bpf: Generalize ptr_or_null regs check
bpf: Add PTR_TO_SOCKET verifier type
bpf: Macrofy stack state copy
bpf: Add reference tracking to verifier
bpf: Add helper to retrieve socket in BPF
selftests/bpf: Add tests for reference tracking
libbpf: Support loading individual progs
selftests/bpf: Add C tests for reference tracking
Documentation: Describe bpf reference tracking
Documentation/networking/filter.txt | 64 ++
include/linux/bpf.h | 17 +
include/linux/bpf_verifier.h | 37 +-
include/uapi/linux/bpf.h | 57 +-
kernel/bpf/verifier.c | 594 ++++++++++++++----
net/core/filter.c | 179 +++++-
tools/include/uapi/linux/bpf.h | 57 +-
tools/lib/bpf/libbpf.c | 4 +-
tools/lib/bpf/libbpf.h | 3 +
tools/testing/selftests/bpf/Makefile | 2 +-
tools/testing/selftests/bpf/bpf_helpers.h | 12 +
tools/testing/selftests/bpf/test_progs.c | 38 ++
.../selftests/bpf/test_sk_lookup_kern.c | 137 ++++
tools/testing/selftests/bpf/test_verifier.c | 373 ++++++++++-
14 files changed, 1441 insertions(+), 133 deletions(-)
create mode 100644 tools/testing/selftests/bpf/test_sk_lookup_kern.c
--
2.17.1
^ permalink raw reply
* Re: [PATCH net] af_key: free SKBs under RCU protection
From: stranche @ 2018-09-21 17:09 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, steffen.klassert
In-Reply-To: <6f2e6b62-42dc-f2b3-3758-29f5338f1185@gmail.com>
>>
>> As long as one skb has sock_rfree has its destructor, the socket
>> attached to
>> this skb can not be released. There is no race here.
>>
>> Note that skb_clone() does not propagate the destructor.
>>
>> The issue here is that in the rcu lookup, we can find a socket that
>> has been
>> dismantled, with a 0 refcount.
>>
>> We must not use sock_hold() in this case, since we are not sure the
>> socket refcount is not already 0.
>>
>> pfkey_broadcast() and pfkey_broadcast_one() violate basic RCU rules.
>>
>> When in a RCU lookup, one want to increment an object refcount, it
>> needs
>> to be extra-careful, as I did in my proposal.
>>
>> Note that the race could be automatically detected with
>> CONFIG_REFCOUNT_FULL=y
>
> Bug was added in commit 7f6b9dbd5afb ("af_key: locking change")
Hi Eric,
I tried your refcount idea below, but it still results in the same
crash.
>>>> --- a/net/key/af_key.c
>>>> +++ b/net/key/af_key.c
>>>> @@ -201,7 +201,9 @@ static int pfkey_broadcast_one(struct sk_buff
>>>> *skb, struct sk_buff **skb2,
>>>> {
>>>> int err = -ENOBUFS;
>>>>
>>>> - sock_hold(sk);
>>>> + if (!refcount_inc_not_zero(&sk->sk_refcnt))
>>>> + return -ENOENT;
>>>> +
>>>> if (*skb2 == NULL) {
>>>> if (refcount_read(&skb->users) != 1) {
>>>> *skb2 = skb_clone(skb, allocation);
I also tried reverting 7f6b9dbd5afb ("af_key: locking change") and
running the
test there and I still see the crash, so it doesn't seem to be an RCU
specific
issue.
Is there anything else that could be causing this?
^ permalink raw reply
* [PATCH bpf-next] bpftool: add support for BPF_MAP_TYPE_REUSEPORT_SOCKARRAY maps
From: Roman Gushchin @ 2018-09-21 22:47 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Kernel Team, Roman Gushchin,
Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski,
Yonghong Song
Add BPF_MAP_TYPE_REUSEPORT_SOCKARRAY map type to the list
of maps types which bpftool recognizes.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Yonghong Song <yhs@fb.com>
---
tools/bpf/bpftool/map.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index af8ad32fa6e9..e22fbe8b975f 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -71,6 +71,7 @@ static const char * const map_type_name[] = {
[BPF_MAP_TYPE_XSKMAP] = "xskmap",
[BPF_MAP_TYPE_SOCKHASH] = "sockhash",
[BPF_MAP_TYPE_CGROUP_STORAGE] = "cgroup_storage",
+ [BPF_MAP_TYPE_REUSEPORT_SOCKARRAY] = "reuseport_sockarray",
};
static bool map_is_per_cpu(__u32 type)
--
2.17.1
^ permalink raw reply related
* Re: [PATCH iproute2] iplink_vxlan: take into account preferred_family creating vxlan device
From: Lorenzo Bianconi @ 2018-09-21 16:35 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20180921085805.7ee078e8@xeon-e3>
> On Fri, 21 Sep 2018 15:34:25 +0200
> Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote:
>
> > Take into account the configured preferred_family if neither saddr or
> > daddr are provided since otherwise vxlan kernel module will use IPv4 as
> > default remote inet family neglecting the one provided by userspace.
> > This behaviour was originally in commit 97d564b90ccb ("vxlan: use
> > preferred address family when neither group or remote is specified").
> > The issue can be triggered with the following reproducer:
> >
> > $ip -6 link add vxlan1 type vxlan id 42 dev enp0s2 \
> > proxy nolearning l2miss l3miss
> > $bridge fdb add 46:47:1f:a7:1c:25 dev vxlan1 dst 2000::2
> > RTNETLINK answers: Address family not supported by protocol
> >
> > Fixes: 1e9b8072de2c ("iplink_vxlan: Get rid of inet_get_addr()")
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
>
> This patch was already in the queue. Is this a new version (if so please mark it as v2).
Hi Stephen,
yes, this is a v2. I missed v2 in the subject and changes respect to v1
(I have just modified the commit log actually). Please drop v1 and sorry for
the noise.
Regards,
Lorenzo
>
^ permalink raw reply
* [PATCH] rtlwifi: btcoex: Use proper enumerated types for Wi-Fi only interface
From: Nathan Chancellor @ 2018-09-21 22:08 UTC (permalink / raw)
To: Ping-Ke Shih, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel, Nathan Chancellor
Clang warns when one enumerated type is implicitly converted to another.
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c:1327:34:
warning: implicit conversion from enumeration type 'enum
btc_chip_interface' to different enumeration type 'enum
wifionly_chip_interface' [-Wenum-conversion]
wifionly_cfg->chip_interface = BTC_INTF_PCI;
~ ^~~~~~~~~~~~
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c:1330:34:
warning: implicit conversion from enumeration type 'enum
btc_chip_interface' to different enumeration type 'enum
wifionly_chip_interface' [-Wenum-conversion]
wifionly_cfg->chip_interface = BTC_INTF_USB;
~ ^~~~~~~~~~~~
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c:1333:34:
warning: implicit conversion from enumeration type 'enum
btc_chip_interface' to different enumeration type 'enum
wifionly_chip_interface' [-Wenum-conversion]
wifionly_cfg->chip_interface = BTC_INTF_UNKNOWN;
~ ^~~~~~~~~~~~~~~~
3 warnings generated.
Use the values from the correct enumerated type, wifionly_chip_interface.
BTC_INTF_UNKNOWN = WIFIONLY_INTF_UNKNOWN = 0
BTC_INTF_PCI = WIFIONLY_INTF_PCI = 0
BTC_INTF_USB = WIFIONLY_INTF_USB = 0
Link: https://github.com/ClangBuiltLinux/linux/issues/135
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
.../net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
index b026e80940a4..6fbf8845a2ab 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
@@ -1324,13 +1324,13 @@ bool exhalbtc_initlize_variables_wifi_only(struct rtl_priv *rtlpriv)
switch (rtlpriv->rtlhal.interface) {
case INTF_PCI:
- wifionly_cfg->chip_interface = BTC_INTF_PCI;
+ wifionly_cfg->chip_interface = WIFIONLY_INTF_PCI;
break;
case INTF_USB:
- wifionly_cfg->chip_interface = BTC_INTF_USB;
+ wifionly_cfg->chip_interface = WIFIONLY_INTF_USB;
break;
default:
- wifionly_cfg->chip_interface = BTC_INTF_UNKNOWN;
+ wifionly_cfg->chip_interface = WIFIONLY_INTF_UNKNOWN;
break;
}
--
2.19.0
^ permalink raw reply related
* Re: [PATCH iproute2 v2 0/3] testsuite: make alltests fixes
From: Stephen Hemminger @ 2018-09-21 16:16 UTC (permalink / raw)
To: Petr Vorel; +Cc: netdev, Phil Sutter, Luca Boccassi
In-Reply-To: <20180919233624.18494-1-petr.vorel@gmail.com>
On Thu, 20 Sep 2018 01:36:21 +0200
Petr Vorel <petr.vorel@gmail.com> wrote:
> Hi,
>
> here are simply fixes to restore 'make alltests'.
> Currently it does not run.
>
> Kind regards,
> Petr
>
> Petr Vorel (3):
> testsuite: Fix missing generate_nlmsg
> testsuite: Generate generate_nlmsg when needed
> testsuite: Warn about empty $(IPVERS)
>
> testsuite/Makefile | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
Applied, it would be good to have 'make tests' at the top level.
^ permalink raw reply
* [PATCH iproute2] Makefile: add help target
From: Stephen Hemminger @ 2018-09-21 16:16 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
Add help target to Makefile.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
Makefile | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Makefile b/Makefile
index ea2f797c933f..25de3893cae4 100644
--- a/Makefile
+++ b/Makefile
@@ -71,6 +71,18 @@ all: config.mk
for i in $(SUBDIRS); \
do echo; echo $$i; $(MAKE) $(MFLAGS) -C $$i; done
+help:
+ @echo "Make Targets:"
+ @echo " all - build binaries"
+ @echo " clean - remove products of build"
+ @echo " distclean - remove configuration and build"
+ @echo " install - install binaries on local machine"
+ @echo " cscope - build cscope database"
+ @echo " snapshot - generate version number header"
+ @echo ""
+ @echo "Make Arguments:"
+ @echo " V=[0|1] - set build verbosity level"
+
config.mk:
sh configure $(KERNEL_INCLUDE)
--
2.18.0
^ permalink raw reply related
* Re: [PATCH v2 net-next] ravb: remove tx buffer addr 4byte alilgnment restriction for R-Car Gen3
From: Sergei Shtylyov @ 2018-09-21 16:14 UTC (permalink / raw)
To: Simon Horman, David Miller
Cc: Magnus Damm, netdev, linux-renesas-soc, Kazuya Mizuguchi
In-Reply-To: <20180919080621.821-1-horms+renesas@verge.net.au>
On 09/19/2018 11:06 AM, Simon Horman wrote:
> From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
>
> This patch sets from two descriptor to one descriptor because R-Car Gen3
> does not have the 4 bytes alignment restriction of the transmission buffer.
>
> Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> v2 [Simon Horman]
> * As per review by Sergi Shtylyov
It's Sergei or (more correctly) Sergey. :-)
> - Use reverse xmas tree for variable declarations
> - Use > rather than >= for conditions
> - Dropped unnecessary parentheses
> - Don't allocate memory for tx_align when it will not be used
> - But, kept NUM_TX_DESC_GEN[23] as I see some value in
> the self-documentation provided by these #defines
[...]
Better late than never (-:
Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
MBR, Sergei
^ permalink raw reply
* Re: [PATCH ipsec] net: xfrm: pass constant family to nf_hook function
From: Florian Westphal @ 2018-09-21 16:10 UTC (permalink / raw)
To: David Ahern; +Cc: Florian Westphal, netdev
In-Reply-To: <b274fc22-03b6-4d71-396f-1d57abf53eb1@gmail.com>
David Ahern <dsahern@gmail.com> wrote:
> On 9/21/18 8:55 AM, Florian Westphal wrote:
> > David Ahern <dsahern@gmail.com> wrote:
> >>> David, i hope this will silence the warning, would be nice
> >>> if you could test it.
> >>
> >> I still the warning.
> >
> > Wait. Do you see this warning everywhere or just in xfrm?
> >
>
> just the one file.
Sigh, ok, i will set up a vm and deal with this somehow.
Steffen, please toss this patch, sorry for the noise.
^ permalink raw reply
* Re: [PATCH iproute2] iplink_vxlan: take into account preferred_family creating vxlan device
From: Stephen Hemminger @ 2018-09-21 15:58 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: netdev
In-Reply-To: <fc0937f4f788f84986fd0ace7722e8b2d3e2755e.1537536423.git.lorenzo.bianconi@redhat.com>
On Fri, 21 Sep 2018 15:34:25 +0200
Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote:
> Take into account the configured preferred_family if neither saddr or
> daddr are provided since otherwise vxlan kernel module will use IPv4 as
> default remote inet family neglecting the one provided by userspace.
> This behaviour was originally in commit 97d564b90ccb ("vxlan: use
> preferred address family when neither group or remote is specified").
> The issue can be triggered with the following reproducer:
>
> $ip -6 link add vxlan1 type vxlan id 42 dev enp0s2 \
> proxy nolearning l2miss l3miss
> $bridge fdb add 46:47:1f:a7:1c:25 dev vxlan1 dst 2000::2
> RTNETLINK answers: Address family not supported by protocol
>
> Fixes: 1e9b8072de2c ("iplink_vxlan: Get rid of inet_get_addr()")
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
This patch was already in the queue. Is this a new version (if so please mark it as v2).
^ permalink raw reply
* RE: [PATCH net-next] dt-bindings: net: ravb: Add support for r8a7744 SoC
From: Biju Das @ 2018-09-21 15:57 UTC (permalink / raw)
To: Sergei Shtylyov, Rob Herring, Mark Rutland
Cc: David S. Miller, Geert Uytterhoeven, Simon Horman,
Fabrizio Castro, Jacopo Mondi, Yoshihiro Shimoda,
netdev@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
devicetree@vger.kernel.org, Simon Horman, Chris Paterson
In-Reply-To: <922d6b1d-764a-ad2d-6a6c-8d992f3fbe15@cogentembedded.com>
Hi Sergei,
Thanks for the feedback.
> Subject: Re: [PATCH net-next] dt-bindings: net: ravb: Add support for
> r8a7744 SoC
>
> Hello!
>
> On 09/21/2018 05:25 PM, Biju Das wrote:
>
> > Document RZ/G1N (R8A7744) SoC bindings.
> >
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > Reviewed-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
>
> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> > ---
> > Documentation/devicetree/bindings/net/renesas,ravb.txt | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/renesas,ravb.txt
> b/Documentation/devicetree/bindings/net/renesas,ravb.txt
> > index da249b7..3530256 100644
> > --- a/Documentation/devicetree/bindings/net/renesas,ravb.txt
> > +++ b/Documentation/devicetree/bindings/net/renesas,ravb.txt
> > @@ -6,6 +6,7 @@ interface contains.
> > Required properties:
> > - compatible: Must contain one or more of the following:
> > - "renesas,etheravb-r8a7743" for the R8A7743 SoC.
> > + - "renesas,etheravb-r8a7744" for the R8A7744 SoC.
>
> Mmm, I thought it was decided to go with the full model #s, like with
> R8A77470 below?
For RZ/G1C case, there are 2 variants 1 with HDMI(R8A77470) and the other without HDMI(R8A77471).
Current support is for R8A77470.
> > - "renesas,etheravb-r8a7745" for the R8A7745 SoC.
> > - "renesas,etheravb-r8a77470" for the R8A77470 SoC.
> > - "renesas,etheravb-r8a7790" for the R8A7790 SoC.
Regards,
Biju
Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
^ permalink raw reply
* Re: [PATCH ipsec] net: xfrm: pass constant family to nf_hook function
From: David Ahern @ 2018-09-21 15:56 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev
In-Reply-To: <20180921155539.azjj4hemohprg2rh@breakpoint.cc>
On 9/21/18 8:55 AM, Florian Westphal wrote:
> David Ahern <dsahern@gmail.com> wrote:
>>> David, i hope this will silence the warning, would be nice
>>> if you could test it.
>>
>> I still the warning.
>
> Wait. Do you see this warning everywhere or just in xfrm?
>
just the one file.
^ permalink raw reply
* Re: [PATCH ipsec] net: xfrm: pass constant family to nf_hook function
From: Florian Westphal @ 2018-09-21 15:55 UTC (permalink / raw)
To: David Ahern; +Cc: Florian Westphal, netdev
In-Reply-To: <299db7b3-dc90-18b6-759e-08d36eed3539@gmail.com>
David Ahern <dsahern@gmail.com> wrote:
> > David, i hope this will silence the warning, would be nice
> > if you could test it.
>
> I still the warning.
Wait. Do you see this warning everywhere or just in xfrm?
^ permalink raw reply
* Re: [PATCH ipsec] net: xfrm: pass constant family to nf_hook function
From: David Ahern @ 2018-09-21 15:54 UTC (permalink / raw)
To: Florian Westphal, netdev
In-Reply-To: <20180921103541.8812-1-fw@strlen.de>
On 9/21/18 3:35 AM, Florian Westphal wrote:
> Unfortunately some versions of gcc emit following warning:
> linux/compiler.h:252:20: warning: array subscript is above array bounds [-Warray-bounds]
> hook_head = rcu_dereference(net->nf.hooks_arp[hook]);
> ^~~~~~~~~~~~~~~~~~~~~
> xfrm_output_resume passes non-const 'pf' argument so compiler can't know
> that the affected statement above will never be executed (we only
> pass either NFPROTO_IPV4 or NFPROTO_IPV6), this change makes this
> explicit.
>
> Another solution would be to increase hooks_arp[] size, but that
> increases struct net size needlesly.
>
> Reported-by: David Ahern <dsahern@gmail.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> David, i hope this will silence the warning, would be nice
> if you could test it.
I still the warning.
>
> I don't really like this patch, but I see no better solution
> expect needless increase of hooks_arp[].
>
> Any other idea?
>
I don't have any time in the next week to look into it.
Sounds like you are not able to reproduce the output. It just debian
stretch and gcc version 6.3.0 20170516 (Debian 6.3.0-18+deb9u1).
^ permalink raw reply
* [PATCH net-next 9/9] net_sched: sch_fq: remove dead code dealing with retransmits
From: Eric Dumazet @ 2018-09-21 15:51 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Van Jacobson, Neal Cardwell, Yuchung Cheng,
Soheil Hassas Yeganeh, Willem de Bruijn, Eric Dumazet,
Eric Dumazet
In-Reply-To: <20180921155154.49489-1-edumazet@google.com>
With the earliest departure time model, we no longer plan
special casing TCP retransmits. We therefore remove dead
code (since most compilers understood skb_is_retransmit()
was false)
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/sched/sch_fq.c | 58 ++++------------------------------------------
1 file changed, 5 insertions(+), 53 deletions(-)
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 77692ad6741de14025bd848741604e775742430b..628a2cdcfc6f2fa69d9402f06881949d2e1423d9 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -106,7 +106,6 @@ struct fq_sched_data {
u64 stat_gc_flows;
u64 stat_internal_packets;
- u64 stat_tcp_retrans;
u64 stat_throttled;
u64 stat_flows_plimit;
u64 stat_pkts_too_long;
@@ -327,62 +326,17 @@ static struct sk_buff *fq_dequeue_head(struct Qdisc *sch, struct fq_flow *flow)
return skb;
}
-/* We might add in the future detection of retransmits
- * For the time being, just return false
- */
-static bool skb_is_retransmit(struct sk_buff *skb)
-{
- return false;
-}
-
-/* add skb to flow queue
- * flow queue is a linked list, kind of FIFO, except for TCP retransmits
- * We special case tcp retransmits to be transmitted before other packets.
- * We rely on fact that TCP retransmits are unlikely, so we do not waste
- * a separate queue or a pointer.
- * head-> [retrans pkt 1]
- * [retrans pkt 2]
- * [ normal pkt 1]
- * [ normal pkt 2]
- * [ normal pkt 3]
- * tail-> [ normal pkt 4]
- */
static void flow_queue_add(struct fq_flow *flow, struct sk_buff *skb)
{
- struct sk_buff *prev, *head = flow->head;
+ struct sk_buff *head = flow->head;
skb->next = NULL;
- if (!head) {
+ if (!head)
flow->head = skb;
- flow->tail = skb;
- return;
- }
- if (likely(!skb_is_retransmit(skb))) {
+ else
flow->tail->next = skb;
- flow->tail = skb;
- return;
- }
- /* This skb is a tcp retransmit,
- * find the last retrans packet in the queue
- */
- prev = NULL;
- while (skb_is_retransmit(head)) {
- prev = head;
- head = head->next;
- if (!head)
- break;
- }
- if (!prev) { /* no rtx packet in queue, become the new head */
- skb->next = flow->head;
- flow->head = skb;
- } else {
- if (prev == flow->tail)
- flow->tail = skb;
- else
- skb->next = prev->next;
- prev->next = skb;
- }
+ flow->tail = skb;
}
static int fq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
@@ -401,8 +355,6 @@ static int fq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
}
f->qlen++;
- if (skb_is_retransmit(skb))
- q->stat_tcp_retrans++;
qdisc_qstats_backlog_inc(sch, skb);
if (fq_flow_is_detached(f)) {
struct sock *sk = skb->sk;
@@ -874,7 +826,7 @@ static int fq_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
st.gc_flows = q->stat_gc_flows;
st.highprio_packets = q->stat_internal_packets;
- st.tcp_retrans = q->stat_tcp_retrans;
+ st.tcp_retrans = 0;
st.throttled = q->stat_throttled;
st.flows_plimit = q->stat_flows_plimit;
st.pkts_too_long = q->stat_pkts_too_long;
--
2.19.0.444.g18242da7ef-goog
^ permalink raw reply related
* [PATCH net-next 8/9] tcp: switch tcp_internal_pacing() to tcp_wstamp_ns
From: Eric Dumazet @ 2018-09-21 15:51 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Van Jacobson, Neal Cardwell, Yuchung Cheng,
Soheil Hassas Yeganeh, Willem de Bruijn, Eric Dumazet,
Eric Dumazet
In-Reply-To: <20180921155154.49489-1-edumazet@google.com>
Now TCP keeps track of tcp_wstamp_ns, recording the earliest
departure time of next packet, we can remove duplicate code
from tcp_internal_pacing()
This removes one ktime_get_tai_ns() call, and a divide.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_output.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 2adb719e97b89021becfa1243d33c87df6cdf8a5..fe7855b090e4feed6a7d1ba6ee874cdb23a9bd0c 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -993,21 +993,12 @@ enum hrtimer_restart tcp_pace_kick(struct hrtimer *timer)
return HRTIMER_NORESTART;
}
-static void tcp_internal_pacing(struct sock *sk, const struct sk_buff *skb)
+static void tcp_internal_pacing(struct sock *sk)
{
- u64 len_ns;
- u32 rate;
-
if (!tcp_needs_internal_pacing(sk))
return;
- rate = sk->sk_pacing_rate;
- if (!rate || rate == ~0U)
- return;
-
- len_ns = (u64)skb->len * NSEC_PER_SEC;
- do_div(len_ns, rate);
hrtimer_start(&tcp_sk(sk)->pacing_timer,
- ktime_add_ns(ktime_get_tai_ns(), len_ns),
+ ns_to_ktime(tcp_sk(sk)->tcp_wstamp_ns),
HRTIMER_MODE_ABS_PINNED_SOFT);
sock_hold(sk);
}
@@ -1026,7 +1017,8 @@ static void tcp_update_skb_after_send(struct sock *sk, struct sk_buff *skb)
*/
if (rate != ~0U && rate && tp->data_segs_out >= 10) {
tp->tcp_wstamp_ns += div_u64((u64)skb->len * NSEC_PER_SEC, rate);
- /* TODO: update internal pacing here */
+
+ tcp_internal_pacing(sk);
}
}
list_move_tail(&skb->tcp_tsorted_anchor, &tp->tsorted_sent_queue);
@@ -1167,7 +1159,6 @@ static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb,
tcp_event_data_sent(tp, sk);
tp->data_segs_out += tcp_skb_pcount(skb);
tp->bytes_sent += skb->len - tcp_header_size;
- tcp_internal_pacing(sk, skb);
}
if (after(tcb->end_seq, tp->snd_nxt) || tcb->seq == tcb->end_seq)
--
2.19.0.444.g18242da7ef-goog
^ permalink raw reply related
* [PATCH net-next 7/9] tcp: switch tcp and sch_fq to new earliest departure time model
From: Eric Dumazet @ 2018-09-21 15:51 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Van Jacobson, Neal Cardwell, Yuchung Cheng,
Soheil Hassas Yeganeh, Willem de Bruijn, Eric Dumazet,
Eric Dumazet
In-Reply-To: <20180921155154.49489-1-edumazet@google.com>
TCP keeps track of tcp_wstamp_ns by itself, meaning sch_fq
no longer has to do it.
Thanks to this model, TCP can get more accurate RTT samples,
since pacing no longer inflates them.
This has the nice effect of removing some delays caused by FQ
quantum mechanism, causing inflated max/P99 latencies.
Also we might relax TCP Small Queue tight limits in the future,
since this new model allow TCP to build bigger batches, since
sch_fq (or a device with earliest departure time offload) ensure
these packets will be delivered on time.
Note that other protocols are not converted (they will probably
never be) so sch_fq has still support for SO_MAX_PACING_RATE
Tested:
Test showing FQ pacing quantum artifact for low-rate flows,
adding unexpected throttles for RPC flows, inflating max and P99 latencies.
The parameters chosen here are to show what happens typically when
a TCP flow has a reduced pacing rate (this can be caused by a reduced
cwin after few losses, or/and rtt above few ms)
MIBS="MIN_LATENCY,MEAN_LATENCY,MAX_LATENCY,P99_LATENCY,STDDEV_LATENCY"
Before :
$ netperf -H 10.246.7.133 -t TCP_RR -Cc -T6,6 -- -q 2000000 -r 100,100 -o $MIBS
MIGRATED TCP REQUEST/RESPONSE TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 10.246.7.133 () port 0 AF_INET : first burst 0 : cpu bind
Minimum Latency Microseconds,Mean Latency Microseconds,Maximum Latency Microseconds,99th Percentile Latency Microseconds,Stddev Latency Microseconds
19,82.78,5279,3825,482.02
After :
$ netperf -H 10.246.7.133 -t TCP_RR -Cc -T6,6 -- -q 2000000 -r 100,100 -o $MIBS
MIGRATED TCP REQUEST/RESPONSE TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 10.246.7.133 () port 0 AF_INET : first burst 0 : cpu bind
Minimum Latency Microseconds,Mean Latency Microseconds,Maximum Latency Microseconds,99th Percentile Latency Microseconds,Stddev Latency Microseconds
20,49.94,128,63,3.18
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_bbr.c | 7 ++++---
net/ipv4/tcp_output.c | 22 ++++++++++++++++++----
net/sched/sch_fq.c | 21 +++++++++++----------
3 files changed, 33 insertions(+), 17 deletions(-)
diff --git a/net/ipv4/tcp_bbr.c b/net/ipv4/tcp_bbr.c
index 02ff2dde96094cf33b662a20994424a7adea509e..a5786e3e2c16ce53a332f29c9a55b9a641eec791 100644
--- a/net/ipv4/tcp_bbr.c
+++ b/net/ipv4/tcp_bbr.c
@@ -128,6 +128,9 @@ static const u32 bbr_probe_rtt_mode_ms = 200;
/* Skip TSO below the following bandwidth (bits/sec): */
static const int bbr_min_tso_rate = 1200000;
+/* Pace at ~1% below estimated bw, on average, to reduce queue at bottleneck. */
+static const int bbr_pacing_marging_percent = 1;
+
/* We use a high_gain value of 2/ln(2) because it's the smallest pacing gain
* that will allow a smoothly increasing pacing rate that will double each RTT
* and send the same number of packets per RTT that an un-paced, slow-starting
@@ -208,12 +211,10 @@ static u64 bbr_rate_bytes_per_sec(struct sock *sk, u64 rate, int gain)
{
unsigned int mss = tcp_sk(sk)->mss_cache;
- if (!tcp_needs_internal_pacing(sk))
- mss = tcp_mss_to_mtu(sk, mss);
rate *= mss;
rate *= gain;
rate >>= BBR_SCALE;
- rate *= USEC_PER_SEC;
+ rate *= USEC_PER_SEC / 100 * (100 - bbr_pacing_marging_percent);
return rate >> BW_SCALE;
}
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index a87068fa9b1aa582310df6371966fd2d6461edb8..2adb719e97b89021becfa1243d33c87df6cdf8a5 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1012,9 +1012,23 @@ static void tcp_internal_pacing(struct sock *sk, const struct sk_buff *skb)
sock_hold(sk);
}
-static void tcp_update_skb_after_send(struct tcp_sock *tp, struct sk_buff *skb)
+static void tcp_update_skb_after_send(struct sock *sk, struct sk_buff *skb)
{
+ struct tcp_sock *tp = tcp_sk(sk);
+
skb->skb_mstamp_ns = tp->tcp_wstamp_ns;
+ if (sk->sk_pacing_status != SK_PACING_NONE) {
+ u32 rate = sk->sk_pacing_rate;
+
+ /* Original sch_fq does not pace first 10 MSS
+ * Note that tp->data_segs_out overflows after 2^32 packets,
+ * this is a minor annoyance.
+ */
+ if (rate != ~0U && rate && tp->data_segs_out >= 10) {
+ tp->tcp_wstamp_ns += div_u64((u64)skb->len * NSEC_PER_SEC, rate);
+ /* TODO: update internal pacing here */
+ }
+ }
list_move_tail(&skb->tcp_tsorted_anchor, &tp->tsorted_sent_queue);
}
@@ -1178,7 +1192,7 @@ static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb,
err = net_xmit_eval(err);
}
if (!err && oskb) {
- tcp_update_skb_after_send(tp, oskb);
+ tcp_update_skb_after_send(sk, oskb);
tcp_rate_skb_sent(sk, oskb);
}
return err;
@@ -2327,7 +2341,7 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
if (unlikely(tp->repair) && tp->repair_queue == TCP_SEND_QUEUE) {
/* "skb_mstamp" is used as a start point for the retransmit timer */
- tcp_update_skb_after_send(tp, skb);
+ tcp_update_skb_after_send(sk, skb);
goto repair; /* Skip network transmission */
}
@@ -2902,7 +2916,7 @@ int __tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs)
} tcp_skb_tsorted_restore(skb);
if (!err) {
- tcp_update_skb_after_send(tp, skb);
+ tcp_update_skb_after_send(sk, skb);
tcp_rate_skb_sent(sk, skb);
}
} else {
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index d5185c44e9a5f521ca99243b6e9b53ec05b84d49..77692ad6741de14025bd848741604e775742430b 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -491,11 +491,16 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
}
skb = f->head;
- if (unlikely(skb && now < f->time_next_packet &&
- !skb_is_tcp_pure_ack(skb))) {
- head->first = f->next;
- fq_flow_set_throttled(q, f);
- goto begin;
+ if (skb && !skb_is_tcp_pure_ack(skb)) {
+ u64 time_next_packet = max_t(u64, ktime_to_ns(skb->tstamp),
+ f->time_next_packet);
+
+ if (now < time_next_packet) {
+ head->first = f->next;
+ f->time_next_packet = time_next_packet;
+ fq_flow_set_throttled(q, f);
+ goto begin;
+ }
}
skb = fq_dequeue_head(sch, f);
@@ -513,11 +518,7 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
prefetch(&skb->end);
f->credit -= qdisc_pkt_len(skb);
- if (!q->rate_enable)
- goto out;
-
- /* Do not pace locally generated ack packets */
- if (skb_is_tcp_pure_ack(skb))
+ if (ktime_to_ns(skb->tstamp) || !q->rate_enable)
goto out;
rate = q->flow_max_rate;
--
2.19.0.444.g18242da7ef-goog
^ permalink raw reply related
* [PATCH net-next 6/9] tcp: switch internal pacing timer to CLOCK_TAI
From: Eric Dumazet @ 2018-09-21 15:51 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Van Jacobson, Neal Cardwell, Yuchung Cheng,
Soheil Hassas Yeganeh, Willem de Bruijn, Eric Dumazet,
Eric Dumazet
In-Reply-To: <20180921155154.49489-1-edumazet@google.com>
Next patch will use tcp_wstamp_ns to feed internal
TCP pacing timer, so switch to CLOCK_TAI to share same base.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_output.c | 2 +-
net/ipv4/tcp_timer.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 957f7a0e21c06cae9f0d3bed57017bbc0a36c880..a87068fa9b1aa582310df6371966fd2d6461edb8 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1007,7 +1007,7 @@ static void tcp_internal_pacing(struct sock *sk, const struct sk_buff *skb)
len_ns = (u64)skb->len * NSEC_PER_SEC;
do_div(len_ns, rate);
hrtimer_start(&tcp_sk(sk)->pacing_timer,
- ktime_add_ns(ktime_get(), len_ns),
+ ktime_add_ns(ktime_get_tai_ns(), len_ns),
HRTIMER_MODE_ABS_PINNED_SOFT);
sock_hold(sk);
}
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 61023d50cd604d5e19464a32c33b65d29c75c81e..4f661e178da8465203266ff4dfa3e8743e60ff82 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -758,7 +758,7 @@ void tcp_init_xmit_timers(struct sock *sk)
{
inet_csk_init_xmit_timers(sk, &tcp_write_timer, &tcp_delack_timer,
&tcp_keepalive_timer);
- hrtimer_init(&tcp_sk(sk)->pacing_timer, CLOCK_MONOTONIC,
+ hrtimer_init(&tcp_sk(sk)->pacing_timer, CLOCK_TAI,
HRTIMER_MODE_ABS_PINNED_SOFT);
tcp_sk(sk)->pacing_timer.function = tcp_pace_kick;
--
2.19.0.444.g18242da7ef-goog
^ permalink raw reply related
* [PATCH net-next 5/9] tcp: provide earliest departure time in skb->tstamp
From: Eric Dumazet @ 2018-09-21 15:51 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Van Jacobson, Neal Cardwell, Yuchung Cheng,
Soheil Hassas Yeganeh, Willem de Bruijn, Eric Dumazet,
Eric Dumazet
In-Reply-To: <20180921155154.49489-1-edumazet@google.com>
Switch internal TCP skb->skb_mstamp to skb->skb_mstamp_ns,
from usec units to nsec units.
Do not clear skb->tstamp before entering IP stacks in TX,
so that qdisc or devices can implement pacing based on the
earliest departure time instead of socket sk->sk_pacing_rate
Packets are fed with tcp_wstamp_ns, and following patch
will update tcp_wstamp_ns when both TCP and sch_fq switch to
the earliest departure time mechanism.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/skbuff.h | 2 +-
include/net/tcp.h | 6 +++---
net/ipv4/syncookies.c | 2 +-
net/ipv4/tcp.c | 2 +-
net/ipv4/tcp_output.c | 13 ++++++-------
net/ipv4/tcp_timer.c | 2 +-
6 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index e3a53ca4a9b51b84b7d75ce87485d4d9109a4cf2..86f337e9a81d5eff360335a19ab09f26ae48fca8 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -689,7 +689,7 @@ struct sk_buff {
union {
ktime_t tstamp;
- u64 skb_mstamp;
+ u64 skb_mstamp_ns; /* earliest departure time */
};
/*
* This is the control buffer. It is free to use for every
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 370198fdc65d3e863104665e20faefd0e5a09b92..ff15d8e0d525715b17671e64f6abdead9df0a8f3 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -761,13 +761,13 @@ static inline u32 tcp_stamp_us_delta(u64 t1, u64 t0)
static inline u32 tcp_skb_timestamp(const struct sk_buff *skb)
{
- return div_u64(skb->skb_mstamp, USEC_PER_SEC / TCP_TS_HZ);
+ return div_u64(skb->skb_mstamp_ns, NSEC_PER_SEC / TCP_TS_HZ);
}
/* provide the departure time in us unit */
static inline u64 tcp_skb_timestamp_us(const struct sk_buff *skb)
{
- return skb->skb_mstamp;
+ return div_u64(skb->skb_mstamp_ns, NSEC_PER_USEC);
}
@@ -813,7 +813,7 @@ struct tcp_skb_cb {
#define TCPCB_SACKED_RETRANS 0x02 /* SKB retransmitted */
#define TCPCB_LOST 0x04 /* SKB is lost */
#define TCPCB_TAGBITS 0x07 /* All tag bits */
-#define TCPCB_REPAIRED 0x10 /* SKB repaired (no skb_mstamp) */
+#define TCPCB_REPAIRED 0x10 /* SKB repaired (no skb_mstamp_ns) */
#define TCPCB_EVER_RETRANS 0x80 /* Ever retransmitted frame */
#define TCPCB_RETRANS (TCPCB_SACKED_RETRANS|TCPCB_EVER_RETRANS| \
TCPCB_REPAIRED)
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index c3387dfd725bf99bcddefb9fb4f1dc98f5dd7f23..606f868d9f3fde1c3140aa7eecde87d2ec32b5f2 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -88,7 +88,7 @@ u64 cookie_init_timestamp(struct request_sock *req)
ts <<= TSBITS;
ts |= options;
}
- return (u64)ts * (USEC_PER_SEC / TCP_TS_HZ);
+ return (u64)ts * (NSEC_PER_SEC / TCP_TS_HZ);
}
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 67670fac7c8de510df351fe3a835b554cc4759a9..69c236943f56bd0749e5efb18de97e69898f1bde 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1295,7 +1295,7 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
copy = size_goal;
/* All packets are restored as if they have
- * already been sent. skb_mstamp isn't set to
+ * already been sent. skb_mstamp_ns isn't set to
* avoid wrong rtt estimation.
*/
if (tp->repair)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 5a8105e84f7c1a876bbd15e8050c2574c1fbe162..957f7a0e21c06cae9f0d3bed57017bbc0a36c880 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1014,7 +1014,7 @@ static void tcp_internal_pacing(struct sock *sk, const struct sk_buff *skb)
static void tcp_update_skb_after_send(struct tcp_sock *tp, struct sk_buff *skb)
{
- skb->skb_mstamp = tp->tcp_mstamp;
+ skb->skb_mstamp_ns = tp->tcp_wstamp_ns;
list_move_tail(&skb->tcp_tsorted_anchor, &tp->tsorted_sent_queue);
}
@@ -1061,7 +1061,7 @@ static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb,
if (unlikely(!skb))
return -ENOBUFS;
}
- skb->skb_mstamp = tp->tcp_mstamp;
+ skb->skb_mstamp_ns = tp->tcp_wstamp_ns;
inet = inet_sk(sk);
tcb = TCP_SKB_CB(skb);
@@ -1165,8 +1165,7 @@ static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb,
skb_shinfo(skb)->gso_segs = tcp_skb_pcount(skb);
skb_shinfo(skb)->gso_size = tcp_skb_mss(skb);
- /* Our usage of tstamp should remain private */
- skb->tstamp = 0;
+ /* Leave earliest departure time in skb->tstamp (skb->skb_mstamp_ns) */
/* Cleanup our debris for IP stacks */
memset(skb->cb, 0, max(sizeof(struct inet_skb_parm),
@@ -3221,10 +3220,10 @@ struct sk_buff *tcp_make_synack(const struct sock *sk, struct dst_entry *dst,
memset(&opts, 0, sizeof(opts));
#ifdef CONFIG_SYN_COOKIES
if (unlikely(req->cookie_ts))
- skb->skb_mstamp = cookie_init_timestamp(req);
+ skb->skb_mstamp_ns = cookie_init_timestamp(req);
else
#endif
- skb->skb_mstamp = tcp_clock_us();
+ skb->skb_mstamp_ns = tcp_clock_ns();
#ifdef CONFIG_TCP_MD5SIG
rcu_read_lock();
@@ -3440,7 +3439,7 @@ static int tcp_send_syn_data(struct sock *sk, struct sk_buff *syn)
err = tcp_transmit_skb(sk, syn_data, 1, sk->sk_allocation);
- syn->skb_mstamp = syn_data->skb_mstamp;
+ syn->skb_mstamp_ns = syn_data->skb_mstamp_ns;
/* Now full SYN+DATA was cloned and sent (or not),
* remove the SYN from the original skb (syn_data)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 7fdf222a0bdfe9775970082f6b5dcdcc82b2ae1a..61023d50cd604d5e19464a32c33b65d29c75c81e 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -360,7 +360,7 @@ static void tcp_probe_timer(struct sock *sk)
*/
start_ts = tcp_skb_timestamp(skb);
if (!start_ts)
- skb->skb_mstamp = tp->tcp_mstamp;
+ skb->skb_mstamp_ns = tp->tcp_wstamp_ns;
else if (icsk->icsk_user_timeout &&
(s32)(tcp_time_stamp(tp) - start_ts) > icsk->icsk_user_timeout)
goto abort;
--
2.19.0.444.g18242da7ef-goog
^ permalink raw reply related
* [PATCH net-next 4/9] tcp: add tcp_wstamp_ns socket field
From: Eric Dumazet @ 2018-09-21 15:51 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Van Jacobson, Neal Cardwell, Yuchung Cheng,
Soheil Hassas Yeganeh, Willem de Bruijn, Eric Dumazet,
Eric Dumazet
In-Reply-To: <20180921155154.49489-1-edumazet@google.com>
TCP will soon provide earliest departure time on TX skbs.
It needs to track this in a new variable.
tcp_mstamp_refresh() needs to update this variable, and
became too big to stay an inline.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/tcp.h | 2 ++
include/net/tcp.h | 12 +-----------
net/ipv4/tcp_output.c | 16 ++++++++++++++++
3 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 263e37271afda18f3d61c99272d34da15dfdca29..848f5b25e178288ce870637b68a692ab88dc7d4d 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -248,6 +248,8 @@ struct tcp_sock {
syn_smc:1; /* SYN includes SMC */
u32 tlp_high_seq; /* snd_nxt at the time of TLP retransmit. */
+ u64 tcp_wstamp_ns; /* departure time for next sent data packet */
+
/* RTT measurement */
u64 tcp_mstamp; /* most recent packet received/sent */
u32 srtt_us; /* smoothed round trip time << 3 in usecs */
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 0ca5ea10dc06f3552597c94de31dcd0c8e0ecc32..370198fdc65d3e863104665e20faefd0e5a09b92 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -752,17 +752,7 @@ static inline u32 tcp_time_stamp_raw(void)
return div_u64(tcp_clock_ns(), NSEC_PER_SEC / TCP_TS_HZ);
}
-
-/* Refresh 1us clock of a TCP socket,
- * ensuring monotically increasing values.
- */
-static inline void tcp_mstamp_refresh(struct tcp_sock *tp)
-{
- u64 val = tcp_clock_us();
-
- if (val > tp->tcp_mstamp)
- tp->tcp_mstamp = val;
-}
+void tcp_mstamp_refresh(struct tcp_sock *tp);
static inline u32 tcp_stamp_us_delta(u64 t1, u64 t0)
{
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index b95aa72d88233dd6376a70ccd7cbb13744444889..5a8105e84f7c1a876bbd15e8050c2574c1fbe162 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -45,6 +45,22 @@
#include <trace/events/tcp.h>
+/* Refresh clocks of a TCP socket,
+ * ensuring monotically increasing values.
+ */
+void tcp_mstamp_refresh(struct tcp_sock *tp)
+{
+ u64 val = tcp_clock_ns();
+
+ /* departure time for next data packet */
+ if (val > tp->tcp_wstamp_ns)
+ tp->tcp_wstamp_ns = val;
+
+ val = div_u64(val, NSEC_PER_USEC);
+ if (val > tp->tcp_mstamp)
+ tp->tcp_mstamp = val;
+}
+
static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
int push_one, gfp_t gfp);
--
2.19.0.444.g18242da7ef-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox