17 Dec, 2017

1 commit

  • [ Upstream commit 0c19f846d582af919db66a5914a0189f9f92c936 ]

    Tuntap and similar devices can inject GSO packets. Accept type
    VIRTIO_NET_HDR_GSO_UDP, even though not generating UFO natively.

    Processes are expected to use feature negotiation such as TUNSETOFFLOAD
    to detect supported offload types and refrain from injecting other
    packets. This process breaks down with live migration: guest kernels
    do not renegotiate flags, so destination hosts need to expose all
    features that the source host does.

    Partially revert the UFO removal from 182e0b6b5846~1..d9d30adf5677.
    This patch introduces nearly(*) no new code to simplify verification.
    It brings back verbatim tuntap UFO negotiation, VIRTIO_NET_HDR_GSO_UDP
    insertion and software UFO segmentation.

    It does not reinstate protocol stack support, hardware offload
    (NETIF_F_UFO), SKB_GSO_UDP tunneling in SKB_GSO_SOFTWARE or reception
    of VIRTIO_NET_HDR_GSO_UDP packets in tuntap.

    To support SKB_GSO_UDP reappearing in the stack, also reinstate
    logic in act_csum and openvswitch. Achieve equivalence with v4.13 HEAD
    by squashing in commit 939912216fa8 ("net: skb_needs_check() removes
    CHECKSUM_UNNECESSARY check for tx.") and reverting commit 8d63bee643f1
    ("net: avoid skb_warn_bad_offload false positives on UFO").

    (*) To avoid having to bring back skb_shinfo(skb)->ip6_frag_id,
    ipv6_proxy_select_ident is changed to return a __be32 and this is
    assigned directly to the frag_hdr. Also, SKB_GSO_UDP is inserted
    at the end of the enum to minimize code churn.

    Tested
    Booted a v4.13 guest kernel with QEMU. On a host kernel before this
    patch `ethtool -k eth0` shows UFO disabled. After the patch, it is
    enabled, same as on a v4.13 host kernel.

    A UFO packet sent from the guest appears on the tap device:
    host:
    nc -l -p -u 8000 &
    tcpdump -n -i tap0

    guest:
    dd if=/dev/zero of=payload.txt bs=1 count=2000
    nc -u 192.16.1.1 8000 < payload.txt

    Direct tap to tap transmission of VIRTIO_NET_HDR_GSO_UDP succeeds,
    packets arriving fragmented:

    ./with_tap_pair.sh ./tap_send_ufo tap0 tap1
    (from https://github.com/wdebruij/kerneltools/tree/master/tests)

    Changes
    v1 -> v2
    - simplified set_offload change (review comment)
    - documented test procedure

    Link: http://lkml.kernel.org/r/
    Fixes: fb652fdfe837 ("macvlan/macvtap: Remove NETIF_F_UFO advertisement.")
    Reported-by: Michal Kubecek
    Signed-off-by: Willem de Bruijn
    Acked-by: Jason Wang
    Signed-off-by: David S. Miller
    Signed-off-by: Greg Kroah-Hartman

    Willem de Bruijn
     

09 Nov, 2017

1 commit

  • This reverts commit ceffcc5e254b450e6159f173e4538215cebf1b59.
    If we hold that refcnt, the netns can never be destroyed until
    all actions are destroyed by user, this breaks our netns design
    which we expect all actions are destroyed when we destroy the
    whole netns.

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

    Cong Wang
     

03 Nov, 2017

1 commit

  • TC actions have been destroyed asynchronously for a long time,
    previously in a RCU callback and now in a workqueue. If we
    don't hold a refcnt for its netns, we could use the per netns
    data structure, struct tcf_idrinfo, after it has been freed by
    netns workqueue.

    Hold refcnt to ensure netns destroy happens after all actions
    are gone.

    Fixes: ddf97ccdd7cb ("net_sched: add network namespace support for tc actions")
    Reported-by: Lucas Bates
    Tested-by: Lucas Bates
    Cc: Jamal Hadi Salim
    Cc: Jiri Pirko
    Signed-off-by: Cong Wang
    Signed-off-by: David S. Miller

    Cong Wang
     

31 Aug, 2017

1 commit

  • Typically, each TC filter has its own action. All the actions of the
    same type are saved in its hash table. But the hash buckets are too
    small that it degrades to a list. And the performance is greatly
    affected. For example, it takes about 0m11.914s to insert 64K rules.
    If we convert the hash table to IDR, it only takes about 0m1.500s.
    The improvement is huge.

    But please note that the test result is based on previous patch that
    cls_flower uses IDR.

    Signed-off-by: Chris Mi
    Signed-off-by: Jiri Pirko
    Acked-by: Jamal Hadi Salim
    Signed-off-by: David S. Miller

    Chris Mi
     

18 Jul, 2017

1 commit


20 May, 2017

1 commit

  • skb->csum_not_inet carries the indication on which algorithm is needed to
    compute checksum on skb in the transmit path, when skb->ip_summed is equal
    to CHECKSUM_PARTIAL. If skb carries a SCTP packet and crc32c hasn't been
    yet written in L4 header, skb->csum_not_inet is assigned to 1; otherwise,
    assume Internet Checksum is needed and thus set skb->csum_not_inet to 0.

    Suggested-by: Tom Herbert
    Signed-off-by: Davide Caratti
    Acked-by: Tom Herbert
    Signed-off-by: David S. Miller

    Davide Caratti
     

14 Apr, 2017

1 commit


24 Mar, 2017

1 commit

  • after act_csum computes the checksum on skbs carrying GSO TCP/UDP packets,
    subsequent segmentation fails because skb_needs_check(skb, true) returns
    true. Because of that, skb_warn_bad_offload() is invoked and the following
    message is displayed:

    WARNING: CPU: 3 PID: 28 at net/core/dev.c:2553 skb_warn_bad_offload+0xf0/0xfd

    [] skb_warn_bad_offload+0xf0/0xfd
    [] __skb_gso_segment+0xec/0x110
    [] validate_xmit_skb+0x12d/0x2b0
    [] validate_xmit_skb_list+0x42/0x70
    [] sch_direct_xmit+0xd0/0x1b0
    [] __qdisc_run+0x120/0x270
    [] __dev_queue_xmit+0x23d/0x690
    [] dev_queue_xmit+0x10/0x20

    Since GSO is able to compute checksum on individual segments of such skbs,
    we can simply skip mangling the packet.

    Signed-off-by: Davide Caratti
    Signed-off-by: David S. Miller

    Davide Caratti
     

10 Jan, 2017

1 commit


18 Nov, 2016

1 commit

  • Make struct pernet_operations::id unsigned.

    There are 2 reasons to do so:

    1)
    This field is really an index into an zero based array and
    thus is unsigned entity. Using negative value is out-of-bound
    access by definition.

    2)
    On x86_64 unsigned 32-bit data which are mixed with pointers
    via array indexing or offsets added or subtracted to pointers
    are preffered to signed 32-bit data.

    "int" being used as an array index needs to be sign-extended
    to 64-bit before being used.

    void f(long *p, int i)
    {
    g(p[i]);
    }

    roughly translates to

    movsx rsi, esi
    mov rdi, [rsi+...]
    call g

    MOVSX is 3 byte instruction which isn't necessary if the variable is
    unsigned because x86_64 is zero extending by default.

    Now, there is net_generic() function which, you guessed it right, uses
    "int" as an array index:

    static inline void *net_generic(const struct net *net, int id)
    {
    ...
    ptr = ng->ptr[id - 1];
    ...
    }

    And this function is used a lot, so those sign extensions add up.

    Patch snipes ~1730 bytes on allyesconfig kernel (without all junk
    messing with code generation):

    add/remove: 0/0 grow/shrink: 70/598 up/down: 396/-2126 (-1730)

    Unfortunately some functions actually grow bigger.
    This is a semmingly random artefact of code generation with register
    allocator being used differently. gcc decides that some variable
    needs to live in new r8+ registers and every access now requires REX
    prefix. Or it is shifted into r12, so [r12+0] addressing mode has to be
    used which is longer than [r8]

    However, overall balance is in negative direction:

    add/remove: 0/0 grow/shrink: 70/598 up/down: 396/-2126 (-1730)
    function old new delta
    nfsd4_lock 3886 3959 +73
    tipc_link_build_proto_msg 1096 1140 +44
    mac80211_hwsim_new_radio 2776 2808 +32
    tipc_mon_rcv 1032 1058 +26
    svcauth_gss_legacy_init 1413 1429 +16
    tipc_bcbase_select_primary 379 392 +13
    nfsd4_exchange_id 1247 1260 +13
    nfsd4_setclientid_confirm 782 793 +11
    ...
    put_client_renew_locked 494 480 -14
    ip_set_sockfn_get 730 716 -14
    geneve_sock_add 829 813 -16
    nfsd4_sequence_done 721 703 -18
    nlmclnt_lookup_host 708 686 -22
    nfsd4_lockt 1085 1063 -22
    nfs_get_client 1077 1050 -27
    tcf_bpf_init 1106 1076 -30
    nfsd4_encode_fattr 5997 5930 -67
    Total: Before=154856051, After=154854321, chg -0.00%

    Signed-off-by: Alexey Dobriyan
    Signed-off-by: David S. Miller

    Alexey Dobriyan
     

20 Sep, 2016

1 commit


26 Jul, 2016

1 commit

  • 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
     

08 Jun, 2016

3 commits


27 Apr, 2016

1 commit


26 Feb, 2016

1 commit

  • 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
     

22 Feb, 2016

1 commit

  • When we're dealing with clones and the area is not writeable, try
    harder and get a copy via pskb_expand_head(). Replace also other
    occurences in tc actions with the new skb_try_make_writable().

    Reported-by: Ashhad Sheikh
    Signed-off-by: Daniel Borkmann
    Acked-by: Alexei Starovoitov
    Signed-off-by: David S. Miller

    Daniel Borkmann
     

09 Jul, 2015

1 commit

  • 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
     

14 Jan, 2015

1 commit

  • tc code implicitly considers skb->protocol even in case of accelerated
    vlan paths and expects vlan protocol type here. However, on rx path,
    if the vlan header was already stripped, skb->protocol contains value
    of next header. Similar situation is on tx path.

    So for skbs that use skb->vlan_tci for tagging, use skb->vlan_proto instead.

    Reported-by: Jamal Hadi Salim
    Signed-off-by: Jiri Pirko
    Acked-by: Jamal Hadi Salim
    Signed-off-by: David S. Miller

    Jiri Pirko
     

13 Feb, 2014

3 commits


22 Jan, 2014

1 commit


20 Jan, 2014

1 commit


14 Jan, 2014

1 commit


07 Jan, 2014

1 commit


28 Dec, 2013

1 commit

  • This is a bug fix. The existing code tries to kill many
    birds with one stone: Handling binding of actions to
    filters, new actions and replacing of action
    attributes. A simple test case to illustrate:

    XXXX
    moja@fe1:~$ sudo tc actions add action drop index 12
    moja@fe1:~$ actions get action gact index 12
    action order 1: gact action drop
    random type none pass val 0
    index 12 ref 1 bind 0
    moja@fe1:~$ sudo tc actions replace action ok index 12
    moja@fe1:~$ actions get action gact index 12
    action order 1: gact action drop
    random type none pass val 0
    index 12 ref 2 bind 0
    XXXX

    The above shows the refcounf being wrongly incremented on replace.
    There are more complex scenarios with binding of actions to filters
    that i am leaving out that didnt work as well...

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

    Jamal Hadi Salim
     

21 Dec, 2013

1 commit

  • This patch fixes:
    1) pass mask rather than size to tcf_hashinfo_init()
    2) the cleanup should be in reversed order in mirred_cleanup_module()

    Reported-by: Eric Dumazet
    Fixes: 369ba56787d7469c0afd ("net_sched: init struct tcf_hashinfo at register time")
    Cc: Eric Dumazet
    Cc: David S. Miller
    Signed-off-by: Cong Wang
    Signed-off-by: Jamal Hadi Salim
    Signed-off-by: David S. Miller

    WANG Cong
     

19 Dec, 2013

1 commit


06 Dec, 2013

2 commits


13 Apr, 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
     

18 May, 2012

1 commit


02 Apr, 2012

1 commit


06 Jul, 2011

1 commit


20 Jan, 2011

1 commit


11 Jan, 2011

1 commit

  • HTB takes into account skb is segmented in stats updates.
    Generalize this to all schedulers.

    They should use qdisc_bstats_update() helper instead of manipulating
    bstats.bytes and bstats.packets

    Add bstats_update() helper too for classes that use
    gnet_stats_basic_packed fields.

    Note : Right now, TCQ_F_CAN_BYPASS shortcurt can be taken only if no
    stab is setup on qdisc.

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

    Eric Dumazet
     

24 Aug, 2010

1 commit