18 Aug, 2016
3 commits
-
As pointed out by Jamal, an action could be shared by
multiple filters, so we can't use list to chain them
any more after we get rid of the original tc_action.
Instead, we could just save pointers to these actions
in tcf_exts, since they are refcount'ed, so convert
the list to an array of pointers.The "ugly" part is the action API still accepts list
as a parameter, I just introduce a helper function to
convert the array of pointers to a list, instead of
relying on the C99 feature to iterate the array.Fixes: a85a970af265 ("net_sched: move tc_action into tcf_common")
Reported-by: Jamal Hadi Salim
Cc: Jamal Hadi Salim
Signed-off-by: Cong Wang
Acked-by: Jamal Hadi Salim
Signed-off-by: David S. Miller -
struct tcf_exts belongs to filters, should not be visible
to plain tc actions.Cc: Ido Schimmel
Signed-off-by: Cong Wang
Acked-by: Jamal Hadi Salim
Signed-off-by: David S. Miller -
It is harmless because all users pass 'a' to this macro.
Fixes: 00175aec941e ("net/sched: Macro instead of CONFIG_NET_CLS_ACT ifdef")
Cc: Amir Vadai
Signed-off-by: Cong Wang
Acked-by: Jamal Hadi Salim
Signed-off-by: David S. Miller
26 Jul, 2016
3 commits
-
After the previous patch, struct tc_action should be enough
to represent the generic tc action, tcf_common is not necessary
any more. This patch gets rid of it to make tc action code
more readable.Cc: Jamal Hadi Salim
Signed-off-by: Cong Wang
Signed-off-by: David S. Miller -
struct tc_action is confusing, currently we use it for two purposes:
1) Pass in arguments and carry out results from helper functions
2) A generic representation for tc actionsThe first one is error-prone, since we need to make sure we don't
miss anything. This patch aims to get rid of this use, by moving
tc_action into tcf_common, so that they are allocated together
in hashtable and can be cast'ed easily.And together with the following patch, we could really make
tc_action a generic representation for all tc actions and each
type of action can inherit from it.Cc: Jamal Hadi Salim
Signed-off-by: Cong Wang
Signed-off-by: David S. Miller -
When CONFIG_NET_CLS_ACT isn't set 'struct tcf_exts' has no member named
'actions' and we therefore must not access it. Otherwise compilation
fails.Fix this by introducing a new macro similar to tc_no_actions(), which
always returns 'false' if CONFIG_NET_CLS_ACT isn't set.Fixes: 763b4b70afcd ("mlxsw: spectrum: Add support in matchall mirror TC offloading")
Reported-by: kbuild test robot
Signed-off-by: Jiri Pirko
Signed-off-by: Ido Schimmel
Signed-off-by: David S. Miller
16 Jun, 2016
1 commit
-
Cc: Jamal Hadi Salim
Signed-off-by: Cong Wang
Acked-by: Jamal Hadi Salim
Signed-off-by: David S. Miller
08 Jun, 2016
3 commits
-
Signed-off-by: Jamal Hadi Salim
Acked-by: Cong Wang -
Signed-off-by: Jamal Hadi Salim
Signed-off-by: David S. Miller -
Useful to know when the action was first used for accounting
(and debugging)Signed-off-by: Jamal Hadi Salim
Signed-off-by: David S. Miller
21 May, 2016
1 commit
-
When CONFIG_NET_CLS_ACT is disabled, we get a new warning in the mlx5
ethernet driver because the tc_for_each_action() loop never references
the iterator:mellanox/mlx5/core/en_tc.c: In function 'mlx5e_stats_flower':
mellanox/mlx5/core/en_tc.c:431:20: error: unused variable 'a' [-Werror=unused-variable]
struct tc_action *a;This changes the dummy tc_for_each_action() macro by adding a
cast to void, letting the compiler know that the variable is
intentionally declared but not used here. I could not come up
with a nicer workaround, but this seems to do the trick.Signed-off-by: Arnd Bergmann
Fixes: aad7e08d39bd ("net/mlx5e: Hardware offloaded flower filter statistics support")
Fixes: 00175aec941e ("net/sched: Macro instead of CONFIG_NET_CLS_ACT ifdef")
Acked-By: Amir Vadai
Signed-off-by: David S. Miller
17 May, 2016
1 commit
-
Introduce stats_update callback. netdev driver could call it for offloaded
actions to update the basic statistics (packets, bytes and last use).
Since bstats_update() and bstats_cpu_update() use skb as an argument to
get the counters, _bstats_update() and _bstats_cpu_update(), that get
bytes and packets as arguments, were added.Signed-off-by: Amir Vadai
Signed-off-by: David S. Miller
07 Apr, 2016
1 commit
-
Fixes: ddf97ccdd7cb ("net_sched: add network namespace support for tc actions")
Reported-by: Dmitry Vyukov
Tested-by: Dmitry Vyukov
Cc: Jamal Hadi Salim
Signed-off-by: Cong Wang
Signed-off-by: David S. Miller
11 Mar, 2016
1 commit
-
Introduce the macros tc_no_actions and tc_for_each_action to make code
clearer.
Extracted struct tc_action out of the ifdef to make calls to
is_tcf_gact_shot() and similar functions valid, even when it is a nop.Acked-by: Jiri Pirko
Acked-by: John Fastabend
Suggested-by: Jiri Pirko
Signed-off-by: Amir Vadai
Signed-off-by: David S. Miller
26 Feb, 2016
2 commits
-
Currently tc actions are stored in a per-module hashtable,
therefore are visible to all network namespaces. This is
probably the last part of the tc subsystem which is not
aware of netns now. This patch makes them per-netns,
several tc action API's need to be adjusted for this.The tc action API code is ugly due to historical reasons,
we need to refactor that code in the future.Cc: Jamal Hadi Salim
Signed-off-by: Cong Wang
Acked-by: Jamal Hadi Salim
Signed-off-by: David S. Miller -
We only release the memory of the hashtable itself, not its
entries inside. This is not a problem yet since we only call
it in module release path, and module is refcount'ed by
actions. This would be a problem after we move the per module
hinfo into per netns in the latter patch.Cc: Jamal Hadi Salim
Signed-off-by: Cong Wang
Acked-by: Jamal Hadi Salim
Signed-off-by: David S. Miller
27 Aug, 2015
1 commit
-
tcf_hash_destroy() used once. Make it static.
Signed-off-by: Alexei Starovoitov
Acked-by: Daniel Borkmann
Signed-off-by: David S. Miller
01 Aug, 2015
1 commit
-
Conflicts:
arch/s390/net/bpf_jit_comp.c
drivers/net/ethernet/ti/netcp_ethss.c
net/bridge/br_multicast.c
net/ipv4/ip_fragment.cAll four conflicts were cases of simple overlapping
changes.Signed-off-by: David S. Miller
31 Jul, 2015
1 commit
-
Since commit 55334a5db5cd ("net_sched: act: refuse to remove bound action
outside"), we end up with a wrong reference count for a tc action.Test case 1:
FOO="1,6 0 0 4294967295,"
BAR="1,6 0 0 4294967294,"
tc filter add dev foo parent 1: bpf bytecode "$FOO" flowid 1:1 \
action bpf bytecode "$FOO"
tc actions show action bpf
action order 0: bpf bytecode '1,6 0 0 4294967295' default-action pipe
index 1 ref 1 bind 1
tc actions replace action bpf bytecode "$BAR" index 1
tc actions show action bpf
action order 0: bpf bytecode '1,6 0 0 4294967294' default-action pipe
index 1 ref 2 bind 1
tc actions replace action bpf bytecode "$FOO" index 1
tc actions show action bpf
action order 0: bpf bytecode '1,6 0 0 4294967295' default-action pipe
index 1 ref 3 bind 1Test case 2:
FOO="1,6 0 0 4294967295,"
tc filter add dev foo parent 1: bpf bytecode "$FOO" flowid 1:1 action ok
tc actions show action gact
action order 0: gact action pass
random type none pass val 0
index 1 ref 1 bind 1
tc actions add action drop index 1
RTNETLINK answers: File exists [...]
tc actions show action gact
action order 0: gact action pass
random type none pass val 0
index 1 ref 2 bind 1
tc actions add action drop index 1
RTNETLINK answers: File exists [...]
tc actions show action gact
action order 0: gact action pass
random type none pass val 0
index 1 ref 3 bind 1What happens is that in tcf_hash_check(), we check tcf_common for a given
index and increase tcfc_refcnt and conditionally tcfc_bindcnt when we've
found an existing action. Now there are the following cases:1) We do a late binding of an action. In that case, we leave the
tcfc_refcnt/tcfc_bindcnt increased and are done with the ->init()
handler. This is correctly handeled.2) We replace the given action, or we try to add one without replacing
and find out that the action at a specific index already exists
(thus, we go out with error in that case).In case of 2), we have to undo the reference count increase from
tcf_hash_check() in the tcf_hash_check() function. Currently, we fail to
do so because of the 'tcfc_bindcnt > 0' check which bails out early with
an -EPERM error.Now, while commit 55334a5db5cd prevents 'tc actions del action ...' on an
already classifier-bound action to drop the reference count (which could
then become negative, wrap around etc), this restriction only accounts for
invocations outside a specific action's ->init() handler.One possible solution would be to add a flag thus we possibly trigger
the -EPERM ony in situations where it is indeed relevant.After the patch, above test cases have correct reference count again.
Fixes: 55334a5db5cd ("net_sched: act: refuse to remove bound action outside")
Signed-off-by: Daniel Borkmann
Reviewed-by: Cong Wang
Signed-off-by: David S. Miller
09 Jul, 2015
2 commits
-
Final step for gact RCU operation :
1) Use percpu stats
2) update lastuse only every clock tick to avoid false sharing
3) Remove spinlock acquisition, as it is no longer needed.Since this is the last contended lock in packet RX when tc gact is used,
this gives impressive gain.My host with 8 RX queues was handling 5 Mpps before the patch,
and more than 11 Mpps after patch.Tested:
On receiver :
dev=eth0
tc qdisc del dev $dev ingress 2>/dev/null
tc qdisc add dev $dev ingress
tc filter del dev $dev root pref 10 2>/dev/null
tc filter del dev $dev pref 10 2>/dev/null
tc filter add dev $dev est 1sec 4sec parent ffff: protocol ip prio 1 \
u32 match ip src 7.0.0.0/8 flowid 1:15 action dropSender sends packets flood from 7/8 network
Signed-off-by: Eric Dumazet
Acked-by: Alexei Starovoitov
Acked-by: Jamal Hadi Salim
Acked-by: John Fastabend
Signed-off-by: David S. Miller -
Reuse existing percpu infrastructure John Fastabend added for qdisc.
This patch adds a new cpustats parameter to tcf_hash_create() and all
actions pass false, meaning this patch should have no effect yet.Signed-off-by: Eric Dumazet
Cc: Alexei Starovoitov
Acked-by: Jamal Hadi Salim
Acked-by: John Fastabend
Signed-off-by: David S. Miller
13 Feb, 2014
4 commits
-
When an action is bonnd to a filter, there is no point to
remove it outside. Currently we just silently decrease the refcnt,
we should reject this explicitly with EPERM.Cc: Jamal Hadi Salim
Cc: David S. Miller
Signed-off-by: Cong Wang
Signed-off-by: Jamal Hadi Salim
Signed-off-by: David S. Miller -
Cc: Jamal Hadi Salim
Cc: David S. Miller
Signed-off-by: Cong Wang
Signed-off-by: Jamal Hadi Salim
Signed-off-by: David S. Miller -
For bindcnt and refcnt etc., they are common for all actions,
not need to repeat such operations for their own, they can be unified
now. Actions just need to do its specific cleanup if needed.Cc: Jamal Hadi Salim
Cc: David S. Miller
Signed-off-by: Cong Wang
Signed-off-by: Jamal Hadi Salim
Signed-off-by: David S. Miller -
Now we can totally hide it from modules. tcf_hash_*() API's
will operate on struct tc_action, modules don't need to care about
the details.Cc: Jamal Hadi Salim
Cc: David S. Miller
Signed-off-by: Cong Wang
Signed-off-by: Jamal Hadi Salim
Signed-off-by: David S. Miller
22 Jan, 2014
2 commits
-
So that we will not expose struct tcf_common to modules.
Cc: Jamal Hadi Salim
Cc: David S. Miller
Signed-off-by: Cong Wang
Signed-off-by: Jamal Hadi Salim
Signed-off-by: David S. Miller -
Every action ops has a pointer to hash info, so we don't need to
hard-code it in each module.Cc: Jamal Hadi Salim
Cc: David S. Miller
Signed-off-by: Cong Wang
Signed-off-by: Jamal Hadi Salim
Signed-off-by: David S. Miller
20 Jan, 2014
1 commit
-
It is not actually implemented.
Cc: Jamal Hadi Salim
Cc: David S. Miller
Signed-off-by: Cong Wang
Signed-off-by: David S. Miller
14 Jan, 2014
2 commits
-
It is not necessary at all.
Cc: Jamal Hadi Salim
Cc: David S. Miller
Signed-off-by: Cong Wang
Signed-off-by: Jamal Hadi Salim
Signed-off-by: David S. Miller -
There is no need to store the index separatedly
since tcf_hashinfo is allocated statically too.Cc: Jamal Hadi Salim
Cc: David S. Miller
Signed-off-by: Cong Wang
Signed-off-by: Jamal Hadi Salim
Signed-off-by: David S. Miller
02 Jan, 2014
1 commit
-
No need to export functions only used in one file.
Signed-off-by: Stephen Hemminger
Acked-by: Jamal Hadi Salim
Signed-off-by: David S. Miller
19 Dec, 2013
5 commits
-
We don't need to maintain our own singly linked list code.
Cc: Jamal Hadi Salim
Cc: David S. Miller
Signed-off-by: Cong Wang
Signed-off-by: Jamal Hadi Salim
Signed-off-by: David S. Miller -
So that we don't need to play with singly linked list,
and since the code is not on hot path, we can use spinlock
instead of rwlock.Cc: Jamal Hadi Salim
Cc: David S. Miller
Signed-off-by: Cong Wang
Signed-off-by: Jamal Hadi Salim
Signed-off-by: David S. Miller -
It looks weird to store the lock out of the struct but
still points to a static variable. Just move them into the struct.Cc: Jamal Hadi Salim
Cc: David S. Miller
Signed-off-by: Cong Wang
Signed-off-by: Jamal Hadi Salim
Signed-off-by: David S. Miller -
Currently actions are chained by a singly linked list,
therefore it is a bit hard to add and remove a specific
entry. Convert it to struct list_head so that in the
latter patch we can remove an action without finding
its head.Cc: Jamal Hadi Salim
Cc: David S. Miller
Signed-off-by: Cong Wang
Signed-off-by: Jamal Hadi Salim
Signed-off-by: David S. Miller -
It is not used.
Cc: Jamal Hadi Salim
Cc: David S. Miller
Signed-off-by: Cong Wang
Signed-off-by: Jamal Hadi Salim
Signed-off-by: David S. Miller
01 Aug, 2013
1 commit
-
There are a mix of function prototypes with and without extern
in the kernel sources. Standardize on not using extern for
function prototypes.Function prototypes don't need to be written with extern.
extern is assumed by the compiler. Its use is as unnecessary as
using auto to declare automatic/local variables in a block.Reflow modified prototypes to 80 columns.
Signed-off-by: Joe Perches
Signed-off-by: David S. Miller
11 Jun, 2013
1 commit
-
struct gnet_stats_rate_est contains u32 fields, so the bytes per second
field can wrap at 34360Mbit.Add a new gnet_stats_rate_est64 structure to get 64bit bps/pps fields,
and switch the kernel to use this structure natively.This structure is dumped to user space as a new attribute :
TCA_STATS_RATE_EST64
Old tc command will now display the capped bps (to 34360Mbit), instead
of wrapped values, and updated tc command will display correct
information.Old tc command output, after patch :
eric:~# tc -s -d qd sh dev lo
qdisc pfifo 8001: root refcnt 2 limit 1000p
Sent 80868245400 bytes 1978837 pkt (dropped 0, overlimits 0 requeues 0)
rate 34360Mbit 189696pps backlog 0b 0p requeues 0This patch carefully reorganizes "struct Qdisc" layout to get optimal
performance on SMP.Signed-off-by: Eric Dumazet
Cc: Ben Hutchings
Signed-off-by: David S. Miller
13 Feb, 2013
1 commit
-
It's not used anywhere else, so move it.
Signed-off-by: Jiri Pirko
Acked-by: Jamal Hadi Salim
Acked-by: Eric Dumazet
Signed-off-by: David S. Miller
15 Jan, 2013
1 commit
-
Eric Dumazet pointed out that act_mirred needs to find the current net_ns,
and struct net pointer is not provided in the call chain. His original
patch made use of current->nsproxy->net_ns to find the network namespace,
but this fails to work correctly for userspace code that makes use of
netlink sockets in different network namespaces. Instead, pass the
"struct net *" down along the call chain to where it is needed.This version removes the ifb changes as Eric has submitted that patch
separately, but is otherwise identical to the previous version.Signed-off-by: Benjamin LaHaise
Tested-by: Eric Dumazet
Acked-by: Jamal Hadi Salim
Signed-off-by: David S. Miller