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

    WANG Cong
     
  • 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

    WANG Cong
     
  • 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

    WANG Cong
     

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

    WANG Cong
     
  • 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 actions

    The 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

    WANG Cong
     
  • 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

    Ido Schimmel
     

16 Jun, 2016

1 commit


08 Jun, 2016

3 commits


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

    Arnd Bergmann
     

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

    Amir Vadai
     

07 Apr, 2016

1 commit


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

    Amir Vadai
     

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

    WANG Cong
     
  • 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

    WANG Cong
     

27 Aug, 2015

1 commit


01 Aug, 2015

1 commit


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 1

    Test 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 1

    What 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

    Daniel Borkmann
     

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 drop

    Sender 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

    Eric Dumazet
     
  • 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

    Eric Dumazet
     

13 Feb, 2014

4 commits


22 Jan, 2014

2 commits


20 Jan, 2014

1 commit


14 Jan, 2014

2 commits


02 Jan, 2014

1 commit


19 Dec, 2013

5 commits


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

    Joe Perches
     

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 0

    This 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

    Eric Dumazet
     

13 Feb, 2013

1 commit


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

    Benjamin LaHaise