* [PATCH] ipv4: Namespaceify tcp_fastopen knob
From: Haishuang Yan @ 2017-09-12 10:30 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, Eric Dumazet
Cc: netdev, linux-kernel, Haishuang Yan
Different namespace application might require enable TCP Fast Open
feature independently of the host.
Reported-by: Luca BRUNO <lucab@debian.org>
Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
include/net/netns/ipv4.h | 2 ++
include/net/tcp.h | 1 -
net/ipv4/af_inet.c | 7 ++++---
net/ipv4/sysctl_net_ipv4.c | 42 +++++++++++++++++++++---------------------
net/ipv4/tcp.c | 4 ++--
net/ipv4/tcp_fastopen.c | 13 ++++++-------
net/ipv4/tcp_ipv4.c | 2 ++
samples/bpf/test_ipip.sh | 2 ++
8 files changed, 39 insertions(+), 34 deletions(-)
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 305e031..ea0953b 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -128,6 +128,8 @@ struct netns_ipv4 {
struct inet_timewait_death_row tcp_death_row;
int sysctl_max_syn_backlog;
int sysctl_tcp_max_orphans;
+ int sysctl_tcp_fastopen;
+ unsigned int sysctl_tcp_fastopen_blackhole_timeout;
#ifdef CONFIG_NET_L3_MASTER_DEV
int sysctl_udp_l3mdev_accept;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index ac2d998..e4cc0dd 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -240,7 +240,6 @@
/* sysctl variables for tcp */
-extern int sysctl_tcp_fastopen;
extern int sysctl_tcp_retrans_collapse;
extern int sysctl_tcp_stdurg;
extern int sysctl_tcp_rfc1337;
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index e31108e..309b849 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -195,7 +195,7 @@ int inet_listen(struct socket *sock, int backlog)
{
struct sock *sk = sock->sk;
unsigned char old_state;
- int err;
+ int err, tcp_fastopen;
lock_sock(sk);
@@ -217,8 +217,9 @@ int inet_listen(struct socket *sock, int backlog)
* because the socket was in TCP_LISTEN state previously but
* was shutdown() rather than close().
*/
- if ((sysctl_tcp_fastopen & TFO_SERVER_WO_SOCKOPT1) &&
- (sysctl_tcp_fastopen & TFO_SERVER_ENABLE) &&
+ tcp_fastopen = sock_net(sk)->ipv4.sysctl_tcp_fastopen;
+ if ((tcp_fastopen & TFO_SERVER_WO_SOCKOPT1) &&
+ (tcp_fastopen & TFO_SERVER_ENABLE) &&
!inet_csk(sk)->icsk_accept_queue.fastopenq.max_qlen) {
fastopen_queue_tune(sk, backlog);
tcp_fastopen_init_key_once(true);
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 4f26c8d3..30ebeb9 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -394,27 +394,6 @@ static int proc_tcp_available_ulp(struct ctl_table *ctl,
.proc_handler = proc_dointvec
},
{
- .procname = "tcp_fastopen",
- .data = &sysctl_tcp_fastopen,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_dointvec,
- },
- {
- .procname = "tcp_fastopen_key",
- .mode = 0600,
- .maxlen = ((TCP_FASTOPEN_KEY_LENGTH * 2) + 10),
- .proc_handler = proc_tcp_fastopen_key,
- },
- {
- .procname = "tcp_fastopen_blackhole_timeout_sec",
- .data = &sysctl_tcp_fastopen_blackhole_timeout,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_tfo_blackhole_detect_timeout,
- .extra1 = &zero,
- },
- {
.procname = "tcp_abort_on_overflow",
.data = &sysctl_tcp_abort_on_overflow,
.maxlen = sizeof(int),
@@ -1085,6 +1064,27 @@ static int proc_tcp_available_ulp(struct ctl_table *ctl,
.mode = 0644,
.proc_handler = proc_dointvec
},
+ {
+ .procname = "tcp_fastopen",
+ .data = &init_net.ipv4.sysctl_tcp_fastopen,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
+ {
+ .procname = "tcp_fastopen_key",
+ .mode = 0600,
+ .maxlen = ((TCP_FASTOPEN_KEY_LENGTH * 2) + 10),
+ .proc_handler = proc_tcp_fastopen_key,
+ },
+ {
+ .procname = "tcp_fastopen_blackhole_timeout_sec",
+ .data = &init_net.ipv4.sysctl_tcp_fastopen_blackhole_timeout,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_tfo_blackhole_detect_timeout,
+ .extra1 = &zero,
+ },
#ifdef CONFIG_IP_ROUTE_MULTIPATH
{
.procname = "fib_multipath_use_neigh",
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 39187ac..b3a2ffc 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1126,7 +1126,7 @@ static int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
struct sockaddr *uaddr = msg->msg_name;
int err, flags;
- if (!(sysctl_tcp_fastopen & TFO_CLIENT_ENABLE) ||
+ if (!(sock_net(sk)->ipv4.sysctl_tcp_fastopen & TFO_CLIENT_ENABLE) ||
(uaddr && msg->msg_namelen >= sizeof(uaddr->sa_family) &&
uaddr->sa_family == AF_UNSPEC))
return -EOPNOTSUPP;
@@ -2759,7 +2759,7 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
case TCP_FASTOPEN_CONNECT:
if (val > 1 || val < 0) {
err = -EINVAL;
- } else if (sysctl_tcp_fastopen & TFO_CLIENT_ENABLE) {
+ } else if (net->ipv4.sysctl_tcp_fastopen & TFO_CLIENT_ENABLE) {
if (sk->sk_state == TCP_CLOSE)
tp->fastopen_connect = val;
else
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index e3c3322..1bf57e8 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -9,8 +9,6 @@
#include <net/inetpeer.h>
#include <net/tcp.h>
-int sysctl_tcp_fastopen __read_mostly = TFO_CLIENT_ENABLE;
-
struct tcp_fastopen_context __rcu *tcp_fastopen_ctx;
static DEFINE_SPINLOCK(tcp_fastopen_ctx_lock);
@@ -282,18 +280,19 @@ struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
struct tcp_fastopen_cookie valid_foc = { .len = -1 };
bool syn_data = TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(skb)->seq + 1;
struct sock *child;
+ int tcp_fastopen = sock_net(sk)->ipv4.sysctl_tcp_fastopen;
if (foc->len == 0) /* Client requests a cookie */
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPFASTOPENCOOKIEREQD);
- if (!((sysctl_tcp_fastopen & TFO_SERVER_ENABLE) &&
+ if (!((tcp_fastopen & TFO_SERVER_ENABLE) &&
(syn_data || foc->len >= 0) &&
tcp_fastopen_queue_check(sk))) {
foc->len = -1;
return NULL;
}
- if (syn_data && (sysctl_tcp_fastopen & TFO_SERVER_COOKIE_NOT_REQD))
+ if (syn_data && (tcp_fastopen & TFO_SERVER_COOKIE_NOT_REQD))
goto fastopen;
if (foc->len >= 0 && /* Client presents or requests a cookie */
@@ -347,7 +346,7 @@ bool tcp_fastopen_cookie_check(struct sock *sk, u16 *mss,
return false;
}
- if (sysctl_tcp_fastopen & TFO_CLIENT_NO_COOKIE) {
+ if (sock_net(sk)->ipv4.sysctl_tcp_fastopen & TFO_CLIENT_NO_COOKIE) {
cookie->len = -1;
return true;
}
@@ -402,7 +401,6 @@ bool tcp_fastopen_defer_connect(struct sock *sk, int *err)
*/
/* Default to 1hr */
-unsigned int sysctl_tcp_fastopen_blackhole_timeout __read_mostly = 60 * 60;
static atomic_t tfo_active_disable_times __read_mostly = ATOMIC_INIT(0);
static unsigned long tfo_active_disable_stamp __read_mostly;
@@ -431,13 +429,14 @@ bool tcp_fastopen_active_should_disable(struct sock *sk)
int tfo_da_times = atomic_read(&tfo_active_disable_times);
int multiplier;
unsigned long timeout;
+ unsigned int tfo_bh_timeout = sock_net(sk)->ipv4.sysctl_tcp_fastopen_blackhole_timeout;
if (!tfo_da_times)
return false;
/* Limit timout to max: 2^6 * initial timeout */
multiplier = 1 << min(tfo_da_times - 1, 6);
- timeout = multiplier * sysctl_tcp_fastopen_blackhole_timeout * HZ;
+ timeout = multiplier * tfo_bh_timeout * HZ;
if (time_before(jiffies, tfo_active_disable_stamp + timeout))
return true;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 4b17a91..38d30ea 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2473,6 +2473,8 @@ static int __net_init tcp_sk_init(struct net *net)
net->ipv4.sysctl_tcp_window_scaling = 1;
net->ipv4.sysctl_tcp_timestamps = 1;
+ net->ipv4.sysctl_tcp_fastopen = TFO_CLIENT_ENABLE;
+ net->ipv4.sysctl_tcp_fastopen_blackhole_timeout = 60 * 60;
return 0;
fail:
tcp_sk_exit(net);
diff --git a/samples/bpf/test_ipip.sh b/samples/bpf/test_ipip.sh
index 1969254..7bbc521 100755
--- a/samples/bpf/test_ipip.sh
+++ b/samples/bpf/test_ipip.sh
@@ -173,6 +173,8 @@ function cleanup {
cleanup
echo "Testing IP tunnels..."
test_ipip
+sleep 1
test_ipip6
+sleep 1
test_ip6ip6
echo "*** PASS ***"
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] ieee802154: fix gcc-4.9 warnings
From: Stefan Schmidt @ 2017-09-12 10:24 UTC (permalink / raw)
To: Arnd Bergmann, Harry Morris, linuxdev, Alexander Aring
Cc: Marcel Holtmann, David S. Miller, Markus Elfring,
Gustavo A. R. Silva, Florian Westphal, Johannes Berg,
Christophe JAILLET, Colin Ian King, linux-wpan, netdev,
linux-kernel
In-Reply-To: <20170912101636.3811626-1-arnd@arndb.de>
Hello.
On 09/12/2017 12:16 PM, Arnd Bergmann wrote:
> All older compiler versions up to gcc-4.9 produce these
> harmless warnings:
>
> drivers/net/ieee802154/ca8210.c: In function 'ca8210_skb_tx':
> drivers/net/ieee802154/ca8210.c:1947:9: warning: missing braces around initializer [-Wmissing-braces]
>
> This changes the syntax to something that works on all versions
> without warnings.
>
> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/net/ieee802154/ca8210.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> index 24a1eabbbc9d..e6b8ce81a6c3 100644
> --- a/drivers/net/ieee802154/ca8210.c
> +++ b/drivers/net/ieee802154/ca8210.c
> @@ -1944,7 +1944,7 @@ static int ca8210_skb_tx(
> )
> {
> int status;
> - struct ieee802154_hdr header = { 0 };
> + struct ieee802154_hdr header = { };
> struct secspec secspec;
> unsigned int mac_len;
>
Acked-by: Stefan Schmidt <stefan@osg.samsung.com>
regards
Stefan Schmidt
^ permalink raw reply
* [PATCH] ieee802154: fix gcc-4.9 warnings
From: Arnd Bergmann @ 2017-09-12 10:16 UTC (permalink / raw)
To: Harry Morris, linuxdev, Alexander Aring, Stefan Schmidt
Cc: Arnd Bergmann, Marcel Holtmann, David S. Miller, Markus Elfring,
Gustavo A. R. Silva, Florian Westphal, Johannes Berg,
Christophe JAILLET, Colin Ian King, linux-wpan, netdev,
linux-kernel
All older compiler versions up to gcc-4.9 produce these
harmless warnings:
drivers/net/ieee802154/ca8210.c: In function 'ca8210_skb_tx':
drivers/net/ieee802154/ca8210.c:1947:9: warning: missing braces around initializer [-Wmissing-braces]
This changes the syntax to something that works on all versions
without warnings.
Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ieee802154/ca8210.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index 24a1eabbbc9d..e6b8ce81a6c3 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -1944,7 +1944,7 @@ static int ca8210_skb_tx(
)
{
int status;
- struct ieee802154_hdr header = { 0 };
+ struct ieee802154_hdr header = { };
struct secspec secspec;
unsigned int mac_len;
--
2.9.0
^ permalink raw reply related
* [PATCH net-next v2 2/2] net: phy: realtek: add RTL8201F phy-id and functions
From: Kunihiko Hayashi @ 2017-09-12 9:54 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, netdev
Cc: Jassi Brar, Jongsung Kim, Kunihiko Hayashi
In-Reply-To: <1505210076-32311-1-git-send-email-hayashi.kunihiko@socionext.com>
From: Jassi Brar <jaswinder.singh@linaro.org>
Add RTL8201F phy-id and the related functions to the driver.
The original patch is as follows:
https://patchwork.kernel.org/patch/2538341/
Signed-off-by: Jongsung Kim <neidhard.kim@lge.com>
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes since v1:
- use RTL821x_PAGE_SELECT instead of defining RTL8201F_PAGE_SELECT
---
drivers/net/phy/realtek.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 99c3297..d4670ec 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -29,10 +29,22 @@
#define RTL8211F_INSR 0x1d
#define RTL8211F_TX_DELAY 0x100
+#define RTL8201F_ISR 0x1e
+#define RTL8201F_IER 0x13
+
MODULE_DESCRIPTION("Realtek PHY driver");
MODULE_AUTHOR("Johnson Leung");
MODULE_LICENSE("GPL");
+static int rtl8201_ack_interrupt(struct phy_device *phydev)
+{
+ int err;
+
+ err = phy_read(phydev, RTL8201F_ISR);
+
+ return (err < 0) ? err : 0;
+}
+
static int rtl821x_ack_interrupt(struct phy_device *phydev)
{
int err;
@@ -54,6 +66,25 @@ static int rtl8211f_ack_interrupt(struct phy_device *phydev)
return (err < 0) ? err : 0;
}
+static int rtl8201_config_intr(struct phy_device *phydev)
+{
+ int err;
+
+ /* switch to page 7 */
+ phy_write(phydev, RTL821x_PAGE_SELECT, 0x7);
+
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+ err = phy_write(phydev, RTL8201F_IER,
+ BIT(13) | BIT(12) | BIT(11));
+ else
+ err = phy_write(phydev, RTL8201F_IER, 0);
+
+ /* restore to default page 0 */
+ phy_write(phydev, RTL821x_PAGE_SELECT, 0x0);
+
+ return err;
+}
+
static int rtl8211b_config_intr(struct phy_device *phydev)
{
int err;
@@ -129,6 +160,18 @@ static struct phy_driver realtek_drvs[] = {
.config_aneg = &genphy_config_aneg,
.read_status = &genphy_read_status,
}, {
+ .phy_id = 0x001cc816,
+ .name = "RTL8201F 10/100Mbps Ethernet",
+ .phy_id_mask = 0x001fffff,
+ .features = PHY_BASIC_FEATURES,
+ .flags = PHY_HAS_INTERRUPT,
+ .config_aneg = &genphy_config_aneg,
+ .read_status = &genphy_read_status,
+ .ack_interrupt = &rtl8201_ack_interrupt,
+ .config_intr = &rtl8201_config_intr,
+ .suspend = genphy_suspend,
+ .resume = genphy_resume,
+ }, {
.phy_id = 0x001cc912,
.name = "RTL8211B Gigabit Ethernet",
.phy_id_mask = 0x001fffff,
@@ -181,6 +224,7 @@ static struct phy_driver realtek_drvs[] = {
module_phy_driver(realtek_drvs);
static struct mdio_device_id __maybe_unused realtek_tbl[] = {
+ { 0x001cc816, 0x001fffff },
{ 0x001cc912, 0x001fffff },
{ 0x001cc914, 0x001fffff },
{ 0x001cc915, 0x001fffff },
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 1/2] net: phy: realtek: rename RTL8211F_PAGE_SELECT to RTL821x_PAGE_SELECT
From: Kunihiko Hayashi @ 2017-09-12 9:54 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, netdev; +Cc: Jassi Brar, Kunihiko Hayashi
This renames the definition of page select register from
RTL8211F_PAGE_SELECT to RTL821x_PAGE_SELECT to use it across models.
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
Changes since v1:
- new patch in this series
---
drivers/net/phy/realtek.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 9cbe645..99c3297 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -22,11 +22,11 @@
#define RTL821x_INER 0x12
#define RTL821x_INER_INIT 0x6400
#define RTL821x_INSR 0x13
+#define RTL821x_PAGE_SELECT 0x1f
#define RTL8211E_INER_LINK_STATUS 0x400
#define RTL8211F_INER_LINK_STATUS 0x0010
#define RTL8211F_INSR 0x1d
-#define RTL8211F_PAGE_SELECT 0x1f
#define RTL8211F_TX_DELAY 0x100
MODULE_DESCRIPTION("Realtek PHY driver");
@@ -46,10 +46,10 @@ static int rtl8211f_ack_interrupt(struct phy_device *phydev)
{
int err;
- phy_write(phydev, RTL8211F_PAGE_SELECT, 0xa43);
+ phy_write(phydev, RTL821x_PAGE_SELECT, 0xa43);
err = phy_read(phydev, RTL8211F_INSR);
/* restore to default page 0 */
- phy_write(phydev, RTL8211F_PAGE_SELECT, 0x0);
+ phy_write(phydev, RTL821x_PAGE_SELECT, 0x0);
return (err < 0) ? err : 0;
}
@@ -102,7 +102,7 @@ static int rtl8211f_config_init(struct phy_device *phydev)
if (ret < 0)
return ret;
- phy_write(phydev, RTL8211F_PAGE_SELECT, 0xd08);
+ phy_write(phydev, RTL821x_PAGE_SELECT, 0xd08);
reg = phy_read(phydev, 0x11);
/* enable TX-delay for rgmii-id and rgmii-txid, otherwise disable it */
@@ -114,7 +114,7 @@ static int rtl8211f_config_init(struct phy_device *phydev)
phy_write(phydev, 0x11, reg);
/* restore to default page 0 */
- phy_write(phydev, RTL8211F_PAGE_SELECT, 0x0);
+ phy_write(phydev, RTL821x_PAGE_SELECT, 0x0);
return 0;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v4 2/2] ip6_tunnel: fix ip6 tunnel lookup in collect_md mode
From: Haishuang Yan @ 2017-09-12 9:47 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI
Cc: netdev, linux-kernel, Haishuang Yan, Alexei Starovoitov
In-Reply-To: <1505209677-12728-1-git-send-email-yanhaishuang@cmss.chinamobile.com>
In collect_md mode, if the tun dev is down, it still can call
__ip6_tnl_rcv to receive on packets, and the rx statistics increase
improperly.
When the md tunnel is down, it's not neccessary to increase RX drops
for the tunnel device, packets would be recieved on fallback tunnel,
and the RX drops on fallback device will be increased as expected.
Fixes: 8d79266bc48c ("ip6_tunnel: add collect_md mode to IPv6 tunnels")
Cc: Alexei Starovoitov <ast@fb.com>
Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
Change since v4:
* Make the commit message more clearer
* Fix wrong recipient address
---
net/ipv6/ip6_tunnel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 10a693a..ae73164 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -171,7 +171,7 @@ static struct net_device_stats *ip6_get_stats(struct net_device *dev)
}
t = rcu_dereference(ip6n->collect_md_tun);
- if (t)
+ if (t && t->dev->flags & IFF_UP)
return t;
t = rcu_dereference(ip6n->tnls_wc[0]);
--
1.8.3.1
^ permalink raw reply related
* [PATCH v4 1/2] ip_tunnel: fix ip tunnel lookup in collect_md mode
From: Haishuang Yan @ 2017-09-12 9:47 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI
Cc: netdev, linux-kernel, Haishuang Yan, Pravin B Shelar
In collect_md mode, if the tun dev is down, it still can call
ip_tunnel_rcv to receive on packets, and the rx statistics increase
improperly.
When the md tunnel is down, it's not neccessary to increase RX drops
for the tunnel device, packets would be recieved on fallback tunnel,
and the RX drops on fallback device will be increased as expected.
Fixes: 2e15ea390e6f ("ip_gre: Add support to collect tunnel metadata.")
Cc: Pravin B Shelar <pshelar@nicira.com>
Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
Change since v4:
* Make the commit message more clearer.
* Fix wrong recipient addresss
---
net/ipv4/ip_tunnel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index e1856bf..e9805ad 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -176,7 +176,7 @@ struct ip_tunnel *ip_tunnel_lookup(struct ip_tunnel_net *itn,
return cand;
t = rcu_dereference(itn->collect_md_tun);
- if (t)
+ if (t && t->dev->flags & IFF_UP)
return t;
if (itn->fb_tunnel_dev && itn->fb_tunnel_dev->flags & IFF_UP)
--
1.8.3.1
^ permalink raw reply related
* Re: [Patch net v3 1/3] net_sched: get rid of tcfa_rcu
From: Jiri Pirko @ 2017-09-12 9:42 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, jiri, jakub.kicinski, jhs, Eric Dumazet
In-Reply-To: <20170911233332.7594-2-xiyou.wangcong@gmail.com>
Tue, Sep 12, 2017 at 01:33:30AM CEST, xiyou.wangcong@gmail.com wrote:
>gen estimator has been rewritten in commit 1c0d32fde5bd
>("net_sched: gen_estimator: complete rewrite of rate estimators"),
>the caller is no longer needed to wait for a grace period.
>So this patch gets rid of it.
>
>This also completely closes a race condition between action free
>path and filter chain add/remove path for the following patch.
>Because otherwise the nested RCU callback can't be caught by
>rcu_barrier().
>
>Please see also the comments in code.
Looks like this is causing a null pointer dereference bug for me, 100%
of the time. Just add and remove any rule with action and you get:
[ 598.599825] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030
[ 598.607782] IP: tcf_action_destroy+0xc0/0x140
[ 598.612231] PGD 0 P4D 0
[ 598.614797] Oops: 0000 [#1] SMP KASAN
[ 598.618525] Modules linked in: act_gact cls_flower sch_ingress rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache intel_rapl x86_pkg_temp_thermal coretemp mlxsw_spectrum kvm_intel mlxfw kvm parman bridge sunrpc irqbypass iTCO_wdt iTCO_vendor_support stp crct10dif_pclmul llc crc32_pclmul crc32c_intel mlxsw_pci ghash_clmulni_intel mlxsw_core i2c_i801 e1000e pcspkr ptp tpm_tis mei_me pps_core mei tpm_tis_core lpc_ich tpm shpchp video
[ 598.659010] CPU: 1 PID: 758 Comm: bash Tainted: G B 4.13.0jiri+ #70
[ 598.666509] Hardware name: Mellanox Technologies Ltd. Mellanox switch/Mellanox x86 mezzanine board, BIOS 4.6.5 08/02/2016
[ 598.677630] task: ffff880371624bc0 task.stack: ffff880387808000
[ 598.683648] RIP: 0010:tcf_action_destroy+0xc0/0x140
[ 598.688617] RSP: 0018:ffff88038d107cb8 EFLAGS: 00010282
[ 598.693922] RAX: 0000000000000000 RBX: ffff88038d107d28 RCX: ffffffff820b80e0
[ 598.701184] RDX: 0000000000000000 RSI: 0000000000000008 RDI: 0000000000000030
[ 598.708405] RBP: ffff88038d107ce8 R08: 0000000000000001 R09: 0000000000000001
[ 598.715607] R10: ffff88038d107b27 R11: fffffbfff0bcf36c R12: 0000000000000000
[ 598.722816] R13: ffff88038d107d38 R14: ffff88036bf75650 R15: 0000000000000001
[ 598.730047] FS: 00007f398050b700(0000) GS:ffff88038d100000(0000) knlGS:0000000000000000
[ 598.738253] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 598.744086] CR2: 0000000000000030 CR3: 0000000371ac4001 CR4: 00000000001606e0
[ 598.751328] Call Trace:
[ 598.753809] <IRQ>
[ 598.755871] tcf_exts_destroy+0x17f/0x260
[ 598.775969] fl_destroy_filter+0x1d/0x30 [cls_flower]
[ 598.781069] rcu_process_callbacks+0x6b2/0xe00
Kasan says:
[ 597.503005] BUG: KASAN: use-after-free in tcf_action_destroy+0xad/0x140
[ 597.509751] Read of size 8 at addr ffff88036bf75640 by task bash/758
[ 597.516222]
[ 597.517761] CPU: 1 PID: 758 Comm: bash Not tainted 4.13.0jiri+ #70
[ 597.524075] Hardware name: Mellanox Technologies Ltd. Mellanox switch/Mellanox x86 mezzanine board, BIOS 4.6.5 08/02/2016
[ 597.535132] Call Trace:
[ 597.537630] <IRQ>
[ 597.539718] dump_stack+0xd5/0x150
[ 597.554853] print_address_description+0x86/0x410
[ 597.559667] kasan_report+0x181/0x4c0
[ 597.583360] tcf_action_destroy+0xad/0x140
[ 597.587551] tcf_exts_destroy+0x17f/0x260
Ubsan says:
[ 598.184033] UBSAN: Undefined behaviour in net/sched/act_api.c:523:4
[ 598.190409] member access within null pointer of type 'const struct tc_action_ops'
[ 598.198076] CPU: 1 PID: 758 Comm: bash Tainted: G B 4.13.0jiri+ #70
[ 598.205570] Hardware name: Mellanox Technologies Ltd. Mellanox switch/Mellanox x86 mezzanine board, BIOS 4.6.5 08/02/2016
[ 598.216669] Call Trace:
[ 598.219157] <IRQ>
[ 598.221245] dump_stack+0xd5/0x150
[ 598.228703] ubsan_epilogue+0xd/0x4e
[ 598.232333] __ubsan_handle_type_mismatch+0xf2/0x293
[ 598.252880] tcf_action_destroy+0x115/0x140
[ 598.257151] tcf_exts_destroy+0x17f/0x260
[ 598.277336] fl_destroy_filter+0x1d/0x30 [cls_flower]
[ 598.282472] rcu_process_callbacks+0x6b2/0xe00
Looks like you need to save owner of the module before you call
__tcf_idr_release so you can later on use it for module_put
^ permalink raw reply
* RE: [PATCH] tipc: Use bsearch library function
From: David Laight @ 2017-09-12 9:24 UTC (permalink / raw)
To: 'David Miller', thomas@m3y3r.de
Cc: jon.maloy@ericsson.com, ying.xue@windriver.com,
netdev@vger.kernel.org, tipc-discussion@lists.sourceforge.net,
linux-kernel@vger.kernel.org
In-Reply-To: <20170911.143025.555018840006192902.davem@davemloft.net>
From: David Miller
> Sent: 11 September 2017 22:30
> From: Thomas Meyer <thomas@m3y3r.de>
> Date: Sat, 9 Sep 2017 05:18:19 +0200
>
> > @@ -168,6 +169,18 @@ static struct name_seq *tipc_nameseq_create(u32 type, struct hlist_head
> *seq_hea
> > return nseq;
> > }
> >
> > +static int nameseq_find_subseq_cmp(const void *key, const void *elt)
> > +{
> > + u32 instance = *(u32 *)key;
> > + struct sub_seq *sseq = (struct sub_seq *)elt;
>
> Please order local variables from longest to shortest (ie. reverse
> christmas tree).
You probably just need to remove the unnecessary cast of 'void *'.
Although adding the 'const' qualifier will make it wrong again.
You probably ought to make the 'key' a structure - even if it only
contains a single u32.
Casting pointers to numeric types is often wrong.
David
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: ethernet: socionext: add AVE ethernet driver
From: Kunihiko Hayashi @ 2017-09-12 9:24 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, David S. Miller, Florian Fainelli, Rob Herring,
Mark Rutland, linux-arm-kernel, linux-kernel, devicetree,
Masahiro Yamada, Masami Hiramatsu, Jassi Brar
In-Reply-To: <20170911120009.GA24174@lunn.ch>
Hi Andrew,
On Mon, 11 Sep 2017 14:00:09 +0200 Andrew Lunn <andrew@lunn.ch> wrote:
> > > > +static irqreturn_t ave_interrupt(int irq, void *netdev)
> > > > +{
> > > > + struct net_device *ndev = (struct net_device *)netdev;
> > > > + struct ave_private *priv = netdev_priv(ndev);
> > > > + u32 gimr_val, gisr_val;
> > > > +
> > > > + gimr_val = ave_irq_disable_all(ndev);
> > > > +
> > > > + /* get interrupt status */
> > > > + gisr_val = ave_r32(ndev, AVE_GISR);
> > > > +
> > > > + /* PHY */
> > > > + if (gisr_val & AVE_GI_PHY) {
> > > > + ave_w32(ndev, AVE_GISR, AVE_GI_PHY);
> > > > + if (priv->internal_phy_interrupt)
> > > > + phy_mac_interrupt(ndev->phydev, ndev->phydev->link);
> > >
> > > Humm. I don't think this is correct. You are supposed to give it the
> > > new link state, not the old.
> > >
> > > What does a PHY interrupt mean here?
> >
> > In the general case, I think PHY events like changing link state are transmitted
> > to CPU as interrupt via interrupt controller, then PHY driver itself can handle
> > the interrupt.
> >
> > And in this case, PHY events are transmitted to MAC as one of its interrupt factor,
> > then I thought that MAC driver had to tell the events to PHY.
>
> Could this be in-band SGMI signalling from the PHY to the MAC? Does
> the documentation give examples of when this interrupt will happen?
>
> Andrew
Unfortunately this MAC doesn't support SGMII.
And there aren't any examples of when this interrupt will happen.
This interrupt happens after ave_open() is called and link is established.
However, I found that auto negotiation didn't start when this interrupt wasn't handled.
Although ave_init() calls phy_start_aneg(), it doesn't make sense
because phy driver doesn't start yet.
And according to Florian's comment in ave_init(),
> + phy_start_interrupts(phydev);
> +
> + phy_start_aneg(phydev);
>
> No, no, phy_start() would take care of all of that correctly for you,
> you don't have have to do it just there because ave_open() eventually
> calls phy_start() for you, so just drop these two calls.
When moving phy_start_aneg() to ave_open(), the handler doesn't need to call
phy_mac_interrupt() and I can remove it from the handler.
---
Best Regards,
Kunihiko Hayashi
^ permalink raw reply
* Re: [PATCH iproute2 1/2] lib/libnetlink: re malloc buff if size is not enough
From: Michal Kubecek @ 2017-09-12 9:09 UTC (permalink / raw)
To: Hangbin Liu; +Cc: Phil Sutter, netdev, Stephen Hemminger
In-Reply-To: <20170912084748.m4wz6kxfclc2mkxd@unicorn.suse.cz>
On Tue, Sep 12, 2017 at 10:47:48AM +0200, Michal Kubecek wrote:
> On Mon, Sep 11, 2017 at 03:19:55PM +0800, Hangbin Liu wrote:
> > On Fri, Sep 08, 2017 at 04:51:13PM +0200, Phil Sutter wrote:
> > > Regarding Michal's concern about reentrancy, maybe we should go into a
> > > different direction and make rtnl_recvmsg() return a newly allocated
> > > buffer which the caller has to free.
> >
> > Hmm... But we could not free the buf in __rtnl_talk(). Because in
> > __rtnl_talk() we assign the answer with the buf address and return to caller.
> >
> > for (h = (struct nlmsghdr *)buf; status >= sizeof(*h); ) {
> > [...]
> > if (answer) {
> > *answer= h;
> > return 0;
> > }
> > }
> >
> > And the caller will keep use it in later code. Since there are plenty of
> > functions called rtnl_talk. I think it would be much more complex to free
> > the buffer every time.
> >
> >
> > Hi Michal,
> >
> > Would you like to tell me more about your concern with reentrancy? It's looks
> > arpd doesn't call rtnl_talk() or rtnl_dump_filter_l().
>
> I checked again and arpd indeed isn't a problem. It doesn't seem to call
> any of the two functions (directly or indirectly) and while it's linked
> with "-lpthread", it's not really multithreaded.
>
> But my concern was rather about other potential users of libnetlink
> (i.e. those which are not part of iproute2). I must admit, though, that
> I'm not sure if libnetlink code is reentrant as of now. (And people are
> discouraged from using it in its own manual page.)
>
> That being said, I still like Phil's idea for a different reason. While
> investigating the issue with "ip link show dev eth ..." which led me to
> commit 6599162b958e ("iplink: check for message truncation in
> iplink_get()"), I quickly peeked at some other callers of rtnl_talk()
> and I'm afraid there may be others which wouldn't handle truncated
> message correctly. I assume the maxlen argument was always chosen to be
> sufficient for any expected messages but as the example of iplink_get()
> shows, messages returned by kernel my grow over time.
>
> That's why I like the idea of __rtnl_talk() returning a pointer to newly
> allocated buffer (of sufficient size) rather than copying the response
> into a buffer provided by caller and potentially truncating it.
I'm sorry, I managed to forget that your patch 2 does already address
this problem. But the fact that any caller must keep in mind that he
must not call the same function again until the previous response is no
longer needed still feels like a trap. It's something you need to keep
in mind (where "you" in fact means any future contributor) and it's
easy to forget. That's why I prefer the reentrant functions like
strerror_r() or localtime_r() even in code which is not intended to be
multithreaded. Getting an object which is "mine" to do with whatever
I want until I no longer need it feels like a cleaner interface to me.
Michal Kubecek
^ permalink raw reply
* scheduling while atomic from vmci_transport_recv_stream_cb in 3.16 kernels
From: Michal Hocko @ 2017-09-12 9:08 UTC (permalink / raw)
To: Jorgen Hansen
Cc: Aditya Asarwade, Thomas Hellstrom, LKML, netdev, Masik Petr,
Ben Hutchings, Sasha Levin, Stable tree
Hi,
we are seeing the following splat with Debian 3.16 stable kernel
BUG: scheduling while atomic: MATLAB/26771/0x00000100
Modules linked in: veeamsnap(O) hmac cbc cts nfsv4 dns_resolver rpcsec_gss_krb5 nfsd auth_rpcgss oid_registry nfs_acl nfs lockd fscache sunrpc vmw_vso$
CPU: 0 PID: 26771 Comm: MATLAB Tainted: G O 3.16.0-4-amd64 #1 Debian 3.16.7-ckt20-1+deb8u3
Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 09/21/2015
ffff88315c1e4c20 ffffffff8150db3f ffff88193f803dc8 ffffffff8150acdf
ffffffff815103a2 0000000000012f00 ffff8819423dbfd8 0000000000012f00
ffff88315c1e4c20 ffff88193f803dc8 ffff88193f803d50 ffff88193f803dc0
Call Trace:
<IRQ> [<ffffffff8150db3f>] ? dump_stack+0x41/0x51
[<ffffffff8150acdf>] ? __schedule_bug+0x48/0x55
[<ffffffff815103a2>] ? __schedule+0x5d2/0x700
[<ffffffff8150f9b9>] ? schedule_timeout+0x229/0x2a0
[<ffffffff8109ba70>] ? select_task_rq_fair+0x390/0x700
[<ffffffff8109f780>] ? check_preempt_wakeup+0x120/0x1d0
[<ffffffff81510eb8>] ? wait_for_completion+0xa8/0x120
[<ffffffff81096de0>] ? wake_up_state+0x10/0x10
[<ffffffff810c3da0>] ? call_rcu_bh+0x20/0x20
[<ffffffff810c180b>] ? wait_rcu_gp+0x4b/0x60
[<ffffffff810c17b0>] ? ftrace_raw_output_rcu_utilization+0x40/0x40
[<ffffffffa02ca6f5>] ? vmci_event_unsubscribe+0x75/0xb0 [vmw_vmci]
[<ffffffffa031f5cd>] ? vmci_transport_destruct+0x1d/0xe0 [vmw_vsock_vmci_transport]
[<ffffffffa03167e3>] ? vsock_sk_destruct+0x13/0x60 [vsock]
[<ffffffff81409f7a>] ? __sk_free+0x1a/0x130
[<ffffffffa0320218>] ? vmci_transport_recv_stream_cb+0x1e8/0x2d0 [vmw_vsock_vmci_transport]
[<ffffffffa02c9cba>] ? vmci_datagram_invoke_guest_handler+0xaa/0xd0 [vmw_vmci]
[<ffffffffa02cab51>] ? vmci_dispatch_dgs+0xc1/0x200 [vmw_vmci]
[<ffffffff8106c294>] ? tasklet_action+0xf4/0x100
[<ffffffff8106c681>] ? __do_softirq+0xf1/0x290
[<ffffffff8106ca55>] ? irq_exit+0x95/0xa0
[<ffffffff81516b22>] ? do_IRQ+0x52/0xe0
[<ffffffff8151496d>] ? common_interrupt+0x6d/0x6d
AFAICS this has been fixed by 4ef7ea9195ea ("VSOCK: sock_put wasn't safe
to call in interrupt context") but this patch hasn't been backported to
stable trees. It applies cleanly on top of 3.16 stable tree but I am not
familiar with the code to send the backport to the stable maintainer
directly.
Could you double check that the patch below (just a blind cherry-pick)
is correct and it doesn't need additional patches on top?
Ben could you take this to your stable 3.16 branch if the patch is correct?
I am CCing Sasha for 4.1 stable tree as well. I haven't checked whether
pre 3.16 kernels are affected as well.
---
commit fac774c40b5c512113b6373cad498f35bee7a409
Author: Jorgen Hansen <jhansen@vmware.com>
Date: Wed Oct 21 04:53:56 2015 -0700
VSOCK: sock_put wasn't safe to call in interrupt context
commit 4ef7ea9195ea73262cd9730fb54e1eb726da157b upstream.
In the vsock vmci_transport driver, sock_put wasn't safe to call
in interrupt context, since that may call the vsock destructor
which in turn calls several functions that should only be called
from process context. This change defers the callling of these
functions to a worker thread. All these functions were
deallocation of resources related to the transport itself.
Furthermore, an unused callback was removed to simplify the
cleanup.
Multiple customers have been hitting this issue when using
VMware tools on vSphere 2015.
Also added a version to the vmci transport module (starting from
1.0.2.0-k since up until now it appears that this module was
sharing version with vsock that is currently at 1.0.1.0-k).
Reviewed-by: Aditya Asarwade <asarwade@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Jorgen Hansen <jhansen@vmware.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
index 9bb63ffec4f2..aed136d27b01 100644
--- a/net/vmw_vsock/vmci_transport.c
+++ b/net/vmw_vsock/vmci_transport.c
@@ -40,13 +40,11 @@
static int vmci_transport_recv_dgram_cb(void *data, struct vmci_datagram *dg);
static int vmci_transport_recv_stream_cb(void *data, struct vmci_datagram *dg);
-static void vmci_transport_peer_attach_cb(u32 sub_id,
- const struct vmci_event_data *ed,
- void *client_data);
static void vmci_transport_peer_detach_cb(u32 sub_id,
const struct vmci_event_data *ed,
void *client_data);
static void vmci_transport_recv_pkt_work(struct work_struct *work);
+static void vmci_transport_cleanup(struct work_struct *work);
static int vmci_transport_recv_listen(struct sock *sk,
struct vmci_transport_packet *pkt);
static int vmci_transport_recv_connecting_server(
@@ -75,6 +73,10 @@ struct vmci_transport_recv_pkt_info {
struct vmci_transport_packet pkt;
};
+static LIST_HEAD(vmci_transport_cleanup_list);
+static DEFINE_SPINLOCK(vmci_transport_cleanup_lock);
+static DECLARE_WORK(vmci_transport_cleanup_work, vmci_transport_cleanup);
+
static struct vmci_handle vmci_transport_stream_handle = { VMCI_INVALID_ID,
VMCI_INVALID_ID };
static u32 vmci_transport_qp_resumed_sub_id = VMCI_INVALID_ID;
@@ -791,44 +793,6 @@ static int vmci_transport_recv_stream_cb(void *data, struct vmci_datagram *dg)
return err;
}
-static void vmci_transport_peer_attach_cb(u32 sub_id,
- const struct vmci_event_data *e_data,
- void *client_data)
-{
- struct sock *sk = client_data;
- const struct vmci_event_payload_qp *e_payload;
- struct vsock_sock *vsk;
-
- e_payload = vmci_event_data_const_payload(e_data);
-
- vsk = vsock_sk(sk);
-
- /* We don't ask for delayed CBs when we subscribe to this event (we
- * pass 0 as flags to vmci_event_subscribe()). VMCI makes no
- * guarantees in that case about what context we might be running in,
- * so it could be BH or process, blockable or non-blockable. So we
- * need to account for all possible contexts here.
- */
- local_bh_disable();
- bh_lock_sock(sk);
-
- /* XXX This is lame, we should provide a way to lookup sockets by
- * qp_handle.
- */
- if (vmci_handle_is_equal(vmci_trans(vsk)->qp_handle,
- e_payload->handle)) {
- /* XXX This doesn't do anything, but in the future we may want
- * to set a flag here to verify the attach really did occur and
- * we weren't just sent a datagram claiming it was.
- */
- goto out;
- }
-
-out:
- bh_unlock_sock(sk);
- local_bh_enable();
-}
-
static void vmci_transport_handle_detach(struct sock *sk)
{
struct vsock_sock *vsk;
@@ -871,28 +835,38 @@ static void vmci_transport_peer_detach_cb(u32 sub_id,
const struct vmci_event_data *e_data,
void *client_data)
{
- struct sock *sk = client_data;
+ struct vmci_transport *trans = client_data;
const struct vmci_event_payload_qp *e_payload;
- struct vsock_sock *vsk;
e_payload = vmci_event_data_const_payload(e_data);
- vsk = vsock_sk(sk);
- if (vmci_handle_is_invalid(e_payload->handle))
- return;
-
- /* Same rules for locking as for peer_attach_cb(). */
- local_bh_disable();
- bh_lock_sock(sk);
/* XXX This is lame, we should provide a way to lookup sockets by
* qp_handle.
*/
- if (vmci_handle_is_equal(vmci_trans(vsk)->qp_handle,
- e_payload->handle))
- vmci_transport_handle_detach(sk);
+ if (vmci_handle_is_invalid(e_payload->handle) ||
+ vmci_handle_is_equal(trans->qp_handle, e_payload->handle))
+ return;
- bh_unlock_sock(sk);
- local_bh_enable();
+ /* We don't ask for delayed CBs when we subscribe to this event (we
+ * pass 0 as flags to vmci_event_subscribe()). VMCI makes no
+ * guarantees in that case about what context we might be running in,
+ * so it could be BH or process, blockable or non-blockable. So we
+ * need to account for all possible contexts here.
+ */
+ spin_lock_bh(&trans->lock);
+ if (!trans->sk)
+ goto out;
+
+ /* Apart from here, trans->lock is only grabbed as part of sk destruct,
+ * where trans->sk isn't locked.
+ */
+ bh_lock_sock(trans->sk);
+
+ vmci_transport_handle_detach(trans->sk);
+
+ bh_unlock_sock(trans->sk);
+ out:
+ spin_unlock_bh(&trans->lock);
}
static void vmci_transport_qp_resumed_cb(u32 sub_id,
@@ -1181,7 +1155,7 @@ vmci_transport_recv_connecting_server(struct sock *listener,
*/
err = vmci_event_subscribe(VMCI_EVENT_QP_PEER_DETACH,
vmci_transport_peer_detach_cb,
- pending, &detach_sub_id);
+ vmci_trans(vpending), &detach_sub_id);
if (err < VMCI_SUCCESS) {
vmci_transport_send_reset(pending, pkt);
err = vmci_transport_error_to_vsock_error(err);
@@ -1321,7 +1295,6 @@ vmci_transport_recv_connecting_client(struct sock *sk,
|| vmci_trans(vsk)->qpair
|| vmci_trans(vsk)->produce_size != 0
|| vmci_trans(vsk)->consume_size != 0
- || vmci_trans(vsk)->attach_sub_id != VMCI_INVALID_ID
|| vmci_trans(vsk)->detach_sub_id != VMCI_INVALID_ID) {
skerr = EPROTO;
err = -EINVAL;
@@ -1389,7 +1362,6 @@ static int vmci_transport_recv_connecting_client_negotiate(
struct vsock_sock *vsk;
struct vmci_handle handle;
struct vmci_qp *qpair;
- u32 attach_sub_id;
u32 detach_sub_id;
bool is_local;
u32 flags;
@@ -1399,7 +1371,6 @@ static int vmci_transport_recv_connecting_client_negotiate(
vsk = vsock_sk(sk);
handle = VMCI_INVALID_HANDLE;
- attach_sub_id = VMCI_INVALID_ID;
detach_sub_id = VMCI_INVALID_ID;
/* If we have gotten here then we should be past the point where old
@@ -1444,23 +1415,15 @@ static int vmci_transport_recv_connecting_client_negotiate(
goto destroy;
}
- /* Subscribe to attach and detach events first.
+ /* Subscribe to detach events first.
*
* XXX We attach once for each queue pair created for now so it is easy
* to find the socket (it's provided), but later we should only
* subscribe once and add a way to lookup sockets by queue pair handle.
*/
- err = vmci_event_subscribe(VMCI_EVENT_QP_PEER_ATTACH,
- vmci_transport_peer_attach_cb,
- sk, &attach_sub_id);
- if (err < VMCI_SUCCESS) {
- err = vmci_transport_error_to_vsock_error(err);
- goto destroy;
- }
-
err = vmci_event_subscribe(VMCI_EVENT_QP_PEER_DETACH,
vmci_transport_peer_detach_cb,
- sk, &detach_sub_id);
+ vmci_trans(vsk), &detach_sub_id);
if (err < VMCI_SUCCESS) {
err = vmci_transport_error_to_vsock_error(err);
goto destroy;
@@ -1496,7 +1459,6 @@ static int vmci_transport_recv_connecting_client_negotiate(
vmci_trans(vsk)->produce_size = vmci_trans(vsk)->consume_size =
pkt->u.size;
- vmci_trans(vsk)->attach_sub_id = attach_sub_id;
vmci_trans(vsk)->detach_sub_id = detach_sub_id;
vmci_trans(vsk)->notify_ops->process_negotiate(sk);
@@ -1504,9 +1466,6 @@ static int vmci_transport_recv_connecting_client_negotiate(
return 0;
destroy:
- if (attach_sub_id != VMCI_INVALID_ID)
- vmci_event_unsubscribe(attach_sub_id);
-
if (detach_sub_id != VMCI_INVALID_ID)
vmci_event_unsubscribe(detach_sub_id);
@@ -1607,9 +1566,11 @@ static int vmci_transport_socket_init(struct vsock_sock *vsk,
vmci_trans(vsk)->qp_handle = VMCI_INVALID_HANDLE;
vmci_trans(vsk)->qpair = NULL;
vmci_trans(vsk)->produce_size = vmci_trans(vsk)->consume_size = 0;
- vmci_trans(vsk)->attach_sub_id = vmci_trans(vsk)->detach_sub_id =
- VMCI_INVALID_ID;
+ vmci_trans(vsk)->detach_sub_id = VMCI_INVALID_ID;
vmci_trans(vsk)->notify_ops = NULL;
+ INIT_LIST_HEAD(&vmci_trans(vsk)->elem);
+ vmci_trans(vsk)->sk = &vsk->sk;
+ vmci_trans(vsk)->lock = __SPIN_LOCK_UNLOCKED(vmci_trans(vsk)->lock);
if (psk) {
vmci_trans(vsk)->queue_pair_size =
vmci_trans(psk)->queue_pair_size;
@@ -1629,29 +1590,57 @@ static int vmci_transport_socket_init(struct vsock_sock *vsk,
return 0;
}
-static void vmci_transport_destruct(struct vsock_sock *vsk)
+static void vmci_transport_free_resources(struct list_head *transport_list)
{
- if (vmci_trans(vsk)->attach_sub_id != VMCI_INVALID_ID) {
- vmci_event_unsubscribe(vmci_trans(vsk)->attach_sub_id);
- vmci_trans(vsk)->attach_sub_id = VMCI_INVALID_ID;
- }
+ while (!list_empty(transport_list)) {
+ struct vmci_transport *transport =
+ list_first_entry(transport_list, struct vmci_transport,
+ elem);
+ list_del(&transport->elem);
- if (vmci_trans(vsk)->detach_sub_id != VMCI_INVALID_ID) {
- vmci_event_unsubscribe(vmci_trans(vsk)->detach_sub_id);
- vmci_trans(vsk)->detach_sub_id = VMCI_INVALID_ID;
- }
+ if (transport->detach_sub_id != VMCI_INVALID_ID) {
+ vmci_event_unsubscribe(transport->detach_sub_id);
+ transport->detach_sub_id = VMCI_INVALID_ID;
+ }
- if (!vmci_handle_is_invalid(vmci_trans(vsk)->qp_handle)) {
- vmci_qpair_detach(&vmci_trans(vsk)->qpair);
- vmci_trans(vsk)->qp_handle = VMCI_INVALID_HANDLE;
- vmci_trans(vsk)->produce_size = 0;
- vmci_trans(vsk)->consume_size = 0;
+ if (!vmci_handle_is_invalid(transport->qp_handle)) {
+ vmci_qpair_detach(&transport->qpair);
+ transport->qp_handle = VMCI_INVALID_HANDLE;
+ transport->produce_size = 0;
+ transport->consume_size = 0;
+ }
+
+ kfree(transport);
}
+}
+
+static void vmci_transport_cleanup(struct work_struct *work)
+{
+ LIST_HEAD(pending);
+
+ spin_lock_bh(&vmci_transport_cleanup_lock);
+ list_replace_init(&vmci_transport_cleanup_list, &pending);
+ spin_unlock_bh(&vmci_transport_cleanup_lock);
+ vmci_transport_free_resources(&pending);
+}
+
+static void vmci_transport_destruct(struct vsock_sock *vsk)
+{
+ /* Ensure that the detach callback doesn't use the sk/vsk
+ * we are about to destruct.
+ */
+ spin_lock_bh(&vmci_trans(vsk)->lock);
+ vmci_trans(vsk)->sk = NULL;
+ spin_unlock_bh(&vmci_trans(vsk)->lock);
if (vmci_trans(vsk)->notify_ops)
vmci_trans(vsk)->notify_ops->socket_destruct(vsk);
- kfree(vsk->trans);
+ spin_lock_bh(&vmci_transport_cleanup_lock);
+ list_add(&vmci_trans(vsk)->elem, &vmci_transport_cleanup_list);
+ spin_unlock_bh(&vmci_transport_cleanup_lock);
+ schedule_work(&vmci_transport_cleanup_work);
+
vsk->trans = NULL;
}
@@ -2148,6 +2137,9 @@ module_init(vmci_transport_init);
static void __exit vmci_transport_exit(void)
{
+ cancel_work_sync(&vmci_transport_cleanup_work);
+ vmci_transport_free_resources(&vmci_transport_cleanup_list);
+
if (!vmci_handle_is_invalid(vmci_transport_stream_handle)) {
if (vmci_datagram_destroy_handle(
vmci_transport_stream_handle) != VMCI_SUCCESS)
@@ -2166,6 +2158,7 @@ module_exit(vmci_transport_exit);
MODULE_AUTHOR("VMware, Inc.");
MODULE_DESCRIPTION("VMCI transport for Virtual Sockets");
+MODULE_VERSION("1.0.2.0-k");
MODULE_LICENSE("GPL v2");
MODULE_ALIAS("vmware_vsock");
MODULE_ALIAS_NETPROTO(PF_VSOCK);
diff --git a/net/vmw_vsock/vmci_transport.h b/net/vmw_vsock/vmci_transport.h
index ce6c9623d5f0..2ad46f39649f 100644
--- a/net/vmw_vsock/vmci_transport.h
+++ b/net/vmw_vsock/vmci_transport.h
@@ -119,10 +119,12 @@ struct vmci_transport {
u64 queue_pair_size;
u64 queue_pair_min_size;
u64 queue_pair_max_size;
- u32 attach_sub_id;
u32 detach_sub_id;
union vmci_transport_notify notify;
struct vmci_transport_notify_ops *notify_ops;
+ struct list_head elem;
+ struct sock *sk;
+ spinlock_t lock; /* protects sk. */
};
int vmci_transport_register(void);
--
Michal Hocko
SUSE Labs
^ permalink raw reply related
* Re: [PATCH iproute2 1/2] lib/libnetlink: re malloc buff if size is not enough
From: Michal Kubecek @ 2017-09-12 8:47 UTC (permalink / raw)
To: Hangbin Liu; +Cc: Phil Sutter, netdev, Stephen Hemminger
In-Reply-To: <20170911071955.GZ5465@leo.usersys.redhat.com>
On Mon, Sep 11, 2017 at 03:19:55PM +0800, Hangbin Liu wrote:
> On Fri, Sep 08, 2017 at 04:51:13PM +0200, Phil Sutter wrote:
> > Regarding Michal's concern about reentrancy, maybe we should go into a
> > different direction and make rtnl_recvmsg() return a newly allocated
> > buffer which the caller has to free.
>
> Hmm... But we could not free the buf in __rtnl_talk(). Because in
> __rtnl_talk() we assign the answer with the buf address and return to caller.
>
> for (h = (struct nlmsghdr *)buf; status >= sizeof(*h); ) {
> [...]
> if (answer) {
> *answer= h;
> return 0;
> }
> }
>
> And the caller will keep use it in later code. Since there are plenty of
> functions called rtnl_talk. I think it would be much more complex to free
> the buffer every time.
>
>
> Hi Michal,
>
> Would you like to tell me more about your concern with reentrancy? It's looks
> arpd doesn't call rtnl_talk() or rtnl_dump_filter_l().
I checked again and arpd indeed isn't a problem. It doesn't seem to call
any of the two functions (directly or indirectly) and while it's linked
with "-lpthread", it's not really multithreaded.
But my concern was rather about other potential users of libnetlink
(i.e. those which are not part of iproute2). I must admit, though, that
I'm not sure if libnetlink code is reentrant as of now. (And people are
discouraged from using it in its own manual page.)
That being said, I still like Phil's idea for a different reason. While
investigating the issue with "ip link show dev eth ..." which led me to
commit 6599162b958e ("iplink: check for message truncation in
iplink_get()"), I quickly peeked at some other callers of rtnl_talk()
and I'm afraid there may be others which wouldn't handle truncated
message correctly. I assume the maxlen argument was always chosen to be
sufficient for any expected messages but as the example of iplink_get()
shows, messages returned by kernel my grow over time.
That's why I like the idea of __rtnl_talk() returning a pointer to newly
allocated buffer (of sufficient size) rather than copying the response
into a buffer provided by caller and potentially truncating it.
Michal Kubecek
^ permalink raw reply
* Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs
From: Corentin Labbe @ 2017-09-12 7:54 UTC (permalink / raw)
To: Andrew Lunn
Cc: robh+dt, mark.rutland, maxime.ripard, wens, linux,
catalin.marinas, will.deacon, peppe.cavallaro, alexandre.torgue,
f.fainelli, netdev, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20170911201920.GA5983@lunn.ch>
On Mon, Sep 11, 2017 at 10:19:20PM +0200, Andrew Lunn wrote:
> > Even with CLK_BUS_EPHY/RST_BUS_EPHY enabled, the MAC reset timeout.
> > So no the CLK/RST are really for the PHY.
>
> Thanks for trying that.
>
> You said it was probably during scanning of the bus it times out. What
> address is causing the timeout? 0 or 1? If the internal bus can only
> have one PHY on it, maybe we need to set bus->phy_mask to 0x1?
>
I have added a trace in begin and end of stmmac_mdio_read()
[ 18.145451] libphy: stmmac: probed
[ 18.148398] libphy: mdio_mux: probed
[ 18.148650] dwmac-sun8i 1c30000.ethernet: Switch mux to internal PHY
[ 18.248751] dwmac-sun8i 1c30000.ethernet: EMAC reset timeout
[ 18.249297] libphy: mdio_mux: probed
[ 18.249362] dwmac-sun8i 1c30000.ethernet: Switch mux to external PHY
[ 18.249391] stmmac_mdio_read 0 2
[ 18.249598] stmmac_mdio_read 0 2 1c
[ 18.249623] stmmac_mdio_read 0 3
[ 18.249811] stmmac_mdio_read 0 3 c915
[ 20.737271] EXT4-fs (mmcblk0p1): re-mounted. Opts: (null)
[ 31.294868] stmmac_mdio_read 0 0
[ 31.295311] stmmac_mdio_read 0 0 1140
It seems that the timeout is unrelated to MDIO bus.
Regards
^ permalink raw reply
* [PATCH v2] geneve: Fix setting ttl value in collect metadata mode
From: Haishuang Yan @ 2017-09-12 7:05 UTC (permalink / raw)
To: David S. Miller, pravin shelar, Girish Moodalbail
Cc: netdev, linux-kernel, Haishuang Yan
Similar to vxlan/ipip tunnel, if key->tos is zero in collect metadata
mode, tos should also fallback to ip{4,6}_dst_hoplimit.
Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
Changes since v2:
* Make the commit message more clearer.
---
drivers/net/geneve.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index f640407..d52a65f 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -834,11 +834,10 @@ static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
if (geneve->collect_md) {
tos = ip_tunnel_ecn_encap(key->tos, ip_hdr(skb), skb);
- ttl = key->ttl;
} else {
tos = ip_tunnel_ecn_encap(fl4.flowi4_tos, ip_hdr(skb), skb);
- ttl = key->ttl ? : ip4_dst_hoplimit(&rt->dst);
}
+ ttl = key->ttl ? : ip4_dst_hoplimit(&rt->dst);
df = key->tun_flags & TUNNEL_DONT_FRAGMENT ? htons(IP_DF) : 0;
err = geneve_build_skb(&rt->dst, skb, info, xnet, sizeof(struct iphdr));
@@ -873,12 +872,11 @@ static int geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
if (geneve->collect_md) {
prio = ip_tunnel_ecn_encap(key->tos, ip_hdr(skb), skb);
- ttl = key->ttl;
} else {
prio = ip_tunnel_ecn_encap(ip6_tclass(fl6.flowlabel),
ip_hdr(skb), skb);
- ttl = key->ttl ? : ip6_dst_hoplimit(dst);
}
+ ttl = key->ttl ? : ip6_dst_hoplimit(dst);
err = geneve_build_skb(dst, skb, info, xnet, sizeof(struct ipv6hdr));
if (unlikely(err))
return err;
--
1.8.3.1
^ permalink raw reply related
* [patch net] mlxsw: spectrum: Prevent mirred-related crash on removal
From: Jiri Pirko @ 2017-09-12 6:50 UTC (permalink / raw)
To: netdev; +Cc: davem, yuvalm, idosch, flokli, mlxsw
From: Yuval Mintz <yuvalm@mellanox.com>
When removing the offloading of mirred actions under
matchall classifiers, mlxsw would find the destination port
associated with the offloaded action and utilize it for undoing
the configuration.
Depending on the order by which ports are removed, it's possible that
the destination port would get removed before the source port.
In such a scenario, when actions would be flushed for the source port
mlxsw would perform an illegal dereference as the destination port is
no longer listed.
Since the only item necessary for undoing the configuration on the
destination side is the port-id and that in turn is already maintained
by mlxsw on the source-port, simply stop trying to access the
destination port and use the port-id directly instead.
Fixes: 763b4b70af ("mlxsw: spectrum: Add support in matchall mirror TC offloading")
Signed-off-by: Yuval Mintz <yuvalm@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index e080459..696b99e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -575,15 +575,14 @@ static void mlxsw_sp_span_entry_destroy(struct mlxsw_sp *mlxsw_sp,
}
static struct mlxsw_sp_span_entry *
-mlxsw_sp_span_entry_find(struct mlxsw_sp_port *port)
+mlxsw_sp_span_entry_find(struct mlxsw_sp *mlxsw_sp, u8 local_port)
{
- struct mlxsw_sp *mlxsw_sp = port->mlxsw_sp;
int i;
for (i = 0; i < mlxsw_sp->span.entries_count; i++) {
struct mlxsw_sp_span_entry *curr = &mlxsw_sp->span.entries[i];
- if (curr->used && curr->local_port == port->local_port)
+ if (curr->used && curr->local_port == local_port)
return curr;
}
return NULL;
@@ -594,7 +593,8 @@ static struct mlxsw_sp_span_entry
{
struct mlxsw_sp_span_entry *span_entry;
- span_entry = mlxsw_sp_span_entry_find(port);
+ span_entry = mlxsw_sp_span_entry_find(port->mlxsw_sp,
+ port->local_port);
if (span_entry) {
/* Already exists, just take a reference */
span_entry->ref_count++;
@@ -783,12 +783,13 @@ static int mlxsw_sp_span_mirror_add(struct mlxsw_sp_port *from,
}
static void mlxsw_sp_span_mirror_remove(struct mlxsw_sp_port *from,
- struct mlxsw_sp_port *to,
+ u8 destination_port,
enum mlxsw_sp_span_type type)
{
struct mlxsw_sp_span_entry *span_entry;
- span_entry = mlxsw_sp_span_entry_find(to);
+ span_entry = mlxsw_sp_span_entry_find(from->mlxsw_sp,
+ destination_port);
if (!span_entry) {
netdev_err(from->dev, "no span entry found\n");
return;
@@ -1563,14 +1564,12 @@ static void
mlxsw_sp_port_del_cls_matchall_mirror(struct mlxsw_sp_port *mlxsw_sp_port,
struct mlxsw_sp_port_mall_mirror_tc_entry *mirror)
{
- struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
enum mlxsw_sp_span_type span_type;
- struct mlxsw_sp_port *to_port;
- to_port = mlxsw_sp->ports[mirror->to_local_port];
span_type = mirror->ingress ?
MLXSW_SP_SPAN_INGRESS : MLXSW_SP_SPAN_EGRESS;
- mlxsw_sp_span_mirror_remove(mlxsw_sp_port, to_port, span_type);
+ mlxsw_sp_span_mirror_remove(mlxsw_sp_port, mirror->to_local_port,
+ span_type);
}
static int
--
2.9.3
^ permalink raw reply related
* Re: [RFC PATCH] net: Introduce a socket option to enable picking tx queue based on rx queue.
From: Samudrala, Sridhar @ 2017-09-12 6:27 UTC (permalink / raw)
To: Eric Dumazet, Tom Herbert
Cc: Alexander Duyck, Linux Kernel Network Developers
In-Reply-To: <1505188437.15310.137.camel@edumazet-glaptop3.roam.corp.google.com>
On 9/11/2017 8:53 PM, Eric Dumazet wrote:
> On Mon, 2017-09-11 at 20:12 -0700, Tom Herbert wrote:
>
>> Two ints in sock_common for this purpose is quite expensive and the
>> use case for this is limited-- even if a RX->TX queue mapping were
>> introduced to eliminate the queue pair assumption this still won't
>> help if the receive and transmit interfaces are different for the
>> connection. I think we really need to see some very compelling results
>> to be able to justify this.
Will try to collect and post some perf data with symmetric queue
configuration.
> Yes, this is unreasonable cost.
>
> XPS should really cover the case already.
>
Eric,
Can you clarify how XPS covers the RX-> TX queue mapping case?
Is it possible to configure XPS to select TX queue based on the RX queue
of a flow?
IIUC, it is based on the CPU of the thread doing the transmit OR based
on skb->priority to TC mapping?
It may be possible to get this effect if the the threads are pinned to a
core, but if the app threads are
freely moving, i am not sure how XPS can be configured to select the TX
queue based on the RX queue of a flow.
Thanks
Sridhar
^ permalink raw reply
* Re: [PATCH] tcp: TCP_USER_TIMEOUT can not work in tcp_probe_timer()
From: liujian @ 2017-09-12 6:08 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, kuznet, yoshfuji, edumazet, ycheng, hkchu, netdev,
weiyongjun1, wangkefeng 00227729
In-Reply-To: <1505143348.15310.118.camel@edumazet-glaptop3.roam.corp.google.com>
Hi,
In the scenario, tcp server side IP changed, and at that memont,
userspace application still send data continuously;
tcp_send_head(sk)'s timestamp always be refreshed.
Here is the packetdrill script:
0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0
+0 < S 0:0(0) win 0 <mss 1460,sackOK,nop,nop,nop,wscale 7>
+0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 7>
+.1 < . 1:1(0) ack 1 win 65530
+0 accept(3, ..., ...) = 4
+0 setsockopt(4, SOL_TCP, TCP_USER_TIMEOUT, [3000], 4) = 0
+0 write(4, ..., 24) = 24
+0 > P. 1:25(24) ack 1 win 229
+.1 < . 1:1(0) ack 25 win 65530
//change the ipaddress
+1 `ifconfig tun0 192.168.0.10/16`
+1 write(4, ..., 24) = 24
+1 write(4, ..., 24) = 24
+1 write(4, ..., 24) = 24
+1 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+0 `ifconfig tun0 192.168.0.1/16`
+0 < . 1:1(0) ack 1 win 1000
+0 write(4, ..., 24) = -1
[root@localhost ~]# time ./gtests/net/packetdrill/packetdrill test.pkt
test.pkt:50: runtime error in write call: Expected result -1 but got 24 with errno 2 (No such file or directory)
real 1m11.364s
user 0m0.028s
sys 0m0.106s
[root@localhost ~]# netstat -toen
Active Internet connections (w/o servers)
Proto Recv-Q Send-Q Local Address Foreign Address State User Inode Timer
tcp 0 504 192.168.0.1:8080 192.0.2.1:33993 ESTABLISHED 0 45453 probe (22.38/0/7)
since the script didn't wait for enough time, here only got 7 probes.
在 2017/9/11 23:22, Eric Dumazet 写道:
> On Mon, 2017-09-11 at 08:13 -0700, Eric Dumazet wrote:
>
>> You can see we got only 3 probes, not 4.
>
> Here is complete packetdrill test showing that code behaves as expected.
>
> 0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
> +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
> +0 bind(3, ..., ...) = 0
> +0 listen(3, 1) = 0
>
> +0 < S 0:0(0) win 0 <mss 1460,sackOK,nop,nop,nop,wscale 7>
> +0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 8>
>
> // Client advertises a zero receive window, so we can't send.
> +.1 < . 1:1(0) ack 1 win 0
> +0 accept(3, ..., ...) = 4
>
> +0 setsockopt(4, SOL_TCP, TCP_USER_TIMEOUT, [3000], 4) = 0
> +0 write(4, ..., 2920) = 2920
>
> // Window probes are scheduled just like RTOs.
> +.3~+.31 > . 0:0(0) ack 1
> +.6~+.62 > . 0:0(0) ack 1
> +1.2~+1.24 > . 0:0(0) ack 1
>
> // Peer opens its window too late !
> +3 < . 1:1(0) ack 1 win 1000
> +0 > R 1:1(0)
>
>
>
> .
>
^ permalink raw reply
* Re: [PATCH v2] openvswitch: Fix an error handling path in 'ovs_nla_init_match_and_action()'
From: Pravin Shelar @ 2017-09-12 4:57 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Pravin Shelar, David S. Miller, Tonghao Zhang,
Linux Kernel Network Developers, ovs dev, linux-kernel,
kernel-janitors
In-Reply-To: <20170911195620.17931-1-christophe.jaillet@wanadoo.fr>
On Mon, Sep 11, 2017 at 12:56 PM, Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
> All other error handling paths in this function go through the 'error'
> label. This one should do the same.
>
> Fixes: 9cc9a5cb176c ("datapath: Avoid using stack larger than 1024.")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> I think that the comment above the function could be improved. It looks
> like the commit log which has introduced this function.
>
> I'm also not sure that commit 9cc9a5cb176c is of any help. It is
> supposed to remove a warning, and I guess it does. But 'ovs_nla_init_match_and_action()'
> is called unconditionnaly from 'ovs_flow_cmd_set()'. So even if the stack
> used by each function is reduced, the overall stack should be the same, if
> not larger.
>
It depends on which function stack depth are are looking at. for some
function it remains same. For nested function it goes down.
> So this commit sounds like adding a bug where the code was fine and states
> to fix an issue but, at the best, only hides it.
>
> Instead of fixing the code with the proposed patch, reverting the initial
> commit could also be considered.
>
> V2: update Subject line
> ---
> net/openvswitch/datapath.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 76cf273a56c7..c3aec6227c91 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -1112,7 +1112,8 @@ static int ovs_nla_init_match_and_action(struct net *net,
> if (!a[OVS_FLOW_ATTR_KEY]) {
> OVS_NLERR(log,
> "Flow key attribute not present in set flow.");
> - return -EINVAL;
> + error = -EINVAL;
> + goto error;
> }
>
Patch looks good to me.
Acked-by: Pravin B Shelar <pshelar@ovn.org>
^ permalink raw reply
* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Richard Cochran @ 2017-09-12 4:56 UTC (permalink / raw)
To: Vinicius Costa Gomes
Cc: Henrik Austad, netdev, jhs, xiyou.wangcong, jiri, intel-wired-lan,
andre.guedes, ivan.briano, jesus.sanchez-palencia, boon.leong.ong
In-Reply-To: <87k21am10j.fsf@intel.com>
On Thu, Sep 07, 2017 at 06:29:00PM -0700, Vinicius Costa Gomes wrote:
> >> * Time-aware shaper (802.1Qbv):
> >>
> >> The idea we are currently exploring is to add a "time-aware", priority based
> >> qdisc, that also exposes the Tx queues available and provides a mechanism for
> >> mapping priority <-> traffic class <-> Tx queues in a similar fashion as
> >> mqprio. We are calling this qdisc 'taprio', and its 'tc' cmd line would be:
> >
> > As far as I know, this is not supported by i210, and if time-aware shaping
> > is enabled in the network - you'll be queued on a bridge until the window
> > opens as time-aware shaping is enforced on the tx-port and not on rx. Is
> > this required in this driver?
>
> Yeah, i210 doesn't support the time-aware shaper. I think the second
> part of your question doesn't really apply, then.
Actually, you can implement 802.1Qbv (as an end station) quite easily
using the i210. I'll show how by posting a series after net-next
opens up again.
Thanks,
Richard
^ permalink raw reply
* Re: [RFC PATCH] net: Introduce a socket option to enable picking tx queue based on rx queue.
From: Eric Dumazet @ 2017-09-12 3:53 UTC (permalink / raw)
To: Tom Herbert
Cc: Sridhar Samudrala, Alexander Duyck,
Linux Kernel Network Developers
In-Reply-To: <CALx6S35sM1CDrFuNE+L59Op_wKTRpATLAdRJafihr0mB9+vQ8g@mail.gmail.com>
On Mon, 2017-09-11 at 20:12 -0700, Tom Herbert wrote:
> Two ints in sock_common for this purpose is quite expensive and the
> use case for this is limited-- even if a RX->TX queue mapping were
> introduced to eliminate the queue pair assumption this still won't
> help if the receive and transmit interfaces are different for the
> connection. I think we really need to see some very compelling results
> to be able to justify this.
>
Yes, this is unreasonable cost.
XPS should really cover the case already.
^ permalink raw reply
* Subject: [PATCH] vxlan: only reduce known arp boardcast request to support, virtual IP
From: oc @ 2017-09-12 3:26 UTC (permalink / raw)
To: davem, jbenc, Linux Kernel Network Developers, oc
The purpose of vxlan arp reduce feature is to reply the boardcast
arp request in vtep instead of sending it out to save traffic.
The current implemention drops arp packet, if the ip cannot be
found in neigh table. In the case of virtual IP address, user
defines IP address without management from SDN controller. The IP
address does not exist in neigh table, so the arp boardcast request
from a client can not be sent to the server who owns the virtual IP
address.
This patch allow the arp request to be sent out if:
1. not arp boardcast request
2. cannot be found in neigh table
3. arp record status is not NUD_CONNECTED
The user defined of virtual IP address works while arp reduce still
suppress the arp boardcast for IP address managed by SDN controller
with this patch.
Signed-off-by: Chen Haiquan <oc@yunify.com>
---
drivers/net/vxlan.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index d7c49cf1d5e9..913b838b260b 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1473,7 +1473,7 @@ static int arp_reduce(struct net_device *dev,
struct sk_buff *skb, __be32 vni)
parp->ar_op != htons(ARPOP_REQUEST) ||
parp->ar_hln != dev->addr_len ||
parp->ar_pln != 4)
- goto out;
+ return 1;
arpptr = (u8 *)parp + sizeof(struct arphdr);
sha = arpptr;
arpptr += dev->addr_len; /* sha */
@@ -1494,7 +1494,7 @@ static int arp_reduce(struct net_device *dev,
struct sk_buff *skb, __be32 vni)
if (!(n->nud_state & NUD_CONNECTED)) {
neigh_release(n);
- goto out;
+ return 1;
}
f = vxlan_find_mac(vxlan, n->ha, vni);
@@ -1526,6 +1526,10 @@ static int arp_reduce(struct net_device *dev,
struct sk_buff *skb, __be32 vni)
};
vxlan_ip_miss(dev, &ipa);
+ return 1;
+ } else {
+ /* broadcast unknown arp */
+ return 1;
}
out:
consume_skb(skb);
@@ -1642,7 +1646,7 @@ static int neigh_reduce(struct net_device *dev,
struct sk_buff *skb, __be32 vni)
msg = (struct nd_msg *)(iphdr + 1);
if (msg->icmph.icmp6_code != 0 ||
msg->icmph.icmp6_type != NDISC_NEIGHBOUR_SOLICITATION)
- goto out;
+ return 1;
if (ipv6_addr_loopback(daddr) ||
ipv6_addr_is_multicast(&msg->target))
@@ -1656,7 +1660,7 @@ static int neigh_reduce(struct net_device *dev,
struct sk_buff *skb, __be32 vni)
if (!(n->nud_state & NUD_CONNECTED)) {
neigh_release(n);
- goto out;
+ return 1;
}
f = vxlan_find_mac(vxlan, n->ha, vni);
@@ -1684,6 +1688,10 @@ static int neigh_reduce(struct net_device *dev,
struct sk_buff *skb, __be32 vni)
};
vxlan_ip_miss(dev, &ipa);
+ return 1;
+ } else {
+ /* broadcast unknown neigh */
+ return 1;
}
out:
@@ -2266,8 +2274,10 @@ static netdev_tx_t vxlan_xmit(struct sk_buff
*skb, struct net_device *dev)
if (vxlan->cfg.flags & VXLAN_F_PROXY) {
eth = eth_hdr(skb);
- if (ntohs(eth->h_proto) == ETH_P_ARP)
- return arp_reduce(dev, skb, vni);
+ if (ntohs(eth->h_proto) == ETH_P_ARP){
+ if (NETDEV_TX_OK == arp_reduce(dev, skb, vni))
+ return NETDEV_TX_OK;
+ }
#if IS_ENABLED(CONFIG_IPV6)
else if (ntohs(eth->h_proto) == ETH_P_IPV6) {
struct ipv6hdr *hdr, _hdr;
@@ -2275,7 +2285,9 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb,
struct net_device *dev)
skb_network_offset(skb),
sizeof(_hdr), &_hdr)) &&
hdr->nexthdr == IPPROTO_ICMPV6)
- return neigh_reduce(dev, skb, vni);
+ if (NETDEV_TX_OK == neigh_reduce(dev,
+ skb, vni))
+ return NETDEV_TX_OK;
}
#endif
}
--
2.7.4
^ permalink raw reply related
* Re: [RFC PATCH] net: Introduce a socket option to enable picking tx queue based on rx queue.
From: Tom Herbert @ 2017-09-12 3:12 UTC (permalink / raw)
To: Sridhar Samudrala; +Cc: Alexander Duyck, Linux Kernel Network Developers
In-Reply-To: <1504222032-6337-1-git-send-email-sridhar.samudrala@intel.com>
On Thu, Aug 31, 2017 at 4:27 PM, Sridhar Samudrala
<sridhar.samudrala@intel.com> wrote:
> This patch introduces a new socket option SO_SYMMETRIC_QUEUES that can be used
> to enable symmetric tx and rx queues on a socket.
>
> This option is specifically useful for epoll based multi threaded workloads
> where each thread handles packets received on a single RX queue . In this model,
> we have noticed that it helps to send the packets on the same TX queue
> corresponding to the queue-pair associated with the RX queue specifically when
> busy poll is enabled with epoll().
>
> Two new fields are added to struct sock_common to cache the last rx ifindex and
> the rx queue in the receive path of an SKB. __netdev_pick_tx() returns the cached
> rx queue when this option is enabled and the TX is happening on the same device.
>
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> ---
> include/net/request_sock.h | 1 +
> include/net/sock.h | 17 +++++++++++++++++
> include/uapi/asm-generic/socket.h | 2 ++
> net/core/dev.c | 8 +++++++-
> net/core/sock.c | 10 ++++++++++
> net/ipv4/tcp_input.c | 1 +
> net/ipv4/tcp_ipv4.c | 1 +
> net/ipv4/tcp_minisocks.c | 1 +
> 8 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/request_sock.h b/include/net/request_sock.h
> index 23e2205..c3bc12e 100644
> --- a/include/net/request_sock.h
> +++ b/include/net/request_sock.h
> @@ -100,6 +100,7 @@ static inline struct sock *req_to_sk(struct request_sock *req)
> req_to_sk(req)->sk_prot = sk_listener->sk_prot;
> sk_node_init(&req_to_sk(req)->sk_node);
> sk_tx_queue_clear(req_to_sk(req));
> + req_to_sk(req)->sk_symmetric_queues = sk_listener->sk_symmetric_queues;
> req->saved_syn = NULL;
> refcount_set(&req->rsk_refcnt, 0);
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 03a3625..3421809 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -138,11 +138,14 @@ void SOCK_DEBUG(const struct sock *sk, const char *msg, ...)
> * @skc_node: main hash linkage for various protocol lookup tables
> * @skc_nulls_node: main hash linkage for TCP/UDP/UDP-Lite protocol
> * @skc_tx_queue_mapping: tx queue number for this connection
> + * @skc_rx_queue_mapping: rx queue number for this connection
> + * @skc_rx_ifindex: rx ifindex for this connection
> * @skc_flags: place holder for sk_flags
> * %SO_LINGER (l_onoff), %SO_BROADCAST, %SO_KEEPALIVE,
> * %SO_OOBINLINE settings, %SO_TIMESTAMPING settings
> * @skc_incoming_cpu: record/match cpu processing incoming packets
> * @skc_refcnt: reference count
> + * @skc_symmetric_queues: symmetric tx/rx queues
> *
> * This is the minimal network layer representation of sockets, the header
> * for struct sock and struct inet_timewait_sock.
> @@ -177,6 +180,7 @@ struct sock_common {
> unsigned char skc_reuseport:1;
> unsigned char skc_ipv6only:1;
> unsigned char skc_net_refcnt:1;
> + unsigned char skc_symmetric_queues:1;
> int skc_bound_dev_if;
> union {
> struct hlist_node skc_bind_node;
> @@ -214,6 +218,8 @@ struct sock_common {
> struct hlist_nulls_node skc_nulls_node;
> };
> int skc_tx_queue_mapping;
> + int skc_rx_queue_mapping;
> + int skc_rx_ifindex;
Two ints in sock_common for this purpose is quite expensive and the
use case for this is limited-- even if a RX->TX queue mapping were
introduced to eliminate the queue pair assumption this still won't
help if the receive and transmit interfaces are different for the
connection. I think we really need to see some very compelling results
to be able to justify this.
Thanks,
Tom
^ permalink raw reply
* RE: Extended IDR API
From: Chris Mi @ 2017-09-12 2:27 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Jiri Pirko, David S. Miller, Tejun Heo,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
Rehas Sachdeva
In-Reply-To: <20170911211408.GA26747@bombadil.infradead.org>
This improvement is good. But I have a concern that
the parameters of idr_alloc and idr_alloc_ul are different.
I mean in idr_alloc, we have start and end.
In our new API, we keep them. So our design goal is to
make them consistent. Your new API has its advantage surely.
If you want to change it, I don't object personally.
> -----Original Message-----
> From: Matthew Wilcox [mailto:willy@infradead.org]
> Sent: Tuesday, September 12, 2017 5:14 AM
> To: Chris Mi <chrism@mellanox.com>
> Cc: Jiri Pirko <jiri@mellanox.com>; David S. Miller <davem@davemloft.net>;
> Tejun Heo <tj@kernel.org>; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; Rehas Sachdeva <aquannie@gmail.com>
> Subject: Extended IDR API
>
>
> I really don't like your new API. I wish you'd discussed it before merging it.
> Here's my redesign. Does anybody have a suggestion for improvement?
>
> We have a lovely new test-suite for the IDR (in tools/testing/radix-tree) ...
> when adding a new API, it's polite to update the test-suite too.
> Do you have any plans to add test cases?
OK, we will add it once these APIs are stabilized.
Thanks,
Chris
>
> (Compile tested only; I'm at a conference. Also, I didn't check the kerneldoc
> because I don't have Sphinx installed on my laptop.)
>
> From ff45b2a6806cd0e4177c5a10f26c97999164c10c Mon Sep 17 00:00:00 2001
> From: Matthew Wilcox <mawilcox@microsoft.com>
> Date: Mon, 11 Sep 2017 16:16:29 -0400
> Subject: [PATCH] idr: Rewrite extended IDR API
>
> - Rename the API to be 'ul' for unsigned long instead of 'ext'. This
> fits better with other usage in the Linux kernel.
> - idr_alloc() moves back to being a function instead of inline
> - idr_alloc_ul() takes 'nextid' as an in-out parameter like idr_get_next(),
> instead of having 'index' as an out-only parameter.
> - idr_alloc_ul() needs a __must_check to ensure that users don't look at
> the result without checking whether the function succeeded.
> - idr_alloc_ul() takes 'max' rather than 'end', or it is impossible to
> allocate the ULONG_MAX id.
> - idr_replace() can simply take an unsigned long parameter instead of
> an int.
> - idr_remove() and idr_find() are the same as idr_replace().
> - We don't need separate idr_get_free() and idr_get_free_ext().
> - Add kerneldoc for idr_alloc_ul().
>
> Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
> ---
> include/linux/idr.h | 75 +++++----------------------------------
> include/linux/radix-tree.h | 17 +--------
> lib/idr.c | 88 +++++++++++++++++++++++++++++++++-------------
> lib/radix-tree.c | 2 +-
> net/sched/act_api.c | 22 ++++++------
> net/sched/cls_flower.c | 16 +++++----
> 6 files changed, 95 insertions(+), 125 deletions(-)
>
> diff --git a/include/linux/idr.h b/include/linux/idr.h index
> 7c3a365f7e12..90faf8279559 100644
> --- a/include/linux/idr.h
> +++ b/include/linux/idr.h
> @@ -81,74 +81,22 @@ static inline void idr_set_cursor(struct idr *idr,
> unsigned int val)
>
> void idr_preload(gfp_t gfp_mask);
>
> -int idr_alloc_cmn(struct idr *idr, void *ptr, unsigned long *index,
> - unsigned long start, unsigned long end, gfp_t gfp,
> - bool ext);
> -
> -/**
> - * idr_alloc - allocate an id
> - * @idr: idr handle
> - * @ptr: pointer to be associated with the new id
> - * @start: the minimum id (inclusive)
> - * @end: the maximum id (exclusive)
> - * @gfp: memory allocation flags
> - *
> - * Allocates an unused ID in the range [start, end). Returns -ENOSPC
> - * if there are no unused IDs in that range.
> - *
> - * Note that @end is treated as max when <= 0. This is to always allow
> - * using @start + N as @end as long as N is inside integer range.
> - *
> - * Simultaneous modifications to the @idr are not allowed and should be
> - * prevented by the user, usually with a lock. idr_alloc() may be called
> - * concurrently with read-only accesses to the @idr, such as idr_find() and
> - * idr_for_each_entry().
> - */
> -static inline int idr_alloc(struct idr *idr, void *ptr,
> - int start, int end, gfp_t gfp)
> -{
> - unsigned long id;
> - int ret;
> -
> - if (WARN_ON_ONCE(start < 0))
> - return -EINVAL;
> -
> - ret = idr_alloc_cmn(idr, ptr, &id, start, end, gfp, false);
> -
> - if (ret)
> - return ret;
> -
> - return id;
> -}
> -
> -static inline int idr_alloc_ext(struct idr *idr, void *ptr,
> - unsigned long *index,
> - unsigned long start,
> - unsigned long end,
> - gfp_t gfp)
> -{
> - return idr_alloc_cmn(idr, ptr, index, start, end, gfp, true);
> -}
> -
> +int idr_alloc(struct idr *, void *, int start, int end, gfp_t); int
> +__must_check idr_alloc_ul(struct idr *, void *, unsigned long *nextid,
> + unsigned long max, gfp_t);
> int idr_alloc_cyclic(struct idr *, void *entry, int start, int end, gfp_t); int
> idr_for_each(const struct idr *,
> int (*fn)(int id, void *p, void *data), void *data); void
> *idr_get_next(struct idr *, int *nextid); -void *idr_get_next_ext(struct idr
> *idr, unsigned long *nextid); -void *idr_replace(struct idr *, void *, int id); -
> void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long id);
> +void *idr_get_next_ul(struct idr *, unsigned long *nextid); void
> +*idr_replace(struct idr *, void *, unsigned long id);
> void idr_destroy(struct idr *);
>
> -static inline void *idr_remove_ext(struct idr *idr, unsigned long id)
> +static inline void *idr_remove(struct idr *idr, unsigned long id)
> {
> return radix_tree_delete_item(&idr->idr_rt, id, NULL); }
>
> -static inline void *idr_remove(struct idr *idr, int id) -{
> - return idr_remove_ext(idr, id);
> -}
> -
> static inline void idr_init(struct idr *idr) {
> INIT_RADIX_TREE(&idr->idr_rt, IDR_RT_MARKER); @@ -184,16
> +132,11 @@ static inline void idr_preload_end(void)
> * This function can be called under rcu_read_lock(), given that the leaf
> * pointers lifetimes are correctly managed.
> */
> -static inline void *idr_find_ext(const struct idr *idr, unsigned long id)
> +static inline void *idr_find(const struct idr *idr, unsigned long id)
> {
> return radix_tree_lookup(&idr->idr_rt, id); }
>
> -static inline void *idr_find(const struct idr *idr, int id) -{
> - return idr_find_ext(idr, id);
> -}
> -
> /**
> * idr_for_each_entry - iterate over an idr's elements of a given type
> * @idr: idr handle
> @@ -206,8 +149,8 @@ static inline void *idr_find(const struct idr *idr, int id)
> */
> #define idr_for_each_entry(idr, entry, id) \
> for (id = 0; ((entry) = idr_get_next(idr, &(id))) != NULL; ++id)
> -#define idr_for_each_entry_ext(idr, entry, id) \
> - for (id = 0; ((entry) = idr_get_next_ext(idr, &(id))) != NULL; ++id)
> +#define idr_for_each_entry_ul(idr, entry, id) \
> + for (id = 0; ((entry) = idr_get_next_ul(idr, &(id))) != NULL; ++id)
>
> /**
> * idr_for_each_entry_continue - continue iteration over an idr's elements
> of a given type diff --git a/include/linux/radix-tree.h b/include/linux/radix-
> tree.h index 567ebb5eaab0..8275fc2ed0f4 100644
> --- a/include/linux/radix-tree.h
> +++ b/include/linux/radix-tree.h
> @@ -358,24 +358,9 @@ int radix_tree_split(struct radix_tree_root *,
> unsigned long index, int radix_tree_join(struct radix_tree_root *, unsigned
> long index,
> unsigned new_order, void *);
>
> -void __rcu **idr_get_free_cmn(struct radix_tree_root *root,
> +void __rcu **idr_get_free(struct radix_tree_root *root,
> struct radix_tree_iter *iter, gfp_t gfp,
> unsigned long max);
> -static inline void __rcu **idr_get_free(struct radix_tree_root *root,
> - struct radix_tree_iter *iter,
> - gfp_t gfp,
> - int end)
> -{
> - return idr_get_free_cmn(root, iter, gfp, end > 0 ? end - 1 : INT_MAX);
> -}
> -
> -static inline void __rcu **idr_get_free_ext(struct radix_tree_root *root,
> - struct radix_tree_iter *iter,
> - gfp_t gfp,
> - unsigned long end)
> -{
> - return idr_get_free_cmn(root, iter, gfp, end - 1);
> -}
>
> enum {
> RADIX_TREE_ITER_TAG_MASK = 0x0f, /* tag index in lower nybble
> */
> diff --git a/lib/idr.c b/lib/idr.c
> index 082778cf883e..230879a65d99 100644
> --- a/lib/idr.c
> +++ b/lib/idr.c
> @@ -7,9 +7,26 @@
> DEFINE_PER_CPU(struct ida_bitmap *, ida_bitmap); static
> DEFINE_SPINLOCK(simple_ida_lock);
>
> -int idr_alloc_cmn(struct idr *idr, void *ptr, unsigned long *index,
> - unsigned long start, unsigned long end, gfp_t gfp,
> - bool ext)
> +/**
> + * idr_alloc_ul - allocate a large ID
> + * @idr: idr handle
> + * @ptr: pointer to be associated with the new ID
> + * @nextid: Pointer to minimum ID to allocate
> + * @max: the maximum ID (inclusive)
> + * @gfp: memory allocation flags
> + *
> + * Allocates an unused ID in the range [*nextid, end] and stores it in
> + * @nextid. Note that @max differs from the @end parameter to
> idr_alloc().
> + *
> + * Simultaneous modifications to the @idr are not allowed and should be
> + * prevented by the user, usually with a lock. idr_alloc_ul() may be
> +called
> + * concurrently with read-only accesses to the @idr, such as idr_find()
> +and
> + * idr_for_each_entry().
> + *
> + * Return: 0 on success or a negative errno on failure (ENOMEM or
> +ENOSPC) */ int idr_alloc_ul(struct idr *idr, void *ptr, unsigned long
> +*nextid,
> + unsigned long max, gfp_t gfp)
> {
> struct radix_tree_iter iter;
> void __rcu **slot;
> @@ -17,22 +34,54 @@ int idr_alloc_cmn(struct idr *idr, void *ptr, unsigned
> long *index,
> if (WARN_ON_ONCE(radix_tree_is_internal_node(ptr)))
> return -EINVAL;
>
> - radix_tree_iter_init(&iter, start);
> - if (ext)
> - slot = idr_get_free_ext(&idr->idr_rt, &iter, gfp, end);
> - else
> - slot = idr_get_free(&idr->idr_rt, &iter, gfp, end);
> + radix_tree_iter_init(&iter, *nextid);
> + slot = idr_get_free(&idr->idr_rt, &iter, gfp, max);
> if (IS_ERR(slot))
> return PTR_ERR(slot);
>
> radix_tree_iter_replace(&idr->idr_rt, &iter, slot, ptr);
> radix_tree_iter_tag_clear(&idr->idr_rt, &iter, IDR_FREE);
>
> - if (index)
> - *index = iter.index;
> + *nextid = iter.index;
> return 0;
> }
> -EXPORT_SYMBOL_GPL(idr_alloc_cmn);
> +EXPORT_SYMBOL_GPL(idr_alloc_ul);
> +
> +/**
> + * idr_alloc - allocate an id
> + * @idr: idr handle
> + * @ptr: pointer to be associated with the new id
> + * @start: the minimum id (inclusive)
> + * @end: the maximum id (exclusive)
> + * @gfp: memory allocation flags
> + *
> + * Allocates an unused ID in the range [start, end). Returns -ENOSPC
> + * if there are no unused IDs in that range.
> + *
> + * Note that @end is treated as max when <= 0. This is to always allow
> + * using @start + N as @end as long as N is inside integer range.
> + *
> + * Simultaneous modifications to the @idr are not allowed and should be
> + * prevented by the user, usually with a lock. idr_alloc() may be
> +called
> + * concurrently with read-only accesses to the @idr, such as idr_find()
> +and
> + * idr_for_each_entry().
> + */
> +int idr_alloc(struct idr *idr, void *ptr, int start, int end, gfp_t
> +gfp) {
> + unsigned long id = start;
> + int ret;
> +
> + if (WARN_ON_ONCE(start < 0))
> + return -EINVAL;
> +
> + ret = idr_alloc_ul(idr, ptr, &id, end > 0 ? end - 1 : INT_MAX, gfp);
> +
> + if (ret)
> + return ret;
> +
> + return id;
> +}
> +EXPORT_SYMBOL_GPL(idr_alloc);
>
> /**
> * idr_alloc_cyclic - allocate new idr entry in a cyclical fashion @@ -121,7
> +170,7 @@ void *idr_get_next(struct idr *idr, int *nextid) }
> EXPORT_SYMBOL(idr_get_next);
>
> -void *idr_get_next_ext(struct idr *idr, unsigned long *nextid)
> +void *idr_get_next_ul(struct idr *idr, unsigned long *nextid)
> {
> struct radix_tree_iter iter;
> void __rcu **slot;
> @@ -133,7 +182,7 @@ void *idr_get_next_ext(struct idr *idr, unsigned long
> *nextid)
> *nextid = iter.index;
> return rcu_dereference_raw(*slot);
> }
> -EXPORT_SYMBOL(idr_get_next_ext);
> +EXPORT_SYMBOL(idr_get_next_ul);
>
> /**
> * idr_replace - replace pointer for given id @@ -149,16 +198,7 @@
> EXPORT_SYMBOL(idr_get_next_ext);
> * Returns: 0 on success. %-ENOENT indicates that @id was not found.
> * %-EINVAL indicates that @id or @ptr were not valid.
> */
> -void *idr_replace(struct idr *idr, void *ptr, int id) -{
> - if (WARN_ON_ONCE(id < 0))
> - return ERR_PTR(-EINVAL);
> -
> - return idr_replace_ext(idr, ptr, id);
> -}
> -EXPORT_SYMBOL(idr_replace);
> -
> -void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long id)
> +void *idr_replace(struct idr *idr, void *ptr, unsigned long id)
> {
> struct radix_tree_node *node;
> void __rcu **slot = NULL;
> @@ -175,7 +215,7 @@ void *idr_replace_ext(struct idr *idr, void *ptr,
> unsigned long id)
>
> return entry;
> }
> -EXPORT_SYMBOL(idr_replace_ext);
> +EXPORT_SYMBOL(idr_replace);
>
> /**
> * DOC: IDA description
> diff --git a/lib/radix-tree.c b/lib/radix-tree.c index
> 8b1feca1230a..9fcd4e5c5237 100644
> --- a/lib/radix-tree.c
> +++ b/lib/radix-tree.c
> @@ -2139,7 +2139,7 @@ int ida_pre_get(struct ida *ida, gfp_t gfp) }
> EXPORT_SYMBOL(ida_pre_get);
>
> -void __rcu **idr_get_free_cmn(struct radix_tree_root *root,
> +void __rcu **idr_get_free(struct radix_tree_root *root,
> struct radix_tree_iter *iter, gfp_t gfp,
> unsigned long max)
> {
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c index
> a306974e2fb4..131817ab3ad3 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -73,7 +73,7 @@ static void free_tcf(struct rcu_head *head) static void
> tcf_idr_remove(struct tcf_idrinfo *idrinfo, struct tc_action *p) {
> spin_lock_bh(&idrinfo->lock);
> - idr_remove_ext(&idrinfo->action_idr, p->tcfa_index);
> + idr_remove(&idrinfo->action_idr, p->tcfa_index);
> spin_unlock_bh(&idrinfo->lock);
> gen_kill_estimator(&p->tcfa_rate_est);
> /*
> @@ -121,7 +121,7 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo,
> struct sk_buff *skb,
>
> s_i = cb->args[0];
>
> - idr_for_each_entry_ext(idr, p, id) {
> + idr_for_each_entry_ul(idr, p, id) {
> index++;
> if (index < s_i)
> continue;
> @@ -178,7 +178,7 @@ static int tcf_del_walker(struct tcf_idrinfo *idrinfo,
> struct sk_buff *skb,
> if (nla_put_string(skb, TCA_KIND, ops->kind))
> goto nla_put_failure;
>
> - idr_for_each_entry_ext(idr, p, id) {
> + idr_for_each_entry_ul(idr, p, id) {
> ret = __tcf_idr_release(p, false, true);
> if (ret == ACT_P_DELETED) {
> module_put(p->ops->owner);
> @@ -216,10 +216,10 @@ EXPORT_SYMBOL(tcf_generic_walker);
>
> static struct tc_action *tcf_idr_lookup(u32 index, struct tcf_idrinfo *idrinfo)
> {
> - struct tc_action *p = NULL;
> + struct tc_action *p;
>
> spin_lock_bh(&idrinfo->lock);
> - p = idr_find_ext(&idrinfo->action_idr, index);
> + p = idr_find(&idrinfo->action_idr, index);
> spin_unlock_bh(&idrinfo->lock);
>
> return p;
> @@ -296,10 +296,10 @@ int tcf_idr_create(struct tc_action_net *tn, u32
> index, struct nlattr *est,
> spin_lock_init(&p->tcfa_lock);
> /* user doesn't specify an index */
> if (!index) {
> + idr_index = 1;
> idr_preload(GFP_KERNEL);
> spin_lock_bh(&idrinfo->lock);
> - err = idr_alloc_ext(idr, NULL, &idr_index, 1, 0,
> - GFP_ATOMIC);
> + err = idr_alloc_ul(idr, NULL, &idr_index, UINT_MAX,
> GFP_ATOMIC);
> spin_unlock_bh(&idrinfo->lock);
> idr_preload_end();
> if (err) {
> @@ -309,10 +309,10 @@ int tcf_idr_create(struct tc_action_net *tn, u32
> index, struct nlattr *est,
> }
> p->tcfa_index = idr_index;
> } else {
> + idr_index = index;
> idr_preload(GFP_KERNEL);
> spin_lock_bh(&idrinfo->lock);
> - err = idr_alloc_ext(idr, NULL, NULL, index, index + 1,
> - GFP_ATOMIC);
> + err = idr_alloc_ul(idr, NULL, &idr_index, index, GFP_ATOMIC);
> spin_unlock_bh(&idrinfo->lock);
> idr_preload_end();
> if (err)
> @@ -345,7 +345,7 @@ void tcf_idr_insert(struct tc_action_net *tn, struct
> tc_action *a)
> struct tcf_idrinfo *idrinfo = tn->idrinfo;
>
> spin_lock_bh(&idrinfo->lock);
> - idr_replace_ext(&idrinfo->action_idr, a, a->tcfa_index);
> + idr_replace(&idrinfo->action_idr, a, a->tcfa_index);
> spin_unlock_bh(&idrinfo->lock);
> }
> EXPORT_SYMBOL(tcf_idr_insert);
> @@ -358,7 +358,7 @@ void tcf_idrinfo_destroy(const struct tc_action_ops
> *ops,
> int ret;
> unsigned long id = 1;
>
> - idr_for_each_entry_ext(idr, p, id) {
> + idr_for_each_entry_ul(idr, p, id) {
> ret = __tcf_idr_release(p, false, true);
> if (ret == ACT_P_DELETED)
> module_put(ops->owner);
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index
> 1a267e77c6de..ad30968f587d 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -298,7 +298,7 @@ static void __fl_delete(struct tcf_proto *tp, struct
> cls_fl_filter *f) {
> struct cls_fl_head *head = rtnl_dereference(tp->root);
>
> - idr_remove_ext(&head->handle_idr, f->handle);
> + idr_remove(&head->handle_idr, f->handle);
> list_del_rcu(&f->list);
> if (!tc_skip_hw(f->flags))
> fl_hw_destroy_filter(tp, f);
> @@ -341,7 +341,7 @@ static void *fl_get(struct tcf_proto *tp, u32 handle) {
> struct cls_fl_head *head = rtnl_dereference(tp->root);
>
> - return idr_find_ext(&head->handle_idr, handle);
> + return idr_find(&head->handle_idr, handle);
> }
>
> static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = { @@ -901,8
> +901,9 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
> goto errout;
>
> if (!handle) {
> - err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
> - 1, 0x80000000, GFP_KERNEL);
> + idr_index = 1;
> + err = idr_alloc_ul(&head->handle_idr, fnew, &idr_index,
> + INT_MAX, GFP_KERNEL);
> if (err)
> goto errout;
> fnew->handle = idr_index;
> @@ -910,8 +911,9 @@ static int fl_change(struct net *net, struct sk_buff
> *in_skb,
>
> /* user specifies a handle and it doesn't exist */
> if (handle && !fold) {
> - err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
> - handle, handle + 1, GFP_KERNEL);
> + idr_index = handle;
> + err = idr_alloc_ul(&head->handle_idr, fnew, &idr_index,
> + handle, GFP_KERNEL);
> if (err)
> goto errout;
> fnew->handle = idr_index;
> @@ -970,7 +972,7 @@ static int fl_change(struct net *net, struct sk_buff
> *in_skb,
>
> if (fold) {
> fnew->handle = handle;
> - idr_replace_ext(&head->handle_idr, fnew, fnew->handle);
> + idr_replace(&head->handle_idr, fnew, fnew->handle);
> list_replace_rcu(&fold->list, &fnew->list);
> tcf_unbind_filter(tp, &fold->res);
> call_rcu(&fold->rcu, fl_destroy_filter);
> --
> 2.14.1
>
^ permalink raw reply
* RE: [PATCH] mwifiex: remove unnecessary call to memset
From: Xinming Hu @ 2017-09-12 1:36 UTC (permalink / raw)
To: Himanshu Jha, amitkarwar@gmail.com
Cc: Nishant Sarmukadam, Ganapathi Bhat, kvalo@codeaurora.org,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org
Hi Himanshu,
> -----Original Message-----
> From: Himanshu Jha [mailto:himanshujha199640@gmail.com]
> Sent: 2017年9月11日 20:46
> To: amitkarwar@gmail.com
> Cc: Nishant Sarmukadam <nishants@marvell.com>; Ganapathi Bhat
> <gbhat@marvell.com>; Xinming Hu <huxm@marvell.com>;
> kvalo@codeaurora.org; linux-wireless@vger.kernel.org;
> netdev@vger.kernel.org; Himanshu Jha <himanshujha199640@gmail.com>
> Subject: [EXT] [PATCH] mwifiex: remove unnecessary call to memset
>
> External Email
>
> ----------------------------------------------------------------------
> call to memset to assign 0 value immediately after allocating memory with
> kzalloc is unnecesaary as kzalloc allocates the memory filled with 0 value.
>
Looks find to me, Thanks.
Regards,
Simon
> Semantic patch used to resolve this issue:
>
> @@
> expression e,e2; constant c;
> statement S;
> @@
>
> e = kzalloc(e2, c);
> if(e == NULL) S
> - memset(e, 0, e2);
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> ---
> drivers/net/wireless/marvell/mwifiex/scan.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c
> b/drivers/net/wireless/marvell/mwifiex/scan.c
> index c9d41ed..8838b88 100644
> --- a/drivers/net/wireless/marvell/mwifiex/scan.c
> +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
> @@ -1936,8 +1936,6 @@ mwifiex_active_scan_req_for_passive_chan(struct
> mwifiex_private *priv)
> if (!user_scan_cfg)
> return -ENOMEM;
>
> - memset(user_scan_cfg, 0, sizeof(*user_scan_cfg));
> -
> for (id = 0; id < MWIFIEX_USER_SCAN_CHAN_MAX; id++) {
> if (!priv->hidden_chan[id].chan_number)
> break;
> --
> 2.7.4
^ 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