* Re: [PATCH 0/5] m68k: IO Fixes and Cleanups
From: Greg Ungerer @ 2018-07-03 2:15 UTC (permalink / raw)
To: Geert Uytterhoeven, David S . Miller, Dmitry Torokhov,
Helge Deller
Cc: linux-m68k, netdev, linux-input, linux-kernel
In-Reply-To: <20180702133532.5412-1-geert@linux-m68k.org>
Hi Geert,
On 02/07/18 23:35, Geert Uytterhoeven wrote:
> Hi all,
>
> This patch series contains fixes and cleanups for I/O accessors on m68k
> platforms (with MMU).
>
> The first patch contains small fixes without any dependencies.
> Patches 2 and 3 make small adjustments to drivers that are dependencies
> for further cleanup.
> Patch 4 and 5 complete the cleanup.
>
> Given the dependencies, I think it's easiest if the respective
> maintainers would provide their Acked-by, so all patches can go in
> through the m68k tree.
>
> Thanks for your comments!
I like it alot. If we can just fix up the warnings caused by patch
number 1 I am more than happy to ack.
Regards
Greg
> Geert Uytterhoeven (5):
> m68k/io: Add missing ioremap define guards, fix typo
> net: mac8390: Use standard memcpy_{from,to}io()
> Input: hilkbd - Add casts to HP9000/300 I/O accessors
> m68k/io: Move mem*io define guards to <asm/kmap.h>
> m68k/io: Switch mmu variant to <asm-generic/io.h>
>
> arch/m68k/include/asm/io.h | 7 +++++
> arch/m68k/include/asm/io_mm.h | 40 +++--------------------------
> arch/m68k/include/asm/io_no.h | 12 ---------
> arch/m68k/include/asm/kmap.h | 7 ++++-
> drivers/input/keyboard/hilkbd.c | 4 +--
> drivers/net/ethernet/8390/mac8390.c | 20 +++++++--------
> 6 files changed, 28 insertions(+), 62 deletions(-)
>
^ permalink raw reply
* Re: [PATCH 1/5] m68k/io: Add missing ioremap define guards, fix typo
From: Greg Ungerer @ 2018-07-03 2:13 UTC (permalink / raw)
To: Geert Uytterhoeven, David S . Miller, Dmitry Torokhov,
Helge Deller
Cc: linux-m68k, netdev, linux-input, linux-kernel
In-Reply-To: <20180702133532.5412-2-geert@linux-m68k.org>
Hi Geert,
On 02/07/18 23:35, Geert Uytterhoeven wrote:
> - Add missing define guard for ioremap_wt(),
> - Fix typo s/ioremap_fillcache/ioremap_fullcache/,
> - Add define guard for iounmap() for consistency with other
> architectures.
>
> Fixes: 9746882f547d2f00 ("m68k: group io mapping definitions and functions")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
If I build for the m5475evb defconfig then I get warnings like this:
CC init/main.o
In file included from ./include/asm-generic/io.h:19:0,
from ./arch/m68k/include/asm/io_no.h:147,
from ./arch/m68k/include/asm/io.h:3,
from ./include/linux/bio.h:28,
from ./include/linux/blkdev.h:21,
from init/main.c:80:
./include/asm-generic/iomap.h:79:0: warning: "ioremap_wt" redefined
#define ioremap_wt ioremap_nocache
^
In file included from ./arch/m68k/include/asm/io_no.h:145:0,
from ./arch/m68k/include/asm/io.h:3,
from ./include/linux/bio.h:28,
from ./include/linux/blkdev.h:21,
from init/main.c:80:
./arch/m68k/include/asm/kmap.h:37:0: note: this is the location of the previous definition
#define ioremap_wt ioremap_wt
^
Regards
Greg
> ---
> arch/m68k/include/asm/kmap.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/m68k/include/asm/kmap.h b/arch/m68k/include/asm/kmap.h
> index 84b8333db8ad1987..bf1026def698f21f 100644
> --- a/arch/m68k/include/asm/kmap.h
> +++ b/arch/m68k/include/asm/kmap.h
> @@ -16,6 +16,7 @@
> */
> extern void __iomem *__ioremap(unsigned long physaddr, unsigned long size,
> int cacheflag);
> +#define iounmap iounmap
> extern void iounmap(void __iomem *addr);
> extern void __iounmap(void *addr, unsigned long size);
>
> @@ -33,13 +34,14 @@ static inline void __iomem *ioremap_nocache(unsigned long physaddr,
> }
>
> #define ioremap_uc ioremap_nocache
> +#define ioremap_wt ioremap_wt
> static inline void __iomem *ioremap_wt(unsigned long physaddr,
> unsigned long size)
> {
> return __ioremap(physaddr, size, IOMAP_WRITETHROUGH);
> }
>
> -#define ioremap_fillcache ioremap_fullcache
> +#define ioremap_fullcache ioremap_fullcache
> static inline void __iomem *ioremap_fullcache(unsigned long physaddr,
> unsigned long size)
> {
>
^ permalink raw reply
* Re: [PATCH net-next v4 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
From: Jason Wang @ 2018-07-03 2:12 UTC (permalink / raw)
To: xiangxia.m.yue
Cc: mst, makita.toshiaki, virtualization, netdev, Tonghao Zhang
In-Reply-To: <1530536228-17462-4-git-send-email-xiangxia.m.yue@gmail.com>
On 2018年07月02日 20:57, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> Factor out generic busy polling logic and will be
> used for in tx path in the next patch. And with the patch,
> qemu can set differently the busyloop_timeout for rx queue.
>
> Signed-off-by: Tonghao Zhang <zhangtonghao@didichuxing.com>
> ---
> drivers/vhost/net.c | 94 +++++++++++++++++++++++++++++++----------------------
> 1 file changed, 55 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 62bb8e8..2790959 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -429,6 +429,52 @@ static int vhost_net_enable_vq(struct vhost_net *n,
> return vhost_poll_start(poll, sock->file);
> }
>
> +static int sk_has_rx_data(struct sock *sk)
> +{
> + struct socket *sock = sk->sk_socket;
> +
> + if (sock->ops->peek_len)
> + return sock->ops->peek_len(sock);
> +
> + return skb_queue_empty(&sk->sk_receive_queue);
> +}
> +
> +static void vhost_net_busy_poll(struct vhost_net *net,
> + struct vhost_virtqueue *rvq,
> + struct vhost_virtqueue *tvq,
> + bool rx)
> +{
> + unsigned long uninitialized_var(endtime);
> + unsigned long busyloop_timeout;
> + struct socket *sock;
> + struct vhost_virtqueue *vq = rx ? tvq : rvq;
> +
> + mutex_lock_nested(&vq->mutex, rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX);
> +
> + vhost_disable_notify(&net->dev, vq);
> + sock = rvq->private_data;
> + busyloop_timeout = rx ? rvq->busyloop_timeout : tvq->busyloop_timeout;
> +
> + preempt_disable();
> + endtime = busy_clock() + busyloop_timeout;
> + while (vhost_can_busy_poll(tvq->dev, endtime) &&
> + !(sock && sk_has_rx_data(sock->sk)) &&
> + vhost_vq_avail_empty(tvq->dev, tvq))
> + cpu_relax();
> + preempt_enable();
> +
> + if ((rx && !vhost_vq_avail_empty(&net->dev, vq)) ||
> + (!rx && (sock && sk_has_rx_data(sock->sk)))) {
> + vhost_poll_queue(&vq->poll);
> + } else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
One last question, do we need this for rx? This check will be always
true under light or medium load.
Thanks
> + vhost_disable_notify(&net->dev, vq);
> + vhost_poll_queue(&vq->poll);
> + }
> +
> + mutex_unlock(&vq->mutex);
> +}
> +
> +
> static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
> struct vhost_virtqueue *vq,
> struct iovec iov[], unsigned int iov_size,
> @@ -621,16 +667,6 @@ static int peek_head_len(struct vhost_net_virtqueue *rvq, struct sock *sk)
> return len;
> }
>
> -static int sk_has_rx_data(struct sock *sk)
> -{
> - struct socket *sock = sk->sk_socket;
> -
> - if (sock->ops->peek_len)
> - return sock->ops->peek_len(sock);
> -
> - return skb_queue_empty(&sk->sk_receive_queue);
> -}
> -
> static void vhost_rx_signal_used(struct vhost_net_virtqueue *nvq)
> {
> struct vhost_virtqueue *vq = &nvq->vq;
> @@ -645,39 +681,19 @@ static void vhost_rx_signal_used(struct vhost_net_virtqueue *nvq)
>
> static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk)
> {
> - struct vhost_net_virtqueue *rvq = &net->vqs[VHOST_NET_VQ_RX];
> - struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
> - struct vhost_virtqueue *vq = &nvq->vq;
> - unsigned long uninitialized_var(endtime);
> - int len = peek_head_len(rvq, sk);
> + struct vhost_net_virtqueue *rnvq = &net->vqs[VHOST_NET_VQ_RX];
> + struct vhost_net_virtqueue *tnvq = &net->vqs[VHOST_NET_VQ_TX];
>
> - if (!len && vq->busyloop_timeout) {
> - /* Flush batched heads first */
> - vhost_rx_signal_used(rvq);
> - /* Both tx vq and rx socket were polled here */
> - mutex_lock_nested(&vq->mutex, VHOST_NET_VQ_TX);
> - vhost_disable_notify(&net->dev, vq);
> + int len = peek_head_len(rnvq, sk);
>
> - preempt_disable();
> - endtime = busy_clock() + vq->busyloop_timeout;
> -
> - while (vhost_can_busy_poll(&net->dev, endtime) &&
> - !sk_has_rx_data(sk) &&
> - vhost_vq_avail_empty(&net->dev, vq))
> - cpu_relax();
> -
> - preempt_enable();
> -
> - if (!vhost_vq_avail_empty(&net->dev, vq))
> - vhost_poll_queue(&vq->poll);
> - else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> - vhost_disable_notify(&net->dev, vq);
> - vhost_poll_queue(&vq->poll);
> - }
> + if (!len && rnvq->vq.busyloop_timeout) {
> + /* Flush batched heads first */
> + vhost_rx_signal_used(rnvq);
>
> - mutex_unlock(&vq->mutex);
> + /* Both tx vq and rx socket were polled here */
> + vhost_net_busy_poll(net, &rnvq->vq, &tnvq->vq, true);
>
> - len = peek_head_len(rvq, sk);
> + len = peek_head_len(rnvq, sk);
> }
>
> return len;
^ permalink raw reply
* Re: [PATCHv2 net-next 0/5] sctp: fully support for dscp and flowlabel per transport
From: Marcelo Ricardo Leitner @ 2018-07-03 1:54 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem, hideaki.yoshifuji
In-Reply-To: <cover.1530526661.git.lucien.xin@gmail.com>
On Mon, Jul 02, 2018 at 06:21:10PM +0800, Xin Long wrote:
> Now dscp and flowlabel are set from sock when sending the packets,
> but being multi-homing, sctp also supports for dscp and flowlabel
> per transport, which is described in section 8.1.12 in RFC6458.
>
> v1->v2:
> - define ip_queue_xmit as inline in net/ip.h, instead of exporting
> it in Patch 1/5 according to David's suggestion.
> - fix the param len check in sctp_s/getsockopt_peer_addr_params()
> in Patch 3/5 to guarantee that an old app built with old kernel
> headers could work on the newer kernel per Marcelo's point.
>
> Xin Long (5):
> ipv4: add __ip_queue_xmit() that supports tos param
> sctp: add support for dscp and flowlabel per transport
> sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams
> sctp: add support for setting flowlabel when adding a transport
> sctp: check for ipv6_pinfo legal sndflow with flowlabel in
> sctp_v6_get_dst
Series
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
^ permalink raw reply
* Re: [PATCH v2] atmel: using strlcpy() to avoid possible buffer overflows
From: YueHaibing @ 2018-07-03 1:48 UTC (permalink / raw)
To: Kalle Valo
Cc: simon-xn1N/tgparsycpQjotevgVpr/1R2p/CL,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q,
andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <8736x1snu9.fsf-HodKDYzPHsUD5k0oWYwrnHL1okKdlPRT@public.gmane.org>
On 2018/7/2 22:40, Kalle Valo wrote:
> YueHaibing <yuehaibing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> writes:
>
>> 'firmware' is a module param which may been longer than firmware_id,
>> so using strlcpy() to guard against overflows. Also priv is allocated
>> with zeroed memory,no need to set firmware_id[0] to '\0'.
>>
>> v1 -> v2: remove priv->firmware_id[0] = '\0';
>>
>> Signed-off-by: YueHaibing <yuehaibing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>> drivers/net/wireless/atmel/atmel.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> The changelog should be after "---" line. I can fix it this time, but in
> the future please use the correct location.
Thank you,will notice it.
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#changelog_missing
>
^ permalink raw reply
* Re: [PATCH net] sctp: fix the issue that pathmtu may be set lower than MINSEGMENT
From: Marcelo Ricardo Leitner @ 2018-07-03 1:43 UTC (permalink / raw)
To: Neil Horman; +Cc: Xin Long, network dev, linux-sctp, davem, syzkaller
In-Reply-To: <20180702114512.GA15612@hmswarspite.think-freely.org>
On Mon, Jul 02, 2018 at 07:45:12AM -0400, Neil Horman wrote:
> On Mon, Jul 02, 2018 at 02:51:16PM +0800, Xin Long wrote:
> > After commit b6c5734db070 ("sctp: fix the handling of ICMP Frag Needed
> > for too small MTUs"), sctp_transport_update_pmtu would refetch pathmtu
> > from the dst and set it to transport's pathmtu without any check.
> >
> > The new pathmtu may be lower than MINSEGMENT if the dst is obsolete and
> > updated by .get_dst() in sctp_transport_update_pmtu.
In this case, it could have a smaller MTU as well, and thus we should
validate it against MINSEGMENT instead.
> >
> > Syzbot reported a warning in sctp_mtu_payload caused by this.
> >
> > This fix uses the refetched pathmtu only when it's greater than the
> > frag_needed pmtu.
> >
> > Fixes: b6c5734db070 ("sctp: fix the handling of ICMP Frag Needed for too small MTUs")
> > Reported-by: syzbot+f0d9d7cba052f9344b03@syzkaller.appspotmail.com
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > ---
> > net/sctp/transport.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/sctp/transport.c b/net/sctp/transport.c
> > index 445b7ef..ddfb687 100644
> > --- a/net/sctp/transport.c
> > +++ b/net/sctp/transport.c
> > @@ -282,7 +282,10 @@ bool sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu)
> >
> > if (dst) {
> > /* Re-fetch, as under layers may have a higher minimum size */
> > - pmtu = SCTP_TRUNC4(dst_mtu(dst));
> > + u32 mtu = SCTP_TRUNC4(dst_mtu(dst));
> > +
> > + if (pmtu < mtu)
> > + pmtu = mtu;
> nit, but why not u32 mtu = min(pmtu, SCTP_TRUNC4(dst_mtu(dst))) here ?
sctp_dst_mtu() is wrapping all that for us :)
- pmtu = SCTP_TRUNC4(dst_mtu(dst));
+ pmtu = sctp_dst_mtu(dst);
>
> Neil
>
> > change = t->pathmtu != pmtu;
> > }
> > t->pathmtu = pmtu;
> > --
> > 2.1.0
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH bpf v2 0/4] Bug fixes to the SKB TX path of AF_XDP
From: Alexei Starovoitov @ 2018-07-03 1:39 UTC (permalink / raw)
To: Magnus Karlsson
Cc: bjorn.topel, ast, daniel, netdev, eric.dumazet, liu.song.a23,
qi.z.zhang, pavel
In-Reply-To: <1530258500-9126-1-git-send-email-magnus.karlsson@intel.com>
On Fri, Jun 29, 2018 at 09:48:16AM +0200, Magnus Karlsson wrote:
> This patch set fixes three bugs in the SKB TX path of AF_XDP.
> Details in the individual commits.
>
> The structure of the patch set is as follows:
>
> Patch 1: Fix for lost completion message
> Patch 2-3: Fix for possible multiple completions of single packet
> Patch 4: Fix potential race during error
>
> Changes from v1:
>
> * Added explanation of race in commit message of patch 4.
Applied, Thanks
^ permalink raw reply
* [PATCH net-next] tcp: Improve setsockopt() TCP_USER_TIMEOUT accuracy
From: Jon Maxwell @ 2018-07-03 1:17 UTC (permalink / raw)
To: davem; +Cc: edumazet, kuznet, yoshfuji, netdev, linux-kernel, jmaxwell
Every time the TCP retransmission timer fires. It checks to see if there is a
timeout before scheduling the next retransmit timer. The retransmit interval
between each retransmission increases exponentially. The issue is that in order
for the timeout to occur the retransmit timer needs to fire again. If the user
timeout check happens after the 9th retransmit for example. It needs to wait for
the 10th retransmit timer to fire in order to evaluate whether a timeout has
occurred or not. If the interval is large enough then the timeout will be
inaccurate.
For example with a TCP_USER_TIMEOUT of 10 seconds without patch:
1st retransmit:
22:25:18.973488 IP host1.49310 > host2.search-agent: Flags [.]
Last retransmit:
22:25:26.205499 IP host1.49310 > host2.search-agent: Flags [.]
Timeout:
send: Connection timed out
Sun Jul 1 22:25:34 EDT 2018
We can see that last retransmit took ~7 seconds. Which pushed the total
timeout to ~15 seconds instead of the expected 10 seconds. This gets more
inaccurate the larger the TCP_USER_TIMEOUT value. As the interval increases.
Fix this by recalculating the last retransmit interval so that it fires when
the timeout should occur. Only implement when icsk->icsk_user_timeout is set.
Test results with the patch is the expected 10 second timeout:
1st retransmit:
01:37:59.022555 IP host1.49310 > host2.search-agent: Flags [.]
Last retransmit:
01:38:06.486558 IP host1.49310 > host2.search-agent: Flags [.]
Timeout:
send: Connection timed out
Mon Jul 2 01:38:09 EDT 2018
Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
---
net/ipv4/tcp_timer.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 3b3611729928..94491a481722 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -407,6 +407,7 @@ void tcp_retransmit_timer(struct sock *sk)
struct tcp_sock *tp = tcp_sk(sk);
struct net *net = sock_net(sk);
struct inet_connection_sock *icsk = inet_csk(sk);
+ __u32 time_remaining = 0;
if (tp->fastopen_rsk) {
WARN_ON_ONCE(sk->sk_state != TCP_SYN_RECV &&
@@ -535,6 +536,12 @@ void tcp_retransmit_timer(struct sock *sk)
/* Use normal (exponential) backoff */
icsk->icsk_rto = min(icsk->icsk_rto << 1, TCP_RTO_MAX);
}
+ if (icsk->icsk_user_timeout) {
+ time_remaining = jiffies_to_msecs(icsk->icsk_user_timeout) -
+ (tcp_time_stamp(tcp_sk(sk)) - tcp_sk(sk)->retrans_stamp);
+ if (time_remaining < icsk->icsk_rto)
+ icsk->icsk_rto = time_remaining;
+ }
inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, icsk->icsk_rto, TCP_RTO_MAX);
if (retransmits_timed_out(sk, net->ipv4.sysctl_tcp_retries1 + 1, 0))
__sk_dst_reset(sk);
--
2.13.6
^ permalink raw reply related
* Re: [PATCH net-next v2 0/2] tcp: fix high tail latencies in DCTCP
From: Neal Cardwell @ 2018-07-03 0:51 UTC (permalink / raw)
To: Lawrence Brakmo
Cc: Netdev, Kernel Team, bmatheny, ast, Yuchung Cheng, Steve Ibanez,
Eric Dumazet
In-Reply-To: <20180702213908.1246455-1-brakmo@fb.com>
On Mon, Jul 2, 2018 at 5:39 PM Lawrence Brakmo <brakmo@fb.com> wrote:
>
> When have observed high tail latencies when using DCTCP for RPCs as
> compared to using Cubic. For example, in one setup there are 2 hosts
> sending to a 3rd one, with each sender having 3 flows (1 stream,
> 1 1MB back-to-back RPCs and 1 10KB back-to-back RPCs). The following
> table shows the 99% and 99.9% latencies for both Cubic and dctcp:
>
> Cubic 99% Cubic 99.9% dctcp 99% dctcp 99.9%
> 1MB RPCs 2.6ms 5.5ms 43ms 208ms
> 10KB RPCs 1.1ms 1.3ms 53ms 212ms
>
> Looking at tcpdump traces showed that there are two causes for the
> latency.
>
> 1) RTOs caused by the receiver sending a dup ACK and not ACKing
> the last (and only) packet sent.
> 2) Delaying ACKs when the sender has a cwnd of 1, so everything
> pauses for the duration of the delayed ACK.
>
> The first patch fixes the cause of the dup ACKs, not updating DCTCP
> state when an ACK that was initially delayed has been sent with a
> data packet.
>
> The second patch insures that an ACK is sent immediately when a
> CWR marked packet arrives.
>
> With the patches the latencies for DCTCP now look like:
>
> dctcp 99% dctcp 99.9%
> 1MB RPCs 5.8ms 6.9ms
> 10KB RPCs 146us 203us
>
> Note that while the 1MB RPCs tail latencies are higher than Cubic's,
> the 10KB latencies are much smaller than Cubic's. These patches fix
> issues on the receiver, but tcpdump traces indicate there is an
> opportunity to also fix an issue at the sender that adds about 3ms
> to the tail latencies.
>
> The following trace shows the issue that tiggers an RTO (fixed by these patches):
>
> Host A sends the last packets of the request
> Host B receives them, and the last packet is marked with congestion (CE)
> Host B sends ACKs for packets not marked with congestion
> Host B sends data packet with reply and ACK for packet marked with
> congestion (TCP flag ECE)
> Host A receives ACKs with no ECE flag
> Host A receives data packet with ACK for the last packet of request
> and which has TCP ECE bit set
> Host A sends 1st data packet of the next request with TCP flag CWR
> Host B receives the packet (as seen in tcpdump at B), no CE flag
> Host B sends a dup ACK that also has the TCP ECE flag
> Host A RTO timer fires!
> Host A to send the next packet
> Host A receives an ACK for everything it has sent (i.e. Host B
> did receive 1st packet of request)
> Host A send more packets…
>
> [PATCH net-next v2 1/2] tcp: notify when a delayed ack is sent
> [PATCH net-next v2 2/2] tcp: ack immediately when a cwr packet
>
> net/ipv4/tcp_input.c | 16 +++++++++++-----
> net/ipv4/tcp_output.c | 4 ++--
> 2 files changed, 13 insertions(+), 7 deletions(-)
Thanks, Larry. Just for context, can you please let us know whether
your tests included zero, one, or both of Eric's recent commits
(listed below) that tuned the number of ACKs after ECN events? (Or
maybe the tests were literally using a net-next kernel?) Just wanted
to get a better handle on any possible interactions there.
Thanks!
neal
---
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=522040ea5fdd1c33bbf75e1d7c7c0422b96a94ef
commit 522040ea5fdd1c33bbf75e1d7c7c0422b96a94ef
Author: Eric Dumazet <edumazet@google.com>
Date: Mon May 21 15:08:57 2018 -0700
tcp: do not aggressively quick ack after ECN events
ECN signals currently forces TCP to enter quickack mode for
up to 16 (TCP_MAX_QUICKACKS) following incoming packets.
We believe this is not needed, and only sending one immediate ack
for the current packet should be enough.
This should reduce the extra load noticed in DCTCP environments,
after congestion events.
This is part 2 of our effort to reduce pure ACK packets.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=15ecbe94a45ef88491ca459b26efdd02f91edb6d
commit 15ecbe94a45ef88491ca459b26efdd02f91edb6d
Author: Eric Dumazet <edumazet@google.com>
Date: Wed Jun 27 08:47:21 2018 -0700
tcp: add one more quick ack after after ECN events
Larry Brakmo proposal ( https://patchwork.ozlabs.org/patch/935233/
tcp: force cwnd at least 2 in tcp_cwnd_reduction) made us rethink
about our recent patch removing ~16 quick acks after ECN events.
tcp_enter_quickack_mode(sk, 1) makes sure one immediate ack is sent,
but in the case the sender cwnd was lowered to 1, we do not want
to have a delayed ack for the next packet we will receive.
Fixes: 522040ea5fdd ("tcp: do not aggressively quick ack after ECN events")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Neal Cardwell <ncardwell@google.com>
Cc: Lawrence Brakmo <brakmo@fb.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* linux-next: build failure after merge of the net-next tree
From: Stephen Rothwell @ 2018-07-03 0:47 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux-Next Mailing List, Linux Kernel Mailing List,
Sabrina Dubroca
[-- Attachment #1: Type: text/plain, Size: 2971 bytes --]
Hi all,
After merging the net-next tree, today's linux-next build (x86_64
allmodconfig) failed like this:
net/ipv4/fou.c: In function 'gue_gro_receive':
net/ipv4/fou.c:451:35: error: passing argument 2 of 'skb_gro_flush_final_remcsum' from incompatible pointer type [-Werror=incompatible-pointer-types]
skb_gro_flush_final_remcsum(skb, pp, flush, &grc);
^~
In file included from include/net/inet_sock.h:23:0,
from include/linux/udp.h:20,
from net/ipv4/fou.c:6:
include/linux/netdevice.h:2799:20: note: expected 'struct sk_buff **' but argument is of type 'struct sk_buff *'
static inline void skb_gro_flush_final_remcsum(struct sk_buff *skb,
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/vxlan.c: In function 'vxlan_gro_receive':
drivers/net/vxlan.c:627:35: error: passing argument 2 of 'skb_gro_flush_final_remcsum' from incompatible pointer type [-Werror=incompatible-pointer-types]
skb_gro_flush_final_remcsum(skb, pp, flush, &grc);
^~
In file included from include/net/inet_sock.h:23:0,
from include/linux/udp.h:20,
from drivers/net/vxlan.c:17:
include/linux/netdevice.h:2799:20: note: expected 'struct sk_buff **' but argument is of type 'struct sk_buff *'
static inline void skb_gro_flush_final_remcsum(struct sk_buff *skb,
^~~~~~~~~~~~~~~~~~~~~~~~~~~
Caused by commit
d4546c2509b1 ("net: Convert GRO SKB handling to list_head.")
from the net-next tree interacting with commit
603d4cf8fe09 ("net: fix use-after-free in GRO with ESP")
from Linus' tree.
I took a guess and aplied the following merge fix patch:
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 3 Jul 2018 10:37:05 +1000
Subject: [PATCH] net: update for conversion of GRO SKB handling
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
include/linux/netdevice.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e1d4084c0651..64480a0f2c16 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2797,7 +2797,7 @@ static inline void skb_gro_flush_final(struct sk_buff *skb, struct sk_buff *pp,
NAPI_GRO_CB(skb)->flush |= flush;
}
static inline void skb_gro_flush_final_remcsum(struct sk_buff *skb,
- struct sk_buff **pp,
+ struct sk_buff *pp,
int flush,
struct gro_remcsum *grc)
{
@@ -2813,7 +2813,7 @@ static inline void skb_gro_flush_final(struct sk_buff *skb, struct sk_buff *pp,
NAPI_GRO_CB(skb)->flush |= flush;
}
static inline void skb_gro_flush_final_remcsum(struct sk_buff *skb,
- struct sk_buff **pp,
+ struct sk_buff *pp,
int flush,
struct gro_remcsum *grc)
{
--
2.17.1
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related
* [PATCH ethtool] ethtool: ixgbe dump strings for security registers
From: Shannon Nelson @ 2018-07-03 0:09 UTC (permalink / raw)
To: linville; +Cc: netdev
Add the ixgbe's security configuration registers into
the register dump.
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
ixgbe.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
NOTE: Obviously this should wait until Intel accepts and pushes the related
ixgbe patch and Dave accepts it into net-next, but I figured I may
as well send them out together.
diff --git a/ixgbe.c b/ixgbe.c
index c632137..6779402 100644
--- a/ixgbe.c
+++ b/ixgbe.c
@@ -1265,5 +1265,31 @@ ixgbe_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
regs_buff[1127]);
}
+ if (regs_buff_len > 1139 && mac_type != ixgbe_mac_82598EB) {
+ fprintf(stdout,
+ "0x08800: SECTXCTRL (Security Tx Control) 0x%08X\n",
+ regs_buff[1139]);
+
+ fprintf(stdout,
+ "0x08804: SECTXSTAT (Security Tx Status) 0x%08X\n",
+ regs_buff[1140]);
+
+ fprintf(stdout,
+ "0x08808: SECTXBUFFAF (Security Tx Buffer Almost Full) 0x%08X\n",
+ regs_buff[1141]);
+
+ fprintf(stdout,
+ "0x08800: SECTXMINIFG (Security Tx Buffer Minimum IFG) 0x%08X\n",
+ regs_buff[1142]);
+
+ fprintf(stdout,
+ "0x08800: SECRXCTRL (Security Rx Control) 0x%08X\n",
+ regs_buff[1143]);
+
+ fprintf(stdout,
+ "0x08800: SECRXSTAT (Security Rx Status) 0x%08X\n",
+ regs_buff[1144]);
+ }
+
return 0;
}
--
2.7.4
^ permalink raw reply related
* [PATCH next-queue] ixgbe: add ipsec security registers into ethtool register dump
From: Shannon Nelson @ 2018-07-03 0:09 UTC (permalink / raw)
To: intel-wired-lan, jeffrey.t.kirsher; +Cc: netdev
Add the ixgbe's security configuration registers into
the register dump.
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index bd1ba88..1d68884 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -511,7 +511,7 @@ static void ixgbe_set_msglevel(struct net_device *netdev, u32 data)
static int ixgbe_get_regs_len(struct net_device *netdev)
{
-#define IXGBE_REGS_LEN 1139
+#define IXGBE_REGS_LEN 1145
return IXGBE_REGS_LEN * sizeof(u32);
}
@@ -874,6 +874,14 @@ static void ixgbe_get_regs(struct net_device *netdev,
/* X540 specific DCB registers */
regs_buff[1137] = IXGBE_READ_REG(hw, IXGBE_RTTQCNCR);
regs_buff[1138] = IXGBE_READ_REG(hw, IXGBE_RTTQCNTG);
+
+ /* Security config registers */
+ regs_buff[1139] = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+ regs_buff[1140] = IXGBE_READ_REG(hw, IXGBE_SECTXSTAT);
+ regs_buff[1141] = IXGBE_READ_REG(hw, IXGBE_SECTXBUFFAF);
+ regs_buff[1142] = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
+ regs_buff[1143] = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
+ regs_buff[1144] = IXGBE_READ_REG(hw, IXGBE_SECRXSTAT);
}
static int ixgbe_get_eeprom_len(struct net_device *netdev)
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next v2 1/2] tcp: notify when a delayed ack is sent
From: Yuchung Cheng @ 2018-07-02 23:48 UTC (permalink / raw)
To: Lawrence Brakmo
Cc: netdev, Kernel Team, Blake Matheny, Alexei Starovoitov,
Neal Cardwell, Steve Ibanez, Eric Dumazet
In-Reply-To: <20180702213908.1246455-2-brakmo@fb.com>
On Mon, Jul 2, 2018 at 2:39 PM, Lawrence Brakmo <brakmo@fb.com> wrote:
>
> DCTCP depends on the CA_EVENT_NON_DELAYED_ACK and CA_EVENT_DELAYED_ACK
> notifications to keep track if it needs to send an ACK for packets that
> were received with a particular ECN state but whose ACK was delayed.
>
> Under some circumstances, for example when a delayed ACK is sent with a
> data packet, DCTCP state was not being updated due to a lack of
> notification that the previously delayed ACK was sent. As a result, it
> would sometimes send a duplicate ACK when a new data packet arrived.
>
> This patch insures that DCTCP's state is correctly updated so it will
> not send the duplicate ACK.
Sorry to chime-in late here (lame excuse: IETF deadline)
IIRC this issue would exist prior to 4.11 kernel. While it'd be good
to fix that, it's not clear which patch introduced the regression
between 4.11 and 4.16? I assume you tested Eric's most recent quickack
fix.
In terms of the fix itself, it seems odd the tcp_send_ack() call in
DCTCP generates NON_DELAYED_ACK event to toggle DCTCP's
delayed_ack_reserved bit. Shouldn't the fix to have DCTCP send the
"prior" ACK w/o cancelling delayed-ACK and mis-recording that it's
cancelled, because that prior-ACK really is a supplementary old ACK.
But it's still unclear how this bug introduces the regression 4.11 - 4.16
>
> Improved based on comments from Neal Cardwell <ncardwell@google.com>.
>
> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
> ---
> net/ipv4/tcp_output.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index f8f6129160dd..acefb64e8280 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -172,6 +172,8 @@ static inline void tcp_event_ack_sent(struct sock *sk, unsigned int pkts)
> __sock_put(sk);
> }
> tcp_dec_quickack_mode(sk, pkts);
> + if (inet_csk_ack_scheduled(sk))
> + tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK);
> inet_csk_clear_xmit_timer(sk, ICSK_TIME_DACK);
> }
>
> @@ -3567,8 +3569,6 @@ void tcp_send_ack(struct sock *sk)
> if (sk->sk_state == TCP_CLOSE)
> return;
>
> - tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK);
> -
> /* We are not putting this on the write queue, so
> * tcp_transmit_skb() will set the ownership to this
> * sock.
> --
> 2.17.1
>
^ permalink raw reply
* RE: [net] fq_codel: fix NULL pointer deref in fq_codel_reset
From: Keller, Jacob E @ 2018-07-02 23:27 UTC (permalink / raw)
To: Cong Wang
Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
nhorman@redhat.com, sassmann@redhat.com, jogreene@redhat.com,
Eric Dumazet
In-Reply-To: <CAM_iQpXyR2Q-O1EfbGaCFfsQUAw=g_AUq8E6evzM9jVXQuWPXg@mail.gmail.com>
> -----Original Message-----
> From: Cong Wang [mailto:xiyou.wangcong@gmail.com]
> Sent: Monday, June 11, 2018 1:23 PM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; davem@davemloft.net;
> netdev@vger.kernel.org; nhorman@redhat.com; sassmann@redhat.com;
> jogreene@redhat.com; Eric Dumazet <edumazet@google.com>
> Subject: Re: [net] fq_codel: fix NULL pointer deref in fq_codel_reset
>
> On Mon, Jun 11, 2018 at 12:57 PM, Keller, Jacob E
> <jacob.e.keller@intel.com> wrote:
> >
> > I'm open to alternative suggestinos for fixing this, I think Eric suggested that
> maybe we should just remove the ->reset() call from qdisc_destroy..?
>
> You can't remove ->reset() for non-failure call path.
>
> For failure path, yeah, but it is much simpler to just make
> q->flows_cnt be 0 for this specific case.
Alright. I'll rework the patch to set flows_cnt to 0 when init fails.
Thanks,
Jake
^ permalink raw reply
* [PATCH v4,net-next] vlan: implement vlan id and protocol changes
From: Chas Williams @ 2018-07-02 22:35 UTC (permalink / raw)
To: davem; +Cc: netdev, Chas Williams
In-Reply-To: <20180610231912.1543-1-3chas3@gmail.com>
vlan_changelink silently ignores attempts to change the vlan id
or protocol id of an existing vlan interface. Implement by adding
the new vlan id and protocol to the interface's vlan group and then
removing the old vlan id and protocol from the vlan group. This
avoids the netlink churn of deleting and re-adding an interface
to change the vlan id.
Signed-off-by: Chas Williams <3chas3@gmail.com>
---
include/linux/netdevice.h | 1 +
net/8021q/vlan.c | 4 ++--
net/8021q/vlan.h | 2 ++
net/8021q/vlan_netlink.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++-
net/core/dev.c | 1 +
5 files changed, 54 insertions(+), 3 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8bf8d6149f79..bf3f557eed8c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2416,6 +2416,7 @@ enum netdev_cmd {
NETDEV_CVLAN_FILTER_DROP_INFO,
NETDEV_SVLAN_FILTER_PUSH_INFO,
NETDEV_SVLAN_FILTER_DROP_INFO,
+ NETDEV_CHANGEVLAN,
};
const char *netdev_cmd_to_name(enum netdev_cmd cmd);
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 99141986efa0..b6d0b2e2ada0 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -51,8 +51,8 @@ const char vlan_version[] = DRV_VERSION;
/* End of global variables definitions. */
-static int vlan_group_prealloc_vid(struct vlan_group *vg,
- __be16 vlan_proto, u16 vlan_id)
+int vlan_group_prealloc_vid(struct vlan_group *vg,
+ __be16 vlan_proto, u16 vlan_id)
{
struct net_device **array;
unsigned int pidx, vidx;
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index 44df1c3df02d..c734dd21d70d 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -116,6 +116,8 @@ int register_vlan_dev(struct net_device *dev, struct netlink_ext_ack *extack);
void unregister_vlan_dev(struct net_device *dev, struct list_head *head);
bool vlan_dev_inherit_address(struct net_device *dev,
struct net_device *real_dev);
+int vlan_group_prealloc_vid(struct vlan_group *vg,
+ __be16 vlan_proto, u16 vlan_id);
static inline u32 vlan_get_ingress_priority(struct net_device *dev,
u16 vlan_tci)
diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index 9b60c1e399e2..0a4ae6b15d89 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -107,10 +107,56 @@ static int vlan_changelink(struct net_device *dev, struct nlattr *tb[],
struct nlattr *data[],
struct netlink_ext_ack *extack)
{
+ struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
struct ifla_vlan_flags *flags;
struct ifla_vlan_qos_mapping *m;
struct nlattr *attr;
int rem;
+ int err = 0;
+ __be16 vlan_proto = vlan->vlan_proto;
+ u16 vlan_id = vlan->vlan_id;
+
+ if (data[IFLA_VLAN_ID])
+ vlan_id = nla_get_u16(data[IFLA_VLAN_ID]);
+
+ if (data[IFLA_VLAN_PROTOCOL])
+ vlan_proto = nla_get_be16(data[IFLA_VLAN_PROTOCOL]);
+
+ if (vlan->vlan_id != vlan_id || vlan->vlan_proto != vlan_proto) {
+ struct net_device *real_dev = vlan->real_dev;
+ struct vlan_info *vlan_info;
+ struct vlan_group *grp;
+ __be16 old_vlan_proto = vlan->vlan_proto;
+ u16 old_vlan_id = vlan->vlan_id;
+
+ err = vlan_vid_add(real_dev, vlan_proto, vlan_id);
+ if (err)
+ goto out;
+ vlan_info = rtnl_dereference(real_dev->vlan_info);
+ grp = &vlan_info->grp;
+ err = vlan_group_prealloc_vid(grp, vlan_proto, vlan_id);
+ if (err < 0) {
+ vlan_vid_del(real_dev, vlan_proto, vlan_id);
+ return err;
+ }
+ vlan_group_set_device(grp, vlan_proto, vlan_id, dev);
+ vlan->vlan_proto = vlan_proto;
+ vlan->vlan_id = vlan_id;
+
+ err = call_netdevice_notifiers(NETDEV_CHANGEVLAN, dev);
+ err = notifier_to_errno(err);
+ if (err) {
+ /* rollback */
+ vlan_group_set_device(grp, vlan_proto, vlan_id, NULL);
+ vlan_vid_del(real_dev, vlan_proto, vlan_id);
+ vlan->vlan_proto = old_vlan_proto;
+ vlan->vlan_id = old_vlan_id;
+ } else {
+ vlan_group_set_device(grp, old_vlan_proto,
+ old_vlan_id, NULL);
+ vlan_vid_del(real_dev, old_vlan_proto, old_vlan_id);
+ }
+ }
if (data[IFLA_VLAN_FLAGS]) {
flags = nla_data(data[IFLA_VLAN_FLAGS]);
@@ -128,7 +174,8 @@ static int vlan_changelink(struct net_device *dev, struct nlattr *tb[],
vlan_dev_set_egress_priority(dev, m->from, m->to);
}
}
- return 0;
+out:
+ return err;
}
static int vlan_newlink(struct net *src_net, struct net_device *dev,
diff --git a/net/core/dev.c b/net/core/dev.c
index 08d58e0debe5..63adc37e3548 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1587,6 +1587,7 @@ const char *netdev_cmd_to_name(enum netdev_cmd cmd)
N(UDP_TUNNEL_DROP_INFO) N(CHANGE_TX_QUEUE_LEN)
N(CVLAN_FILTER_PUSH_INFO) N(CVLAN_FILTER_DROP_INFO)
N(SVLAN_FILTER_PUSH_INFO) N(SVLAN_FILTER_DROP_INFO)
+ N(CHANGEVLAN)
}
#undef N
return "UNKNOWN_NETDEV_EVENT";
--
2.14.4
^ permalink raw reply related
* [PATCH net] net/ipv6: Revert attempt to simplify route replace and append
From: dsahern @ 2018-07-02 22:03 UTC (permalink / raw)
To: netdev; +Cc: idosch, sharpd, Thomas.Winter, David Ahern
From: David Ahern <dsahern@gmail.com>
NetworkManager likes to manage linklocal prefix routes and does so with
the NLM_F_APPEND flag, breaking attempts to simplify the IPv6 route
code and by extension enable multipath routes with device only nexthops.
Revert f34436a43092 and its followup
6eba08c3626b ("ipv6: Only emit append events for appended routes").
Update the test cases to reflect the old behavior.
Fixes: f34436a43092 ("net/ipv6: Simplify route replace and appending into multipath route")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
Ido: I left 5a15a1b07c51. FIB_EVENT_ENTRY_APPEND is not generated for
IPv6, so no harm in leaving it.
include/net/ip6_route.h | 6 ++
net/ipv6/ip6_fib.c | 156 +++++++++++++++++--------------
net/ipv6/route.c | 3 +-
tools/testing/selftests/net/fib_tests.sh | 41 --------
4 files changed, 93 insertions(+), 113 deletions(-)
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 59656fc580df..7b9c82de11cc 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -66,6 +66,12 @@ static inline bool rt6_need_strict(const struct in6_addr *daddr)
(IPV6_ADDR_MULTICAST | IPV6_ADDR_LINKLOCAL | IPV6_ADDR_LOOPBACK);
}
+static inline bool rt6_qualify_for_ecmp(const struct fib6_info *f6i)
+{
+ return (f6i->fib6_flags & (RTF_GATEWAY|RTF_ADDRCONF|RTF_DYNAMIC)) ==
+ RTF_GATEWAY;
+}
+
void ip6_route_input(struct sk_buff *skb);
struct dst_entry *ip6_route_input_lookup(struct net *net,
struct net_device *dev,
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 1fb2f3118d60..d212738e9d10 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -935,20 +935,19 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
{
struct fib6_info *leaf = rcu_dereference_protected(fn->leaf,
lockdep_is_held(&rt->fib6_table->tb6_lock));
- enum fib_event_type event = FIB_EVENT_ENTRY_ADD;
- struct fib6_info *iter = NULL, *match = NULL;
+ struct fib6_info *iter = NULL;
struct fib6_info __rcu **ins;
+ struct fib6_info __rcu **fallback_ins = NULL;
int replace = (info->nlh &&
(info->nlh->nlmsg_flags & NLM_F_REPLACE));
- int append = (info->nlh &&
- (info->nlh->nlmsg_flags & NLM_F_APPEND));
int add = (!info->nlh ||
(info->nlh->nlmsg_flags & NLM_F_CREATE));
int found = 0;
+ bool rt_can_ecmp = rt6_qualify_for_ecmp(rt);
u16 nlflags = NLM_F_EXCL;
int err;
- if (append)
+ if (info->nlh && (info->nlh->nlmsg_flags & NLM_F_APPEND))
nlflags |= NLM_F_APPEND;
ins = &fn->leaf;
@@ -970,8 +969,13 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
nlflags &= ~NLM_F_EXCL;
if (replace) {
- found++;
- break;
+ if (rt_can_ecmp == rt6_qualify_for_ecmp(iter)) {
+ found++;
+ break;
+ }
+ if (rt_can_ecmp)
+ fallback_ins = fallback_ins ?: ins;
+ goto next_iter;
}
if (rt6_duplicate_nexthop(iter, rt)) {
@@ -986,51 +990,71 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
fib6_metric_set(iter, RTAX_MTU, rt->fib6_pmtu);
return -EEXIST;
}
-
- /* first route that matches */
- if (!match)
- match = iter;
+ /* If we have the same destination and the same metric,
+ * but not the same gateway, then the route we try to
+ * add is sibling to this route, increment our counter
+ * of siblings, and later we will add our route to the
+ * list.
+ * Only static routes (which don't have flag
+ * RTF_EXPIRES) are used for ECMPv6.
+ *
+ * To avoid long list, we only had siblings if the
+ * route have a gateway.
+ */
+ if (rt_can_ecmp &&
+ rt6_qualify_for_ecmp(iter))
+ rt->fib6_nsiblings++;
}
if (iter->fib6_metric > rt->fib6_metric)
break;
+next_iter:
ins = &iter->fib6_next;
}
+ if (fallback_ins && !found) {
+ /* No ECMP-able route found, replace first non-ECMP one */
+ ins = fallback_ins;
+ iter = rcu_dereference_protected(*ins,
+ lockdep_is_held(&rt->fib6_table->tb6_lock));
+ found++;
+ }
+
/* Reset round-robin state, if necessary */
if (ins == &fn->leaf)
fn->rr_ptr = NULL;
/* Link this route to others same route. */
- if (append && match) {
+ if (rt->fib6_nsiblings) {
+ unsigned int fib6_nsiblings;
struct fib6_info *sibling, *temp_sibling;
- if (rt->fib6_flags & RTF_REJECT) {
- NL_SET_ERR_MSG(extack,
- "Can not append a REJECT route");
- return -EINVAL;
- } else if (match->fib6_flags & RTF_REJECT) {
- NL_SET_ERR_MSG(extack,
- "Can not append to a REJECT route");
- return -EINVAL;
+ /* Find the first route that have the same metric */
+ sibling = leaf;
+ while (sibling) {
+ if (sibling->fib6_metric == rt->fib6_metric &&
+ rt6_qualify_for_ecmp(sibling)) {
+ list_add_tail(&rt->fib6_siblings,
+ &sibling->fib6_siblings);
+ break;
+ }
+ sibling = rcu_dereference_protected(sibling->fib6_next,
+ lockdep_is_held(&rt->fib6_table->tb6_lock));
}
- event = FIB_EVENT_ENTRY_APPEND;
- rt->fib6_nsiblings = match->fib6_nsiblings;
- list_add_tail(&rt->fib6_siblings, &match->fib6_siblings);
- match->fib6_nsiblings++;
-
/* For each sibling in the list, increment the counter of
* siblings. BUG() if counters does not match, list of siblings
* is broken!
*/
+ fib6_nsiblings = 0;
list_for_each_entry_safe(sibling, temp_sibling,
- &match->fib6_siblings, fib6_siblings) {
+ &rt->fib6_siblings, fib6_siblings) {
sibling->fib6_nsiblings++;
- BUG_ON(sibling->fib6_nsiblings != match->fib6_nsiblings);
+ BUG_ON(sibling->fib6_nsiblings != rt->fib6_nsiblings);
+ fib6_nsiblings++;
}
-
- rt6_multipath_rebalance(match);
+ BUG_ON(fib6_nsiblings != rt->fib6_nsiblings);
+ rt6_multipath_rebalance(temp_sibling);
}
/*
@@ -1043,8 +1067,9 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
add:
nlflags |= NLM_F_CREATE;
- err = call_fib6_entry_notifiers(info->nl_net, event, rt,
- extack);
+ err = call_fib6_entry_notifiers(info->nl_net,
+ FIB_EVENT_ENTRY_ADD,
+ rt, extack);
if (err)
return err;
@@ -1062,7 +1087,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
}
} else {
- struct fib6_info *tmp;
+ int nsiblings;
if (!found) {
if (add)
@@ -1077,57 +1102,48 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
if (err)
return err;
- /* if route being replaced has siblings, set tmp to
- * last one, otherwise tmp is current route. this is
- * used to set fib6_next for new route
- */
- if (iter->fib6_nsiblings)
- tmp = list_last_entry(&iter->fib6_siblings,
- struct fib6_info,
- fib6_siblings);
- else
- tmp = iter;
-
- /* insert new route */
atomic_inc(&rt->fib6_ref);
rcu_assign_pointer(rt->fib6_node, fn);
- rt->fib6_next = tmp->fib6_next;
+ rt->fib6_next = iter->fib6_next;
rcu_assign_pointer(*ins, rt);
-
if (!info->skip_notify)
inet6_rt_notify(RTM_NEWROUTE, rt, info, NLM_F_REPLACE);
if (!(fn->fn_flags & RTN_RTINFO)) {
info->nl_net->ipv6.rt6_stats->fib_route_nodes++;
fn->fn_flags |= RTN_RTINFO;
}
+ nsiblings = iter->fib6_nsiblings;
+ iter->fib6_node = NULL;
+ fib6_purge_rt(iter, fn, info->nl_net);
+ if (rcu_access_pointer(fn->rr_ptr) == iter)
+ fn->rr_ptr = NULL;
+ fib6_info_release(iter);
- /* delete old route */
- rt = iter;
-
- if (rt->fib6_nsiblings) {
- struct fib6_info *tmp;
-
+ if (nsiblings) {
/* Replacing an ECMP route, remove all siblings */
- list_for_each_entry_safe(iter, tmp, &rt->fib6_siblings,
- fib6_siblings) {
- iter->fib6_node = NULL;
- fib6_purge_rt(iter, fn, info->nl_net);
- if (rcu_access_pointer(fn->rr_ptr) == iter)
- fn->rr_ptr = NULL;
- fib6_info_release(iter);
-
- rt->fib6_nsiblings--;
- info->nl_net->ipv6.rt6_stats->fib_rt_entries--;
+ ins = &rt->fib6_next;
+ iter = rcu_dereference_protected(*ins,
+ lockdep_is_held(&rt->fib6_table->tb6_lock));
+ while (iter) {
+ if (iter->fib6_metric > rt->fib6_metric)
+ break;
+ if (rt6_qualify_for_ecmp(iter)) {
+ *ins = iter->fib6_next;
+ iter->fib6_node = NULL;
+ fib6_purge_rt(iter, fn, info->nl_net);
+ if (rcu_access_pointer(fn->rr_ptr) == iter)
+ fn->rr_ptr = NULL;
+ fib6_info_release(iter);
+ nsiblings--;
+ info->nl_net->ipv6.rt6_stats->fib_rt_entries--;
+ } else {
+ ins = &iter->fib6_next;
+ }
+ iter = rcu_dereference_protected(*ins,
+ lockdep_is_held(&rt->fib6_table->tb6_lock));
}
+ WARN_ON(nsiblings != 0);
}
-
- WARN_ON(rt->fib6_nsiblings != 0);
-
- rt->fib6_node = NULL;
- fib6_purge_rt(rt, fn, info->nl_net);
- if (rcu_access_pointer(fn->rr_ptr) == rt)
- fn->rr_ptr = NULL;
- fib6_info_release(rt);
}
return 0;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 86a0e4333d42..63f99411f0de 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3842,7 +3842,7 @@ static struct fib6_info *rt6_multipath_first_sibling(const struct fib6_info *rt)
lockdep_is_held(&rt->fib6_table->tb6_lock));
while (iter) {
if (iter->fib6_metric == rt->fib6_metric &&
- iter->fib6_nsiblings)
+ rt6_qualify_for_ecmp(iter))
return iter;
iter = rcu_dereference_protected(iter->fib6_next,
lockdep_is_held(&rt->fib6_table->tb6_lock));
@@ -4439,7 +4439,6 @@ static int ip6_route_multipath_add(struct fib6_config *cfg,
*/
cfg->fc_nlinfo.nlh->nlmsg_flags &= ~(NLM_F_EXCL |
NLM_F_REPLACE);
- cfg->fc_nlinfo.nlh->nlmsg_flags |= NLM_F_APPEND;
nhn++;
}
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 78245d60d8bc..0f45633bd634 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -740,13 +740,6 @@ ipv6_rt_add()
run_cmd "$IP -6 ro add unreachable 2001:db8:104::/64"
log_test $? 2 "Attempt to add duplicate route - reject route"
- # iproute2 prepend only sets NLM_F_CREATE
- # - adds a new route; does NOT convert existing route to ECMP
- add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
- run_cmd "$IP -6 ro prepend 2001:db8:104::/64 via 2001:db8:103::2"
- check_route6 "2001:db8:104::/64 via 2001:db8:101::2 dev veth1 metric 1024 2001:db8:104::/64 via 2001:db8:103::2 dev veth3 metric 1024"
- log_test $? 0 "Add new route for existing prefix (w/o NLM_F_EXCL)"
-
# route append with same prefix adds a new route
# - iproute2 sets NLM_F_CREATE | NLM_F_APPEND
add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
@@ -754,27 +747,6 @@ ipv6_rt_add()
check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::2 dev veth1 weight 1 nexthop via 2001:db8:103::2 dev veth3 weight 1"
log_test $? 0 "Append nexthop to existing route - gw"
- add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
- run_cmd "$IP -6 ro append 2001:db8:104::/64 dev veth3"
- check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::2 dev veth1 weight 1 nexthop dev veth3 weight 1"
- log_test $? 0 "Append nexthop to existing route - dev only"
-
- # multipath route can not have a nexthop that is a reject route
- add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
- run_cmd "$IP -6 ro append unreachable 2001:db8:104::/64"
- log_test $? 2 "Append nexthop to existing route - reject route"
-
- # reject route can not be converted to multipath route
- run_cmd "$IP -6 ro flush 2001:db8:104::/64"
- run_cmd "$IP -6 ro add unreachable 2001:db8:104::/64"
- run_cmd "$IP -6 ro append 2001:db8:104::/64 via 2001:db8:103::2"
- log_test $? 2 "Append nexthop to existing reject route - gw"
-
- run_cmd "$IP -6 ro flush 2001:db8:104::/64"
- run_cmd "$IP -6 ro add unreachable 2001:db8:104::/64"
- run_cmd "$IP -6 ro append 2001:db8:104::/64 dev veth3"
- log_test $? 2 "Append nexthop to existing reject route - dev only"
-
# insert mpath directly
add_route6 "2001:db8:104::/64" "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::2 dev veth1 weight 1 nexthop via 2001:db8:103::2 dev veth3 weight 1"
@@ -819,13 +791,6 @@ ipv6_rt_replace_single()
check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::3 dev veth1 weight 1 nexthop via 2001:db8:103::2 dev veth3 weight 1"
log_test $? 0 "Single path with multipath"
- # single path with reject
- #
- add_initial_route6 "nexthop via 2001:db8:101::2"
- run_cmd "$IP -6 ro replace unreachable 2001:db8:104::/64"
- check_route6 "unreachable 2001:db8:104::/64 dev lo metric 1024"
- log_test $? 0 "Single path with reject route"
-
# single path with single path using MULTIPATH attribute
#
add_initial_route6 "via 2001:db8:101::2"
@@ -873,12 +838,6 @@ ipv6_rt_replace_mpath()
check_route6 "2001:db8:104::/64 via 2001:db8:101::3 dev veth1 metric 1024"
log_test $? 0 "Multipath with single path via multipath attribute"
- # multipath with reject
- add_initial_route6 "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
- run_cmd "$IP -6 ro replace unreachable 2001:db8:104::/64"
- check_route6 "unreachable 2001:db8:104::/64 dev lo metric 1024"
- log_test $? 0 "Multipath with reject route"
-
# route replace fails - invalid nexthop 1
add_initial_route6 "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
run_cmd "$IP -6 ro replace 2001:db8:104::/64 nexthop via 2001:db8:111::3 nexthop via 2001:db8:103::3"
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next 08/10] r8169: remove rtl8169_set_speed_xmii
From: Heiner Kallweit @ 2018-07-02 21:54 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <20180702212150.GG12564@lunn.ch>
On 02.07.2018 23:21, Andrew Lunn wrote:
>> - auto_nego |= ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM;
>
> This bit you probably want to keep. The PHY never says it support
> Pause. The MAC needs to enable pause if the MAC supports pause.
>
Actually I assumed that phylib would do this for me. But:
In phy_probe() first phydev->supported is copied to
phydev->advertising, and only after this both pause flags are added
to phydev->supported. Therefore I think they are not advertised.
Is this intentional? It sounds a little weird to me to add the
pause flags to the supported features per default, but not
advertise them.
Except e.g. we call by chance phy_set_max_speed(), which copies
phydev->supported to phydev->advertising after having adjusted
the supported speeds.
If this is not a bug, then where would be the right place to add
the pause flags to phydev->advertising?
> Andrew
>
Heiner
^ permalink raw reply
* [PATCH net-next v2 2/2] tcp: ack immediately when a cwr packet arrives
From: Lawrence Brakmo @ 2018-07-02 21:39 UTC (permalink / raw)
To: netdev
Cc: Kernel Team, Blake Matheny, Alexei Starovoitov, Neal Cardwell,
Yuchung Cheng, Steve Ibanez, Eric Dumazet
In-Reply-To: <20180702213908.1246455-1-brakmo@fb.com>
We observed high 99 and 99.9% latencies when doing RPCs with DCTCP. The
problem is triggered when the last packet of a request arrives CE
marked. The reply will carry the ECE mark causing TCP to shrink its cwnd
to 1 (because there are no packets in flight). When the 1st packet of
the next request arrives, the ACK was sometimes delayed even though it
is CWR marked, adding up to 40ms to the RPC latency.
This patch insures that CWR makred data packets arriving will be acked
immediately.
Modified based on comments by Neal Cardwell <ncardwell@google.com>
Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
---
net/ipv4/tcp_input.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 76ca88f63b70..6fd1f2378f6c 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -254,10 +254,16 @@ static void tcp_ecn_withdraw_cwr(struct tcp_sock *tp)
tp->ecn_flags &= ~TCP_ECN_DEMAND_CWR;
}
-static void __tcp_ecn_check_ce(struct sock *sk, const struct sk_buff *skb)
+static void __tcp_ecn_check(struct sock *sk, const struct sk_buff *skb)
{
struct tcp_sock *tp = tcp_sk(sk);
+ /* If the sender is telling us it has entered CWR, then its cwnd may be
+ * very low (even just 1 packet), so we should ACK immediately.
+ */
+ if (tcp_hdr(skb)->cwr)
+ tcp_enter_quickack_mode(sk, 2);
+
switch (TCP_SKB_CB(skb)->ip_dsfield & INET_ECN_MASK) {
case INET_ECN_NOT_ECT:
/* Funny extension: if ECT is not set on a segment,
@@ -286,10 +292,10 @@ static void __tcp_ecn_check_ce(struct sock *sk, const struct sk_buff *skb)
}
}
-static void tcp_ecn_check_ce(struct sock *sk, const struct sk_buff *skb)
+static void tcp_ecn_check(struct sock *sk, const struct sk_buff *skb)
{
if (tcp_sk(sk)->ecn_flags & TCP_ECN_OK)
- __tcp_ecn_check_ce(sk, skb);
+ __tcp_ecn_check(sk, skb);
}
static void tcp_ecn_rcv_synack(struct tcp_sock *tp, const struct tcphdr *th)
@@ -715,7 +721,7 @@ static void tcp_event_data_recv(struct sock *sk, struct sk_buff *skb)
}
icsk->icsk_ack.lrcvtime = now;
- tcp_ecn_check_ce(sk, skb);
+ tcp_ecn_check(sk, skb);
if (skb->len >= 128)
tcp_grow_window(sk, skb);
@@ -4439,7 +4445,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
u32 seq, end_seq;
bool fragstolen;
- tcp_ecn_check_ce(sk, skb);
+ tcp_ecn_check(sk, skb);
if (unlikely(tcp_try_rmem_schedule(sk, skb, skb->truesize))) {
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFODROP);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v2 1/2] tcp: notify when a delayed ack is sent
From: Lawrence Brakmo @ 2018-07-02 21:39 UTC (permalink / raw)
To: netdev
Cc: Kernel Team, Blake Matheny, Alexei Starovoitov, Neal Cardwell,
Yuchung Cheng, Steve Ibanez, Eric Dumazet
In-Reply-To: <20180702213908.1246455-1-brakmo@fb.com>
DCTCP depends on the CA_EVENT_NON_DELAYED_ACK and CA_EVENT_DELAYED_ACK
notifications to keep track if it needs to send an ACK for packets that
were received with a particular ECN state but whose ACK was delayed.
Under some circumstances, for example when a delayed ACK is sent with a
data packet, DCTCP state was not being updated due to a lack of
notification that the previously delayed ACK was sent. As a result, it
would sometimes send a duplicate ACK when a new data packet arrived.
This patch insures that DCTCP's state is correctly updated so it will
not send the duplicate ACK.
Improved based on comments from Neal Cardwell <ncardwell@google.com>.
Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
---
net/ipv4/tcp_output.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f8f6129160dd..acefb64e8280 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -172,6 +172,8 @@ static inline void tcp_event_ack_sent(struct sock *sk, unsigned int pkts)
__sock_put(sk);
}
tcp_dec_quickack_mode(sk, pkts);
+ if (inet_csk_ack_scheduled(sk))
+ tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK);
inet_csk_clear_xmit_timer(sk, ICSK_TIME_DACK);
}
@@ -3567,8 +3569,6 @@ void tcp_send_ack(struct sock *sk)
if (sk->sk_state == TCP_CLOSE)
return;
- tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK);
-
/* We are not putting this on the write queue, so
* tcp_transmit_skb() will set the ownership to this
* sock.
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v2 0/2] tcp: fix high tail latencies in DCTCP
From: Lawrence Brakmo @ 2018-07-02 21:39 UTC (permalink / raw)
To: netdev
Cc: Kernel Team, Blake Matheny, Alexei Starovoitov, Neal Cardwell,
Yuchung Cheng, Steve Ibanez, Eric Dumazet
When have observed high tail latencies when using DCTCP for RPCs as
compared to using Cubic. For example, in one setup there are 2 hosts
sending to a 3rd one, with each sender having 3 flows (1 stream,
1 1MB back-to-back RPCs and 1 10KB back-to-back RPCs). The following
table shows the 99% and 99.9% latencies for both Cubic and dctcp:
Cubic 99% Cubic 99.9% dctcp 99% dctcp 99.9%
1MB RPCs 2.6ms 5.5ms 43ms 208ms
10KB RPCs 1.1ms 1.3ms 53ms 212ms
Looking at tcpdump traces showed that there are two causes for the
latency.
1) RTOs caused by the receiver sending a dup ACK and not ACKing
the last (and only) packet sent.
2) Delaying ACKs when the sender has a cwnd of 1, so everything
pauses for the duration of the delayed ACK.
The first patch fixes the cause of the dup ACKs, not updating DCTCP
state when an ACK that was initially delayed has been sent with a
data packet.
The second patch insures that an ACK is sent immediately when a
CWR marked packet arrives.
With the patches the latencies for DCTCP now look like:
dctcp 99% dctcp 99.9%
1MB RPCs 5.8ms 6.9ms
10KB RPCs 146us 203us
Note that while the 1MB RPCs tail latencies are higher than Cubic's,
the 10KB latencies are much smaller than Cubic's. These patches fix
issues on the receiver, but tcpdump traces indicate there is an
opportunity to also fix an issue at the sender that adds about 3ms
to the tail latencies.
The following trace shows the issue that tiggers an RTO (fixed by these patches):
Host A sends the last packets of the request
Host B receives them, and the last packet is marked with congestion (CE)
Host B sends ACKs for packets not marked with congestion
Host B sends data packet with reply and ACK for packet marked with
congestion (TCP flag ECE)
Host A receives ACKs with no ECE flag
Host A receives data packet with ACK for the last packet of request
and which has TCP ECE bit set
Host A sends 1st data packet of the next request with TCP flag CWR
Host B receives the packet (as seen in tcpdump at B), no CE flag
Host B sends a dup ACK that also has the TCP ECE flag
Host A RTO timer fires!
Host A to send the next packet
Host A receives an ACK for everything it has sent (i.e. Host B
did receive 1st packet of request)
Host A send more packets…
[PATCH net-next v2 1/2] tcp: notify when a delayed ack is sent
[PATCH net-next v2 2/2] tcp: ack immediately when a cwr packet
net/ipv4/tcp_input.c | 16 +++++++++++-----
net/ipv4/tcp_output.c | 4 ++--
2 files changed, 13 insertions(+), 7 deletions(-)
^ permalink raw reply
* Re: [PATCH] 6lowpan: iphc: reset mac_header after decompress to fix panic
From: Alexander Aring @ 2018-07-02 21:31 UTC (permalink / raw)
To: Michael Scott
Cc: Jukka Rissanen, David S. Miller, linux-bluetooth, linux-wpan,
netdev, linux-kernel
In-Reply-To: <20180702204346.d7bynetvzw3ayn5m@x220t>
Hi,
On Mon, Jul 02, 2018 at 04:43:46PM -0400, Alexander Aring wrote:
> Hi,
>
> On Mon, Jul 02, 2018 at 12:45:41PM -0700, Michael Scott wrote:
> > Hello Alexander,
> >
> ...
> > > Question is for me: which upper layer wants access MAC header here on
> > > receive path?
> > > It cannot parsed anyhow because so far I know no upper layer can parse
> > > at the moment 802.15.4 frames (which is a complex format). Maybe over
> > > some header_ops callback?
> >
> > I was testing a C program which performs NAT64 handling on packets
> > destined to a certain IPv6 subnet (64:ff9b::). To do this, the application
> > opens a RAW socket like this: sniff_sock = socket(PF_PACKET, SOCK_RAW,
> > htons(ETH_P_ALL)); It then sets promiscuous mode and enters a looping call
> > of:
> > length = recv(sniff_sock, buffer, PACKET_BUFFER, MSG_TRUNC); My host PC
> > kernel would then promptly crash on me. (I'm going to purposely avoid the
> > obvious point of: this probably isn't the best way to parse packets for
> > NAT64 translation as you will get every single packet incoming or outgoing
> > on the host.) Turns out, testing the program on an 802.15.4 6lowpan
> > interface exposed some of the issues which this mailing list (but not
> > myself) is well aware of (no L2 data in the RAW packets) and also led me to
> > debugging this patch to stop the kernel crash. TL;DR: To summarize, any
> > PF_PACKET SOCK_RAW socket which recv()'s IPv6 data from a 6lowpan node will
> > cause this kernel crash eventually (checked on kernel 4.15, 4.16, 4.17 and
> > 4.18-rc1). - Mike
> > >
>
> "any PF_PACKET SOCK_RAW" can't be otherwise I would also see it with my
> sniffer programs e.g. wireshark or tcpdump which use libpcap.
>
> There need to be some different in the handling. This is what I have
> currently in my mind.
>
> I currently not sure how to set skb->mac_header if interface is RAW_IP.
> It seems there is an indicator that mac header is not set. Example:
>
> diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
> index 6b1042e21656..e6ec2df3afe0 100644
> --- a/net/6lowpan/iphc.c
> +++ b/net/6lowpan/iphc.c
> @@ -770,6 +770,7 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
> hdr.hop_limit, &hdr.daddr);
>
> skb_push(skb, sizeof(hdr));
> + skb->mac_header = (typeof(skb->mac_header))~0U;
> skb_reset_network_header(skb);
> skb_copy_to_linear_data(skb, &hdr, sizeof(hdr));
>
>
> Maybe we should lookup what skb->mac_header points to on tun interfaces
> then do the same.
So far I see [0] tun does the same as your patch approach does. network
header and mac header points to the same.
I think then we should go with your patch.
Then we just need to solve the issue to get mac header information on
top of lowpan socket layer... out of scope issue but indeed we need to
solve that. As we talked there exists even UDP protocols which needs mac
header information. It's in a pretty early state, I have no idea how
this fits sometimes with fragmentation handling together. At least for
L2 address handling and fragmentation it should be fine (one of the
fragment indentifier).
- Alex
[0] https://elixir.bootlin.com/linux/v4.18-rc3/source/drivers/net/tun.c#L1912
^ permalink raw reply
* Re: [PATCH net-next 07/10] r8169: migrate speed_down function to phylib
From: Heiner Kallweit @ 2018-07-02 21:31 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <20180702212005.GF12564@lunn.ch>
On 02.07.2018 23:20, Andrew Lunn wrote:
> On Mon, Jul 02, 2018 at 09:37:08PM +0200, Heiner Kallweit wrote:
>> Change rtl_speed_down() to use phylib.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> drivers/net/ethernet/realtek/r8169.c | 33 +++++++++++++---------------
>> 1 file changed, 15 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
>> index 311321ee..807fbc75 100644
>> --- a/drivers/net/ethernet/realtek/r8169.c
>> +++ b/drivers/net/ethernet/realtek/r8169.c
>> @@ -4240,6 +4240,10 @@ static void rtl8169_init_phy(struct net_device *dev, struct rtl8169_private *tp)
>> rtl_writephy(tp, 0x0b, 0x0000); //w 0x0b 15 0 0
>> }
>>
>> + /* We may have called rtl_speed_down before */
>> + dev->phydev->advertising = dev->phydev->supported;
>> + genphy_config_aneg(dev->phydev);
>> +
>> genphy_soft_reset(dev->phydev);
>>
>> rtl8169_set_speed(dev, AUTONEG_ENABLE, SPEED_1000, DUPLEX_FULL,
>> @@ -4323,28 +4327,21 @@ static void rtl_init_mdio_ops(struct rtl8169_private *tp)
>> }
>> }
>>
>> +#define BASET10 (ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full)
>> +#define BASET100 (ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full)
>> +#define BASET1000 (ADVERTISED_1000baseT_Half | ADVERTISED_1000baseT_Full)
>> +
>> static void rtl_speed_down(struct rtl8169_private *tp)
>> {
>> - u32 adv;
>> - int lpa;
>> + struct phy_device *phydev = tp->dev->phydev;
>> + u32 adv = phydev->lp_advertising & phydev->supported;
>>
>> - rtl_writephy(tp, 0x1f, 0x0000);
>> - lpa = rtl_readphy(tp, MII_LPA);
>> + if (adv & BASET10)
>> + phydev->advertising &= ~(BASET100 | BASET1000);
>> + else if (adv & BASET100)
>> + phydev->advertising &= ~BASET1000;
>>
>> - if (lpa & (LPA_10HALF | LPA_10FULL))
>> - adv = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full;
>> - else if (lpa & (LPA_100HALF | LPA_100FULL))
>> - adv = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full |
>> - ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full;
>> - else
>> - adv = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full |
>> - ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full |
>> - (tp->mii.supports_gmii ?
>> - ADVERTISED_1000baseT_Half |
>> - ADVERTISED_1000baseT_Full : 0);
>> -
>> - rtl8169_set_speed(tp->dev, AUTONEG_ENABLE, SPEED_1000, DUPLEX_FULL,
>> - adv);
>> + genphy_config_aneg(phydev);
>> }
>
> It probably it is me being too tired, but i don't get what this is
> doing? Changing the local advertisement based on what the remote is
> advertising. Why?
>
It also took me some time to understand what this speed_down is doing.
If we suspend and wait for a WoL packet, then we don't have to burn all
the energy for a GBit connection. Therefore we switch to the lowest
speed supported by chip and link partner. This is done by removing
higher speeds from the advertised modes and restarting an autonego.
> Andrew
>
^ permalink raw reply
* Re: [PATCH net-next 02/10] r8169: use phy_resume/phy_suspend
From: Heiner Kallweit @ 2018-07-02 21:24 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <20180702210646.GB12564@lunn.ch>
On 02.07.2018 23:06, Andrew Lunn wrote:
>> static void r8168_pll_power_down(struct rtl8169_private *tp)
>> {
>> if (r8168_check_dash(tp))
>> @@ -4510,7 +4469,8 @@ static void r8168_pll_power_down(struct rtl8169_private *tp)
>> if (rtl_wol_pll_power_down(tp))
>> return;
>>
>> - r8168_phy_power_down(tp);
>> + /* cover the case that PHY isn't connected */
>> + phy_suspend(mdiobus_get_phy(tp->mii_bus, 0));
>
> This could do some more explanation. Why would it not be connected?
>
The PHY gets connected when the net_device is opened. If a network
port isn't used then it will be runtime-suspended a few seconds after
boot. In this case we call r8168_pll_power_down() with the PHY not
being connected.
> Andrew
>
^ permalink raw reply
* Re: [PATCH net-next 01/10] r8169: add basic phylib support
From: Heiner Kallweit @ 2018-07-02 21:15 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <20180702210239.GA12564@lunn.ch>
On 02.07.2018 23:02, Andrew Lunn wrote:
>> +static int r8169_mdio_read_reg(struct mii_bus *mii_bus, int phyaddr, int phyreg)
>> +{
>> + struct rtl8169_private *tp = mii_bus->priv;
>> +
>> + return rtl_readphy(tp, phyreg);
>
> So there is no support for phyaddr?
>
Right, the chip can access only the one internal PHY, therefore it
doesn't support phyaddr.
> It would be better to trap the phyaddr which are not supported and
> return 0xffff.
>
OK
> Andrew
>
^ permalink raw reply
* Re: [PATCH net-next 2/2] tcp: ack immediately when a cwr packet arrives
From: Lawrence Brakmo @ 2018-07-02 21:24 UTC (permalink / raw)
To: Neal Cardwell
Cc: Netdev, Kernel Team, Blake Matheny, Alexei Starovoitov,
Yuchung Cheng, Steve Ibanez, Eric Dumazet
In-Reply-To: <CADVnQy=3s4AAX-SyHaRURPgA=mN=m4DBP=Jz3HGt5pBPbiRaaA@mail.gmail.com>
On 7/2/18, 7:57 AM, "Neal Cardwell" <ncardwell@google.com> wrote:
On Sat, Jun 30, 2018 at 9:47 PM Lawrence Brakmo <brakmo@fb.com> wrote:
> I see two issues, one is that entering quickack mode as you
> mentioned does not insure that it will still be on when the CWR
> arrives. The second issue is that the problem occurs right after the
> receiver sends a small reply which results in entering pingpong mode
> right before the sender starts the new request by sending just one
> packet (i.e. forces delayed ack).
>
> I compiled and tested your patch. Both 99 and 99.9 percentile
> latencies are around 40ms. Looking at the packet traces shows that
> some CWR marked packets are not being ack immediately (delayed by
> 40ms).
Thanks, Larry! So your tests provide nice, specific evidence that it
is good to force an immediate ACK when a receiver receives a packet
with CWR marked. Given that, I am wondering what the simplest way is
to achieve that goal.
What if, rather than plumbing a new specific signal into
__tcp_ack_snd_check(), we use the existing general quick-ack
mechanism, where various parts of the TCP stack (like
__tcp_ecn_check_ce()) are already using the quick-ack mechanism to
"remotely" signal to __tcp_ack_snd_check() that they want an immediate
ACK.
For example, would it work to do something like:
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index c53ae5fc834a5..8168d1938b376 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -262,6 +262,12 @@ static void __tcp_ecn_check_ce(struct sock *sk,
const struct sk_buff *skb)
{
struct tcp_sock *tp = tcp_sk(sk);
+ /* If the sender is telling us it has entered CWR, then its cwnd may be
+ * very low (even just 1 packet), so we should ACK immediately.
+ */
+ if (tcp_hdr(skb)->cwr)
+ tcp_enter_quickack_mode(sk, 2);
+
switch (TCP_SKB_CB(skb)->ip_dsfield & INET_ECN_MASK) {
case INET_ECN_NOT_ECT:
/* Insure that GCN will not continue to mark packets. */
And then since that broadens the mission of this function beyond
checking just the ECT/CE bits, I supposed we could rename the
__tcp_ecn_check_ce() and tcp_ecn_check_ce() functions to
__tcp_ecn_check() and tcp_ecn_check(), or something like that.
Would that work for this particular issue?
Neal
Thanks Neal, it does work and is cleaner than what I was doing. I will submit a revised patch set.
^ 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