* Re: [RFC bpf-next 06/11] bpf: Add reference tracking to verifier
From: Alexei Starovoitov @ 2018-05-15 3:04 UTC (permalink / raw)
To: Joe Stringer; +Cc: daniel, netdev, ast, john.fastabend, kafai
In-Reply-To: <20180509210709.7201-7-joe@wand.net.nz>
On Wed, May 09, 2018 at 02:07:04PM -0700, Joe Stringer wrote:
> Allow helper functions to acquire a reference and return it into a
> register. Specific pointer types such as the PTR_TO_SOCKET will
> implicitly represent such a reference. The verifier must ensure that
> these references are released exactly once in each path through the
> program.
>
> To achieve this, this commit assigns an id to the pointer and tracks it
> in the 'bpf_func_state', then when the function or program exits,
> verifies that all of the acquired references have been freed. When the
> pointer is passed to a function that frees the reference, it is removed
> from the 'bpf_func_state` and all existing copies of the pointer in
> registers are marked invalid.
>
> Signed-off-by: Joe Stringer <joe@wand.net.nz>
> ---
> include/linux/bpf_verifier.h | 18 ++-
> kernel/bpf/verifier.c | 295 ++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 292 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 9dcd87f1d322..8dbee360b3ec 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -104,6 +104,11 @@ struct bpf_stack_state {
> u8 slot_type[BPF_REG_SIZE];
> };
>
> +struct bpf_reference_state {
> + int id;
> + int insn_idx; /* allocation insn */
the insn_idx is for more verbose messages, right?
It doesn't seem to affect the safety of algorithm.
Please add a comment to clarify that.
> +};
> +
> /* state of the program:
> * type of all registers and stack info
> */
> @@ -122,7 +127,9 @@ struct bpf_func_state {
> */
> u32 subprogno;
>
> - /* should be second to last. See copy_func_state() */
> + /* The following fields should be last. See copy_func_state() */
> + int acquired_refs;
> + struct bpf_reference_state *refs;
> int allocated_stack;
> struct bpf_stack_state *stack;
> };
> @@ -218,11 +225,16 @@ void bpf_verifier_vlog(struct bpf_verifier_log *log, const char *fmt,
> __printf(2, 3) void bpf_verifier_log_write(struct bpf_verifier_env *env,
> const char *fmt, ...);
>
> -static inline struct bpf_reg_state *cur_regs(struct bpf_verifier_env *env)
> +static inline struct bpf_func_state *cur_func(struct bpf_verifier_env *env)
> {
> struct bpf_verifier_state *cur = env->cur_state;
>
> - return cur->frame[cur->curframe]->regs;
> + return cur->frame[cur->curframe];
> +}
> +
> +static inline struct bpf_reg_state *cur_regs(struct bpf_verifier_env *env)
> +{
> + return cur_func(env)->regs;
> }
>
> int bpf_prog_offload_verifier_prep(struct bpf_verifier_env *env);
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index f426ebf2b6bf..92b9a5dc465a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -1,5 +1,6 @@
> /* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
> * Copyright (c) 2016 Facebook
> + * Copyright (c) 2018 Covalent IO, Inc. http://covalent.io
> *
> * This program is free software; you can redistribute it and/or
> * modify it under the terms of version 2 of the GNU General Public
> @@ -140,6 +141,18 @@ static const struct bpf_verifier_ops * const bpf_verifier_ops[] = {
> *
> * After the call R0 is set to return type of the function and registers R1-R5
> * are set to NOT_INIT to indicate that they are no longer readable.
> + *
> + * The following reference types represent a potential reference to a kernel
> + * resource which, after first being allocated, must be checked and freed by
> + * the BPF program:
> + * - PTR_TO_SOCKET_OR_NULL, PTR_TO_SOCKET
> + *
> + * When the verifier sees a helper call return a reference type, it allocates a
> + * pointer id for the reference and stores it in the current function state.
> + * Similar to the way that PTR_TO_MAP_VALUE_OR_NULL is converted into
> + * PTR_TO_MAP_VALUE, PTR_TO_SOCKET_OR_NULL becomes PTR_TO_SOCKET when the type
> + * passes through a NULL-check conditional. For the branch wherein the state is
> + * changed to CONST_IMM, the verifier releases the reference.
> */
>
> /* verifier_state + insn_idx are pushed to stack when branch is encountered */
> @@ -229,7 +242,42 @@ static bool type_is_pkt_pointer(enum bpf_reg_type type)
>
> static bool reg_type_may_be_null(enum bpf_reg_type type)
> {
> - return type == PTR_TO_MAP_VALUE_OR_NULL;
> + return type == PTR_TO_MAP_VALUE_OR_NULL ||
> + type == PTR_TO_SOCKET_OR_NULL;
> +}
> +
> +static bool type_is_refcounted(enum bpf_reg_type type)
> +{
> + return type == PTR_TO_SOCKET;
> +}
> +
> +static bool type_is_refcounted_or_null(enum bpf_reg_type type)
> +{
> + return type == PTR_TO_SOCKET || type == PTR_TO_SOCKET_OR_NULL;
> +}
> +
> +static bool reg_is_refcounted(const struct bpf_reg_state *reg)
> +{
> + return type_is_refcounted(reg->type);
> +}
> +
> +static bool reg_is_refcounted_or_null(const struct bpf_reg_state *reg)
> +{
> + return type_is_refcounted_or_null(reg->type);
> +}
> +
> +static bool arg_type_is_refcounted(enum bpf_arg_type type)
> +{
> + return type == ARG_PTR_TO_SOCKET;
> +}
> +
> +/* Determine whether the function releases some resources allocated by another
> + * function call. The first reference type argument will be assumed to be
> + * released by release_reference().
> + */
> +static bool is_release_function(enum bpf_func_id func_id)
> +{
> + return false;
> }
>
> /* string representation of 'enum bpf_reg_type' */
> @@ -344,6 +392,12 @@ static void print_verifier_state(struct bpf_verifier_env *env,
> if (state->stack[i].slot_type[0] == STACK_ZERO)
> verbose(env, " fp%d=0", (-i - 1) * BPF_REG_SIZE);
> }
> + if (state->acquired_refs && state->refs[0].id) {
> + verbose(env, " refs=%d", state->refs[0].id);
> + for (i = 1; i < state->acquired_refs; i++)
> + if (state->refs[i].id)
> + verbose(env, ",%d", state->refs[i].id);
> + }
> verbose(env, "\n");
> }
>
> @@ -362,6 +416,8 @@ static int copy_##NAME##_state(struct bpf_func_state *dst, \
> sizeof(*src->FIELD) * (src->COUNT / SIZE)); \
> return 0; \
> }
> +/* copy_reference_state() */
> +COPY_STATE_FN(reference, acquired_refs, refs, 1)
> /* copy_stack_state() */
> COPY_STATE_FN(stack, allocated_stack, stack, BPF_REG_SIZE)
> #undef COPY_STATE_FN
> @@ -400,6 +456,8 @@ static int realloc_##NAME##_state(struct bpf_func_state *state, int size, \
> state->FIELD = new_##FIELD; \
> return 0; \
> }
> +/* realloc_reference_state() */
> +REALLOC_STATE_FN(reference, acquired_refs, refs, 1)
> /* realloc_stack_state() */
> REALLOC_STATE_FN(stack, allocated_stack, stack, BPF_REG_SIZE)
> #undef REALLOC_STATE_FN
> @@ -411,16 +469,89 @@ REALLOC_STATE_FN(stack, allocated_stack, stack, BPF_REG_SIZE)
> * 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)
> +static int realloc_func_state(struct bpf_func_state *state, int stack_size,
> + int refs_size, bool copy_old)
> {
> - return realloc_stack_state(state, size, copy_old);
> + int err = realloc_reference_state(state, refs_size, copy_old);
> + if (err)
> + return err;
> + return realloc_stack_state(state, stack_size, copy_old);
> +}
> +
> +/* Acquire a pointer id from the env and update the state->refs to include
> + * this new pointer reference.
> + * On success, returns a valid pointer id to associate with the register
> + * On failure, returns a negative errno.
> + */
> +static int acquire_reference_state(struct bpf_verifier_env *env, int insn_idx)
> +{
> + struct bpf_func_state *state = cur_func(env);
> + int new_ofs = state->acquired_refs;
> + int id, err;
> +
> + err = realloc_reference_state(state, state->acquired_refs + 1, true);
> + if (err)
> + return err;
> + id = ++env->id_gen;
> + state->refs[new_ofs].id = id;
> + state->refs[new_ofs].insn_idx = insn_idx;
I thought that we may avoid this extra 'ref_state' array if we store
'id' into 'aux' array which is one to one to array of instructions
and avoid this expensive reallocs, but then I realized we can go
through the same instruction that returns a pointer to socket
multiple times and every time it needs to be different 'id' and
tracked indepdently, so yeah. All that infra is necessary.
Would be good to document the algorithm a bit more.
> +
> + return id;
> +}
> +
> +/* release function corresponding to acquire_reference_state(). Idempotent. */
> +static int __release_reference_state(struct bpf_func_state *state, int ptr_id)
> +{
> + int i, last_idx;
> +
> + if (!ptr_id)
> + return 0;
> +
> + last_idx = state->acquired_refs - 1;
> + for (i = 0; i < state->acquired_refs; i++) {
> + if (state->refs[i].id == ptr_id) {
> + if (last_idx && i != last_idx)
> + memcpy(&state->refs[i], &state->refs[last_idx],
> + sizeof(*state->refs));
> + memset(&state->refs[last_idx], 0, sizeof(*state->refs));
> + state->acquired_refs--;
> + return 0;
> + }
> + }
> + return -EFAULT;
> +}
> +
> +/* variation on the above for cases where we expect that there must be an
> + * outstanding reference for the specified ptr_id.
> + */
> +static int release_reference_state(struct bpf_verifier_env *env, int ptr_id)
> +{
> + struct bpf_func_state *state = cur_func(env);
> + int err;
> +
> + err = __release_reference_state(state, ptr_id);
> + if (WARN_ON_ONCE(err != 0))
> + verbose(env, "verifier internal error: can't release reference\n");
> + return err;
> +}
> +
> +static int transfer_reference_state(struct bpf_func_state *dst,
> + struct bpf_func_state *src)
> +{
> + int err = realloc_reference_state(dst, src->acquired_refs, false);
> + if (err)
> + return err;
> + err = copy_reference_state(dst, src);
> + if (err)
> + return err;
> + return 0;
> }
>
> static void free_func_state(struct bpf_func_state *state)
> {
> if (!state)
> return;
> + kfree(state->refs);
> kfree(state->stack);
> kfree(state);
> }
> @@ -446,10 +577,14 @@ static int copy_func_state(struct bpf_func_state *dst,
> {
> int err;
>
> - err = realloc_func_state(dst, src->allocated_stack, false);
> + err = realloc_func_state(dst, src->allocated_stack, src->acquired_refs,
> + false);
> + if (err)
> + return err;
> + memcpy(dst, src, offsetof(struct bpf_func_state, acquired_refs));
> + err = copy_reference_state(dst, src);
> if (err)
> return err;
> - memcpy(dst, src, offsetof(struct bpf_func_state, allocated_stack));
> return copy_stack_state(dst, src);
> }
>
> @@ -1019,7 +1154,7 @@ static int check_stack_write(struct bpf_verifier_env *env,
> enum bpf_reg_type type;
>
> err = realloc_func_state(state, round_up(slot + 1, BPF_REG_SIZE),
> - true);
> + state->acquired_refs, true);
> if (err)
> return err;
> /* caller checked that off % size == 0 and -MAX_BPF_STACK <= off < 0,
> @@ -2259,10 +2394,32 @@ static bool check_arg_pair_ok(const struct bpf_func_proto *fn)
> return true;
> }
>
> +static bool check_refcount_ok(const struct bpf_func_proto *fn)
> +{
> + int count = 0;
> +
> + if (arg_type_is_refcounted(fn->arg1_type))
> + count++;
> + if (arg_type_is_refcounted(fn->arg2_type))
> + count++;
> + if (arg_type_is_refcounted(fn->arg3_type))
> + count++;
> + if (arg_type_is_refcounted(fn->arg4_type))
> + count++;
> + if (arg_type_is_refcounted(fn->arg5_type))
> + count++;
> +
> + /* We only support one arg being unreferenced at the moment,
> + * which is sufficient for the helper functions we have right now.
> + */
> + return count <= 1;
> +}
> +
> static int check_func_proto(const struct bpf_func_proto *fn)
> {
> return check_raw_mode_ok(fn) &&
> - check_arg_pair_ok(fn) ? 0 : -EINVAL;
> + check_arg_pair_ok(fn) &&
> + check_refcount_ok(fn) ? 0 : -EINVAL;
> }
>
> /* Packet data might have moved, any old PTR_TO_PACKET[_META,_END]
> @@ -2295,12 +2452,57 @@ static void clear_all_pkt_pointers(struct bpf_verifier_env *env)
> __clear_all_pkt_pointers(env, vstate->frame[i]);
> }
>
> +static void release_reg_references(struct bpf_verifier_env *env,
> + struct bpf_func_state *state, int id)
> +{
> + struct bpf_reg_state *regs = state->regs, *reg;
> + int i;
> +
> + for (i = 0; i < MAX_BPF_REG; i++)
> + if (regs[i].id == id)
> + mark_reg_unknown(env, regs, i);
> +
> + for_each_spilled_reg(i, state, reg) {
> + if (!reg)
> + continue;
> + if (reg_is_refcounted(reg) && reg->id == id)
> + __mark_reg_unknown(reg);
> + }
> +}
> +
> +/* The pointer with the specified id has released its reference to kernel
> + * resources. Identify all copies of the same pointer and clear the reference.
> + */
> +static int release_reference(struct bpf_verifier_env *env)
> +{
> + struct bpf_verifier_state *vstate = env->cur_state;
> + struct bpf_reg_state *regs = cur_regs(env);
> + int i, ptr_id = 0;
> +
> + for (i = BPF_REG_1; i < BPF_REG_6; i++) {
> + if (reg_is_refcounted(®s[i])) {
> + ptr_id = regs[i].id;
> + break;
> + }
> + }
> + if (WARN_ON_ONCE(!ptr_id)) {
> + /* references must be special pointer types that are checked
> + * against argument requirements for the release function. */
> + verbose(env, "verifier internal error: can't locate refcounted arg\n");
> + return -EFAULT;
> + }
> + for (i = 0; i <= vstate->curframe; i++)
> + release_reg_references(env, vstate->frame[i], ptr_id);
> +
> + return release_reference_state(env, ptr_id);
> +}
> +
> static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> int *insn_idx)
> {
> struct bpf_verifier_state *state = env->cur_state;
> struct bpf_func_state *caller, *callee;
> - int i, subprog, target_insn;
> + int i, err, subprog, target_insn;
>
> if (state->curframe + 1 >= MAX_CALL_FRAMES) {
> verbose(env, "the call stack of %d frames is too deep\n",
> @@ -2338,6 +2540,11 @@ 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 */);
>
> + /* Transfer references to the callee */
> + err = transfer_reference_state(callee, caller);
> + if (err)
> + return err;
> +
> /* copy r1 - r5 args that callee can access */
> for (i = BPF_REG_1; i <= BPF_REG_5; i++)
> callee->regs[i] = caller->regs[i];
> @@ -2368,6 +2575,7 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
> struct bpf_verifier_state *state = env->cur_state;
> struct bpf_func_state *caller, *callee;
> struct bpf_reg_state *r0;
> + int err;
>
> callee = state->frame[state->curframe];
> r0 = &callee->regs[BPF_REG_0];
> @@ -2387,6 +2595,11 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
> /* return to the caller whatever r0 had in the callee */
> caller->regs[BPF_REG_0] = *r0;
>
> + /* Transfer references to the caller */
> + err = transfer_reference_state(caller, callee);
> + if (err)
> + return err;
> +
> *insn_idx = callee->callsite + 1;
> if (env->log.level) {
> verbose(env, "returning from callee:\n");
> @@ -2498,6 +2711,15 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
> return err;
> }
>
> + /* If the function is a release() function, mark all copies of the same
> + * pointer as "freed" in all registers and in the stack.
> + */
> + if (is_release_function(func_id)) {
> + err = release_reference(env);
I think this can be improved if check_func_arg() stores ptr_id into meta.
Then this loop
for (i = BPF_REG_1; i < BPF_REG_6; i++) {
if (reg_is_refcounted(®s[i])) {
in release_reference() won't be needed.
Also the macros from the previous patch look ugly, but considering this patch
I guess it's justified. At least I don't see a better way of doing it.
^ permalink raw reply
* Re: [PATCH net-next v3 0/8] sctp: refactor sctp_outq_flush
From: David Miller @ 2018-05-15 2:57 UTC (permalink / raw)
To: marcelo.leitner; +Cc: netdev, linux-sctp, nhorman, lucien.xin, vyasevich
In-Reply-To: <cover.1526318522.git.marcelo.leitner@gmail.com>
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Mon, 14 May 2018 14:34:35 -0300
> Currently sctp_outq_flush does many different things and arguably
> unrelated, such as doing transport selection and outq dequeueing.
>
> This patchset refactors it into smaller and more dedicated functions.
> The end behavior should be the same.
>
> The next patchset will rework the function parameters.
>
> Changes since v1:
> - fix build issues on patches 3 and 4, and updated 5 and 8 because of
> it.
>
> Changes since v2:
> - fixed panic if building with just up to patch 3 applied
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH] lib/rhashtable: reorder some inititalization sequences
From: David Miller @ 2018-05-15 2:52 UTC (permalink / raw)
To: dave; +Cc: akpm, tgraf, herbert, netdev, linux-kernel, dbueso
In-Reply-To: <20180514151332.31352-1-dave@stgolabs.net>
From: Davidlohr Bueso <dave@stgolabs.net>
Date: Mon, 14 May 2018 08:13:32 -0700
> rhashtable_init() allocates memory at the very end of the
> call, once everything is setup; with the exception of the
> nelems parameter. However, unless the user is doing something
> bogus with params for which -EINVAL is returned, memory
> allocation is the only operation that can trigger the call
> to fail.
>
> Thus move bucket_table_alloc() up such that we fail back to
> the caller asap, instead of doing useless checks. This is
> safe as the the table allocation isn't using the halfly
> setup 'ht' structure and bucket_table_alloc() call chain only
> ends up using the ht->nulls_base member in INIT_RHT_NULLS_HEAD.
>
> Also move the locking initialization down to the end.
>
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
The user potentially "doing something bogus" is why the most
expensive part of the initialization (the memory allocation)
is done after everything else is validated.
I think it's best to keep things as-is.
^ permalink raw reply
* [PATCH RFC net-next 7/7] selftests: fib_tests: Add ipv4 route add append replace tests
From: David Ahern @ 2018-05-15 2:51 UTC (permalink / raw)
To: netdev; +Cc: Thomas.Winter, idosch, sharpd, roopa, David Ahern
In-Reply-To: <20180515025112.16647-1-dsahern@gmail.com>
Add IPv4 route tests covering add, append and replace permutations.
Assumes the ability to add a basic single path route works; this is
required for example when adding an address to an interface.
$ fib_tests.sh -t ipv4_rt
IPv4 route add / append tests
TEST: Attempt to add duplicate route - gw [ OK ]
TEST: Attempt to add duplicate route - dev only [ OK ]
TEST: Attempt to add duplicate route - reject route [ OK ]
TEST: Add new nexthop for existing prefix [ OK ]
TEST: Append leg to existing route - gw [ OK ]
TEST: Append leg to existing route - dev only [ OK ]
TEST: Append leg to existing route - reject route [ OK ]
TEST: Append leg to existing reject route - gw [ OK ]
TEST: Append leg to existing reject route - dev only [ OK ]
TEST: add multipath route [ OK ]
TEST: Attempt to add duplicate multipath route [ OK ]
TEST: route add with different metrics [ OK ]
TEST: route delete with metric [ OK ]
IPv4 route replace tests
TEST: single path with single path [ OK ]
TEST: single path with multipath [ OK ]
TEST: single path with reject route [ OK ]
TEST: single path with single path via multipath attribute [ OK ]
TEST: invalid nexthop [ OK ]
TEST: single path - replace of non-existent route [ OK ]
TEST: multipath with multipath [ OK ]
TEST: multipath with single path [ OK ]
TEST: multipath with single path via multipath attribute [ OK ]
TEST: multipath with reject route [ OK ]
TEST: multipath - invalid first nexthop [ OK ]
TEST: multipath - invalid second nexthop [ OK ]
TEST: multipath - replace of non-existent route [ OK ]
Signed-off-by: David Ahern <dsahern@gmail.com>
---
tools/testing/selftests/net/fib_tests.sh | 277 ++++++++++++++++++++++++++++++-
1 file changed, 276 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 20cb1d2c4e72..19248453a2ba 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -6,7 +6,7 @@
ret=0
-TESTS="unregister down carrier nexthop ipv6_rt"
+TESTS="unregister down carrier nexthop ipv6_rt ipv4_rt"
VERBOSE=0
PAUSE_ON_FAIL=no
PAUSE=no
@@ -702,6 +702,12 @@ route_setup()
$IP -6 addr add 2001:db8:103::2/64 dev veth4
$IP -6 addr add 2001:db8:104::1/64 dev dummy1
+ $IP addr add 172.16.101.1/24 dev veth1
+ $IP addr add 172.16.101.2/24 dev veth2
+ $IP addr add 172.16.103.1/24 dev veth3
+ $IP addr add 172.16.103.2/24 dev veth4
+ $IP addr add 172.16.104.1/24 dev dummy1
+
set +ex
}
@@ -901,6 +907,274 @@ ipv6_route_test()
route_cleanup
}
+# add route for a prefix, flushing any existing routes first
+# expected to be the first step of a test
+add_route()
+{
+ local pfx="$1"
+ local nh="$2"
+ local out
+
+ if [ "$VERBOSE" = "1" ]; then
+ echo
+ echo " ##################################################"
+ echo
+ fi
+
+ run_cmd "$IP ro flush ${pfx}"
+ [ $? -ne 0 ] && exit 1
+
+ out=$($IP ro ls match ${pfx})
+ if [ -n "$out" ]; then
+ echo "Failed to flush routes for prefix used for tests."
+ exit 1
+ fi
+
+ run_cmd "$IP ro add ${pfx} ${nh}"
+ if [ $? -ne 0 ]; then
+ echo "Failed to add initial route for test."
+ exit 1
+ fi
+}
+
+# add initial route - used in replace route tests
+add_initial_route()
+{
+ add_route "172.16.104.0/24" "$1"
+}
+
+check_route()
+{
+ local pfx="172.16.104.0/24"
+ local expected="$1"
+ local out
+ local rc=0
+
+ out=$($IP ro ls match ${pfx})
+ if [ -z "${out}" ]; then
+ if [ "$VERBOSE" = "1" ]; then
+ printf "\nNo route entry found\n"
+ printf "Expected:\n"
+ printf " ${expected}\n"
+ fi
+ return 1
+ fi
+
+ # tricky way to convert output to 1-line without ip's
+ # messy '\'; this drops all extra white space
+ out=$(echo ${out})
+ if [ "${out}" != "${expected}" ]; then
+ rc=1
+ if [ "${VERBOSE}" = "1" ]; then
+ printf " Unexpected route entry. Have:\n"
+ printf " ${out}\n"
+ printf " Expected:\n"
+ printf " ${expected}\n\n"
+ fi
+ fi
+
+ return $rc
+}
+
+# assumption is that basic add of a single path route works
+# otherwise just adding an address on an interface is broken
+ipv4_rt_add()
+{
+ local rc
+
+ echo
+ echo "IPv4 route add / append tests"
+
+ # route add same prefix - fails with EEXISTS b/c ip adds NLM_F_EXCL
+ add_route "172.16.104.0/24" "via 172.16.101.2"
+ run_cmd "$IP ro add 172.16.104.0/24 via 172.16.103.2"
+ log_test $? 2 "Attempt to add duplicate route - gw"
+
+ # route add same prefix - fails with EEXISTS b/c ip adds NLM_F_EXCL
+ add_route "172.16.104.0/24" "via 172.16.101.2"
+ run_cmd "$IP ro add 172.16.104.0/24 dev veth3"
+ log_test $? 2 "Attempt to add duplicate route - dev only"
+
+ # route add same prefix - fails with EEXISTS b/c ip adds NLM_F_EXCL
+ add_route "172.16.104.0/24" "via 172.16.101.2"
+ run_cmd "$IP ro add unreachable 172.16.104.0/24"
+ log_test $? 2 "Attempt to add duplicate route - reject route"
+
+ # iproute2 prepend only sets NLM_F_CREATE
+ # - adds a new route; does NOT convert existing route to ECMP
+ add_route "172.16.104.0/24" "via 172.16.101.2"
+ run_cmd "$IP ro prepend 172.16.104.0/24 via 172.16.103.2"
+ check_route "172.16.104.0/24 via 172.16.103.2 dev veth3 172.16.104.0/24 via 172.16.101.2 dev veth1"
+ log_test $? 0 "Add new nexthop for existing prefix"
+
+ # route append with same prefix adds a new route
+ # - iproute2 sets NLM_F_CREATE | NLM_F_APPEND
+ add_route "172.16.104.0/24" "via 172.16.101.2"
+ run_cmd "$IP ro append 172.16.104.0/24 via 172.16.103.2"
+ check_route "172.16.104.0/24 via 172.16.101.2 dev veth1 172.16.104.0/24 via 172.16.103.2 dev veth3"
+ log_test $? 0 "Append leg to existing route - gw"
+
+ add_route "172.16.104.0/24" "via 172.16.101.2"
+ run_cmd "$IP ro append 172.16.104.0/24 dev veth3"
+ check_route "172.16.104.0/24 via 172.16.101.2 dev veth1 172.16.104.0/24 dev veth3 scope link"
+ log_test $? 0 "Append leg to existing route - dev only"
+
+ add_route "172.16.104.0/24" "via 172.16.101.2"
+ run_cmd "$IP ro append unreachable 172.16.104.0/24"
+ check_route "172.16.104.0/24 via 172.16.101.2 dev veth1 unreachable 172.16.104.0/24"
+ log_test $? 0 "Append leg to existing route - reject route"
+
+ run_cmd "$IP ro flush 172.16.104.0/24"
+ run_cmd "$IP ro add unreachable 172.16.104.0/24"
+ run_cmd "$IP ro append 172.16.104.0/24 via 172.16.103.2"
+ check_route "unreachable 172.16.104.0/24 172.16.104.0/24 via 172.16.103.2 dev veth3"
+ log_test $? 0 "Append leg to existing reject route - gw"
+
+ run_cmd "$IP ro flush 172.16.104.0/24"
+ run_cmd "$IP ro add unreachable 172.16.104.0/24"
+ run_cmd "$IP ro append 172.16.104.0/24 dev veth3"
+ check_route "unreachable 172.16.104.0/24 172.16.104.0/24 dev veth3 scope link"
+ log_test $? 0 "Append leg to existing reject route - dev only"
+
+ # insert mpath directly
+ add_route "172.16.104.0/24" "nexthop via 172.16.101.2 nexthop via 172.16.103.2"
+ check_route "172.16.104.0/24 nexthop via 172.16.101.2 dev veth1 weight 1 nexthop via 172.16.103.2 dev veth3 weight 1"
+ log_test $? 0 "add multipath route"
+
+ add_route "172.16.104.0/24" "nexthop via 172.16.101.2 nexthop via 172.16.103.2"
+ run_cmd "$IP ro add 172.16.104.0/24 nexthop via 172.16.101.2 nexthop via 172.16.103.2"
+ log_test $? 2 "Attempt to add duplicate multipath route"
+
+ # insert of a second route without append but different metric
+ add_route "172.16.104.0/24" "via 172.16.101.2"
+ run_cmd "$IP ro add 172.16.104.0/24 via 172.16.103.2 metric 512"
+ rc=$?
+ if [ $rc -eq 0 ]; then
+ run_cmd "$IP ro add 172.16.104.0/24 via 172.16.103.3 metric 256"
+ rc=$?
+ fi
+ log_test $rc 0 "route add with different metrics"
+
+ run_cmd "$IP ro del 172.16.104.0/24 metric 512"
+ rc=$?
+ if [ $rc -eq 0 ]; then
+ check_route "172.16.104.0/24 via 172.16.101.2 dev veth1 172.16.104.0/24 via 172.16.103.3 dev veth3 metric 256"
+ rc=$?
+ fi
+ log_test $rc 0 "route delete with metric"
+}
+
+ipv4_rt_replace_single()
+{
+ # single path with single path
+ #
+ add_initial_route "via 172.16.101.2"
+ run_cmd "$IP ro replace 172.16.104.0/24 via 172.16.103.2"
+ check_route "172.16.104.0/24 via 172.16.103.2 dev veth3"
+ log_test $? 0 "single path with single path"
+
+ # single path with multipath
+ #
+ add_initial_route "nexthop via 172.16.101.2"
+ run_cmd "$IP ro replace 172.16.104.0/24 nexthop via 172.16.101.3 nexthop via 172.16.103.2"
+ check_route "172.16.104.0/24 nexthop via 172.16.101.3 dev veth1 weight 1 nexthop via 172.16.103.2 dev veth3 weight 1"
+ log_test $? 0 "single path with multipath"
+
+ # single path with reject
+ #
+ add_initial_route "nexthop via 172.16.101.2"
+ run_cmd "$IP ro replace unreachable 172.16.104.0/24"
+ check_route "unreachable 172.16.104.0/24"
+ log_test $? 0 "single path with reject route"
+
+ # single path with single path using MULTIPATH attribute
+ #
+ add_initial_route "via 172.16.101.2"
+ run_cmd "$IP ro replace 172.16.104.0/24 nexthop via 172.16.103.2"
+ check_route "172.16.104.0/24 via 172.16.103.2 dev veth3"
+ log_test $? 0 "single path with single path via multipath attribute"
+
+ # route replace fails - invalid nexthop
+ add_initial_route "via 172.16.101.2"
+ run_cmd "$IP ro replace 172.16.104.0/24 via 2001:db8:104::2"
+ if [ $? -eq 0 ]; then
+ log_test 0 1 "invalid nexthop"
+ else
+ check_route "172.16.104.0/24 via 172.16.101.2 dev veth1"
+ log_test $? 0 "invalid nexthop"
+ fi
+
+ # replace non-existent route
+ # - note use of change versus replace since ip adds NLM_F_CREATE
+ # for replace
+ add_initial_route "via 172.16.101.2"
+ run_cmd "$IP ro change 172.16.105.0/24 via 172.16.101.2"
+ log_test $? 2 "single path - replace of non-existent route"
+}
+
+ipv4_rt_replace_mpath()
+{
+ # multipath with multipath
+ add_initial_route "nexthop via 172.16.101.2 nexthop via 172.16.103.2"
+ run_cmd "$IP ro replace 172.16.104.0/24 nexthop via 172.16.101.3 nexthop via 172.16.103.3"
+ check_route "172.16.104.0/24 nexthop via 172.16.101.3 dev veth1 weight 1 nexthop via 172.16.103.3 dev veth3 weight 1"
+ log_test $? 0 "multipath with multipath"
+
+ # multipath with single
+ add_initial_route "nexthop via 172.16.101.2 nexthop via 172.16.103.2"
+ run_cmd "$IP ro replace 172.16.104.0/24 via 172.16.101.3"
+ check_route "172.16.104.0/24 via 172.16.101.3 dev veth1"
+ log_test $? 0 "multipath with single path"
+
+ # multipath with single
+ add_initial_route "nexthop via 172.16.101.2 nexthop via 172.16.103.2"
+ run_cmd "$IP ro replace 172.16.104.0/24 nexthop via 172.16.101.3"
+ check_route "172.16.104.0/24 via 172.16.101.3 dev veth1"
+ log_test $? 0 "multipath with single path via multipath attribute"
+
+ # multipath with reject
+ add_initial_route "nexthop via 172.16.101.2 nexthop via 172.16.103.2"
+ run_cmd "$IP ro replace unreachable 172.16.104.0/24"
+ check_route "unreachable 172.16.104.0/24"
+ log_test $? 0 "multipath with reject route"
+
+ # route replace fails - invalid nexthop 1
+ add_initial_route "nexthop via 172.16.101.2 nexthop via 172.16.103.2"
+ run_cmd "$IP ro replace 172.16.104.0/24 nexthop via 172.16.111.3 nexthop via 172.16.103.3"
+ check_route "172.16.104.0/24 nexthop via 172.16.101.2 dev veth1 weight 1 nexthop via 172.16.103.2 dev veth3 weight 1"
+ log_test $? 0 "multipath - invalid first nexthop"
+
+ # route replace fails - invalid nexthop 2
+ add_initial_route "nexthop via 172.16.101.2 nexthop via 172.16.103.2"
+ run_cmd "$IP ro replace 172.16.104.0/24 nexthop via 172.16.101.3 nexthop via 172.16.113.3"
+ check_route "172.16.104.0/24 nexthop via 172.16.101.2 dev veth1 weight 1 nexthop via 172.16.103.2 dev veth3 weight 1"
+ log_test $? 0 "multipath - invalid second nexthop"
+
+ # multipath non-existent route
+ add_initial_route "nexthop via 172.16.101.2 nexthop via 172.16.103.2"
+ run_cmd "$IP ro change 172.16.105.0/24 nexthop via 172.16.101.3 nexthop via 172.16.103.3"
+ log_test $? 2 "multipath - replace of non-existent route"
+}
+
+ipv4_rt_replace()
+{
+ echo
+ echo "IPv4 route replace tests"
+
+ ipv4_rt_replace_single
+ ipv4_rt_replace_mpath
+}
+
+ipv4_route_test()
+{
+ route_setup
+
+ ipv4_rt_add
+ ipv4_rt_replace
+
+ route_cleanup
+}
+
################################################################################
# usage
@@ -964,6 +1238,7 @@ do
fib_carrier_test|carrier) fib_carrier_test;;
fib_nexthop_test|nexthop) fib_nexthop_test;;
ipv6_route_test|ipv6_rt) ipv6_route_test;;
+ ipv4_route_test|ipv4_rt) ipv4_route_test;;
help) echo "Test names: $TESTS"; exit 0;;
esac
--
2.11.0
^ permalink raw reply related
* [PATCH RFC net-next 6/7] selftests: fib_tests: Add ipv6 route add append replace tests
From: David Ahern @ 2018-05-15 2:51 UTC (permalink / raw)
To: netdev; +Cc: Thomas.Winter, idosch, sharpd, roopa, David Ahern
In-Reply-To: <20180515025112.16647-1-dsahern@gmail.com>
Add IPv6 route tests covering add, append and replace permutations.
Assumes the ability to add a basic single path route works; this is
required for example when adding an address to an interface.
$ fib_tests.sh -t ipv6_rt
IPv6 route add / append tests
TEST: Attempt to add duplicate route - gw [ OK ]
TEST: Attempt to add duplicate route - dev only [ OK ]
TEST: Attempt to add duplicate route - reject route [ OK ]
TEST: Add new nexthop for existing prefix [ OK ]
TEST: Append leg to existing route - gw [ OK ]
TEST: Append leg to existing route - dev only [ OK ]
TEST: Append leg to existing route - reject route [ OK ]
TEST: Append leg to existing reject route - gw [ OK ]
TEST: Append leg to existing reject route - dev only [ OK ]
TEST: add multipath route [ OK ]
TEST: Attempt to add duplicate multipath route [ OK ]
TEST: route add with different metrics [ OK ]
TEST: route delete with metric [ OK ]
IPv6 route replace tests
TEST: single path with single path [ OK ]
TEST: single path with multipath [ OK ]
TEST: single path with reject route [ OK ]
TEST: single path with single path via multipath attribute [ OK ]
TEST: invalid nexthop [ OK ]
TEST: single path - replace of non-existent route [ OK ]
TEST: multipath with multipath [ OK ]
TEST: multipath with single path [ OK ]
TEST: multipath with single path via multipath attribute [ OK ]
TEST: multipath with reject route [ OK ]
TEST: multipath - invalid first nexthop [ OK ]
TEST: multipath - invalid second nexthop [ OK ]
TEST: multipath - replace of non-existent route [ OK ]
Signed-off-by: David Ahern <dsahern@gmail.com>
---
tools/testing/selftests/net/fib_tests.sh | 331 ++++++++++++++++++++++++++++++-
1 file changed, 330 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 12b648826151..20cb1d2c4e72 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -6,7 +6,7 @@
ret=0
-TESTS="unregister down carrier nexthop"
+TESTS="unregister down carrier nexthop ipv6_rt"
VERBOSE=0
PAUSE_ON_FAIL=no
PAUSE=no
@@ -574,6 +574,334 @@ fib_nexthop_test()
}
################################################################################
+# Tests on route add and replace
+
+run_cmd()
+{
+ local cmd="$1"
+ local out
+ local stderr="2>/dev/null"
+
+ if [ "$VERBOSE" = "1" ]; then
+ printf " COMMAND: $cmd\n"
+ stderr=
+ fi
+
+ out=$(eval $cmd $stderr)
+ rc=$?
+ if [ "$VERBOSE" = "1" -a -n "$out" ]; then
+ echo " $out"
+ fi
+
+ [ "$VERBOSE" = "1" ] && echo
+
+ return $rc
+}
+
+# add route for a prefix, flushing any existing routes first
+# expected to be the first step of a test
+add_route6()
+{
+ local pfx="$1"
+ local nh="$2"
+ local out
+
+ if [ "$VERBOSE" = "1" ]; then
+ echo
+ echo " ##################################################"
+ echo
+ fi
+
+ run_cmd "$IP -6 ro flush ${pfx}"
+ [ $? -ne 0 ] && exit 1
+
+ out=$($IP -6 ro ls match ${pfx})
+ if [ -n "$out" ]; then
+ echo "Failed to flush routes for prefix used for tests."
+ exit 1
+ fi
+
+ run_cmd "$IP -6 ro add ${pfx} ${nh}"
+ if [ $? -ne 0 ]; then
+ echo "Failed to add initial route for test."
+ exit 1
+ fi
+}
+
+# add initial route - used in replace route tests
+add_initial_route6()
+{
+ add_route6 "2001:db8:104::/64" "$1"
+}
+
+check_route6()
+{
+ local pfx="2001:db8:104::/64"
+ local expected="$1"
+ local out
+ local rc=0
+
+ out=$($IP -6 ro ls match ${pfx} | sed -e 's/ pref medium//')
+ if [ -z "${out}" ]; then
+ if [ "$VERBOSE" = "1" ]; then
+ printf "\nNo route entry found\n"
+ printf "Expected:\n"
+ printf " ${expected}\n"
+ fi
+ return 1
+ fi
+
+ # tricky way to convert output to 1-line without ip's
+ # messy '\'; this drops all extra white space
+ out=$(echo ${out})
+ if [ "${out}" != "${expected}" ]; then
+ rc=1
+ if [ "${VERBOSE}" = "1" ]; then
+ printf " Unexpected route entry. Have:\n"
+ printf " ${out}\n"
+ printf " Expected:\n"
+ printf " ${expected}\n\n"
+ fi
+ fi
+
+ return $rc
+}
+
+route_cleanup()
+{
+ $IP li del red 2>/dev/null
+ $IP li del dummy1 2>/dev/null
+ $IP li del veth1 2>/dev/null
+ $IP li del veth3 2>/dev/null
+
+ cleanup &> /dev/null
+}
+
+route_setup()
+{
+ route_cleanup
+ setup
+
+ [ "${VERBOSE}" = "1" ] && set -x
+ set -e
+
+ $IP li add red up type vrf table 101
+ $IP li add veth1 type veth peer name veth2
+ $IP li add veth3 type veth peer name veth4
+
+ $IP li set veth1 up
+ $IP li set veth3 up
+ $IP li set veth2 vrf red up
+ $IP li set veth4 vrf red up
+ $IP li add dummy1 type dummy
+ $IP li set dummy1 vrf red up
+
+ $IP -6 addr add 2001:db8:101::1/64 dev veth1
+ $IP -6 addr add 2001:db8:101::2/64 dev veth2
+ $IP -6 addr add 2001:db8:103::1/64 dev veth3
+ $IP -6 addr add 2001:db8:103::2/64 dev veth4
+ $IP -6 addr add 2001:db8:104::1/64 dev dummy1
+
+ set +ex
+}
+
+# assumption is that basic add of a single path route works
+# otherwise just adding an address on an interface is broken
+ipv6_rt_add()
+{
+ local rc
+
+ echo
+ echo "IPv6 route add / append tests"
+
+ # route add same prefix - fails with EEXISTS b/c ip adds NLM_F_EXCL
+ add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
+ run_cmd "$IP -6 ro add 2001:db8:104::/64 via 2001:db8:103::2"
+ log_test $? 2 "Attempt to add duplicate route - gw"
+
+ # route add same prefix - fails with EEXISTS b/c ip adds NLM_F_EXCL
+ add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
+ run_cmd "$IP -6 ro add 2001:db8:104::/64 dev veth3"
+ log_test $? 2 "Attempt to add duplicate route - dev only"
+
+ # route add same prefix - fails with EEXISTS b/c ip adds NLM_F_EXCL
+ add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
+ run_cmd "$IP -6 ro add unreachable 2001:db8:104::/64"
+ log_test $? 2 "Attempt to add duplicate route - reject route"
+
+ # iproute2 prepend only sets NLM_F_CREATE
+ # - adds a new route; does NOT convert existing route to ECMP
+ add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
+ run_cmd "$IP -6 ro prepend 2001:db8:104::/64 via 2001:db8:103::2"
+ check_route6 "2001:db8:104::/64 via 2001:db8:101::2 dev veth1 metric 1024 2001:db8:104::/64 via 2001:db8:103::2 dev veth3 metric 1024"
+ log_test $? 0 "Add new nexthop for existing prefix"
+
+ # route append with same prefix adds a new route
+ # - iproute2 sets NLM_F_CREATE | NLM_F_APPEND
+ add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
+ run_cmd "$IP -6 ro append 2001:db8:104::/64 via 2001:db8:103::2"
+ check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::2 dev veth1 weight 1 nexthop via 2001:db8:103::2 dev veth3 weight 1"
+ log_test $? 0 "Append leg to existing route - gw"
+
+ add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
+ run_cmd "$IP -6 ro append 2001:db8:104::/64 dev veth3"
+ check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::2 dev veth1 weight 1 nexthop dev veth3 weight 1"
+ log_test $? 0 "Append leg to existing route - dev only"
+
+ add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
+ run_cmd "$IP -6 ro append unreachable 2001:db8:104::/64"
+ log_test $? 2 "Append leg to existing route - reject route"
+
+ run_cmd "$IP -6 ro flush 2001:db8:104::/64"
+ run_cmd "$IP -6 ro add unreachable 2001:db8:104::/64"
+ run_cmd "$IP -6 ro append 2001:db8:104::/64 via 2001:db8:103::2"
+ log_test $? 2 "Append leg to existing reject route - gw"
+
+ run_cmd "$IP -6 ro flush 2001:db8:104::/64"
+ run_cmd "$IP -6 ro add unreachable 2001:db8:104::/64"
+ run_cmd "$IP -6 ro append 2001:db8:104::/64 dev veth3"
+ log_test $? 2 "Append leg to existing reject route - dev only"
+
+ # insert mpath directly
+ add_route6 "2001:db8:104::/64" "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
+ check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::2 dev veth1 weight 1 nexthop via 2001:db8:103::2 dev veth3 weight 1"
+ log_test $? 0 "add multipath route"
+
+ add_route6 "2001:db8:104::/64" "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
+ run_cmd "$IP -6 ro add 2001:db8:104::/64 nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
+ log_test $? 2 "Attempt to add duplicate multipath route"
+
+ # insert of a second route without append but different metric
+ add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
+ run_cmd "$IP -6 ro add 2001:db8:104::/64 via 2001:db8:103::2 metric 512"
+ rc=$?
+ if [ $rc -eq 0 ]; then
+ run_cmd "$IP -6 ro add 2001:db8:104::/64 via 2001:db8:103::3 metric 256"
+ rc=$?
+ fi
+ log_test $rc 0 "route add with different metrics"
+
+ run_cmd "$IP -6 ro del 2001:db8:104::/64 metric 512"
+ rc=$?
+ if [ $rc -eq 0 ]; then
+ check_route6 "2001:db8:104::/64 via 2001:db8:103::3 dev veth3 metric 256 2001:db8:104::/64 via 2001:db8:101::2 dev veth1 metric 1024"
+ rc=$?
+ fi
+ log_test $rc 0 "route delete with metric"
+}
+
+ipv6_rt_replace_single()
+{
+ # single path with single path
+ #
+ add_initial_route6 "via 2001:db8:101::2"
+ run_cmd "$IP -6 ro replace 2001:db8:104::/64 via 2001:db8:103::2"
+ check_route6 "2001:db8:104::/64 via 2001:db8:103::2 dev veth3 metric 1024"
+ log_test $? 0 "single path with single path"
+
+ # single path with multipath
+ #
+ add_initial_route6 "nexthop via 2001:db8:101::2"
+ run_cmd "$IP -6 ro replace 2001:db8:104::/64 nexthop via 2001:db8:101::3 nexthop via 2001:db8:103::2"
+ check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::3 dev veth1 weight 1 nexthop via 2001:db8:103::2 dev veth3 weight 1"
+ log_test $? 0 "single path with multipath"
+
+ # single path with reject
+ #
+ add_initial_route6 "nexthop via 2001:db8:101::2"
+ run_cmd "$IP -6 ro replace unreachable 2001:db8:104::/64"
+ check_route6 "unreachable 2001:db8:104::/64 dev lo metric 1024"
+ log_test $? 0 "single path with reject route"
+
+ # single path with single path using MULTIPATH attribute
+ #
+ add_initial_route6 "via 2001:db8:101::2"
+ run_cmd "$IP -6 ro replace 2001:db8:104::/64 nexthop via 2001:db8:103::2"
+ check_route6 "2001:db8:104::/64 via 2001:db8:103::2 dev veth3 metric 1024"
+ log_test $? 0 "single path with single path via multipath attribute"
+
+ # route replace fails - invalid nexthop
+ add_initial_route6 "via 2001:db8:101::2"
+ run_cmd "$IP -6 ro replace 2001:db8:104::/64 via 2001:db8:104::2"
+ if [ $? -eq 0 ]; then
+ log_test 0 1 "invalid nexthop"
+ else
+ check_route6 "2001:db8:104::/64 via 2001:db8:101::2 dev veth1 metric 1024"
+ log_test $? 0 "invalid nexthop"
+ fi
+
+ # replace non-existent route
+ # - note use of change versus replace since ip adds NLM_F_CREATE
+ # for replace
+ add_initial_route6 "via 2001:db8:101::2"
+ run_cmd "$IP -6 ro change 2001:db8:105::/64 via 2001:db8:101::2"
+ log_test $? 2 "single path - replace of non-existent route"
+}
+
+ipv6_rt_replace_mpath()
+{
+ # multipath with multipath
+ add_initial_route6 "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
+ run_cmd "$IP -6 ro replace 2001:db8:104::/64 nexthop via 2001:db8:101::3 nexthop via 2001:db8:103::3"
+ check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::3 dev veth1 weight 1 nexthop via 2001:db8:103::3 dev veth3 weight 1"
+ log_test $? 0 "multipath with multipath"
+
+ # multipath with single
+ add_initial_route6 "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
+ run_cmd "$IP -6 ro replace 2001:db8:104::/64 via 2001:db8:101::3"
+ check_route6 "2001:db8:104::/64 via 2001:db8:101::3 dev veth1 metric 1024"
+ log_test $? 0 "multipath with single path"
+
+ # multipath with single
+ add_initial_route6 "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
+ run_cmd "$IP -6 ro replace 2001:db8:104::/64 nexthop via 2001:db8:101::3"
+ check_route6 "2001:db8:104::/64 via 2001:db8:101::3 dev veth1 metric 1024"
+ log_test $? 0 "multipath with single path via multipath attribute"
+
+ # multipath with reject
+ add_initial_route6 "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
+ run_cmd "$IP -6 ro replace unreachable 2001:db8:104::/64"
+ check_route6 "unreachable 2001:db8:104::/64 dev lo metric 1024"
+ log_test $? 0 "multipath with reject route"
+
+ # route replace fails - invalid nexthop 1
+ add_initial_route6 "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
+ run_cmd "$IP -6 ro replace 2001:db8:104::/64 nexthop via 2001:db8:111::3 nexthop via 2001:db8:103::3"
+ check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::2 dev veth1 weight 1 nexthop via 2001:db8:103::2 dev veth3 weight 1"
+ log_test $? 0 "multipath - invalid first nexthop"
+
+ # route replace fails - invalid nexthop 2
+ add_initial_route6 "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
+ run_cmd "$IP -6 ro replace 2001:db8:104::/64 nexthop via 2001:db8:101::3 nexthop via 2001:db8:113::3"
+ check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::2 dev veth1 weight 1 nexthop via 2001:db8:103::2 dev veth3 weight 1"
+ log_test $? 0 "multipath - invalid second nexthop"
+
+ # multipath non-existent route
+ add_initial_route6 "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
+ run_cmd "$IP -6 ro change 2001:db8:105::/64 nexthop via 2001:db8:101::3 nexthop via 2001:db8:103::3"
+ log_test $? 2 "multipath - replace of non-existent route"
+}
+
+ipv6_rt_replace()
+{
+ echo
+ echo "IPv6 route replace tests"
+
+ ipv6_rt_replace_single
+ ipv6_rt_replace_mpath
+}
+
+ipv6_route_test()
+{
+ route_setup
+
+ ipv6_rt_add
+ ipv6_rt_replace
+
+ route_cleanup
+}
+
+################################################################################
# usage
usage()
@@ -635,6 +963,7 @@ do
fib_down_test|down) fib_down_test;;
fib_carrier_test|carrier) fib_carrier_test;;
fib_nexthop_test|nexthop) fib_nexthop_test;;
+ ipv6_route_test|ipv6_rt) ipv6_route_test;;
help) echo "Test names: $TESTS"; exit 0;;
esac
--
2.11.0
^ permalink raw reply related
* [PATCH RFC net-next 5/7] selftests: fib_tests: Add option to pause after each test
From: David Ahern @ 2018-05-15 2:51 UTC (permalink / raw)
To: netdev; +Cc: Thomas.Winter, idosch, sharpd, roopa, David Ahern
In-Reply-To: <20180515025112.16647-1-dsahern@gmail.com>
Add option to pause after each test before cleanup is done. Allows
user to do manual inspection or more ad-hoc testing after each test
with the setup in tact.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
tools/testing/selftests/net/fib_tests.sh | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 8c99f0689efc..12b648826151 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -9,6 +9,7 @@ ret=0
TESTS="unregister down carrier nexthop"
VERBOSE=0
PAUSE_ON_FAIL=no
+PAUSE=no
IP="ip -netns testns"
log_test()
@@ -31,6 +32,13 @@ log_test()
[ "$a" = "q" ] && exit 1
fi
fi
+
+ if [ "${PAUSE}" = "yes" ]; then
+ echo
+ echo "hit enter to continue, 'q' to quit"
+ read a
+ [ "$a" = "q" ] && exit 1
+ fi
}
setup()
@@ -576,6 +584,7 @@ usage: ${0##*/} OPTS
-t <test> Test(s) to run (default: all)
(options: $TESTS)
-p Pause on fail
+ -P Pause after each test before cleanup
-v verbose mode (show commands and output)
EOF
}
@@ -588,6 +597,7 @@ do
case $o in
t) TESTS=$OPTARG;;
p) PAUSE_ON_FAIL=yes;;
+ P) PAUSE=yes;;
v) VERBOSE=$(($VERBOSE + 1));;
h) usage; exit 0;;
*) usage; exit 1;;
@@ -596,6 +606,9 @@ done
PEER_CMD="ip netns exec ${PEER_NS}"
+# make sure we don't pause twice
+[ "${PAUSE}" = "yes" ] && PAUSE_ON_FAIL=no
+
if [ "$(id -u)" -ne 0 ];then
echo "SKIP: Need root privileges"
exit 0
--
2.11.0
^ permalink raw reply related
* [PATCH RFC net-next 4/7] selftests: fib_tests: Add command line options
From: David Ahern @ 2018-05-15 2:51 UTC (permalink / raw)
To: netdev; +Cc: Thomas.Winter, idosch, sharpd, roopa, David Ahern
In-Reply-To: <20180515025112.16647-1-dsahern@gmail.com>
Add command line options for controlling pause on fail, controlling
specific tests to run and verbose mode rather than relying on environment
variables.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
tools/testing/selftests/net/fib_tests.sh | 53 ++++++++++++++++++++++++--------
1 file changed, 40 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 7e2291161e15..8c99f0689efc 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -6,8 +6,9 @@
ret=0
-VERBOSE=${VERBOSE:=0}
-PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
+TESTS="unregister down carrier nexthop"
+VERBOSE=0
+PAUSE_ON_FAIL=no
IP="ip -netns testns"
log_test()
@@ -565,20 +566,36 @@ fib_nexthop_test()
}
################################################################################
-#
+# usage
-fib_test()
+usage()
{
- if [ -n "$TEST" ]; then
- eval $TEST
- else
- fib_unreg_test
- fib_down_test
- fib_carrier_test
- fib_nexthop_test
- fi
+ cat <<EOF
+usage: ${0##*/} OPTS
+
+ -t <test> Test(s) to run (default: all)
+ (options: $TESTS)
+ -p Pause on fail
+ -v verbose mode (show commands and output)
+EOF
}
+################################################################################
+# main
+
+while getopts :t:pPhv o
+do
+ case $o in
+ t) TESTS=$OPTARG;;
+ p) PAUSE_ON_FAIL=yes;;
+ v) VERBOSE=$(($VERBOSE + 1));;
+ h) usage; exit 0;;
+ *) usage; exit 1;;
+ esac
+done
+
+PEER_CMD="ip netns exec ${PEER_NS}"
+
if [ "$(id -u)" -ne 0 ];then
echo "SKIP: Need root privileges"
exit 0
@@ -598,7 +615,17 @@ fi
# start clean
cleanup &> /dev/null
-fib_test
+for t in $TESTS
+do
+ case $t in
+ fib_unreg_test|unregister) fib_unreg_test;;
+ fib_down_test|down) fib_down_test;;
+ fib_carrier_test|carrier) fib_carrier_test;;
+ fib_nexthop_test|nexthop) fib_nexthop_test;;
+
+ help) echo "Test names: $TESTS"; exit 0;;
+ esac
+done
if [ "$TESTS" != "none" ]; then
printf "\nTests passed: %3d\n" ${nsuccess}
--
2.11.0
^ permalink raw reply related
* [PATCH RFC net-next 3/7] selftests: fib_tests: Add success-fail counts
From: David Ahern @ 2018-05-15 2:51 UTC (permalink / raw)
To: netdev; +Cc: Thomas.Winter, idosch, sharpd, roopa, David Ahern
In-Reply-To: <20180515025112.16647-1-dsahern@gmail.com>
As more tests are added, it is convenient to have a tally at the end.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
tools/testing/selftests/net/fib_tests.sh | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 9164e60d4b66..7e2291161e15 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -18,8 +18,10 @@ log_test()
if [ ${rc} -eq ${expected} ]; then
printf " TEST: %-60s [ OK ]\n" "${msg}"
+ nsuccess=$((nsuccess+1))
else
ret=1
+ nfail=$((nfail+1))
printf " TEST: %-60s [FAIL]\n" "${msg}"
if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
echo
@@ -598,4 +600,9 @@ cleanup &> /dev/null
fib_test
+if [ "$TESTS" != "none" ]; then
+ printf "\nTests passed: %3d\n" ${nsuccess}
+ printf "Tests failed: %3d\n" ${nfail}
+fi
+
exit $ret
--
2.11.0
^ permalink raw reply related
* [PATCH RFC net-next 2/7] net/ipv6: Simplify route replace and appending into multipath route
From: David Ahern @ 2018-05-15 2:51 UTC (permalink / raw)
To: netdev; +Cc: Thomas.Winter, idosch, sharpd, roopa, David Ahern
In-Reply-To: <20180515025112.16647-1-dsahern@gmail.com>
Remove rt6_qualify_for_ecmp which is just guess work. It fails in 2 cases:
1. can not replace a route with a reject route. Existing code appends
a new route instead of replacing the existing one.
2. can not have a multipath route where a leg uses a dev only nexthop
Existing use cases affected by this change:
1. adding a route with existing prefix and metric using NLM_F_CREATE
without NLM_F_APPEND or NLM_F_EXCL (ie., what iproute2 calls
'prepend'). Existing code auto-determines that the new nexthop can
be appended to an existing route to create a multipath route. This
change breaks that by requiring the APPEND flag for the new route
to be added to an existing one. Instead the prepend just adds another
route entry.
2. route replace. Existing code replaces first matching multipath route
if new route is multipath capable and fallback to first matching
non-ECMP route (reject or dev only route) in case one isn't available.
New behavior replaces first matching route. (Thanks to Ido for spotting
this one)
Note: Newer iproute2 is needed to display multipath routes with a dev-only
nexthop. This is due to a bug in iproute2 and parsing nexthops.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
include/net/ip6_route.h | 6 --
net/ipv6/ip6_fib.c | 157 ++++++++++++++++++++++--------------------------
net/ipv6/route.c | 3 +-
3 files changed, 73 insertions(+), 93 deletions(-)
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 4cf1ef935ed9..9e4d0f0aeb6d 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -66,12 +66,6 @@ static inline bool rt6_need_strict(const struct in6_addr *daddr)
(IPV6_ADDR_MULTICAST | IPV6_ADDR_LINKLOCAL | IPV6_ADDR_LOOPBACK);
}
-static inline bool rt6_qualify_for_ecmp(const struct fib6_info *f6i)
-{
- return (f6i->fib6_flags & (RTF_GATEWAY|RTF_ADDRCONF|RTF_DYNAMIC)) ==
- RTF_GATEWAY;
-}
-
void ip6_route_input(struct sk_buff *skb);
struct dst_entry *ip6_route_input_lookup(struct net *net,
struct net_device *dev,
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index f0a4262a4789..f57ec3bc12d1 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -927,19 +927,19 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
{
struct fib6_info *leaf = rcu_dereference_protected(fn->leaf,
lockdep_is_held(&rt->fib6_table->tb6_lock));
- struct fib6_info *iter = NULL;
+ struct fib6_info *iter = NULL, *match = NULL;
struct fib6_info __rcu **ins;
- struct fib6_info __rcu **fallback_ins = NULL;
int replace = (info->nlh &&
(info->nlh->nlmsg_flags & NLM_F_REPLACE));
+ int append = (info->nlh &&
+ (info->nlh->nlmsg_flags & NLM_F_APPEND));
int add = (!info->nlh ||
(info->nlh->nlmsg_flags & NLM_F_CREATE));
int found = 0;
- bool rt_can_ecmp = rt6_qualify_for_ecmp(rt);
u16 nlflags = NLM_F_EXCL;
int err;
- if (info->nlh && (info->nlh->nlmsg_flags & NLM_F_APPEND))
+ if (append)
nlflags |= NLM_F_APPEND;
ins = &fn->leaf;
@@ -961,13 +961,8 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
nlflags &= ~NLM_F_EXCL;
if (replace) {
- if (rt_can_ecmp == rt6_qualify_for_ecmp(iter)) {
- found++;
- break;
- }
- if (rt_can_ecmp)
- fallback_ins = fallback_ins ?: ins;
- goto next_iter;
+ found++;
+ break;
}
if (rt6_duplicate_nexthop(iter, rt)) {
@@ -982,86 +977,67 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
fib6_metric_set(iter, RTAX_MTU, rt->fib6_pmtu);
return -EEXIST;
}
- /* If we have the same destination and the same metric,
- * but not the same gateway, then the route we try to
- * add is sibling to this route, increment our counter
- * of siblings, and later we will add our route to the
- * list.
- * Only static routes (which don't have flag
- * RTF_EXPIRES) are used for ECMPv6.
- *
- * To avoid long list, we only had siblings if the
- * route have a gateway.
- */
- if (rt_can_ecmp &&
- rt6_qualify_for_ecmp(iter))
- rt->fib6_nsiblings++;
+
+ /* first route that matches */
+ if (!match)
+ match = iter;
}
if (iter->fib6_metric > rt->fib6_metric)
break;
-next_iter:
ins = &iter->fib6_next;
}
- if (fallback_ins && !found) {
- /* No ECMP-able route found, replace first non-ECMP one */
- ins = fallback_ins;
- iter = rcu_dereference_protected(*ins,
- lockdep_is_held(&rt->fib6_table->tb6_lock));
- found++;
- }
-
/* Reset round-robin state, if necessary */
if (ins == &fn->leaf)
fn->rr_ptr = NULL;
/* Link this route to others same route. */
- if (rt->fib6_nsiblings) {
- unsigned int fib6_nsiblings;
+ if (append && match) {
struct fib6_info *sibling, *temp_sibling;
- /* Find the first route that have the same metric */
- sibling = leaf;
- while (sibling) {
- if (sibling->fib6_metric == rt->fib6_metric &&
- rt6_qualify_for_ecmp(sibling)) {
- list_add_tail(&rt->fib6_siblings,
- &sibling->fib6_siblings);
- break;
- }
- sibling = rcu_dereference_protected(sibling->fib6_next,
- lockdep_is_held(&rt->fib6_table->tb6_lock));
+ if (rt->fib6_flags & RTF_REJECT) {
+ NL_SET_ERR_MSG(extack,
+ "Can not append a REJECT route");
+ return -EINVAL;
+ } else if (match->fib6_flags & RTF_REJECT) {
+ NL_SET_ERR_MSG(extack,
+ "Can not append to a REJECT route");
+ return -EINVAL;
}
+ rt->fib6_nsiblings = match->fib6_nsiblings;
+ list_add_tail(&rt->fib6_siblings, &match->fib6_siblings);
+ match->fib6_nsiblings++;
+
/* For each sibling in the list, increment the counter of
* siblings. BUG() if counters does not match, list of siblings
* is broken!
*/
- fib6_nsiblings = 0;
list_for_each_entry_safe(sibling, temp_sibling,
- &rt->fib6_siblings, fib6_siblings) {
+ &match->fib6_siblings, fib6_siblings) {
sibling->fib6_nsiblings++;
- BUG_ON(sibling->fib6_nsiblings != rt->fib6_nsiblings);
- fib6_nsiblings++;
+ BUG_ON(sibling->fib6_nsiblings != match->fib6_nsiblings);
}
- BUG_ON(fib6_nsiblings != rt->fib6_nsiblings);
- rt6_multipath_rebalance(temp_sibling);
+
+ rt6_multipath_rebalance(match);
}
/*
* insert node
*/
if (!replace) {
+ enum fib_event_type event;
+
if (!add)
pr_warn("NLM_F_CREATE should be set when creating new route\n");
add:
nlflags |= NLM_F_CREATE;
- err = call_fib6_entry_notifiers(info->nl_net,
- FIB_EVENT_ENTRY_ADD,
- rt, extack);
+ event = append ? FIB_EVENT_ENTRY_APPEND : FIB_EVENT_ENTRY_ADD;
+ err = call_fib6_entry_notifiers(info->nl_net, event, rt,
+ extack);
if (err)
return err;
@@ -1079,7 +1055,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
}
} else {
- int nsiblings;
+ struct fib6_info *tmp;
if (!found) {
if (add)
@@ -1094,48 +1070,57 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
if (err)
return err;
+ /* if route being replaced has siblings, set tmp to
+ * last one, otherwise tmp is current route. this is
+ * used to set fib6_next for new route
+ */
+ if (iter->fib6_nsiblings)
+ tmp = list_last_entry(&iter->fib6_siblings,
+ struct fib6_info,
+ fib6_siblings);
+ else
+ tmp = iter;
+
+ /* insert new route */
atomic_inc(&rt->fib6_ref);
rcu_assign_pointer(rt->fib6_node, fn);
- rt->fib6_next = iter->fib6_next;
+ rt->fib6_next = tmp->fib6_next;
rcu_assign_pointer(*ins, rt);
+
if (!info->skip_notify)
inet6_rt_notify(RTM_NEWROUTE, rt, info, NLM_F_REPLACE);
if (!(fn->fn_flags & RTN_RTINFO)) {
info->nl_net->ipv6.rt6_stats->fib_route_nodes++;
fn->fn_flags |= RTN_RTINFO;
}
- nsiblings = iter->fib6_nsiblings;
- iter->fib6_node = NULL;
- fib6_purge_rt(iter, fn, info->nl_net);
- if (rcu_access_pointer(fn->rr_ptr) == iter)
- fn->rr_ptr = NULL;
- fib6_info_release(iter);
- if (nsiblings) {
+ /* delete old route */
+ rt = iter;
+
+ if (rt->fib6_nsiblings) {
+ struct fib6_info *tmp;
+
/* Replacing an ECMP route, remove all siblings */
- ins = &rt->fib6_next;
- iter = rcu_dereference_protected(*ins,
- lockdep_is_held(&rt->fib6_table->tb6_lock));
- while (iter) {
- if (iter->fib6_metric > rt->fib6_metric)
- break;
- if (rt6_qualify_for_ecmp(iter)) {
- *ins = iter->fib6_next;
- iter->fib6_node = NULL;
- fib6_purge_rt(iter, fn, info->nl_net);
- if (rcu_access_pointer(fn->rr_ptr) == iter)
- fn->rr_ptr = NULL;
- fib6_info_release(iter);
- nsiblings--;
- info->nl_net->ipv6.rt6_stats->fib_rt_entries--;
- } else {
- ins = &iter->fib6_next;
- }
- iter = rcu_dereference_protected(*ins,
- lockdep_is_held(&rt->fib6_table->tb6_lock));
+ list_for_each_entry_safe(iter, tmp, &rt->fib6_siblings,
+ fib6_siblings) {
+ iter->fib6_node = NULL;
+ fib6_purge_rt(iter, fn, info->nl_net);
+ if (rcu_access_pointer(fn->rr_ptr) == iter)
+ fn->rr_ptr = NULL;
+ fib6_info_release(iter);
+
+ rt->fib6_nsiblings--;
+ info->nl_net->ipv6.rt6_stats->fib_rt_entries--;
}
- WARN_ON(nsiblings != 0);
}
+
+ WARN_ON(rt->fib6_nsiblings != 0);
+
+ rt->fib6_node = NULL;
+ fib6_purge_rt(rt, fn, info->nl_net);
+ if (rcu_access_pointer(fn->rr_ptr) == rt)
+ fn->rr_ptr = NULL;
+ fib6_info_release(rt);
}
return 0;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index af0416701fb2..f267e320ee2e 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3781,7 +3781,7 @@ static struct fib6_info *rt6_multipath_first_sibling(const struct fib6_info *rt)
lockdep_is_held(&rt->fib6_table->tb6_lock));
while (iter) {
if (iter->fib6_metric == rt->fib6_metric &&
- rt6_qualify_for_ecmp(iter))
+ iter->fib6_nsiblings)
return iter;
iter = rcu_dereference_protected(iter->fib6_next,
lockdep_is_held(&rt->fib6_table->tb6_lock));
@@ -4371,6 +4371,7 @@ static int ip6_route_multipath_add(struct fib6_config *cfg,
*/
cfg->fc_nlinfo.nlh->nlmsg_flags &= ~(NLM_F_EXCL |
NLM_F_REPLACE);
+ cfg->fc_nlinfo.nlh->nlmsg_flags |= NLM_F_APPEND;
nhn++;
}
--
2.11.0
^ permalink raw reply related
* [PATCH RFC net-next 1/7] mlxsw: spectrum_router: Add support for route append
From: David Ahern @ 2018-05-15 2:51 UTC (permalink / raw)
To: netdev; +Cc: Thomas.Winter, idosch, sharpd, roopa, David Ahern
In-Reply-To: <20180515025112.16647-1-dsahern@gmail.com>
Handle append for gateway based routes. Dev-only multipath routes will
be handled by a follow on patch.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: David Ahern <dsahern@gmail.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 8028d221aece..77b2adb29341 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -5725,6 +5725,7 @@ static void mlxsw_sp_router_fib6_event_work(struct work_struct *work)
switch (fib_work->event) {
case FIB_EVENT_ENTRY_REPLACE: /* fall through */
+ case FIB_EVENT_ENTRY_APPEND: /* fall through */
case FIB_EVENT_ENTRY_ADD:
replace = fib_work->event == FIB_EVENT_ENTRY_REPLACE;
err = mlxsw_sp_router_fib6_add(mlxsw_sp,
@@ -5831,6 +5832,7 @@ static void mlxsw_sp_router_fib6_event(struct mlxsw_sp_fib_event_work *fib_work,
switch (fib_work->event) {
case FIB_EVENT_ENTRY_REPLACE: /* fall through */
+ case FIB_EVENT_ENTRY_APPEND: /* fall through */
case FIB_EVENT_ENTRY_ADD: /* fall through */
case FIB_EVENT_ENTRY_DEL:
fen6_info = container_of(info, struct fib6_entry_notifier_info,
--
2.11.0
^ permalink raw reply related
* [PATCH RFC net-next 0/7] net/ipv6: Fix route append and replace use cases
From: David Ahern @ 2018-05-15 2:51 UTC (permalink / raw)
To: netdev; +Cc: Thomas.Winter, idosch, sharpd, roopa, David Ahern
This patch set fixes a few append and replace uses cases for IPv6 and
adds test cases that codifies the expectations of how append and replace
are expected to work. In paricular it allows a multipath route to have
a dev-only nexthop, something Thomas tried to accomplish with commit
edd7ceb78296 ("ipv6: Allow non-gateway ECMP for IPv6") which had to be
reverted because of breakage, and to replace an existing FIB entry
with a reject route.
There are a number of inconsistent and surprising aspects to the Linux
API for adding, deleting, replacing and changing FIB entries. For example,
with IPv4 NLM_F_APPEND means insert the route after any existing entries
with the same key (prefix + priority + TOS for IPv4) and NLM_F_CREATE
without the append flag inserts the new route before any existing entries.
IPv6 on the other hand attempts to guess whether a new route should be
appended to an existing one, possibly creating a multipath route, or to
add a new entry after any existing ones. This applies to both the 'append'
(NLM_F_CREATE + NLM_F_APPEND) and 'prepend' (NLM_F_CREATE only) cases
meaning for IPv6 the NLM_F_APPEND is basically ignored. This guessing
whether the route should be added to a multipath route (gateway routes)
or inserted after existing entries (non-gateway based routes) means a
multipath route can not have a dev only nexthop (potentially required in
some cases - tunnels or VRF route leaking for example) and route 'replace'
is a bit adhoc treating gateway based routes and dev-only / reject routes
differently.
This has led to frustration with developers working on routing suites
such as FRR where workarounds such as delete and add.
After this patch set there are 2 differences between IPv4 and IPv6:
1. 'ip ro prepend' = NLM_F_CREATE only
IPv4 adds the new route before any existing ones
IPv6 adds new route after any existing ones
2. 'ip ro append' = NLM_F_CREATE|NLM_F_APPEND
IPv4 adds the new route after any existing ones
IPv6 adds the nexthop to existing routes converting to multipath
For the former, there are cases where we want same prefix routes added
after existing ones (e.g., multicast, prefix routes for macvlan when used
for virtual router redundancy). Requiring the APPEND flag to add a new
route to an existing one helps here but is a slight change in behavior
since prepend with gateway routes now create a separate entry.
For the latter IPv6 behavior is preferred - appending a route for the same
prefix and metric to make a multipath route, so really IPv4 not allowing an
existing route to be updated is the limiter. This will be fixed when
nexthops become separate objects - a future patch set.
Thank you to Thomas and Ido for testing earlier versions of this set, and
to Ido for providing an update to the mlxsw driver.
David Ahern (7):
mlxsw: spectrum_router: Add support for route append
net/ipv6: Simplify appending route into multipath route
selftests: fib_tests: Add success-fail counts
selftests: fib_tests: Add command line options
selftests: fib_tests: Add option to pause after each test
selftests: fib_tests: Add ipv6 route add append replace tests
selftests: fib_tests: Add ipv4 route add append replace tests
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 2 +
include/net/ip6_route.h | 6 -
net/ipv6/ip6_fib.c | 157 +++--
net/ipv6/route.c | 3 +-
tools/testing/selftests/net/fib_tests.sh | 673 ++++++++++++++++++++-
5 files changed, 737 insertions(+), 104 deletions(-)
--
2.11.0
^ permalink raw reply
* Re: [PATCH net-next] cxgb4: add tc flower match support for tunnel VNI
From: David Miller @ 2018-05-15 2:50 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan, indranil, venkatesh, kumaras
In-Reply-To: <1526300481-19115-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Mon, 14 May 2018 17:51:21 +0530
> From: Kumar Sanghvi <kumaras@chelsio.com>
>
> Adds support for matching flows based on tunnel VNI value.
> Introduces fw APIs for allocating/removing MPS entries related
> to encapsulation. And uses the same while adding/deleting filters
> for offloading flows based on tunnel VNI match.
>
> Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net 2/2] vmxnet3: use DMA memory barriers where required
From: David Miller @ 2018-05-15 2:45 UTC (permalink / raw)
To: hpreg; +Cc: netdev, doshir, pv-drivers, linux-kernel
In-Reply-To: <1526300090-78546-1-git-send-email-hpreg@vmware.com>
From: <hpreg@vmware.com>
Date: Mon, 14 May 2018 08:14:49 -0400
> The gen bits must be read first from (resp. written last to) DMA memory.
> The proper way to enforce this on Linux is to call dma_rmb() (resp.
> dma_wmb()).
>
> Signed-off-by: Regis Duchesne <hpreg@vmware.com>
> Acked-by: Ronak Doshi <doshir@vmware.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net 1/2] vmxnet3: set the DMA mask before the first DMA map operation
From: David Miller @ 2018-05-15 2:44 UTC (permalink / raw)
To: hpreg; +Cc: netdev, doshir, pv-drivers, linux-kernel
In-Reply-To: <1526300907-79831-1-git-send-email-hpreg@vmware.com>
From: <hpreg@vmware.com>
Date: Mon, 14 May 2018 08:28:26 -0400
> The DMA mask must be set before, not after, the first DMA map operation, or
> the first DMA map operation could in theory fail on some systems.
>
> Fixes: b0eb57cb97e78 ("VMXNET3: Add support for virtual IOMMU")
> Signed-off-by: Regis Duchesne <hpreg@vmware.com>
> Acked-by: Ronak Doshi <doshir@vmware.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net] cxgb4: Correct ntuple mask validation for hash filters
From: David Miller @ 2018-05-15 2:43 UTC (permalink / raw)
To: ganeshgr
Cc: netdev, nirranjan, indranil, venkatesh, rahul.lakkireddy, kumaras
In-Reply-To: <1526295454-12650-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Mon, 14 May 2018 16:27:34 +0530
> From: Kumar Sanghvi <kumaras@chelsio.com>
>
> Earlier code of doing bitwise AND with field width bits was wrong.
> Instead, simplify code to calculate ntuple_mask based on supplied
> fields and then compare with mask configured in hw - which is the
> correct and simpler way to validate ntuple mask.
>
> Fixes: 3eb8b62d5a26 ("cxgb4: add support to create hash-filters via tc-flower offload")
> Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net-next] net: stmmac: Add Jose Abreu as co-maintainer
From: David Miller @ 2018-05-15 2:40 UTC (permalink / raw)
To: Jose.Abreu; +Cc: netdev, Joao.Pinto, alexandre.torgue, peppe.cavallaro
In-Reply-To: <06350075c76ee1c13f84c94346444507e9770ed2.1526290012.git.joabreu@synopsys.com>
From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Mon, 14 May 2018 10:29:56 +0100
> I'm offering to be a co-maintainer for stmmac driver.
>
> As per discussion with Alexandre, I will arranje to get STM32 boards to
> test patches in GMAC version 3.x and 4.1. I also have HW to test GMAC
> version 5.
>
> Looking forward to contribute to net-dev!
>
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Applied with commit message typo fixed.
^ permalink raw reply
* Re: [RFC bpf-next 04/11] bpf: Add PTR_TO_SOCKET verifier type
From: Alexei Starovoitov @ 2018-05-15 2:37 UTC (permalink / raw)
To: Joe Stringer; +Cc: daniel, netdev, ast, john.fastabend, kafai
In-Reply-To: <20180509210709.7201-5-joe@wand.net.nz>
On Wed, May 09, 2018 at 02:07:02PM -0700, Joe Stringer wrote:
> 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>
> ---
> include/linux/bpf.h | 19 +++++++++-
> include/linux/bpf_verifier.h | 2 ++
> kernel/bpf/verifier.c | 86 ++++++++++++++++++++++++++++++++++++++------
> net/core/filter.c | 30 +++++++++-------
> 4 files changed, 114 insertions(+), 23 deletions(-)
Ack for patches 1-3. In this one few nits:
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index a38e474bf7ee..a03b4b0edcb6 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -136,7 +136,7 @@ enum bpf_arg_type {
> /* the following constraints used to prototype bpf_memcmp() and other
> * functions that access data on eBPF program stack
> */
> - ARG_PTR_TO_MEM, /* pointer to valid memory (stack, packet, map value) */
> + ARG_PTR_TO_MEM, /* pointer to valid memory (stack, packet, map value, socket) */
I don't see where in this patch this change happens...
> ARG_PTR_TO_MEM_OR_NULL, /* pointer to valid memory or NULL */
> ARG_PTR_TO_UNINIT_MEM, /* pointer to memory does not need to be initialized,
> * helper function must fill all bytes or clear
> @@ -148,6 +148,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 */
> @@ -155,6 +156,7 @@ enum bpf_return_type {
> RET_INTEGER, /* function returns integer */
> RET_VOID, /* function doesn't return anything */
> 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
> @@ -205,6 +207,8 @@ enum bpf_reg_type {
> PTR_TO_PACKET_META, /* skb->data - meta_len */
> PTR_TO_PACKET, /* reg points to skb->data */
> PTR_TO_PACKET_END, /* skb->data + headlen */
> + 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
> @@ -326,6 +330,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);
> @@ -729,4 +738,12 @@ extern const struct bpf_func_proto bpf_sock_map_update_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 a613b52ce939..9dcd87f1d322 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -57,6 +57,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;
> /* Ordering of fields matters. See states_equal() */
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1b31b805dea4..d38c7c1e9da6 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.
> @@ -244,6 +244,8 @@ static const char * const reg_type_str[] = {
> [PTR_TO_PACKET] = "pkt",
> [PTR_TO_PACKET_META] = "pkt_meta",
> [PTR_TO_PACKET_END] = "pkt_end",
> + [PTR_TO_SOCKET] = "sock",
> + [PTR_TO_SOCKET_OR_NULL] = "sock_or_null",
> };
>
> static void print_liveness(struct bpf_verifier_env *env,
> @@ -977,6 +979,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type)
> case PTR_TO_PACKET_META:
> case PTR_TO_PACKET_END:
> case CONST_PTR_TO_MAP:
> + case PTR_TO_SOCKET:
> + case PTR_TO_SOCKET_OR_NULL:
> return true;
> default:
> return false;
> @@ -1360,6 +1364,28 @@ static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off,
> return -EACCES;
> }
>
> +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)
> {
> @@ -1475,6 +1501,9 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
> */
> strict = true;
> break;
> + case PTR_TO_SOCKET:
> + pointer_desc = "sock ";
> + break;
> default:
> break;
> }
> @@ -1723,6 +1752,16 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> err = check_packet_access(env, regno, off, size, false);
> 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 && t == BPF_READ && value_regno >= 0)
t == BPF_READ check is unnecessary.
> + mark_reg_unknown(env, regs, value_regno);
> +
> } else {
> verbose(env, "R%d invalid mem access '%s'\n", regno,
> reg_type_str[reg->type]);
> @@ -1941,6 +1980,10 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
> expected_type = PTR_TO_CTX;
> if (type != expected_type)
> goto err_type;
> + } 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
> @@ -2477,6 +2520,10 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
> insn_aux->map_ptr = meta.map_ptr;
> else if (insn_aux->map_ptr != meta.map_ptr)
> insn_aux->map_ptr = BPF_MAP_PTR_POISON;
> + } 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);
> @@ -2614,6 +2661,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;
> @@ -3559,6 +3608,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
> @@ -4333,6 +4384,8 @@ static bool regsafe(struct bpf_reg_state *rold, struct bpf_reg_state *rcur,
> case PTR_TO_CTX:
> case CONST_PTR_TO_MAP:
> case PTR_TO_PACKET_END:
> + case PTR_TO_SOCKET:
> + case PTR_TO_SOCKET_OR_NULL:
> /* Only valid matches are exact, which memcmp() above
> * would have accepted
> */
> @@ -5188,10 +5241,14 @@ 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)
> +static int convert_ctx_accesses(struct bpf_verifier_env *env,
> + bpf_convert_ctx_access_t convert_ctx_access,
> + enum bpf_reg_type ctx_type)
> {
> const struct bpf_verifier_ops *ops = env->ops;
> int i, cnt, size, ctx_field_size, delta = 0;
> @@ -5218,12 +5275,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 (!convert_ctx_access || bpf_prog_is_dev_bound(env->prog->aux))
> return 0;
>
> insn = env->prog->insnsi + delta;
>
> for (i = 0; i < insn_cnt; i++, insn++) {
> + enum bpf_reg_type ptr_type;
> +
> 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) ||
> @@ -5237,7 +5296,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
> else
> continue;
>
> - if (env->insn_aux_data[i + delta].ptr_type != PTR_TO_CTX)
> + ptr_type = env->insn_aux_data[i + delta].ptr_type;
> + if (ptr_type != ctx_type)
> continue;
>
> ctx_field_size = env->insn_aux_data[i + delta].ctx_field_size;
> @@ -5269,8 +5329,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");
> @@ -5785,7 +5845,13 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr)
>
> if (ret == 0)
> /* program is valid, convert *(u32*)(ctx + off) accesses */
> - ret = convert_ctx_accesses(env);
> + ret = convert_ctx_accesses(env, env->ops->convert_ctx_access,
> + PTR_TO_CTX);
> +
> + if (ret == 0)
> + /* Convert *(u32*)(sock_ops + off) accesses */
> + ret = convert_ctx_accesses(env, bpf_sock_convert_ctx_access,
> + PTR_TO_SOCKET);
Overall looks great.
Only this part is missing for PTR_TO_SOCKET:
} else if (dst_reg_type != *prev_dst_type &&
(dst_reg_type == PTR_TO_CTX ||
*prev_dst_type == PTR_TO_CTX)) {
verbose(env, "same insn cannot be used with different pointers\n");
return -EINVAL;
similar logic has to be added.
Otherwise the following will be accepted:
R1 = sock_ptr
goto X;
...
R1 = some_other_valid_ptr;
goto X;
...
R2 = *(u32 *)(R1 + 0);
this will be rewritten for first branch,
but it's wrong for second.
^ permalink raw reply
* Re: [PATCH v1 2/4] media: bpf: allow raw IR decoder bpf programs to be used
From: kbuild test robot @ 2018-05-15 2:34 UTC (permalink / raw)
To: Sean Young
Cc: kbuild-all, linux-media, linux-kernel, Alexei Starovoitov,
Mauro Carvalho Chehab, Daniel Borkmann, netdev, Matthias Reichl,
Devin Heitmueller
In-Reply-To: <cd3a5e27ef4122fab90daae2af6031982df77282.1526331777.git.sean@mess.org>
[-- Attachment #1: Type: text/plain, Size: 1224 bytes --]
Hi Sean,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc5]
[cannot apply to next-20180514]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Sean-Young/media-rc-introduce-BPF_PROG_IR_DECODER/20180515-093234
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
>> drivers/media/rc/lirc_dev.c:32:10: fatal error: linux/bpf-rcdev.h: No such file or directory
#include <linux/bpf-rcdev.h>
^~~~~~~~~~~~~~~~~~~
compilation terminated.
--
>> kernel/bpf/syscall.c:30:10: fatal error: linux/bpf-rcdev.h: No such file or directory
#include <linux/bpf-rcdev.h>
^~~~~~~~~~~~~~~~~~~
compilation terminated.
vim +32 drivers/media/rc/lirc_dev.c
31
> 32 #include <linux/bpf-rcdev.h>
33
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 62940 bytes --]
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/8] Minor follow-up cleanups in BPF JITs and optimized imm emission
From: Alexei Starovoitov @ 2018-05-15 2:19 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev
In-Reply-To: <20180514212234.2661-1-daniel@iogearbox.net>
On Mon, May 14, 2018 at 11:22:26PM +0200, Daniel Borkmann wrote:
> This series follows up mostly with with some minor cleanups on top
> of 'Move ld_abs/ld_ind to native BPF' as well as implements better
> 32/64 bit immediate load into register and saves tail call init on
> cBPF for the arm64 JIT. Last but not least we add a couple of test
> cases. For details please see individual patches. Thanks!
>
> v1 -> v2:
> - Minor fix in i64_i16_blocks() to remove 24 shift.
> - Added last two patches.
> - Added Acks from prior round.
Applied, thanks.
^ permalink raw reply
* Re: [PATCH bpf-next v2 8/8] bpf: add ld64 imm test cases
From: Y Song @ 2018-05-15 2:18 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Alexei Starovoitov, netdev
In-Reply-To: <20180514212234.2661-9-daniel@iogearbox.net>
On Mon, May 14, 2018 at 2:22 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> Add test cases where we combine semi-random imm values, mainly for testing
> JITs when they have different encoding options for 64 bit immediates in
> order to reduce resulting image size.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply
* Re: [PATCH v1 3/4] media: rc bpf: move ir_raw_event to uapi
From: kbuild test robot @ 2018-05-15 1:59 UTC (permalink / raw)
To: Sean Young
Cc: kbuild-all, linux-media, linux-kernel, Alexei Starovoitov,
Mauro Carvalho Chehab, Daniel Borkmann, netdev, Matthias Reichl,
Devin Heitmueller
In-Reply-To: <6ecdbd01b8c42c8784f2235c1e5109dac3dd86a5.1526331777.git.sean@mess.org>
[-- Attachment #1: Type: text/plain, Size: 870 bytes --]
Hi Sean,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v4.17-rc5]
[cannot apply to next-20180514]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Sean-Young/media-rc-introduce-BPF_PROG_IR_DECODER/20180515-093234
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
>> ./usr/include/linux/bpf_rcdev.h:13: found __[us]{8,16,32,64} type without #include <linux/types.h>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6302 bytes --]
^ permalink raw reply
* Re: [PATCH v4 1/1] drivers core: multi-threading device shutdown
From: Pavel Tatashin @ 2018-05-15 1:53 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Steven Sistare, Daniel Jordan, Linux Kernel Mailing List,
Kirsher, Jeffrey T, intel-wired-lan, netdev, Greg Kroah-Hartman,
alexander.duyck, tobin
In-Reply-To: <CAHp75VeopFMFHq=rJCUcAw02kznJ5+0vLWr5500DsGtERiX6cw@mail.gmail.com>
Hi Andy,
Thank you for your comments. I will send an updated patch soon. My replies are below:
On 05/14/2018 04:04 PM, Andy Shevchenko wrote:
> Can we still preserve an order here? (Yes, even if the entire list is
> not fully ordered)
> In the context I see it would go before netdevice.h.
Sure, I will move kthread.h.
>> +static struct device *
>> +device_get_child_by_index(struct device *parent, int index)
>> +{
>> + struct klist_iter i;
>> + struct device *dev = NULL, *d;
>> + int child_index = 0;
>> +
>> + if (!parent->p || index < 0)
>> + return NULL;
>> +
>> + klist_iter_init(&parent->p->klist_children, &i);
>> + while ((d = next_device(&i))) {
>> + if (child_index == index) {
>> + dev = d;
>> + break;
>> + }
>> + child_index++;
>> + }
>> + klist_iter_exit(&i);
>> +
>> + return dev;
>> +}
>
> This can be implemented as a subfunction to device_find_child(), can't it be?
Yes, but that would make it very inefficient to search for an index in a list via function pointer call.
>
>> +/**
>
> Hmm... Why it's marked as kernel doc while it's just a plain comment?
> Same applies to the rest of similar comments.
Fixed this, thanks!
>
>> + for (i = 0; i < children_count; i++) {
>> + if (device_shutdown_serial) {
>> + device_shutdown_child_task(&tdata);
>> + } else {
>> + kthread_run(device_shutdown_child_task,
>> + &tdata, "device_shutdown.%s",
>> + dev_name(dev));
>> + }
>> + }
>
> Can't we just use device_for_each_child() instead?
No, at least without doing some memory allocation. Notice in this loop we are not traversing through children, instead we are starting number of children threads, and each thread finds a child to work on. Otherwise we would have to pass child pointer via argument, and we would need to keep that argument in some memory.
Pavel
^ permalink raw reply
* [RFC PATCH] net: sched: __tcf_idr_check() can be static
From: kbuild test robot @ 2018-05-15 1:38 UTC (permalink / raw)
To: Vlad Buslov
Cc: kbuild-all, netdev, davem, jhs, xiyou.wangcong, jiri, pablo,
kadlec, fw, ast, daniel, edumazet, vladbu, keescook, linux-kernel,
netfilter-devel, coreteam, kliteyn
In-Reply-To: <1526308035-12484-6-git-send-email-vladbu@mellanox.com>
Fixes: 446adedb5339 ("net: sched: always take reference to action")
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
act_api.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 9459cce..27e80cf 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -284,8 +284,8 @@ int tcf_generic_walker(struct tc_action_net *tn, struct sk_buff *skb,
}
EXPORT_SYMBOL(tcf_generic_walker);
-bool __tcf_idr_check(struct tc_action_net *tn, u32 index, struct tc_action **a,
- int bind)
+static bool __tcf_idr_check(struct tc_action_net *tn, u32 index, struct tc_action **a,
+ int bind)
{
struct tcf_idrinfo *idrinfo = tn->idrinfo;
struct tc_action *p;
^ permalink raw reply related
* Re: [PATCH 05/14] net: sched: always take reference to action
From: kbuild test robot @ 2018-05-15 1:38 UTC (permalink / raw)
To: Vlad Buslov
Cc: kbuild-all, netdev, davem, jhs, xiyou.wangcong, jiri, pablo,
kadlec, fw, ast, daniel, edumazet, vladbu, keescook, linux-kernel,
netfilter-devel, coreteam, kliteyn
In-Reply-To: <1526308035-12484-6-git-send-email-vladbu@mellanox.com>
Hi Vlad,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net/master]
[also build test WARNING on v4.17-rc5 next-20180514]
[cannot apply to net-next/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Vlad-Buslov/Modify-action-API-for-implementing-lockless-actions/20180515-025420
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
net/sched/act_api.c:71:15: sparse: incorrect type in initializer (different address spaces) @@ expected struct tc_cookie [noderef] <asn:4>*__ret @@ got [noderef] <asn:4>*__ret @@
net/sched/act_api.c:71:15: expected struct tc_cookie [noderef] <asn:4>*__ret
net/sched/act_api.c:71:15: got struct tc_cookie *new_cookie
net/sched/act_api.c:71:13: sparse: incorrect type in assignment (different address spaces) @@ expected struct tc_cookie *old @@ got struct tc_cookie [noderef] <struct tc_cookie *old @@
net/sched/act_api.c:71:13: expected struct tc_cookie *old
net/sched/act_api.c:71:13: got struct tc_cookie [noderef] <asn:4>*[assigned] __ret
>> net/sched/act_api.c:287:6: sparse: symbol '__tcf_idr_check' was not declared. Should it be static?
net/sched/act_api.c:144:48: sparse: dereference of noderef expression
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [kbuild-all] [PATCH net-next 2/2] sctp: add sctp_make_op_error_limited and reuse inner functions
From: Ye Xiaolong @ 2018-05-15 1:23 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: kbuild test robot, Xin Long, Neil Horman, netdev, Vlad Yasevich,
linux-sctp, kbuild-all
In-Reply-To: <20180514121605.GB5105@localhost.localdomain>
On 05/14, Marcelo Ricardo Leitner wrote:
>On Mon, May 14, 2018 at 07:47:20PM +0800, Ye Xiaolong wrote:
>> On 05/14, Marcelo Ricardo Leitner wrote:
>> >On Mon, May 14, 2018 at 03:40:53PM +0800, Ye Xiaolong wrote:
>> >> >> config: x86_64-randconfig-x006-201817 (attached as .config)
>> >> >> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
>> >> >> reproduce:
>> >> >> # save the attached .config to linux build tree
>> >> >> make ARCH=x86_64
>> >> >>
>> >> >> All errors (new ones prefixed by >>):
>> >> >>
>> >> >> net//sctp/sm_make_chunk.c: In function 'sctp_make_op_error_limited':
>> >> >> >> net//sctp/sm_make_chunk.c:1260:9: error: implicit declaration of function 'sctp_mtu_payload'; did you mean 'sctp_do_peeloff'? [-Werror=implicit-function-declaration]
>> >> >> size = sctp_mtu_payload(sp, size, sizeof(struct sctp_errhdr));
>> >> >> ^~~~~~~~~~~~~~~~
>> >> >> sctp_do_peeloff
>> >> >> cc1: some warnings being treated as errors
>> >> >
>> >> >Seems the test didn't pick up the MTU refactor patchset yet.
>> >>
>> >> Do you mean your patchset require MTU refactor patchset as prerequisites?
>> >
>> >Yes.
>>
>> Then it is recommended to use '--base' option of git format-patch, it would record
>> the base tree info in the first patch or cover letter, 0day bot would apply your
>> patchset to right base according to it.
>
>Nice. I wasn't aware of it. Thanks.
>
>Considering that the MTU refactor patchset was already applied on
>net-next when the bot did the test, why should I have to specify the
>base?
Could you share me the subjects or commits of MTU refactor patcheset, I'll double
check what was wrong.
Thanks,
Xiaolong
>
> Marcelo
^ 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