| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1339335061.6001.466.camel@edumazet-glaptop>
Date: Sun, 10 Jun 2012 15:31:01 +0200
From: Eric Dumazet <eric.dumazet@...il.com>
To: Hong zhi guo <honkiko@...il.com>
Cc: davem@...emloft.net, Denys Fedoryshchenko <denys@...p.net.lb>,
Benjamin LaHaise <bcrl@...ck.org>,
Francois Romieu <romieu@...zoreil.com>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: Deadlock, L2TP over IP are not working, 3.4.1
On Sun, 2012-06-10 at 20:14 +0800, Hong zhi guo wrote:
> Is there any conclusion about the problem? Will bh_lock_sock_nested
> fix the lockdep violation?
lockdep violation could indeed be fixed like that, but LLTX seems more
appropriate.
But there is still a bug because skb->len is used after
l2tp_xmit_skb(session, skb, session->hdr_len);
There is also a bug in l2tp_core.c, because it assumes RX path is not
reentrant. That should be fixed eventually.
I believe we could avoid percpu stuf and new locking, adding the
following unions in net_device_stats. It seems enough to have machine
long words stats, no need to force 64bit stats on 32bit arches.
include/linux/netdevice.h | 40 ++++++++++++++++++++++++++++--------
net/l2tp/l2tp_eth.c | 29 +++++++++++++-------------
2 files changed, 47 insertions(+), 22 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d94cb14..1dee75a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -171,14 +171,38 @@ static inline bool dev_xmit_complete(int rc)
*/
struct net_device_stats {
- unsigned long rx_packets;
- unsigned long tx_packets;
- unsigned long rx_bytes;
- unsigned long tx_bytes;
- unsigned long rx_errors;
- unsigned long tx_errors;
- unsigned long rx_dropped;
- unsigned long tx_dropped;
+ union {
+ unsigned long rx_packets;
+ atomic_long_t rx_packets_atomic;
+ };
+ union {
+ unsigned long tx_packets;
+ atomic_long_t tx_packets_atomic;
+ };
+ union {
+ unsigned long rx_bytes;
+ atomic_long_t rx_bytes_atomic;
+ };
+ union {
+ unsigned long tx_bytes;
+ atomic_long_t tx_bytes_atomic;
+ };
+ union {
+ unsigned long rx_errors;
+ atomic_long_t rx_errors_atomic;
+ };
+ union {
+ unsigned long tx_errors;
+ atomic_long_t tx_errors_atomic;
+ };
+ union {
+ unsigned long rx_dropped;
+ atomic_long_t rx_dropped_atomic;
+ };
+ union {
+ unsigned long tx_dropped;
+ atomic_long_t tx_dropped_atomic;
+ };
unsigned long multicast;
unsigned long collisions;
unsigned long rx_length_errors;
diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
index 185f12f..9e80ec4 100644
--- a/net/l2tp/l2tp_eth.c
+++ b/net/l2tp/l2tp_eth.c
@@ -83,20 +83,20 @@ static void l2tp_eth_dev_uninit(struct net_device *dev)
dev_put(dev);
}
-static int l2tp_eth_dev_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t l2tp_eth_dev_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct l2tp_eth *priv = netdev_priv(dev);
struct l2tp_session *session = priv->session;
- l2tp_xmit_skb(session, skb, session->hdr_len);
+ atomic_long_add(skb->len, &dev->stats.tx_bytes_atomic);
+ atomic_long_inc(&dev->stats.tx_packets_atomic);
- dev->stats.tx_bytes += skb->len;
- dev->stats.tx_packets++;
+ l2tp_xmit_skb(session, skb, session->hdr_len);
- return 0;
+ return NETDEV_TX_OK;
}
-static struct net_device_ops l2tp_eth_netdev_ops = {
+static const struct net_device_ops l2tp_eth_netdev_ops = {
.ndo_init = l2tp_eth_dev_init,
.ndo_uninit = l2tp_eth_dev_uninit,
.ndo_start_xmit = l2tp_eth_dev_xmit,
@@ -106,8 +106,9 @@ static void l2tp_eth_dev_setup(struct net_device *dev)
{
ether_setup(dev);
dev->priv_flags &= ~IFF_TX_SKB_SHARING;
- dev->netdev_ops = &l2tp_eth_netdev_ops;
- dev->destructor = free_netdev;
+ dev->features |= NETIF_F_LLTX;
+ dev->netdev_ops = &l2tp_eth_netdev_ops;
+ dev->destructor = free_netdev;
}
static void l2tp_eth_dev_recv(struct l2tp_session *session, struct sk_buff *skb, int data_len)
@@ -139,15 +140,15 @@ static void l2tp_eth_dev_recv(struct l2tp_session *session, struct sk_buff *skb,
nf_reset(skb);
if (dev_forward_skb(dev, skb) == NET_RX_SUCCESS) {
- dev->stats.rx_packets++;
- dev->stats.rx_bytes += data_len;
- } else
- dev->stats.rx_errors++;
-
+ atomic_long_inc(&dev->stats.rx_packets_atomic);
+ atomic_long_add(data_len, &dev->stats.rx_bytes_atomic);
+ } else {
+ atomic_long_inc(&dev->stats.rx_errors_atomic);
+ }
return;
error:
- dev->stats.rx_errors++;
+ atomic_long_inc(&dev->stats.rx_errors_atomic);
kfree_skb(skb);
}
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists