* Re: [PATCH v2] net: bluetooth: hci_sock: Use 'const void *' instead of 'void *' for 2nd parameter of hci_test_bit()
From: Chen Gang S @ 2015-02-03 2:59 UTC (permalink / raw)
To: Joe Perches
Cc: marcel, gustavo, johan.hedberg, David S. Miller, linux-bluetooth,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <54D0332B.6050701@sunrus.com.cn>
On 2/3/15 10:32, Chen Gang S wrote:
> On 2/3/15 05:20, Joe Perches wrote:
>> On Tue, 2015-02-03 at 05:14 +0800, Chen Gang S wrote:
>>> hci_test_bit() does not modify 2nd parameter, so it is better to let it
>>> be constant, or may cause build warning. The related warning (with
>>> allmodconfig under xtensa):
>> []
>>> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
>> []
>>> @@ -46,7 +46,7 @@ struct hci_pinfo {
>>> unsigned short channel;
>>> };
>>>
>>> -static inline int hci_test_bit(int nr, void *addr)
>>> +static inline int hci_test_bit(int nr, const void *addr)
>>> {
>>> return *((__u32 *) addr + (nr >> 5)) & ((__u32) 1 << (nr & 31));
>>> }
>>
>> It's probably better to use const __u32 * here too, but the
>> real thing I wonder is whether or not there's an issue with
>> one of the 2 uses of this function.
>>
>> One of them passes a unsigned long *, the other a u32 *.
>>
>> $ git grep -w hci_test_bit
>> net/bluetooth/hci_sock.c:static inline int hci_test_bit(int nr, void *addr)
>> net/bluetooth/hci_sock.c: if (!hci_test_bit(flt_event, &flt->event_mask))
>> net/bluetooth/hci_sock.c: !hci_test_bit(ocf & HCI_FLT_OCF_BITS,
>> net/bluetooth/hci_sock.c- &hci_sec_filter.ocf_mask[ogf])) &&
>>
>> hci_sec_filter.ocf_mask is __u32
>> but flt->event_mask is unsigned long.
>>
>> Any possible issue here on 64-bit systems?
>>
>
> For me, it can not cause issue on 64-bit systems. hci_test_bit() treats
> 'addr' as "__u32 *", and has to use the pointer to do something.
>
'event_mask' is intended to type cast to "__u32 *" within 'hci_sock.c'.
So for me, "const __u32 *" is better than "const void *" for 2nd
parameter of hci_test_bit().
If what I said above is correct, and also if necessary, I shall patch v3
for it.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply
* Re: [PATCH net-next] cxgb4 : Improve IEEE DCBx support, other minor open-lldp fixes
From: David Miller @ 2015-02-03 2:55 UTC (permalink / raw)
To: anish; +Cc: netdev, hariprasad
In-Reply-To: <1422667217-2610-1-git-send-email-anish@chelsio.com>
From: Anish Bhatt <anish@chelsio.com>
Date: Fri, 30 Jan 2015 17:20:17 -0800
> * Add support for IEEE ets & pfc api.
> * Fix bug that resulted in incorrect bandwidth percentage being returned for
> CEE peers
> * Convert pfc enabled info from firmware format to what dcbnl expects before
> returning
>
> Signed-off-by: Anish Bhatt <anish@chelsio.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net/tulip: don't warn about unknown ARM architecture
From: David Miller @ 2015-02-03 2:54 UTC (permalink / raw)
To: arnd; +Cc: netdev, linux-arm-kernel, linux-kernel, grundler
In-Reply-To: <1780413.LLpv9s01CB@wuerfel>
From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 30 Jan 2015 22:58:19 +0100
> ARM has 32-byte cache lines, which according to the comment in
> the init registers function seems to work best with the default
> value of 0x4800 that is also used on sparc and parisc.
>
> This adds ARM to the same list, to use that default but no
> longer warn about it.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied, thanks a lot.
^ permalink raw reply
* Re: [PATCH] net: hip04: add missing MODULE_LICENSE
From: David Miller @ 2015-02-03 2:51 UTC (permalink / raw)
To: arnd; +Cc: netdev, zhangfei.gao, dingtianhong
In-Reply-To: <4018099.9I7f6kd5jC@wuerfel>
From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 30 Jan 2015 22:57:01 +0100
> The hip04 ethernet driver causes a new compile-time warning
> when built as a loadable module:
>
> WARNING: modpost: missing MODULE_LICENSE() in drivers/net/ethernet/hisilicon/hip04_eth.o
> see include/linux/module.h for more information
>
> This adds the license as "GPL", which matches the header of the file.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied.
^ permalink raw reply
* Re: [PATCH] Documentation: Update netlink_mmap.txt
From: David Miller @ 2015-02-03 2:50 UTC (permalink / raw)
To: richard; +Cc: corbet, shemming, linux-doc, netdev, linux-kernel
In-Reply-To: <1422647444-4004-1-git-send-email-richard@nod.at>
From: Richard Weinberger <richard@nod.at>
Date: Fri, 30 Jan 2015 20:50:44 +0100
> Update netlink_mmap.txt wrt. commit 4682a0358639b29cf
> ("netlink: Always copy on mmap TX.").
>
> Signed-off-by: Richard Weinberger <richard@nod.at>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH -next] net: dctcp: loosen requirement to assert ECT(0) during 3WHS
From: David Miller @ 2015-02-03 2:49 UTC (permalink / raw)
To: fw; +Cc: netdev, daniel, glenn.judd
In-Reply-To: <1422647120-27252-1-git-send-email-fw@strlen.de>
From: Florian Westphal <fw@strlen.de>
Date: Fri, 30 Jan 2015 20:45:20 +0100
> One deployment requirement of DCTCP is to be able to run
> in a DC setting along with TCP traffic. As Glenn Judd's
> NSDI'15 paper "Attaining the Promise and Avoiding the Pitfalls
> of TCP in the Datacenter" [1] (tba) explains, one way to
> solve this on switch side is to split DCTCP and TCP traffic
> in two queues per switch port based on the DSCP: one queue
> soley intended for DCTCP traffic and one for non-DCTCP traffic.
>
> For the DCTCP queue, there's the marking threshold K as
> explained in commit e3118e8359bb ("net: tcp: add DCTCP congestion
> control algorithm") for RED marking ECT(0) packets with CE.
> For the non-DCTCP queue, there's f.e. a classic tail drop queue.
> As already explained in e3118e8359bb, running DCTCP at scale
> when not marking SYN/SYN-ACK packets with ECT(0) has severe
> consequences as for non-ECT(0) packets, traversing the RED
> marking DCTCP queue will result in a severe reduction of
> connection probability.
>
> This is due to the DCTCP queue being dominated by ECT(0) traffic
> and switches handle non-ECT traffic in the RED marking queue
> after passing K as drops, where K is usually a low watermark
> in order to leave enough tailroom for bursts. Splitting DCTCP
> traffic among several queues (ECN and non-ECN queue) is being
> considered a terrible idea in the network community as it
> splits single flows across multiple network paths.
>
> Therefore, commit e3118e8359bb implements this on Linux as
> ECT(0) marked traffic, as we argue that marking all packets
> of a DCTCP flow is the only viable solution and also doesn't
> speak against the draft.
>
> However, recently, a DCTCP implementation for FreeBSD hit also
> their mainline kernel [2]. In order to let them play well
> together with Linux' DCTCP, we would need to loosen the
> requirement that ECT(0) has to be asserted during the 3WHS as
> not implemented in FreeBSD. This simplifies the ECN test and
> lets DCTCP work together with FreeBSD.
>
> Joint work with Daniel Borkmann.
>
> [1] https://www.usenix.org/conference/nsdi15/technical-sessions/presentation/judd
> [2] https://github.com/freebsd/freebsd/commit/8ad879445281027858a7fa706d13e458095b595f
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Applied.
^ permalink raw reply
* Re: [PATCH net-next v3 0/3] net-timestamp: blinding
From: David Miller @ 2015-02-03 2:47 UTC (permalink / raw)
To: richardcochran; +Cc: willemb, netdev, luto
In-Reply-To: <20150201112931.GA4323@localhost.localdomain>
From: Richard Cochran <richardcochran@gmail.com>
Date: Sun, 1 Feb 2015 12:29:31 +0100
> On Fri, Jan 30, 2015 at 01:29:30PM -0500, Willem de Bruijn wrote:
>> Two issues were raised during recent timestamping discussions:
>> 1. looping full packets on the error queue exposes packet headers
>> 2. TCP timestamping with retransmissions generates many timestamps
>
> For the series,
>
> Acked-by: Richard Cochran <richardcochran@gmail.com>
Series applied, thanks everyone.
^ permalink raw reply
* Re: [PATCHv2 net] net: restore lro after device detached from bridge
From: Fan Du @ 2015-02-03 2:29 UTC (permalink / raw)
To: Michal Kubecek; +Cc: Alexander Duyck, Fan Du, bhutchings, davem, netdev
In-Reply-To: <20150202111556.GA8655@unicorn.suse.cz>
于 2015年02月02日 19:15, Michal Kubecek 写道:
> On Mon, Feb 02, 2015 at 10:20:12AM +0800, Fan Du wrote:
>>
>> I think you are talking about bad scenarios when net device is
>> attached to a bridge. Then what's the good reason user has to pay
>> extra cpu power for using GRO, instead of using hw capable LRO/RSC
>> when this net device is detached from bridge acting as a standalone
>> NIC?
>
> Being bridged is only one of the situations when LRO needs to be
> disabled. Does your patch make sure it doesn't enable LRO if there are
> other reasons for it to be disabled, e.g. if forwarding is enabled for
> it or any of its upper devices?
>
> I'm afraid the only way to make the automatic reenabling work correctly
> would be to keep track if LRO was disabled manually (e.g. by ethtool) or
> only automatically because the device is bridged, forwarding is enabled
> for it, LRO is disabled for any upper device etc. And to reenable LRO
> only in the second case and even then only if none of the possible
> reasons holds. I don't think it's worth the effort.
>
>> Note, SRC is defaulted to *ON* in practice for ALL ixgbe NICs, as same
>> other RSC capable NICs.
>
> A very bad idea, IMHO. A lot of bug reports resulted from it.
Why are you saying this an idea?? this a fact for all RSC capable NIC drivers.
search drivers/net/ethernet/ to find more.
>> Attaching net device to a bridge _once_ should not changed its default
>> configuration, moreover it's a subtle change without any message that
>> user won't noticed at all.
> IMHO the key point here is that LRO enabled when it shouldn't is much
> more serious problem than LRO disabled when it could be enabled.
I agree with you here more than I disagree.
btw, I see this subtle behaviour not because I "seeing issues with routing or
bridging being enabled and" as Alexander assuming, it's because I see my testbed
82599EB supports LRO, and the driver enabled after probing from code review,
but for no reason ethtool -k reports lro is off, it looks like this interface
is bound to bridge by one of service.
>
> Michal Kubecek
>
^ permalink raw reply
* Re: [PATCH v2] net: bluetooth: hci_sock: Use 'const void *' instead of 'void *' for 2nd parameter of hci_test_bit()
From: Chen Gang S @ 2015-02-03 2:32 UTC (permalink / raw)
To: Joe Perches
Cc: marcel-kz+m5ild9QBg9hUCZPvPmw, gustavo-THi1TnShQwVAfugRpC6u6w,
johan.hedberg-Re5JQEeQqe8AvxtiuMwx3w, David S. Miller,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1422912017.30476.28.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
On 2/3/15 05:20, Joe Perches wrote:
> On Tue, 2015-02-03 at 05:14 +0800, Chen Gang S wrote:
>> hci_test_bit() does not modify 2nd parameter, so it is better to let it
>> be constant, or may cause build warning. The related warning (with
>> allmodconfig under xtensa):
> []
>> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> []
>> @@ -46,7 +46,7 @@ struct hci_pinfo {
>> unsigned short channel;
>> };
>>
>> -static inline int hci_test_bit(int nr, void *addr)
>> +static inline int hci_test_bit(int nr, const void *addr)
>> {
>> return *((__u32 *) addr + (nr >> 5)) & ((__u32) 1 << (nr & 31));
>> }
>
> It's probably better to use const __u32 * here too, but the
> real thing I wonder is whether or not there's an issue with
> one of the 2 uses of this function.
>
> One of them passes a unsigned long *, the other a u32 *.
>
> $ git grep -w hci_test_bit
> net/bluetooth/hci_sock.c:static inline int hci_test_bit(int nr, void *addr)
> net/bluetooth/hci_sock.c: if (!hci_test_bit(flt_event, &flt->event_mask))
> net/bluetooth/hci_sock.c: !hci_test_bit(ocf & HCI_FLT_OCF_BITS,
> net/bluetooth/hci_sock.c- &hci_sec_filter.ocf_mask[ogf])) &&
>
> hci_sec_filter.ocf_mask is __u32
> but flt->event_mask is unsigned long.
>
> Any possible issue here on 64-bit systems?
>
For me, it can not cause issue on 64-bit systems. hci_test_bit() treats
'addr' as "__u32 *", and has to use the pointer to do something.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply
* Re: [PATCH net] sunvnet: set queue mapping when doing packet copies
From: David Miller @ 2015-02-03 2:21 UTC (permalink / raw)
To: david.stevens; +Cc: netdev, sowmini.varadhan, eric.dumazet
In-Reply-To: <54CBBF89.4070800@oracle.com>
From: David L Stevens <david.stevens@oracle.com>
Date: Fri, 30 Jan 2015 12:29:45 -0500
>
>
> This patch fixes a bug where vnet_skb_shape() didn't set the already-selected
> queue mapping when a packet copy was required. This results in using the
> wrong queue index for stops/starts, hung tx queues and watchdog timeouts
> under heavy load.
>
> Signed-off-by: David L Stevens <david.stevens@oracle.com>
> Acked-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
>
> --
> I'm resubmitting this in its original form. Since skb_segment() doesn't do
> TCP flow control either, due to an incorrect destructor check, and those
> issues raised by Eric are a matter of fairness, while this missing queue_mapping
> assignment results in crashes, I'd like to get this piece fixed separately.
Fair enough, applied.
Please sort out the callback issues with Eric, thanks.
^ permalink raw reply
* Re: [PATCH net-next] pkt_sched: fq: better control of DDOS traffic
From: David Miller @ 2015-02-03 2:18 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1422626772.21689.90.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 30 Jan 2015 06:06:12 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> FQ has a fast path for skb attached to a socket, as it does not
> have to compute a flow hash. But for other packets, FQ being non
> stochastic means that hosts exposed to random Internet traffic
> can allocate million of flows structure (104 bytes each) pretty
> easily. Not only host can OOM, but lookup in RB trees can take
> too much cpu and memory resources.
>
> This patch adds a new attribute, orphan_mask, that is adding
> possibility of having a stochastic hash for orphaned skb.
>
> Its default value is 1024 slots.
>
> This patch also handles the specific case of SYNACK messages:
>
> They are attached to the listener socket, and therefore all map
> to a single hash bucket. If listener have set SO_MAX_PACING_RATE,
> hoping to have new accepted socket inherit this rate, SYNACK
> might be paced and even dropped.
>
> This is very similar to an internal patch Google have used more
> than one year.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Can you document the mask value a little bit more?
For example, I don't understand why "(1024 - 1) << 1" means 1024
slots just from looking at this change.
Thanks.
^ permalink raw reply
* Re: Question: should local address be expired when updating PMTU?
From: Calvin Owens @ 2015-02-03 2:10 UTC (permalink / raw)
To: Alex Gartrell
Cc: shengyong, davem, netdev, yangyingling, steffen.klassert, hannes,
lvs-devel, kernel-team
In-Reply-To: <54D01BEA.2070501@fb.com>
On Monday 02/02 at 16:52 -0800, Alex Gartrell wrote:
> Hello Shengyong,
>
> > diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> > index b2614b2..b80317a 100644
> > --- a/net/ipv6/route.c
> > +++ b/net/ipv6/route.c
> > @@ -1136,6 +1136,9 @@ static void ip6_rt_update_pmtu(struct
> dst_entry *dst, struct sock *sk,
> > {
> > struct rt6_info *rt6 = (struct rt6_info*)dst;
> >
> > + if (rt6->rt6i_flags & RTF_LOCAL)
> > + return;
> > +
> > dst_confirm(dst);
> > if (mtu < dst_mtu(dst) && rt6->rt6i_dst.plen == 128) {
> > struct net *net = dev_net(dst->dev);
> >
> > So is this modification correct? Or how can we avoid such expiring?
>
> FWIW, we encountered this problem with IPVS tunneling. Here's a
> patch done by Calvin (cc'ed) that fixes my attempted fix for this.
> We're not particularly proud of this...
>
> At a high level, I don't think the RTF_LOCAL check was sufficient,
> but I didn't investigate deeply enough and hopefully Calvin can say
> why.
I honestly didn't spend much time at all finding the underlying cause
because it appeared to be fixed upstream: on 3.19-rc5 you get all 3
expected routes after the last step of my repro below. I just really
needed to get this working at the time, and the gross disgusting
horrible ugly awful [more negative adjectives] patch included below made
it work.
FWIW, the explanation I wrote down in my notes was:
"The absence of RTF_NONEXTHOP is causing COWs to happen, which are
always marked as RTF_CACHE. Somehow that's screwing things up in
rt6_do_redirect()"
That could be BS though, I don't at all remember how I came to that
conclusion.
(/me resolves to write better notes in the future...)
Here's how to get the weird behavior on 3.10 (+stable):
$ sudo ip addr add local 4444::1 dev lo
### Now I have 2 routes in /proc/net/ipv6_route, a local and a non-local
### Both have the RTF_NONEXTHOP flag set (0x00200000)
$ sudo ip route add local 4444::1 dev lo
### Now I have 3 routes in /proc/net/ipv6_route to 4444::1
### Notice the new route does NOT have the RTF_NONEXTHOP flag set
$ sudo ip addr del local 4444::1 dev lo
### Now I just have the one route I created before
$ sudo ip addr add local 4444::1 dev lo
### And now I have 3 routes again
$ sudo ping6 4444::1
[blah blah blah successful ping]
$ sudo ip addr del local 4444::1 dev lo
$ sudo ip addr add local 4444::1 dev lo
### Still have 3 routes
$ sudo ip addr del local 4444::1 dev lo
### Now I just have my one route yet again
### Now, *without the address on lo*, talk to it (it works), then re-add it
$ ping6 4444::1
[blah blah blah successful ping]
$ sudo ip addr add local 4444::1 dev lo
### Now I only have 2 routes... WAT!?
### Notice the LOCAL (0x80000000) route doesn't have the RTF_NONEXTHOP flag set
Thanks,
Calvin
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index f14d49b..c607a42 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1159,18 +1159,18 @@ static void ip6_rt_update_pmtu(struct
> dst_entry *dst, struct sock *sk,
> }
> dst_metric_set(dst, RTAX_MTU, mtu);
>
> - /* FACEBOOK HACK: We need to not expire local non-expiring
> - * routes so that we don't accidentally start blackholing
> - * ipvs traffic when we happen to use it locally for
> - * healthchecking (see ip_vs_xmit.c --
> - * __ip_vs_get_out_rt_v6 invokes update_pmtu if the rt is
> - * associated with a socket)
> - * Alex Gartrell <agartrell@fb.com>
> + /*
> + * FACEBOOK HACK: Only expire routes that aren't destined for
> + * the loopback interface.
> + *
> + * This prevents the strange route coalescing that happens when
> + * you add an address to the loopback that had a route that had
> + * been used when the address didn't exist from getting expired
> + * and causing packet loss in shiv.
> */
> - if (!(rt6->rt6i_flags & RTF_LOCAL) ||
> - (rt6->rt6i_flags & (RTF_EXPIRES | RTF_CACHE)))
> - rt6_update_expires(
> - rt6, net->ipv6.sysctl.ip6_rt_mtu_expires);
> + if (!(dst->dev->flags & IFF_LOOPBACK))
> + rt6_update_expires(rt6,
> + net->ipv6.sysctl.ip6_rt_mtu_expires);
> }
> }
>
>
> Cheers,
> --
> Alex Gartrell <agartrell@fb.com>
^ permalink raw reply
* Re: [PATCH net] qlge: Fix qlge_update_hw_vlan_features to handle if interface is down
From: David Miller @ 2015-02-03 1:51 UTC (permalink / raw)
To: mleitner; +Cc: netdev, harish.patil, cdupuis
In-Reply-To: <10e24de024f81e60ab64a6f0bf9647d27eb40be1.1422618157.git.mleitner@redhat.com>
From: Marcelo Ricardo Leitner <mleitner@redhat.com>
Date: Fri, 30 Jan 2015 09:56:01 -0200
> Currently qlge_update_hw_vlan_features() will always first put the
> interface down, then update features and then bring it up again. But it
> is possible to hit this code while the adapter is down and this causes a
> non-paired call to napi_disable(), which will get stuck.
>
> This patch fixes it by skipping these down/up actions if the interface
> is already down.
>
> Fixes: a45adbe8d352 ("qlge: Enhance nested VLAN (Q-in-Q) handling.")
> Cc: Harish Patil <harish.patil@qlogic.com>
> Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH v3 0/3] Restore UFO support to virtio_net devices
From: David Miller @ 2015-02-03 1:49 UTC (permalink / raw)
To: vyasevich
Cc: netdev, virtualization, mst, ben, eric.dumazet, hannes, vyasevic
In-Reply-To: <20150202.131158.2302675247156677150.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Mon, 02 Feb 2015 13:11:58 -0800 (PST)
> Vlad, this still fails the same way.
...
> Please build allmodconfig, that is the exact build I use to test your
> and everyone else's changes.
Oh, also, there is trailing whitespace in one of the comments added
by patch #1. I've been silently fixing it up for you, but since you
have to respin this one more time I'd appreciate it if you're fix
it up in your copy this time. :)
Thanks.
^ permalink raw reply
* Re: Question: should local address be expired when updating PMTU?
From: shengyong @ 2015-02-03 1:28 UTC (permalink / raw)
To: Alex Gartrell
Cc: davem, netdev, yangyingliang, steffen.klassert, hannes, lvs-devel,
Calvin Owens, kernel-team
In-Reply-To: <54D01BEA.2070501@fb.com>
在 2015/2/3 8:52, Alex Gartrell 写道:
> Hello Shengyong,
>
>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>> index b2614b2..b80317a 100644
>> --- a/net/ipv6/route.c
>> +++ b/net/ipv6/route.c
>> @@ -1136,6 +1136,9 @@ static void ip6_rt_update_pmtu(struct dst_entry *dst, struct sock *sk,
>> {
>> struct rt6_info *rt6 = (struct rt6_info*)dst;
>>
>> + if (rt6->rt6i_flags & RTF_LOCAL)
>> + return;
>> +
>> dst_confirm(dst);
>> if (mtu < dst_mtu(dst) && rt6->rt6i_dst.plen == 128) {
>> struct net *net = dev_net(dst->dev);
>>
>> So is this modification correct? Or how can we avoid such expiring?
>
>
> FWIW, we encountered this problem with IPVS tunneling. Here's a patch done by Calvin (cc'ed) that fixes my attempted fix for this. We're not particularly proud of this...
>
> At a high level, I don't think the RTF_LOCAL check was sufficient, but I didn't investigate deeply enough and hopefully Calvin can say why.
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index f14d49b..c607a42 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1159,18 +1159,18 @@ static void ip6_rt_update_pmtu(struct dst_entry *dst, struct sock *sk,
> }
> dst_metric_set(dst, RTAX_MTU, mtu);
>
> - /* FACEBOOK HACK: We need to not expire local non-expiring
> - * routes so that we don't accidentally start blackholing
> - * ipvs traffic when we happen to use it locally for
> - * healthchecking (see ip_vs_xmit.c --
> - * __ip_vs_get_out_rt_v6 invokes update_pmtu if the rt is
> - * associated with a socket)
> - * Alex Gartrell <agartrell@fb.com>
> + /*
> + * FACEBOOK HACK: Only expire routes that aren't destined for
> + * the loopback interface.
> + *
> + * This prevents the strange route coalescing that happens when
> + * you add an address to the loopback that had a route that had
> + * been used when the address didn't exist from getting expired
> + * and causing packet loss in shiv.
> */
> - if (!(rt6->rt6i_flags & RTF_LOCAL) ||
> - (rt6->rt6i_flags & (RTF_EXPIRES | RTF_CACHE)))
> - rt6_update_expires(
> - rt6, net->ipv6.sysctl.ip6_rt_mtu_expires);
> + if (!(dst->dev->flags & IFF_LOOPBACK))
> + rt6_update_expires(rt6,
> + net->ipv6.sysctl.ip6_rt_mtu_expires);
> }
> }
Thanks, your approach can also solve the problem I met. I just a bit confuse that
is this kind of packets (like I sent in the first mail) normal? and if they are
abnormal, I think we'd better drop them before update rt6i_flags.
thx,
Sheng
>
>
> Cheers,
^ permalink raw reply
* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Eric Dumazet @ 2015-02-03 1:18 UTC (permalink / raw)
To: Michal Kazior
Cc: linux-wireless, Network Development,
eyalpe-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb
In-Reply-To: <1422903136.21689.114.camel-XN9IlZ5yJG9HTL0Zs8A6p/gx64E7kk8eUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
On Mon, 2015-02-02 at 10:52 -0800, Eric Dumazet wrote:
> It seems to break ACK clocking badly (linux stack has a somewhat buggy
> tcp_tso_should_defer(), which relies on ACK being received smoothly, as
> no timer is setup to split the TSO packet.)
Following patch might help the TSO split defer logic.
It would avoid setting the TSO defer 'pseudo timer' twice, if/when TCP
Small Queue logic prevented the xmit at the expiration of first 'timer'.
This patch clears the tso_deferred variable only if we could really
send something.
Please try it, thanks !
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 65caf8b95e17..e735f38557db 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1821,7 +1821,6 @@ static bool tcp_tso_should_defer(struct sock *sk,
struct sk_buff *skb,
return true;
send_now:
- tp->tso_deferred = 0;
return false;
}
@@ -2070,6 +2069,7 @@ static bool tcp_write_xmit(struct sock *sk,
unsigned int mss_now, int nonagle,
if (unlikely(tcp_transmit_skb(sk, skb, 1, gfp)))
break;
+ tp->tso_deferred = 0;
repair:
/* Advance the send_head. This one is sent out.
* This call will increment packets_out.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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 related
* Re: Question: should local address be expired when updating PMTU?
From: Alex Gartrell @ 2015-02-03 0:52 UTC (permalink / raw)
To: shengyong, davem
Cc: netdev, yangyingling, steffen.klassert, hannes, lvs-devel,
Calvin Owens, kernel-team
In-Reply-To: <54CF3348.40207@huawei.com>
Hello Shengyong,
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index b2614b2..b80317a 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1136,6 +1136,9 @@ static void ip6_rt_update_pmtu(struct dst_entry
*dst, struct sock *sk,
> {
> struct rt6_info *rt6 = (struct rt6_info*)dst;
>
> + if (rt6->rt6i_flags & RTF_LOCAL)
> + return;
> +
> dst_confirm(dst);
> if (mtu < dst_mtu(dst) && rt6->rt6i_dst.plen == 128) {
> struct net *net = dev_net(dst->dev);
>
> So is this modification correct? Or how can we avoid such expiring?
FWIW, we encountered this problem with IPVS tunneling. Here's a patch
done by Calvin (cc'ed) that fixes my attempted fix for this. We're not
particularly proud of this...
At a high level, I don't think the RTF_LOCAL check was sufficient, but I
didn't investigate deeply enough and hopefully Calvin can say why.
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index f14d49b..c607a42 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1159,18 +1159,18 @@ static void ip6_rt_update_pmtu(struct dst_entry
*dst, struct sock *sk,
}
dst_metric_set(dst, RTAX_MTU, mtu);
- /* FACEBOOK HACK: We need to not expire local non-expiring
- * routes so that we don't accidentally start blackholing
- * ipvs traffic when we happen to use it locally for
- * healthchecking (see ip_vs_xmit.c --
- * __ip_vs_get_out_rt_v6 invokes update_pmtu if the rt is
- * associated with a socket)
- * Alex Gartrell <agartrell@fb.com>
+ /*
+ * FACEBOOK HACK: Only expire routes that aren't
destined for
+ * the loopback interface.
+ *
+ * This prevents the strange route coalescing that
happens when
+ * you add an address to the loopback that had a route
that had
+ * been used when the address didn't exist from getting
expired
+ * and causing packet loss in shiv.
*/
- if (!(rt6->rt6i_flags & RTF_LOCAL) ||
- (rt6->rt6i_flags & (RTF_EXPIRES | RTF_CACHE)))
- rt6_update_expires(
- rt6, net->ipv6.sysctl.ip6_rt_mtu_expires);
+ if (!(dst->dev->flags & IFF_LOOPBACK))
+ rt6_update_expires(rt6,
+
net->ipv6.sysctl.ip6_rt_mtu_expires);
}
}
Cheers,
--
Alex Gartrell <agartrell@fb.com>
^ permalink raw reply related
* Re: [PATCH v2 18/18] vhost: vhost_scsi_handle_vq() should just use copy_from_user()
From: Nicholas A. Bellinger @ 2015-02-03 0:42 UTC (permalink / raw)
To: Al Viro; +Cc: David Miller, netdev, Michael S. Tsirkin, kvm, target-devel
In-Reply-To: <1422863977-17668-18-git-send-email-viro@ZenIV.linux.org.uk>
Hi Al,
On Mon, 2015-02-02 at 07:59 +0000, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
>
> it has just verified that it asks no more than the length of the
> first segment of iovec.
>
> And with that the last user of stuff in lib/iovec.c is gone.
> RIP.
>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Nicholas A. Bellinger <nab@linux-iscsi.org>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> drivers/vhost/scsi.c | 2 +-
> include/linux/uio.h | 2 --
> lib/Makefile | 2 +-
> lib/iovec.c | 36 ------------------------------------
> 4 files changed, 2 insertions(+), 40 deletions(-)
> delete mode 100644 lib/iovec.c
>
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index d695b16..dc78d87 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1079,7 +1079,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
> req_size, vq->iov[0].iov_len);
> break;
> }
> - ret = memcpy_fromiovecend(req, &vq->iov[0], 0, req_size);
> + ret = copy_from_user(req, vq->iov[0].iov_base, req_size);
> if (unlikely(ret)) {
> vq_err(vq, "Faulted on virtio_scsi_cmd_req\n");
> break;
Is this in for-next yet..?
If not, please push out ASAP so SFR can hit the conflict between
target-pending/for-next as a heads up for Linus..
--nab
^ permalink raw reply
* Re: [target:for-next 16/21] drivers/vhost/scsi.c:1081:5: sparse: symbol 'vhost_skip_iovec_bytes' was not declared. Should it be static?
From: Nicholas A. Bellinger @ 2015-02-03 0:24 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, Michael S. Tsirkin, kvm, virtualization, netdev,
linux-kernel, target-devel
In-Reply-To: <201502021411.KBLYtCzS%fengguang.wu@intel.com>
On Mon, 2015-02-02 at 14:25 +0800, kbuild test robot wrote:
> tree: git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git for-next
> head: 2936f1d4f3e8247bd519feba7892371d5e4c6603
> commit: 105acf608f25d5e0d9fef669299a5438b7b114ee [16/21] vhost/scsi: Add ANY_LAYOUT vhost_skip_iovec_bytes helper
> reproduce:
> # apt-get install sparse
> git checkout 105acf608f25d5e0d9fef669299a5438b7b114ee
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
> >> drivers/vhost/scsi.c:1081:5: sparse: symbol 'vhost_skip_iovec_bytes' was not declared. Should it be static?
> drivers/vhost/scsi.c:969:1: warning: 'vhost_scsi_mapal' defined but not used [-Wunused-function]
> vhost_scsi_mapal(struct tcm_vhost_cmd *cmd, int max_niov,
> ^
>
> Please review and possibly fold the followup patch.
Fixing up for -v3.
Thanks Fengguang!
--nab
^ permalink raw reply
* Re: [PATCH] net: ipv6: Make address flushing on ifdown optional - v2
From: Stephen Hemminger @ 2015-02-03 0:18 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <54D0124C.70202@gmail.com>
On Mon, 02 Feb 2015 17:11:56 -0700
David Ahern <dsahern@gmail.com> wrote:
> On 2/2/15 4:10 PM, Stephen Hemminger wrote:
> > Checkpatch complains about space before tab, which could be your
> > mailer mangling the patch.
>
> Yes, found that a couple of days ago. I use 'git send-email' to send the
> patches, and I have a pre-commit hook to run checkpatch.pl. Perhaps I
> goofed something in the recent latptop transitions.
>
> >
> > This should probably go into the netconf netlink message as well.
> >
>
> Not sure I understand this point. Can you point me to a code reference
> to look up?
Look at how forwarding flag is propagated already via NETCONFA_FORWARDING.
^ permalink raw reply
* Re: [PATCH] net: ipv6: Make address flushing on ifdown optional - v2
From: David Ahern @ 2015-02-03 0:11 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <20150203001057.3e0e22b2@uryu.home.lan>
On 2/2/15 4:10 PM, Stephen Hemminger wrote:
> Checkpatch complains about space before tab, which could be your
> mailer mangling the patch.
Yes, found that a couple of days ago. I use 'git send-email' to send the
patches, and I have a pre-commit hook to run checkpatch.pl. Perhaps I
goofed something in the recent latptop transitions.
>
> This should probably go into the netconf netlink message as well.
>
Not sure I understand this point. Can you point me to a code reference
to look up?
Though that did remind me that I should add a blurb in
Documentation/networking/ip-sysctl.txt.
David
^ permalink raw reply
* [PATCH net-next] net: add skb functions to process remote checksum offload
From: Tom Herbert @ 2015-02-03 0:07 UTC (permalink / raw)
To: davem, netdev
This patch adds skb_remcsum_process and skb_gro_remcsum_process to
perform the appropriate adjustments to the skb when receiving
remote checksum offload.
Updated vxlan and gue to use these functions.
Tested: Ran TCP_RR and TCP_STREAM netperf for VXLAN and GUE, did
not see any change in performance.
Signed-off-by: Tom Herbert <therbert@google.com>
---
drivers/net/vxlan.c | 18 ++----------------
include/linux/netdevice.h | 15 +++++++++++++++
include/linux/skbuff.h | 21 +++++++++++++++++++++
net/ipv4/fou.c | 18 ++----------------
4 files changed, 40 insertions(+), 32 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 19d3664..ffea961 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -556,7 +556,6 @@ static struct vxlanhdr *vxlan_gro_remcsum(struct sk_buff *skb,
u32 data)
{
size_t start, offset, plen;
- __wsum delta;
if (skb->remcsum_offload)
return vh;
@@ -578,12 +577,7 @@ static struct vxlanhdr *vxlan_gro_remcsum(struct sk_buff *skb,
return NULL;
}
- delta = remcsum_adjust((void *)vh + hdrlen,
- NAPI_GRO_CB(skb)->csum, start, offset);
-
- /* Adjust skb->csum since we changed the packet */
- skb->csum = csum_add(skb->csum, delta);
- NAPI_GRO_CB(skb)->csum = csum_add(NAPI_GRO_CB(skb)->csum, delta);
+ skb_gro_remcsum_process(skb, (void *)vh + hdrlen, start, offset);
skb->remcsum_offload = 1;
@@ -1157,7 +1151,6 @@ static struct vxlanhdr *vxlan_remcsum(struct sk_buff *skb, struct vxlanhdr *vh,
size_t hdrlen, u32 data)
{
size_t start, offset, plen;
- __wsum delta;
if (skb->remcsum_offload) {
/* Already processed in GRO path */
@@ -1177,14 +1170,7 @@ static struct vxlanhdr *vxlan_remcsum(struct sk_buff *skb, struct vxlanhdr *vh,
vh = (struct vxlanhdr *)(udp_hdr(skb) + 1);
- if (unlikely(skb->ip_summed != CHECKSUM_COMPLETE))
- __skb_checksum_complete(skb);
-
- delta = remcsum_adjust((void *)vh + hdrlen,
- skb->csum, start, offset);
-
- /* Adjust skb->csum since we changed the packet */
- skb->csum = csum_add(skb->csum, delta);
+ skb_remcsum_process(skb, (void *)vh + hdrlen, start, offset);
return vh;
}
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 642d426..38af7b3 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2314,6 +2314,21 @@ do { \
compute_pseudo(skb, proto)); \
} while (0)
+static inline void skb_gro_remcsum_process(struct sk_buff *skb, void *ptr,
+ int start, int offset)
+{
+ __wsum delta;
+
+ BUG_ON(!NAPI_GRO_CB(skb)->csum_valid);
+
+ delta = remcsum_adjust(ptr, NAPI_GRO_CB(skb)->csum, start, offset);
+
+ /* Adjust skb->csum since we changed the packet */
+ skb->csum = csum_add(skb->csum, delta);
+ NAPI_GRO_CB(skb)->csum = csum_add(NAPI_GRO_CB(skb)->csum, delta);
+}
+
+
static inline int dev_hard_header(struct sk_buff *skb, struct net_device *dev,
unsigned short type,
const void *daddr, const void *saddr,
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 85ab7d7..67f8b76 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3096,6 +3096,27 @@ do { \
compute_pseudo(skb, proto)); \
} while (0)
+/* Update skbuf and packet to reflect the remote checksum offload operation.
+ * When called, ptr indicates the starting point for skb->csum when
+ * ip_summed is CHECKSUM_COMPLETE. If we need create checksum complete
+ * here, skb_postpull_rcsum is done so skb->csum start is ptr.
+ */
+static inline void skb_remcsum_process(struct sk_buff *skb, void *ptr,
+ int start, int offset)
+{
+ __wsum delta;
+
+ if (unlikely(skb->ip_summed != CHECKSUM_COMPLETE)) {
+ __skb_checksum_complete(skb);
+ skb_postpull_rcsum(skb, skb->data, ptr - (void *)skb->data);
+ }
+
+ delta = remcsum_adjust(ptr, skb->csum, start, offset);
+
+ /* Adjust skb->csum since we changed the packet */
+ skb->csum = csum_add(skb->csum, delta);
+}
+
#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
void nf_conntrack_destroy(struct nf_conntrack *nfct);
static inline void nf_conntrack_put(struct nf_conntrack *nfct)
diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index 3bc0cf0..92ddea1 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -70,7 +70,6 @@ static struct guehdr *gue_remcsum(struct sk_buff *skb, struct guehdr *guehdr,
size_t start = ntohs(pd[0]);
size_t offset = ntohs(pd[1]);
size_t plen = hdrlen + max_t(size_t, offset + sizeof(u16), start);
- __wsum delta;
if (skb->remcsum_offload) {
/* Already processed in GRO path */
@@ -82,14 +81,7 @@ static struct guehdr *gue_remcsum(struct sk_buff *skb, struct guehdr *guehdr,
return NULL;
guehdr = (struct guehdr *)&udp_hdr(skb)[1];
- if (unlikely(skb->ip_summed != CHECKSUM_COMPLETE))
- __skb_checksum_complete(skb);
-
- delta = remcsum_adjust((void *)guehdr + hdrlen,
- skb->csum, start, offset);
-
- /* Adjust skb->csum since we changed the packet */
- skb->csum = csum_add(skb->csum, delta);
+ skb_remcsum_process(skb, (void *)guehdr + hdrlen, start, offset);
return guehdr;
}
@@ -228,7 +220,6 @@ static struct guehdr *gue_gro_remcsum(struct sk_buff *skb, unsigned int off,
size_t start = ntohs(pd[0]);
size_t offset = ntohs(pd[1]);
size_t plen = hdrlen + max_t(size_t, offset + sizeof(u16), start);
- __wsum delta;
if (skb->remcsum_offload)
return guehdr;
@@ -243,12 +234,7 @@ static struct guehdr *gue_gro_remcsum(struct sk_buff *skb, unsigned int off,
return NULL;
}
- delta = remcsum_adjust((void *)guehdr + hdrlen,
- NAPI_GRO_CB(skb)->csum, start, offset);
-
- /* Adjust skb->csum since we changed the packet */
- skb->csum = csum_add(skb->csum, delta);
- NAPI_GRO_CB(skb)->csum = csum_add(NAPI_GRO_CB(skb)->csum, delta);
+ skb_gro_remcsum_process(skb, (void *)guehdr + hdrlen, start, offset);
skb->remcsum_offload = 1;
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* Re: [PATCH] net: ipv6: Make address flushing on ifdown optional - v2
From: Stephen Hemminger @ 2015-02-02 23:10 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1422504065-17445-1-git-send-email-dsahern@gmail.com>
On Wed, 28 Jan 2015 21:01:05 -0700
David Ahern <dsahern@gmail.com> wrote:
> Currently, all ipv6 addresses are flushed when the interface is configured
> down, even static address:
>
> [root@f20 ~]# ip -6 addr add dev eth1 2000:11:1:1::1/64
> [root@f20 ~]# ip addr show dev eth1
> 3: eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default qlen 1000
> link/ether 02:04:11:22:33:01 brd ff:ff:ff:ff:ff:ff
> inet6 2000:11:1:1::1/64 scope global tentative
> valid_lft forever preferred_lft forever
> [root@f20 ~]# ip link set dev eth1 up
> [root@f20 ~]# ip link set dev eth1 down
> [root@f20 ~]# ip addr show dev eth1
> 3: eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc pfifo_fast state DOWN group default qlen 1000
> link/ether 02:04:11:22:33:01 brd ff:ff:ff:ff:ff:ff
>
> Add a new sysctl to make this behavior optional. The new setting defaults to
> flush all addresses to maintain backwards compatibility. When the setting is
> reset static addresses are not flushed:
>
> [root@f20 ~]# echo 0 > /proc/sys/net/ipv6/conf/eth1/flush_addr_on_down
> [root@f20 ~]# ip -6 addr add dev eth1 2000:11:1:1::1/64
> [root@f20 ~]# ip addr show dev eth1
> 3: eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc pfifo_fast state DOWN group default qlen 1000
> link/ether 02:04:11:22:33:01 brd ff:ff:ff:ff:ff:ff
> inet6 2000:11:1:1::1/64 scope global tentative
> valid_lft forever preferred_lft forever
> [root@f20 ~]# ip link set dev eth1 up
> [root@f20 ~]# ip link set dev eth1 down
> [root@f20 ~]# ip addr show dev eth1
> 3: eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc pfifo_fast state DOWN group default qlen 1000
> link/ether 02:04:11:22:33:01 brd ff:ff:ff:ff:ff:ff
> inet6 2000:11:1:1::1/64 scope global
> valid_lft forever preferred_lft forever
> inet6 fe80::4:11ff:fe22:3301/64 scope link
> valid_lft forever preferred_lft forever
>
> v2:
> - only keep static addresses as suggested by Hannes
> - added new managed flag to track configured addresses
> - on ifdown do not remove from configured address from inet6_addr_lst
> - on ifdown reset the TENTATIVE flag and set state to DAD so that DAD is
> redone when link is brought up again
>
> Suggested-by: Hannes Frederic Sowa <hannes@redhat.com>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> Cc: Hannes Frederic Sowa <hannes@redhat.com>
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
Checkpatch complains about space before tab, which could be your
mailer mangling the patch.
This should probably go into the netconf netlink message as well.
^ permalink raw reply
* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Eric Dumazet @ 2015-02-02 23:06 UTC (permalink / raw)
To: Ben Greear
Cc: Michal Kazior, linux-wireless, Network Development,
eyalpe-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb
In-Reply-To: <54CFEB46.3050006-my8/4N5VtI7c+919tysfdA@public.gmane.org>
On Mon, 2015-02-02 at 13:25 -0800, Ben Greear wrote:
> It is a big throughput win to have fewer TCP ack packets on
> wireless since it is a half-duplex environment. Is there anything
> we could improve so that we can have fewer acks and still get
> good tcp stack behaviour?
First apply TCP stretch ack fixes to the sender. There is no way to get
good performance if the sender does not handle stretch ack.
d6b1a8a92a14 tcp: fix timing issue in CUBIC slope calculation
9cd981dcf174 tcp: fix stretch ACK bugs in CUBIC
c22bdca94782 tcp: fix stretch ACK bugs in Reno
814d488c6126 tcp: fix the timid additive increase on stretch ACKs
e73ebb0881ea tcp: stretch ACK fixes prep
Then, make sure you do not throttle ACK too long, especially if you hope
to get Gbit line rate on a 4 ms RTT flow.
GRO does not mean : send one ACK every ms, or after 3ms delay...
It is literally :
aggregate X packets at receive, and send the ACK asap.
If the receiver expects to have 64 ACK packets in the TX ring buffer to
actually send them (wifi aggregation), then you certainly do not want to
compress ACK too much.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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: [PATCH net-next 0/3] openvswitch: Add STT support.
From: Tom Herbert @ 2015-02-02 22:49 UTC (permalink / raw)
To: Jesse Gross; +Cc: Pravin Shelar, David Miller, Linux Netdev List
In-Reply-To: <CAEP_g=_rBmDKDM7fjud78YY=HLnf1r3OdcXux=tm55TxG1DFig@mail.gmail.com>
On Mon, Feb 2, 2015 at 12:39 PM, Jesse Gross <jesse@nicira.com> wrote:
> On Mon, Feb 2, 2015 at 8:23 AM, Tom Herbert <therbert@google.com> wrote:
>>> I would recommend you take a look at the draft if you haven't already:
>>> http://tools.ietf.org/html/draft-davie-stt-06
>>>
>>> It is currently in the final stages of the RFC publication process.
>>
>> Sorry, but this statement is completely wrong and misleading.
>> According to datatracker this draft has been expired since October,
>> there's been no discussion on it in IETF, and this has not gone to
>> IESG. You cannot say this is an IETF standard nor that it is about to
>> be published as one. See
>> https://datatracker.ietf.org/doc/draft-davie-stt/ and please read the
>> Internet Standards process in RFC2026.
>>
>> I suggest that you update the draft and post it on both nvo3 and tsvwg
>> so there can be some real discussion on the implications of
>> repurposing an IP protocol number and breaking TCP protocol standards.
>
> Seriously, Tom?
>
> It's currently in the ISE queue to be published as an RFC, which you
> can see in the history in the datatracker link. I didn't say that it
> was being published as a standard, I said RFC. This is the same
> process and status that VXLAN has.. It also was presented in nvo3
> before you started coming.
>
> Please get your facts straight before making accusations.
The draft has not come up before the IESG, I have confirmed that with the ADs.
^ 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