* Re: [PATCH] net: adaptec: starfire: add checks for dma mapping errors
From: David Miller @ 2017-01-23 17:53 UTC (permalink / raw)
To: khoroshilov; +Cc: ionut, netdev, linux-kernel, ldv-project
In-Reply-To: <1484952744-9815-1-git-send-email-khoroshilov@ispras.ru>
From: Alexey Khoroshilov <khoroshilov@ispras.ru>
Date: Sat, 21 Jan 2017 01:52:24 +0300
> init_ring() and refill_rx_ring() don't check if mapping dma memory succeed.
> The patch adds the checks and failure handling.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
The transmit side has the same exact problem, can you take care of both
of them at the same time?
Thanks.
^ permalink raw reply
* Re: [PATCH] [net-next] net: qcom/emac: rename emac_phy to emac_sgmii and move it
From: David Miller @ 2017-01-23 17:55 UTC (permalink / raw)
To: timur; +Cc: netdev, LinoSanfilippo
In-Reply-To: <1484954464-2622-1-git-send-email-timur@codeaurora.org>
From: Timur Tabi <timur@codeaurora.org>
Date: Fri, 20 Jan 2017 17:21:03 -0600
> The EMAC has an internal PHY that is often called the "SGMII". This
> SGMII is also connected to an external PHY, which is managed by phylib.
> These dual PHYs often cause confusion. In this case, the data structure
> for managing the SGMII was mis-named and located in the wrong header file.
>
> Structure emac_phy is renamed to emac_sgmii to clearly indicate it applies
> to the internal PHY only. It also also moved from emac_phy.h (which
> supports the external PHY) to emac_sgmii.h (where it belongs).
>
> To keep the changes minimal, only the structure name is changed, not
> the names of any variables of that type.
>
> Signed-off-by: Timur Tabi <timur@codeaurora.org>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] [net-next][v2] net: qcom/emac: claim the irq only when the device is opened
From: David Miller @ 2017-01-23 18:03 UTC (permalink / raw)
To: timur; +Cc: netdev, LinoSanfilippo
In-Reply-To: <1484954464-2622-2-git-send-email-timur@codeaurora.org>
From: Timur Tabi <timur@codeaurora.org>
Date: Fri, 20 Jan 2017 17:21:04 -0600
> During reset, functions emac_mac_down() and emac_mac_up() are called,
> so we don't want to free and claim the IRQ unnecessarily. Move those
> operations to open/close.
>
> Signed-off-by: Timur Tabi <timur@codeaurora.org>
> ---
>
> Notes:
> v2: keep synchronize_irq call where it is
Applied.
^ permalink raw reply
* Re: wrong smp_mb__after_atomic() in tcp_check_space() ?
From: Eric Dumazet @ 2017-01-23 18:04 UTC (permalink / raw)
To: Jason Baron
Cc: Oleg Nesterov, David S. Miller, Herbert Xu, Yauheni Kaliuta,
netdev
In-Reply-To: <f5f9c542-2b23-9df9-098c-ad4007bf7e78@akamai.com>
On Mon, 2017-01-23 at 11:56 -0500, Jason Baron wrote:
> On 01/23/2017 09:30 AM, Oleg Nesterov wrote:
> > Hello,
> >
> > smp_mb__after_atomic() looks wrong and misleading, sock_reset_flag() does the
> > non-atomic __clear_bit() and thus it can not guarantee test_bit(SOCK_NOSPACE)
> > (non-atomic too) won't be reordered.
> >
>
> Indeed. Here's a bit of discussion on it:
> http://marc.info/?l=linux-netdev&m=146662325920596&w=2
>
> > It was added by 3c7151275c0c9a "tcp: add memory barriers to write space paths"
> > and the patch looks correct in that we need the barriers in tcp_check_space()
> > and tcp_poll() in theory, so it seems tcp_check_space() needs smp_mb() ?
> >
>
> Yes, I think it should be upgraded to an smp_mb() there. If you agree
> with this analysis, I will send a patch to upgrade it. Note, I did not
> actually run into this race in practice.
SOCK_QUEUE_SHRUNK is used locally in TCP, it is not used by tcp_poll().
(Otherwise it would be using atomic set/clear operations)
I do not see obvious reason why we have this smp_mb__after_atomic() in
tcp_check_space().
But looking at this code, it seems we lack one barrier if sk_sndbuf is
ever increased. Fortunately this almost never happen during TCP session
lifetime...
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index bfa165cc455ad0a9aea44964aa663dbe6085aebd..3692e9f4c852cebf8c4d46c141f112e75e4ae66d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -331,8 +331,13 @@ static void tcp_sndbuf_expand(struct sock *sk)
sndmem = ca_ops->sndbuf_expand ? ca_ops->sndbuf_expand(sk) : 2;
sndmem *= nr_segs * per_mss;
- if (sk->sk_sndbuf < sndmem)
+ if (sk->sk_sndbuf < sndmem) {
sk->sk_sndbuf = min(sndmem, sysctl_tcp_wmem[2]);
+ /* Paired with second sk_stream_is_writeable(sk)
+ * test from tcp_poll()
+ */
+ smp_wmb();
+ }
}
/* 2. Tuning advertised window (window_clamp, rcv_ssthresh)
^ permalink raw reply related
* Re: [net-next 0/8] More MV88E6390 patches
From: David Miller @ 2017-01-23 18:06 UTC (permalink / raw)
To: andrew; +Cc: netdev, vivien.didelot
In-Reply-To: <1484955062-26718-1-git-send-email-andrew@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Sat, 21 Jan 2017 00:30:54 +0100
> This is the ongoing work to add support for the Marvell 9390 family of
> switches. There are now two MDIO busses, one for the internal PHYs and
> an external bus for external PHYs. Add support for this external bus.
I am anticipating another respin of this based upon feedback, just
FYI.
Correct me if my expectations are wrong.
Thanks.
^ permalink raw reply
* Re: [PATCH v4 net-next] net: mvneta: implement .set_wol and .get_wol
From: Andrew Lunn @ 2017-01-23 18:10 UTC (permalink / raw)
To: Jisheng Zhang; +Cc: thomas.petazzoni, davem, netdev, linux-kernel, Jingju Hou
In-Reply-To: <20170123065507.2557-1-jszhang@marvell.com>
On Mon, Jan 23, 2017 at 02:55:07PM +0800, Jisheng Zhang wrote:
> From: Jingju Hou <houjingj@marvell.com>
>
> From: Jingju Hou <houjingj@marvell.com>
>
> The mvneta itself does not support WOL, but the PHY might.
> So pass the calls to the PHY
>
> Signed-off-by: Jingju Hou <houjingj@marvell.com>
> Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> ---
> since v3:
> - really fix the build error
Keep trying....
But maybe tomorrow, after you have taken the pause Dave said you
should take, and maybe ask Jingju to really review it, in detail.
>
> since v2,v1:
> - using phy_dev member in struct net_device
> - add commit msg
>
> drivers/net/ethernet/marvell/mvneta.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 6dcc951af0ff..02611fa1c3b8 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -3929,6 +3929,25 @@ static int mvneta_ethtool_get_rxfh(struct net_device *dev, u32 *indir, u8 *key,
> return 0;
> }
>
> +static void mvneta_ethtool_get_wol(struct net_device *dev,
> + struct ethtool_wolinfo *wol)
> +{
> + wol->supported = 0;
> + wol->wolopts = 0;
> +
> + if (dev->phydev)
> + return phy_ethtool_get_wol(dev->phydev, wol);
This is a void function. And you are returning a value. And
phy_ethtool_get_wol() is also a void function, so does not actually
return anything.
0-day was telling you this. Please read what is said.
Andrew
^ permalink raw reply
* Re: [PATCH] net: ethernet: mvneta: add support for 2.5G DRSGMII mode
From: Andrew Lunn @ 2017-01-23 18:18 UTC (permalink / raw)
To: Jan Luebbe
Cc: netdev, devicetree, davem, Rob Herring, Mark Rutland,
Thomas Petazzoni, Florian Fainelli
In-Reply-To: <20170123142206.5390-1-jlu@pengutronix.de>
On Mon, Jan 23, 2017 at 03:22:06PM +0100, Jan Luebbe wrote:
> The Marvell MVNETA Ethernet controller supports a 2.5 Gbps SGMII mode
> called DRSGMII.
>
> This patch adds a corresponding phy-mode string 'drsgmii' and parses it
> from DT. The MVNETA then configures the SERDES protocol value
> accordingly.
Hi Jan
So this sets the PHY to 2.5Gbps. What about the MAC? Currently,
mvneta_adjust_link() knows about SPEED_1000 and SPEED_100. Does it now
need to know about SPEED_2500? mvneta_ethtool_set_link_ksettings()?
Thanks
Andrew
^ permalink raw reply
* Re: [PATCHv5 net-next 0/5] sctp: add sender-side procedures for stream reconf asoc reset and add streams
From: Marcelo Ricardo Leitner @ 2017-01-23 18:42 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, Vlad Yasevich, davem
In-Reply-To: <cover.1484993773.git.lucien.xin@gmail.com>
On Sat, Jan 21, 2017 at 06:24:21PM +0800, Xin Long wrote:
> Patch 3/5 is to implement sender-side procedures for the SSN/TSN Reset
> Request Parameter described in rfc6525 section 5.1.4, patch 2/5 is
> ahead of it to define a function to make the request chunk for it.
>
> Patch 5/5 is to implement sender-side procedures for the Add Incoming
> and Outgoing Streams Request Parameter Request Parameter described in
> rfc6525 section 5.1.5 and 5.1.6, patch 4/5 is ahead of it to define a
> function to make the request chunk for it.
>
> Patch 1/5 is a fix to recover streams states when it fails to send
> request.
>
> v1->v2:
> - put these into a smaller group.
> - rename some temporary variables in the codes.
> - rename the titles of the commits and improve some changelogs.
> v2->v3:
> - re-split the patchset and make sure it has no dead codes for review.
> - move some codes into stream.c from socket.c.
> v3->v4:
> - add one more patch to fix a send reset stream request issue.
> - doing actual work only when request is sent successfully.
> - reduce some indents in sctp_send_add_streams.
> v4->v5:
> - close streams before sending request and recover them when sending
> fails in patch 1/5 and patch 3/5
To make it clear, the notes on v3 patchset also apply to this one, which
are:
- usage of krealloc()
- the removal of __packed attribute
So there should be a v6 of it soon.
Thanks
>
> Xin Long (5):
> sctp: streams should be recovered when it fails to send request.
> sctp: add support for generating stream reconf ssn/tsn reset request
> chunk
> sctp: implement sender-side procedures for SSN/TSN Reset Request
> Parameter
> sctp: add support for generating stream reconf add incoming/outgoing
> streams request chunk
> sctp: implement sender-side procedures for Add Incoming/Outgoing
> Streams Request Parameter
>
> include/linux/sctp.h | 12 ++++
> include/net/sctp/sctp.h | 3 +
> include/net/sctp/sm.h | 5 ++
> include/uapi/linux/sctp.h | 8 +++
> net/sctp/sm_make_chunk.c | 75 +++++++++++++++++++++++++
> net/sctp/socket.c | 58 +++++++++++++++++++
> net/sctp/stream.c | 138 +++++++++++++++++++++++++++++++++++++++++++++-
> 7 files changed, 298 insertions(+), 1 deletion(-)
>
> --
> 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
>
^ permalink raw reply
* Re: wrong smp_mb__after_atomic() in tcp_check_space() ?
From: Jason Baron @ 2017-01-23 18:45 UTC (permalink / raw)
To: Eric Dumazet
Cc: Oleg Nesterov, David S. Miller, Herbert Xu, Yauheni Kaliuta,
netdev
In-Reply-To: <1485194671.16328.195.camel@edumazet-glaptop3.roam.corp.google.com>
On 01/23/2017 01:04 PM, Eric Dumazet wrote:
> On Mon, 2017-01-23 at 11:56 -0500, Jason Baron wrote:
>> On 01/23/2017 09:30 AM, Oleg Nesterov wrote:
>>> Hello,
>>>
>>> smp_mb__after_atomic() looks wrong and misleading, sock_reset_flag() does the
>>> non-atomic __clear_bit() and thus it can not guarantee test_bit(SOCK_NOSPACE)
>>> (non-atomic too) won't be reordered.
>>>
>>
>> Indeed. Here's a bit of discussion on it:
>> http://marc.info/?l=linux-netdev&m=146662325920596&w=2
>>
>>> It was added by 3c7151275c0c9a "tcp: add memory barriers to write space paths"
>>> and the patch looks correct in that we need the barriers in tcp_check_space()
>>> and tcp_poll() in theory, so it seems tcp_check_space() needs smp_mb() ?
>>>
>>
>> Yes, I think it should be upgraded to an smp_mb() there. If you agree
>> with this analysis, I will send a patch to upgrade it. Note, I did not
>> actually run into this race in practice.
>
> SOCK_QUEUE_SHRUNK is used locally in TCP, it is not used by tcp_poll().
>
> (Otherwise it would be using atomic set/clear operations)
>
> I do not see obvious reason why we have this smp_mb__after_atomic() in
> tcp_check_space().
>
>
The idea of the smp_mb__after_atomic() in tcp_check_space() was to
ensure that the 'read' of SOCK_NOSPACE there didn't happen before any of
the 'write' to make sk_stream_is_writeable() true. Otherwise, we could
miss doing the wakeup from tcp_check_space(). There is probably an
argument here that there will likely be a subsequent call to
tcp_check_space() that will see the SOCK_NOSPACE bit set, but in theory
we could have a small send buffer, or a lot of data could be ack'd in
one go.
What I missed in the original patch was that sock_reset_flag() isn't an
atomic operation and thus the smp_mb__after_atomic() is wrong.
> But looking at this code, it seems we lack one barrier if sk_sndbuf is
> ever increased. Fortunately this almost never happen during TCP session
> lifetime...
>
But the wakeup from sk->sk_write_space(sk) will imply a smp_wmb() as per
the comment in __wake_up() ?
Thanks,
-Jason
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index bfa165cc455ad0a9aea44964aa663dbe6085aebd..3692e9f4c852cebf8c4d46c141f112e75e4ae66d 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -331,8 +331,13 @@ static void tcp_sndbuf_expand(struct sock *sk)
> sndmem = ca_ops->sndbuf_expand ? ca_ops->sndbuf_expand(sk) : 2;
> sndmem *= nr_segs * per_mss;
>
> - if (sk->sk_sndbuf < sndmem)
> + if (sk->sk_sndbuf < sndmem) {
> sk->sk_sndbuf = min(sndmem, sysctl_tcp_wmem[2]);
> + /* Paired with second sk_stream_is_writeable(sk)
> + * test from tcp_poll()
> + */
> + smp_wmb();
> + }
> }
>
> /* 2. Tuning advertised window (window_clamp, rcv_ssthresh)
>
>
^ permalink raw reply
* Question about veth_xmit()
From: Xiangning Yu @ 2017-01-23 18:46 UTC (permalink / raw)
To: netdev
Hi netdev folks,
It looks like we call dev_forward_skb() in veth_xmit(), which calls
netif_rx() eventually.
While netif_rx() will enqueue the skb to the CPU RX backlog before the
actual processing takes place. So, this actually means a TX skb has to
wait some un-related RX skbs to finish. And this will happen twice for
a single ping, because the veth device always works as a pair?
IMHO this might lead to some latency issue under certain workload,
can we change the call to dev_forward_skb() to something like this?
if (likely(__dev_forward_skb(rcv, skb) == NET_RX_SUCCESS)) {
local_bh_disable();
netif_receive_skb(skb);
local_bh_enable();
Could you please shed some light on this change? And please feel free
to correct my if my understanding is wrong.
Thanks,
- Xiangning
^ permalink raw reply
* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
From: Marcelo Ricardo Leitner @ 2017-01-23 18:47 UTC (permalink / raw)
To: David Laight
Cc: 'Xin Long', network dev, linux-sctp@vger.kernel.org,
Neil Horman, Vlad Yasevich, davem@davemloft.net
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB026B563@AcuExch.aculab.com>
On Mon, Jan 23, 2017 at 11:25:56AM +0000, David Laight wrote:
> From: Xin Long
> > Sent: 19 January 2017 17:19
> > This patch is to implement Sender-Side Procedures for the Add
> > Outgoing and Incoming Streams Request Parameter described in
> > rfc6525 section 5.1.5-5.1.6.
> >
> > It is also to add sockopt SCTP_ADD_STREAMS in rfc6525 section
> > 6.3.4 for users.
> ...
> > + out = params->sas_outstrms;
> > + in = params->sas_instrms;
> > +
> > + if (!out && !in)
> > + goto out;
> > +
> > + if (out) {
> > + __u16 nums = stream->outcnt + out;
>
> Make nums 'unsigned int', the code will be smaller and you can
> use the value for the overflow check.
Smaller as in to avoid the sum below?
>
> > + /* Check for overflow, can't use nums here */
> > + if (stream->outcnt + out > SCTP_MAX_STREAM)
> > + goto out;
> > +
> > + /* Use ksize to check if stream array really needs to realloc */
> > + if (ksize(stream->out) / sizeof(*stream->out) < nums) {
> > + struct sctp_stream_out *streamout;
> > +
> > + streamout = kcalloc(nums, sizeof(*streamout),
> > + GFP_KERNEL);
> > + if (!streamout) {
> > + retval = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + memcpy(streamout, stream->out,
> > + sizeof(*streamout) * stream->outcnt);
> > +
> > + kfree(stream->out);
> > + stream->out = streamout;
> > + }
>
> Does kcalloc() zero the entire area, or just the length you ask for?
> If the latter you need to zero the rest here.
> ...
>
> David
>
> --
> 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
* [PATCH net-next 0/3] net/tcp-fastopen: Add new userspace API support
From: Wei Wang @ 2017-01-23 18:59 UTC (permalink / raw)
To: netdev, David Miller; +Cc: Eric Dumazet, Yuchung Cheng, Wei Wang
From: Wei Wang <weiwan@google.com>
The patch series is to add support for new userspace API for TCP fastopen
sockets.
In the current code, user has to call sendto()/sendmsg() with special flag
MSG_FASTOPEN for TCP fastopen sockets. This API is quite different from the
normal TCP socket API and can be cumbersome for applications to make use
fastopen sockets.
So this new patch introduces a new way of using TCP fastopen sockets which
is similar to normal TCP sockets with a new sockopt TCP_FASTOPEN_CONNECT.
More details about it is described in the third patch.
(First 2 patches are preparations for the third patch.)
Wei Wang (3):
net/tcp-fastopen: refactor cookie check logic
net: Remove __sk_dst_reset() in tcp_v6_connect()
net/tcp-fastopen: Add new API support
include/linux/tcp.h | 3 ++-
include/net/inet_sock.h | 6 +++++-
include/net/tcp.h | 3 +++
include/uapi/linux/tcp.h | 1 +
net/ipv4/af_inet.c | 31 ++++++++++++++++++++-------
net/ipv4/tcp.c | 35 ++++++++++++++++++++++++++++++-
net/ipv4/tcp_fastopen.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
net/ipv4/tcp_ipv4.c | 7 ++++++-
net/ipv4/tcp_output.c | 16 ++------------
net/ipv6/tcp_ipv6.c | 6 +++++-
10 files changed, 136 insertions(+), 26 deletions(-)
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply
* Re: fs, net: deadlock between bind/splice on af_unix
From: Cong Wang @ 2017-01-23 19:00 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Al Viro, linux-fsdevel@vger.kernel.org, LKML, David Miller,
Rainer Weikusat, Hannes Frederic Sowa, netdev, Eric Dumazet,
syzkaller
In-Reply-To: <CACT4Y+Yf5S1OiNdd8WH=bmge=25m3zOh48QB8iq2nsKESG6Z_Q@mail.gmail.com>
On Fri, Jan 20, 2017 at 2:52 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>
> This works! I did not see the deadlock warning, nor any other related crashes.
>
> Tested-by: Dmitry Vyukov <dvyukov@google.com>
Thanks for verifying it. I will send it out formally soon.
^ permalink raw reply
* [PATCH net-next 1/3] net/tcp-fastopen: refactor cookie check logic
From: Wei Wang @ 2017-01-23 18:59 UTC (permalink / raw)
To: netdev, David Miller; +Cc: Eric Dumazet, Yuchung Cheng, Wei Wang
In-Reply-To: <20170123185922.48046-1-tracywwnj@gmail.com>
From: Wei Wang <weiwan@google.com>
Refactor the cookie check logic in tcp_send_syn_data() into a function.
This function will be called else where in later changes.
Signed-off-by: Wei Wang <weiwan@google.com>
---
include/net/tcp.h | 2 ++
net/ipv4/tcp_fastopen.c | 21 +++++++++++++++++++++
net/ipv4/tcp_output.c | 16 ++--------------
3 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index c55d65f74f7f..de67541d7adf 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1493,6 +1493,8 @@ struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
struct tcp_fastopen_cookie *foc,
struct dst_entry *dst);
void tcp_fastopen_init_key_once(bool publish);
+bool tcp_fastopen_cookie_check(struct sock *sk, u16 *mss,
+ struct tcp_fastopen_cookie *cookie);
#define TCP_FASTOPEN_KEY_LENGTH 16
/* Fastopen key context */
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index f51919535ca7..f90e09e1ff4c 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -325,3 +325,24 @@ struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
*foc = valid_foc;
return NULL;
}
+
+bool tcp_fastopen_cookie_check(struct sock *sk, u16 *mss,
+ struct tcp_fastopen_cookie *cookie)
+{
+ unsigned long last_syn_loss = 0;
+ int syn_loss = 0;
+
+ tcp_fastopen_cache_get(sk, mss, cookie, &syn_loss, &last_syn_loss);
+
+ /* Recurring FO SYN losses: no cookie or data in SYN */
+ if (syn_loss > 1 &&
+ time_before(jiffies, last_syn_loss + (60*HZ << syn_loss))) {
+ cookie->len = -1;
+ return false;
+ }
+ if (sysctl_tcp_fastopen & TFO_CLIENT_NO_COOKIE) {
+ cookie->len = -1;
+ return true;
+ }
+ return cookie->len > 0;
+}
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 9a1a1494b9dd..671c69535671 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3267,23 +3267,11 @@ static int tcp_send_syn_data(struct sock *sk, struct sk_buff *syn)
{
struct tcp_sock *tp = tcp_sk(sk);
struct tcp_fastopen_request *fo = tp->fastopen_req;
- int syn_loss = 0, space, err = 0;
- unsigned long last_syn_loss = 0;
+ int space, err = 0;
struct sk_buff *syn_data;
tp->rx_opt.mss_clamp = tp->advmss; /* If MSS is not cached */
- tcp_fastopen_cache_get(sk, &tp->rx_opt.mss_clamp, &fo->cookie,
- &syn_loss, &last_syn_loss);
- /* Recurring FO SYN losses: revert to regular handshake temporarily */
- if (syn_loss > 1 &&
- time_before(jiffies, last_syn_loss + (60*HZ << syn_loss))) {
- fo->cookie.len = -1;
- goto fallback;
- }
-
- if (sysctl_tcp_fastopen & TFO_CLIENT_NO_COOKIE)
- fo->cookie.len = -1;
- else if (fo->cookie.len <= 0)
+ if (!tcp_fastopen_cookie_check(sk, &tp->rx_opt.mss_clamp, &fo->cookie))
goto fallback;
/* MSS for SYN-data is based on cached MSS and bounded by PMTU and
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH net-next 2/3] net: Remove __sk_dst_reset() in tcp_v6_connect()
From: Wei Wang @ 2017-01-23 18:59 UTC (permalink / raw)
To: netdev, David Miller; +Cc: Eric Dumazet, Yuchung Cheng, Wei Wang
In-Reply-To: <20170123185922.48046-1-tracywwnj@gmail.com>
From: Wei Wang <weiwan@google.com>
Remove __sk_dst_reset() in the failure handling because __sk_dst_reset()
will eventually get called when sk is released. No need to handle it in
the protocol specific connect call.
This is also to make the code path consistent with ipv4.
Signed-off-by: Wei Wang <weiwan@google.com>
---
net/ipv6/tcp_ipv6.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index f72100eedd5d..0b7cd3d009b6 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -295,7 +295,6 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
late_failure:
tcp_set_state(sk, TCP_CLOSE);
- __sk_dst_reset(sk);
failure:
inet->inet_dport = 0;
sk->sk_route_caps = 0;
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [PATCH net-next 3/3] net/tcp-fastopen: Add new API support
From: Wei Wang @ 2017-01-23 18:59 UTC (permalink / raw)
To: netdev, David Miller; +Cc: Eric Dumazet, Yuchung Cheng, Wei Wang
In-Reply-To: <20170123185922.48046-1-tracywwnj@gmail.com>
From: Wei Wang <weiwan@google.com>
This patch adds a new socket option, TCP_FASTOPEN_CONNECT, as an
alternative way to perform Fast Open on the active side (client). Prior
to this patch, a client needs to replace the connect() call with
sendto(MSG_FASTOPEN). This can be cumbersome for applications who want
to use Fast Open: these socket operations are often done in lower layer
libraries used by many other applications. Changing these libraries
and/or the socket call sequences are not trivial. A more convenient
approach is to perform Fast Open by simply enabling a socket option when
the socket is created w/o changing other socket calls sequence:
s = socket()
create a new socket
setsockopt(s, IPPROTO_TCP, TCP_FASTOPEN_CONNECT …);
newly introduced sockopt
If set, new functionality described below will be used.
Return ENOTSUPP if TFO is not supported or not enabled in the
kernel.
connect()
With cookie present, return 0 immediately.
With no cookie, initiate 3WHS with TFO cookie-request option and
return -1 with errno = EINPROGRESS.
write()/sendmsg()
With cookie present, send out SYN with data and return the number of
bytes buffered.
With no cookie, and 3WHS not yet completed, return -1 with errno =
EINPROGRESS.
No MSG_FASTOPEN flag is needed.
read()
Return -1 with errno = EWOULDBLOCK/EAGAIN if connect() is called but
write() is not called yet.
Return -1 with errno = EWOULDBLOCK/EAGAIN if connection is
established but no msg is received yet.
Return number of bytes read if socket is established and there is
msg received.
The new API simplifies life for applications that always perform a write()
immediately after a successful connect(). Such applications can now take
advantage of Fast Open by merely making one new setsockopt() call at the time
of creating the socket. Nothing else about the application's socket call
sequence needs to change.
Signed-off-by: Wei Wang <weiwan@google.com>
---
include/linux/tcp.h | 3 ++-
include/net/inet_sock.h | 6 +++++-
include/net/tcp.h | 1 +
include/uapi/linux/tcp.h | 1 +
net/ipv4/af_inet.c | 31 ++++++++++++++++++++++++-------
net/ipv4/tcp.c | 35 ++++++++++++++++++++++++++++++++++-
net/ipv4/tcp_fastopen.c | 33 +++++++++++++++++++++++++++++++++
net/ipv4/tcp_ipv4.c | 7 ++++++-
net/ipv6/tcp_ipv6.c | 5 +++++
9 files changed, 111 insertions(+), 11 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 5371b3d70cfe..f88f4649ba6f 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -222,7 +222,8 @@ struct tcp_sock {
u32 chrono_stat[3]; /* Time in jiffies for chrono_stat stats */
u8 chrono_type:2, /* current chronograph type */
rate_app_limited:1, /* rate_{delivered,interval_us} limited? */
- unused:5;
+ fastopen_connect:1, /* FASTOPEN_CONNECT sockopt */
+ unused:4;
u8 nonagle : 4,/* Disable Nagle algorithm? */
thin_lto : 1,/* Use linear timeouts for thin streams */
unused1 : 1,
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index c9cff977a7fb..aa95053dfc78 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -206,7 +206,11 @@ struct inet_sock {
transparent:1,
mc_all:1,
nodefrag:1;
- __u8 bind_address_no_port:1;
+ __u8 bind_address_no_port:1,
+ defer_connect:1; /* Indicates that fastopen_connect is set
+ * and cookie exists so we defer connect
+ * until first data frame is written
+ */
__u8 rcv_tos;
__u8 convert_csum;
int uc_index;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index de67541d7adf..6ec4ea652f3f 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1495,6 +1495,7 @@ struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
void tcp_fastopen_init_key_once(bool publish);
bool tcp_fastopen_cookie_check(struct sock *sk, u16 *mss,
struct tcp_fastopen_cookie *cookie);
+bool tcp_fastopen_defer_connect(struct sock *sk, int *err);
#define TCP_FASTOPEN_KEY_LENGTH 16
/* Fastopen key context */
diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
index c53de2691cec..6ff35eb48d10 100644
--- a/include/uapi/linux/tcp.h
+++ b/include/uapi/linux/tcp.h
@@ -116,6 +116,7 @@ enum {
#define TCP_SAVE_SYN 27 /* Record SYN headers for new connections */
#define TCP_SAVED_SYN 28 /* Get SYN headers recorded for connection */
#define TCP_REPAIR_WINDOW 29 /* Get/set window parameters */
+#define TCP_FASTOPEN_CONNECT 30 /* Attempt FastOpen with connect */
struct tcp_repair_opt {
__u32 opt_code;
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index aae410bb655a..d8a0dc076f97 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -576,13 +576,24 @@ int __inet_stream_connect(struct socket *sock, struct sockaddr *uaddr,
int err;
long timeo;
- if (addr_len < sizeof(uaddr->sa_family))
- return -EINVAL;
+ /*
+ * uaddr can be NULL and addr_len can be 0 if:
+ * sk is a TCP fastopen active socket and
+ * TCP_FASTOPEN_CONNECT sockopt is set and
+ * we already have a valid cookie for this socket.
+ * In this case, user can call write() after connect().
+ * write() will invoke tcp_sendmsg_fastopen() which calls
+ * __inet_stream_connect().
+ */
+ if (uaddr) {
+ if (addr_len < sizeof(uaddr->sa_family))
+ return -EINVAL;
- if (uaddr->sa_family == AF_UNSPEC) {
- err = sk->sk_prot->disconnect(sk, flags);
- sock->state = err ? SS_DISCONNECTING : SS_UNCONNECTED;
- goto out;
+ if (uaddr->sa_family == AF_UNSPEC) {
+ err = sk->sk_prot->disconnect(sk, flags);
+ sock->state = err ? SS_DISCONNECTING : SS_UNCONNECTED;
+ goto out;
+ }
}
switch (sock->state) {
@@ -593,7 +604,10 @@ int __inet_stream_connect(struct socket *sock, struct sockaddr *uaddr,
err = -EISCONN;
goto out;
case SS_CONNECTING:
- err = -EALREADY;
+ if (inet_sk(sk)->defer_connect)
+ err = -EINPROGRESS;
+ else
+ err = -EALREADY;
/* Fall out of switch with err, set for this state */
break;
case SS_UNCONNECTED:
@@ -607,6 +621,9 @@ int __inet_stream_connect(struct socket *sock, struct sockaddr *uaddr,
sock->state = SS_CONNECTING;
+ if (!err && inet_sk(sk)->defer_connect)
+ goto out;
+
/* Just entered SS_CONNECTING state; the only
* difference is that return value in non-blocking
* case is EINPROGRESS, rather than EALREADY.
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index c43eb1a831d7..d9735b76d073 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -533,6 +533,12 @@ unsigned int tcp_poll(struct file *file, struct socket *sock, poll_table *wait)
if (tp->urg_data & TCP_URG_VALID)
mask |= POLLPRI;
+ } else if (sk->sk_state == TCP_SYN_SENT && inet_sk(sk)->defer_connect) {
+ /* Active TCP fastopen socket with defer_connect
+ * Return POLLOUT so application can call write()
+ * in order for kernel to generate SYN+data
+ */
+ mask |= POLLOUT | POLLWRNORM;
}
/* This barrier is coupled with smp_wmb() in tcp_reset() */
smp_rmb();
@@ -1071,6 +1077,7 @@ static int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
int *copied, size_t size)
{
struct tcp_sock *tp = tcp_sk(sk);
+ struct inet_sock *inet = inet_sk(sk);
int err, flags;
if (!(sysctl_tcp_fastopen & TFO_CLIENT_ENABLE))
@@ -1085,9 +1092,19 @@ static int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
tp->fastopen_req->data = msg;
tp->fastopen_req->size = size;
+ if (inet->defer_connect) {
+ err = tcp_connect(sk);
+ /* Same failure procedure as in tcp_v4/6_connect */
+ if (err) {
+ tcp_set_state(sk, TCP_CLOSE);
+ inet->inet_dport = 0;
+ sk->sk_route_caps = 0;
+ }
+ }
flags = (msg->msg_flags & MSG_DONTWAIT) ? O_NONBLOCK : 0;
err = __inet_stream_connect(sk->sk_socket, msg->msg_name,
msg->msg_namelen, flags);
+ inet->defer_connect = 0;
*copied = tp->fastopen_req->copied;
tcp_free_fastopen_req(tp);
return err;
@@ -1107,7 +1124,7 @@ int tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
lock_sock(sk);
flags = msg->msg_flags;
- if (flags & MSG_FASTOPEN) {
+ if (unlikely(flags & MSG_FASTOPEN || inet_sk(sk)->defer_connect)) {
err = tcp_sendmsg_fastopen(sk, msg, &copied_syn, size);
if (err == -EINPROGRESS && copied_syn > 0)
goto out;
@@ -2656,6 +2673,18 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
err = -EINVAL;
}
break;
+ case TCP_FASTOPEN_CONNECT:
+ if (val > 1 || val < 0) {
+ err = -EINVAL;
+ } else if (sysctl_tcp_fastopen & TFO_CLIENT_ENABLE) {
+ if (sk->sk_state == TCP_CLOSE)
+ tp->fastopen_connect = val;
+ else
+ err = -EINVAL;
+ } else {
+ err = -EOPNOTSUPP;
+ }
+ break;
case TCP_TIMESTAMP:
if (!tp->repair)
err = -EPERM;
@@ -3016,6 +3045,10 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
val = icsk->icsk_accept_queue.fastopenq.max_qlen;
break;
+ case TCP_FASTOPEN_CONNECT:
+ val = tp->fastopen_connect;
+ break;
+
case TCP_TIMESTAMP:
val = tcp_time_stamp + tp->tsoffset;
break;
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index f90e09e1ff4c..9674bec4a0f8 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -346,3 +346,36 @@ bool tcp_fastopen_cookie_check(struct sock *sk, u16 *mss,
}
return cookie->len > 0;
}
+
+/* This function checks if we want to defer sending SYN until the first
+ * write(). We defer under the following conditions:
+ * 1. fastopen_connect sockopt is set
+ * 2. we have a valid cookie
+ * Return value: return true if we want to defer until application writes data
+ * return false if we want to send out SYN immediately
+ */
+bool tcp_fastopen_defer_connect(struct sock *sk, int *err)
+{
+ struct tcp_fastopen_cookie cookie = { .len = 0 };
+ struct tcp_sock *tp = tcp_sk(sk);
+ u16 mss;
+
+ if (tp->fastopen_connect && !tp->fastopen_req) {
+ if (tcp_fastopen_cookie_check(sk, &mss, &cookie)) {
+ inet_sk(sk)->defer_connect = 1;
+ return true;
+ }
+
+ /* Alloc fastopen_req in order for FO option to be included
+ * in SYN
+ */
+ tp->fastopen_req = kzalloc(sizeof(*tp->fastopen_req),
+ sk->sk_allocation);
+ if (tp->fastopen_req)
+ tp->fastopen_req->cookie = cookie;
+ else
+ *err = -ENOBUFS;
+ }
+ return false;
+}
+EXPORT_SYMBOL(tcp_fastopen_defer_connect);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index f7325b25b06e..27b726c96459 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -232,6 +232,7 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
/* OK, now commit destination to socket. */
sk->sk_gso_type = SKB_GSO_TCPV4;
sk_setup_caps(sk, &rt->dst);
+ rt = NULL;
if (!tp->write_seq && likely(!tp->repair))
tp->write_seq = secure_tcp_sequence_number(inet->inet_saddr,
@@ -242,9 +243,13 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
inet->inet_id = tp->write_seq ^ jiffies;
+ if (tcp_fastopen_defer_connect(sk, &err))
+ return err;
+ if (err)
+ goto failure;
+
err = tcp_connect(sk);
- rt = NULL;
if (err)
goto failure;
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 0b7cd3d009b6..95c05e5293b1 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -287,6 +287,11 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
inet->inet_dport,
&tp->tsoffset);
+ if (tcp_fastopen_defer_connect(sk, &err))
+ return err;
+ if (err)
+ goto late_failure;
+
err = tcp_connect(sk);
if (err)
goto late_failure;
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* Re: Question about veth_xmit()
From: Eric Dumazet @ 2017-01-23 19:07 UTC (permalink / raw)
To: Xiangning Yu; +Cc: netdev
In-Reply-To: <CAOwmpL3vg56ZqHaYVBy+9dJJ1=GCbgy-m8NY0JrNCV-Z42R_eg@mail.gmail.com>
On Mon, 2017-01-23 at 10:46 -0800, Xiangning Yu wrote:
> Hi netdev folks,
>
> It looks like we call dev_forward_skb() in veth_xmit(), which calls
> netif_rx() eventually.
>
> While netif_rx() will enqueue the skb to the CPU RX backlog before the
> actual processing takes place. So, this actually means a TX skb has to
> wait some un-related RX skbs to finish. And this will happen twice for
> a single ping, because the veth device always works as a pair?
>
> IMHO this might lead to some latency issue under certain workload,
> can we change the call to dev_forward_skb() to something like this?
>
> if (likely(__dev_forward_skb(rcv, skb) == NET_RX_SUCCESS)) {
> local_bh_disable();
> netif_receive_skb(skb);
> local_bh_enable();
>
> Could you please shed some light on this change? And please feel free
> to correct my if my understanding is wrong.
How veth would have different latency requirement than loopback device ?
Calling netif_receive_skb() is considered too dangerous here (or from
any ndo_start_xmit()) because of possible kernel stack exhaustion.
^ permalink raw reply
* Re: [PATCH net-next 1/3] net/tcp-fastopen: refactor cookie check logic
From: Eric Dumazet @ 2017-01-23 19:13 UTC (permalink / raw)
To: Wei Wang; +Cc: netdev, David Miller, Eric Dumazet, Yuchung Cheng, Wei Wang
In-Reply-To: <20170123185922.48046-2-tracywwnj@gmail.com>
On Mon, 2017-01-23 at 10:59 -0800, Wei Wang wrote:
> From: Wei Wang <weiwan@google.com>
>
> Refactor the cookie check logic in tcp_send_syn_data() into a function.
> This function will be called else where in later changes.
>
> Signed-off-by: Wei Wang <weiwan@google.com>
> ---
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: Remove __sk_dst_reset() in tcp_v6_connect()
From: Eric Dumazet @ 2017-01-23 19:14 UTC (permalink / raw)
To: Wei Wang; +Cc: netdev, David Miller, Eric Dumazet, Yuchung Cheng, Wei Wang
In-Reply-To: <20170123185922.48046-3-tracywwnj@gmail.com>
On Mon, 2017-01-23 at 10:59 -0800, Wei Wang wrote:
> From: Wei Wang <weiwan@google.com>
>
> Remove __sk_dst_reset() in the failure handling because __sk_dst_reset()
> will eventually get called when sk is released. No need to handle it in
> the protocol specific connect call.
> This is also to make the code path consistent with ipv4.
>
> Signed-off-by: Wei Wang <weiwan@google.com>
> ---
Suggested-by: Eric Dumazet <edumazet@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support
From: Eric Dumazet @ 2017-01-23 19:15 UTC (permalink / raw)
To: Wei Wang; +Cc: netdev, David Miller, Eric Dumazet, Yuchung Cheng, Wei Wang
In-Reply-To: <20170123185922.48046-4-tracywwnj@gmail.com>
On Mon, 2017-01-23 at 10:59 -0800, Wei Wang wrote:
> From: Wei Wang <weiwan@google.com>
>
> This patch adds a new socket option, TCP_FASTOPEN_CONNECT, as an
> alternative way to perform Fast Open on the active side (client). Prior
> to this patch, a client needs to replace the connect() call with
> sendto(MSG_FASTOPEN). This can be cumbersome for applications who want
> to use Fast Open: these socket operations are often done in lower layer
> libraries used by many other applications. Changing these libraries
> and/or the socket call sequences are not trivial. A more convenient
> approach is to perform Fast Open by simply enabling a socket option when
> the socket is created w/o changing other socket calls sequence:
> Signed-off-by: Wei Wang <weiwan@google.com>
> ---
Thanks for this hard work Wei.
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [Patch net] af_unix: move unix_mknod() out of bindlock
From: Cong Wang @ 2017-01-23 19:17 UTC (permalink / raw)
To: netdev; +Cc: dvyukov, Cong Wang, Rainer Weikusat, Al Viro
Dmitry reported a deadlock scenario:
unix_bind() path:
u->bindlock ==> sb_writer
do_splice() path:
sb_writer ==> pipe->mutex ==> u->bindlock
In the unix_bind() code path, unix_mknod() does not have to
be done with u->bindlock held, since it is a pure fs operation,
so we can just move unix_mknod() out.
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Tested-by: Dmitry Vyukov <dvyukov@google.com>
Cc: Rainer Weikusat <rweikusat@mobileactivedefense.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/unix/af_unix.c | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 127656e..cef7987 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -995,6 +995,7 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
unsigned int hash;
struct unix_address *addr;
struct hlist_head *list;
+ struct path path = { NULL, NULL };
err = -EINVAL;
if (sunaddr->sun_family != AF_UNIX)
@@ -1010,9 +1011,20 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
goto out;
addr_len = err;
+ if (sun_path[0]) {
+ umode_t mode = S_IFSOCK |
+ (SOCK_INODE(sock)->i_mode & ~current_umask());
+ err = unix_mknod(sun_path, mode, &path);
+ if (err) {
+ if (err == -EEXIST)
+ err = -EADDRINUSE;
+ goto out;
+ }
+ }
+
err = mutex_lock_interruptible(&u->bindlock);
if (err)
- goto out;
+ goto out_put;
err = -EINVAL;
if (u->addr)
@@ -1029,16 +1041,6 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
atomic_set(&addr->refcnt, 1);
if (sun_path[0]) {
- struct path path;
- umode_t mode = S_IFSOCK |
- (SOCK_INODE(sock)->i_mode & ~current_umask());
- err = unix_mknod(sun_path, mode, &path);
- if (err) {
- if (err == -EEXIST)
- err = -EADDRINUSE;
- unix_release_addr(addr);
- goto out_up;
- }
addr->hash = UNIX_HASH_SIZE;
hash = d_backing_inode(path.dentry)->i_ino & (UNIX_HASH_SIZE - 1);
spin_lock(&unix_table_lock);
@@ -1065,6 +1067,9 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
spin_unlock(&unix_table_lock);
out_up:
mutex_unlock(&u->bindlock);
+out_put:
+ if (err)
+ path_put(&path);
out:
return err;
}
--
2.5.5
^ permalink raw reply related
* Re: [net PATCH v5 6/6] virtio_net: XDP support for adjust_head
From: Michael S. Tsirkin @ 2017-01-23 19:22 UTC (permalink / raw)
To: John Fastabend
Cc: jasowang, john.r.fastabend, netdev, alexei.starovoitov, daniel
In-Reply-To: <20170117222259.20280.24625.stgit@john-Precision-Tower-5810>
On Tue, Jan 17, 2017 at 02:22:59PM -0800, John Fastabend wrote:
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 62dbf4b..3b129b4 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -41,6 +41,9 @@
> #define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
> #define GOOD_COPY_LEN 128
>
> +/* Amount of XDP headroom to prepend to packets for use by xdp_adjust_head */
> +#define VIRTIO_XDP_HEADROOM 256
> +
> /* RX packet size EWMA. The average packet size is used to determine the packet
> * buffer size when refilling RX rings. As the entire RX ring may be refilled
> * at once, the weight is chosen so that the EWMA will be insensitive to short-
I wonder where does this number come from? This is quite a lot and
means that using XDP_PASS will slow down any sockets on top of it.
Which in turn means people will try to remove XDP when not in use,
causing resets. E.g. build_skb (which I have a patch to switch to) uses
a much more reasonable NET_SKB_PAD.
--
MST
^ permalink raw reply
* Re: [PATCH 3/3] powerpc: bpf: implement in-register swap for 64-bit endian operations
From: 'Naveen N. Rao' @ 2017-01-23 19:22 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: David Laight, netdev@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, davem@davemloft.net,
daniel@iogearbox.net, ast@fb.com, Madhavan Srinivasan,
Michael Ellerman
In-Reply-To: <1484492458.11927.17.camel@au1.ibm.com>
On 2017/01/15 09:00AM, Benjamin Herrenschmidt wrote:
> On Fri, 2017-01-13 at 23:22 +0530, 'Naveen N. Rao' wrote:
> > > That rather depends on whether the processor has a store to load forwarder
> > > that will satisfy the read from the store buffer.
> > > I don't know about ppc, but at least some x86 will do that.
> >
> > Interesting - good to know that.
> >
> > However, I don't think powerpc does that and in-register swap is likely
> > faster regardless. Note also that gcc prefers this form at higher
> > optimization levels.
>
> Of course powerpc has a load-store forwarder these days, however, I
> wouldn't be surprised if the in-register form was still faster on some
> implementations, but this needs to be tested.
Thanks for clarifying! To test this, I wrote a simple (perhaps naive)
test that just issues a whole lot of endian swaps and in _that_ test, it
does look like the load-store forwarder is doing pretty well.
The tests:
bpf-bswap.S:
-----------
.file "bpf-bswap.S"
.abiversion 2
.section ".text"
.align 2
.globl main
.type main, @function
main:
mflr 0
std 0,16(1)
stdu 1,-32760(1)
addi 3,1,32
li 4,0
li 5,32720
li 11,32720
mulli 11,11,8
li 10,0
li 7,16
1: ldx 6,3,4
stdx 6,1,7
ldbrx 6,1,7
stdx 6,3,4
addi 4,4,8
cmpd 4,5
beq 2f
b 1b
2: addi 10,10,1
li 4,0
cmpd 10,11
beq 3f
b 1b
3: li 3,0
addi 1,1,32760
ld 0,16(1)
mtlr 0
blr
bpf-bswap-reg.S:
---------------
.file "bpf-bswap-reg.S"
.abiversion 2
.section ".text"
.align 2
.globl main
.type main, @function
main:
mflr 0
std 0,16(1)
stdu 1,-32760(1)
addi 3,1,32
li 4,0
li 5,32720
li 11,32720
mulli 11,11,8
li 10,0
1: ldx 6,3,4
rldicl 7,6,32,32
rlwinm 8,6,24,0,31
rlwimi 8,6,8,8,15
rlwinm 9,7,24,0,31
rlwimi 8,6,8,24,31
rlwimi 9,7,8,8,15
rlwimi 9,7,8,24,31
rldicr 8,8,32,31
or 6,8,9
stdx 6,3,4
addi 4,4,8
cmpd 4,5
beq 2f
b 1b
2: addi 10,10,1
li 4,0
cmpd 10,11
beq 3f
b 1b
3: li 3,0
addi 1,1,32760
ld 0,16(1)
mtlr 0
blr
Profiling the two variants:
# perf stat ./bpf-bswap
Performance counter stats for './bpf-bswap':
1395.979224 task-clock (msec) # 0.999 CPUs utilized
0 context-switches # 0.000 K/sec
0 cpu-migrations # 0.000 K/sec
45 page-faults # 0.032 K/sec
4,651,874,673 cycles # 3.332 GHz (66.87%)
3,141,186 stalled-cycles-frontend # 0.07% frontend cycles idle (50.57%)
1,117,289,485 stalled-cycles-backend # 24.02% backend cycles idle (50.57%)
8,565,963,861 instructions # 1.84 insn per cycle
# 0.13 stalled cycles per insn (67.05%)
2,174,029,771 branches # 1557.351 M/sec (49.69%)
262,656 branch-misses # 0.01% of all branches (50.05%)
1.396893189 seconds time elapsed
# perf stat ./bpf-bswap-reg
Performance counter stats for './bpf-bswap-reg':
1819.758102 task-clock (msec) # 0.999 CPUs utilized
3 context-switches # 0.002 K/sec
0 cpu-migrations # 0.000 K/sec
44 page-faults # 0.024 K/sec
6,034,777,602 cycles # 3.316 GHz (66.83%)
2,010,983 stalled-cycles-frontend # 0.03% frontend cycles idle (50.47%)
1,024,975,759 stalled-cycles-backend # 16.98% backend cycles idle (50.52%)
16,043,732,849 instructions # 2.66 insn per cycle
# 0.06 stalled cycles per insn (67.01%)
2,148,710,750 branches # 1180.767 M/sec (49.57%)
268,046 branch-misses # 0.01% of all branches (49.52%)
1.821501345 seconds time elapsed
This is all in a POWER8 vm. On POWER7, the in-register variant is around
4 times faster than the ldbrx variant.
So, yes, unless I've missed something, the ldbrx variant seems to
perform better, if not on par with the in-register swap variant on
POWER8.
>
> Ideally, you'd want to try to "optimize" load+swap or swap+store
> though.
Agreed. This is already the case with BPF for packet access - those use
skb helpers which issue the appropriate lhbrx/lwbrx/ldbrx. The newer
BPF_FROM_LE/BPF_FROM_BE are for endian operations with other BPF
programs.
We can probably implement an extra pass to detect use of endian swap and
try to match it up with a previous load or a subsequent store though...
Thanks!
- Naveen
^ permalink raw reply
* Re: Question about veth_xmit()
From: Xiangning Yu @ 2017-01-23 19:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1485198462.16328.208.camel@edumazet-glaptop3.roam.corp.google.com>
On Mon, Jan 23, 2017 at 11:07 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2017-01-23 at 10:46 -0800, Xiangning Yu wrote:
>> Hi netdev folks,
>>
>> It looks like we call dev_forward_skb() in veth_xmit(), which calls
>> netif_rx() eventually.
>>
>> While netif_rx() will enqueue the skb to the CPU RX backlog before the
>> actual processing takes place. So, this actually means a TX skb has to
>> wait some un-related RX skbs to finish. And this will happen twice for
>> a single ping, because the veth device always works as a pair?
>>
>> IMHO this might lead to some latency issue under certain workload,
>> can we change the call to dev_forward_skb() to something like this?
>>
>> if (likely(__dev_forward_skb(rcv, skb) == NET_RX_SUCCESS)) {
>> local_bh_disable();
>> netif_receive_skb(skb);
>> local_bh_enable();
>>
>> Could you please shed some light on this change? And please feel free
>> to correct my if my understanding is wrong.
>
> How veth would have different latency requirement than loopback device ?
>
The traffic from those veth device will reach external network, and
normally those are RPC type traffic.
> Calling netif_receive_skb() is considered too dangerous here (or from
> any ndo_start_xmit()) because of possible kernel stack exhaustion.
>
I agree, stack space is a concern, especially if the traffic is
loopback-ed to another namespace.
Thanks,
- Xiangning
>
>
^ permalink raw reply
* [PATCH v2] virtio_net: fix PAGE_SIZE > 64k
From: Michael S. Tsirkin @ 2017-01-23 19:37 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, John Fastabend, virtualization
I don't have any guests with PAGE_SIZE > 64k but the
code seems to be clearly broken in that case
as PAGE_SIZE / MERGEABLE_BUFFER_ALIGN will need
more than 8 bit and so the code in mergeable_ctx_to_buf_address
does not give us the actual true size.
Cc: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
changes from v1:
fix build warnings
drivers/net/virtio_net.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 4a10500..4dc373b 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -48,8 +48,16 @@ module_param(gso, bool, 0444);
*/
DECLARE_EWMA(pkt_len, 1, 64)
+/* With mergeable buffers we align buffer address and use the low bits to
+ * encode its true size. Buffer size is up to 1 page so we need to align to
+ * square root of page size to ensure we reserve enough bits to encode the true
+ * size.
+ */
+#define MERGEABLE_BUFFER_MIN_ALIGN_SHIFT ((PAGE_SHIFT + 1) / 2)
+
/* Minimum alignment for mergeable packet buffers. */
-#define MERGEABLE_BUFFER_ALIGN max(L1_CACHE_BYTES, 256)
+#define MERGEABLE_BUFFER_ALIGN max(L1_CACHE_BYTES, \
+ 1 << MERGEABLE_BUFFER_MIN_ALIGN_SHIFT)
#define VIRTNET_DRIVER_VERSION "1.0.0"
--
MST
^ permalink raw reply related
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