09 Nov, 2017

1 commit

  • Hold netns refcnt before call_rcu() and release it after
    the tcf_exts_destroy() is done.

    Note, on ->destroy() path we have to respect the return value
    of tcf_exts_get_net(), on other paths it should always return
    true, so we don't need to care.

    Cc: Lucas Bates
    Cc: Jamal Hadi Salim
    Cc: Jiri Pirko
    Signed-off-by: Cong Wang
    Signed-off-by: David S. Miller

    Cong Wang
     

29 Oct, 2017

1 commit

  • Defer the tcf_exts_destroy() in RCU callback to
    tc filter workqueue and get RTNL lock.

    Reported-by: Chris Mi
    Cc: Daniel Borkmann
    Cc: Jiri Pirko
    Cc: John Fastabend
    Cc: Jamal Hadi Salim
    Cc: "Paul E. McKenney"
    Signed-off-by: Cong Wang
    Signed-off-by: David S. Miller

    Cong Wang
     

01 Sep, 2017

1 commit

  • TC filters when used as classifiers are bound to TC classes.
    However, there is a hidden difference when adding them in different
    orders:

    1. If we add tc classes before its filters, everything is fine.
    Logically, the classes exist before we specify their ID's in
    filters, it is easy to bind them together, just as in the current
    code base.

    2. If we add tc filters before the tc classes they bind, we have to
    do dynamic lookup in fast path. What's worse, this happens all
    the time not just once, because on fast path tcf_result is passed
    on stack, there is no way to propagate back to the one in tc filters.

    This hidden difference hurts performance silently if we have many tc
    classes in hierarchy.

    This patch intends to close this gap by doing the reverse binding when
    we create a new class, in this case we can actually search all the
    filters in its parent, match and fixup by classid. And because
    tcf_result is specific to each type of tc filter, we have to introduce
    a new ops for each filter to tell how to bind the class.

    Note, we still can NOT totally get rid of those class lookup in
    ->enqueue() because cgroup and flow filters have no way to determine
    the classid at setup time, they still have to go through dynamic lookup.

    Cc: Jamal Hadi Salim
    Signed-off-by: Cong Wang
    Signed-off-by: David S. Miller

    Cong Wang
     

08 Aug, 2017

1 commit

  • Now we use 'unsigned long fh' as a pointer in every place,
    it is safe to convert it to a void pointer now. This gets
    rid of many casts to pointer.

    Cc: Jamal Hadi Salim
    Cc: Jiri Pirko
    Signed-off-by: Cong Wang
    Acked-by: Jamal Hadi Salim
    Signed-off-by: David S. Miller

    WANG Cong
     

05 Aug, 2017

2 commits


22 Apr, 2017

2 commits

  • There is no need to NULL tp->root in ->destroy(), since tp is
    going to be freed very soon, and existing readers are still
    safe to read them.

    For cls_route, we always init its tp->root, so it can't be NULL,
    we can drop more useless code.

    Cc: Daniel Borkmann
    Cc: John Fastabend
    Cc: Jamal Hadi Salim
    Signed-off-by: Cong Wang
    Signed-off-by: David S. Miller

    WANG Cong
     
  • We could have a race condition where in ->classify() path we
    dereference tp->root and meanwhile a parallel ->destroy() makes it
    a NULL. Daniel cured this bug in commit d936377414fa
    ("net, sched: respect rcu grace period on cls destruction").

    This happens when ->destroy() is called for deleting a filter to
    check if we are the last one in tp, this tp is still linked and
    visible at that time. The root cause of this problem is the semantic
    of ->destroy(), it does two things (for non-force case):

    1) check if tp is empty
    2) if tp is empty we could really destroy it

    and its caller, if cares, needs to check its return value to see if it
    is really destroyed. Therefore we can't unlink tp unless we know it is
    empty.

    As suggested by Daniel, we could actually move the test logic to ->delete()
    so that we can safely unlink tp after ->delete() tells us the last one is
    just deleted and before ->destroy().

    Fixes: 1e052be69d04 ("net_sched: destroy proto tp when all filters are gone")
    Cc: Roi Dayan
    Cc: Daniel Borkmann
    Cc: John Fastabend
    Cc: Jamal Hadi Salim
    Signed-off-by: Cong Wang
    Acked-by: Daniel Borkmann
    Signed-off-by: David S. Miller

    WANG Cong
     

14 Apr, 2017

1 commit


23 Sep, 2016

1 commit

  • On error path in route4_change(), 'f' could be NULL,
    so we should check NULL before calling tcf_exts_destroy().

    Fixes: b9a24bb76bf6 ("net_sched: properly handle failure case of tcf_exts_init()")
    Reported-by: kbuild test robot
    Cc: Jamal Hadi Salim
    Signed-off-by: Cong Wang
    Acked-by: Jamal Hadi Salim
    Signed-off-by: David S. Miller

    WANG Cong
     

20 Sep, 2016

1 commit


23 Aug, 2016

1 commit

  • After commit 22dc13c837c3 ("net_sched: convert tcf_exts from list to pointer array")
    we do dynamic allocation in tcf_exts_init(), therefore we need
    to handle the ENOMEM case properly.

    Cc: Jamal Hadi Salim
    Signed-off-by: Cong Wang
    Acked-by: Jamal Hadi Salim
    Acked-by: Jamal Hadi Salim
    Signed-off-by: David S. Miller

    WANG Cong
     

10 Mar, 2015

1 commit

  • Kernel automatically creates a tp for each
    (kind, protocol, priority) tuple, which has handle 0,
    when we add a new filter, but it still is left there
    after we remove our own, unless we don't specify the
    handle (literally means all the filters under
    the tuple). For example this one is left:

    # tc filter show dev eth0
    filter parent 8001: protocol arp pref 49152 basic

    The user-space is hard to clean up these for kernel
    because filters like u32 are organized in a complex way.
    So kernel is responsible to remove it after all filters
    are gone. Each type of filter has its own way to
    store the filters, so each type has to provide its
    way to check if all filters are gone.

    Cc: Jamal Hadi Salim
    Signed-off-by: Cong Wang
    Signed-off-by: Cong Wang
    Acked-by: Jamal Hadi Salim
    Signed-off-by: David S. Miller

    Cong Wang
     

06 Mar, 2015

1 commit


10 Dec, 2014

2 commits


08 Oct, 2014

1 commit

  • Testing xmit_more support with netperf and connected UDP sockets,
    I found strange dst refcount false sharing.

    Current handling of IFF_XMIT_DST_RELEASE is not optimal.

    Dropping dst in validate_xmit_skb() is certainly too late in case
    packet was queued by cpu X but dequeued by cpu Y

    The logical point to take care of drop/force is in __dev_queue_xmit()
    before even taking qdisc lock.

    As Julian Anastasov pointed out, need for skb_dst() might come from some
    packet schedulers or classifiers.

    This patch adds new helper to cleanly express needs of various drivers
    or qdiscs/classifiers.

    Drivers that need skb_dst() in their ndo_start_xmit() should call
    following helper in their setup instead of the prior :

    dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
    ->
    netif_keep_dst(dev);

    Instead of using a single bit, we use two bits, one being
    eventually rebuilt in bonding/team drivers.

    The other one, is permanent and blocks IFF_XMIT_DST_RELEASE being
    rebuilt in bonding/team. Eventually, we could add something
    smarter later.

    Signed-off-by: Eric Dumazet
    Cc: Julian Anastasov
    Signed-off-by: David S. Miller

    Eric Dumazet
     

07 Oct, 2014

1 commit

  • Using the tcf_proto pointer 'tp' from inside the classifiers callback
    is not valid because it may have been cleaned up by another call_rcu
    occuring on another CPU.

    'tp' is currently being used by tcf_unbind_filter() in this patch we
    move instances of tcf_unbind_filter outside of the call_rcu() context.
    This is safe to do because any running schedulers will either read the
    valid class field or it will be zeroed.

    And all schedulers today when the class is 0 do a lookup using the
    same call used by the tcf_exts_bind(). So even if we have a running
    classifier hit the null class pointer it will do a lookup and get
    to the same result. This is particularly fragile at the moment because
    the only way to verify this is to audit the schedulers call sites.

    Reported-by: Cong Wang
    Signed-off-by: John Fastabend
    Acked-by: Cong Wang
    Signed-off-by: David S. Miller

    John Fastabend
     

29 Sep, 2014

1 commit


14 Sep, 2014

1 commit

  • RCUify the route classifier. For now however spinlock's are used to
    protect fastmap cache.

    The issue here is the fastmap may be read by one CPU while the
    cache is being updated by another. An array of pointers could be
    one possible solution.

    Signed-off-by: John Fastabend
    Acked-by: Eric Dumazet
    Signed-off-by: David S. Miller

    John Fastabend
     

28 Apr, 2014

1 commit

  • When actions are attached to a filter, they are a part of the filter
    itself, so when changing a filter we should allow to overwrite the actions
    inside as well.

    In my specific case, when I tried to _append_ a new action to an existing
    filter which already has an action, I got EEXIST since kernel refused
    to overwrite the existing one in kernel.

    This patch checks if we are changing the filter checking NLM_F_CREATE flag
    (Sigh, filters don't use NLM_F_REPLACE...) and then passes the boolean down
    to actions. This fixes the problem above.

    Cc: Jamal Hadi Salim
    Cc: David S. Miller
    Signed-off-by: Cong Wang
    Signed-off-by: Cong Wang
    Signed-off-by: Jamal Hadi Salim
    Signed-off-by: David S. Miller

    Cong Wang
     

14 Jan, 2014

2 commits


19 Dec, 2013

2 commits

  • These information can be saved in tcf_exts, and this will
    simplify the 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

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

    WANG Cong
     

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
     

15 Aug, 2012

1 commit

  • cls_flow.c plays with uids and gids. Unless I misread that
    code it is possible for classifiers to depend on the specific uid and
    gid values. Therefore I need to know the user namespace of the
    netlink socket that is installing the packet classifiers. Pass
    in the rtnetlink skb so I can access the NETLINK_CB of the passed
    packet. In particular I want access to sk_user_ns(NETLINK_CB(in_skb).ssk).

    Pass in not the user namespace but the incomming rtnetlink skb into
    the the classifier change routines as that is generally the more useful
    parameter.

    Cc: Jamal Hadi Salim
    Acked-by: David S. Miller
    Acked-by: Serge Hallyn
    Signed-off-by: Eric W. Biederman

    Eric W. Biederman
     

24 Jul, 2012

1 commit

  • Use inet_iif() consistently, and for TCP record the input interface of
    cached RX dst in inet sock.

    rt->rt_iif is going to be encoded differently, so that we can
    legitimately cache input routes in the FIB info more aggressively.

    When the input interface is "use SKB device index" the rt->rt_iif will
    be set to zero.

    This forces us to move the TCP RX dst cache installation into the ipv4
    specific code, and as well it should since doing the route caching for
    ipv6 is pointless at the moment since it is not inspected in the ipv6
    input paths yet.

    Also, remove the unlikely on dst->obsolete, all ipv4 dsts have
    obsolete set to a non-zero value to force invocation of the check
    callback.

    Signed-off-by: David S. Miller

    David S. Miller
     

02 Apr, 2012

1 commit


06 Jul, 2011

1 commit


05 Mar, 2011

1 commit


20 Jan, 2011

1 commit


30 Mar, 2010

1 commit

  • …it slab.h inclusion from percpu.h

    percpu.h is included by sched.h and module.h and thus ends up being
    included when building most .c files. percpu.h includes slab.h which
    in turn includes gfp.h making everything defined by the two files
    universally available and complicating inclusion dependencies.

    percpu.h -> slab.h dependency is about to be removed. Prepare for
    this change by updating users of gfp and slab facilities include those
    headers directly instead of assuming availability. As this conversion
    needs to touch large number of source files, the following script is
    used as the basis of conversion.

    http://userweb.kernel.org/~tj/misc/slabh-sweep.py

    The script does the followings.

    * Scan files for gfp and slab usages and update includes such that
    only the necessary includes are there. ie. if only gfp is used,
    gfp.h, if slab is used, slab.h.

    * When the script inserts a new include, it looks at the include
    blocks and try to put the new include such that its order conforms
    to its surrounding. It's put in the include block which contains
    core kernel includes, in the same order that the rest are ordered -
    alphabetical, Christmas tree, rev-Xmas-tree or at the end if there
    doesn't seem to be any matching order.

    * If the script can't find a place to put a new include (mostly
    because the file doesn't have fitting include block), it prints out
    an error message indicating which .h file needs to be added to the
    file.

    The conversion was done in the following steps.

    1. The initial automatic conversion of all .c files updated slightly
    over 4000 files, deleting around 700 includes and adding ~480 gfp.h
    and ~3000 slab.h inclusions. The script emitted errors for ~400
    files.

    2. Each error was manually checked. Some didn't need the inclusion,
    some needed manual addition while adding it to implementation .h or
    embedding .c file was more appropriate for others. This step added
    inclusions to around 150 files.

    3. The script was run again and the output was compared to the edits
    from #2 to make sure no file was left behind.

    4. Several build tests were done and a couple of problems were fixed.
    e.g. lib/decompress_*.c used malloc/free() wrappers around slab
    APIs requiring slab.h to be added manually.

    5. The script was run on all .h files but without automatically
    editing them as sprinkling gfp.h and slab.h inclusions around .h
    files could easily lead to inclusion dependency hell. Most gfp.h
    inclusion directives were ignored as stuff from gfp.h was usually
    wildly available and often used in preprocessor macros. Each
    slab.h inclusion directive was examined and added manually as
    necessary.

    6. percpu.h was updated not to include slab.h.

    7. Build test were done on the following configurations and failures
    were fixed. CONFIG_GCOV_KERNEL was turned off for all tests (as my
    distributed build env didn't work with gcov compiles) and a few
    more options had to be turned off depending on archs to make things
    build (like ipr on powerpc/64 which failed due to missing writeq).

    * x86 and x86_64 UP and SMP allmodconfig and a custom test config.
    * powerpc and powerpc64 SMP allmodconfig
    * sparc and sparc64 SMP allmodconfig
    * ia64 SMP allmodconfig
    * s390 SMP allmodconfig
    * alpha SMP allmodconfig
    * um on x86_64 SMP allmodconfig

    8. percpu.h modifications were reverted so that it could be applied as
    a separate patch and serve as bisection point.

    Given the fact that I had only a couple of failures from tests on step
    6, I'm fairly confident about the coverage of this conversion patch.
    If there is a breakage, it's likely to be something in one of the arch
    headers which should be easily discoverable easily on most builds of
    the specific arch.

    Signed-off-by: Tejun Heo <tj@kernel.org>
    Guess-its-ok-by: Christoph Lameter <cl@linux-foundation.org>
    Cc: Ingo Molnar <mingo@redhat.com>
    Cc: Lee Schermerhorn <Lee.Schermerhorn@hp.com>

    Tejun Heo
     

03 Jun, 2009

1 commit

  • Define three accessors to get/set dst attached to a skb

    struct dst_entry *skb_dst(const struct sk_buff *skb)

    void skb_dst_set(struct sk_buff *skb, struct dst_entry *dst)

    void skb_dst_drop(struct sk_buff *skb)
    This one should replace occurrences of :
    dst_release(skb->dst)
    skb->dst = NULL;

    Delete skb->dst field

    Signed-off-by: Eric Dumazet
    Signed-off-by: David S. Miller

    Eric Dumazet
     

20 Nov, 2008

1 commit

  • The use of xchg() hasn't been necessary since 2.2.something when proper
    locking was added to packet schedulers. In the case of classifiers they
    mostly weren't even necessary before that since they're mainly used
    to assign a NULL pointer to the filter root in the ->destroy path;
    the root is destroyed immediately after that.

    Signed-off-by: Patrick McHardy
    Signed-off-by: David S. Miller

    Patrick McHardy
     

30 Aug, 2008

1 commit

  • Use qdisc_root_sleeping_lock() instead of qdisc_root_lock() where
    appropriate. The only difference is while dev is deactivated, when
    currently we can use a sleeping qdisc with the lock of noop_qdisc.
    This shouldn't be dangerous since after deactivation root lock could
    be used only by gen_estimator code, but looks wrong anyway.

    Signed-off-by: Jarek Poplawski
    Signed-off-by: David S. Miller

    Jarek Poplawski
     

18 Jul, 2008

1 commit


09 Jul, 2008

1 commit


01 Feb, 2008

1 commit


29 Jan, 2008

1 commit