* Re: [PATCH net] sfc: fix addr_list_lock spinlock use before init
From: Nikolay Aleksandrov @ 2014-09-16 15:20 UTC (permalink / raw)
To: Edward Cree
Cc: netdev, davem, Ben Hutchings, Shradha Shah,
Solarflare linux maintainers
In-Reply-To: <alpine.LFD.2.03.1409161540490.25290@solarflare.com>
On 16/09/14 17:05, Edward Cree wrote:
> On Mon, 15 Sep 2014, Nikolay Aleksandrov wrote:
>> When the module is initializing the ports it may call a function that
>> uses addr_list_lock before register_netdevice() has been called for that
>> port i.e. addr_list_lock is still uninitialized. The function in
>> question is efx_farch_filter_sync_rx_mode(), now it looks pointless to call
>> it before the port has been registered so alter the reconfigure_mac
>> callbacks to check if the port has been registered using the existing
>> efx_dev_registered() macro.
>
> Weak NAK as this should really be done in efx_farch_filter_sync_rx_mode()
Indeed, seems like the best place.
> rather than its callers. In fact it seems our out-of-tree driver has done
> this for a while but we forgot to upstream it, though curiously we did
> upstream the corresponding fix for EF10.
Ah yes, I saw it in the ef10 code. I should've checked with the out-of-tree
driver first, will note for future submissions :-)
> Will follow up with a patch after sanity testing it.
>
> --
> -Edward Cree
Okay, fair enough.
Thanks,
Nik
^ permalink raw reply
* [PATCH 1/2 nf-next] net: bridge: don't register netfilter call_iptables hooks by default
From: Florian Westphal @ 2014-09-16 15:47 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, Florian Westphal
Jesper reports that kernels with CONFIG_BRIDGE_NETFILTER=n show significantly
better performance vs. CONFIG_BRIDGE_NETFILTER=y, even with
bridge-nf-call-iptables=0.
This is because bridge registers some bridge netfilter hooks at
module load time, so the static key to bypass nf rule evaluation
via NF_HOOK() is false.
The hooks serve no purpose, unless iptables filtering for bridges is
desired (i.e., bridge-nf-call-*=1 and active iptables rules present).
The proper solution would be to just change the bridge-nf-call-iptables sysctl
default value to 0 and then register the hooks when user enables call-iptables
sysctl. We cannot do that though since it breaks existing setups.
The next best solution is to delay registering of the hooks until
we know that
a) call-iptables sysctl is enabled (this is the default)
AND
b) ip(6)tables rules are loaded.
This adds br_nf_check_call_iptables() helper in bridge input before
bridge 'prerouting' (sic) hooks to perform this check.
IOW, if user does not turn off call-iptables sysctl on the bridge, hook
registering is only done if NFPROTO_IPV4/IPV6 hooks are registered as
well once the first packet arrives on a bridge port.
Doing this check for every packet is still faster than registering
the hooks unconditionally. To not add overhead for setups where
the call-iptables hooks are required, a static key shortcut is provided.
As its not possible to register hooks from bh context (grabs mutex)
its scheduled via workqueue.
Note that, to not make this overly complicated, the hooks are not
unregistered again when the sysctl is disabled later on.
If user toggles call-iptables sysctl to 0 before adding any ports
to the bridge, those hooks are not registered even if iptables rules
are loaded.
Daniel reports following results for super_netperf/200/TCP_RR:
CONFIG_BRIDGE_NETFILTER=n: ~988k TPS
CONFIG_BRIDGE_NETFILTER=y: ~971k TPS
CONFIG_BRIDGE_NETFILTER=y /w patch: ~988k TPS
Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
Tested-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/bridge/br_input.c | 41 +++++++++++++++++++++++++++++
net/bridge/br_netfilter.c | 67 ++++++++++++++++++++++++++++++++++++++---------
net/bridge/br_private.h | 11 ++++++++
3 files changed, 106 insertions(+), 13 deletions(-)
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 366c436..bd770b7 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -18,6 +18,11 @@
#include <linux/netfilter_bridge.h>
#include <linux/export.h>
#include <linux/rculist.h>
+
+#ifdef CONFIG_BRIDGE_NETFILTER
+#include <linux/jump_label.h>
+#endif
+
#include "br_private.h"
/* Hook for brouter */
@@ -153,6 +158,39 @@ static int br_handle_local_finish(struct sk_buff *skb)
return 0; /* process further */
}
+#ifdef CONFIG_BRIDGE_NETFILTER
+extern struct static_key brnf_hooks_active;
+
+static inline int br_nf_check_call_iptables(const struct net_bridge *br)
+{
+ bool ip, ip6, arp;
+ int i;
+
+ if (static_key_true(&brnf_hooks_active))
+ return 0;
+
+ ip = brnf_call_iptables || br->nf_call_iptables;
+ ip6 = brnf_call_ip6tables || br->nf_call_ip6tables;
+ arp = brnf_call_arptables || br->nf_call_arptables;
+
+ for (i=0; i < NF_MAX_HOOKS; i++) {
+ if (nf_hooks_active(NFPROTO_IPV4, i) && ip)
+ break;
+ if (nf_hooks_active(NFPROTO_IPV6, i) && ip6)
+ break;
+ if (nf_hooks_active(NFPROTO_ARP, i) && arp)
+ break;
+ }
+
+ if (i < NF_MAX_HOOKS)
+ return br_netfiler_hooks_init();
+
+ return 0;
+}
+#else
+static inline int br_nf_check_call_iptables(const struct net_bridge *br){}
+#endif
+
/*
* Return NULL if skb is handled
* note: already called with rcu_read_lock
@@ -176,6 +214,9 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
p = br_port_get_rcu(skb->dev);
+ if (br_nf_check_call_iptables(p->br))
+ goto drop;
+
if (unlikely(is_link_local_ether_addr(dest))) {
u16 fwd_mask = p->br->group_fwd_mask_required;
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index a615264..c6715a2 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -31,6 +31,8 @@
#include <linux/netfilter_arp.h>
#include <linux/in_route.h>
#include <linux/inetdevice.h>
+#include <linux/workqueue.h>
+#include <linux/jump_label.h>
#include <net/ip.h>
#include <net/ipv6.h>
@@ -47,18 +49,19 @@
#define store_orig_dstaddr(skb) (skb_origaddr(skb) = ip_hdr(skb)->daddr)
#define dnat_took_place(skb) (skb_origaddr(skb) != ip_hdr(skb)->daddr)
+struct static_key brnf_hooks_active __read_mostly;
+static DEFINE_MUTEX(brnf_hook_mutex);
+
#ifdef CONFIG_SYSCTL
static struct ctl_table_header *brnf_sysctl_header;
-static int brnf_call_iptables __read_mostly = 1;
-static int brnf_call_ip6tables __read_mostly = 1;
-static int brnf_call_arptables __read_mostly = 1;
static int brnf_filter_vlan_tagged __read_mostly = 0;
static int brnf_filter_pppoe_tagged __read_mostly = 0;
static int brnf_pass_vlan_indev __read_mostly = 0;
+
+int brnf_call_iptables __read_mostly = 1;
+int brnf_call_ip6tables __read_mostly = 1;
+int brnf_call_arptables __read_mostly = 1;
#else
-#define brnf_call_iptables 1
-#define brnf_call_ip6tables 1
-#define brnf_call_arptables 1
#define brnf_filter_vlan_tagged 0
#define brnf_filter_pppoe_tagged 0
#define brnf_pass_vlan_indev 0
@@ -1059,6 +1062,45 @@ static struct ctl_table brnf_table[] = {
};
#endif
+static void __br_register_hooks_init(struct work_struct *w)
+{
+ int ret = nf_register_hooks(br_nf_ops, ARRAY_SIZE(br_nf_ops));
+ if (ret == 0)
+ static_key_slow_inc(&brnf_hooks_active);
+
+ mutex_unlock(&brnf_hook_mutex);
+ kfree(w);
+
+ WARN_ONCE(ret, "could not register bridge netfilter hook (error %d)",
+ ret);
+}
+
+int br_netfiler_hooks_init(void)
+{
+ struct work_struct *w;
+
+ /* called from bh context, cannot sleep */
+ if (!mutex_trylock(&brnf_hook_mutex))
+ return -EBUSY;
+
+ if (static_key_true(&brnf_hooks_active)) {
+ mutex_unlock(&brnf_hook_mutex);
+ return 0;
+ }
+
+ w = kzalloc(sizeof(*w), GFP_ATOMIC);
+ if (!w) {
+ mutex_unlock(&brnf_hook_mutex);
+ return -ENOMEM;
+ }
+
+ INIT_WORK(w, __br_register_hooks_init);
+ schedule_work(w);
+ /* worker will unlock mutex */
+
+ return 0;
+}
+
int __init br_netfilter_init(void)
{
int ret;
@@ -1067,17 +1109,11 @@ int __init br_netfilter_init(void)
if (ret < 0)
return ret;
- ret = nf_register_hooks(br_nf_ops, ARRAY_SIZE(br_nf_ops));
- if (ret < 0) {
- dst_entries_destroy(&fake_dst_ops);
- return ret;
- }
#ifdef CONFIG_SYSCTL
brnf_sysctl_header = register_net_sysctl(&init_net, "net/bridge", brnf_table);
if (brnf_sysctl_header == NULL) {
printk(KERN_WARNING
"br_netfilter: can't register to sysctl.\n");
- nf_unregister_hooks(br_nf_ops, ARRAY_SIZE(br_nf_ops));
dst_entries_destroy(&fake_dst_ops);
return -ENOMEM;
}
@@ -1088,7 +1124,12 @@ int __init br_netfilter_init(void)
void br_netfilter_fini(void)
{
- nf_unregister_hooks(br_nf_ops, ARRAY_SIZE(br_nf_ops));
+ /* sync with possibly running __br_register_hooks_init() */
+ mutex_lock(&brnf_hook_mutex);
+
+ if (static_key_enabled(&brnf_hooks_active))
+ nf_unregister_hooks(br_nf_ops, ARRAY_SIZE(br_nf_ops));
+ mutex_unlock(&brnf_hook_mutex);
#ifdef CONFIG_SYSCTL
unregister_net_sysctl_table(brnf_sysctl_header);
#endif
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 62a7fa2..e3b80ce 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -755,6 +755,17 @@ static inline int br_vlan_enabled(struct net_bridge *br)
int br_netfilter_init(void);
void br_netfilter_fini(void);
void br_netfilter_rtable_init(struct net_bridge *);
+int br_netfiler_hooks_init(void);
+
+#ifdef CONFIG_SYSCTL
+extern int brnf_call_iptables;
+extern int brnf_call_ip6tables;
+extern int brnf_call_arptables;
+#else
+#define brnf_call_iptables 1
+#define brnf_call_ip6tables 1
+#define brnf_call_arptables 1
+#endif
#else
#define br_netfilter_init() (0)
#define br_netfilter_fini() do { } while (0)
--
1.8.1.5
^ permalink raw reply related
* [PATCH 2/2 nf-next] net: bridge: deprecate call_iptables=1 default
From: Florian Westphal @ 2014-09-16 15:47 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, Florian Westphal
In-Reply-To: <1410882457-16641-1-git-send-email-fw@strlen.de>
add a warning and tell users to set call_iptables to 0 or 1
depending on desired behaviour before adding ports to the bridge.
Might allow changing default to 0 in the future.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/bridge/br_netfilter.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index c6715a2..0b19a81 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -58,9 +58,9 @@ static int brnf_filter_vlan_tagged __read_mostly = 0;
static int brnf_filter_pppoe_tagged __read_mostly = 0;
static int brnf_pass_vlan_indev __read_mostly = 0;
-int brnf_call_iptables __read_mostly = 1;
-int brnf_call_ip6tables __read_mostly = 1;
-int brnf_call_arptables __read_mostly = 1;
+int brnf_call_iptables __read_mostly = 2;
+int brnf_call_ip6tables __read_mostly = 2;
+int brnf_call_arptables __read_mostly = 2;
#else
#define brnf_filter_vlan_tagged 0
#define brnf_filter_pppoe_tagged 0
@@ -1065,9 +1065,16 @@ static struct ctl_table brnf_table[] = {
static void __br_register_hooks_init(struct work_struct *w)
{
int ret = nf_register_hooks(br_nf_ops, ARRAY_SIZE(br_nf_ops));
- if (ret == 0)
+ if (ret == 0) {
static_key_slow_inc(&brnf_hooks_active);
+ if ((brnf_call_iptables|brnf_call_ip6tables|brnf_call_arptables) == 2)
+ pr_info("bridge: automatic filtering via arp/ip/ip6tables "
+ "is deprecated and it will "
+ "be disabled soon. "
+ "Set bridge-nf-call-{ip,ip6,arp}tables sysctls to 1 or 0 "
+ "before adding bridge ports instead.\n");
+ }
mutex_unlock(&brnf_hook_mutex);
kfree(w);
--
1.8.1.5
^ permalink raw reply related
* Re: Qdisc: Measuring Head-of-Line blocking with netperf-wrapper
From: Tom Herbert @ 2014-09-16 15:52 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Eric Dumazet, netdev@vger.kernel.org, Stephen Hemminger,
David Miller, Hannes Frederic Sowa, Daniel Borkmann,
Florian Westphal, Toke Høiland-Jørgensen, Dave Taht
In-Reply-To: <20140916083020.4f46015c@redhat.com>
On Mon, Sep 15, 2014 at 11:30 PM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> On Mon, 15 Sep 2014 10:24:23 -0700
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> On Mon, 2014-09-15 at 10:10 -0700, Tom Herbert wrote:
>> > On Mon, Sep 15, 2014 at 9:45 AM, Jesper Dangaard Brouer
>> > <brouer@redhat.com> wrote:
>> > >
>> > > Hi Eric,
>> > >
>> > > I've constructed a "netperf-wrapper" test for measuring Head-of-Line
>> > > blocking, called "tcp_upload_prio", that I hope you will approve of?
>> > >
>> > > https://github.com/tohojo/netperf-wrapper/commit/1e6b755e8051b6
>> > >
>> > > The basic idea is to have ping packets with TOS bit 0x10, which end-up
>> > > in the high-prio band of pfifo_fast. While two TCP uploads utilize
>> > > all the bandwidth.
>> > >
>> > > These high-prio ping packet should then demonstrate the Head-of-Line
>> > > blocking occurring due to 1) packets in the HW TX ring buffer, or
>> > > 2) in the qdisc layers requeue mechanism. Disgusting these two case
>> > > might be a little difficult.
>> > >
>> > >
>> > >
>> > > Special care need to be take for using this on the default
>> > > qdisc MQ which have pfifo_fast assigned for every HW queue.
>> > >
>> > > Setup requirements:
>> > > 1. IRQ align CPUs to NIC HW queues
>> > > 2. Force netperf-wrapper subcommands to run the same CPU
>> > > E.g: taskset -c 2 ./netperf-wrapper -H IP tcp_upload_prio
>> > >
>> > > This will force all measurements to go through the same qdisc. This
>> > > is needed so the ping/latency tests measures the real property of
>> > > the qdisc and Head-of-Line blocking effect.
>> > >
>> > >
>> > > Basically the same as:
>> > > sudo taskset -c 2 ping -Q 0x10 192.168.8.2
>> > > sudo taskset -c 2 ping 192.168.8.2
>> > > sudo taskset -c 2 netperf -H 192.168.8.2 -t TCP_STREAM -l 120
>> > > sudo taskset -c 2 netperf -H 192.168.8.2 -t TCP_STREAM -l 120
>> > > --
>> > ping is a very coarse way to measure latency and in network devices it
>> > doesn't follow same path as TCP/UDP (no 4-tuple for RSS, ECMP) so it's
>> > biased and not a very realistic workload. You might want to try using
>> > netperf TCP_RR at higher priority for a fairer comparison (this is
>> > what I used to verify BQL benefits).
>
> I worry about starvation, when putting too much/heavy traffic in the
> high prio queue.
>
Each TCP_RR flow only has one packet outstanding by default.
> I've played with UDP_RR (in high prio queue) to measure the latency, it
> worked well (much less fluctuations than ping) for GSO and TSO , but
> for the none-GSO case it disturbed the two TCP uploads so much, that
> they could not utilize the link.
>
> For TCP_RR I worry what happens if a packet loss and RTO happens, but I
> guess putting this in the high prio queue should make drops (a lot)
> less likely.
>
Are you actually seeing packet loss when you run tour test? If you're
testing between two hosts in a controlled environment, you should be
able to configure test parameters for no loss.
>> > Also, you probably want to make
>> > sure to have enough antagonist flows to saturate all links when using
>> > MQ.
>
> For the none-GSO case, I guess adding more TCP uploads might help, but
> they might just get starvated. I'll give it a try.
>
>
>> Jesper, relevant netperf option is :
>>
>> -y local,remote Set the socket priority
>
> Check, netperf-wrapper already supports setting these.
>
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Sr. Network Kernel Developer at Red Hat
> Author of http://www.iptv-analyzer.org
> LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: Rusty away 18th September -- 11th October
From: Rusty Russell @ 2014-09-16 15:53 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, Linus Torvalds, LKML, virtualization
In-Reply-To: <20140914062116.GA23868@redhat.com>
"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Fri, Sep 12, 2014 at 10:58:03AM +0930, Rusty Russell wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> > On Thu, Sep 11, 2014 at 10:26:52AM +0930, Rusty Russell wrote:
>> >> Hi all,
>> >>
>> >> Probably won't read mail. Linus, I'll have pull requests early
>> >> next week; if there's anything needed I'm sure Michael Tsirkin can
>> >> handle it.
>> >
>> > Sure.
>> > Rusty, there's a small chance virtio 1.0 bits will be ready in time.
>> > I started working on them based on your virtio-pci-new-layout branch.
>> >
>> > If ready before the merge window, are you ok with me merging this
>> > support (without you having the opportunity to review first)?
>>
>> Sorry, absolutely not. I *really* want to review this in depth; if we
>> make a mistake, it's going to hurt us significantly.
>
> Right but it's only a merge window - we can fix bugs and even disable
> the new driver afterwards, can't we?
Yes, but if we get something wrong we have to live with it. We made
that mistake with virtio-pci the first time around...
It isn't enough that the code 'works', it has to match the spec. And
that requires careful thought and review.
>> And until we have the qemu bits ready, it's really hard to tell if we've
>> got this right.
>
> Sure, I meant if qemu bits are ready too, and if things work together.
> It wouldn't do to merge a driver that isn't ready.
>
>
>> So I'd rather delay and make sure we're solid.
>>
>> Thanks,
>> Rusty.
>
> Something else we can do, is refactoring that splits driver
> out to common and legacy bits, and changes APIs.
> No?
Yes, for sure. But there's no hurry to get those into the merge window.
Cheers,
Rusty.
^ permalink raw reply
* Re: Qdisc: Measuring Head-of-Line blocking with netperf-wrapper
From: Jesper Dangaard Brouer @ 2014-09-16 15:56 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev@vger.kernel.org, Stephen Hemminger, Tom Herbert,
David Miller, Hannes Frederic Sowa, Daniel Borkmann,
Florian Westphal, Toke Høiland-Jørgensen, Dave Taht,
brouer
In-Reply-To: <1410875959.7106.200.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, 16 Sep 2014 06:59:19 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> With the TCP usec rtt work I did lately, you'll get more precise results
> from a TCP_RR flow, as Tom and I explained.
Here you go, developed a new test:
http://people.netfilter.org/hawk/qdisc/experiment01/README.txt
http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol.conf
https://github.com/netoptimizer/netperf-wrapper/commit/7d0241a78e5
The test includes both a TCP_RR and UDP_RR test that derive the
latency, also kept the ping tests for comparison.
One problem: The NoneXSO test is basically invalid, because I cannot
make it exhaust the bandwidth, see "Total Upload":
http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__totals--NoneXSO_net_next.png
Looking at the ping test, there is a clear difference between priority
bands, this just shows that the priority band are working as expected
and the qdisc is backlogged.
http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__ping--GSO_net_next.png
E.g. ping test for NoneXSO show it is not backlogged, a broken test:
http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__ping--NoneXSO_net_next.png
Zooming in on the high priority band, we see how the different
high-prio band measurements are working.
Here for GSO:
http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__ping_hiprio--GSO_net_next.png
Here for TSO:
http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__ping_hiprio--TSO_net_next.png
I've created a new graph called "rr_latency" that further zooms in on
the difference between TCP_RR and UDP_RR measurements:
Here for GSO:
http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__rr_latency--GSO_net_next.png
Here for TSO:
http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__rr_latency--TSO_net_next.png
A compare graph:
http://people.netfilter.org/hawk/qdisc/experiment01/compare_TSO_vs_GSO__rr_latency.png
I found the interactions a little strange in the above graphs.
Even more strange, I started to play with the ixgbe cleanup interval,
adjusting via cmdline:
sudo ethtool -C eth4 rx-usecs 30
Then the "rr_latency" graph change, significantly lowering the latetency.
Here for GSO:
http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__rr_latency--rxusecs30_GSO_net_next.png
Here for TSO:
http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__totals--rxusecs30_TSO_net_next.png
Compare graph for GSO:
http://people.netfilter.org/hawk/qdisc/experiment01/compare_GSO_vs_GSO_with_rxusec30__rr_latency.png
Compare graph for TSO:
http://people.netfilter.org/hawk/qdisc/experiment01/compare_TSO_vs_TSO_with_rxusec30__rr_latency.png
Comparing TSO vs GSO both with rx-usecs 30, which is almost equal.
http://people.netfilter.org/hawk/qdisc/experiment01/compare_TSO_vs_GSO_both_with_rxusec30__rr_latency.png
Checking ping, still follow TCP_RR and UDP_RR, with rx-usecs 30:
http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__ping_hiprio--rxusecs30_GSO_net_next.png
http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__ping_hiprio--rxusecs30_TSO_net_next.png
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH net] sfc: fix addr_list_lock spinlock use before init
From: Edward Cree @ 2014-09-16 15:57 UTC (permalink / raw)
To: Edward Cree
Cc: Nikolay Aleksandrov, netdev, davem, Ben Hutchings, Shradha Shah,
Solarflare linux maintainers
In-Reply-To: <alpine.LFD.2.03.1409161540490.25290@solarflare.com>
Reported by Nikolay Aleksandrov. In efx_init_port() we call
efx_mac_reconfigure() to work around a Falcon/A1 limitation, and this calls
efx_{arch}_filter_sync_rx_mode(), which takes the addr_list_lock; but this
lock is uninitialised, because we haven't called register_netdevice() yet.
So, in efx_farch_filter_sync_rx_mode(), check efx_dev_registered() before
doing anything else.
The EF10 equivalent, efx_ef10_filter_sync_rx_mode(), already has the
corresponding check.
---
drivers/net/ethernet/sfc/farch.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/sfc/farch.c b/drivers/net/ethernet/sfc/farch.c
index 0537381..6859437 100644
--- a/drivers/net/ethernet/sfc/farch.c
+++ b/drivers/net/ethernet/sfc/farch.c
@@ -2933,6 +2933,9 @@ void efx_farch_filter_sync_rx_mode(struct efx_nic *efx)
u32 crc;
int bit;
+ if (!efx_dev_registered(efx))
+ return;
+
netif_addr_lock_bh(net_dev);
efx->unicast_filter = !(net_dev->flags & IFF_PROMISC);
--
1.7.11.7
^ permalink raw reply related
* Re: [patch net-next 00/13] introduce rocker switch driver with openvswitch hardware accelerated datapath
From: Jiri Pirko @ 2014-09-16 15:58 UTC (permalink / raw)
To: Thomas Graf
Cc: netdev, davem, nhorman, andy, dborkman, ogerlitz, jesse, pshelar,
azhou, ben, stephen, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
john.r.fastabend, edumazet, jhs, sfeldma, f.fainelli, roopa,
linville, dev, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a,
buytenh, aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye
In-Reply-To: <20140908135413.GA5995@casper.infradead.org>
Mon, Sep 08, 2014 at 03:54:13PM CEST, tgraf@suug.ch wrote:
>On 09/03/14 at 11:24am, Jiri Pirko wrote:
>> This patchset can be divided into 3 main sections:
>> - introduce switchdev api for implementing switch drivers
>> - add hardware acceleration bits into openvswitch datapath, This uses
>> previously mentioned switchdev api
>> - introduce rocker switch driver which implements switchdev api
>
>Jiri, Scott,
>
>Enclosed is the GOOG doc which outlines some details on my particular
>interests [0]. It includes several diagrams which might help to
>understand the overall arch. It is highly related to John's work as
>well. Please let me know if something does not align with the model
>you have in mind.
Hi Thomas.
Sorry for late answer, I returned from vacation yesterday.
I went over your document, I did not find anything which would not align
with our approach. Looks good to me.
>
>Summary:
>The full virtual tunnel endpoint flow offload attempts to offload full
>flows to the hardware and utilize the embedded switch on the host NIC
>to empower the eSwitch with the required flexibility of the software
>driven network. In this model, the guest (VM or LXC) attaches through a
>SR-IOV VF which serves as the primary path. A slow path / software path
>is provided via the CPU which can route packets back into the VF by
>tagging packets with forwarding metadata and sending the frame back to
>the NIC.
>
>[0] https://docs.google.com/document/d/195waUliu7G5YYVuXHmLmHgJ38DFSte321WPq0oaFhyU/edit?usp=sharing
>(Publicly accessible and open for comments)
^ permalink raw reply
* Re: [PATCH net] sfc: fix addr_list_lock spinlock use before init
From: Nikolay Aleksandrov @ 2014-09-16 16:02 UTC (permalink / raw)
To: Edward Cree
Cc: netdev, davem, Ben Hutchings, Shradha Shah,
Solarflare linux maintainers
In-Reply-To: <alpine.LFD.2.03.1409161654370.25290@solarflare.com>
On 16/09/14 17:57, Edward Cree wrote:
> Reported by Nikolay Aleksandrov. In efx_init_port() we call
> efx_mac_reconfigure() to work around a Falcon/A1 limitation, and this calls
> efx_{arch}_filter_sync_rx_mode(), which takes the addr_list_lock; but this
> lock is uninitialised, because we haven't called register_netdevice() yet.
> So, in efx_farch_filter_sync_rx_mode(), check efx_dev_registered() before
> doing anything else.
> The EF10 equivalent, efx_ef10_filter_sync_rx_mode(), already has the
> corresponding check.
> ---
> drivers/net/ethernet/sfc/farch.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/sfc/farch.c b/drivers/net/ethernet/sfc/farch.c
> index 0537381..6859437 100644
> --- a/drivers/net/ethernet/sfc/farch.c
> +++ b/drivers/net/ethernet/sfc/farch.c
> @@ -2933,6 +2933,9 @@ void efx_farch_filter_sync_rx_mode(struct efx_nic *efx)
> u32 crc;
> int bit;
>
> + if (!efx_dev_registered(efx))
> + return;
> +
> netif_addr_lock_bh(net_dev);
>
> efx->unicast_filter = !(net_dev->flags & IFF_PROMISC);
>
You should add a Signed-off-by, otherwise looks good to me.
FWIW, you can add my Tested-by as well.
Tested-by: Nikolay Aleksandrov <nikolay@redhat.com>
^ permalink raw reply
* Re: [PATCH] bridge: Fix br_should_learn to check vlan_enabled
From: David Miller @ 2014-09-16 16:05 UTC (permalink / raw)
To: makita.toshiaki; +Cc: vyasevich, netdev, toshiaki.makita1, vyasevic
In-Reply-To: <541813AB.2060301@lab.ntt.co.jp>
From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Date: Tue, 16 Sep 2014 19:40:43 +0900
> On 2014/09/16 6:38, David Miller wrote:
>> From: Vladislav Yasevich <vyasevich@gmail.com>
>> Date: Mon, 15 Sep 2014 15:24:26 -0400
>>
>>> As Toshiaki Makita pointed out, the BRIDGE_INPUT_SKB_CB will
>>> not be initialized in br_should_learn() as that function
>>> is called only from br_handle_local_finish(). That is
>>> an input handler for link-local ethernet traffic so it perfectly
>>> correct to check br->vlan_enabled here.
>>>
>>> Reported-by: Toshiaki Makita<toshiaki.makita1@gmail.com>
>>> Fixes: 20adfa1 bridge: Check if vlan filtering is enabled only once.
>>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>>
>> Applied, thanks Vlad.
>
> Hi David,
>
> Could you queue this for -stable as well?
Done.
^ permalink raw reply
* Re: Qdisc: Measuring Head-of-Line blocking with netperf-wrapper
From: Dave Taht @ 2014-09-16 16:08 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Eric Dumazet, netdev@vger.kernel.org, Stephen Hemminger,
Tom Herbert, David Miller, Hannes Frederic Sowa, Daniel Borkmann,
Florian Westphal, Toke Høiland-Jørgensen
In-Reply-To: <20140916175600.018350fe@redhat.com>
On Tue, Sep 16, 2014 at 6:56 PM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> On Tue, 16 Sep 2014 06:59:19 -0700
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> With the TCP usec rtt work I did lately, you'll get more precise results
>> from a TCP_RR flow, as Tom and I explained.
>
> Here you go, developed a new test:
> http://people.netfilter.org/hawk/qdisc/experiment01/README.txt
> http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol.conf
> https://github.com/netoptimizer/netperf-wrapper/commit/7d0241a78e5
>
> The test includes both a TCP_RR and UDP_RR test that derive the
> latency, also kept the ping tests for comparison.
You have incidentally overwhelmed *me* with data. Thank you very much
for including the *.json* files in your experiments, I'll be able to parse
and compare them later with netperf-wrapper when I get more time.
> One problem: The NoneXSO test is basically invalid, because I cannot
> make it exhaust the bandwidth, see "Total Upload":
> http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__totals--NoneXSO_net_next.png
Well, i've long conceded that TSO and GSO offloads were needed at 10GigE speeds.
I'd love to get a grip on how bursty they are since some moderation
fixes landed a few
versions back.
>
> Looking at the ping test, there is a clear difference between priority
> bands, this just shows that the priority band are working as expected
> and the qdisc is backlogged.
> http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__ping--GSO_net_next.png
> E.g. ping test for NoneXSO show it is not backlogged, a broken test:
> http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__ping--NoneXSO_net_next.png
>
I look forward to seeing sch_fq and fq_codel data, for comparison.
>
> Zooming in on the high priority band, we see how the different
> high-prio band measurements are working.
> Here for GSO:
> http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__ping_hiprio--GSO_net_next.png
> Here for TSO:
> http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__ping_hiprio--TSO_net_next.png
>
> I've created a new graph called "rr_latency" that further zooms in on
> the difference between TCP_RR and UDP_RR measurements:
> Here for GSO:
> http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__rr_latency--GSO_net_next.png
> Here for TSO:
> http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__rr_latency--TSO_net_next.png
> A compare graph:
> http://people.netfilter.org/hawk/qdisc/experiment01/compare_TSO_vs_GSO__rr_latency.png
>
> I found the interactions a little strange in the above graphs.
I note that toke had started coding netperf-wrapper back in the day
when 10mbits and RTTs measured in seconds were the norm. I am
DELIGHTED to see it works at all at 10GigE. Other network measurement
tools, like netalyzr, peak out at 20mbits....
You can capture more detail about the tc setup, in particular, if you
invoke it with the -x option.
You might get more detail on your plots if you run as root with
--step-size .01 for a 10ms sampling interval
rather than a 200ms one. This doesn't quite work on a few older tests,
notably rrul.
> Even more strange, I started to play with the ixgbe cleanup interval,
> adjusting via cmdline:
> sudo ethtool -C eth4 rx-usecs 30
>
> Then the "rr_latency" graph change, significantly lowering the latetency.
> Here for GSO:
> http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__rr_latency--rxusecs30_GSO_net_next.png
> Here for TSO:
> http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__totals--rxusecs30_TSO_net_next.png
There is always a tradeoff between better batching and better latency.
I've kind of hoped that with the new post-ivy-bridge architectures,
that the weight, even napi weight, was shifting towards dealing with
low latency packets already in cache was more efficient than batching
up processing. The numbers the DPDK folk were getting were astounding.
(but still can't make heads or tails of where you are going with all this)
> Compare graph for GSO:
> http://people.netfilter.org/hawk/qdisc/experiment01/compare_GSO_vs_GSO_with_rxusec30__rr_latency.png
> Compare graph for TSO:
> http://people.netfilter.org/hawk/qdisc/experiment01/compare_TSO_vs_TSO_with_rxusec30__rr_latency.png
> Comparing TSO vs GSO both with rx-usecs 30, which is almost equal.
> http://people.netfilter.org/hawk/qdisc/experiment01/compare_TSO_vs_GSO_both_with_rxusec30__rr_latency.png
>
>
> Checking ping, still follow TCP_RR and UDP_RR, with rx-usecs 30:
> http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__ping_hiprio--rxusecs30_GSO_net_next.png
> http://people.netfilter.org/hawk/qdisc/experiment01/qdisc_prio_hol__ping_hiprio--rxusecs30_TSO_net_next.png
I have generally found that it is easier to present all this data on
graphs or combined graph, on a web page, rather than in email.
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Sr. Network Kernel Developer at Red Hat
> Author of http://www.iptv-analyzer.org
> LinkedIn: http://www.linkedin.com/in/brouer
--
Dave Täht
https://www.bufferbloat.net/projects/make-wifi-fast
^ permalink raw reply
* [PATCH net] sfc: fix addr_list_lock spinlock use before init
From: Edward Cree @ 2014-09-16 16:05 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Edward Cree, netdev, davem, Ben Hutchings, Shradha Shah,
Solarflare linux maintainers
In-Reply-To: <54185F18.4020003@redhat.com>
Reported by Nikolay Aleksandrov. In efx_init_port() we call
efx_mac_reconfigure() to work around a Falcon/A1 limitation, and this calls
efx_{arch}_filter_sync_rx_mode(), which takes the addr_list_lock; but this
lock is uninitialised, because we haven't called register_netdevice() yet.
So, in efx_farch_filter_sync_rx_mode(), check efx_dev_registered() before
doing anything else.
The EF10 equivalent, efx_ef10_filter_sync_rx_mode(), already has the
corresponding check.
Signed-off-by: Edward Cree <ecree@solarflare.com>
Tested-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
drivers/net/ethernet/sfc/farch.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/sfc/farch.c b/drivers/net/ethernet/sfc/farch.c
index 0537381..6859437 100644
--- a/drivers/net/ethernet/sfc/farch.c
+++ b/drivers/net/ethernet/sfc/farch.c
@@ -2933,6 +2933,9 @@ void efx_farch_filter_sync_rx_mode(struct efx_nic *efx)
u32 crc;
int bit;
+ if (!efx_dev_registered(efx))
+ return;
+
netif_addr_lock_bh(net_dev);
efx->unicast_filter = !(net_dev->flags & IFF_PROMISC);
--
1.7.11.7
^ permalink raw reply related
* Issue with ping source address display
From: Daniele Orlandi @ 2014-09-16 16:01 UTC (permalink / raw)
To: netdev
Hello,
I noticed that when ping receives ICMP messages from different sources
the first IP address is always used and displayed:
vihai@seviolab:~$ ping -V
ping utility, iputils-s20121221
This is a (simulated) flapping route:
vihai@seviolab:~$ ping 10.254.10.140
PING 10.254.10.140 (10.254.10.140) 56(84) bytes of data.
From 192.168.1.1 icmp_seq=1 Destination Host Unreachable
From 192.168.1.1 icmp_seq=2 Destination Host Unreachable
From 192.168.1.1 icmp_seq=3 Destination Host Unreachable
64 bytes from 192.168.1.1: icmp_seq=4 ttl=61 time=24.7 ms
64 bytes from 192.168.1.1: icmp_seq=5 ttl=61 time=25.6 ms
64 bytes from 192.168.1.1: icmp_seq=6 ttl=61 time=69.6 ms
From 192.168.1.1 icmp_seq=7 Destination Host Unreachable
From 192.168.1.1 icmp_seq=8 Destination Host Unreachable
From 192.168.1.1 icmp_seq=9 Destination Host Unreachable
^C
--- 10.254.10.140 ping statistics ---
9 packets transmitted, 3 received, +6 errors, 66% packet loss, time 8001ms
rtt min/avg/max/mdev = 24.797/40.061/69.692/20.955 ms
The sources, however are different:
vihai@seviolab:~$ sudo tcpdump -n icmp
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on eth0, link-type EN10MB (Ethernet), capture size 65535 bytes
^[OA^[OA^[OA17:09:55.932981 IP 192.168.1.21 > 10.254.10.140: ICMP echo
request, id 9278, seq 1, length 64
17:09:55.933234 IP 192.168.1.1 > 192.168.1.21: ICMP host 10.254.10.140
unreachable, length 92
17:09:56.933169 IP 192.168.1.21 > 10.254.10.140: ICMP echo request, id
9278, seq 2, length 64
17:09:56.933416 IP 192.168.1.1 > 192.168.1.21: ICMP host 10.254.10.140
unreachable, length 92
17:09:57.933160 IP 192.168.1.21 > 10.254.10.140: ICMP echo request, id
9278, seq 3, length 64
17:09:57.933404 IP 192.168.1.1 > 192.168.1.21: ICMP host 10.254.10.140
unreachable, length 92
17:09:58.933163 IP 192.168.1.21 > 10.254.10.140: ICMP echo request, id
9278, seq 4, length 64
17:09:58.957939 IP 10.254.10.140 > 192.168.1.21: ICMP echo reply, id
9278, seq 4, length 64
17:09:59.935050 IP 192.168.1.21 > 10.254.10.140: ICMP echo request, id
9278, seq 5, length 64
17:09:59.960724 IP 10.254.10.140 > 192.168.1.21: ICMP echo reply, id
9278, seq 5, length 64
17:10:00.936177 IP 192.168.1.21 > 10.254.10.140: ICMP echo request, id
9278, seq 6, length 64
17:10:01.005849 IP 10.254.10.140 > 192.168.1.21: ICMP echo reply, id
9278, seq 6, length 64
17:10:01.936313 IP 192.168.1.21 > 10.254.10.140: ICMP echo request, id
9278, seq 7, length 64
17:10:01.936626 IP 192.168.1.1 > 192.168.1.21: ICMP host 10.254.10.140
unreachable, length 92
17:10:02.935321 IP 192.168.1.21 > 10.254.10.140: ICMP echo request, id
9278, seq 8, length 64
17:10:02.935591 IP 192.168.1.1 > 192.168.1.21: ICMP host 10.254.10.140
unreachable, length 92
17:10:03.934322 IP 192.168.1.21 > 10.254.10.140: ICMP echo request, id
9278, seq 9, length 64
17:10:03.934613 IP 192.168.1.1 > 192.168.1.21: ICMP host 10.254.10.140
unreachable, length 92
Tried with a different ping implementation (RouterOS) and the behaviour
seems correct:
[vihai@SevioLab SW1] > ping 10.254.10.140
HOST SIZE TTL TIME STATUS
192.168.1.1 84 64 0ms host unreachable
192.168.1.1 84 64 0ms host unreachable
192.168.1.1 84 64 0ms host unreachable
10.254.10.140 56 61 20ms
10.254.10.140 56 61 46ms
10.254.10.140 56 61 37ms
192.168.1.1 84 64 0ms host unreachable
192.168.1.1 84 64 0ms host unreachable
192.168.1.1 84 64 0ms host unreachable
sent=9 received=3 packet-loss=66% min-rtt=20ms avg-rtt=34ms
max-rtt=46ms
I think you may be interested as this issue caught me in a
troubleshooting session introducing strangeness and might do the same to
you too :)
Bye,
^ permalink raw reply
* Re: [PATCH] bridge: Fix br_should_learn to check vlan_enabled
From: David Miller @ 2014-09-16 16:16 UTC (permalink / raw)
To: toshiaki.makita1; +Cc: makita.toshiaki, vyasevich, netdev, vyasevic
In-Reply-To: <54184303.9070503@gmail.com>
From: Toshiaki Makita <toshiaki.makita1@gmail.com>
Date: Tue, 16 Sep 2014 23:02:43 +0900
> (14/09/16 (火) 19:40), Toshiaki Makita wrote:
>> On 2014/09/16 6:38, David Miller wrote:
>>> From: Vladislav Yasevich <vyasevich@gmail.com>
>>> Date: Mon, 15 Sep 2014 15:24:26 -0400
>>>
>>>> As Toshiaki Makita pointed out, the BRIDGE_INPUT_SKB_CB will
>>>> not be initialized in br_should_learn() as that function
>>>> is called only from br_handle_local_finish(). That is
>>>> an input handler for link-local ethernet traffic so it perfectly
>>>> correct to check br->vlan_enabled here.
>>>>
>>>> Reported-by: Toshiaki Makita<toshiaki.makita1@gmail.com>
>>>> Fixes: 20adfa1 bridge: Check if vlan filtering is enabled only once.
>>>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>>>
>>> Applied, thanks Vlad.
>>
>> Hi David,
>>
>> Could you queue this for -stable as well?
>> Without this, FDB can be poisoned by disallowed ports.
>> (the same problem as stated in e0d7968ab6c8 "bridge: Prevent insertion
>> of FDB entry with disallowed vlan")
>
> I'm sorry, I was confusued.
> This doesn't cause that problem, because if vlan_filtered is 0, fdb is
> always updated with vid 0. Such an entry is never used as long as
> vlan_filtering is enabled.
> Please ignore my previous mail.
Ok.
^ permalink raw reply
* Re: [net-next PATCH v2 1/4] net: sched: fix unsued cpu variable
From: Cong Wang @ 2014-09-16 16:17 UTC (permalink / raw)
To: John Fastabend
Cc: Cong Wang, David Miller, Eric Dumazet, netdev, Jamal Hadi Salim
In-Reply-To: <20140916063024.2905.55403.stgit@nitbit.x32>
On Mon, Sep 15, 2014 at 11:30 PM, John Fastabend
<john.fastabend@gmail.com> wrote:
> kbuild test robot reported an unused variable cpu in cls_u32.c
> after the patch below. This happens when PERF and MARK config
> variables are disabled
>
> Fix this is to use separate variables for perf and mark
> and define the cpu variable inside the ifdef logic.
>
> 'commit 459d5f626da7 ("net: sched: make cls_u32 per cpu")'
Please use the Fixes: tag.
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Other than that,
Acked-by: Cong Wang <cwang@twopensource.com>
Thanks!
^ permalink raw reply
* Re: [PATCH RFC 1/6] ipv6: also increase fib6_node sernum on deletion events
From: Nicolas Dichtel @ 2014-09-16 16:18 UTC (permalink / raw)
To: Hannes Frederic Sowa, netdev; +Cc: Eric Dumazet, Vlad Yasevich
In-Reply-To: <4f55afa99da1dab957b2dc5e7b313a1b70f864f3.1410477596.git.hannes@stressinduktion.org>
Le 12/09/2014 01:21, Hannes Frederic Sowa a écrit :
> fib6_add increases the fn_sernum of fib6_nodes while it traverses the
> tree. This serial number is used by ip6_dst_check to judge whether a
> relookup for the socket cache should be done (e.g. a better route is
> available).
>
> We didn't do so for fib6_del, so we missed relookups on ipv6 address
> deletion events. Because this caused trouble in the SCTP stack, instead
> the genid for ipv6 was bumped. Also TCP connections used old source
> addresses, which were not available anymore.
>
> Because we have static rt6_nodes in the tree (no RTF_GATEWAY,
> RTF_NONEXTHOP nor RTF_CACHE nodes but still DST_HOST) flag, we ended up
> in a situation where the genid of the routing node was always smaller
> than the published genid in the namespace. That caused ip6_dst_check to
> always discard the current dst_entry and a relookup happend.
>
> This patch prepares for the removal of the ipv6 genid by also modifying
> the fn_sernum on route deletion.
>
> Thanks to Eric Dumazet who noticed this problem!
>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Vlad Yasevich <vyasevich@gmail.com>
> Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
This serie looks good to me. Thank you for working on this topic!
Regards,
Nicolas
^ permalink raw reply
* Re: [net-next PATCH v2 3/4] net: sched: cls_cgroup fix possible memory leak of 'new'
From: Cong Wang @ 2014-09-16 16:19 UTC (permalink / raw)
To: John Fastabend
Cc: Cong Wang, David Miller, Eric Dumazet, netdev, Jamal Hadi Salim
In-Reply-To: <20140916063115.2905.53827.stgit@nitbit.x32>
On Mon, Sep 15, 2014 at 11:31 PM, John Fastabend
<john.fastabend@gmail.com> wrote:
> tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
> head: 54996b529ab70ca1d6f40677cd2698c4f7127e87
> commit: c7953ef23042b7c4fc2be5ecdd216aacff6df5eb [625/646] net: sched: cls_cgroup use RCU
>
> net/sched/cls_cgroup.c:130 cls_cgroup_change() warn: possible memory leak of 'new'
> net/sched/cls_cgroup.c:135 cls_cgroup_change() warn: possible memory leak of 'new'
> net/sched/cls_cgroup.c:139 cls_cgroup_change() warn: possible memory leak of 'new'
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Fixes: commit: c7953ef23042b7c4fc2be5ecdd216aac ("net: sched:
cls_cgroup use RCU")
Acked-by: Cong Wang <cwang@twopensource.com>
^ permalink raw reply
* Re: Rusty away 18th September -- 11th October
From: Michael S. Tsirkin @ 2014-09-16 16:20 UTC (permalink / raw)
To: Rusty Russell; +Cc: netdev, Linus Torvalds, LKML, virtualization
In-Reply-To: <87a95zfto5.fsf@rustcorp.com.au>
On Wed, Sep 17, 2014 at 01:23:46AM +0930, Rusty Russell wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > On Fri, Sep 12, 2014 at 10:58:03AM +0930, Rusty Russell wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> > On Thu, Sep 11, 2014 at 10:26:52AM +0930, Rusty Russell wrote:
> >> >> Hi all,
> >> >>
> >> >> Probably won't read mail. Linus, I'll have pull requests early
> >> >> next week; if there's anything needed I'm sure Michael Tsirkin can
> >> >> handle it.
> >> >
> >> > Sure.
> >> > Rusty, there's a small chance virtio 1.0 bits will be ready in time.
> >> > I started working on them based on your virtio-pci-new-layout branch.
> >> >
> >> > If ready before the merge window, are you ok with me merging this
> >> > support (without you having the opportunity to review first)?
> >>
> >> Sorry, absolutely not. I *really* want to review this in depth; if we
> >> make a mistake, it's going to hurt us significantly.
> >
> > Right but it's only a merge window - we can fix bugs and even disable
> > the new driver afterwards, can't we?
>
> Yes, but if we get something wrong we have to live with it. We made
> that mistake with virtio-pci the first time around...
>
> It isn't enough that the code 'works', it has to match the spec. And
> that requires careful thought and review.
>
> >> And until we have the qemu bits ready, it's really hard to tell if we've
> >> got this right.
> >
> > Sure, I meant if qemu bits are ready too, and if things work together.
> > It wouldn't do to merge a driver that isn't ready.
> >
> >
> >> So I'd rather delay and make sure we're solid.
> >>
> >> Thanks,
> >> Rusty.
> >
> > Something else we can do, is refactoring that splits driver
> > out to common and legacy bits, and changes APIs.
> > No?
>
> Yes, for sure. But there's no hurry to get those into the merge window.
>
> Cheers,
> Rusty.
Well my point was if we do put them into the merge window,
just adding a new driver will be easy and is often allowed
pretty late in the cycle.
--
MST
^ permalink raw reply
* Re: [net-next PATCH v2 4/4] net: sched: cls_fw: add missing tcf_exts_init call in fw_change()
From: Cong Wang @ 2014-09-16 16:23 UTC (permalink / raw)
To: John Fastabend
Cc: Cong Wang, David Miller, Eric Dumazet, netdev, Jamal Hadi Salim
In-Reply-To: <20140916063141.2905.59421.stgit@nitbit.x32>
On Mon, Sep 15, 2014 at 11:31 PM, John Fastabend
<john.fastabend@gmail.com> wrote:
> When allocating a new structure we also need to call tcf_exts_init
> to initialize exts.
>
> A follow up patch might be in order to remove some of this code
> and do tcf_exts_assign(). With this we could remove the
> tcf_exts_init/tcf_exts_change pattern for some of the classifiers.
> As part of the future tcf_actions RCU series this will need to be
> done. For now fix the call here.
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Fixes: commit e35a8ee5993ba81fd6c0 ("net: sched: fw use RCU")
Acked-by: Cong Wang <cwang@twopensource.com>
^ permalink raw reply
* Re: [net-next PATCH] net: sched: cls_cgroup need tcf_exts_init in all cases
From: Cong Wang @ 2014-09-16 16:26 UTC (permalink / raw)
To: John Fastabend
Cc: Cong Wang, David Miller, Eric Dumazet, netdev, Jamal Hadi Salim
In-Reply-To: <20140916073341.2597.36240.stgit@nitbit.x32>
On Tue, Sep 16, 2014 at 12:33 AM, John Fastabend
<john.fastabend@gmail.com> wrote:
> This ensures the tcf_exts_init() is called for all cases.
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Fixes: commit 952313bd62589cae216a57 ("net: sched: cls_cgroup use RCU")
Acked-by: Cong Wang <cwang@twopensource.com>
^ permalink raw reply
* phy link status not stable
From: Mugunthan V N @ 2014-09-16 16:26 UTC (permalink / raw)
To: netdev; +Cc: David Miller
Hi
While doing ifdown/ifup test, phy link up status is very much delayed
and also link up/down comes alternatively without physical link changes
which leads to ifup failure. in v3.12 internal tree there is no failure.
Pasting the logs below.
root@am437x-evm:~# ifup eth0
[ 652.633204] net eth0: initializing cpsw version 1.15 (0)
[ 652.717428] net eth0: phy found : id is : 0x221622
udhcpc (v1.20.2) started
Sending discover...
Sending discover...
[ 658.717859] cpsw 4a100000.ethernet eth0: Link is Up - 1Gbps/Full -
flow control rx/tx
Sending discover...
[ 660.717894] cpsw 4a100000.ethernet eth0: Link is Down
[ 661.717937] cpsw 4a100000.ethernet eth0: Link is Up - 1Gbps/Full -
flow control rx/tx
No lease, failing
root@am437x-evm:~# udhcpc
udhcpc (v1.20.2) started
Sending discover...
Sending select for 172.24.190.6...
Lease of 172.24.190.6 obtained, lease time 3600
/etc/udhcpc.d/50default: Adding DNS 192.0.2.2
/etc/udhcpc.d/50default: Adding DNS 192.0.2.3
root@am437x-evm:~#
Is this a known issue or any pointers where to look for clues?
Setup: TI-AM437x EVM
Phy: Micrel KSZ9031 Gigabit PHY
Thanks,
Mugunthan V N
^ permalink raw reply
* Re: Qdisc: Measuring Head-of-Line blocking with netperf-wrapper
From: Eric Dumazet @ 2014-09-16 16:30 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev@vger.kernel.org, Stephen Hemminger, Tom Herbert,
David Miller, Hannes Frederic Sowa, Daniel Borkmann,
Florian Westphal, Toke Høiland-Jørgensen, Dave Taht
In-Reply-To: <20140916175600.018350fe@redhat.com>
On Tue, 2014-09-16 at 17:56 +0200, Jesper Dangaard Brouer wrote:
> On Tue, 16 Sep 2014 06:59:19 -0700
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> > With the TCP usec rtt work I did lately, you'll get more precise results
> > from a TCP_RR flow, as Tom and I explained.
>
> Here you go, developed a new test:
Just to make sure I understand (sorry I dont have time going all your
graphs right now)
The target of your high prio flow is different from target of the
antagonist flows ?
Otherwise, you are not only measuring head of line blocking of your
host, but the whole chain, including scheduling latencies of the
(shared) target.
^ permalink raw reply
* Re: TCP connection will hang in FIN_WAIT1 after closing if zero window is advertised
From: Neal Cardwell @ 2014-09-16 16:31 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Eric Dumazet, Andrey Dmitrov, Hannes Frederic Sowa, netdev,
Alexandra N. Kossovsky, Konstantin Ushakov
In-Reply-To: <CAK6E8=eNtNwREF8J+eF1LWhmg-aZmkYycDkXEAKn7Xb0_xv2rA@mail.gmail.com>
On Tue, Sep 16, 2014 at 11:11 AM, Yuchung Cheng <ycheng@google.com> wrote:
> On Tue, Sep 16, 2014 at 6:09 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> Normally SO_LINGER could be used, or TCP_USER_TIMEOUT. This requires a
>> system call before doing the close().
>>
>> 1) TCP_USER_TIMEOUT would be the fit for this, but its current
>> implementation do not take care of the probes sent, even in FIN_WAIT
>> state when in this zero window mode. A patch would be needed.
> Yes that's what I meant. I am proposing we should patch
> TCP_USER_TIMEOUT to do this.
We should probably be careful here. It would be a non-trivial change
in semantics to have TCP_USER_TIMEOUT solve this issue.
TCP_USER_TIMEOUT, in both the man page, the RFC, and the existing
code, is about a user-specific limit on the maximum amount of time the
TCP stack will attempt to transmit a single packet. (For example, the
man page: "specifies the maximum amount of time in milliseconds that
transmitted data may remain unacknowledged before TCP will forcibly
close the corresponding connection"). Any existing apps that are
setting TCP_USER_TIMEOUT are probably setting it to something in the
range of seconds to a few minutes, and may reasonably expect their
orphaned connections to last minutes to hours, as long as they are
making progress (each of the packets is ACKed in the
seconds-to-minutes range).
By contrast, AFAICT what we are talking about here for these
ZWP-forever/tarpit scenarios is to be able to cap the maximum overall
lifetime of an orphan connection. That's a different parameter, and
folks might want to set it in the minutes-to-hours range.
neal
^ permalink raw reply
* Re: [PATCH -net] scsi: fix users of SCSI_FC_ATTRS to depend on NET
From: David Miller @ 2014-09-16 16:41 UTC (permalink / raw)
To: maier; +Cc: rdunlap, netdev, linux-scsi, jbottomley, fengguang.wu
In-Reply-To: <54184FB7.7020201@linux.vnet.ibm.com>
From: Steffen Maier <maier@linux.vnet.ibm.com>
Date: Tue, 16 Sep 2014 16:56:55 +0200
> I think zfcp does not have any (direct) dependency on NET.
>
> It looks like SCSI_FC_ATTRS selects SCSI_NETLINK (declaring
> scsi_nl_sock) and only depends on SCSI but not on NET.
> SCSI_NETLINK itself only selects NET but does not model its direct
> depencency on NET?
SCSI_NETLINK now has:
depends on NET
^ permalink raw reply
* tg3 issue with tcp checksums and vlan packets
From: Vlad Yasevich @ 2014-09-16 16:43 UTC (permalink / raw)
To: Prashant Sreedharan, Michael Chan; +Cc: netdev@vger.kernel.org
Prashant and Michael
I am seeing a strange issue with tg3 driver when I try to pass to it tcp packets
that have partial checksums and inline (non-accelerated) vlan header.
Looking at the packet at the receiver, it appears as if the tcp checksum is never
updated. If I strip the vlan header and set vlan_tci, then everything works ok.
You can easily reproduce this by configuring 802.1ad vlans on top of tg3 device.
This will force software tagging, tg3 will not fix checksums and tcp connections
will not be established.
I've looked at tg3 driver and it doesn't look like it tries to pass any checksum
offsets to the nic/firmware. So it looks like a possible firmware issue.
Can you please take a look.
Thanks
-vlad
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox