* [PATCH] ipv4: Don't increase PMTU with Datagram Too Big message.
From: Li Wei @ 2015-01-29 8:09 UTC (permalink / raw)
To: davem; +Cc: netdev, Li Wei
RFC 1191 said, "a host MUST not increase its estimate of the Path
MTU in response to the contents of a Datagram Too Big message."
Signed-off-by: Li Wei <lw@cn.fujitsu.com>
---
net/ipv4/route.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index d58dd0e..52e1f2b 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -966,6 +966,9 @@ static void __ip_rt_update_pmtu(struct rtable *rt, struct flowi4 *fl4, u32 mtu)
if (dst->dev->mtu < mtu)
return;
+ if (rt->rt_pmtu && rt->rt_pmtu < mtu)
+ return;
+
if (mtu < ip_rt_min_pmtu)
mtu = ip_rt_min_pmtu;
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] net: remove sock_iocb
From: David Miller @ 2015-01-29 8:01 UTC (permalink / raw)
To: viro; +Cc: hch, netdev
In-Reply-To: <20150129075721.GD29656@ZenIV.linux.org.uk>
From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Thu, 29 Jan 2015 07:57:21 +0000
> You know, that's getting _really_ interesting. The thing is, now
> there's only one ->sendmsg() instance using iocb argument at all,
> and it's a really weird one. TIPC. Which only compares it with
> NULL, and that - to tell the normal calls (== done by sock_sendmsg()
> et.al.) from tipc_{accept,connect}()-generated ones. And the way
> it's used is
> if (iocb)
> lock_sock(sk);
> in tipc_send_stream(). IOW, "tipc_accept() and tipc_connect() would like
> to use the guts of tipc_send_stream(), but they are already holding the
> socket locked; let's just pass NULL iocb (which net/socket.c never does)
> to tell it to leave the fucking lock alone, thank you very much".
>
> And no ->recvmsg() are using iocb at all now. How about we take the
> guts of tipc_send_stream() into a helper function and have tipc_accept/connect
> use _that_? Then we could drop iocb argument completely and for ->sendmsg()
> it would be the difference between 4 and 3 arguments, which has interesting
> effects on certain register-starved architectures...
>
> While we are at it, size (both for sendmsg and recvmsg) is always equal to
> iov_iter_count(&msg->msg_iter), so that's not the only redundant argument
> there...
No objections from me.
^ permalink raw reply
* Re: [PATCH] net: remove sock_iocb
From: Al Viro @ 2015-01-29 7:57 UTC (permalink / raw)
To: David Miller; +Cc: hch, netdev
In-Reply-To: <20150128.232211.1157376728217761804.davem@davemloft.net>
On Wed, Jan 28, 2015 at 11:22:11PM -0800, David Miller wrote:
> From: Christoph Hellwig <hch@lst.de>
> Date: Wed, 28 Jan 2015 18:04:53 +0100
>
> > The sock_iocb structure is allocate on stack for each read/write-like
> > operation on sockets, and contains various fields of which only the
> > embedded msghdr and sometimes a pointer to the scm_cookie is ever used.
> > Get rid of the sock_iocb and put a msghdr directly on the stack and pass
> > the scm_cookie explicitly to netlink_mmap_sendmsg.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> Looks good, applied, thanks.
You know, that's getting _really_ interesting. The thing is, now
there's only one ->sendmsg() instance using iocb argument at all,
and it's a really weird one. TIPC. Which only compares it with
NULL, and that - to tell the normal calls (== done by sock_sendmsg()
et.al.) from tipc_{accept,connect}()-generated ones. And the way
it's used is
if (iocb)
lock_sock(sk);
in tipc_send_stream(). IOW, "tipc_accept() and tipc_connect() would like
to use the guts of tipc_send_stream(), but they are already holding the
socket locked; let's just pass NULL iocb (which net/socket.c never does)
to tell it to leave the fucking lock alone, thank you very much".
And no ->recvmsg() are using iocb at all now. How about we take the
guts of tipc_send_stream() into a helper function and have tipc_accept/connect
use _that_? Then we could drop iocb argument completely and for ->sendmsg()
it would be the difference between 4 and 3 arguments, which has interesting
effects on certain register-starved architectures...
While we are at it, size (both for sendmsg and recvmsg) is always equal to
iov_iter_count(&msg->msg_iter), so that's not the only redundant argument
there...
Comments?
^ permalink raw reply
* Re: [PATCH net] tcp: ipv4: initialize unicast_sock sk_pacing_rate
From: David Miller @ 2015-01-29 7:25 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1422452831.29618.49.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 28 Jan 2015 05:47:11 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> When I added sk_pacing_rate field, I forgot to initialize its value
> in the per cpu unicast_sock used in ip_send_unicast_reply()
>
> This means that for sch_fq users, RST packets, or ACK packets sent
> on behalf of TIME_WAIT sockets might be sent to slowly or even dropped
> once we reach the per flow limit.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Fixes: 95bd09eb2750 ("tcp: TSO packets automatic sizing")
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net-next] pkt_sched: fq: remove useless TIME_WAIT check
From: David Miller @ 2015-01-29 7:24 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1422453996.29618.64.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 28 Jan 2015 06:06:36 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> TIME_WAIT sockets are not owning any skb.
>
> ip_send_unicast_reply() and tcp_v6_send_response() both use
> regular sockets.
>
> We can safely remove a test in sch_fq and save one cache line miss,
> as sk_state is far away from sk_pacing_rate.
>
> Tested at Google for about one year.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH] act_connmark: fix dependencies better
From: David Miller @ 2015-01-29 7:23 UTC (permalink / raw)
To: arnd; +Cc: netdev, tgraf, nbd, jhs
In-Reply-To: <1806649.OoZh1UV3MM@wuerfel>
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 28 Jan 2015 17:30:09 +0100
> NET_ACT_CONNMARK fails to build if NF_CONNTRACK_MARK is disabled,
> and d7924450e14ea4 ("act_connmark: Add missing dependency on
> NF_CONNTRACK_MARK") fixed that case, but missed the cased where
> NF_CONNTRACK is a loadable module.
>
> This adds the second dependency to ensure that NET_ACT_CONNMARK
> can only be built-in if NF_CONNTRACK is also part of the kernel
> rather than a loadable module.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied, thanks Arnd.
^ permalink raw reply
* Re: [PATCH] net: remove sock_iocb
From: David Miller @ 2015-01-29 7:22 UTC (permalink / raw)
To: hch; +Cc: netdev
In-Reply-To: <1422464693-7827-1-git-send-email-hch@lst.de>
From: Christoph Hellwig <hch@lst.de>
Date: Wed, 28 Jan 2015 18:04:53 +0100
> The sock_iocb structure is allocate on stack for each read/write-like
> operation on sockets, and contains various fields of which only the
> embedded msghdr and sometimes a pointer to the scm_cookie is ever used.
> Get rid of the sock_iocb and put a msghdr directly on the stack and pass
> the scm_cookie explicitly to netlink_mmap_sendmsg.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks good, applied, thanks.
^ permalink raw reply
* Re: [patch net-next] hisilicon: add some missing curly braces
From: David Miller @ 2015-01-29 7:07 UTC (permalink / raw)
To: dingtianhong; +Cc: dan.carpenter, zhangfei.gao, arnd, netdev, kernel-janitors
In-Reply-To: <54C99FFB.2080207@huawei.com>
From: Ding Tianhong <dingtianhong@huawei.com>
Date: Thu, 29 Jan 2015 10:50:35 +0800
> On 2015/1/29 2:58, Dan Carpenter wrote:
>> The if block was supposed to have curly braces. In the current code we
>> complain about dropped rx packets when we shouldn't.
>>
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>
> Acked-by: Ding Tianhong <dingtianhong@huawei.com>
Applied, thanks everyone.
^ permalink raw reply
* Re: [PATCH v2 linux-trace 1/8] tracing: attach eBPF programs to tracepoints and syscalls
From: Alexei Starovoitov @ 2015-01-29 7:04 UTC (permalink / raw)
To: Namhyung Kim
Cc: Steven Rostedt, Ingo Molnar, Arnaldo Carvalho de Melo, Jiri Olsa,
Masami Hiramatsu, Linux API, Network Development, LKML
On Wed, Jan 28, 2015 at 10:41 PM, Namhyung Kim <namhyung@kernel.org> wrote:
>
> I think it's not a problem of bpf. An user process can be killed
> anytime while it enabed events without bpf. The only thing it should
> care is the auto-unload IMHO.
ok. I think it does indeed make sense to decouple the logic.
We can add 'auto_enable' file to achieve desired Ctrl-C behavior.
While the 'auto_enable' file is open the event will be enabled
and writes to 'enable' file will be ignored.
As soon as file closes, the event is auto-disabled.
Then user space will use 'bpf' file to attach/auto-unload
and 'auto_enable' file together.
Seem there would be a use for such 'auto_enable'
without bpf as well.
> I'm okay for not calling bpf program in NMI but not for disabling events.
>
> Suppose an user was collecting an event (including in NMI) and then
> [s]he also wanted to run a bpf program. So [s]he wrote a program
> always return 1. But after attaching the program, it didn't record
> the event in NMI.. Isn't that a problem?
ok, I think 'if (in_nmi()) return 1;' will work then, right?
Or you're thinking something else ?
> Right. I think bpf programs belong to a user process but events are
> global resource. Maybe you also need to consider attaching bpf
> program via perf (ioctl?) interface..
yes. I did. Please see my reply to Masami.
ioctl only works for tracepoints.
^ permalink raw reply
* Re: [PATCH net-next] openvswitch: Add support for checksums on UDP tunnels.
From: David Miller @ 2015-01-29 7:04 UTC (permalink / raw)
To: jesse; +Cc: pshelar, netdev, dev
In-Reply-To: <1422491566-80453-1-git-send-email-jesse@nicira.com>
From: Jesse Gross <jesse@nicira.com>
Date: Wed, 28 Jan 2015 16:32:46 -0800
> Currently, it isn't possible to request checksums on the outer UDP
> header of tunnels - the TUNNEL_CSUM flag is ignored. This adds
> support for requesting that UDP checksums be computed on transmit
> and properly reported if they are present on receive.
>
> Signed-off-by: Jesse Gross <jesse@nicira.com>
Applied.
^ permalink raw reply
* Re: [GIT] [3.20] NFC update
From: David Miller @ 2015-01-29 7:02 UTC (permalink / raw)
To: sameo; +Cc: netdev
In-Reply-To: <20150129005216.GA1137@ribalta.ger.corp.intel.com>
From: Samuel Ortiz <sameo@linux.intel.com>
Date: Thu, 29 Jan 2015 01:52:16 +0100
> This is the first NFC pull request for 3.20.
>
> With this one we have:
>
> - Secure element support for the ST Micro st21nfca driver. This depends
> on a few HCI internal changes in order for example to support more
> than one secure element per controller.
>
> - ACPI support for NXP's pn544 HCI driver. This controller is found on
> many x86 SoCs and is typically enumerated on the ACPI bus there.
>
> - A few st21nfca and st21nfcb fixes. Since they're not critical fixes,
> I assumed it's too late for a 3.19 inclusion. Please let me know if
> you'd prefer to see them going through a net pull request instead.
Pulled, thanks Sam.
^ permalink raw reply
* Re: [GIT] [3.20] NFC update
From: David Miller @ 2015-01-29 6:50 UTC (permalink / raw)
To: sameo; +Cc: netdev
In-Reply-To: <20150129005216.GA1137@ribalta.ger.corp.intel.com>
From: Samuel Ortiz <sameo@linux.intel.com>
Date: Thu, 29 Jan 2015 01:52:16 +0100
> This is the first NFC pull request for 3.20.
>
> With this one we have:
>
> - Secure element support for the ST Micro st21nfca driver. This depends
> on a few HCI internal changes in order for example to support more
> than one secure element per controller.
>
> - ACPI support for NXP's pn544 HCI driver. This controller is found on
> many x86 SoCs and is typically enumerated on the ACPI bus there.
>
> - A few st21nfca and st21nfcb fixes. Since they're not critical fixes,
> I assumed it's too late for a 3.19 inclusion. Please let me know if
> you'd prefer to see them going through a net pull request instead.
Applied, thanks Samuel.
^ permalink raw reply
* Re: [PATCH v2] bridge: dont send notification when skb->len == 0 in rtnl_bridge_notify
From: David Miller @ 2015-01-29 6:22 UTC (permalink / raw)
To: roopa; +Cc: netdev, stephen, rami.rosen
In-Reply-To: <1422490991-4033-1-git-send-email-roopa@cumulusnetworks.com>
From: roopa@cumulusnetworks.com
Date: Wed, 28 Jan 2015 16:23:11 -0800
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> Reported in: https://bugzilla.kernel.org/show_bug.cgi?id=92081
>
> This patch avoids calling rtnl_notify if the device ndo_bridge_getlink
> handler does not return any bytes in the skb.
>
> Alternately, the skb->len check can be moved inside rtnl_notify.
>
> For the bridge vlan case described in 92081, there is also a fix needed
> in bridge driver to generate a proper notification. Will fix that in
> subsequent patch.
>
> v2: rebase patch on net tree
>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net, ethernet, LLVMLinux: Add missing MODULE_DEVICE_TABLE()
From: David Miller @ 2015-01-29 6:42 UTC (permalink / raw)
To: behanw
Cc: sathya.perla, ajit.khaparde, linux-kernel, netdev,
subbu.seetharaman, arnd
In-Reply-To: <1422495374-22835-1-git-send-email-behanw@converseincode.com>
From: Behan Webster <behanw@converseincode.com>
Date: Wed, 28 Jan 2015 17:36:14 -0800
> Missing MODULE_DEVICE_TABLE for pci ids from benet driver found by clang.
>
> Signed-off-by: Behan Webster <behanw@converseincode.com>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
Why are you removing the device table?
Second of all, your Subject needs to be adjusted, using "net" and
"LLVMLinux" in your subsystem prefix is not appropriate. Simply
"be2net: ", the name of this driver, is sufficient.
^ permalink raw reply
* Re: [PATCH v2 linux-trace 1/8] tracing: attach eBPF programs to tracepoints and syscalls
From: Namhyung Kim @ 2015-01-29 6:41 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Steven Rostedt, Ingo Molnar, Arnaldo Carvalho de Melo, Jiri Olsa,
Masami Hiramatsu, Linux API, Network Development, LKML
In-Reply-To: <CAMEtUuz+N52ms4ZFh8+fSGhg1UaXpBdk9RVPU_zBDCjaZ4bj4A@mail.gmail.com>
On Thu, Jan 29, 2015 at 1:40 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> On Wed, Jan 28, 2015 at 4:46 PM, Namhyung Kim <namhyung@kernel.org> wrote:
>>>
>>> +static int event_filter_release(struct inode *inode, struct file *filp)
>>> +{
>>> + struct ftrace_event_file *file;
>>> + char buf[2] = "0";
>>> +
>>> + mutex_lock(&event_mutex);
>>> + file = event_file_data(filp);
>>> + if (file) {
>>> + if (file->flags & TRACE_EVENT_FL_BPF) {
>>> + /* auto-disable the filter */
>>> + ftrace_event_enable_disable(file, 0);
>>
>> Hmm.. what if user already enabled an event, attached a bpf filter and
>> then detached the filter - I'm not sure we can always auto-disable
>> it..
>
> why not?
> I think it makes sense auto enable/disable, since that
> is cleaner user experience.
> Otherwise Ctrl-C of the user process will have bpf program dangling.
> not good. If we auto-unload bpf program only, it's equally bad.
> Since Ctrl-C of the process will auto-onload only
> and will keep tracepoint enabled which will be spamming
> the trace buffer.
I think it's not a problem of bpf. An user process can be killed
anytime while it enabed events without bpf. The only thing it should
care is the auto-unload IMHO.
>
>>> +unsigned int trace_filter_call_bpf(struct event_filter *filter, void *ctx)
>>> +{
>>> + unsigned int ret;
>>> +
>>> + if (in_nmi()) /* not supported yet */
>>> + return 0;
>>
>> But doesn't this mean to auto-disable all attached events during NMI
>> as returning 0 will prevent the event going to ring buffer?
>
> well, it means that if tracepoint fired during nmi the program
> won't be called and event won't be sent to trace buffer.
> The program might be broken (like divide by zero) and
> it will self-terminate with 'return 0'
> so zero should be the safest return value that
> causes minimum disturbance to the whole system overall.
I'm okay for not calling bpf program in NMI but not for disabling events.
Suppose an user was collecting an event (including in NMI) and then
[s]he also wanted to run a bpf program. So [s]he wrote a program
always return 1. But after attaching the program, it didn't record
the event in NMI.. Isn't that a problem?
>
>> I think it'd be better to keep an attached event in a soft-disabled
>> state like event trigger and give control of enabling to users..
>
> I think it suffers from the same Ctrl-C issue.
> Say, attaching bpf program activates tracepoint and keeps
> it in soft-disabled. Then user space clears soft-disabled.
> Then user Ctrl-C it. Now bpf program must auto-detach
> and unload, since prog_fd is closing.
> If we don't completely deactivate tracepoint, then
> Ctrl-C will leave the state of the system in the state
> different from it was before user process started running.
> I think we must avoid such situation.
> 'kill pid' should be completely cleaning all resources
> that user process was using.
> Yes. It's different from typical usage of /sys/.../tracing
> that has all global knobs, but, imo, it's cleaner this way.
Right. I think bpf programs belong to a user process but events are
global resource. Maybe you also need to consider attaching bpf
program via perf (ioctl?) interface..
Thanks,
Namhyung
^ permalink raw reply
* Re: [PATCH v2 linux-trace 1/8] tracing: attach eBPF programs to tracepoints and syscalls
From: Alexei Starovoitov @ 2015-01-29 6:39 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Namhyung Kim, Steven Rostedt, Ingo Molnar,
Arnaldo Carvalho de Melo, Jiri Olsa, Linux API,
Network Development, LKML
On Wed, Jan 28, 2015 at 9:39 PM, Masami Hiramatsu
<masami.hiramatsu.pt@hitachi.com> wrote:
>
> Maybe, would we need a reference counter for each event? :)
when we would want multiple users to attach different programs
to the same event, then yes.
Right now I'd rather have things simple.
> Actually, ftrace event is not similar to perf-event which ktap
> is based on, ftrace event interface is always exported via
> debugfs, this means users can share the event for different
> usage.
yes.I've been thinking to extend perf_event ioctl to attach programs,
but right now it's only supporting tracepoints and kprobe
seems not trivial to add.
So I went for tracefs style of attaching for now.
> One possible other solution is to add a instance-lock interface
> for each ftrace instance and lock it by bpf. Then, other users
> can not enable/disable the events in the instance.
the user space can synchronize itself via flock. kernel doesn't
need to arbitrate. If one user process attached a program
that auto-enabled an event and another process did
'echo 0 > enable', it's fine. I think it's a feature instead of a bug.
Both users are root anyway.
The more we talk about it, the more I like a new 'bpf' file
approach within tracefs (that I've mentioned in cover letter)
with auto-enable/disable logic to make it clear that
it's different from traditional global 'filter' file.
^ permalink raw reply
* Re: [PATCH] net:wan:Change location of debug printk statement in the function,hss_hdlc_poll
From: Krzysztof Hałasa @ 2015-01-29 6:35 UTC (permalink / raw)
To: Nicholas Krause; +Cc: netdev, linux-kernel
In-Reply-To: <1422143181-4479-1-git-send-email-xerofoify@gmail.com>
Nicholas Krause <xerofoify@gmail.com> writes:
> This changes the location of the printk for montioring if the stucture pointer
> desc of type structure desc has a error count due to failing in the switch
> statement for checking it's status from the default switch case in this switch
> to the area of the function for checking if there is a error count. Due to this
> this part of the function can now be uncommented as it's now needed for printing
> out important information if the structure pointer,desc has a error count for
> debugging purposes.
This error count seems to be secondary to the desc->status. It's isn't
normally logged (RX errors on serial links are quite normal) and only
the "default" case, normally not seen, would be logged (to indicate
unhandled condition). In the "normal" error path, the driver ignores
error count, because there could be several errors counted for a single
erroneous frame (or non-frame).
Also the patch seems to remove the beginning of a preprocessor directive
(#if) without removing its end (#endif). I'd be surprised if it compiles.
> --- a/drivers/net/wan/ixp4xx_hss.c
> +++ b/drivers/net/wan/ixp4xx_hss.c
> @@ -697,7 +697,6 @@ static int hss_hdlc_poll(struct napi_struct *napi, int budget)
> }
>
> desc = rx_desc_ptr(port, n);
> -#if 0 /* FIXME - error_count counts modulo 256, perhaps we should use it */
> if (desc->error_count)
> printk(KERN_DEBUG "%s: hss_hdlc_poll status 0x%02X"
> " errors %u\n", dev->name, desc->status,
> @@ -735,9 +734,7 @@ static int hss_hdlc_poll(struct napi_struct *napi, int budget)
> dev->stats.rx_length_errors++;
> dev->stats.rx_errors++;
> break;
> - default: /* FIXME - remove printk */
> - netdev_err(dev, "hss_hdlc_poll: status 0x%02X errors %u\n",
> - desc->status, desc->error_count);
> + default:
> dev->stats.rx_errors++;
> }
--
Krzysztof Halasa
Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland
^ permalink raw reply
* Re: [PATCH v2] lib/checksum.c: fix carry in csum_tcpudp_nofold
From: David Miller @ 2015-01-29 6:32 UTC (permalink / raw)
To: karl.beldan
Cc: karl.beldan, viro, eric.dumazet, arnd, vapier, netdev,
linux-kernel, stable
In-Reply-To: <1422439091-31413-1-git-send-email-karl.beldan@rivierawaves.com>
From: Karl Beldan <karl.beldan@gmail.com>
Date: Wed, 28 Jan 2015 10:58:11 +0100
> The carry from the 64->32bits folding was dropped, e.g with:
> saddr=0xFFFFFFFF daddr=0xFF0000FF len=0xFFFF proto=0 sum=1,
> csum_tcpudp_nofold returned 0 instead of 1.
>
> Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
Applied, thanks.
^ permalink raw reply
* LOL
From: Paul @ 2015-01-28 21:12 UTC (permalink / raw)
To: netdev
Hahahaha! Is that you on that pic???? :)
http://tinyurl.com/nv3udu4
No more such info? Simply answer <NO>
^ permalink raw reply
* Re: [PATCH net-next v2 0/5] fix stretch ACK bugs in TCP CUBIC and Reno
From: David Miller @ 2015-01-29 6:22 UTC (permalink / raw)
To: ncardwell; +Cc: netdev
In-Reply-To: <1422493299-22114-1-git-send-email-ncardwell@google.com>
From: Neal Cardwell <ncardwell@google.com>
Date: Wed, 28 Jan 2015 20:01:34 -0500
> This patch series fixes the TCP CUBIC and Reno congestion control
> modules to properly handle stretch ACKs in their respective additive
> increase modes, and in the transitions from slow start to additive
> increase.
>
> This finishes the project started by commit 9f9843a751d0a2057 ("tcp:
> properly handle stretch acks in slow start"), which fixed behavior for
> TCP congestion control when handling stretch ACKs in slow start mode.
>
> Motivation: In the Jan 2015 netdev thread 'BW regression after "tcp:
> refine TSO autosizing"', Eyal Perry documented a regression that Eric
> Dumazet determined was caused by improper handling of TCP stretch
> ACKs.
>
> Background: LRO, GRO, delayed ACKs, and middleboxes can cause "stretch
> ACKs" that cover more than the RFC-specified maximum of 2
> packets. These stretch ACKs can cause serious performance shortfalls
> in common congestion control algorithms, like Reno and CUBIC, which
> were designed and tuned years ago with receiver hosts that were not
> using LRO or GRO, and were instead ACKing every other packet.
>
> Testing: at Google we have been using this approach for handling
> stretch ACKs for CUBIC datacenter and Internet traffic for several
> years, with good results.
>
> v2:
> * fixed return type of tcp_slow_start() to be u32 instead of int
Series applied, thanks Neal.
^ permalink raw reply
* Re: [PATCH v2 linux-trace 1/8] tracing: attach eBPF programs to tracepoints and syscalls
From: Masami Hiramatsu @ 2015-01-29 5:39 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Namhyung Kim, Steven Rostedt, Ingo Molnar,
Arnaldo Carvalho de Melo, Jiri Olsa, Linux API,
Network Development, LKML
In-Reply-To: <CAMEtUuz+N52ms4ZFh8+fSGhg1UaXpBdk9RVPU_zBDCjaZ4bj4A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
(2015/01/29 13:40), Alexei Starovoitov wrote:
> On Wed, Jan 28, 2015 at 4:46 PM, Namhyung Kim <namhyung-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>>
>>> +static int event_filter_release(struct inode *inode, struct file *filp)
>>> +{
>>> + struct ftrace_event_file *file;
>>> + char buf[2] = "0";
>>> +
>>> + mutex_lock(&event_mutex);
>>> + file = event_file_data(filp);
>>> + if (file) {
>>> + if (file->flags & TRACE_EVENT_FL_BPF) {
>>> + /* auto-disable the filter */
>>> + ftrace_event_enable_disable(file, 0);
>>
>> Hmm.. what if user already enabled an event, attached a bpf filter and
>> then detached the filter - I'm not sure we can always auto-disable
>> it..
>
> why not?
> I think it makes sense auto enable/disable, since that
> is cleaner user experience.
Maybe, would we need a reference counter for each event? :)
Actually, ftrace event is not similar to perf-event which ktap
is based on, ftrace event interface is always exported via
debugfs, this means users can share the event for different
usage.
One possible other solution is to add a instance-lock interface
for each ftrace instance and lock it by bpf. Then, other users
can not enable/disable the events in the instance.
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org
^ permalink raw reply
* Re: [PATCH net v2] net: ipv6: allow explicitly choosing optimistic addresses
From: Lorenzo Colitti @ 2015-01-29 5:25 UTC (permalink / raw)
To: Erik Kline; +Cc: netdev@vger.kernel.org, Hannes Frederic Sowa
In-Reply-To: <1422446619-9502-1-git-send-email-ek@google.com>
On Wed, Jan 28, 2015 at 9:03 PM, Erik Kline <ek@google.com> wrote:
> if (ipv6_addr_equal(&ifp->addr, addr) &&
> - !(ifp->flags&IFA_F_TENTATIVE) &&
> + (!(ifp->flags&banned_flags) ||
> + ifp->flags&IFA_F_OPTIMISTIC&~banned_flags) &&
Is this if statement correct?
The intent here is "if ifp has IFA_F_OPTIMISTIC set, then
IFA_F_TENTATIVE is allowed, even if the caller explicitly banned
IFA_F_TENTATIVE", right? Not "if ifp has IFA_F_OPTIMISTIC set, then
any flags are allowed, even ones explicitly baned by the caller". For
example, suppose that:
banned_flags = IFA_F_SECONDARY
ifp->flags = IFA_F_SECONDARY | IFA_F_OPTIMISTIC
In that case, won't the if statement match ifp, even though it
contains a flag that is explicitly banned?
^ permalink raw reply
* RE: cxgb4: CONFIG_CXGB4_DCB?
From: Anish Bhatt @ 2015-01-29 5:16 UTC (permalink / raw)
To: Paul Bolle, Hariprasad S
Cc: Valentin Rothberg, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1422480916.3621.53.camel@x220>
It got to the maintainer nevertheless. Thanks for bringing this to our notice
Paul, fix for this is already in the patchwork queue :
https://patchwork.ozlabs.org/patch/434279/
-Anish
^ permalink raw reply
* Re: [PATCH v2 linux-trace 1/8] tracing: attach eBPF programs to tracepoints and syscalls
From: Alexei Starovoitov @ 2015-01-29 4:40 UTC (permalink / raw)
To: Namhyung Kim
Cc: Steven Rostedt, Ingo Molnar, Arnaldo Carvalho de Melo, Jiri Olsa,
Masami Hiramatsu, Linux API, Network Development, LKML
On Wed, Jan 28, 2015 at 4:46 PM, Namhyung Kim <namhyung@kernel.org> wrote:
>>
>> +static int event_filter_release(struct inode *inode, struct file *filp)
>> +{
>> + struct ftrace_event_file *file;
>> + char buf[2] = "0";
>> +
>> + mutex_lock(&event_mutex);
>> + file = event_file_data(filp);
>> + if (file) {
>> + if (file->flags & TRACE_EVENT_FL_BPF) {
>> + /* auto-disable the filter */
>> + ftrace_event_enable_disable(file, 0);
>
> Hmm.. what if user already enabled an event, attached a bpf filter and
> then detached the filter - I'm not sure we can always auto-disable
> it..
why not?
I think it makes sense auto enable/disable, since that
is cleaner user experience.
Otherwise Ctrl-C of the user process will have bpf program dangling.
not good. If we auto-unload bpf program only, it's equally bad.
Since Ctrl-C of the process will auto-onload only
and will keep tracepoint enabled which will be spamming
the trace buffer.
>> +unsigned int trace_filter_call_bpf(struct event_filter *filter, void *ctx)
>> +{
>> + unsigned int ret;
>> +
>> + if (in_nmi()) /* not supported yet */
>> + return 0;
>
> But doesn't this mean to auto-disable all attached events during NMI
> as returning 0 will prevent the event going to ring buffer?
well, it means that if tracepoint fired during nmi the program
won't be called and event won't be sent to trace buffer.
The program might be broken (like divide by zero) and
it will self-terminate with 'return 0'
so zero should be the safest return value that
causes minimum disturbance to the whole system overall.
> I think it'd be better to keep an attached event in a soft-disabled
> state like event trigger and give control of enabling to users..
I think it suffers from the same Ctrl-C issue.
Say, attaching bpf program activates tracepoint and keeps
it in soft-disabled. Then user space clears soft-disabled.
Then user Ctrl-C it. Now bpf program must auto-detach
and unload, since prog_fd is closing.
If we don't completely deactivate tracepoint, then
Ctrl-C will leave the state of the system in the state
different from it was before user process started running.
I think we must avoid such situation.
'kill pid' should be completely cleaning all resources
that user process was using.
Yes. It's different from typical usage of /sys/.../tracing
that has all global knobs, but, imo, it's cleaner this way.
^ permalink raw reply
* Re: [PATCH v2 linux-trace 4/8] samples: bpf: simple tracing example in C
From: Alexei Starovoitov @ 2015-01-29 4:19 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Steven Rostedt, Ingo Molnar, Namhyung Kim, Jiri Olsa,
Masami Hiramatsu, Linux API, Network Development, LKML
On Wed, Jan 28, 2015 at 12:44 PM, Arnaldo Carvalho de Melo
<acme-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
>> > #define memcmp(s1, s2, n) bpf_memcmp(s1, s2, n)
>
>> >> in bpf_helpers.h to have it work?
>
>> yes, that will work just fine.
>> Since it's an example I made it explicit that bpf_memcmp()
>> has memcmp() semantics, but little bit different:
>> int bpf_memcmp(void *unsafe_ptr, void *safe_ptr, int size)
>
> Not knowing about the safe/unsafe pointers (at this point in my
> conceptual eBPF learning process), I would think that it would be easier
> to understand if it would reuse another well known idiom:
>
> #define memcmp_from_user(kernel, user, n) bpf_memcmp(user, kernel, n)
that actually won't be right. It's not reading from user.
'unsafe' means that pointer is not known to be safe
within the program. So far, bpf verifier recognizes pointer
to context, stack and maps. Everything else is unsafe.
Therefore all pointer walking of unknown data structures
need to use bpf_fetch_*() helpers to access memory.
Similarly bpf_memcmp() is used to do memcmp() of
two memory regions: one known to be safe and one
unknown.
> I don't reuse anything I've learned before trying to understand eBPF,
> not I see any well known marker (__user) that would help me understand
> that that pointer needs special treatment/belongs to a different "domain".
I tried to do exactly that actually :)
bpf_fetch_*() helpers suppose to match existing fetch*()
primitives used to do the same pointer walking for kprobes.
Maybe bpf_probe_memcmp() would be a better name?
Hard to tell.
>> bpf_fetch_*() helpers are also explicit in examples.
>> If one need to do a lot of pointer walking, then macro like
>> #define D(P) ((typeof(P))bpf_fetch_ptr(&P))
>> would be easier to use: p = D(D(skb->dev)->ifalias)
>> multiple pointer derefs would look more natural...
>
> And if possible, i.e. if the eBPF compiler would take care of that
> somehow, would indeed be preferred as it looks more natural :-)
yes, on the user space side we have all the freedom.
Note, these examples are using user's bpf_helpers.h
which defines it as:
static int (*bpf_memcmp)(void *unsafe_ptr, void *safe_ptr, int size) =
(void *) BPF_FUNC_memcmp;
so whether we want to call the function bpf_memcmp() or
bpf_probe_memcmp() is up to user space.
The BPF_FUNC_memcmp name is what matters which
is defined in uapi/linux/bpf.h
I'll try to come up with more explicit name...
^ 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