* [PATCH net-next 3/3] tipc: redesign connection-level flow control
From: Jon Maloy @ 2016-05-02 14:22 UTC (permalink / raw)
To: davem; +Cc: Jon Maloy, netdev, Paul Gortmaker, tipc-discussion
In-Reply-To: <1462198956-30181-1-git-send-email-jon.maloy@ericsson.com>
There are two flow control mechanisms in TIPC; one at link level that
handles network congestion, burst control, and retransmission, and one
at connection level which' only remaining task is to prevent overflow
in the receiving socket buffer. In TIPC, the latter task has to be
solved end-to-end because messages can not be thrown away once they
have been accepted and delivered upwards from the link layer, i.e, we
can never permit the receive buffer to overflow.
Currently, this algorithm is message based. A counter in the receiving
socket keeps track of number of consumed messages, and sends a dedicated
acknowledge message back to the sender for each 256 consumed message.
A counter at the sending end keeps track of the sent, not yet
acknowledged messages, and blocks the sender if this number ever reaches
512 unacknowledged messages. When the missing acknowledge arrives, the
socket is then woken up for renewed transmission. This works well for
keeping the message flow running, as it almost never happens that a
sender socket is blocked this way.
A problem with the current mechanism is that it potentially is very
memory consuming. Since we don't distinguish bewteen small and large
messages, we have to dimension the socket receive buffer according
to a worst-case of both. I.e., the window size must be chosen large
enough to sustain a reasonable throughput even for the smallest
messages, while we must still consider a scenario where all messages
are of maximum size. Hence, the current fix window size of 512 messages
and a maximum message size of 66k results in a receive buffer of 66 MB
when truesize(66k) = 131k is taken into account. It is possible to do
much better.
This commit introduces an algorithm where we instead use 1024-byte
blocks as base unit. This unit, always rounded upwards from the
actual message size, is used when we advertise windows as well as when
we count and acknowledge transmitted data. The advertised window is
based on the configured receive buffer size in such a way that even
the worst-case truesize/msgsize ratio always is covered. Since the
smallest possible message size (from a flow control viewpoint) now is
1024 bytes, we can safely assume this ratio to be less than four, which
is the value we are now using.
This way, we have been able to reduce the default receive buffer size
from 66 MB to 2 MB with maintained performance.
In order to keep this solution backwards compatible, we introduce a
new capability bit in the discovery protocol, and use this throughout
the message sending/reception path to always select the right unit.
Acked-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/core.c | 8 ++--
net/tipc/msg.h | 14 +++++-
net/tipc/node.h | 5 +-
net/tipc/socket.c | 140 +++++++++++++++++++++++++++++++++++-------------------
net/tipc/socket.h | 17 +++++--
5 files changed, 122 insertions(+), 62 deletions(-)
diff --git a/net/tipc/core.c b/net/tipc/core.c
index e2bdb07a..fe1b062 100644
--- a/net/tipc/core.c
+++ b/net/tipc/core.c
@@ -112,11 +112,9 @@ static int __init tipc_init(void)
pr_info("Activated (version " TIPC_MOD_VER ")\n");
- sysctl_tipc_rmem[0] = TIPC_CONN_OVERLOAD_LIMIT >> 4 <<
- TIPC_LOW_IMPORTANCE;
- sysctl_tipc_rmem[1] = TIPC_CONN_OVERLOAD_LIMIT >> 4 <<
- TIPC_CRITICAL_IMPORTANCE;
- sysctl_tipc_rmem[2] = TIPC_CONN_OVERLOAD_LIMIT;
+ sysctl_tipc_rmem[0] = RCVBUF_MIN;
+ sysctl_tipc_rmem[1] = RCVBUF_DEF;
+ sysctl_tipc_rmem[2] = RCVBUF_MAX;
err = tipc_netlink_start();
if (err)
diff --git a/net/tipc/msg.h b/net/tipc/msg.h
index 58bf515..024da8a 100644
--- a/net/tipc/msg.h
+++ b/net/tipc/msg.h
@@ -743,16 +743,26 @@ static inline void msg_set_msgcnt(struct tipc_msg *m, u16 n)
msg_set_bits(m, 9, 16, 0xffff, n);
}
-static inline u32 msg_bcast_tag(struct tipc_msg *m)
+static inline u32 msg_conn_ack(struct tipc_msg *m)
{
return msg_bits(m, 9, 16, 0xffff);
}
-static inline void msg_set_bcast_tag(struct tipc_msg *m, u32 n)
+static inline void msg_set_conn_ack(struct tipc_msg *m, u32 n)
{
msg_set_bits(m, 9, 16, 0xffff, n);
}
+static inline u32 msg_adv_win(struct tipc_msg *m)
+{
+ return msg_bits(m, 9, 0, 0xffff);
+}
+
+static inline void msg_set_adv_win(struct tipc_msg *m, u32 n)
+{
+ msg_set_bits(m, 9, 0, 0xffff, n);
+}
+
static inline u32 msg_max_pkt(struct tipc_msg *m)
{
return msg_bits(m, 9, 16, 0xffff) * 4;
diff --git a/net/tipc/node.h b/net/tipc/node.h
index 1823768..8264b3d 100644
--- a/net/tipc/node.h
+++ b/net/tipc/node.h
@@ -45,10 +45,11 @@
/* Optional capabilities supported by this code version
*/
enum {
- TIPC_BCAST_SYNCH = (1 << 1)
+ TIPC_BCAST_SYNCH = (1 << 1),
+ TIPC_BLOCK_FLOWCTL = (2 << 1)
};
-#define TIPC_NODE_CAPABILITIES TIPC_BCAST_SYNCH
+#define TIPC_NODE_CAPABILITIES (TIPC_BCAST_SYNCH | TIPC_BLOCK_FLOWCTL)
#define INVALID_BEARER_ID -1
void tipc_node_stop(struct net *net);
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 94bd286..1262889 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -96,9 +96,11 @@ struct tipc_sock {
uint conn_timeout;
atomic_t dupl_rcvcnt;
bool link_cong;
- uint sent_unacked;
- uint rcv_unacked;
+ u16 snt_unacked;
+ u16 snd_win;
u16 peer_caps;
+ u16 rcv_unacked;
+ u16 rcv_win;
struct sockaddr_tipc remote;
struct rhash_head node;
struct rcu_head rcu;
@@ -228,9 +230,29 @@ static struct tipc_sock *tipc_sk(const struct sock *sk)
return container_of(sk, struct tipc_sock, sk);
}
-static int tsk_conn_cong(struct tipc_sock *tsk)
+static bool tsk_conn_cong(struct tipc_sock *tsk)
{
- return tsk->sent_unacked >= TIPC_FLOWCTRL_WIN;
+ return tsk->snt_unacked >= tsk->snd_win;
+}
+
+/* tsk_blocks(): translate a buffer size in bytes to number of
+ * advertisable blocks, taking into account the ratio truesize(len)/len
+ * We can trust that this ratio is always < 4 for len >= FLOWCTL_BLK_SZ
+ */
+static u16 tsk_adv_blocks(int len)
+{
+ return len / FLOWCTL_BLK_SZ / 4;
+}
+
+/* tsk_inc(): increment counter for sent or received data
+ * - If block based flow control is not supported by peer we
+ * fall back to message based ditto, incrementing the counter
+ */
+static u16 tsk_inc(struct tipc_sock *tsk, int msglen)
+{
+ if (likely(tsk->peer_caps & TIPC_BLOCK_FLOWCTL))
+ return ((msglen / FLOWCTL_BLK_SZ) + 1);
+ return 1;
}
/**
@@ -378,9 +400,12 @@ static int tipc_sk_create(struct net *net, struct socket *sock,
sk->sk_write_space = tipc_write_space;
sk->sk_destruct = tipc_sock_destruct;
tsk->conn_timeout = CONN_TIMEOUT_DEFAULT;
- tsk->sent_unacked = 0;
atomic_set(&tsk->dupl_rcvcnt, 0);
+ /* Start out with safe limits until we receive an advertised window */
+ tsk->snd_win = tsk_adv_blocks(RCVBUF_MIN);
+ tsk->rcv_win = tsk->snd_win;
+
if (sock->state == SS_READY) {
tsk_set_unreturnable(tsk, true);
if (sock->type == SOCK_DGRAM)
@@ -776,7 +801,7 @@ static void tipc_sk_proto_rcv(struct tipc_sock *tsk, struct sk_buff *skb)
struct sock *sk = &tsk->sk;
struct tipc_msg *hdr = buf_msg(skb);
int mtyp = msg_type(hdr);
- int conn_cong;
+ bool conn_cong;
/* Ignore if connection cannot be validated: */
if (!tsk_peer_msg(tsk, hdr))
@@ -790,7 +815,9 @@ static void tipc_sk_proto_rcv(struct tipc_sock *tsk, struct sk_buff *skb)
return;
} else if (mtyp == CONN_ACK) {
conn_cong = tsk_conn_cong(tsk);
- tsk->sent_unacked -= msg_msgcnt(hdr);
+ tsk->snt_unacked -= msg_conn_ack(hdr);
+ if (tsk->peer_caps & TIPC_BLOCK_FLOWCTL)
+ tsk->snd_win = msg_adv_win(hdr);
if (conn_cong)
sk->sk_write_space(sk);
} else if (mtyp != CONN_PROBE_REPLY) {
@@ -1021,12 +1048,14 @@ static int __tipc_send_stream(struct socket *sock, struct msghdr *m, size_t dsz)
u32 dnode;
uint mtu, send, sent = 0;
struct iov_iter save;
+ int hlen = MIN_H_SIZE;
/* Handle implied connection establishment */
if (unlikely(dest)) {
rc = __tipc_sendmsg(sock, m, dsz);
+ hlen = msg_hdr_sz(mhdr);
if (dsz && (dsz == rc))
- tsk->sent_unacked = 1;
+ tsk->snt_unacked = tsk_inc(tsk, dsz + hlen);
return rc;
}
if (dsz > (uint)INT_MAX)
@@ -1055,7 +1084,7 @@ next:
if (likely(!tsk_conn_cong(tsk))) {
rc = tipc_node_xmit(net, &pktchain, dnode, portid);
if (likely(!rc)) {
- tsk->sent_unacked++;
+ tsk->snt_unacked += tsk_inc(tsk, send + hlen);
sent += send;
if (sent == dsz)
return dsz;
@@ -1120,6 +1149,12 @@ static void tipc_sk_finish_conn(struct tipc_sock *tsk, u32 peer_port,
tipc_node_add_conn(net, peer_node, tsk->portid, peer_port);
tsk->max_pkt = tipc_node_get_mtu(net, peer_node, tsk->portid);
tsk->peer_caps = tipc_node_get_capabilities(net, peer_node);
+ if (tsk->peer_caps & TIPC_BLOCK_FLOWCTL)
+ return;
+
+ /* Fall back to message based flow control */
+ tsk->rcv_win = FLOWCTL_MSG_WIN;
+ tsk->snd_win = FLOWCTL_MSG_WIN;
}
/**
@@ -1216,7 +1251,7 @@ static int tipc_sk_anc_data_recv(struct msghdr *m, struct tipc_msg *msg,
return 0;
}
-static void tipc_sk_send_ack(struct tipc_sock *tsk, uint ack)
+static void tipc_sk_send_ack(struct tipc_sock *tsk)
{
struct net *net = sock_net(&tsk->sk);
struct sk_buff *skb = NULL;
@@ -1232,7 +1267,14 @@ static void tipc_sk_send_ack(struct tipc_sock *tsk, uint ack)
if (!skb)
return;
msg = buf_msg(skb);
- msg_set_msgcnt(msg, ack);
+ msg_set_conn_ack(msg, tsk->rcv_unacked);
+ tsk->rcv_unacked = 0;
+
+ /* Adjust to and advertize the correct window limit */
+ if (tsk->peer_caps & TIPC_BLOCK_FLOWCTL) {
+ tsk->rcv_win = tsk_adv_blocks(tsk->sk.sk_rcvbuf);
+ msg_set_adv_win(msg, tsk->rcv_win);
+ }
tipc_node_xmit_skb(net, skb, dnode, msg_link_selector(msg));
}
@@ -1290,7 +1332,7 @@ static int tipc_recvmsg(struct socket *sock, struct msghdr *m, size_t buf_len,
long timeo;
unsigned int sz;
u32 err;
- int res;
+ int res, hlen;
/* Catch invalid receive requests */
if (unlikely(!buf_len))
@@ -1315,6 +1357,7 @@ restart:
buf = skb_peek(&sk->sk_receive_queue);
msg = buf_msg(buf);
sz = msg_data_sz(msg);
+ hlen = msg_hdr_sz(msg);
err = msg_errcode(msg);
/* Discard an empty non-errored message & try again */
@@ -1337,7 +1380,7 @@ restart:
sz = buf_len;
m->msg_flags |= MSG_TRUNC;
}
- res = skb_copy_datagram_msg(buf, msg_hdr_sz(msg), m, sz);
+ res = skb_copy_datagram_msg(buf, hlen, m, sz);
if (res)
goto exit;
res = sz;
@@ -1349,15 +1392,15 @@ restart:
res = -ECONNRESET;
}
- /* Consume received message (optional) */
- if (likely(!(flags & MSG_PEEK))) {
- if ((sock->state != SS_READY) &&
- (++tsk->rcv_unacked >= TIPC_CONNACK_INTV)) {
- tipc_sk_send_ack(tsk, tsk->rcv_unacked);
- tsk->rcv_unacked = 0;
- }
- tsk_advance_rx_queue(sk);
+ if (unlikely(flags & MSG_PEEK))
+ goto exit;
+
+ if (likely(sock->state != SS_READY)) {
+ tsk->rcv_unacked += tsk_inc(tsk, hlen + sz);
+ if (unlikely(tsk->rcv_unacked >= (tsk->rcv_win / 4)))
+ tipc_sk_send_ack(tsk);
}
+ tsk_advance_rx_queue(sk);
exit:
release_sock(sk);
return res;
@@ -1386,7 +1429,7 @@ static int tipc_recv_stream(struct socket *sock, struct msghdr *m,
int sz_to_copy, target, needed;
int sz_copied = 0;
u32 err;
- int res = 0;
+ int res = 0, hlen;
/* Catch invalid receive attempts */
if (unlikely(!buf_len))
@@ -1412,6 +1455,7 @@ restart:
buf = skb_peek(&sk->sk_receive_queue);
msg = buf_msg(buf);
sz = msg_data_sz(msg);
+ hlen = msg_hdr_sz(msg);
err = msg_errcode(msg);
/* Discard an empty non-errored message & try again */
@@ -1436,8 +1480,7 @@ restart:
needed = (buf_len - sz_copied);
sz_to_copy = (sz <= needed) ? sz : needed;
- res = skb_copy_datagram_msg(buf, msg_hdr_sz(msg) + offset,
- m, sz_to_copy);
+ res = skb_copy_datagram_msg(buf, hlen + offset, m, sz_to_copy);
if (res)
goto exit;
@@ -1459,20 +1502,18 @@ restart:
res = -ECONNRESET;
}
- /* Consume received message (optional) */
- if (likely(!(flags & MSG_PEEK))) {
- if (unlikely(++tsk->rcv_unacked >= TIPC_CONNACK_INTV)) {
- tipc_sk_send_ack(tsk, tsk->rcv_unacked);
- tsk->rcv_unacked = 0;
- }
- tsk_advance_rx_queue(sk);
- }
+ if (unlikely(flags & MSG_PEEK))
+ goto exit;
+
+ tsk->rcv_unacked += tsk_inc(tsk, hlen + sz);
+ if (unlikely(tsk->rcv_unacked >= (tsk->rcv_win / 4)))
+ tipc_sk_send_ack(tsk);
+ tsk_advance_rx_queue(sk);
/* Loop around if more data is required */
if ((sz_copied < buf_len) && /* didn't get all requested data */
(!skb_queue_empty(&sk->sk_receive_queue) ||
(sz_copied < target)) && /* and more is ready or required */
- (!(flags & MSG_PEEK)) && /* and aren't just peeking at data */
(!err)) /* and haven't reached a FIN */
goto restart;
@@ -1604,30 +1645,33 @@ static bool filter_connect(struct tipc_sock *tsk, struct sk_buff *skb)
/**
* rcvbuf_limit - get proper overload limit of socket receive queue
* @sk: socket
- * @buf: message
+ * @skb: message
*
- * For all connection oriented messages, irrespective of importance,
- * the default overload value (i.e. 67MB) is set as limit.
+ * For connection oriented messages, irrespective of importance,
+ * default queue limit is 2 MB.
*
- * For all connectionless messages, by default new queue limits are
- * as belows:
+ * For connectionless messages, queue limits are based on message
+ * importance as follows:
*
- * TIPC_LOW_IMPORTANCE (4 MB)
- * TIPC_MEDIUM_IMPORTANCE (8 MB)
- * TIPC_HIGH_IMPORTANCE (16 MB)
- * TIPC_CRITICAL_IMPORTANCE (32 MB)
+ * TIPC_LOW_IMPORTANCE (2 MB)
+ * TIPC_MEDIUM_IMPORTANCE (4 MB)
+ * TIPC_HIGH_IMPORTANCE (8 MB)
+ * TIPC_CRITICAL_IMPORTANCE (16 MB)
*
* Returns overload limit according to corresponding message importance
*/
-static unsigned int rcvbuf_limit(struct sock *sk, struct sk_buff *buf)
+static unsigned int rcvbuf_limit(struct sock *sk, struct sk_buff *skb)
{
- struct tipc_msg *msg = buf_msg(buf);
+ struct tipc_sock *tsk = tipc_sk(sk);
+ struct tipc_msg *hdr = buf_msg(skb);
+
+ if (unlikely(!msg_connected(hdr)))
+ return sk->sk_rcvbuf << msg_importance(hdr);
- if (msg_connected(msg))
- return sysctl_tipc_rmem[2];
+ if (likely(tsk->peer_caps & TIPC_BLOCK_FLOWCTL))
+ return sk->sk_rcvbuf;
- return sk->sk_rcvbuf >> TIPC_CRITICAL_IMPORTANCE <<
- msg_importance(msg);
+ return FLOWCTL_MSG_LIM;
}
/**
diff --git a/net/tipc/socket.h b/net/tipc/socket.h
index 4241f22..06fb594 100644
--- a/net/tipc/socket.h
+++ b/net/tipc/socket.h
@@ -1,6 +1,6 @@
/* net/tipc/socket.h: Include file for TIPC socket code
*
- * Copyright (c) 2014-2015, Ericsson AB
+ * Copyright (c) 2014-2016, Ericsson AB
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -38,10 +38,17 @@
#include <net/sock.h>
#include <net/genetlink.h>
-#define TIPC_CONNACK_INTV 256
-#define TIPC_FLOWCTRL_WIN (TIPC_CONNACK_INTV * 2)
-#define TIPC_CONN_OVERLOAD_LIMIT ((TIPC_FLOWCTRL_WIN * 2 + 1) * \
- SKB_TRUESIZE(TIPC_MAX_USER_MSG_SIZE))
+/* Compatibility values for deprecated message based flow control */
+#define FLOWCTL_MSG_WIN 512
+#define FLOWCTL_MSG_LIM ((FLOWCTL_MSG_WIN * 2 + 1) * SKB_TRUESIZE(MAX_MSG_SIZE))
+
+#define FLOWCTL_BLK_SZ 1024
+
+/* Socket receive buffer sizes */
+#define RCVBUF_MIN (FLOWCTL_BLK_SZ * 512)
+#define RCVBUF_DEF (FLOWCTL_BLK_SZ * 1024 * 2)
+#define RCVBUF_MAX (FLOWCTL_BLK_SZ * 1024 * 16)
+
int tipc_socket_init(void);
void tipc_socket_stop(void);
void tipc_sk_rcv(struct net *net, struct sk_buff_head *inputq);
--
1.9.1
------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
^ permalink raw reply related
* Re: [PATCH net-next] fq_codel: add batch ability to fq_codel_drop()
From: Eric Dumazet @ 2016-05-02 14:34 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: David Miller, netdev, Dave Taht, Jonathan Morton
In-Reply-To: <20160502094954.24cc9549@redhat.com>
On Mon, 2016-05-02 at 09:49 +0200, Jesper Dangaard Brouer wrote:
> What about using bulk free of SKBs here?
>
> There is a very high probability that we are hitting SLUB slowpath,
> which involves an expensive locked cmpxchg_double per packet. Instead
> we can amortize this cost via kmem_cache_free_bulk().
>
> Maybe extend kfree_skb_list() to hide the slab/kmem_cache call?
Sounds tricky, because of skb destructors. skb are complex objects.
For each skb, need to free the frags, skb->head, and skb.
^ permalink raw reply
* [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger
From: João Paulo Rechi Vita @ 2016-05-02 14:39 UTC (permalink / raw)
To: Johannes Berg
Cc: David S. Miller, Darren Hart, linux-wireless, netdev,
platform-driver-x86, linux-api, linux-doc, linux-kernel, linux,
João Paulo Rechi Vita, João Paulo Rechi Vita
In-Reply-To: <1462199948-6424-1-git-send-email-jprvita@endlessm.com>
From: João Paulo Rechi Vita <jprvita@gmail.com>
This creates a new LED trigger to be used by platform drivers as a
default trigger for airplane-mode indicator LEDs.
By default this trigger will fire when RFKILL_OP_CHANGE_ALL is called
for all types (RFKILL_TYPE_ALL), setting the LED brightness to LED_FULL
when the changing the state to blocked, and to LED_OFF when the changing
the state to unblocked. In the future there will be a mechanism for
userspace to override the default policy, so it can implement its own.
This trigger will be used by the asus-wireless x86 platform driver.
Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
---
Documentation/rfkill.txt | 2 ++
net/rfkill/core.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index 1f0c270..b13025a 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -85,6 +85,8 @@ device). Don't do this unless you cannot get the event in any other way.
RFKill provides per-switch LED triggers, which can be used to drive LEDs
according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
+An airplane-mode indicator LED trigger is also available, which triggers
+LED_FULL when all radios known by RFKill are blocked, and LED_OFF otherwise.
5. Userspace support
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 884027f..9adf95e 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -126,6 +126,30 @@ static bool rfkill_epo_lock_active;
#ifdef CONFIG_RFKILL_LEDS
+static struct led_trigger rfkill_apm_led_trigger;
+
+static void rfkill_apm_led_trigger_event(bool state)
+{
+ led_trigger_event(&rfkill_apm_led_trigger, state ? LED_FULL : LED_OFF);
+}
+
+static void rfkill_apm_led_trigger_activate(struct led_classdev *led)
+{
+ rfkill_apm_led_trigger_event(!rfkill_default_state);
+}
+
+static int rfkill_apm_led_trigger_register(void)
+{
+ rfkill_apm_led_trigger.name = "rfkill-airplane-mode";
+ rfkill_apm_led_trigger.activate = rfkill_apm_led_trigger_activate;
+ return led_trigger_register(&rfkill_apm_led_trigger);
+}
+
+static void rfkill_apm_led_trigger_unregister(void)
+{
+ led_trigger_unregister(&rfkill_apm_led_trigger);
+}
+
static void rfkill_led_trigger_event(struct rfkill *rfkill)
{
struct led_trigger *trigger;
@@ -177,6 +201,19 @@ static void rfkill_led_trigger_unregister(struct rfkill *rfkill)
led_trigger_unregister(&rfkill->led_trigger);
}
#else
+static void rfkill_apm_led_trigger_event(bool state)
+{
+}
+
+static int rfkill_apm_led_trigger_register(void)
+{
+ return 0;
+}
+
+static void rfkill_apm_led_trigger_unregister(void)
+{
+}
+
static void rfkill_led_trigger_event(struct rfkill *rfkill)
{
}
@@ -313,6 +350,7 @@ static void rfkill_update_global_state(enum rfkill_type type, bool blocked)
for (i = 0; i < NUM_RFKILL_TYPES; i++)
rfkill_global_states[i].cur = blocked;
+ rfkill_apm_led_trigger_event(blocked);
}
#ifdef CONFIG_RFKILL_INPUT
@@ -1262,15 +1300,22 @@ static int __init rfkill_init(void)
{
int error;
+ error = rfkill_apm_led_trigger_register();
+ if (error)
+ goto out;
+
rfkill_update_global_state(RFKILL_TYPE_ALL, !rfkill_default_state);
error = class_register(&rfkill_class);
- if (error)
+ if (error) {
+ rfkill_apm_led_trigger_unregister();
goto out;
+ }
error = misc_register(&rfkill_miscdev);
if (error) {
class_unregister(&rfkill_class);
+ rfkill_apm_led_trigger_unregister();
goto out;
}
@@ -1279,6 +1324,7 @@ static int __init rfkill_init(void)
if (error) {
misc_deregister(&rfkill_miscdev);
class_unregister(&rfkill_class);
+ rfkill_apm_led_trigger_unregister();
goto out;
}
#endif
@@ -1295,5 +1341,6 @@ static void __exit rfkill_exit(void)
#endif
misc_deregister(&rfkill_miscdev);
class_unregister(&rfkill_class);
+ rfkill_apm_led_trigger_unregister();
}
module_exit(rfkill_exit);
--
2.5.0
^ permalink raw reply related
* [RESEND PATCH 2/3] rfkill: Userspace control for airplane mode
From: João Paulo Rechi Vita @ 2016-05-02 14:39 UTC (permalink / raw)
To: Johannes Berg
Cc: David S. Miller, Darren Hart, linux-wireless, netdev,
platform-driver-x86, linux-api, linux-doc, linux-kernel, linux,
João Paulo Rechi Vita, João Paulo Rechi Vita
In-Reply-To: <1462199948-6424-1-git-send-email-jprvita@endlessm.com>
From: João Paulo Rechi Vita <jprvita@gmail.com>
Provide an interface for the airplane-mode indicator be controlled from
userspace. User has to first acquire the control through
RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE and keep the fd open for the
whole time it wants to be in control of the indicator. Closing the fd
restores the default policy.
To change state of the indicator, the
RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE operation is used, passing the
value on "struct rfkill_event.soft". If the caller has not acquired the
airplane-mode control beforehand, the operation fails.
Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
---
Documentation/rfkill.txt | 10 ++++++++++
include/uapi/linux/rfkill.h | 6 ++++++
net/rfkill/core.c | 40 ++++++++++++++++++++++++++++++++++++++--
3 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index b13025a..9dbe3fc 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -87,6 +87,7 @@ RFKill provides per-switch LED triggers, which can be used to drive LEDs
according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
An airplane-mode indicator LED trigger is also available, which triggers
LED_FULL when all radios known by RFKill are blocked, and LED_OFF otherwise.
+The airplane-mode indicator LED trigger policy can be overridden by userspace.
5. Userspace support
@@ -123,5 +124,14 @@ RFKILL_TYPE
The contents of these variables corresponds to the "name", "state" and
"type" sysfs files explained above.
+Userspace can also override the default airplane-mode indicator policy through
+/dev/rfkill. Control of the airplane mode indicator has to be acquired first,
+using RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE, and is only available for one
+userspace application at a time. Closing the fd reverts the airplane-mode
+indicator back to the default kernel policy and makes it available for other
+applications to take control. Changes to the airplane-mode indicator state can
+be made using RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE, passing the new value
+in the 'soft' field of 'struct rfkill_event'.
+
For further details consult Documentation/ABI/stable/sysfs-class-rfkill.
diff --git a/include/uapi/linux/rfkill.h b/include/uapi/linux/rfkill.h
index 2e00dce..36e0770 100644
--- a/include/uapi/linux/rfkill.h
+++ b/include/uapi/linux/rfkill.h
@@ -61,12 +61,18 @@ enum rfkill_type {
* @RFKILL_OP_CHANGE_ALL: userspace changes all devices (of a type, or all)
* into a state, also updating the default state used for devices that
* are hot-plugged later.
+ * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE: userspace acquires control of
+ * the airplane-mode indicator.
+ * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE: userspace changes the
+ * airplane-mode indicator state.
*/
enum rfkill_operation {
RFKILL_OP_ADD = 0,
RFKILL_OP_DEL,
RFKILL_OP_CHANGE,
RFKILL_OP_CHANGE_ALL,
+ RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE,
+ RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE,
};
/**
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 9adf95e..95824b3 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -89,6 +89,7 @@ struct rfkill_data {
struct mutex mtx;
wait_queue_head_t read_wait;
bool input_handler;
+ bool is_apm_owner;
};
@@ -123,7 +124,7 @@ static struct {
} rfkill_global_states[NUM_RFKILL_TYPES];
static bool rfkill_epo_lock_active;
-
+static bool rfkill_apm_owned;
#ifdef CONFIG_RFKILL_LEDS
static struct led_trigger rfkill_apm_led_trigger;
@@ -350,7 +351,8 @@ static void rfkill_update_global_state(enum rfkill_type type, bool blocked)
for (i = 0; i < NUM_RFKILL_TYPES; i++)
rfkill_global_states[i].cur = blocked;
- rfkill_apm_led_trigger_event(blocked);
+ if (!rfkill_apm_owned)
+ rfkill_apm_led_trigger_event(blocked);
}
#ifdef CONFIG_RFKILL_INPUT
@@ -1174,9 +1176,23 @@ static ssize_t rfkill_fop_read(struct file *file, char __user *buf,
return ret;
}
+static int rfkill_airplane_mode_release(struct rfkill_data *data)
+{
+ bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
+
+ if (rfkill_apm_owned && data->is_apm_owner) {
+ rfkill_apm_owned = false;
+ data->is_apm_owner = false;
+ rfkill_apm_led_trigger_event(state);
+ return 0;
+ }
+ return -EACCES;
+}
+
static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
size_t count, loff_t *pos)
{
+ struct rfkill_data *data = file->private_data;
struct rfkill *rfkill;
struct rfkill_event ev;
int ret;
@@ -1216,6 +1232,25 @@ static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
rfkill_set_block(rfkill, ev.soft);
ret = 0;
break;
+ case RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE:
+ if (rfkill_apm_owned && !data->is_apm_owner) {
+ ret = -EACCES;
+ break;
+ }
+
+ rfkill_apm_owned = true;
+ data->is_apm_owner = true;
+ ret = 0;
+ break;
+ case RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE:
+ if (!rfkill_apm_owned || !data->is_apm_owner) {
+ ret = -EACCES;
+ break;
+ }
+
+ rfkill_apm_led_trigger_event(ev.soft);
+ ret = 0;
+ break;
default:
ret = -EINVAL;
break;
@@ -1232,6 +1267,7 @@ static int rfkill_fop_release(struct inode *inode, struct file *file)
struct rfkill_int_event *ev, *tmp;
mutex_lock(&rfkill_global_mutex);
+ rfkill_airplane_mode_release(data);
list_del(&data->list);
mutex_unlock(&rfkill_global_mutex);
--
2.5.0
^ permalink raw reply related
* [RESEND PATCH 0/3] RFKill airplane-mode indicator
From: João Paulo Rechi Vita @ 2016-05-02 14:39 UTC (permalink / raw)
To: Johannes Berg
Cc: David S. Miller, Darren Hart, linux-wireless, netdev,
platform-driver-x86, linux-api, linux-doc, linux-kernel, linux,
João Paulo Rechi Vita
This series implements an airplane-mode indicator LED trigger, which can be
used by platform drivers. By default the trigger fires on RFKILL_OP_CHANGE_ALL,
but this policy can be overwritten by userspace using the new operations
_AIRPLANE_MODE_INDICATOR_ACQUIRE and _AIRPLANE_MODE_INDICATOR_CHANGE. When the
airplane-mode indicator state changes, userspace gets notifications through the
RFKill control misc device (/dev/rfkill). I also have patches to the rfkill
userspace tool that makes use of this interface, so it can serve as API usage
example and to do quick checks on a running system, which I'll send in a
separate series.
After some hiatus I found the time to get back to this, and this time I've used
Jouni's hwsim suite to test the patches on top of mac80211-next/master. Lines
containing rfkill are pasted bellow:
START wext_rfkill 14/1880
PASS wext_rfkill 4.100002 2016-04-29 12:30:23.792682
START rfkill_wpas 571/1880
PASS rfkill_wpas 1.245307 2016-04-29 12:48:51.804344
START rfkill_autogo 572/1880
PASS rfkill_autogo 1.154174 2016-04-29 12:48:52.959605
START rfkill_p2p_discovery 573/1880
PASS rfkill_p2p_discovery 0.534903 2016-04-29 12:48:53.495547
START rfkill_open 574/1880
PASS rfkill_open 0.34073 2016-04-29 12:48:53.836963
START rfkill_p2p_discovery_p2p_dev 575/1880
PASS rfkill_p2p_discovery_p2p_dev 1.159446 2016-04-29 12:48:54.997555
START rfkill_hostapd 576/1880
PASS rfkill_hostapd 3.686868 2016-04-29 12:48:58.685162
START rfkill_wpa2_psk 577/1880
PASS rfkill_wpa2_psk 0.330014 2016-04-29 12:48:59.016711
João Paulo Rechi Vita (3):
rfkill: Create "rfkill-airplane-mode" LED trigger
rfkill: Userspace control for airplane mode
rfkill: Notify userspace of airplane-mode state changes
Documentation/rfkill.txt | 15 +++++++
include/uapi/linux/rfkill.h | 6 +++
net/rfkill/core.c | 100 +++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 119 insertions(+), 2 deletions(-)
--
2.5.0
^ permalink raw reply
* [RESEND PATCH 3/3] rfkill: Notify userspace of airplane-mode state changes
From: João Paulo Rechi Vita @ 2016-05-02 14:39 UTC (permalink / raw)
To: Johannes Berg
Cc: David S. Miller, Darren Hart, linux-wireless, netdev,
platform-driver-x86, linux-api, linux-doc, linux-kernel, linux,
João Paulo Rechi Vita, João Paulo Rechi Vita
In-Reply-To: <1462199948-6424-1-git-send-email-jprvita@endlessm.com>
From: João Paulo Rechi Vita <jprvita@gmail.com>
Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
---
Documentation/rfkill.txt | 3 +++
include/uapi/linux/rfkill.h | 4 ++--
net/rfkill/core.c | 13 +++++++++++++
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index 9dbe3fc..588b4bf 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -133,5 +133,8 @@ applications to take control. Changes to the airplane-mode indicator state can
be made using RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE, passing the new value
in the 'soft' field of 'struct rfkill_event'.
+This same API is also used to provide userspace with notifications of changes
+to airplane-mode indicator state.
+
For further details consult Documentation/ABI/stable/sysfs-class-rfkill.
diff --git a/include/uapi/linux/rfkill.h b/include/uapi/linux/rfkill.h
index 36e0770..2ccb02f 100644
--- a/include/uapi/linux/rfkill.h
+++ b/include/uapi/linux/rfkill.h
@@ -63,8 +63,8 @@ enum rfkill_type {
* are hot-plugged later.
* @RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE: userspace acquires control of
* the airplane-mode indicator.
- * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE: userspace changes the
- * airplane-mode indicator state.
+ * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE: the airplane-mode indicator state
+ * changed -- userspace changes the airplane-mode indicator state.
*/
enum rfkill_operation {
RFKILL_OP_ADD = 0,
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 95824b3..c4bbd19 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -131,7 +131,20 @@ static struct led_trigger rfkill_apm_led_trigger;
static void rfkill_apm_led_trigger_event(bool state)
{
+ struct rfkill_data *data;
+ struct rfkill_int_event *ev;
+
led_trigger_event(&rfkill_apm_led_trigger, state ? LED_FULL : LED_OFF);
+
+ list_for_each_entry(data, &rfkill_fds, list) {
+ ev = kzalloc(sizeof(*ev), GFP_KERNEL);
+ if (!ev)
+ continue;
+ ev->ev.op = RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE;
+ ev->ev.soft = state;
+ list_add_tail(&ev->list, &data->events);
+ wake_up_interruptible(&data->read_wait);
+ }
}
static void rfkill_apm_led_trigger_activate(struct led_classdev *led)
--
2.5.0
^ permalink raw reply related
* Re: [PATCH] rtlwifi:rtl_watchdog_wq_callback: fix calling rtl_lps_enter|rtl_lps_leave in opposite condition
From: Larry Finger @ 2016-05-02 15:40 UTC (permalink / raw)
To: Wang YanQing, kvalo, linux-wireless, netdev, linux-kernel
In-Reply-To: <20160502053754.GA30313@udknight>
On 05/02/2016 12:37 AM, Wang YanQing wrote:
> Commit a269913c52ad37952a4d9953bb6d748f7299c304
> ("rtlwifi: Rework rtl_lps_leave() and rtl_lps_enter() to use work queue")
> make a mistake, change the meaning of num_tx|rx_inperiod comparison test.
>
> Commit fd09ff958777cf583d7541f180991c0fc50bd2f7
> ("rtlwifi: Remove extra workqueue for enter/leave power state")
> follow previous mistake, bring us to current code.
>
> This patch fix it.
>
> Signed-off-by: Wang YanQing <udknight@gmail.com>
> ---
> I think this patch should be ported back to stable kernels, 3.10+.
> In my machine, I will lost wifi connection after minutes if I enable
> fwlps.
>
> drivers/net/wireless/realtek/rtlwifi/base.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/base.c b/drivers/net/wireless/realtek/rtlwifi/base.c
> index c74eb13..264466f 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/base.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/base.c
> @@ -1660,9 +1660,9 @@ void rtl_watchdog_wq_callback(void *data)
> if (((rtlpriv->link_info.num_rx_inperiod +
> rtlpriv->link_info.num_tx_inperiod) > 8) ||
> (rtlpriv->link_info.num_rx_inperiod > 2))
> - rtl_lps_enter(hw);
> - else
> rtl_lps_leave(hw);
> + else
> + rtl_lps_enter(hw);
> }
>
> rtlpriv->link_info.num_rx_inperiod = 0;
>
NACK
This patch is correct. There is a logic error in entering/exiting power-save
mode. Thus the code part is OK; however, the subject and commit message need to
be improved. If I had prepared this patch, my subject would have been "rtlwifi:
Fix logic error in enter/exit power-save mode". For the commit message, I would
have used the following:
In commit fd09ff958777 ("rtlwifi: Remove extra workqueue for enter/leave power
state"), the tests for enter/exit power-save mode were inverted. With this
change applied, the wifi connection becomes much more stable.
Fixes: fd09ff958777 ("rtlwifi: Remove extra workqueue for enter/leave power state")
Signed-off-by: Wang YanQing <udknight@gmail.com>
CC: Stable <stable@vger.kernel.org> [3.10+]
---
Thanks for finding this problem.
Larry
^ permalink raw reply
* Re: [net-next PATCH v2 5/9] mlx4: Add support for UDP tunnel segmentation with outer checksum offload
From: Alexander Duyck @ 2016-05-02 15:41 UTC (permalink / raw)
To: Or Gerlitz
Cc: Alexander Duyck, talal@mellanox.com, Linux Netdev List,
Michael Chan, David Miller, Gal Pressman, Or Gerlitz,
Eran Ben Elisha
In-Reply-To: <CAJ3xEMj_Vez_pP0ZTVATR=AMNDF1fneZ3dVrfWT7NP1mj4APRQ@mail.gmail.com>
On Mon, May 2, 2016 at 12:19 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> On Mon, May 2, 2016 at 5:25 AM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Sun, May 1, 2016 at 1:35 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>> On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
>>>> This patch assumes that the mlx4 hardware will ignore existing IPv4/v6
>>>> header fields for length and checksum as well as the length and checksum
>>>> fields for outer UDP headers.
>
>>> I see now the above text appearing in bunch of similar commit of
>>> yours, specifically to Intel drivers and mlx5... could you please
>>> elaborate a bit more what you mean here and what are the practical
>>> consequences of that characteristics?
>
>>> I got that right NETIF_F_GSO_UDP_TUNNEL_CSUM means that the HW can do
>>> segmentation for TCP packets encapsulated by UDP tunnel e.g VXLAN
>>> where the outer checksum is not zero. AFAIK, any other outer checksum
>>> value can't correctly be a constant... are you assuming here RCO or
>>> LCO?
>
>> Actually it is really easy for outer UDP checksum to be constant as
>> long as we keep the length of all segments constant. This all ties
>> back into LCO. As long as the fields between the start of the UDP
>> header and the start of the TCP header are either constant, as in the
>> case of IPv6, or have their own checksum as in the case of IPv4 we
>> will end up with the checksum of the outer header being constant.
>
> cool. I would love seeing this documented somewhere, either in the
> change log if you do a respin or on some kernel networking
> documentation, is that part of the LCO documentation?
I have some documentation in
Documentation/networking/segmentation-offloads.txt. Feel free to
review it and provide any additional feedback and/or patches you
believe it needs. To me it made sense but I already understood how
all this stuff worked.
>> So in effect as long as we can trust the hardware to segment every
>> frame to the specified size and that it won't insert any extra data
>> anywhere in that region that we aren't expecting we can guarantee that
>> each frame will have the same checksum for the outer UDP header.
>
> Wow, that is really cool, thanks for taking the time and explaining it over.
>
> Just one more piece to clarify... in the general case (e.g inner
> packet size 1.5k...64k), the last segment would not have the same
> length as the other segments, what happens on that case?
Actually in the case of GSO partial we have go through the software
segmentation code and trim off any last bit that doesn't match the MSS
of the rest of the frame. That way you end up with one frame that has
some number of MSS sized chunks, and then one remainder if there is a
frame that would be a different size.
- Alex
^ permalink raw reply
* [PATCH net-next 1/3] tipc: re-enable compensation for socket receive buffer double counting
From: Jon Maloy @ 2016-05-02 15:58 UTC (permalink / raw)
To: davem; +Cc: Jon Maloy, netdev, Paul Gortmaker, tipc-discussion
In-Reply-To: <1462204727-9224-1-git-send-email-jon.maloy@ericsson.com>
In the refactoring commit d570d86497ee ("tipc: enqueue arrived buffers
in socket in separate function") we did by accident replace the test
if (sk->sk_backlog.len == 0)
atomic_set(&tsk->dupl_rcvcnt, 0);
with
if (sk->sk_backlog.len)
atomic_set(&tsk->dupl_rcvcnt, 0);
This effectively disables the compensation we have for the double
receive buffer accounting that occurs temporarily when buffers are
moved from the backlog to the socket receive queue. Until now, this
has gone unnoticed because of the large receive buffer limits we are
applying, but becomes indispensable when we reduce this buffer limit
later in this series.
We now fix this by inverting the mentioned condition.
Acked-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/socket.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 3eeb50a..d37a940 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1748,7 +1748,7 @@ static void tipc_sk_enqueue(struct sk_buff_head *inputq, struct sock *sk,
/* Try backlog, compensating for double-counted bytes */
dcnt = &tipc_sk(sk)->dupl_rcvcnt;
- if (sk->sk_backlog.len)
+ if (!sk->sk_backlog.len)
atomic_set(dcnt, 0);
lim = rcvbuf_limit(sk, skb) + atomic_read(dcnt);
if (likely(!sk_add_backlog(sk, skb, lim)))
--
1.9.1
------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
^ permalink raw reply related
* [PATCH net-next 2/3] tipc: propagate peer node capabilities to socket layer
From: Jon Maloy @ 2016-05-02 15:58 UTC (permalink / raw)
To: davem; +Cc: Jon Maloy, netdev, Paul Gortmaker, tipc-discussion
In-Reply-To: <1462204727-9224-1-git-send-email-jon.maloy@ericsson.com>
During neighbor discovery, nodes advertise their capabilities as a bit
map in a dedicated 16-bit field in the discovery message header. This
bit map has so far only be stored in the node structure on the peer
nodes, but we now see the need to keep a copy even in the socket
structure.
This commit adds this functionality.
Acked-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/node.c | 21 +++++++++++++++++++--
net/tipc/node.h | 1 +
net/tipc/socket.c | 2 ++
3 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/net/tipc/node.c b/net/tipc/node.c
index c299156..29cc853 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -1,7 +1,7 @@
/*
* net/tipc/node.c: TIPC node management routines
*
- * Copyright (c) 2000-2006, 2012-2015, Ericsson AB
+ * Copyright (c) 2000-2006, 2012-2016, Ericsson AB
* Copyright (c) 2005-2006, 2010-2014, Wind River Systems
* All rights reserved.
*
@@ -191,6 +191,20 @@ int tipc_node_get_mtu(struct net *net, u32 addr, u32 sel)
tipc_node_put(n);
return mtu;
}
+
+u16 tipc_node_get_capabilities(struct net *net, u32 addr)
+{
+ struct tipc_node *n;
+ u16 caps;
+
+ n = tipc_node_find(net, addr);
+ if (unlikely(!n))
+ return TIPC_NODE_CAPABILITIES;
+ caps = n->capabilities;
+ tipc_node_put(n);
+ return caps;
+}
+
/*
* A trivial power-of-two bitmask technique is used for speed, since this
* operation is done for every incoming TIPC packet. The number of hash table
@@ -304,8 +318,11 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u16 capabilities)
spin_lock_bh(&tn->node_list_lock);
n = tipc_node_find(net, addr);
- if (n)
+ if (n) {
+ /* Same node may come back with new capabilities */
+ n->capabilities = capabilities;
goto exit;
+ }
n = kzalloc(sizeof(*n), GFP_ATOMIC);
if (!n) {
pr_warn("Node creation failed, no memory\n");
diff --git a/net/tipc/node.h b/net/tipc/node.h
index f39d9d0..1823768 100644
--- a/net/tipc/node.h
+++ b/net/tipc/node.h
@@ -70,6 +70,7 @@ void tipc_node_broadcast(struct net *net, struct sk_buff *skb);
int tipc_node_add_conn(struct net *net, u32 dnode, u32 port, u32 peer_port);
void tipc_node_remove_conn(struct net *net, u32 dnode, u32 port);
int tipc_node_get_mtu(struct net *net, u32 addr, u32 sel);
+u16 tipc_node_get_capabilities(struct net *net, u32 addr);
int tipc_nl_node_dump(struct sk_buff *skb, struct netlink_callback *cb);
int tipc_nl_node_dump_link(struct sk_buff *skb, struct netlink_callback *cb);
int tipc_nl_node_reset_link_stats(struct sk_buff *skb, struct genl_info *info);
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index d37a940..94bd286 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -98,6 +98,7 @@ struct tipc_sock {
bool link_cong;
uint sent_unacked;
uint rcv_unacked;
+ u16 peer_caps;
struct sockaddr_tipc remote;
struct rhash_head node;
struct rcu_head rcu;
@@ -1118,6 +1119,7 @@ static void tipc_sk_finish_conn(struct tipc_sock *tsk, u32 peer_port,
sk_reset_timer(sk, &sk->sk_timer, jiffies + tsk->probing_intv);
tipc_node_add_conn(net, peer_node, tsk->portid, peer_port);
tsk->max_pkt = tipc_node_get_mtu(net, peer_node, tsk->portid);
+ tsk->peer_caps = tipc_node_get_capabilities(net, peer_node);
}
/**
--
1.9.1
------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
^ permalink raw reply related
* [PATCH net-next 0/3] tipc: redesign socket-level flow control
From: Jon Maloy @ 2016-05-02 15:58 UTC (permalink / raw)
To: davem
Cc: netdev, Paul Gortmaker, parthasarathy.bhuvaragan, richard.alpe,
ying.xue, maloy, tipc-discussion, Jon Maloy
The socket-level flow control in TIPC has long been due for a major
overhaul. This series fixes this.
Jon Maloy (3):
tipc: re-enable compensation for socket receive buffer double counting
tipc: propagate peer node capabilities to socket layer
tipc: redesign connection-level flow control
net/tipc/core.c | 8 ++-
net/tipc/msg.h | 14 +++++-
net/tipc/node.c | 21 +++++++-
net/tipc/node.h | 6 ++-
net/tipc/socket.c | 144 +++++++++++++++++++++++++++++++++++-------------------
net/tipc/socket.h | 17 +++++--
6 files changed, 145 insertions(+), 65 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH net-next 3/3] tipc: redesign connection-level flow control
From: Jon Maloy @ 2016-05-02 15:58 UTC (permalink / raw)
To: davem
Cc: netdev, Paul Gortmaker, parthasarathy.bhuvaragan, richard.alpe,
ying.xue, maloy, tipc-discussion, Jon Maloy
In-Reply-To: <1462204727-9224-1-git-send-email-jon.maloy@ericsson.com>
There are two flow control mechanisms in TIPC; one at link level that
handles network congestion, burst control, and retransmission, and one
at connection level which' only remaining task is to prevent overflow
in the receiving socket buffer. In TIPC, the latter task has to be
solved end-to-end because messages can not be thrown away once they
have been accepted and delivered upwards from the link layer, i.e, we
can never permit the receive buffer to overflow.
Currently, this algorithm is message based. A counter in the receiving
socket keeps track of number of consumed messages, and sends a dedicated
acknowledge message back to the sender for each 256 consumed message.
A counter at the sending end keeps track of the sent, not yet
acknowledged messages, and blocks the sender if this number ever reaches
512 unacknowledged messages. When the missing acknowledge arrives, the
socket is then woken up for renewed transmission. This works well for
keeping the message flow running, as it almost never happens that a
sender socket is blocked this way.
A problem with the current mechanism is that it potentially is very
memory consuming. Since we don't distinguish between small and large
messages, we have to dimension the socket receive buffer according
to a worst-case of both. I.e., the window size must be chosen large
enough to sustain a reasonable throughput even for the smallest
messages, while we must still consider a scenario where all messages
are of maximum size. Hence, the current fix window size of 512 messages
and a maximum message size of 66k results in a receive buffer of 66 MB
when truesize(66k) = 131k is taken into account. It is possible to do
much better.
This commit introduces an algorithm where we instead use 1024-byte
blocks as base unit. This unit, always rounded upwards from the
actual message size, is used when we advertise windows as well as when
we count and acknowledge transmitted data. The advertised window is
based on the configured receive buffer size in such a way that even
the worst-case truesize/msgsize ratio always is covered. Since the
smallest possible message size (from a flow control viewpoint) now is
1024 bytes, we can safely assume this ratio to be less than four, which
is the value we are now using.
This way, we have been able to reduce the default receive buffer size
from 66 MB to 2 MB with maintained performance.
In order to keep this solution backwards compatible, we introduce a
new capability bit in the discovery protocol, and use this throughout
the message sending/reception path to always select the right unit.
Acked-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/core.c | 8 ++--
net/tipc/msg.h | 14 +++++-
net/tipc/node.h | 5 +-
net/tipc/socket.c | 140 +++++++++++++++++++++++++++++++++++-------------------
net/tipc/socket.h | 17 +++++--
5 files changed, 122 insertions(+), 62 deletions(-)
diff --git a/net/tipc/core.c b/net/tipc/core.c
index e2bdb07a..fe1b062 100644
--- a/net/tipc/core.c
+++ b/net/tipc/core.c
@@ -112,11 +112,9 @@ static int __init tipc_init(void)
pr_info("Activated (version " TIPC_MOD_VER ")\n");
- sysctl_tipc_rmem[0] = TIPC_CONN_OVERLOAD_LIMIT >> 4 <<
- TIPC_LOW_IMPORTANCE;
- sysctl_tipc_rmem[1] = TIPC_CONN_OVERLOAD_LIMIT >> 4 <<
- TIPC_CRITICAL_IMPORTANCE;
- sysctl_tipc_rmem[2] = TIPC_CONN_OVERLOAD_LIMIT;
+ sysctl_tipc_rmem[0] = RCVBUF_MIN;
+ sysctl_tipc_rmem[1] = RCVBUF_DEF;
+ sysctl_tipc_rmem[2] = RCVBUF_MAX;
err = tipc_netlink_start();
if (err)
diff --git a/net/tipc/msg.h b/net/tipc/msg.h
index 58bf515..024da8a 100644
--- a/net/tipc/msg.h
+++ b/net/tipc/msg.h
@@ -743,16 +743,26 @@ static inline void msg_set_msgcnt(struct tipc_msg *m, u16 n)
msg_set_bits(m, 9, 16, 0xffff, n);
}
-static inline u32 msg_bcast_tag(struct tipc_msg *m)
+static inline u32 msg_conn_ack(struct tipc_msg *m)
{
return msg_bits(m, 9, 16, 0xffff);
}
-static inline void msg_set_bcast_tag(struct tipc_msg *m, u32 n)
+static inline void msg_set_conn_ack(struct tipc_msg *m, u32 n)
{
msg_set_bits(m, 9, 16, 0xffff, n);
}
+static inline u32 msg_adv_win(struct tipc_msg *m)
+{
+ return msg_bits(m, 9, 0, 0xffff);
+}
+
+static inline void msg_set_adv_win(struct tipc_msg *m, u32 n)
+{
+ msg_set_bits(m, 9, 0, 0xffff, n);
+}
+
static inline u32 msg_max_pkt(struct tipc_msg *m)
{
return msg_bits(m, 9, 16, 0xffff) * 4;
diff --git a/net/tipc/node.h b/net/tipc/node.h
index 1823768..8264b3d 100644
--- a/net/tipc/node.h
+++ b/net/tipc/node.h
@@ -45,10 +45,11 @@
/* Optional capabilities supported by this code version
*/
enum {
- TIPC_BCAST_SYNCH = (1 << 1)
+ TIPC_BCAST_SYNCH = (1 << 1),
+ TIPC_BLOCK_FLOWCTL = (2 << 1)
};
-#define TIPC_NODE_CAPABILITIES TIPC_BCAST_SYNCH
+#define TIPC_NODE_CAPABILITIES (TIPC_BCAST_SYNCH | TIPC_BLOCK_FLOWCTL)
#define INVALID_BEARER_ID -1
void tipc_node_stop(struct net *net);
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 94bd286..1262889 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -96,9 +96,11 @@ struct tipc_sock {
uint conn_timeout;
atomic_t dupl_rcvcnt;
bool link_cong;
- uint sent_unacked;
- uint rcv_unacked;
+ u16 snt_unacked;
+ u16 snd_win;
u16 peer_caps;
+ u16 rcv_unacked;
+ u16 rcv_win;
struct sockaddr_tipc remote;
struct rhash_head node;
struct rcu_head rcu;
@@ -228,9 +230,29 @@ static struct tipc_sock *tipc_sk(const struct sock *sk)
return container_of(sk, struct tipc_sock, sk);
}
-static int tsk_conn_cong(struct tipc_sock *tsk)
+static bool tsk_conn_cong(struct tipc_sock *tsk)
{
- return tsk->sent_unacked >= TIPC_FLOWCTRL_WIN;
+ return tsk->snt_unacked >= tsk->snd_win;
+}
+
+/* tsk_blocks(): translate a buffer size in bytes to number of
+ * advertisable blocks, taking into account the ratio truesize(len)/len
+ * We can trust that this ratio is always < 4 for len >= FLOWCTL_BLK_SZ
+ */
+static u16 tsk_adv_blocks(int len)
+{
+ return len / FLOWCTL_BLK_SZ / 4;
+}
+
+/* tsk_inc(): increment counter for sent or received data
+ * - If block based flow control is not supported by peer we
+ * fall back to message based ditto, incrementing the counter
+ */
+static u16 tsk_inc(struct tipc_sock *tsk, int msglen)
+{
+ if (likely(tsk->peer_caps & TIPC_BLOCK_FLOWCTL))
+ return ((msglen / FLOWCTL_BLK_SZ) + 1);
+ return 1;
}
/**
@@ -378,9 +400,12 @@ static int tipc_sk_create(struct net *net, struct socket *sock,
sk->sk_write_space = tipc_write_space;
sk->sk_destruct = tipc_sock_destruct;
tsk->conn_timeout = CONN_TIMEOUT_DEFAULT;
- tsk->sent_unacked = 0;
atomic_set(&tsk->dupl_rcvcnt, 0);
+ /* Start out with safe limits until we receive an advertised window */
+ tsk->snd_win = tsk_adv_blocks(RCVBUF_MIN);
+ tsk->rcv_win = tsk->snd_win;
+
if (sock->state == SS_READY) {
tsk_set_unreturnable(tsk, true);
if (sock->type == SOCK_DGRAM)
@@ -776,7 +801,7 @@ static void tipc_sk_proto_rcv(struct tipc_sock *tsk, struct sk_buff *skb)
struct sock *sk = &tsk->sk;
struct tipc_msg *hdr = buf_msg(skb);
int mtyp = msg_type(hdr);
- int conn_cong;
+ bool conn_cong;
/* Ignore if connection cannot be validated: */
if (!tsk_peer_msg(tsk, hdr))
@@ -790,7 +815,9 @@ static void tipc_sk_proto_rcv(struct tipc_sock *tsk, struct sk_buff *skb)
return;
} else if (mtyp == CONN_ACK) {
conn_cong = tsk_conn_cong(tsk);
- tsk->sent_unacked -= msg_msgcnt(hdr);
+ tsk->snt_unacked -= msg_conn_ack(hdr);
+ if (tsk->peer_caps & TIPC_BLOCK_FLOWCTL)
+ tsk->snd_win = msg_adv_win(hdr);
if (conn_cong)
sk->sk_write_space(sk);
} else if (mtyp != CONN_PROBE_REPLY) {
@@ -1021,12 +1048,14 @@ static int __tipc_send_stream(struct socket *sock, struct msghdr *m, size_t dsz)
u32 dnode;
uint mtu, send, sent = 0;
struct iov_iter save;
+ int hlen = MIN_H_SIZE;
/* Handle implied connection establishment */
if (unlikely(dest)) {
rc = __tipc_sendmsg(sock, m, dsz);
+ hlen = msg_hdr_sz(mhdr);
if (dsz && (dsz == rc))
- tsk->sent_unacked = 1;
+ tsk->snt_unacked = tsk_inc(tsk, dsz + hlen);
return rc;
}
if (dsz > (uint)INT_MAX)
@@ -1055,7 +1084,7 @@ next:
if (likely(!tsk_conn_cong(tsk))) {
rc = tipc_node_xmit(net, &pktchain, dnode, portid);
if (likely(!rc)) {
- tsk->sent_unacked++;
+ tsk->snt_unacked += tsk_inc(tsk, send + hlen);
sent += send;
if (sent == dsz)
return dsz;
@@ -1120,6 +1149,12 @@ static void tipc_sk_finish_conn(struct tipc_sock *tsk, u32 peer_port,
tipc_node_add_conn(net, peer_node, tsk->portid, peer_port);
tsk->max_pkt = tipc_node_get_mtu(net, peer_node, tsk->portid);
tsk->peer_caps = tipc_node_get_capabilities(net, peer_node);
+ if (tsk->peer_caps & TIPC_BLOCK_FLOWCTL)
+ return;
+
+ /* Fall back to message based flow control */
+ tsk->rcv_win = FLOWCTL_MSG_WIN;
+ tsk->snd_win = FLOWCTL_MSG_WIN;
}
/**
@@ -1216,7 +1251,7 @@ static int tipc_sk_anc_data_recv(struct msghdr *m, struct tipc_msg *msg,
return 0;
}
-static void tipc_sk_send_ack(struct tipc_sock *tsk, uint ack)
+static void tipc_sk_send_ack(struct tipc_sock *tsk)
{
struct net *net = sock_net(&tsk->sk);
struct sk_buff *skb = NULL;
@@ -1232,7 +1267,14 @@ static void tipc_sk_send_ack(struct tipc_sock *tsk, uint ack)
if (!skb)
return;
msg = buf_msg(skb);
- msg_set_msgcnt(msg, ack);
+ msg_set_conn_ack(msg, tsk->rcv_unacked);
+ tsk->rcv_unacked = 0;
+
+ /* Adjust to and advertize the correct window limit */
+ if (tsk->peer_caps & TIPC_BLOCK_FLOWCTL) {
+ tsk->rcv_win = tsk_adv_blocks(tsk->sk.sk_rcvbuf);
+ msg_set_adv_win(msg, tsk->rcv_win);
+ }
tipc_node_xmit_skb(net, skb, dnode, msg_link_selector(msg));
}
@@ -1290,7 +1332,7 @@ static int tipc_recvmsg(struct socket *sock, struct msghdr *m, size_t buf_len,
long timeo;
unsigned int sz;
u32 err;
- int res;
+ int res, hlen;
/* Catch invalid receive requests */
if (unlikely(!buf_len))
@@ -1315,6 +1357,7 @@ restart:
buf = skb_peek(&sk->sk_receive_queue);
msg = buf_msg(buf);
sz = msg_data_sz(msg);
+ hlen = msg_hdr_sz(msg);
err = msg_errcode(msg);
/* Discard an empty non-errored message & try again */
@@ -1337,7 +1380,7 @@ restart:
sz = buf_len;
m->msg_flags |= MSG_TRUNC;
}
- res = skb_copy_datagram_msg(buf, msg_hdr_sz(msg), m, sz);
+ res = skb_copy_datagram_msg(buf, hlen, m, sz);
if (res)
goto exit;
res = sz;
@@ -1349,15 +1392,15 @@ restart:
res = -ECONNRESET;
}
- /* Consume received message (optional) */
- if (likely(!(flags & MSG_PEEK))) {
- if ((sock->state != SS_READY) &&
- (++tsk->rcv_unacked >= TIPC_CONNACK_INTV)) {
- tipc_sk_send_ack(tsk, tsk->rcv_unacked);
- tsk->rcv_unacked = 0;
- }
- tsk_advance_rx_queue(sk);
+ if (unlikely(flags & MSG_PEEK))
+ goto exit;
+
+ if (likely(sock->state != SS_READY)) {
+ tsk->rcv_unacked += tsk_inc(tsk, hlen + sz);
+ if (unlikely(tsk->rcv_unacked >= (tsk->rcv_win / 4)))
+ tipc_sk_send_ack(tsk);
}
+ tsk_advance_rx_queue(sk);
exit:
release_sock(sk);
return res;
@@ -1386,7 +1429,7 @@ static int tipc_recv_stream(struct socket *sock, struct msghdr *m,
int sz_to_copy, target, needed;
int sz_copied = 0;
u32 err;
- int res = 0;
+ int res = 0, hlen;
/* Catch invalid receive attempts */
if (unlikely(!buf_len))
@@ -1412,6 +1455,7 @@ restart:
buf = skb_peek(&sk->sk_receive_queue);
msg = buf_msg(buf);
sz = msg_data_sz(msg);
+ hlen = msg_hdr_sz(msg);
err = msg_errcode(msg);
/* Discard an empty non-errored message & try again */
@@ -1436,8 +1480,7 @@ restart:
needed = (buf_len - sz_copied);
sz_to_copy = (sz <= needed) ? sz : needed;
- res = skb_copy_datagram_msg(buf, msg_hdr_sz(msg) + offset,
- m, sz_to_copy);
+ res = skb_copy_datagram_msg(buf, hlen + offset, m, sz_to_copy);
if (res)
goto exit;
@@ -1459,20 +1502,18 @@ restart:
res = -ECONNRESET;
}
- /* Consume received message (optional) */
- if (likely(!(flags & MSG_PEEK))) {
- if (unlikely(++tsk->rcv_unacked >= TIPC_CONNACK_INTV)) {
- tipc_sk_send_ack(tsk, tsk->rcv_unacked);
- tsk->rcv_unacked = 0;
- }
- tsk_advance_rx_queue(sk);
- }
+ if (unlikely(flags & MSG_PEEK))
+ goto exit;
+
+ tsk->rcv_unacked += tsk_inc(tsk, hlen + sz);
+ if (unlikely(tsk->rcv_unacked >= (tsk->rcv_win / 4)))
+ tipc_sk_send_ack(tsk);
+ tsk_advance_rx_queue(sk);
/* Loop around if more data is required */
if ((sz_copied < buf_len) && /* didn't get all requested data */
(!skb_queue_empty(&sk->sk_receive_queue) ||
(sz_copied < target)) && /* and more is ready or required */
- (!(flags & MSG_PEEK)) && /* and aren't just peeking at data */
(!err)) /* and haven't reached a FIN */
goto restart;
@@ -1604,30 +1645,33 @@ static bool filter_connect(struct tipc_sock *tsk, struct sk_buff *skb)
/**
* rcvbuf_limit - get proper overload limit of socket receive queue
* @sk: socket
- * @buf: message
+ * @skb: message
*
- * For all connection oriented messages, irrespective of importance,
- * the default overload value (i.e. 67MB) is set as limit.
+ * For connection oriented messages, irrespective of importance,
+ * default queue limit is 2 MB.
*
- * For all connectionless messages, by default new queue limits are
- * as belows:
+ * For connectionless messages, queue limits are based on message
+ * importance as follows:
*
- * TIPC_LOW_IMPORTANCE (4 MB)
- * TIPC_MEDIUM_IMPORTANCE (8 MB)
- * TIPC_HIGH_IMPORTANCE (16 MB)
- * TIPC_CRITICAL_IMPORTANCE (32 MB)
+ * TIPC_LOW_IMPORTANCE (2 MB)
+ * TIPC_MEDIUM_IMPORTANCE (4 MB)
+ * TIPC_HIGH_IMPORTANCE (8 MB)
+ * TIPC_CRITICAL_IMPORTANCE (16 MB)
*
* Returns overload limit according to corresponding message importance
*/
-static unsigned int rcvbuf_limit(struct sock *sk, struct sk_buff *buf)
+static unsigned int rcvbuf_limit(struct sock *sk, struct sk_buff *skb)
{
- struct tipc_msg *msg = buf_msg(buf);
+ struct tipc_sock *tsk = tipc_sk(sk);
+ struct tipc_msg *hdr = buf_msg(skb);
+
+ if (unlikely(!msg_connected(hdr)))
+ return sk->sk_rcvbuf << msg_importance(hdr);
- if (msg_connected(msg))
- return sysctl_tipc_rmem[2];
+ if (likely(tsk->peer_caps & TIPC_BLOCK_FLOWCTL))
+ return sk->sk_rcvbuf;
- return sk->sk_rcvbuf >> TIPC_CRITICAL_IMPORTANCE <<
- msg_importance(msg);
+ return FLOWCTL_MSG_LIM;
}
/**
diff --git a/net/tipc/socket.h b/net/tipc/socket.h
index 4241f22..06fb594 100644
--- a/net/tipc/socket.h
+++ b/net/tipc/socket.h
@@ -1,6 +1,6 @@
/* net/tipc/socket.h: Include file for TIPC socket code
*
- * Copyright (c) 2014-2015, Ericsson AB
+ * Copyright (c) 2014-2016, Ericsson AB
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -38,10 +38,17 @@
#include <net/sock.h>
#include <net/genetlink.h>
-#define TIPC_CONNACK_INTV 256
-#define TIPC_FLOWCTRL_WIN (TIPC_CONNACK_INTV * 2)
-#define TIPC_CONN_OVERLOAD_LIMIT ((TIPC_FLOWCTRL_WIN * 2 + 1) * \
- SKB_TRUESIZE(TIPC_MAX_USER_MSG_SIZE))
+/* Compatibility values for deprecated message based flow control */
+#define FLOWCTL_MSG_WIN 512
+#define FLOWCTL_MSG_LIM ((FLOWCTL_MSG_WIN * 2 + 1) * SKB_TRUESIZE(MAX_MSG_SIZE))
+
+#define FLOWCTL_BLK_SZ 1024
+
+/* Socket receive buffer sizes */
+#define RCVBUF_MIN (FLOWCTL_BLK_SZ * 512)
+#define RCVBUF_DEF (FLOWCTL_BLK_SZ * 1024 * 2)
+#define RCVBUF_MAX (FLOWCTL_BLK_SZ * 1024 * 16)
+
int tipc_socket_init(void);
void tipc_socket_stop(void);
void tipc_sk_rcv(struct net *net, struct sk_buff_head *inputq);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next] fq_codel: add batch ability to fq_codel_drop()
From: Jesper Dangaard Brouer @ 2016-05-02 16:00 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Dave Taht, Jonathan Morton, brouer
In-Reply-To: <1462199668.5535.239.camel@edumazet-glaptop3.roam.corp.google.com>
On Mon, 02 May 2016 07:34:28 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2016-05-02 at 09:49 +0200, Jesper Dangaard Brouer wrote:
>
> > What about using bulk free of SKBs here?
> >
> > There is a very high probability that we are hitting SLUB slowpath,
> > which involves an expensive locked cmpxchg_double per packet. Instead
> > we can amortize this cost via kmem_cache_free_bulk().
> >
> > Maybe extend kfree_skb_list() to hide the slab/kmem_cache call?
>
> Sounds tricky, because of skb destructors. skb are complex objects.
>
> For each skb, need to free the frags, skb->head, and skb.
It is not that complicated, inside kfree_skb_list(), we just call
skb_release_all(skb) on each SKB first, and then bulk free the SKB's
themselves in the end. Example see, _kfree_skb_defer().
The question is where to store the SKB array needed by kmem_cache_free_bulk.
The easy option is just to use the stack of kfree_skb_list(), but we
have to be careful about the stack size, it might not be so good
because skb_release_all() can be deep and via skb_release_data() invoke
kfree_skb_list() a second time.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH net-next] fq_codel: add batch ability to fq_codel_drop()
From: Eric Dumazet @ 2016-05-02 16:12 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: David Miller, netdev, Dave Taht, Jonathan Morton
In-Reply-To: <20160502180036.25bebdfe@redhat.com>
On Mon, 2016-05-02 at 18:00 +0200, Jesper Dangaard Brouer wrote:
> It is not that complicated, inside kfree_skb_list(), we just call
> skb_release_all(skb) on each SKB first, and then bulk free the SKB's
> themselves in the end. Example see, _kfree_skb_defer().
>
> The question is where to store the SKB array needed by kmem_cache_free_bulk.
>
> The easy option is just to use the stack of kfree_skb_list(), but we
> have to be careful about the stack size, it might not be so good
> because skb_release_all() can be deep and via skb_release_data() invoke
> kfree_skb_list() a second time.
>
It sounds you are reinventing the wheel ;)
If drivers use napi_consume_skb(), qdisc should be able to use it the
same, since BH are disabled in their ->enqueue()/->dequeue() handlers.
This would be a separate patch of course.
This fq_codel fix might need to be backported.
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: SOCKWQ_ASYNC_NOSPACE optimizations
From: Jiri Pirko @ 2016-05-02 16:16 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S . Miller, netdev, Eric Dumazet, eladr, idosch
In-Reply-To: <1461605974-4242-2-git-send-email-edumazet@google.com>
Mon, Apr 25, 2016 at 07:39:32PM CEST, edumazet@google.com wrote:
>SOCKWQ_ASYNC_NOSPACE is tested in sock_wake_async()
>so that a SIGIO signal is sent when needed.
>
>tcp_sendmsg() clears the bit.
>tcp_poll() sets the bit when stream is not writeable.
>
>We can avoid two atomic operations by first checking if socket
>is actually interested in the FASYNC business (most sockets in
>real applications do not use AIO, but select()/poll()/epoll())
>
>This also removes one cache line miss to access sk->sk_wq->flags
>in tcp_sendmsg()
>
>Signed-off-by: Eric Dumazet <edumazet@google.com>
I just bisected down to this. This is causing a regression for me when
my nfs mount becomes stuck. I can easily reproduce this if you need to
test the fix.
Thanks.
^ permalink raw reply
* Re: [PATCH net 1/2] RDS:TCP: Synchronize rds_tcp_accept_one with rds_send_xmit when resetting t_sock
From: Santosh Shilimkar @ 2016-05-02 16:20 UTC (permalink / raw)
To: Sowmini Varadhan, netdev, rds-devel; +Cc: davem
In-Reply-To: <470ac585d014a6d8ea1600b8897bdc313e7c2431.1462127059.git.sowmini.varadhan@oracle.com>
On 5/1/2016 4:10 PM, Sowmini Varadhan wrote:
> There is a race condition between rds_send_xmit -> rds_tcp_xmit
> and the code that deals with resolution of duelling syns added
> by commit 241b271952eb ("RDS-TCP: Reset tcp callbacks if re-using an
> outgoing socket in rds_tcp_accept_one()").
>
> Specifically, we may end up derefencing a null pointer in rds_send_xmit
> if we have the interleaving sequencee:
> rds_tcp_accept_one rds_send_xmit
>
> conn is RDS_CONN_UP, so
> invoke rds_tcp_xmit
>
> tc = conn->c_transport_data
> rds_tcp_restore_callbacks
> /* reset t_sock */
> null ptr deref from tc->t_sock
>
> The race condition can be avoided without adding the overhead of
> additional locking in the xmit path: have rds_tcp_accept_one wait
> for rds_tcp_xmit threads to complete before resetting callbacks.
> The synchronization can be done in the same manner as rds_conn_shutdown().
> First set the rds_conn_state to something other than RDS_CONN_UP
> (so that new threads cannot get into rds_tcp_xmit()), then wait for
> RDS_IN_XMIT to be cleared in the conn->c_flags indicating that any
> threads in rds_tcp_xmit are done.
>
> Fixes: 241b271952eb ("RDS-TCP: Reset tcp callbacks if re-using an
> outgoing socket in rds_tcp_accept_one()")
>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---
Mostly looks correct. A question below.
> net/rds/tcp.c | 2 +-
> net/rds/tcp_listen.c | 40 ++++++++++++++++++++++++----------------
> 2 files changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/net/rds/tcp.c b/net/rds/tcp.c
> index 61ed2a8..9134544 100644
> --- a/net/rds/tcp.c
> +++ b/net/rds/tcp.c
> @@ -127,7 +127,7 @@ void rds_tcp_restore_callbacks(struct socket *sock,
>
> /*
> * This is the only path that sets tc->t_sock. Send and receive trust that
> - * it is set. The RDS_CONN_CONNECTED bit protects those paths from being
> + * it is set. The RDS_CONN_UP bit protects those paths from being
> * called while it isn't set.
> */
> void rds_tcp_set_callbacks(struct socket *sock, struct rds_connection *conn)
> diff --git a/net/rds/tcp_listen.c b/net/rds/tcp_listen.c
> index 0936a4a..0896187 100644
> --- a/net/rds/tcp_listen.c
> +++ b/net/rds/tcp_listen.c
> @@ -115,24 +115,32 @@ int rds_tcp_accept_one(struct socket *sock)
> * rds_tcp_state_change() will do that cleanup
> */
> rs_tcp = (struct rds_tcp_connection *)conn->c_transport_data;
> - if (rs_tcp->t_sock &&
> - ntohl(inet->inet_saddr) < ntohl(inet->inet_daddr)) {
> - struct sock *nsk = new_sock->sk;
> -
> - nsk->sk_user_data = NULL;
> - nsk->sk_prot->disconnect(nsk, 0);
> - tcp_done(nsk);
> - new_sock = NULL;
> - ret = 0;
> - goto out;
> - } else if (rs_tcp->t_sock) {
> - rds_tcp_restore_callbacks(rs_tcp->t_sock, rs_tcp);
> - conn->c_outgoing = 0;
> - }
> -
> rds_conn_transition(conn, RDS_CONN_DOWN, RDS_CONN_CONNECTING);
> + if (rs_tcp->t_sock) {
> + /* Need to resolve a duelling SYN between peers.
> + * We have an outstanding SYN to this peer, which may
> + * potentially have transitioned to the RDS_CONN_UP state,
> + * so we must quiesce any send threads before resetting
> + * c_transport_data.
> + */
> + wait_event(conn->c_waitq,
> + !test_bit(RDS_IN_XMIT, &conn->c_flags));
Would it be good to check the return value of rds_conn_transition()
since if CONN is already UP above will fail and then send message
might again race and we will let message through even though passive
hasn't finished its connection.
> + if (ntohl(inet->inet_saddr) < ntohl(inet->inet_daddr)) {
> + struct sock *nsk = new_sock->sk;
> +
> + nsk->sk_user_data = NULL;
> + nsk->sk_prot->disconnect(nsk, 0);
> + tcp_done(nsk);
> + new_sock = NULL;
> + ret = 0;
> + goto out;
> + } else if (rs_tcp->t_sock) {
> + rds_tcp_restore_callbacks(rs_tcp->t_sock, rs_tcp);
> + conn->c_outgoing = 0;
> + }
> + }
> rds_tcp_set_callbacks(new_sock, conn);
> - rds_connect_complete(conn);
> + rds_connect_complete(conn); /* marks RDS_CONN_UP */
> new_sock = NULL;
> ret = 0;
>
>
^ permalink raw reply
* [PATCHv4] netem: Segment GSO packets on enqueue
From: Neil Horman @ 2016-05-02 16:20 UTC (permalink / raw)
To: netdev
Cc: Neil Horman, Jamal Hadi Salim, David S. Miller, netem,
eric.dumazet, stephen
In-Reply-To: <1461692618-21333-1-git-send-email-nhorman@tuxdriver.com>
This was recently reported to me, and reproduced on the latest net kernel,
when attempting to run netperf from a host that had a netem qdisc attached
to the egress interface:
[ 788.073771] ---------------------[ cut here ]---------------------------
[ 788.096716] WARNING: at net/core/dev.c:2253 skb_warn_bad_offload+0xcd/0xda()
[ 788.129521] bnx2: caps=(0x00000001801949b3, 0x0000000000000000) len=2962
data_len=0 gso_size=1448 gso_type=1 ip_summed=3
[ 788.182150] Modules linked in: sch_netem kvm_amd kvm crc32_pclmul ipmi_ssif
ghash_clmulni_intel sp5100_tco amd64_edac_mod aesni_intel lrw gf128mul
glue_helper ablk_helper edac_mce_amd cryptd pcspkr sg edac_core hpilo ipmi_si
i2c_piix4 k10temp fam15h_power hpwdt ipmi_msghandler shpchp acpi_power_meter
pcc_cpufreq nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs libcrc32c
sd_mod crc_t10dif crct10dif_generic mgag200 syscopyarea sysfillrect sysimgblt
i2c_algo_bit drm_kms_helper ahci ata_generic pata_acpi ttm libahci
crct10dif_pclmul pata_atiixp tg3 libata crct10dif_common drm crc32c_intel ptp
serio_raw bnx2 r8169 hpsa pps_core i2c_core mii dm_mirror dm_region_hash dm_log
dm_mod
[ 788.465294] CPU: 16 PID: 0 Comm: swapper/16 Tainted: G W
------------ 3.10.0-327.el7.x86_64 #1
[ 788.511521] Hardware name: HP ProLiant DL385p Gen8, BIOS A28 12/17/2012
[ 788.542260] ffff880437c036b8 f7afc56532a53db9 ffff880437c03670
ffffffff816351f1
[ 788.576332] ffff880437c036a8 ffffffff8107b200 ffff880633e74200
ffff880231674000
[ 788.611943] 0000000000000001 0000000000000003 0000000000000000
ffff880437c03710
[ 788.647241] Call Trace:
[ 788.658817] <IRQ> [<ffffffff816351f1>] dump_stack+0x19/0x1b
[ 788.686193] [<ffffffff8107b200>] warn_slowpath_common+0x70/0xb0
[ 788.713803] [<ffffffff8107b29c>] warn_slowpath_fmt+0x5c/0x80
[ 788.741314] [<ffffffff812f92f3>] ? ___ratelimit+0x93/0x100
[ 788.767018] [<ffffffff81637f49>] skb_warn_bad_offload+0xcd/0xda
[ 788.796117] [<ffffffff8152950c>] skb_checksum_help+0x17c/0x190
[ 788.823392] [<ffffffffa01463a1>] netem_enqueue+0x741/0x7c0 [sch_netem]
[ 788.854487] [<ffffffff8152cb58>] dev_queue_xmit+0x2a8/0x570
[ 788.880870] [<ffffffff8156ae1d>] ip_finish_output+0x53d/0x7d0
...
The problem occurs because netem is not prepared to handle GSO packets (as it
uses skb_checksum_help in its enqueue path, which cannot manipulate these
frames).
The solution I think is to simply segment the skb in a simmilar fashion to the
way we do in __dev_queue_xmit (via validate_xmit_skb), with some minor changes.
When we decide to corrupt an skb, if the frame is GSO, we segment it, corrupt
the first segment, and enqueue the remaining ones.
tested successfully by myself on the latest net kernel, to which this applies
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Jamal Hadi Salim <jhs@mojatatu.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: netem@lists.linux-foundation.org
CC: eric.dumazet@gmail.com
CC: stephen@networkplumber.org
---
Change Notes:
V2) As per request from Eric Dumazet, I rewrote this to limit the need to
segment the skb. Instead of doing so unilaterally, we no only do so now when the
netem qdisc requires determines that a packet must be corrupted, thus avoiding
the failure in skb_checksum_help. This still leaves open concerns with
statistical measurements made on GSO packets being dropped or reordered (i.e.
they are counted as a single packet rather than multiple packets), but I'd
rather fix the immediate problem before we go rewriting everything to fix that
larger issue.
V3) Added back missing call to qdisc_tree_reduce_backlog that I misplaced in the
V2 change.
V4) Fix up length computation and return code. Also clean up some patch
formatting
---
net/sched/sch_netem.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 59 insertions(+), 2 deletions(-)
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 9640bb3..4befe97 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -395,6 +395,25 @@ static void tfifo_enqueue(struct sk_buff *nskb, struct Qdisc *sch)
sch->q.qlen++;
}
+/* netem can't properly corrupt a megapacket (like we get from GSO), so instead
+ * when we statistically choose to corrupt one, we instead segment it, returning
+ * the first packet to be corrupted, and re-enqueue the remaining frames
+ */
+static struct sk_buff *netem_segment(struct sk_buff *skb, struct Qdisc *sch)
+{
+ struct sk_buff *segs;
+ netdev_features_t features = netif_skb_features(skb);
+
+ segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);
+
+ if (IS_ERR_OR_NULL(segs)) {
+ qdisc_reshape_fail(skb, sch);
+ return NULL;
+ }
+ consume_skb(skb);
+ return segs;
+}
+
/*
* Insert one skb into qdisc.
* Note: parent depends on return value to account for queue length.
@@ -407,7 +426,11 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
/* We don't fill cb now as skb_unshare() may invalidate it */
struct netem_skb_cb *cb;
struct sk_buff *skb2;
+ struct sk_buff *segs = NULL;
+ unsigned int len = 0, last_len, prev_len = qdisc_pkt_len(skb);
+ int nb = 0;
int count = 1;
+ int rc = NET_XMIT_SUCCESS;
/* Random duplication */
if (q->duplicate && q->duplicate >= get_crandom(&q->dup_cor))
@@ -453,10 +476,23 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
* do it now in software before we mangle it.
*/
if (q->corrupt && q->corrupt >= get_crandom(&q->corrupt_cor)) {
+ if (skb_is_gso(skb)) {
+ segs = netem_segment(skb, sch);
+ if (!segs)
+ return NET_XMIT_DROP;
+ } else {
+ segs = skb;
+ }
+
+ skb = segs;
+ segs = segs->next;
+
if (!(skb = skb_unshare(skb, GFP_ATOMIC)) ||
(skb->ip_summed == CHECKSUM_PARTIAL &&
- skb_checksum_help(skb)))
- return qdisc_drop(skb, sch);
+ skb_checksum_help(skb))) {
+ rc = qdisc_drop(skb, sch);
+ goto finish_segs;
+ }
skb->data[prandom_u32() % skb_headlen(skb)] ^=
1<<(prandom_u32() % 8);
@@ -516,6 +552,27 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
sch->qstats.requeues++;
}
+finish_segs:
+ if (segs) {
+ while (segs) {
+ skb2 = segs->next;
+ segs->next = NULL;
+ qdisc_skb_cb(segs)->pkt_len = segs->len;
+ last_len = segs->len;
+ rc = qdisc_enqueue(segs, sch);
+ if (rc != NET_XMIT_SUCCESS) {
+ if (net_xmit_drop_count(rc))
+ qdisc_qstats_drop(sch);
+ } else {
+ nb++;
+ len += last_len;
+ }
+ segs = skb2;
+ }
+ sch->q.qlen += nb;
+ if (nb > 1)
+ qdisc_tree_reduce_backlog(sch, 1 - nb, prev_len - len);
+ }
return NET_XMIT_SUCCESS;
}
--
2.5.5
^ permalink raw reply related
* Re: [PATCH net-next 1/2] net: SOCKWQ_ASYNC_NOSPACE optimizations
From: Eric Dumazet @ 2016-05-02 16:22 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Eric Dumazet, David S . Miller, netdev, eladr, idosch
In-Reply-To: <20160502161602.GA1984@nanopsycho.orion>
On Mon, 2016-05-02 at 18:16 +0200, Jiri Pirko wrote:
> Mon, Apr 25, 2016 at 07:39:32PM CEST, edumazet@google.com wrote:
> >SOCKWQ_ASYNC_NOSPACE is tested in sock_wake_async()
> >so that a SIGIO signal is sent when needed.
> >
> >tcp_sendmsg() clears the bit.
> >tcp_poll() sets the bit when stream is not writeable.
> >
> >We can avoid two atomic operations by first checking if socket
> >is actually interested in the FASYNC business (most sockets in
> >real applications do not use AIO, but select()/poll()/epoll())
> >
> >This also removes one cache line miss to access sk->sk_wq->flags
> >in tcp_sendmsg()
> >
> >Signed-off-by: Eric Dumazet <edumazet@google.com>
>
> I just bisected down to this. This is causing a regression for me when
> my nfs mount becomes stuck. I can easily reproduce this if you need to
> test the fix.
What do you mean by 'when nfs mount becomes stuck' ?
Is this patch making nfs not functional , or does it make recovery from
some nfs error bad ?
Thanks.
^ permalink raw reply
* [net PATCH 0/2] Fixes for tunnel checksum and segmentation offloads
From: Alexander Duyck @ 2016-05-02 16:25 UTC (permalink / raw)
To: netdev, ogerlitz, davem, alexander.duyck
This patch series is a subset of patches I had submitted for net-next. I
plan to drop these two patches from the v3 of "Fix Tunnel features and
enable GSO partial for several drivers" and I am instead submitting them
for net since these are truly fixes and likely will need to be backported
to stable branches.
This series addresses 2 specific issues. The first is that we could
request TSO on a v4 inner header while not supporting checksum offload of
the outer IPv6 header. The second is that we could request an IPv6 inner
checksum offload without validating that we could actually support an inner
IPv6 checksum offload.
---
Alexander Duyck (2):
net: Disable segmentation if checksumming is not supported
vxlan: Add checksum check to the features check function
include/linux/if_ether.h | 5 +++++
include/net/vxlan.h | 4 +++-
net/core/dev.c | 2 +-
3 files changed, 9 insertions(+), 2 deletions(-)
^ permalink raw reply
* [net PATCH 1/2] net: Disable segmentation if checksumming is not supported
From: Alexander Duyck @ 2016-05-02 16:25 UTC (permalink / raw)
To: netdev, ogerlitz, davem, alexander.duyck
In-Reply-To: <20160502161621.11701.9271.stgit@ahduyck-xeon-server>
In the case of the mlx4 and mlx5 driver they do not support IPv6 checksum
offload for tunnels. With this being the case we should disable GSO in
addition to the checksum offload features when we find that a device cannot
perform a checksum on a given packet type.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
net/core/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 77a71cd68535..5c925ac50b95 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2802,7 +2802,7 @@ static netdev_features_t harmonize_features(struct sk_buff *skb,
if (skb->ip_summed != CHECKSUM_NONE &&
!can_checksum_protocol(features, type)) {
- features &= ~NETIF_F_CSUM_MASK;
+ features &= ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
} else if (illegal_highdma(skb->dev, skb)) {
features &= ~NETIF_F_SG;
}
^ permalink raw reply related
* [net PATCH 2/2] vxlan: Add checksum check to the features check function
From: Alexander Duyck @ 2016-05-02 16:25 UTC (permalink / raw)
To: netdev, ogerlitz, davem, alexander.duyck
In-Reply-To: <20160502161621.11701.9271.stgit@ahduyck-xeon-server>
We need to perform an additional check on the inner headers to determine if
we can offload the checksum for them. Previously this check didn't occur
so we would generate an invalid frame in the case of an IPv6 header
encapsulated inside of an IPv4 tunnel. To fix this I added a secondary
check to vxlan_features_check so that we can verify that we can offload the
inner checksum.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
include/linux/if_ether.h | 5 +++++
include/net/vxlan.h | 4 +++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h
index d5569734f672..548fd535fd02 100644
--- a/include/linux/if_ether.h
+++ b/include/linux/if_ether.h
@@ -28,6 +28,11 @@ static inline struct ethhdr *eth_hdr(const struct sk_buff *skb)
return (struct ethhdr *)skb_mac_header(skb);
}
+static inline struct ethhdr *inner_eth_hdr(const struct sk_buff *skb)
+{
+ return (struct ethhdr *)skb_inner_mac_header(skb);
+}
+
int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr);
extern ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len);
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 73ed2e951c02..35437c779da8 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -252,7 +252,9 @@ static inline netdev_features_t vxlan_features_check(struct sk_buff *skb,
(skb->inner_protocol_type != ENCAP_TYPE_ETHER ||
skb->inner_protocol != htons(ETH_P_TEB) ||
(skb_inner_mac_header(skb) - skb_transport_header(skb) !=
- sizeof(struct udphdr) + sizeof(struct vxlanhdr))))
+ sizeof(struct udphdr) + sizeof(struct vxlanhdr)) ||
+ (skb->ip_summed != CHECKSUM_NONE &&
+ !can_checksum_protocol(features, inner_eth_hdr(skb)->h_proto))))
return features & ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
return features;
^ permalink raw reply related
* Re: [PATCH net 2/2] RDS: TCP: Synchrnozize accept() and connect() paths on t_conn_lock.
From: Santosh Shilimkar @ 2016-05-02 16:33 UTC (permalink / raw)
To: Sowmini Varadhan, netdev, rds-devel; +Cc: davem
In-Reply-To: <5fc507116182afdda4b824173008e09d5d464a33.1462127059.git.sowmini.varadhan@oracle.com>
On 5/1/2016 4:10 PM, Sowmini Varadhan wrote:
> An arbitration scheme for duelling SYNs is implemented as part of
> commit 241b271952eb ("RDS-TCP: Reset tcp callbacks if re-using an
> outgoing socket in rds_tcp_accept_one()") which ensures that both nodes
> involved will arrive at the same arbitration decision. However, this
> needs to be synchronized with an outgoing SYN to be generated by
> rds_tcp_conn_connect(). This commit achieves the synchronization
> through the t_conn_lock mutex in struct rds_tcp_connection.
>
> The rds_conn_state is checked in rds_tcp_conn_connect() after acquiring
> the t_conn_lock mutex. A SYN is sent out only if the RDS connection is
> not already UP (an UP would indicate that rds_tcp_accept_one() has
> completed 3WH, so no SYN needs to be generated).
>
> Similarly, the rds_conn_state is checked in rds_tcp_accept_one() after
> acquiring the t_conn_lock mutex. The only acceptable states (to
> allow continuation of the arbitration logic) are UP (i.e., outgoing SYN
> was SYN-ACKed by peer after it sent us the SYN) or CONNECTING (we sent
> outgoing SYN before we saw incoming SYN).
>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---
> net/rds/tcp.c | 1 +
> net/rds/tcp.h | 4 ++++
> net/rds/tcp_connect.c | 8 ++++++++
> net/rds/tcp_listen.c | 30 ++++++++++++++++++++----------
> 4 files changed, 33 insertions(+), 10 deletions(-)
>
[...]
> diff --git a/net/rds/tcp_connect.c b/net/rds/tcp_connect.c
> index 5cb1687..49a3fcf 100644
> --- a/net/rds/tcp_connect.c
> +++ b/net/rds/tcp_connect.c
> @@ -78,7 +78,14 @@ int rds_tcp_conn_connect(struct rds_connection *conn)
> struct socket *sock = NULL;
> struct sockaddr_in src, dest;
> int ret;
> + struct rds_tcp_connection *tc = conn->c_transport_data;
> +
> + mutex_lock(&tc->t_conn_lock);
>
> + if (rds_conn_up(conn)) {
> + mutex_unlock(&tc->t_conn_lock);
> + return 0;
> + }
> ret = sock_create_kern(rds_conn_net(conn), PF_INET,
> SOCK_STREAM, IPPROTO_TCP, &sock);
> if (ret < 0)
> @@ -120,6 +127,7 @@ int rds_tcp_conn_connect(struct rds_connection *conn)
> }
>
> out:
> + mutex_unlock(&tc->t_conn_lock);
Just wondering whether the spin_lock() would better here considering
entry into rds_tcp_conn_connect() & rds_tcp_accept_one() might be
from softirq context. Ignore it if its not applicable.
> if (sock)
> sock_release(sock);
> return ret;
> diff --git a/net/rds/tcp_listen.c b/net/rds/tcp_listen.c
> index 0896187..cc8496f 100644
> --- a/net/rds/tcp_listen.c
> +++ b/net/rds/tcp_listen.c
> @@ -76,7 +76,9 @@ int rds_tcp_accept_one(struct socket *sock)
> struct rds_connection *conn;
> int ret;
> struct inet_sock *inet;
> - struct rds_tcp_connection *rs_tcp;
> + struct rds_tcp_connection *rs_tcp = NULL;
> + int conn_state;
> + struct sock *nsk;
>
> ret = sock_create_kern(sock_net(sock->sk), sock->sk->sk_family,
> sock->sk->sk_type, sock->sk->sk_protocol,
> @@ -116,6 +118,10 @@ int rds_tcp_accept_one(struct socket *sock)
> */
> rs_tcp = (struct rds_tcp_connection *)conn->c_transport_data;
> rds_conn_transition(conn, RDS_CONN_DOWN, RDS_CONN_CONNECTING);
Like patch 1/2, probably we can leverage return value of above.
> + conn_state = rds_conn_state(conn);
> + if (conn_state != RDS_CONN_CONNECTING && conn_state != RDS_CONN_UP)
You probably don't need the local 'conn_state' and below should work.
if (!rds_conn_connecting(conn) && !rds_conn_up(conn))
Regards,
Santosh
^ permalink raw reply
* Re: [net PATCH 1/2] net: Disable segmentation if checksumming is not supported
From: Tom Herbert @ 2016-05-02 16:33 UTC (permalink / raw)
To: Alexander Duyck
Cc: Linux Kernel Network Developers, Or Gerlitz, David S. Miller,
Alexander Duyck
In-Reply-To: <20160502162510.11701.82922.stgit@ahduyck-xeon-server>
On Mon, May 2, 2016 at 9:25 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
> In the case of the mlx4 and mlx5 driver they do not support IPv6 checksum
> offload for tunnels. With this being the case we should disable GSO in
> addition to the checksum offload features when we find that a device cannot
> perform a checksum on a given packet type.
>
I'm not sure I understand this. If device can't support checksum
offload for tunnels doesn't that mean we have to do the checksum on
host regardless of whether GSO is being done?
Tom
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
> net/core/dev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 77a71cd68535..5c925ac50b95 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2802,7 +2802,7 @@ static netdev_features_t harmonize_features(struct sk_buff *skb,
>
> if (skb->ip_summed != CHECKSUM_NONE &&
> !can_checksum_protocol(features, type)) {
> - features &= ~NETIF_F_CSUM_MASK;
> + features &= ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
> } else if (illegal_highdma(skb->dev, skb)) {
> features &= ~NETIF_F_SG;
> }
>
^ permalink raw reply
* Re: [PATCH net 1/2] RDS:TCP: Synchronize rds_tcp_accept_one with rds_send_xmit when resetting t_sock
From: Sowmini Varadhan @ 2016-05-02 16:37 UTC (permalink / raw)
To: Santosh Shilimkar; +Cc: netdev, rds-devel, davem
In-Reply-To: <7fac68dc-0ff5-36a5-6a3d-df802d8db82d@oracle.com>
On (05/02/16 09:20), Santosh Shilimkar wrote:
> > rds_conn_transition(conn, RDS_CONN_DOWN, RDS_CONN_CONNECTING);
> >+ if (rs_tcp->t_sock) {
> >+ /* Need to resolve a duelling SYN between peers.
> >+ * We have an outstanding SYN to this peer, which may
> >+ * potentially have transitioned to the RDS_CONN_UP state,
> >+ * so we must quiesce any send threads before resetting
> >+ * c_transport_data.
> >+ */
> >+ wait_event(conn->c_waitq,
> >+ !test_bit(RDS_IN_XMIT, &conn->c_flags));
> Would it be good to check the return value of rds_conn_transition()
> since if CONN is already UP above will fail and then send message
> might again race and we will let message through even though passive
> hasn't finished its connection.
no, that was the original issue that I was running into, which needed
commit 241b2719 - prior to that commit, if the conn was already UP,
we'd end up doing a rds_conn_drop on a good connection, and both sides
would end up in a pair of infinite 3WH loops. Even if we dont do
a rds_conn_drop on the UP connection, we've just (before
rds_tcp_accept_one) sent out a syn-ack on the incoming syn, and now
need to RST that syn-ac. The other side is going to receive the rst,
and get confused about what to clean up (since there's already an UP
connection going on).
In short, when there is a duel, it's cleanest to have a deterministic
arbitration- both sides use the numeric value of saddr and faddr to
figure out which side is active, which side is passive. (Thus the
basis on the BGP router-id based model for 241b2719)
FWIW, much of this is actually a corner case- in practice, its not
frequent to have syns crossing each other at "almost the same time".
--Sowmini
^ permalink raw reply
* [net-next PATCH v3 0/8] Fix Tunnel features and enable GSO partial for several drivers
From: Alexander Duyck @ 2016-05-02 16:38 UTC (permalink / raw)
To: talal, netdev, michael.chan, alexander.duyck, davem, galp,
ogerlitz, eranbe
This patch series is meant to allow us to get the best performance possible
for Mellanox ConnectX-3/4 and Broadcom NetXtreme-C/E adapters in terms of
VXLAN and GRE tunnels.
The first 3 patches address issues I found in regards to GSO_PARTIAL and
TSO_MANGLEID.
The next 4 patches go through and enable GSO_PARTIAL for VXLAN tunnels that
have an outer checksum enabled, and then enable IPv6 support where I can.
One outstanding issue is that I wasn't able to get offloads working with
outer IPv6 headers on mlx4. However that wasn't a feature that was enabled
before so it isn't technically a regression, however I believe Engineers
from Mellanox said they would look into it since they thought it should be
supported.
The last patch enables GSO_PARTIAL for VXLAN and GRE tunnels on the bnxt
driver. One piece of feedback I received on the patch was that the
hardware has globally set IPv6 UDP tunnels to always have the checksum
field computed. I plan to work with Broadcom to get that addressed so that
we only populate the checksum field if it was requested by the network
stack.
v2: Rebased patches off of latest changes to the mlx4/mlx5 drivers.
Added bnxt driver patch as I received feedback on the RFC.
v3: Moved 2 patches into series for net as they were generic fixes.
Added patch to disable GSO partial if frame is less than 2x size of MSS
There are outstanding issues as called out above that need to be
addressed, however they were present before these patches so it isn't
as if they introduce a regression. In addition gains can be easily
seen so there should be no issue with applying the driver patches while
the IPv6 mlx4_en and bnxt issues are being researched.
---
Alexander Duyck (8):
gso: Do not perform partial GSO if number of partial segments is 1 or less
gso: Only allow GSO_PARTIAL if we can checksum the inner protocol
net: Fix netdev_fix_features so that TSO_MANGLEID is only available with TSO
net/mlx4_en: Add support for UDP tunnel segmentation with outer checksum offload
net/mlx4_en: Add support for inner IPv6 checksum offloads and TSO
net/mlx5e: Add support for UDP tunnel segmentation with outer checksum offload
net/mlx5e: Fix IPv6 tunnel checksum offload
bnxt: Add support for segmentation of tunnels with outer checksums
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 ++++-
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 38 +++++++++++++++++----
drivers/net/ethernet/mellanox/mlx4/en_tx.c | 15 +++++++-
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 10 ++++--
net/core/dev.c | 4 ++
net/core/skbuff.c | 11 ++++--
6 files changed, 69 insertions(+), 18 deletions(-)
^ 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