Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v2] geneve: Fix setting ttl value in collect metadata mode
From: Pravin Shelar @ 2017-09-12 23:43 UTC (permalink / raw)
  To: Haishuang Yan
  Cc: David S. Miller, Girish Moodalbail,
	Linux Kernel Network Developers, linux-kernel
In-Reply-To: <1505199911-21153-1-git-send-email-yanhaishuang@cmss.chinamobile.com>

On Tue, Sep 12, 2017 at 12:05 AM, Haishuang Yan
<yanhaishuang@cmss.chinamobile.com> wrote:
> Similar to vxlan/ipip tunnel, if key->tos is zero in collect metadata
> mode, tos should also fallback to ip{4,6}_dst_hoplimit.
>
> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
>
> ---
> Changes since v2:
>   * Make the commit message more clearer.
> ---
>  drivers/net/geneve.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index f640407..d52a65f 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -834,11 +834,10 @@ static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
>         sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
>         if (geneve->collect_md) {
>                 tos = ip_tunnel_ecn_encap(key->tos, ip_hdr(skb), skb);
> -               ttl = key->ttl;
>         } else {
>                 tos = ip_tunnel_ecn_encap(fl4.flowi4_tos, ip_hdr(skb), skb);
> -               ttl = key->ttl ? : ip4_dst_hoplimit(&rt->dst);
>         }
> +       ttl = key->ttl ? : ip4_dst_hoplimit(&rt->dst);
>         df = key->tun_flags & TUNNEL_DONT_FRAGMENT ? htons(IP_DF) : 0;
>
This changes user API of Geneve collect-metadata mode. I do not see
good reason for this. Why user can not set right TTL for the flow?

^ permalink raw reply

* Re: [PATCH v4 1/2] ip_tunnel: fix ip tunnel lookup in collect_md mode
From: Pravin Shelar @ 2017-09-12 23:41 UTC (permalink / raw)
  To: Haishuang Yan
  Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
	Linux Kernel Network Developers, linux-kernel, Pravin B Shelar
In-Reply-To: <1505209677-12728-1-git-send-email-yanhaishuang@cmss.chinamobile.com>

On Tue, Sep 12, 2017 at 2:47 AM, Haishuang Yan
<yanhaishuang@cmss.chinamobile.com> wrote:
> In collect_md mode, if the tun dev is down, it still can call
> ip_tunnel_rcv to receive on packets, and the rx statistics increase
> improperly.
>
> When the md tunnel is down, it's not neccessary to increase RX drops
> for the tunnel device, packets would be recieved on fallback tunnel,
> and the RX drops on fallback device will be increased as expected.
>
> Fixes: 2e15ea390e6f ("ip_gre: Add support to collect tunnel metadata.")
> Cc: Pravin B Shelar <pshelar@nicira.com>
> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
Acked-by: Pravin B Shelar <pshelar@ovn.org>

^ permalink raw reply

* Memory leaks in conntrack
From: Cong Wang @ 2017-09-12 23:20 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Linux Kernel Network Developers

Hello,

While testing my TC filter patches (so not related to conntrack), the
following memory leaks are shown up:


unreferenced object 0xffff9b19ba551228 (size 128):
  comm "chronyd", pid 338, jiffies 4294910829 (age 53.188s)
  hex dump (first 32 bytes):
    6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
    00 00 00 00 18 00 00 30 00 00 00 00 00 00 00 00  .......0........
  backtrace:
    [<ffffffff9f1e1175>] create_object+0x169/0x2aa
    [<ffffffff9fb77fb2>] kmemleak_alloc+0x25/0x41
    [<ffffffff9f1c47ed>] slab_post_alloc_hook+0x44/0x65
    [<ffffffff9f1ca2db>] __kmalloc_track_caller+0x113/0x146
    [<ffffffff9f193c3b>] __krealloc+0x4a/0x69
    [<ffffffff9f948dbd>] nf_ct_ext_add+0xe1/0x145
    [<ffffffff9f942395>] init_conntrack+0x1f7/0x36e
    [<ffffffff9f942762>] nf_conntrack_in+0x1d3/0x326
    [<ffffffff9fa1ea69>] ipv4_conntrack_local+0x4d/0x50
    [<ffffffff9f93ad70>] nf_hook_slow+0x3c/0x9b
    [<ffffffff9f9c7999>] nf_hook.constprop.40+0xbe/0xd8
    [<ffffffff9f9c7ba2>] __ip_local_out+0xb3/0xbf
    [<ffffffff9f9c7bca>] ip_local_out+0x1c/0x36
    [<ffffffff9f9c9216>] ip_send_skb+0x19/0x3d
    [<ffffffff9f9ee3de>] udp_send_skb+0x17e/0x1df
    [<ffffffff9f9eea37>] udp_sendmsg+0x5a2/0x77c
unreferenced object 0xffff9b19a69b3340 (size 336):
  comm "chronyd", pid 338, jiffies 4294910868 (age 53.032s)
  hex dump (first 32 bytes):
    01 00 00 00 5a 5a 5a 5a 00 00 00 00 ad 4e ad de  ....ZZZZ.....N..
    ff ff ff ff 5a 5a 5a 5a ff ff ff ff ff ff ff ff  ....ZZZZ........
  backtrace:
    [<ffffffff9f1e1175>] create_object+0x169/0x2aa
    [<ffffffff9fb77fb2>] kmemleak_alloc+0x25/0x41
    [<ffffffff9f1c47ed>] slab_post_alloc_hook+0x44/0x65
    [<ffffffff9f1c7a7d>] kmem_cache_alloc+0xd7/0x1f1
    [<ffffffff9f941b78>] __nf_conntrack_alloc+0xa2/0x146
    [<ffffffff9f942250>] init_conntrack+0xb2/0x36e
    [<ffffffff9f942762>] nf_conntrack_in+0x1d3/0x326
    [<ffffffff9fa1ea69>] ipv4_conntrack_local+0x4d/0x50
    [<ffffffff9f93ad70>] nf_hook_slow+0x3c/0x9b
    [<ffffffff9f9c7999>] nf_hook.constprop.40+0xbe/0xd8
    [<ffffffff9f9c7ba2>] __ip_local_out+0xb3/0xbf
    [<ffffffff9f9c7bca>] ip_local_out+0x1c/0x36
    [<ffffffff9f9c9216>] ip_send_skb+0x19/0x3d
    [<ffffffff9f9ee3de>] udp_send_skb+0x17e/0x1df
    [<ffffffff9f9eea37>] udp_sendmsg+0x5a2/0x77c
    [<ffffffff9f9f8cb8>] inet_sendmsg+0x37/0x5e

This seems new because I never see this before.

I don't touch chronyd in my VM, so I have no idea why it sends out UDP
packets, my guess is it is some periodical packet.

I don't think I use conntrack either, since /proc/net/ip_conntrack
does not exist.

Here are some related config of my kernel:

$ grep CONNTRACK .config
CONFIG_NF_CONNTRACK=y
CONFIG_NF_CONNTRACK_MARK=y
CONFIG_NF_CONNTRACK_SECMARK=y
CONFIG_NF_CONNTRACK_ZONES=y
CONFIG_NF_CONNTRACK_PROCFS=y
CONFIG_NF_CONNTRACK_EVENTS=y
# CONFIG_NF_CONNTRACK_TIMEOUT is not set
CONFIG_NF_CONNTRACK_TIMESTAMP=y
CONFIG_NF_CONNTRACK_AMANDA=y
CONFIG_NF_CONNTRACK_FTP=y
CONFIG_NF_CONNTRACK_H323=y
CONFIG_NF_CONNTRACK_IRC=y
CONFIG_NF_CONNTRACK_BROADCAST=y
CONFIG_NF_CONNTRACK_NETBIOS_NS=y
CONFIG_NF_CONNTRACK_SNMP=y
CONFIG_NF_CONNTRACK_PPTP=y
CONFIG_NF_CONNTRACK_SANE=y
CONFIG_NF_CONNTRACK_SIP=y
CONFIG_NF_CONNTRACK_TFTP=y
CONFIG_NETFILTER_XT_MATCH_CONNTRACK=y
CONFIG_NF_CONNTRACK_IPV4=y
CONFIG_NF_CONNTRACK_IPV6=y

Please let me know if you need any other information.

Thanks.

^ permalink raw reply

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
From: Josef Bacik @ 2017-09-12 23:12 UTC (permalink / raw)
  To: Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Cole Robinson
In-Reply-To: <1a8ef376-387e-e0fc-7362-e1fd2c2c45d3@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1611 bytes --]

First I’m super sorry for the top post, I’m at plumbers and I forgot to upload my muttrc to my new cloud instance, so I’m screwed using outlook.

I have a completely untested, uncompiled patch that I think will fix the problem, would you mind giving it a go?  Thanks,

Josef

On 9/12/17, 3:36 PM, "Laura Abbott" <labbott@redhat.com> wrote:

Hi,

Fedora got a bug report 
https://bugzilla.redhat.com/show_bug.cgi?id=1432684 of a regression with 
automatic spice port
assignment. The libvirt team reduced this to the attached test
case run as follows:

In a separate terminal, qemu-kvm -vnc 127.0.0.1:0 to grab port 5900. 
Then do this:

$ gcc bind-collision.c && ./a.out
bind: Address already in use
AF_INET check failed.
$ gcc -D CHECK_IPV6 bind-collision.c && ./a.out
AF_INET6 success
AF_INET success
$ gcc bind-collision.c && ./a.out
AF_INET success

Bisection showed this behavior to be caused by

commit 319554f284dda9f2737d09df82ba3610bd8ddea3
Author: Josef Bacik <jbacik@fb.com>
Date:   Thu Jan 19 17:47:46 2017 -0500

     inet: don't use sk_v6_rcv_saddr directly

     When comparing two sockets we need to use inet6_rcv_saddr so we get 
a NULL
     sk_v6_rcv_saddr if the socket isn't AF_INET6, otherwise our 
comparison function
     can be wrong.

     Fixes: 637bc8b ("inet: reset tb->fastreuseport when adding a 
reuseport sk")
     Signed-off-by: Josef Bacik <jbacik@fb.com>
     Signed-off-by: David S. Miller <davem@davemloft.net>


And reverting fixed both the standalone test case and the spice issue.

Any ideas?

Thanks,
Laura



[-- Attachment #2: 0001-net-set-tb-fast_sk_family.patch --]
[-- Type: application/octet-stream, Size: 1307 bytes --]

From b3c0458435d87911f1f99d314013b268e6c850a9 Mon Sep 17 00:00:00 2001
From: Josef Bacik <josef@toxicpanda.com>
Date: Tue, 12 Sep 2017 16:07:00 -0700
Subject: [PATCH] net: set tb->fast_sk_family

We need to set the tb->fast_sk_family properly so we can use the proper
comparison function for all subsequent reuseport bind requests.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 net/ipv4/inet_connection_sock.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 4089c013cb03..3cff95f10995 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -328,6 +328,7 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
 			tb->fastuid = uid;
 			tb->fast_rcv_saddr = sk->sk_rcv_saddr;
 			tb->fast_ipv6_only = ipv6_only_sock(sk);
+			tb->fast_sk_family = sk->sk_family;
 #if IS_ENABLED(CONFIG_IPV6)
 			tb->fast_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
 #endif
@@ -354,6 +355,7 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
 				tb->fastuid = uid;
 				tb->fast_rcv_saddr = sk->sk_rcv_saddr;
 				tb->fast_ipv6_only = ipv6_only_sock(sk);
+				tb->fast_sk_family = sk->sk_family;
 #if IS_ENABLED(CONFIG_IPV6)
 				tb->fast_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
 #endif
-- 
2.13.5


^ permalink raw reply related

* Re: [RFC PATCH] net: Introduce a socket option to enable picking tx queue based on rx queue.
From: Tom Herbert @ 2017-09-12 22:53 UTC (permalink / raw)
  To: Samudrala, Sridhar
  Cc: Eric Dumazet, Alexander Duyck, Linux Kernel Network Developers
In-Reply-To: <ef594f90-0f76-bdf5-63ce-e8750ee0d60f@intel.com>

On Tue, Sep 12, 2017 at 3:31 PM, Samudrala, Sridhar
<sridhar.samudrala@intel.com> wrote:
>
>
> On 9/12/2017 8:47 AM, Eric Dumazet wrote:
>>
>> On Mon, 2017-09-11 at 23:27 -0700, Samudrala, Sridhar wrote:
>>>
>>> On 9/11/2017 8:53 PM, Eric Dumazet wrote:
>>>>
>>>> On Mon, 2017-09-11 at 20:12 -0700, Tom Herbert wrote:
>>>>
>>>>> Two ints in sock_common for this purpose is quite expensive and the
>>>>> use case for this is limited-- even if a RX->TX queue mapping were
>>>>> introduced to eliminate the queue pair assumption this still won't
>>>>> help if the receive and transmit interfaces are different for the
>>>>> connection. I think we really need to see some very compelling results
>>>>> to be able to justify this.
>>>
>>> Will try to collect and post some perf data with symmetric queue
>>> configuration.
>>>
>>>> Yes, this is unreasonable cost.
>>>>
>>>> XPS should really cover the case already.
>>>>
>>>
>>> Eric,
>>>
>>> Can you clarify how XPS covers the RX-> TX queue mapping case?
>>> Is it possible to configure XPS to select TX queue based on the RX queue
>>> of a flow?
>>> IIUC, it is based on the CPU of the thread doing the transmit OR based
>>> on skb->priority to TC mapping?
>>> It may be possible to get this effect if the the threads are pinned to a
>>> core, but if the app threads are
>>> freely moving, i am not sure how XPS can be configured to select the TX
>>> queue based on the RX queue of a flow.
>>
>> If application is freely moving, how NIC can properly select the RX
>> queue so that packets are coming to the appropriate queue ?
>
> The RX queue is selected via RSS and we don't want to move the flow based on
> where the thread is running.

Unless flow director is enabled on the Intel device... This was, I
believe, one of the first attempts to introduce a queue pair notion to
general purpose NICs. The idea was that the device records the TX
queue for a flow and then uses that to determine receive queue in a
symmetric fashion. aRFS is similar, but was under SW control how the
mapping is done. As Eric mentioned there are scalability issues with
these mechanisms, but we also found that flow director can easily
reorder packets whenever the thread moves.

>>
>>
>> This is called aRFS, and it does not scale to millions of flows.
>> We tried in the past, and this went nowhere really, since the setup cost
>> is prohibitive and DDOS vulnerable.
>>
>> XPS will follow the thread, since selection is done on current cpu.
>>
>> The problem is RX side. If application is free to migrate, then special
>> support (aRFS) is needed from the hardware.
>
> This may be true if most of the rx processing is happening in the interrupt
> context.
> But with busy polling,  i think we don't need aRFS as a thread should be
> able to poll
> any queue irrespective of where it is running.

It's not just a problem with interrupt processing, in general we like
to have all receive processing an subsequent transmit of a reply to be
done on one CPU. Silo'ing is good for performance and parallelism.
This can sometimes be relaxed in situations where CPUs share a cache
so crossing CPUs is not not costly.

>>
>>
>> At least for passive connections, we already have all the support in the
>> kernel so that you can have one thread per NIC queue, dealing with
>> sockets that have incoming packets all received on one NIC RX queue.
>> (And of course all TX packets will use the symmetric TX queue)
>>
>> SO_REUSEPORT plus appropriate BPF filter can achieve that.
>>
>> Say you have 32 queues, 32 cpus.
>>
>> Simply use 32 listeners, 32 threads (or 32 pools of threads)
>
> Yes. This will work if each thread is pinned to a core associated with the
> RX interrupt.
> It may not be possible to pin the threads to a core.
> Instead we want to associate a thread to a queue and do all the RX and TX
> completion
> of a queue in the same thread context via busy polling.
>
When that happens it's possible for RX to be done on the completely
wrong CPU which we know is suboptimal. However, this shouldn't
negatively affect TX side since XPS will just use the queue
appropriate for running CPU. Like Eric said, this is really a receive
problem more than a transmit problem. Keeping them as independent
paths seems to be a good approach.

Tom

^ permalink raw reply

* Re: [PATCH] datapath: Fix an error handling path in 'ovs_nla_init_match_and_action()'
From: Greg Rose @ 2017-09-12 22:48 UTC (permalink / raw)
  To: Christophe JAILLET, pshelar, davem, xiangxia.m.yue
  Cc: netdev, dev, linux-kernel, kernel-janitors
In-Reply-To: <20170911192015.17553-1-christophe.jaillet@wanadoo.fr>

On 09/11/2017 12:20 PM, Christophe JAILLET wrote:
> All other error handling paths in this function go through the 'error'
> label. This one should do the same.
> 
> Fixes: 9cc9a5cb176c ("datapath: Avoid using stack larger than 1024.")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> I think that the comment above the function could be improved. It looks
> like the commit log which has introduced this function.
> 
> I'm also not sure that commit 9cc9a5cb176c is of any help. It is
> supposed to remove a warning, and I guess it does. But 'ovs_nla_init_match_and_action()'
> is called unconditionnaly from 'ovs_flow_cmd_set()'. So even if the stack
> used by each function is reduced, the overall stack should be the same, if
> not larger.
> 
> So this commit sounds like adding a bug where the code was fine and states
> to fix an issue but, at the best, only hides it.

Having a large stack frame isn't really a bug per se.  But the Linux kernel
warns about stack frames that are too large so reordering the code to
get the warning to go away seems fine to me.

> 
> Instead of fixing the code with the proposed patch, reverting the initial
> commit could also be considered.

Then the warning will come back.

- Greg

> ---
>   net/openvswitch/datapath.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 76cf273a56c7..c3aec6227c91 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -1112,7 +1112,8 @@ static int ovs_nla_init_match_and_action(struct net *net,
>   		if (!a[OVS_FLOW_ATTR_KEY]) {
>   			OVS_NLERR(log,
>   				  "Flow key attribute not present in set flow.");
> -			return -EINVAL;
> +			error = -EINVAL;
> +			goto error;
>   		}
>   
>   		*acts = get_flow_actions(net, a[OVS_FLOW_ATTR_ACTIONS], key,
> 

^ permalink raw reply

* 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
From: Laura Abbott @ 2017-09-12 22:35 UTC (permalink / raw)
  To: Josef Bacik, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI
  Cc: netdev, linux-kernel, Cole Robinson

[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]

Hi,

Fedora got a bug report 
https://bugzilla.redhat.com/show_bug.cgi?id=1432684 of a regression with 
automatic spice port
assignment. The libvirt team reduced this to the attached test
case run as follows:

In a separate terminal, qemu-kvm -vnc 127.0.0.1:0 to grab port 5900. 
Then do this:

$ gcc bind-collision.c && ./a.out
bind: Address already in use
AF_INET check failed.
$ gcc -D CHECK_IPV6 bind-collision.c && ./a.out
AF_INET6 success
AF_INET success
$ gcc bind-collision.c && ./a.out
AF_INET success

Bisection showed this behavior to be caused by

commit 319554f284dda9f2737d09df82ba3610bd8ddea3
Author: Josef Bacik <jbacik@fb.com>
Date:   Thu Jan 19 17:47:46 2017 -0500

     inet: don't use sk_v6_rcv_saddr directly

     When comparing two sockets we need to use inet6_rcv_saddr so we get 
a NULL
     sk_v6_rcv_saddr if the socket isn't AF_INET6, otherwise our 
comparison function
     can be wrong.

     Fixes: 637bc8b ("inet: reset tb->fastreuseport when adding a 
reuseport sk")
     Signed-off-by: Josef Bacik <jbacik@fb.com>
     Signed-off-by: David S. Miller <davem@davemloft.net>


And reverting fixed both the standalone test case and the spice issue.

Any ideas?

Thanks,
Laura

[-- Attachment #2: bind-collision.c --]
[-- Type: text/x-csrc, Size: 2024 bytes --]

#include <arpa/inet.h>
#include <netinet/in.h>
#include <stdbool.h>
#include <stdio.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <unistd.h>

/* Reproducer for https://bugzilla.redhat.com/show_bug.cgi?id=1432684
   Simply do something like: qemu-kvm -vnc 127.0.0.1:0
 */

#define PORT 5900

int check_port(int family) {
    int fd = -1;
    int reuseaddr = 1;
    int v6only = 1;
    int addrlen;
    int ret = -1;
    bool ipv6 = false;
    struct sockaddr *addr;

    struct sockaddr_in6 addr6 = {
        .sin6_family = AF_INET6,
        .sin6_port = htons(PORT),
        .sin6_addr = in6addr_any
    };
    struct sockaddr_in addr4 = {
        .sin_family = AF_INET,
        .sin_port = htons(PORT),
        .sin_addr.s_addr = htonl(INADDR_ANY)
    };


    if (family == AF_INET6) {
        addr = (struct sockaddr*)&addr6;
        addrlen = sizeof(addr6);
        ipv6 = true;
    } else if (family == AF_INET) {
        addr = (struct sockaddr*)&addr4;
        addrlen = sizeof(addr4);
    } else {
        printf("Unknown family\n");
        goto out;
    }

    if ((fd = socket(family, SOCK_STREAM, 0)) < 0) {
        perror("socket");
        goto out;
    }

    if (ipv6 && setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, (void*)&v6only,
                           sizeof(v6only)) < 0) {
        perror("setsockopt IPV6_V6ONLY");
        goto out;
    }

    if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR,
                   &reuseaddr, sizeof(reuseaddr)) < 0) {
        perror("setsockopt SO_REUSEADDR");
        goto out;
    }

    if (bind(fd, addr, addrlen) < 0) {
        perror("bind");
        goto out;
    }

    ret = 0;
out:
    close(fd);
    return ret;
}

int main(void) {
#ifdef CHECK_IPV6
    if (check_port(AF_INET6) < 0) {
        printf("AF_INET6 check failed.\n");
        return -1;
    }
    printf("AF_INET6 success\n");
#endif

    if (check_port(AF_INET) < 0) {
        printf("AF_INET check failed.\n");
        return -1;
    }
    printf("AF_INET success\n");

    return 0;
}

^ permalink raw reply

* Re: [PATCH] ieee802154: fix gcc-4.9 warnings
From: Joe Perches @ 2017-09-12 22:34 UTC (permalink / raw)
  To: Arnd Bergmann, Harry Morris, linuxdev, Alexander Aring,
	Stefan Schmidt, Andrew Morton
  Cc: Marcel Holtmann, David S. Miller, Markus Elfring,
	Gustavo A. R. Silva, Florian Westphal, Johannes Berg,
	Christophe JAILLET, Colin Ian King, linux-wpan, netdev,
	linux-kernel
In-Reply-To: <20170912101636.3811626-1-arnd@arndb.de>

On Tue, 2017-09-12 at 12:16 +0200, Arnd Bergmann wrote:
> All older compiler versions up to gcc-4.9 produce these
> harmless warnings:
> 
> drivers/net/ieee802154/ca8210.c: In function 'ca8210_skb_tx':
> drivers/net/ieee802154/ca8210.c:1947:9: warning: missing braces around initializer [-Wmissing-braces]
> 
> This changes the syntax to something that works on all versions
> without warnings.
> 
> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
[]
> diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
[]
> @@ -1944,7 +1944,7 @@ static int ca8210_skb_tx(
>  )
>  {
>  	int status;
> -	struct ieee802154_hdr header = { 0 };
> +	struct ieee802154_hdr header = { };
>  	struct secspec secspec;
>  	unsigned int mac_len;

Presumably gcc does this because the first member
of struct ieee802154_hdr is another struct.

I wonder if "struct foo bar = { 0 };" should be
discouraged by checkpatch.

Right now it's about 4:3 in favor of
	struct foo bar = {};
over
	struct foo bar = { 0 };

$ git grep -E "struct\s+\w+\s+\w+\s*=\s*\{\s*0\s*\}\s*[,;]" | wc -l
826
$ git grep -E "struct\s+\w+\s+\w+\s*=\s*\{\s*\}\s*[,;]" | wc -l
990

There are many instances on multiple lines too.
The git grep above doesn't span multiple lines.

^ permalink raw reply

* Re: [RFC PATCH] net: Introduce a socket option to enable picking tx queue based on rx queue.
From: Samudrala, Sridhar @ 2017-09-12 22:31 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Tom Herbert, Alexander Duyck, Linux Kernel Network Developers
In-Reply-To: <1505231262.15310.149.camel@edumazet-glaptop3.roam.corp.google.com>



On 9/12/2017 8:47 AM, Eric Dumazet wrote:
> On Mon, 2017-09-11 at 23:27 -0700, Samudrala, Sridhar wrote:
>> On 9/11/2017 8:53 PM, Eric Dumazet wrote:
>>> On Mon, 2017-09-11 at 20:12 -0700, Tom Herbert wrote:
>>>
>>>> Two ints in sock_common for this purpose is quite expensive and the
>>>> use case for this is limited-- even if a RX->TX queue mapping were
>>>> introduced to eliminate the queue pair assumption this still won't
>>>> help if the receive and transmit interfaces are different for the
>>>> connection. I think we really need to see some very compelling results
>>>> to be able to justify this.
>> Will try to collect and post some perf data with symmetric queue
>> configuration.
>>
>>> Yes, this is unreasonable cost.
>>>
>>> XPS should really cover the case already.
>>>    
>> Eric,
>>
>> Can you clarify how XPS covers the RX-> TX queue mapping case?
>> Is it possible to configure XPS to select TX queue based on the RX queue
>> of a flow?
>> IIUC, it is based on the CPU of the thread doing the transmit OR based
>> on skb->priority to TC mapping?
>> It may be possible to get this effect if the the threads are pinned to a
>> core, but if the app threads are
>> freely moving, i am not sure how XPS can be configured to select the TX
>> queue based on the RX queue of a flow.
> If application is freely moving, how NIC can properly select the RX
> queue so that packets are coming to the appropriate queue ?
The RX queue is selected via RSS and we don't want to move the flow based on
where the thread is running.
>
> This is called aRFS, and it does not scale to millions of flows.
> We tried in the past, and this went nowhere really, since the setup cost
> is prohibitive and DDOS vulnerable.
>
> XPS will follow the thread, since selection is done on current cpu.
>
> The problem is RX side. If application is free to migrate, then special
> support (aRFS) is needed from the hardware.
This may be true if most of the rx processing is happening in the 
interrupt context.
But with busy polling,  i think we don't need aRFS as a thread should be 
able to poll
any queue irrespective of where it is running.
>
> At least for passive connections, we already have all the support in the
> kernel so that you can have one thread per NIC queue, dealing with
> sockets that have incoming packets all received on one NIC RX queue.
> (And of course all TX packets will use the symmetric TX queue)
>
> SO_REUSEPORT plus appropriate BPF filter can achieve that.
>
> Say you have 32 queues, 32 cpus.
>
> Simply use 32 listeners, 32 threads (or 32 pools of threads)
Yes. This will work if each thread is pinned to a core associated with 
the RX interrupt.
It may not be possible to pin the threads to a core.
Instead we want to associate a thread to a queue and do all the RX and 
TX completion
of a queue in the same thread context via busy polling.

Thanks
Sridhar

^ permalink raw reply

* (unknown), 
From: marketing @ 2017-09-12 22:07 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: 43594737937.doc --]
[-- Type: application/msword, Size: 76549 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] net: qcom/emac: add software control for pause frame mode
From: Timur Tabi @ 2017-09-12 22:07 UTC (permalink / raw)
  To: David S. Miller, netdev
In-Reply-To: <1501623460-3575-3-git-send-email-timur@codeaurora.org>

On 08/01/2017 04:37 PM, Timur Tabi wrote:
> The EMAC has the option of sending only a single pause frame when
> flow control is enabled and the RX queue is full.  Although sending
> only one pause frame has little value, this would allow admins to
> enable automatic flow control without having to worry about the EMAC
> flooding nearby switches with pause frames if the kernel hangs.
> 
> The option is enabled by using the single-pause-mode private flag.
> 
> Signed-off-by: Timur Tabi<timur@codeaurora.org>

Dave,

I don't see this patch in net-next.  Can you pick it up for 4.14?

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply

* Re: [Patch net v3 1/3] net_sched: get rid of tcfa_rcu
From: Cong Wang @ 2017-09-12 21:53 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Linux Kernel Network Developers, Jiri Pirko, Jakub Kicinski,
	Jamal Hadi Salim, Eric Dumazet
In-Reply-To: <20170912213634.GS2036@nanopsycho>

On Tue, Sep 12, 2017 at 2:36 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Tue, Sep 12, 2017 at 11:10:22PM CEST, xiyou.wangcong@gmail.com wrote:
>>On Tue, Sep 12, 2017 at 3:40 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>>> This patch helps:
>>
>>Looks good to me. Please feel free to submit a formal patch.
>
> Okay, I will send the patch to you formally so you can add it as a first
> patch of your patchset.

I can carry it by myself if it fits to this patchset. However, I believe it
should be independent since it has to be backported much further
than this patchset. I don't know why no one triggered the crash
before call_rcu() was introduced there.

Anyway, I believe you should submit your patch alone, either before
or after this patchset, there should be no conflict.

^ permalink raw reply

* Re: [PATCH net] net: systemport: Fix 64-bit stats deadlock
From: Florian Fainelli @ 2017-09-12 21:48 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, davem, edumazet, jqiaoulk
In-Reply-To: <1505252293.15310.151.camel@edumazet-glaptop3.roam.corp.google.com>

On 09/12/2017 02:38 PM, Eric Dumazet wrote:
> On Tue, 2017-09-12 at 13:14 -0700, Florian Fainelli wrote:
>> We can enter a deadlock situation because there is no sufficient protection
>> when ndo_get_stats64() runs in process context to guard against RX or TX NAPI
>> contexts running in softirq, this can lead to the following lockdep splat and
>> actual deadlock was experienced as well with an iperf session in the background
>> and a while loop doing ifconfig + ethtool.
> 
>> So just remove the u64_stats_update_begin()/end() pair in ndo_get_stats64()
>> since it does not appear to be useful for anything. No inconsistency was
>> observed with either ifconfig or ethtool, global TX counts equal the sum of
>> per-queue TX counts on a 32-bit architecture.
>>
>> Fixes: 10377ba7673d ("net: systemport: Support 64bit statistics")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  drivers/net/ethernet/broadcom/bcmsysport.c | 3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
>> index a6572b51435a..c3c53f6cd9e6 100644
>> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
>> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
>> @@ -1735,11 +1735,8 @@ static void bcm_sysport_get_stats64(struct net_device *dev,
>>  		stats->tx_packets += tx_packets;
>>  	}
>>  
>> -	/* lockless update tx_bytes and tx_packets */
>> -	u64_stats_update_begin(&priv->syncp);
> 
> Yes, this u64_stats_update_begin()/u64_stats_update_end() is bogus
> 
> But why do we even write on tx_bytes/tx_packets here ??? 

That's for the ethtool -S netdev stats copy (that's on me, I added that
in the driver initial version), so yes, not very robust...

> 
> Seems very wrong anyway.
> 
> (ethtool -S does not call bcm_sysport_get_stats64() to refresh them )

Yes that might actually be the simplest way to get this fixed.

> 
>>  	stats64->tx_bytes = stats->tx_bytes;
>>  	stats64->tx_packets = stats->tx_packets;
>> -	u64_stats_update_end(&priv->syncp);
>>  
>>  	do {
>>  		start = u64_stats_fetch_begin_irq(&priv->syncp);
> 
> 


-- 
Florian

^ permalink raw reply

* Re: [PATCH net] net: systemport: Fix 64-bit stats deadlock
From: Eric Dumazet @ 2017-09-12 21:38 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev, davem, edumazet, jqiaoulk
In-Reply-To: <1505247266-42195-1-git-send-email-f.fainelli@gmail.com>

On Tue, 2017-09-12 at 13:14 -0700, Florian Fainelli wrote:
> We can enter a deadlock situation because there is no sufficient protection
> when ndo_get_stats64() runs in process context to guard against RX or TX NAPI
> contexts running in softirq, this can lead to the following lockdep splat and
> actual deadlock was experienced as well with an iperf session in the background
> and a while loop doing ifconfig + ethtool.

> So just remove the u64_stats_update_begin()/end() pair in ndo_get_stats64()
> since it does not appear to be useful for anything. No inconsistency was
> observed with either ifconfig or ethtool, global TX counts equal the sum of
> per-queue TX counts on a 32-bit architecture.
> 
> Fixes: 10377ba7673d ("net: systemport: Support 64bit statistics")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/net/ethernet/broadcom/bcmsysport.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
> index a6572b51435a..c3c53f6cd9e6 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
> @@ -1735,11 +1735,8 @@ static void bcm_sysport_get_stats64(struct net_device *dev,
>  		stats->tx_packets += tx_packets;
>  	}
>  
> -	/* lockless update tx_bytes and tx_packets */
> -	u64_stats_update_begin(&priv->syncp);

Yes, this u64_stats_update_begin()/u64_stats_update_end() is bogus

But why do we even write on tx_bytes/tx_packets here ??? 

Seems very wrong anyway.

(ethtool -S does not call bcm_sysport_get_stats64() to refresh them )

>  	stats64->tx_bytes = stats->tx_bytes;
>  	stats64->tx_packets = stats->tx_packets;
> -	u64_stats_update_end(&priv->syncp);
>  
>  	do {
>  		start = u64_stats_fetch_begin_irq(&priv->syncp);

^ permalink raw reply

* Re: [Patch net v3 1/3] net_sched: get rid of tcfa_rcu
From: Jiri Pirko @ 2017-09-12 21:36 UTC (permalink / raw)
  To: Cong Wang
  Cc: Linux Kernel Network Developers, Jiri Pirko, Jakub Kicinski,
	Jamal Hadi Salim, Eric Dumazet
In-Reply-To: <CAM_iQpWN+OFdWZgKsm8qk58jFKqssOMMW8JM7N6FConHumQXxg@mail.gmail.com>

Tue, Sep 12, 2017 at 11:10:22PM CEST, xiyou.wangcong@gmail.com wrote:
>On Tue, Sep 12, 2017 at 3:40 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Tue, Sep 12, 2017 at 11:42:15AM CEST, jiri@resnulli.us wrote:
>>>Tue, Sep 12, 2017 at 01:33:30AM CEST, xiyou.wangcong@gmail.com wrote:
>>>>gen estimator has been rewritten in commit 1c0d32fde5bd
>>>>("net_sched: gen_estimator: complete rewrite of rate estimators"),
>>>>the caller is no longer needed to wait for a grace period.
>>>>So this patch gets rid of it.
>>>>
>>>>This also completely closes a race condition between action free
>>>>path and filter chain add/remove path for the following patch.
>>>>Because otherwise the nested RCU callback can't be caught by
>>>>rcu_barrier().
>>>>
>>>>Please see also the comments in code.
>>>
>>>Looks like this is causing a null pointer dereference bug for me, 100%
>>>of the time. Just add and remove any rule with action and you get:
>>>
>>
>> [...]
>>
>>>
>>>Looks like you need to save owner of the module before you call
>>>__tcf_idr_release so you can later on use it for module_put
>
>Why do you believe it is this patch introduces the bug?
>
>That code has been there since the beginning of git history:
>
>+       for (a = act; a; a = act) {
>+               if (a->ops && a->ops->cleanup) {
>+                       DPRINTK("tcf_action_destroy destroying %p next %p\n",
>+                               a, a->next);
>+                       if (a->ops->cleanup(a, bind) == ACT_P_DELETED)
>+                               module_put(a->ops->owner);
>+                       act = act->next;
>
>Seems to be a very old one. The reason why it exposes, I guess,
>is call_rcu() somehow delays the free after module_put().

Yeah, looks like the race was just hard to hit. However with your patch,
it is very easy to hit.


>
>
>>
>> This patch helps:
>
>Looks good to me. Please feel free to submit a formal patch.

Okay, I will send the patch to you formally so you can add it as a first
patch of your patchset.

^ permalink raw reply

* Re: [Patch net v3 1/3] net_sched: get rid of tcfa_rcu
From: Cong Wang @ 2017-09-12 21:10 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Linux Kernel Network Developers, Jiri Pirko, Jakub Kicinski,
	Jamal Hadi Salim, Eric Dumazet
In-Reply-To: <20170912104004.GE2036@nanopsycho>

On Tue, Sep 12, 2017 at 3:40 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Tue, Sep 12, 2017 at 11:42:15AM CEST, jiri@resnulli.us wrote:
>>Tue, Sep 12, 2017 at 01:33:30AM CEST, xiyou.wangcong@gmail.com wrote:
>>>gen estimator has been rewritten in commit 1c0d32fde5bd
>>>("net_sched: gen_estimator: complete rewrite of rate estimators"),
>>>the caller is no longer needed to wait for a grace period.
>>>So this patch gets rid of it.
>>>
>>>This also completely closes a race condition between action free
>>>path and filter chain add/remove path for the following patch.
>>>Because otherwise the nested RCU callback can't be caught by
>>>rcu_barrier().
>>>
>>>Please see also the comments in code.
>>
>>Looks like this is causing a null pointer dereference bug for me, 100%
>>of the time. Just add and remove any rule with action and you get:
>>
>
> [...]
>
>>
>>Looks like you need to save owner of the module before you call
>>__tcf_idr_release so you can later on use it for module_put

Why do you believe it is this patch introduces the bug?

That code has been there since the beginning of git history:

+       for (a = act; a; a = act) {
+               if (a->ops && a->ops->cleanup) {
+                       DPRINTK("tcf_action_destroy destroying %p next %p\n",
+                               a, a->next);
+                       if (a->ops->cleanup(a, bind) == ACT_P_DELETED)
+                               module_put(a->ops->owner);
+                       act = act->next;

Seems to be a very old one. The reason why it exposes, I guess,
is call_rcu() somehow delays the free after module_put().


>
> This patch helps:

Looks good to me. Please feel free to submit a formal patch.

^ permalink raw reply

* Re: Can libpcap filter on vlan tags when vlans are hardware-accelerated?
From: Michal Kubecek @ 2017-09-12 20:26 UTC (permalink / raw)
  To: Ben Greear; +Cc: netdev
In-Reply-To: <51a124d5-8a5f-35d4-5242-fd8a289f50b5@candelatech.com>

On Tue, Sep 12, 2017 at 11:54:43AM -0700, Ben Greear wrote:
> It does not appear to work on Fedora-26, and I'm curious if someone
> knows what needs doing to get this support working?

It's rather complicated. The "vlan" and "vlan <id>" filters didn't
handle the case when vlan information is passed in metadata until commit
04660eb1e561 ("Use BPF extensions in compiled filters"), i.e. libpcap
1.7.0. Unfortunately that commit made libpcap always check only metadata
for the first outermost vlan tag so that it broke the case when vlan
information is passed in packet itself (which is less frequent today).

To handle both cases correctly, you would need libpcap with commits
d739b068ac29 ("Make VLAN filter handle both metadata and inline tags")
and 7c7a19fbd9af ("Fix logic of combined VLAN test") and also the
optimizer fix from

  https://github.com/the-tcpdump-group/libpcap/pull/582/commits/075015a3d17a

(without it the filters generate incorrect BPF in some cases unless the
optimizer is disabled). As far as I can see, these commits are not in
any release yet.

                                                       Michal Kubecek

^ permalink raw reply

* Re: [PATCH/RFC net-next 2/2] net/sched: allow flower to match tunnel options
From: Or Gerlitz @ 2017-09-12 20:23 UTC (permalink / raw)
  To: Simon Horman
  Cc: Jiri Pirko, Jamal Hadi Salim, Cong Wang, Linux Netdev List,
	oss-drivers
In-Reply-To: <1505226037-2758-3-git-send-email-simon.horman@netronome.com>

On Tue, Sep 12, 2017 at 5:20 PM, Simon Horman
<simon.horman@netronome.com> wrote:
> Allow matching on options in tunnel headers.
> This makes use of existing tunnel metadata support.

Simon,

This patch is about matching on tunnel options, right? but

> Options are a bytestring of up to 256 bytes.
> Tunnel implementations may support less or more options,
> or no options at all.
>
>  # ip link add name geneve0 type geneve dstport 0 external
>  # tc qdisc add dev eth0 ingress
>  # tc qdisc del dev eth0 ingress; tc qdisc add dev eth0 ingress
>  # tc filter add dev eth0 protocol ip parent ffff: \
>      flower indev eth0 \
>         ip_proto udp \
>         action tunnel_key \
>             set src_ip 10.0.99.192 \
>             dst_ip 10.0.99.193 \
>             dst_port 4789 \
>             id 11 \
>             opts 0102800100800022 \
>     action mirred egress redirect dev geneve0

the example here is on how to use tunnel options in the tunnel set key actions..

And the other way around in the other patch... the patch is about the
tunnel key set action and the example shows how to match that in
flower... I guess you want to swap the relevant of the change log.

Anyway, is there any human readable/understandable representation of
these options? e.g what does 0102800100800022 means for geneve?

^ permalink raw reply

* [PATCH net] net: systemport: Fix 64-bit stats deadlock
From: Florian Fainelli @ 2017-09-12 20:14 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, jqiaoulk, Florian Fainelli

We can enter a deadlock situation because there is no sufficient protection
when ndo_get_stats64() runs in process context to guard against RX or TX NAPI
contexts running in softirq, this can lead to the following lockdep splat and
actual deadlock was experienced as well with an iperf session in the background
and a while loop doing ifconfig + ethtool.

[    5.780350] ================================
[    5.784679] WARNING: inconsistent lock state
[    5.789011] 4.13.0-rc7-02179-g32fae27c725d #70 Not tainted
[    5.794561] --------------------------------
[    5.798890] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
[    5.804971] swapper/0/0 [HC0[0]:SC1[1]:HE0:SE0] takes:
[    5.810175]  (&syncp->seq#2){+.?...}, at: [<c0768a28>] bcm_sysport_tx_reclaim+0x30/0x54
[    5.818327] {SOFTIRQ-ON-W} state was registered at:
[    5.823278]   bcm_sysport_get_stats64+0x17c/0x258
[    5.828053]   dev_get_stats+0x38/0xac
[    5.831776]   rtnl_fill_stats+0x30/0x118
[    5.835761]   rtnl_fill_ifinfo+0x538/0xe24
[    5.839921]   rtmsg_ifinfo_build_skb+0x6c/0xd8
[    5.844430]   rtmsg_ifinfo_event.part.5+0x14/0x44
[    5.849201]   rtmsg_ifinfo+0x20/0x28
[    5.852837]   register_netdevice+0x628/0x6b8
[    5.857171]   register_netdev+0x14/0x24
[    5.861051]   bcm_sysport_probe+0x30c/0x438
[    5.865280]   platform_drv_probe+0x50/0xb0
[    5.869418]   driver_probe_device+0x2e8/0x450
[    5.873817]   __driver_attach+0x104/0x120
[    5.877871]   bus_for_each_dev+0x7c/0xc0
[    5.881834]   bus_add_driver+0x1b0/0x270
[    5.885797]   driver_register+0x78/0xf4
[    5.889675]   do_one_initcall+0x54/0x190
[    5.893646]   kernel_init_freeable+0x144/0x1d0
[    5.898135]   kernel_init+0x8/0x110
[    5.901665]   ret_from_fork+0x14/0x2c
[    5.905363] irq event stamp: 24263
[    5.908804] hardirqs last  enabled at (24262): [<c08eecf0>] net_rx_action+0xc4/0x4e4
[    5.916624] hardirqs last disabled at (24263): [<c0a7da00>] _raw_spin_lock_irqsave+0x1c/0x98
[    5.925143] softirqs last  enabled at (24258): [<c022a7fc>] irq_enter+0x84/0x98
[    5.932524] softirqs last disabled at (24259): [<c022a918>] irq_exit+0x108/0x16c
[    5.939985]
[    5.939985] other info that might help us debug this:
[    5.946576]  Possible unsafe locking scenario:
[    5.946576]
[    5.952556]        CPU0
[    5.955031]        ----
[    5.957506]   lock(&syncp->seq#2);
[    5.960955]   <Interrupt>
[    5.963604]     lock(&syncp->seq#2);
[    5.967227]
[    5.967227]  *** DEADLOCK ***
[    5.967227]
[    5.973222] 1 lock held by swapper/0/0:
[    5.977092]  #0:  (&(&ring->lock)->rlock){..-...}, at: [<c0768a18>] bcm_sysport_tx_reclaim+0x20/0x54

So just remove the u64_stats_update_begin()/end() pair in ndo_get_stats64()
since it does not appear to be useful for anything. No inconsistency was
observed with either ifconfig or ethtool, global TX counts equal the sum of
per-queue TX counts on a 32-bit architecture.

Fixes: 10377ba7673d ("net: systemport: Support 64bit statistics")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/bcmsysport.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index a6572b51435a..c3c53f6cd9e6 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -1735,11 +1735,8 @@ static void bcm_sysport_get_stats64(struct net_device *dev,
 		stats->tx_packets += tx_packets;
 	}
 
-	/* lockless update tx_bytes and tx_packets */
-	u64_stats_update_begin(&priv->syncp);
 	stats64->tx_bytes = stats->tx_bytes;
 	stats64->tx_packets = stats->tx_packets;
-	u64_stats_update_end(&priv->syncp);
 
 	do {
 		start = u64_stats_fetch_begin_irq(&priv->syncp);
-- 
1.9.1

^ permalink raw reply related

* [PATCH] net: vrf: avoid gcc-4.6 warning
From: Arnd Bergmann @ 2017-09-12 20:10 UTC (permalink / raw)
  To: David Ahern, Shrijeet Mukherjee
  Cc: Arnd Bergmann, David S. Miller, Julian Anastasov,
	Matthias Schiffer, Wei Wang, netdev, linux-kernel

When building an allmodconfig kernel with gcc-4.6, we get a rather
odd warning:

drivers/net/vrf.c: In function ‘vrf_ip6_input_dst’:
drivers/net/vrf.c:964:3: error: initialized field with side-effects overwritten [-Werror]
drivers/net/vrf.c:964:3: error: (near initialization for ‘fl6’) [-Werror]

I have no idea what this warning is even trying to say, but it does
seem like a false positive. Reordering the initialization in to match
the structure definition gets rid of the warning, and might also avoid
whatever gcc thinks is wrong here.

Fixes: 9ff74384600a ("net: vrf: Handle ipv6 multicast and link-local addresses")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/vrf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 7e19051f3230..9b243e6f3008 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -957,12 +957,12 @@ static void vrf_ip6_input_dst(struct sk_buff *skb, struct net_device *vrf_dev,
 {
 	const struct ipv6hdr *iph = ipv6_hdr(skb);
 	struct flowi6 fl6 = {
+		.flowi6_iif     = ifindex,
+		.flowi6_mark    = skb->mark,
+		.flowi6_proto   = iph->nexthdr,
 		.daddr          = iph->daddr,
 		.saddr          = iph->saddr,
 		.flowlabel      = ip6_flowinfo(iph),
-		.flowi6_mark    = skb->mark,
-		.flowi6_proto   = iph->nexthdr,
-		.flowi6_iif     = ifindex,
 	};
 	struct net *net = dev_net(vrf_dev);
 	struct rt6_info *rt6;
-- 
2.9.0

^ permalink raw reply related

* [PATCH] ravb: document R8A77970 bindings
From: Sergei Shtylyov @ 2017-09-12 20:02 UTC (permalink / raw)
  To: Rob Herring, netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Mark Rutland, linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	Sergei Shtylyov

[-- Attachment #1: ravb-document-R8A77970-bindings.patch --]
[-- Type: text/plain, Size: 1747 bytes --]

R-Car V3M (R8A77970) SoC also has the R-Car gen3 compatible EtherAVB
device, so document  the SoC specific bindings.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>

---
The patch is against DaveM's 'net-next.git' repo but I wouldn't mind if it's
applied to 'net.git' instead. :-)

 Documentation/devicetree/bindings/net/renesas,ravb.txt |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: net-next/Documentation/devicetree/bindings/net/renesas,ravb.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/renesas,ravb.txt
+++ net-next/Documentation/devicetree/bindings/net/renesas,ravb.txt
@@ -17,6 +17,7 @@ Required properties:
 
       - "renesas,etheravb-r8a7795" for the R8A7795 SoC.
       - "renesas,etheravb-r8a7796" for the R8A7796 SoC.
+      - "renesas,etheravb-r8a77970" for the R8A77970 SoC.
       - "renesas,etheravb-rcar-gen3" as a fallback for the above
 		R-Car Gen3 devices.
 
@@ -40,7 +41,7 @@ Optional properties:
 - interrupt-parent: the phandle for the interrupt controller that services
 		    interrupts for this device.
 - interrupt-names: A list of interrupt names.
-		   For the R8A779[56] SoCs this property is mandatory;
+		   For the R-Car Gen 3 SoCs this property is mandatory;
 		   it should include one entry per channel, named "ch%u",
 		   where %u is the channel number ranging from 0 to 24.
 		   For other SoCs this property is optional; if present

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: Can libpcap filter on vlan tags when vlans are hardware-accelerated?
From: Ben Greear @ 2017-09-12 18:56 UTC (permalink / raw)
  To: netdev
In-Reply-To: <51a124d5-8a5f-35d4-5242-fd8a289f50b5@candelatech.com>

On 09/12/2017 11:54 AM, Ben Greear wrote:
> It does not appear to work on Fedora-26, and I'm curious if someone knows what needs
> doing to get this support working?
>
> Thanks,
> Ben
>


Gah, I spoke too soon.  system-test guy says it works on cmd-line, but
not when we try to make it work in another way...could be local bug,
I'll poke at this more.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* Can libpcap filter on vlan tags when vlans are hardware-accelerated?
From: Ben Greear @ 2017-09-12 18:54 UTC (permalink / raw)
  To: netdev

It does not appear to work on Fedora-26, and I'm curious if someone knows what needs
doing to get this support working?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* (unknown), 
From: pooks005 @ 2017-09-12 18:53 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: 1825633111058.doc --]
[-- Type: application/msword, Size: 43182 bytes --]

^ permalink raw reply

* Regression in throughput between kvm guests over virtual bridge
From: Matthew Rosato @ 2017-09-12 17:56 UTC (permalink / raw)
  To: netdev, jasowang; +Cc: davem, mst

We are seeing a regression for a subset of workloads across KVM guests
over a virtual bridge between host kernel 4.12 and 4.13.  Bisecting
points to c67df11f "vhost_net: try batch dequing from skb array"

In the regressed environment, we are running 4 kvm guests, 2 running as
uperf servers and 2 running as uperf clients, all on a single host.
They are connected via a virtual bridge.  The uperf client profile looks
like:

<?xml version="1.0"?>
<profile name="TCP_STREAM">
  <group nprocs="1">
    <transaction iterations="1">
      <flowop type="connect" options="remotehost=192.168.122.103
protocol=tcp"/>
    </transaction>
    <transaction duration="300">
      <flowop type="write" options="count=16 size=30000"/>
    </transaction>
    <transaction iterations="1">
      <flowop type="disconnect"/>
    </transaction>
  </group>
</profile>

So, 1 tcp streaming instance per client.  When upgrading the host kernel
from 4.12->4.13, we see about a 30% drop in throughput for this
scenario.  After the bisect, I further verified that reverting c67df11f
on 4.13 "fixes" the throughput for this scenario.

On the other hand, if we increase the load by upping the number of
streaming instances to 50 (nprocs="50") or even 10, we see instead a
~10% increase in throughput when upgrading host from 4.12->4.13.

So it may be the issue is specific to "light load" scenarios.  I would
expect some overhead for the batching, but 30% seems significant...  Any
thoughts on what might be happening here?

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox