* [PATCH 12/15] batman-adv: use the const qualifier in hash functions
From: Antonio Quartulli @ 2013-01-13 23:41 UTC (permalink / raw)
To: davem
Cc: netdev, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner,
Antonio Quartulli
In-Reply-To: <1358120480-25960-1-git-send-email-ordex@autistici.org>
From: Antonio Quartulli <antonio@open-mesh.com>
The data argument in each hash function should carry the
"const" qualifier as it is never modified.
Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
net/batman-adv/bridge_loop_avoidance.c | 2 +-
net/batman-adv/hash.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index 724adf0..5e834c1 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -57,7 +57,7 @@ static inline uint32_t batadv_choose_claim(const void *data, uint32_t size)
static inline uint32_t batadv_choose_backbone_gw(const void *data,
uint32_t size)
{
- struct batadv_claim *claim = (struct batadv_claim *)data;
+ const struct batadv_claim *claim = (struct batadv_claim *)data;
uint32_t hash = 0;
hash = batadv_hash_bytes(hash, &claim->addr, sizeof(claim->addr));
diff --git a/net/batman-adv/hash.h b/net/batman-adv/hash.h
index e053339..ea02148 100644
--- a/net/batman-adv/hash.h
+++ b/net/batman-adv/hash.h
@@ -89,7 +89,7 @@ static inline void batadv_hash_delete(struct batadv_hashtable *hash,
*
* Returns the new hash value.
*/
-static inline uint32_t batadv_hash_bytes(uint32_t hash, void *data,
+static inline uint32_t batadv_hash_bytes(uint32_t hash, const void *data,
uint32_t size)
{
const unsigned char *key = data;
--
1.8.0.2
^ permalink raw reply related
* [PATCH 13/15] batman-adv: fix typo in debug message
From: Antonio Quartulli @ 2013-01-13 23:41 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner
In-Reply-To: <1358120480-25960-1-git-send-email-ordex@autistici.org>
in bat_iv_ogm.c a debug message should print "tq" instead of "td"
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
---
net/batman-adv/bat_iv_ogm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index d90e925..8f2de43 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -1028,7 +1028,7 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr,
is_single_hop_neigh = true;
batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
- "Received BATMAN packet via NB: %pM, IF: %s [%pM] (from OG: %pM, via prev OG: %pM, seqno %u, ttvn %u, crc %#.4x, changes %u, td %d, TTL %d, V %d, IDF %d)\n",
+ "Received BATMAN packet via NB: %pM, IF: %s [%pM] (from OG: %pM, via prev OG: %pM, seqno %u, ttvn %u, crc %#.4x, changes %u, tq %d, TTL %d, V %d, IDF %d)\n",
ethhdr->h_source, if_incoming->net_dev->name,
if_incoming->net_dev->dev_addr, batadv_ogm_packet->orig,
batadv_ogm_packet->prev_sender,
--
1.8.0.2
^ permalink raw reply related
* [PATCH 14/15] batman-adv: remove unused variable from orig_node struct
From: Antonio Quartulli @ 2013-01-13 23:41 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner, Antonio Quartulli
In-Reply-To: <1358120480-25960-1-git-send-email-ordex@autistici.org>
From: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
net/batman-adv/types.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 030ce41..441880a 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -119,7 +119,6 @@ struct batadv_orig_node {
spinlock_t ogm_cnt_lock;
/* bcast_seqno_lock protects bcast_bits, last_bcast_seqno */
spinlock_t bcast_seqno_lock;
- spinlock_t tt_list_lock; /* protects tt_list */
atomic_t bond_candidates;
struct list_head bond_list;
};
--
1.8.0.2
^ permalink raw reply related
* [PATCH 15/15] batman-adv: unbloat batadv_priv if debug is not enabled
From: Antonio Quartulli @ 2013-01-13 23:41 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner, Antonio Quartulli
In-Reply-To: <1358120480-25960-1-git-send-email-ordex@autistici.org>
From: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
net/batman-adv/debugfs.c | 1 -
net/batman-adv/soft-interface.c | 2 ++
net/batman-adv/types.h | 4 ++++
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/debugfs.c b/net/batman-adv/debugfs.c
index 5136d32..55a9007 100644
--- a/net/batman-adv/debugfs.c
+++ b/net/batman-adv/debugfs.c
@@ -229,7 +229,6 @@ static void batadv_debug_log_cleanup(struct batadv_priv *bat_priv)
#else /* CONFIG_BATMAN_ADV_DEBUG */
static int batadv_debug_log_setup(struct batadv_priv *bat_priv)
{
- bat_priv->debug_log = NULL;
return 0;
}
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 57714f8..3d68166 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -493,7 +493,9 @@ struct net_device *batadv_softif_create(const char *name)
atomic_set(&bat_priv->gw_bandwidth, 41);
atomic_set(&bat_priv->orig_interval, 1000);
atomic_set(&bat_priv->hop_penalty, 30);
+#ifdef CONFIG_BATMAN_ADV_DEBUG
atomic_set(&bat_priv->log_level, 0);
+#endif
atomic_set(&bat_priv->fragmentation, 1);
atomic_set(&bat_priv->bcast_queue_left, BATADV_BCAST_QUEUE_LEN);
atomic_set(&bat_priv->batman_queue_left, BATADV_BATMAN_QUEUE_LEN);
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 441880a..d8061ac 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -284,12 +284,16 @@ struct batadv_priv {
atomic_t gw_bandwidth; /* gw bandwidth */
atomic_t orig_interval; /* uint */
atomic_t hop_penalty; /* uint */
+#ifdef CONFIG_BATMAN_ADV_DEBUG
atomic_t log_level; /* uint */
+#endif
atomic_t bcast_seqno;
atomic_t bcast_queue_left;
atomic_t batman_queue_left;
char num_ifaces;
+#ifdef CONFIG_BATMAN_ADV_DEBUG
struct batadv_debug_log *debug_log;
+#endif
struct kobject *mesh_obj;
struct dentry *debug_dir;
struct hlist_head forw_bat_list;
--
1.8.0.2
^ permalink raw reply related
* Re: [PATCH net-next 0/7] IPv6 updates
From: David Miller @ 2013-01-14 1:32 UTC (permalink / raw)
To: yoshfuji; +Cc: netdev
In-Reply-To: <50F2CCA0.4080500@linux-ipv6.org>
From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Date: Mon, 14 Jan 2013 00:02:56 +0900
> Hello.
>
> YOSHIFUJI Hideaki (7):
> ipv6: Introduce ip6_flow_hdr() to fill version, tclass and flowlabel.
> ipv6: Introduce ip6_flowinfo() to extract flowinfo (tclass +
> flowlabel).
> ipv6: Use ipv6_get_dsfield() instead of ipv6_tclass().
> ipv6: Make ipv6_is_mld() inline and use it from ip6_mc_input().
> ipv6 route: Use ipv6_addr_hash() in rt6_info_hash_nhsfn().
> ipv6 xfrm: Use ipv6_addr_hash() in xfrm6_tunnel_spi_hash_byaddr().
> ipv6: Store Router Alert option in IP6CB directly.
Series applied, thanks.
^ permalink raw reply
* Re: pull request: batman-adv 2013-01-14
From: David Miller @ 2013-01-14 1:39 UTC (permalink / raw)
To: ordex; +Cc: netdev, b.a.t.m.a.n
In-Reply-To: <1358120480-25960-1-git-send-email-ordex@autistici.org>
From: Antonio Quartulli <ordex@autistici.org>
Date: Mon, 14 Jan 2013 09:41:05 +1000
> this is our first changeset intended for net-next/linux-3.9.
> In this batch you have mostly code refactoring, style adjustments and output
> beautifications.
> The only new 'behaviours' are:
> - prevent the TT component from learning multicast mac addresses as they are not
> really handled (yet)
> - initialise own lockdep class for each hash table in order to avoid false
> positive from lockdep
...
> git://git.open-mesh.org/linux-merge.git tags/batman-adv-for-davem
Pulled, thanks.
^ permalink raw reply
* [PATCH net-next] ipv6: Move comment to right place.
From: YOSHIFUJI Hideaki @ 2013-01-14 2:02 UTC (permalink / raw)
To: David Miller, netdev; +Cc: YOSHIFUJI Hideaki
IN6ADDR_* and in6addr_* are not exported to userspace, and are defined
in include/linux/in6.h.
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
include/linux/in6.h | 4 ++++
include/uapi/linux/in6.h | 5 -----
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/include/linux/in6.h b/include/linux/in6.h
index 9e2ae26..a16e193 100644
--- a/include/linux/in6.h
+++ b/include/linux/in6.h
@@ -22,6 +22,10 @@
#include <uapi/linux/in6.h>
+/* IPv6 Wildcard Address (::) and Loopback Address (::1) defined in RFC2553
+ * NOTE: Be aware the IN6ADDR_* constants and in6addr_* externals are defined
+ * in network byte order, not in host byte order as are the IPv4 equivalents
+ */
extern const struct in6_addr in6addr_any;
#define IN6ADDR_ANY_INIT { { { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 } } }
extern const struct in6_addr in6addr_loopback;
diff --git a/include/uapi/linux/in6.h b/include/uapi/linux/in6.h
index f79c372..5673b97 100644
--- a/include/uapi/linux/in6.h
+++ b/include/uapi/linux/in6.h
@@ -38,11 +38,6 @@ struct in6_addr {
#define s6_addr32 in6_u.u6_addr32
};
-/* IPv6 Wildcard Address (::) and Loopback Address (::1) defined in RFC2553
- * NOTE: Be aware the IN6ADDR_* constants and in6addr_* externals are defined
- * in network byte order, not in host byte order as are the IPv4 equivalents
- */
-
struct sockaddr_in6 {
unsigned short int sin6_family; /* AF_INET6 */
__be16 sin6_port; /* Transport layer port # */
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH net-next] ipv6: Move comment to right place.
From: David Miller @ 2013-01-14 2:05 UTC (permalink / raw)
To: yoshfuji; +Cc: netdev
In-Reply-To: <50F3671E.2090601@linux-ipv6.org>
From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Date: Mon, 14 Jan 2013 11:02:06 +0900
> IN6ADDR_* and in6addr_* are not exported to userspace, and are defined
> in include/linux/in6.h.
>
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Applied.
^ permalink raw reply
* [net-next:master 126/134] include/net/ipv6.h:555:26: sparse: cast to restricted __be32
From: Fengguang Wu @ 2013-01-14 2:55 UTC (permalink / raw)
To: YOSHIFUJI Hideaki / 吉藤英明; +Cc: netdev
In-Reply-To: <50f370b4.sImtI783JLS2vUID%yuanhan.liu@linux.intel.com>
Hi YOSHIFUJI,
FYI, there are new sparse warnings show up in
tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
head: 25d46f43a911b08c5aa8c8fd4fe7fa9b36445068
commit: 3e4e4c1f2da66b29ee9379ca29f8dd620c2b5a1f [126/134] ipv6: Introduce ip6_flow_hdr() to fill version, tclass and flowlabel.
>> include/net/ipv6.h:555:26: sparse: cast to restricted __be32
>> include/net/ipv6.h:555:63: sparse: restricted __be32 degrades to integer
>> include/net/ipv6.h:555:24: sparse: incorrect type in assignment (different base types)
include/net/ipv6.h:555:24: expected restricted __be32 [usertype] <noident>
include/net/ipv6.h:555:24: got unsigned int
vim +555 include/net/ipv6.h
971f359d YOSHIFUJI Hideaki 2005-11-08 549 /*
3e4e4c1f YOSHIFUJI Hideaki / 吉藤英明 2013-01-13 550 * Header manipulation
3e4e4c1f YOSHIFUJI Hideaki / 吉藤英明 2013-01-13 551 */
3e4e4c1f YOSHIFUJI Hideaki / 吉藤英明 2013-01-13 552 static inline void ip6_flow_hdr(struct ipv6hdr *hdr, unsigned int tclass,
3e4e4c1f YOSHIFUJI Hideaki / 吉藤英明 2013-01-13 553 __be32 flowlabel)
3e4e4c1f YOSHIFUJI Hideaki / 吉藤英明 2013-01-13 554 {
3e4e4c1f YOSHIFUJI Hideaki / 吉藤英明 2013-01-13 @555 *(__be32 *)hdr = ntohl(0x60000000 | (tclass << 20)) | flowlabel;
3e4e4c1f YOSHIFUJI Hideaki / 吉藤英明 2013-01-13 556 }
3e4e4c1f YOSHIFUJI Hideaki / 吉藤英明 2013-01-13 557
3e4e4c1f YOSHIFUJI Hideaki / 吉藤英明 2013-01-13 558 /*
---
0-DAY kernel build testing backend Open Source Technology Center
Fengguang Wu, Yuanhan Liu Intel Corporation
^ permalink raw reply
* Re: [PATCH V3 2/2] vhost: handle polling errors
From: Jason Wang @ 2013-01-14 2:59 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: kvm, eric.dumazet, netdev, linux-kernel, virtualization, davem
In-Reply-To: <20130113111035.GA23431@redhat.com>
On 01/13/2013 07:10 PM, Michael S. Tsirkin wrote:
> On Mon, Jan 07, 2013 at 11:04:32PM +0800, Jason Wang wrote:
>> On 01/07/2013 10:55 PM, Michael S. Tsirkin wrote:
>>> On Mon, Jan 07, 2013 at 12:38:17PM +0800, Jason Wang wrote:
>>>> On 01/06/2013 09:22 PM, Michael S. Tsirkin wrote:
>>>>> On Sun, Jan 06, 2013 at 03:18:38PM +0800, Jason Wang wrote:
>>>>>> Polling errors were ignored by vhost/vhost_net, this may lead to crash when
>>>>>> trying to remove vhost from waitqueue when after the polling is failed. Solve
>>>>>> this problem by:
>>>>>>
>>>>>> - checking the poll->wqh before trying to remove from waitqueue
>>>>>> - report an error when poll() returns a POLLERR in vhost_start_poll()
>>>>>> - report an error when vhost_start_poll() fails in
>>>>>> vhost_vring_ioctl()/vhost_net_set_backend() which is used to notify the
>>>>>> failure to userspace.
>>>>>> - report an error in the data path in vhost_net when meet polling errors.
>>>>>>
>>>>>> After those changes, we can safely drop the tx polling state in vhost_net since
>>>>>> it was replaced by the checking of poll->wqh.
>>>>>>
>>>>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>>>>> ---
>>>>>> drivers/vhost/net.c | 74 ++++++++++++++++--------------------------------
>>>>>> drivers/vhost/vhost.c | 31 +++++++++++++++-----
>>>>>> drivers/vhost/vhost.h | 2 +-
>>>>>> 3 files changed, 49 insertions(+), 58 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>>>>>> index d10ad6f..125c1e5 100644
>>>>>> --- a/drivers/vhost/net.c
>>>>>> +++ b/drivers/vhost/net.c
>>>>>> @@ -64,20 +64,10 @@ enum {
>>>>>> VHOST_NET_VQ_MAX = 2,
>>>>>> };
>>>>>>
>>>>>> -enum vhost_net_poll_state {
>>>>>> - VHOST_NET_POLL_DISABLED = 0,
>>>>>> - VHOST_NET_POLL_STARTED = 1,
>>>>>> - VHOST_NET_POLL_STOPPED = 2,
>>>>>> -};
>>>>>> -
>>>>>> struct vhost_net {
>>>>>> struct vhost_dev dev;
>>>>>> struct vhost_virtqueue vqs[VHOST_NET_VQ_MAX];
>>>>>> struct vhost_poll poll[VHOST_NET_VQ_MAX];
>>>>>> - /* Tells us whether we are polling a socket for TX.
>>>>>> - * We only do this when socket buffer fills up.
>>>>>> - * Protected by tx vq lock. */
>>>>>> - enum vhost_net_poll_state tx_poll_state;
>>>>>> /* Number of TX recently submitted.
>>>>>> * Protected by tx vq lock. */
>>>>>> unsigned tx_packets;
>>>>>> @@ -155,24 +145,6 @@ static void copy_iovec_hdr(const struct iovec *from, struct iovec *to,
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> -/* Caller must have TX VQ lock */
>>>>>> -static void tx_poll_stop(struct vhost_net *net)
>>>>>> -{
>>>>>> - if (likely(net->tx_poll_state != VHOST_NET_POLL_STARTED))
>>>>>> - return;
>>>>>> - vhost_poll_stop(net->poll + VHOST_NET_VQ_TX);
>>>>>> - net->tx_poll_state = VHOST_NET_POLL_STOPPED;
>>>>>> -}
>>>>>> -
>>>>>> -/* Caller must have TX VQ lock */
>>>>>> -static void tx_poll_start(struct vhost_net *net, struct socket *sock)
>>>>>> -{
>>>>>> - if (unlikely(net->tx_poll_state != VHOST_NET_POLL_STOPPED))
>>>>>> - return;
>>>>>> - vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
>>>>>> - net->tx_poll_state = VHOST_NET_POLL_STARTED;
>>>>>> -}
>>>>>> -
>>>>>> /* In case of DMA done not in order in lower device driver for some reason.
>>>>>> * upend_idx is used to track end of used idx, done_idx is used to track head
>>>>>> * of used idx. Once lower device DMA done contiguously, we will signal KVM
>>>>>> @@ -227,6 +199,7 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
>>>>>> static void handle_tx(struct vhost_net *net)
>>>>>> {
>>>>>> struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX];
>>>>>> + struct vhost_poll *poll = net->poll + VHOST_NET_VQ_TX;
>>>>>> unsigned out, in, s;
>>>>>> int head;
>>>>>> struct msghdr msg = {
>>>>>> @@ -252,7 +225,8 @@ static void handle_tx(struct vhost_net *net)
>>>>>> wmem = atomic_read(&sock->sk->sk_wmem_alloc);
>>>>>> if (wmem >= sock->sk->sk_sndbuf) {
>>>>>> mutex_lock(&vq->mutex);
>>>>>> - tx_poll_start(net, sock);
>>>>>> + if (vhost_poll_start(poll, sock->file))
>>>>>> + vq_err(vq, "Fail to start TX polling\n");
>>>>> s/Fail/Failed/
>>>>>
>>>>> A question though: how can this happen? Could you clarify please?
>>>>> Maybe we can find a way to prevent this error?
>>>> Two conditions I think this can happen:
>>>>
>>>> 1) a buggy userspace disable a queue through TUNSETQUEUE
>>>> 2) the net device were gone
>>>>
>>>> For 1, looks like we can delay the disabling until the refcnt goes to
>>>> zero. For 2 may needs more changes.
>>> I'd expect keeping a socket reference would prevent both issues.
>>> Doesn't it?
>> Doesn't work for 2 I think, the socket didn't hold a refcnt of the
>> device, so the device can go away at anytime. Although we can change
>> this, but it's the behaviour before multiqueue support.
> Hmm there's one scenario that does seem to
> trigger this: queue can get disabled
> and then poll fails.
>
> Is this the only issue?
Another one I think we can trigger is:
- start vhost thread
- do ip link del link dev tap0 to delete the tap device
In this case, the netdevice is unregistered but the file/socket still exist.
>
>>>> Not sure it's worth to do this work,
>>>> maybe a warning is enough just like other failure.
>>> With other failures, you normally can correct the error then
>>> kick to have it restart. This is soomething thagt would not
>>> work here.
>> If userspace is wrote correctly, (e.g passing a fd with correct state)
>> it can also be corrected.
>>>>>> mutex_unlock(&vq->mutex);
>>>>>> return;
>>>>>> }
>>>>>> @@ -261,7 +235,7 @@ static void handle_tx(struct vhost_net *net)
>>>>>> vhost_disable_notify(&net->dev, vq);
>>>>>>
>>>>>> if (wmem < sock->sk->sk_sndbuf / 2)
>>>>>> - tx_poll_stop(net);
>>>>>> + vhost_poll_stop(poll);
>>>>>> hdr_size = vq->vhost_hlen;
>>>>>> zcopy = vq->ubufs;
>>>>>>
>>>>>> @@ -283,8 +257,10 @@ static void handle_tx(struct vhost_net *net)
>>>>>>
>>>>>> wmem = atomic_read(&sock->sk->sk_wmem_alloc);
>>>>>> if (wmem >= sock->sk->sk_sndbuf * 3 / 4) {
>>>>>> - tx_poll_start(net, sock);
>>>>>> - set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
>>>>>> + if (vhost_poll_start(poll, sock->file))
>>>>>> + vq_err(vq, "Fail to start TX polling\n");
>>>>>> + else
>>>>>> + set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
>>>>>> break;
>>>>>> }
>>>>>> /* If more outstanding DMAs, queue the work.
>>>>>> @@ -294,8 +270,10 @@ static void handle_tx(struct vhost_net *net)
>>>>>> (vq->upend_idx - vq->done_idx) :
>>>>>> (vq->upend_idx + UIO_MAXIOV - vq->done_idx);
>>>>>> if (unlikely(num_pends > VHOST_MAX_PEND)) {
>>>>>> - tx_poll_start(net, sock);
>>>>>> - set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
>>>>>> + if (vhost_poll_start(poll, sock->file))
>>>>>> + vq_err(vq, "Fail to start TX polling\n");
>>>>>> + else
>>>>>> + set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
>>>>>> break;
>>>>>> }
>>>>>> if (unlikely(vhost_enable_notify(&net->dev, vq))) {
>>>>>> @@ -360,7 +338,8 @@ static void handle_tx(struct vhost_net *net)
>>>>>> }
>>>>>> vhost_discard_vq_desc(vq, 1);
>>>>>> if (err == -EAGAIN || err == -ENOBUFS)
>>>>>> - tx_poll_start(net, sock);
>>>>>> + if (vhost_poll_start(poll, sock->file))
>>>>>> + vq_err(vq, "Fail to start TX polling\n");
>>>>>> break;
>>>>>> }
>>>>>> if (err != len)
>>>>>> @@ -623,7 +602,6 @@ static int vhost_net_open(struct inode *inode, struct file *f)
>>>>>>
>>>>>> vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT, dev);
>>>>>> vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN, dev);
>>>>>> - n->tx_poll_state = VHOST_NET_POLL_DISABLED;
>>>>>>
>>>>>> f->private_data = n;
>>>>>>
>>>>>> @@ -633,29 +611,25 @@ static int vhost_net_open(struct inode *inode, struct file *f)
>>>>>> static void vhost_net_disable_vq(struct vhost_net *n,
>>>>>> struct vhost_virtqueue *vq)
>>>>>> {
>>>>>> + struct vhost_poll *poll = n->poll + (vq - n->vqs);
>>>>>> +
>>>>>> if (!vq->private_data)
>>>>>> return;
>>>>>> - if (vq == n->vqs + VHOST_NET_VQ_TX) {
>>>>>> - tx_poll_stop(n);
>>>>>> - n->tx_poll_state = VHOST_NET_POLL_DISABLED;
>>>>>> - } else
>>>>>> - vhost_poll_stop(n->poll + VHOST_NET_VQ_RX);
>>>>>> + vhost_poll_stop(poll);
>>>>>> }
>>>>>>
>>>>>> -static void vhost_net_enable_vq(struct vhost_net *n,
>>>>>> +static int vhost_net_enable_vq(struct vhost_net *n,
>>>>>> struct vhost_virtqueue *vq)
>>>>>> {
>>>>>> struct socket *sock;
>>>>>> + struct vhost_poll *poll = n->poll + (vq - n->vqs);
>>>>>>
>>>>>> sock = rcu_dereference_protected(vq->private_data,
>>>>>> lockdep_is_held(&vq->mutex));
>>>>>> if (!sock)
>>>>>> - return;
>>>>>> - if (vq == n->vqs + VHOST_NET_VQ_TX) {
>>>>>> - n->tx_poll_state = VHOST_NET_POLL_STOPPED;
>>>>>> - tx_poll_start(n, sock);
>>>>>> - } else
>>>>>> - vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
>>>>>> + return 0;
>>>>>> +
>>>>>> + return vhost_poll_start(poll, sock->file);
>>>>>> }
>>>>>>
>>>>>> static struct socket *vhost_net_stop_vq(struct vhost_net *n,
>>>>>> @@ -833,7 +807,9 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
>>>>>> r = vhost_init_used(vq);
>>>>>> if (r)
>>>>>> goto err_used;
>>>>>> - vhost_net_enable_vq(n, vq);
>>>>>> + r = vhost_net_enable_vq(n, vq);
>>>>>> + if (r)
>>>>>> + goto err_used;
>>>>>>
>>>>>> oldubufs = vq->ubufs;
>>>>>> vq->ubufs = ubufs;
>>>>>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>>>>>> index 34389f7..5c7a466 100644
>>>>>> --- a/drivers/vhost/vhost.c
>>>>>> +++ b/drivers/vhost/vhost.c
>>>>>> @@ -77,26 +77,41 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
>>>>>> init_poll_funcptr(&poll->table, vhost_poll_func);
>>>>>> poll->mask = mask;
>>>>>> poll->dev = dev;
>>>>>> + poll->wqh = NULL;
>>>>>>
>>>>>> vhost_work_init(&poll->work, fn);
>>>>>> }
>>>>>>
>>>>>> +/* Stop polling a file. After this function returns, it becomes safe to drop the
>>>>>> + * file reference. You must also flush afterwards. */
>>>>>> +void vhost_poll_stop(struct vhost_poll *poll)
>>>>>> +{
>>>>>> + if (poll->wqh) {
>>>>>> + remove_wait_queue(poll->wqh, &poll->wait);
>>>>>> + poll->wqh = NULL;
>>>>>> + }
>>>>>> +}
>>>>>> +
>>>>>> /* Start polling a file. We add ourselves to file's wait queue. The caller must
>>>>>> * keep a reference to a file until after vhost_poll_stop is called. */
>>>>>> -void vhost_poll_start(struct vhost_poll *poll, struct file *file)
>>>>>> +int vhost_poll_start(struct vhost_poll *poll, struct file *file)
>>>>>> {
>>>>>> unsigned long mask;
>>>>>> + int ret = 0;
>>>>>> +
>>>>>> + if (poll->wqh)
>>>>>> + return -EBUSY;
>>>>>>
>>>>> I think this should return success: we are already polling.
>>>>> Otherwise this would trigger a bug below I think.
>>>> Ok.
>>>>>> mask = file->f_op->poll(file, &poll->table);
>>>>>> if (mask)
>>>>>> vhost_poll_wakeup(&poll->wait, 0, 0, (void *)mask);
>>>>>> -}
>>>>>>
>>>>>> -/* Stop polling a file. After this function returns, it becomes safe to drop the
>>>>>> - * file reference. You must also flush afterwards. */
>>>>>> -void vhost_poll_stop(struct vhost_poll *poll)
>>>>>> -{
>>>>>> - remove_wait_queue(poll->wqh, &poll->wait);
>>>>>> + if (mask & POLLERR) {
>>>>>> + ret = -EINVAL;
>>>>>> + vhost_poll_stop(poll);
>>>>>> + }
>>>>>> +
>>>>>> + return ret;
>>>>>> }
>>>>>>
>>>>>> static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
>>>>>> @@ -792,7 +807,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
>>>>>> fput(filep);
>>>>>>
>>>>>> if (pollstart && vq->handle_kick)
>>>>>> - vhost_poll_start(&vq->poll, vq->kick);
>>>>>> + r = vhost_poll_start(&vq->poll, vq->kick);
>>>>>>
>>>>>> mutex_unlock(&vq->mutex);
>>>>>>
>>>>>> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
>>>>>> index 2639c58..17261e2 100644
>>>>>> --- a/drivers/vhost/vhost.h
>>>>>> +++ b/drivers/vhost/vhost.h
>>>>>> @@ -42,7 +42,7 @@ void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
>>>>>>
>>>>>> void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
>>>>>> unsigned long mask, struct vhost_dev *dev);
>>>>>> -void vhost_poll_start(struct vhost_poll *poll, struct file *file);
>>>>>> +int vhost_poll_start(struct vhost_poll *poll, struct file *file);
>>>>>> void vhost_poll_stop(struct vhost_poll *poll);
>>>>>> void vhost_poll_flush(struct vhost_poll *poll);
>>>>>> void vhost_poll_queue(struct vhost_poll *poll);
>>>>>> --
>>>>>> 1.7.1
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] tcp: fix a panic on UP machines in reqsk_fastopen_remove
From: Eric Dumazet @ 2013-01-14 4:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Ketan Kulkarni, Jerry Chu, Yuchung Cheng
From: Eric Dumazet <edumazet@google.com>
spin_is_locked() on a non !SMP build is kind of useless.
BUG_ON(!spin_is_locked(xx)) is guaranteed to crash.
Just remove this check in reqsk_fastopen_remove() as
the callers do hold the socket lock.
Reported-by: Ketan Kulkarni <ketkulka@gmail.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Jerry Chu <hkchu@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Dave Taht <dave.taht@gmail.com>
---
net/core/request_sock.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/core/request_sock.c b/net/core/request_sock.c
index c31d9e8..4425148 100644
--- a/net/core/request_sock.c
+++ b/net/core/request_sock.c
@@ -186,8 +186,6 @@ void reqsk_fastopen_remove(struct sock *sk, struct request_sock *req,
struct fastopen_queue *fastopenq =
inet_csk(lsk)->icsk_accept_queue.fastopenq;
- BUG_ON(!spin_is_locked(&sk->sk_lock.slock) && !sock_owned_by_user(sk));
-
tcp_sk(sk)->fastopen_rsk = NULL;
spin_lock_bh(&fastopenq->lock);
fastopenq->qlen--;
^ permalink raw reply related
* [PATCH net-next] drivers/net: Clean up orphaned probes in Space.c
From: Paul Gortmaker @ 2013-01-14 4:55 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Paul Gortmaker
The removal of the 8390 EISA drivers actually comprises the
complete content of the EISA probe block, so we can now remove
that block, and its hook into the unified probe. Note that
the deleted comment mentions PCI probes, but they long since
moved elsewhere, so no PCI probes are touched here.
We get rid of the orphaned EISA probe prototypes, and a couple
of left over MCA probe prototypes at the same time.
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
diff --git a/drivers/net/Space.c b/drivers/net/Space.c
index e3f0fac..5637c26 100644
--- a/drivers/net/Space.c
+++ b/drivers/net/Space.c
@@ -37,10 +37,8 @@
ethernet adaptor have the name "eth[0123...]".
*/
-extern struct net_device *ne2_probe(int unit);
extern struct net_device *hp100_probe(int unit);
extern struct net_device *ultra_probe(int unit);
-extern struct net_device *ultra32_probe(int unit);
extern struct net_device *wd_probe(int unit);
extern struct net_device *el2_probe(int unit);
extern struct net_device *ne_probe(int unit);
@@ -55,11 +53,7 @@ extern struct net_device *i82596_probe(int unit);
extern struct net_device *ewrk3_probe(int unit);
extern struct net_device *el1_probe(int unit);
extern struct net_device *el16_probe(int unit);
-extern struct net_device *elmc_probe(int unit);
extern struct net_device *elplus_probe(int unit);
-extern struct net_device *ac3200_probe(int unit);
-extern struct net_device *es_probe(int unit);
-extern struct net_device *lne390_probe(int unit);
extern struct net_device *e2100_probe(int unit);
extern struct net_device *ni5010_probe(int unit);
extern struct net_device *ni52_probe(int unit);
@@ -77,7 +71,6 @@ extern struct net_device *tc515_probe(int unit);
extern struct net_device *lance_probe(int unit);
extern struct net_device *mac8390_probe(int unit);
extern struct net_device *mac89x0_probe(int unit);
-extern struct net_device *mc32_probe(int unit);
extern struct net_device *cops_probe(int unit);
extern struct net_device *ltpc_probe(void);
@@ -111,29 +104,6 @@ static int __init probe_list2(int unit, struct devprobe2 *p, int autoprobe)
}
/*
- * This is a bit of an artificial separation as there are PCI drivers
- * that also probe for EISA cards (in the PCI group) and there are ISA
- * drivers that probe for EISA cards (in the ISA group). These are the
- * legacy EISA only driver probes, and also the legacy PCI probes
- */
-
-static struct devprobe2 eisa_probes[] __initdata = {
-#ifdef CONFIG_ULTRA32
- {ultra32_probe, 0},
-#endif
-#ifdef CONFIG_AC3200
- {ac3200_probe, 0},
-#endif
-#ifdef CONFIG_ES3210
- {es_probe, 0},
-#endif
-#ifdef CONFIG_LNE390
- {lne390_probe, 0},
-#endif
- {NULL, 0},
-};
-
-/*
* ISA probes that touch addresses < 0x400 (including those that also
* look for EISA/PCI cards in addition to ISA cards).
*/
@@ -264,7 +234,6 @@ static void __init ethif_probe2(int unit)
return;
(void)( probe_list2(unit, m68k_probes, base_addr == 0) &&
- probe_list2(unit, eisa_probes, base_addr == 0) &&
probe_list2(unit, isa_probes, base_addr == 0) &&
probe_list2(unit, parport_probes, base_addr == 0));
}
--
1.8.1
^ permalink raw reply related
* Re: [PATCH net-next] pkt_sched: namespace aware ifb
From: Eric Dumazet @ 2013-01-14 5:40 UTC (permalink / raw)
To: Benjamin LaHaise; +Cc: Jamal Hadi Salim, David Miller, socketcan, netdev
In-Reply-To: <1358096257.8744.112.camel@edumazet-glaptop>
On Sun, 2013-01-13 at 08:57 -0800, Eric Dumazet wrote:
> On Sun, 2013-01-13 at 11:41 -0500, Benjamin LaHaise wrote:
> > pkt_sched: namespace aware ifb v2
> >
> > Eric Dumazet pointed out that act_mirred needs to find the current net_ns,
> > and struct net pointer is not provided in the call chain. His original
> > patch made use of current->nsproxy->net_ns to find the network namespace,
> > but this fails to work correctly for userspace code that makes use of
> > netlink sockets in different network namespaces. Instead, pass the
> > "struct net *" down along the call chain to where it is needed.
> >
> > Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
> > ---
>
> OK I'll test it at the end of the (sunny) day.
>
Could you resend the patch without the ifb part, I tested it
and it worked well for me, thanks !
^ permalink raw reply
* Re: [PATCH V3 2/2] vhost: handle polling errors
From: Michael S. Tsirkin @ 2013-01-14 6:57 UTC (permalink / raw)
To: Jason Wang; +Cc: kvm, eric.dumazet, netdev, linux-kernel, virtualization, davem
In-Reply-To: <50F37476.4070501@redhat.com>
On Mon, Jan 14, 2013 at 10:59:02AM +0800, Jason Wang wrote:
> On 01/13/2013 07:10 PM, Michael S. Tsirkin wrote:
> > On Mon, Jan 07, 2013 at 11:04:32PM +0800, Jason Wang wrote:
> >> On 01/07/2013 10:55 PM, Michael S. Tsirkin wrote:
> >>> On Mon, Jan 07, 2013 at 12:38:17PM +0800, Jason Wang wrote:
> >>>> On 01/06/2013 09:22 PM, Michael S. Tsirkin wrote:
> >>>>> On Sun, Jan 06, 2013 at 03:18:38PM +0800, Jason Wang wrote:
> >>>>>> Polling errors were ignored by vhost/vhost_net, this may lead to crash when
> >>>>>> trying to remove vhost from waitqueue when after the polling is failed. Solve
> >>>>>> this problem by:
> >>>>>>
> >>>>>> - checking the poll->wqh before trying to remove from waitqueue
> >>>>>> - report an error when poll() returns a POLLERR in vhost_start_poll()
> >>>>>> - report an error when vhost_start_poll() fails in
> >>>>>> vhost_vring_ioctl()/vhost_net_set_backend() which is used to notify the
> >>>>>> failure to userspace.
> >>>>>> - report an error in the data path in vhost_net when meet polling errors.
> >>>>>>
> >>>>>> After those changes, we can safely drop the tx polling state in vhost_net since
> >>>>>> it was replaced by the checking of poll->wqh.
> >>>>>>
> >>>>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> >>>>>> ---
> >>>>>> drivers/vhost/net.c | 74 ++++++++++++++++--------------------------------
> >>>>>> drivers/vhost/vhost.c | 31 +++++++++++++++-----
> >>>>>> drivers/vhost/vhost.h | 2 +-
> >>>>>> 3 files changed, 49 insertions(+), 58 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> >>>>>> index d10ad6f..125c1e5 100644
> >>>>>> --- a/drivers/vhost/net.c
> >>>>>> +++ b/drivers/vhost/net.c
> >>>>>> @@ -64,20 +64,10 @@ enum {
> >>>>>> VHOST_NET_VQ_MAX = 2,
> >>>>>> };
> >>>>>>
> >>>>>> -enum vhost_net_poll_state {
> >>>>>> - VHOST_NET_POLL_DISABLED = 0,
> >>>>>> - VHOST_NET_POLL_STARTED = 1,
> >>>>>> - VHOST_NET_POLL_STOPPED = 2,
> >>>>>> -};
> >>>>>> -
> >>>>>> struct vhost_net {
> >>>>>> struct vhost_dev dev;
> >>>>>> struct vhost_virtqueue vqs[VHOST_NET_VQ_MAX];
> >>>>>> struct vhost_poll poll[VHOST_NET_VQ_MAX];
> >>>>>> - /* Tells us whether we are polling a socket for TX.
> >>>>>> - * We only do this when socket buffer fills up.
> >>>>>> - * Protected by tx vq lock. */
> >>>>>> - enum vhost_net_poll_state tx_poll_state;
> >>>>>> /* Number of TX recently submitted.
> >>>>>> * Protected by tx vq lock. */
> >>>>>> unsigned tx_packets;
> >>>>>> @@ -155,24 +145,6 @@ static void copy_iovec_hdr(const struct iovec *from, struct iovec *to,
> >>>>>> }
> >>>>>> }
> >>>>>>
> >>>>>> -/* Caller must have TX VQ lock */
> >>>>>> -static void tx_poll_stop(struct vhost_net *net)
> >>>>>> -{
> >>>>>> - if (likely(net->tx_poll_state != VHOST_NET_POLL_STARTED))
> >>>>>> - return;
> >>>>>> - vhost_poll_stop(net->poll + VHOST_NET_VQ_TX);
> >>>>>> - net->tx_poll_state = VHOST_NET_POLL_STOPPED;
> >>>>>> -}
> >>>>>> -
> >>>>>> -/* Caller must have TX VQ lock */
> >>>>>> -static void tx_poll_start(struct vhost_net *net, struct socket *sock)
> >>>>>> -{
> >>>>>> - if (unlikely(net->tx_poll_state != VHOST_NET_POLL_STOPPED))
> >>>>>> - return;
> >>>>>> - vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
> >>>>>> - net->tx_poll_state = VHOST_NET_POLL_STARTED;
> >>>>>> -}
> >>>>>> -
> >>>>>> /* In case of DMA done not in order in lower device driver for some reason.
> >>>>>> * upend_idx is used to track end of used idx, done_idx is used to track head
> >>>>>> * of used idx. Once lower device DMA done contiguously, we will signal KVM
> >>>>>> @@ -227,6 +199,7 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
> >>>>>> static void handle_tx(struct vhost_net *net)
> >>>>>> {
> >>>>>> struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX];
> >>>>>> + struct vhost_poll *poll = net->poll + VHOST_NET_VQ_TX;
> >>>>>> unsigned out, in, s;
> >>>>>> int head;
> >>>>>> struct msghdr msg = {
> >>>>>> @@ -252,7 +225,8 @@ static void handle_tx(struct vhost_net *net)
> >>>>>> wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> >>>>>> if (wmem >= sock->sk->sk_sndbuf) {
> >>>>>> mutex_lock(&vq->mutex);
> >>>>>> - tx_poll_start(net, sock);
> >>>>>> + if (vhost_poll_start(poll, sock->file))
> >>>>>> + vq_err(vq, "Fail to start TX polling\n");
> >>>>> s/Fail/Failed/
> >>>>>
> >>>>> A question though: how can this happen? Could you clarify please?
> >>>>> Maybe we can find a way to prevent this error?
> >>>> Two conditions I think this can happen:
> >>>>
> >>>> 1) a buggy userspace disable a queue through TUNSETQUEUE
> >>>> 2) the net device were gone
> >>>>
> >>>> For 1, looks like we can delay the disabling until the refcnt goes to
> >>>> zero. For 2 may needs more changes.
> >>> I'd expect keeping a socket reference would prevent both issues.
> >>> Doesn't it?
> >> Doesn't work for 2 I think, the socket didn't hold a refcnt of the
> >> device, so the device can go away at anytime. Although we can change
> >> this, but it's the behaviour before multiqueue support.
> > Hmm there's one scenario that does seem to
> > trigger this: queue can get disabled
> > and then poll fails.
> >
> > Is this the only issue?
>
> Another one I think we can trigger is:
>
> - start vhost thread
> - do ip link del link dev tap0 to delete the tap device
>
> In this case, the netdevice is unregistered but the file/socket still exist.
Yes but in this case poll_wait is called so apparently no issue
with existing code? We only have an issue if poll_wait is
not called right?
Maybe the simplest fix is to invoke poll_wait before
checking if (!tun) ?
> >
> >>>> Not sure it's worth to do this work,
> >>>> maybe a warning is enough just like other failure.
> >>> With other failures, you normally can correct the error then
> >>> kick to have it restart. This is soomething thagt would not
> >>> work here.
> >> If userspace is wrote correctly, (e.g passing a fd with correct state)
> >> it can also be corrected.
> >>>>>> mutex_unlock(&vq->mutex);
> >>>>>> return;
> >>>>>> }
> >>>>>> @@ -261,7 +235,7 @@ static void handle_tx(struct vhost_net *net)
> >>>>>> vhost_disable_notify(&net->dev, vq);
> >>>>>>
> >>>>>> if (wmem < sock->sk->sk_sndbuf / 2)
> >>>>>> - tx_poll_stop(net);
> >>>>>> + vhost_poll_stop(poll);
> >>>>>> hdr_size = vq->vhost_hlen;
> >>>>>> zcopy = vq->ubufs;
> >>>>>>
> >>>>>> @@ -283,8 +257,10 @@ static void handle_tx(struct vhost_net *net)
> >>>>>>
> >>>>>> wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> >>>>>> if (wmem >= sock->sk->sk_sndbuf * 3 / 4) {
> >>>>>> - tx_poll_start(net, sock);
> >>>>>> - set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >>>>>> + if (vhost_poll_start(poll, sock->file))
> >>>>>> + vq_err(vq, "Fail to start TX polling\n");
> >>>>>> + else
> >>>>>> + set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >>>>>> break;
> >>>>>> }
> >>>>>> /* If more outstanding DMAs, queue the work.
> >>>>>> @@ -294,8 +270,10 @@ static void handle_tx(struct vhost_net *net)
> >>>>>> (vq->upend_idx - vq->done_idx) :
> >>>>>> (vq->upend_idx + UIO_MAXIOV - vq->done_idx);
> >>>>>> if (unlikely(num_pends > VHOST_MAX_PEND)) {
> >>>>>> - tx_poll_start(net, sock);
> >>>>>> - set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >>>>>> + if (vhost_poll_start(poll, sock->file))
> >>>>>> + vq_err(vq, "Fail to start TX polling\n");
> >>>>>> + else
> >>>>>> + set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >>>>>> break;
> >>>>>> }
> >>>>>> if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> >>>>>> @@ -360,7 +338,8 @@ static void handle_tx(struct vhost_net *net)
> >>>>>> }
> >>>>>> vhost_discard_vq_desc(vq, 1);
> >>>>>> if (err == -EAGAIN || err == -ENOBUFS)
> >>>>>> - tx_poll_start(net, sock);
> >>>>>> + if (vhost_poll_start(poll, sock->file))
> >>>>>> + vq_err(vq, "Fail to start TX polling\n");
> >>>>>> break;
> >>>>>> }
> >>>>>> if (err != len)
> >>>>>> @@ -623,7 +602,6 @@ static int vhost_net_open(struct inode *inode, struct file *f)
> >>>>>>
> >>>>>> vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT, dev);
> >>>>>> vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN, dev);
> >>>>>> - n->tx_poll_state = VHOST_NET_POLL_DISABLED;
> >>>>>>
> >>>>>> f->private_data = n;
> >>>>>>
> >>>>>> @@ -633,29 +611,25 @@ static int vhost_net_open(struct inode *inode, struct file *f)
> >>>>>> static void vhost_net_disable_vq(struct vhost_net *n,
> >>>>>> struct vhost_virtqueue *vq)
> >>>>>> {
> >>>>>> + struct vhost_poll *poll = n->poll + (vq - n->vqs);
> >>>>>> +
> >>>>>> if (!vq->private_data)
> >>>>>> return;
> >>>>>> - if (vq == n->vqs + VHOST_NET_VQ_TX) {
> >>>>>> - tx_poll_stop(n);
> >>>>>> - n->tx_poll_state = VHOST_NET_POLL_DISABLED;
> >>>>>> - } else
> >>>>>> - vhost_poll_stop(n->poll + VHOST_NET_VQ_RX);
> >>>>>> + vhost_poll_stop(poll);
> >>>>>> }
> >>>>>>
> >>>>>> -static void vhost_net_enable_vq(struct vhost_net *n,
> >>>>>> +static int vhost_net_enable_vq(struct vhost_net *n,
> >>>>>> struct vhost_virtqueue *vq)
> >>>>>> {
> >>>>>> struct socket *sock;
> >>>>>> + struct vhost_poll *poll = n->poll + (vq - n->vqs);
> >>>>>>
> >>>>>> sock = rcu_dereference_protected(vq->private_data,
> >>>>>> lockdep_is_held(&vq->mutex));
> >>>>>> if (!sock)
> >>>>>> - return;
> >>>>>> - if (vq == n->vqs + VHOST_NET_VQ_TX) {
> >>>>>> - n->tx_poll_state = VHOST_NET_POLL_STOPPED;
> >>>>>> - tx_poll_start(n, sock);
> >>>>>> - } else
> >>>>>> - vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
> >>>>>> + return 0;
> >>>>>> +
> >>>>>> + return vhost_poll_start(poll, sock->file);
> >>>>>> }
> >>>>>>
> >>>>>> static struct socket *vhost_net_stop_vq(struct vhost_net *n,
> >>>>>> @@ -833,7 +807,9 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> >>>>>> r = vhost_init_used(vq);
> >>>>>> if (r)
> >>>>>> goto err_used;
> >>>>>> - vhost_net_enable_vq(n, vq);
> >>>>>> + r = vhost_net_enable_vq(n, vq);
> >>>>>> + if (r)
> >>>>>> + goto err_used;
> >>>>>>
> >>>>>> oldubufs = vq->ubufs;
> >>>>>> vq->ubufs = ubufs;
> >>>>>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >>>>>> index 34389f7..5c7a466 100644
> >>>>>> --- a/drivers/vhost/vhost.c
> >>>>>> +++ b/drivers/vhost/vhost.c
> >>>>>> @@ -77,26 +77,41 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> >>>>>> init_poll_funcptr(&poll->table, vhost_poll_func);
> >>>>>> poll->mask = mask;
> >>>>>> poll->dev = dev;
> >>>>>> + poll->wqh = NULL;
> >>>>>>
> >>>>>> vhost_work_init(&poll->work, fn);
> >>>>>> }
> >>>>>>
> >>>>>> +/* Stop polling a file. After this function returns, it becomes safe to drop the
> >>>>>> + * file reference. You must also flush afterwards. */
> >>>>>> +void vhost_poll_stop(struct vhost_poll *poll)
> >>>>>> +{
> >>>>>> + if (poll->wqh) {
> >>>>>> + remove_wait_queue(poll->wqh, &poll->wait);
> >>>>>> + poll->wqh = NULL;
> >>>>>> + }
> >>>>>> +}
> >>>>>> +
> >>>>>> /* Start polling a file. We add ourselves to file's wait queue. The caller must
> >>>>>> * keep a reference to a file until after vhost_poll_stop is called. */
> >>>>>> -void vhost_poll_start(struct vhost_poll *poll, struct file *file)
> >>>>>> +int vhost_poll_start(struct vhost_poll *poll, struct file *file)
> >>>>>> {
> >>>>>> unsigned long mask;
> >>>>>> + int ret = 0;
> >>>>>> +
> >>>>>> + if (poll->wqh)
> >>>>>> + return -EBUSY;
> >>>>>>
> >>>>> I think this should return success: we are already polling.
> >>>>> Otherwise this would trigger a bug below I think.
> >>>> Ok.
> >>>>>> mask = file->f_op->poll(file, &poll->table);
> >>>>>> if (mask)
> >>>>>> vhost_poll_wakeup(&poll->wait, 0, 0, (void *)mask);
> >>>>>> -}
> >>>>>>
> >>>>>> -/* Stop polling a file. After this function returns, it becomes safe to drop the
> >>>>>> - * file reference. You must also flush afterwards. */
> >>>>>> -void vhost_poll_stop(struct vhost_poll *poll)
> >>>>>> -{
> >>>>>> - remove_wait_queue(poll->wqh, &poll->wait);
> >>>>>> + if (mask & POLLERR) {
> >>>>>> + ret = -EINVAL;
> >>>>>> + vhost_poll_stop(poll);
> >>>>>> + }
> >>>>>> +
> >>>>>> + return ret;
> >>>>>> }
> >>>>>>
> >>>>>> static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
> >>>>>> @@ -792,7 +807,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
> >>>>>> fput(filep);
> >>>>>>
> >>>>>> if (pollstart && vq->handle_kick)
> >>>>>> - vhost_poll_start(&vq->poll, vq->kick);
> >>>>>> + r = vhost_poll_start(&vq->poll, vq->kick);
> >>>>>>
> >>>>>> mutex_unlock(&vq->mutex);
> >>>>>>
> >>>>>> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> >>>>>> index 2639c58..17261e2 100644
> >>>>>> --- a/drivers/vhost/vhost.h
> >>>>>> +++ b/drivers/vhost/vhost.h
> >>>>>> @@ -42,7 +42,7 @@ void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
> >>>>>>
> >>>>>> void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> >>>>>> unsigned long mask, struct vhost_dev *dev);
> >>>>>> -void vhost_poll_start(struct vhost_poll *poll, struct file *file);
> >>>>>> +int vhost_poll_start(struct vhost_poll *poll, struct file *file);
> >>>>>> void vhost_poll_stop(struct vhost_poll *poll);
> >>>>>> void vhost_poll_flush(struct vhost_poll *poll);
> >>>>>> void vhost_poll_queue(struct vhost_poll *poll);
> >>>>>> --
> >>>>>> 1.7.1
> >>>>> --
> >>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
> >>>>> the body of a message to majordomo@vger.kernel.org
> >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe kvm" in
> >>> the body of a message to majordomo@vger.kernel.org
> >>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Ipsec autotest tools
From: bill4carson @ 2013-01-14 7:04 UTC (permalink / raw)
To: netdev; +Cc: Steffen Klassert
Hi, all
Is there any open-source ipsec auto test tools to use currently?
btw, I remember last time somebody send a ipsec patch about an wrong return value,
in the commit he said an ipsec auto test tools find it. It's bad I can't find it now.
thanks
--
八百里秦川尘土飞扬,三千万老陕齐吼秦腔。
--bill
^ permalink raw reply
* Re: [PATCH 2/2] ipv6: fix packet corruption when Dest/RT2 options are used
From: Romain KUNTZ @ 2013-01-14 7:21 UTC (permalink / raw)
To: netdev; +Cc: yoshfuji, davem, Steffen Klassert
In-Reply-To: <25F004CB-8D03-4280-BDBD-48C149E7321F@ipflavors.com>
On Jan 11, 2013, at 8:27 , Romain KUNTZ <r.kuntz@ipflavors.com> wrote:
> On Jan 5, 2013, at 17:19 , Romain KUNTZ <r.kuntz@ipflavors.com> wrote:
>> Commit 299b0767 (ipv6: Fix IPsec slowpath fragmentation problem)
>> has introduced a bug that provokes corrupted packets when Destination
>> Options or Routing Header Type 2 are used (such as with Mobile IPv6):
>> rt->rt6i_nfheader_len should be substracted to rt->dst.header_len,
>> and not to exthdrlen.
>>
>> This patch reverts to the original and correct behavior. Successfully
>> tested with and without IPsec activated for MH packets.
>>
>> Signed-off-by: Romain Kuntz <r.kuntz@ipflavors.com>
>> Acked-by: Steffen Klassert <steffen.klassert@secunet.com>
>> ---
>> net/ipv6/ip6_output.c | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
>> index 5552d13..0c7c03d 100644
>> --- a/net/ipv6/ip6_output.c
>> +++ b/net/ipv6/ip6_output.c
>> @@ -1213,10 +1213,10 @@ int ip6_append_data(struct sock *sk, int getfrag(void *from, char *to,
>> if (dst_allfrag(rt->dst.path))
>> cork->flags |= IPCORK_ALLFRAG;
>> cork->length = 0;
>> - exthdrlen = (opt ? opt->opt_flen : 0) - rt->rt6i_nfheader_len;
>> + exthdrlen = (opt ? opt->opt_flen : 0);
>> length += exthdrlen;
>> transhdrlen += exthdrlen;
>> - dst_exthdrlen = rt->dst.header_len;
>> + dst_exthdrlen = rt->dst.header_len - rt->rt6i_nfheader_len;
>> } else {
>> rt = (struct rt6_info *)cork->dst;
>> fl6 = &inet->cork.fl.u.ip6;
>> --
>> 1.7.2.5
>
> Resending this one adding the 'Acked-by: Steffen Klassert'.
I noticed in the patchwork that this patch is in state "Changes Requested" (http://patchwork.ozlabs.org/patch/209684/) but did not get any requests for changes. Any issues/comments regarding this patch?
Thank you,
Romain
^ permalink raw reply
* Re: [PATCH V3 2/2] vhost: handle polling errors
From: Jason Wang @ 2013-01-14 7:37 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: kvm, eric.dumazet, netdev, linux-kernel, virtualization, davem
In-Reply-To: <20130114065712.GA10312@redhat.com>
On 01/14/2013 02:57 PM, Michael S. Tsirkin wrote:
> On Mon, Jan 14, 2013 at 10:59:02AM +0800, Jason Wang wrote:
>> On 01/13/2013 07:10 PM, Michael S. Tsirkin wrote:
>>> On Mon, Jan 07, 2013 at 11:04:32PM +0800, Jason Wang wrote:
>>>> On 01/07/2013 10:55 PM, Michael S. Tsirkin wrote:
>>>>> On Mon, Jan 07, 2013 at 12:38:17PM +0800, Jason Wang wrote:
>>>>>> On 01/06/2013 09:22 PM, Michael S. Tsirkin wrote:
>>>>>>> On Sun, Jan 06, 2013 at 03:18:38PM +0800, Jason Wang wrote:
>>>>>>>> Polling errors were ignored by vhost/vhost_net, this may lead to crash when
>>>>>>>> trying to remove vhost from waitqueue when after the polling is failed. Solve
>>>>>>>> this problem by:
>>>>>>>>
>>>>>>>> - checking the poll->wqh before trying to remove from waitqueue
>>>>>>>> - report an error when poll() returns a POLLERR in vhost_start_poll()
>>>>>>>> - report an error when vhost_start_poll() fails in
>>>>>>>> vhost_vring_ioctl()/vhost_net_set_backend() which is used to notify the
>>>>>>>> failure to userspace.
>>>>>>>> - report an error in the data path in vhost_net when meet polling errors.
>>>>>>>>
>>>>>>>> After those changes, we can safely drop the tx polling state in vhost_net since
>>>>>>>> it was replaced by the checking of poll->wqh.
>>>>>>>>
>>>>>>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>>>>>>> ---
>>>>>>>> drivers/vhost/net.c | 74 ++++++++++++++++--------------------------------
>>>>>>>> drivers/vhost/vhost.c | 31 +++++++++++++++-----
>>>>>>>> drivers/vhost/vhost.h | 2 +-
>>>>>>>> 3 files changed, 49 insertions(+), 58 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>>>>>>>> index d10ad6f..125c1e5 100644
>>>>>>>> --- a/drivers/vhost/net.c
>>>>>>>> +++ b/drivers/vhost/net.c
>>>>>>>> @@ -64,20 +64,10 @@ enum {
>>>>>>>> VHOST_NET_VQ_MAX = 2,
>>>>>>>> };
>>>>>>>>
>>>>>>>> -enum vhost_net_poll_state {
>>>>>>>> - VHOST_NET_POLL_DISABLED = 0,
>>>>>>>> - VHOST_NET_POLL_STARTED = 1,
>>>>>>>> - VHOST_NET_POLL_STOPPED = 2,
>>>>>>>> -};
>>>>>>>> -
>>>>>>>> struct vhost_net {
>>>>>>>> struct vhost_dev dev;
>>>>>>>> struct vhost_virtqueue vqs[VHOST_NET_VQ_MAX];
>>>>>>>> struct vhost_poll poll[VHOST_NET_VQ_MAX];
>>>>>>>> - /* Tells us whether we are polling a socket for TX.
>>>>>>>> - * We only do this when socket buffer fills up.
>>>>>>>> - * Protected by tx vq lock. */
>>>>>>>> - enum vhost_net_poll_state tx_poll_state;
>>>>>>>> /* Number of TX recently submitted.
>>>>>>>> * Protected by tx vq lock. */
>>>>>>>> unsigned tx_packets;
>>>>>>>> @@ -155,24 +145,6 @@ static void copy_iovec_hdr(const struct iovec *from, struct iovec *to,
>>>>>>>> }
>>>>>>>> }
>>>>>>>>
>>>>>>>> -/* Caller must have TX VQ lock */
>>>>>>>> -static void tx_poll_stop(struct vhost_net *net)
>>>>>>>> -{
>>>>>>>> - if (likely(net->tx_poll_state != VHOST_NET_POLL_STARTED))
>>>>>>>> - return;
>>>>>>>> - vhost_poll_stop(net->poll + VHOST_NET_VQ_TX);
>>>>>>>> - net->tx_poll_state = VHOST_NET_POLL_STOPPED;
>>>>>>>> -}
>>>>>>>> -
>>>>>>>> -/* Caller must have TX VQ lock */
>>>>>>>> -static void tx_poll_start(struct vhost_net *net, struct socket *sock)
>>>>>>>> -{
>>>>>>>> - if (unlikely(net->tx_poll_state != VHOST_NET_POLL_STOPPED))
>>>>>>>> - return;
>>>>>>>> - vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
>>>>>>>> - net->tx_poll_state = VHOST_NET_POLL_STARTED;
>>>>>>>> -}
>>>>>>>> -
>>>>>>>> /* In case of DMA done not in order in lower device driver for some reason.
>>>>>>>> * upend_idx is used to track end of used idx, done_idx is used to track head
>>>>>>>> * of used idx. Once lower device DMA done contiguously, we will signal KVM
>>>>>>>> @@ -227,6 +199,7 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
>>>>>>>> static void handle_tx(struct vhost_net *net)
>>>>>>>> {
>>>>>>>> struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX];
>>>>>>>> + struct vhost_poll *poll = net->poll + VHOST_NET_VQ_TX;
>>>>>>>> unsigned out, in, s;
>>>>>>>> int head;
>>>>>>>> struct msghdr msg = {
>>>>>>>> @@ -252,7 +225,8 @@ static void handle_tx(struct vhost_net *net)
>>>>>>>> wmem = atomic_read(&sock->sk->sk_wmem_alloc);
>>>>>>>> if (wmem >= sock->sk->sk_sndbuf) {
>>>>>>>> mutex_lock(&vq->mutex);
>>>>>>>> - tx_poll_start(net, sock);
>>>>>>>> + if (vhost_poll_start(poll, sock->file))
>>>>>>>> + vq_err(vq, "Fail to start TX polling\n");
>>>>>>> s/Fail/Failed/
>>>>>>>
>>>>>>> A question though: how can this happen? Could you clarify please?
>>>>>>> Maybe we can find a way to prevent this error?
>>>>>> Two conditions I think this can happen:
>>>>>>
>>>>>> 1) a buggy userspace disable a queue through TUNSETQUEUE
>>>>>> 2) the net device were gone
>>>>>>
>>>>>> For 1, looks like we can delay the disabling until the refcnt goes to
>>>>>> zero. For 2 may needs more changes.
>>>>> I'd expect keeping a socket reference would prevent both issues.
>>>>> Doesn't it?
>>>> Doesn't work for 2 I think, the socket didn't hold a refcnt of the
>>>> device, so the device can go away at anytime. Although we can change
>>>> this, but it's the behaviour before multiqueue support.
>>> Hmm there's one scenario that does seem to
>>> trigger this: queue can get disabled
>>> and then poll fails.
>>>
>>> Is this the only issue?
>> Another one I think we can trigger is:
>>
>> - start vhost thread
>> - do ip link del link dev tap0 to delete the tap device
>>
>> In this case, the netdevice is unregistered but the file/socket still exist.
> Yes but in this case poll_wait is called so apparently no issue
> with existing code? We only have an issue if poll_wait is
> not called right?
Right.
> Maybe the simplest fix is to invoke poll_wait before
> checking if (!tun) ?
True, but we this depends on the assumption that each kind of backend
must make sure poll_wait() is called in .poll(). And we need to change
macvtap_poll() as well. And we'd better also pass those polling errors
to notify user about this wrong state.
>
>>>>>> Not sure it's worth to do this work,
>>>>>> maybe a warning is enough just like other failure.
>>>>> With other failures, you normally can correct the error then
>>>>> kick to have it restart. This is soomething thagt would not
>>>>> work here.
>>>> If userspace is wrote correctly, (e.g passing a fd with correct state)
>>>> it can also be corrected.
>>>>>>>> mutex_unlock(&vq->mutex);
>>>>>>>> return;
>>>>>>>> }
>>>>>>>> @@ -261,7 +235,7 @@ static void handle_tx(struct vhost_net *net)
>>>>>>>> vhost_disable_notify(&net->dev, vq);
>>>>>>>>
>>>>>>>> if (wmem < sock->sk->sk_sndbuf / 2)
>>>>>>>> - tx_poll_stop(net);
>>>>>>>> + vhost_poll_stop(poll);
>>>>>>>> hdr_size = vq->vhost_hlen;
>>>>>>>> zcopy = vq->ubufs;
>>>>>>>>
>>>>>>>> @@ -283,8 +257,10 @@ static void handle_tx(struct vhost_net *net)
>>>>>>>>
>>>>>>>> wmem = atomic_read(&sock->sk->sk_wmem_alloc);
>>>>>>>> if (wmem >= sock->sk->sk_sndbuf * 3 / 4) {
>>>>>>>> - tx_poll_start(net, sock);
>>>>>>>> - set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
>>>>>>>> + if (vhost_poll_start(poll, sock->file))
>>>>>>>> + vq_err(vq, "Fail to start TX polling\n");
>>>>>>>> + else
>>>>>>>> + set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
>>>>>>>> break;
>>>>>>>> }
>>>>>>>> /* If more outstanding DMAs, queue the work.
>>>>>>>> @@ -294,8 +270,10 @@ static void handle_tx(struct vhost_net *net)
>>>>>>>> (vq->upend_idx - vq->done_idx) :
>>>>>>>> (vq->upend_idx + UIO_MAXIOV - vq->done_idx);
>>>>>>>> if (unlikely(num_pends > VHOST_MAX_PEND)) {
>>>>>>>> - tx_poll_start(net, sock);
>>>>>>>> - set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
>>>>>>>> + if (vhost_poll_start(poll, sock->file))
>>>>>>>> + vq_err(vq, "Fail to start TX polling\n");
>>>>>>>> + else
>>>>>>>> + set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
>>>>>>>> break;
>>>>>>>> }
>>>>>>>> if (unlikely(vhost_enable_notify(&net->dev, vq))) {
>>>>>>>> @@ -360,7 +338,8 @@ static void handle_tx(struct vhost_net *net)
>>>>>>>> }
>>>>>>>> vhost_discard_vq_desc(vq, 1);
>>>>>>>> if (err == -EAGAIN || err == -ENOBUFS)
>>>>>>>> - tx_poll_start(net, sock);
>>>>>>>> + if (vhost_poll_start(poll, sock->file))
>>>>>>>> + vq_err(vq, "Fail to start TX polling\n");
>>>>>>>> break;
>>>>>>>> }
>>>>>>>> if (err != len)
>>>>>>>> @@ -623,7 +602,6 @@ static int vhost_net_open(struct inode *inode, struct file *f)
>>>>>>>>
>>>>>>>> vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT, dev);
>>>>>>>> vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN, dev);
>>>>>>>> - n->tx_poll_state = VHOST_NET_POLL_DISABLED;
>>>>>>>>
>>>>>>>> f->private_data = n;
>>>>>>>>
>>>>>>>> @@ -633,29 +611,25 @@ static int vhost_net_open(struct inode *inode, struct file *f)
>>>>>>>> static void vhost_net_disable_vq(struct vhost_net *n,
>>>>>>>> struct vhost_virtqueue *vq)
>>>>>>>> {
>>>>>>>> + struct vhost_poll *poll = n->poll + (vq - n->vqs);
>>>>>>>> +
>>>>>>>> if (!vq->private_data)
>>>>>>>> return;
>>>>>>>> - if (vq == n->vqs + VHOST_NET_VQ_TX) {
>>>>>>>> - tx_poll_stop(n);
>>>>>>>> - n->tx_poll_state = VHOST_NET_POLL_DISABLED;
>>>>>>>> - } else
>>>>>>>> - vhost_poll_stop(n->poll + VHOST_NET_VQ_RX);
>>>>>>>> + vhost_poll_stop(poll);
>>>>>>>> }
>>>>>>>>
>>>>>>>> -static void vhost_net_enable_vq(struct vhost_net *n,
>>>>>>>> +static int vhost_net_enable_vq(struct vhost_net *n,
>>>>>>>> struct vhost_virtqueue *vq)
>>>>>>>> {
>>>>>>>> struct socket *sock;
>>>>>>>> + struct vhost_poll *poll = n->poll + (vq - n->vqs);
>>>>>>>>
>>>>>>>> sock = rcu_dereference_protected(vq->private_data,
>>>>>>>> lockdep_is_held(&vq->mutex));
>>>>>>>> if (!sock)
>>>>>>>> - return;
>>>>>>>> - if (vq == n->vqs + VHOST_NET_VQ_TX) {
>>>>>>>> - n->tx_poll_state = VHOST_NET_POLL_STOPPED;
>>>>>>>> - tx_poll_start(n, sock);
>>>>>>>> - } else
>>>>>>>> - vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
>>>>>>>> + return 0;
>>>>>>>> +
>>>>>>>> + return vhost_poll_start(poll, sock->file);
>>>>>>>> }
>>>>>>>>
>>>>>>>> static struct socket *vhost_net_stop_vq(struct vhost_net *n,
>>>>>>>> @@ -833,7 +807,9 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
>>>>>>>> r = vhost_init_used(vq);
>>>>>>>> if (r)
>>>>>>>> goto err_used;
>>>>>>>> - vhost_net_enable_vq(n, vq);
>>>>>>>> + r = vhost_net_enable_vq(n, vq);
>>>>>>>> + if (r)
>>>>>>>> + goto err_used;
>>>>>>>>
>>>>>>>> oldubufs = vq->ubufs;
>>>>>>>> vq->ubufs = ubufs;
>>>>>>>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>>>>>>>> index 34389f7..5c7a466 100644
>>>>>>>> --- a/drivers/vhost/vhost.c
>>>>>>>> +++ b/drivers/vhost/vhost.c
>>>>>>>> @@ -77,26 +77,41 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
>>>>>>>> init_poll_funcptr(&poll->table, vhost_poll_func);
>>>>>>>> poll->mask = mask;
>>>>>>>> poll->dev = dev;
>>>>>>>> + poll->wqh = NULL;
>>>>>>>>
>>>>>>>> vhost_work_init(&poll->work, fn);
>>>>>>>> }
>>>>>>>>
>>>>>>>> +/* Stop polling a file. After this function returns, it becomes safe to drop the
>>>>>>>> + * file reference. You must also flush afterwards. */
>>>>>>>> +void vhost_poll_stop(struct vhost_poll *poll)
>>>>>>>> +{
>>>>>>>> + if (poll->wqh) {
>>>>>>>> + remove_wait_queue(poll->wqh, &poll->wait);
>>>>>>>> + poll->wqh = NULL;
>>>>>>>> + }
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> /* Start polling a file. We add ourselves to file's wait queue. The caller must
>>>>>>>> * keep a reference to a file until after vhost_poll_stop is called. */
>>>>>>>> -void vhost_poll_start(struct vhost_poll *poll, struct file *file)
>>>>>>>> +int vhost_poll_start(struct vhost_poll *poll, struct file *file)
>>>>>>>> {
>>>>>>>> unsigned long mask;
>>>>>>>> + int ret = 0;
>>>>>>>> +
>>>>>>>> + if (poll->wqh)
>>>>>>>> + return -EBUSY;
>>>>>>>>
>>>>>>> I think this should return success: we are already polling.
>>>>>>> Otherwise this would trigger a bug below I think.
>>>>>> Ok.
>>>>>>>> mask = file->f_op->poll(file, &poll->table);
>>>>>>>> if (mask)
>>>>>>>> vhost_poll_wakeup(&poll->wait, 0, 0, (void *)mask);
>>>>>>>> -}
>>>>>>>>
>>>>>>>> -/* Stop polling a file. After this function returns, it becomes safe to drop the
>>>>>>>> - * file reference. You must also flush afterwards. */
>>>>>>>> -void vhost_poll_stop(struct vhost_poll *poll)
>>>>>>>> -{
>>>>>>>> - remove_wait_queue(poll->wqh, &poll->wait);
>>>>>>>> + if (mask & POLLERR) {
>>>>>>>> + ret = -EINVAL;
>>>>>>>> + vhost_poll_stop(poll);
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + return ret;
>>>>>>>> }
>>>>>>>>
>>>>>>>> static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
>>>>>>>> @@ -792,7 +807,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
>>>>>>>> fput(filep);
>>>>>>>>
>>>>>>>> if (pollstart && vq->handle_kick)
>>>>>>>> - vhost_poll_start(&vq->poll, vq->kick);
>>>>>>>> + r = vhost_poll_start(&vq->poll, vq->kick);
>>>>>>>>
>>>>>>>> mutex_unlock(&vq->mutex);
>>>>>>>>
>>>>>>>> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
>>>>>>>> index 2639c58..17261e2 100644
>>>>>>>> --- a/drivers/vhost/vhost.h
>>>>>>>> +++ b/drivers/vhost/vhost.h
>>>>>>>> @@ -42,7 +42,7 @@ void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
>>>>>>>>
>>>>>>>> void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
>>>>>>>> unsigned long mask, struct vhost_dev *dev);
>>>>>>>> -void vhost_poll_start(struct vhost_poll *poll, struct file *file);
>>>>>>>> +int vhost_poll_start(struct vhost_poll *poll, struct file *file);
>>>>>>>> void vhost_poll_stop(struct vhost_poll *poll);
>>>>>>>> void vhost_poll_flush(struct vhost_poll *poll);
>>>>>>>> void vhost_poll_queue(struct vhost_poll *poll);
>>>>>>>> --
>>>>>>>> 1.7.1
>>>>>>> --
>>>>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>>>>> the body of a message to majordomo@vger.kernel.org
>>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [RFC PATCH v3] cgroup: net_cls: traffic counter based on classification control cgroup
From: Alexey Perevalov @ 2013-01-14 7:39 UTC (permalink / raw)
To: cgroups; +Cc: netdev, Glauber Costa, Daniel Wagner, Kyungmin Park, netdev
Hello
I would like to represent next version of patch I sent before
cgroup: "net_cls: traffic counter based on classification control cgroup"
The main idea is the same as was. It keeping counter in control groups,
but now uses atomic instead resource_counters.
I have a performance measurement for this patch. It was done by lmbench
on physical machine.
Results are not so representative for 20 tests and some numbers are real
weird.
Daniel Wagner wrote what he is doing something similar, but using
namespaces.
Proposed by me approach is used in upcoming Tizen release, but little
bit different version.
Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
include/net/cls_cgroup.h | 200 ++++++++++++++++++++++++++++++++++----
include/net/cls_counter_holder.h | 26 +++++
init/Kconfig | 25 +++++
kernel/cgroup.c | 2 +
kernel/res_counter.c | 4 +
net/core/dev.c | 6 ++
net/ipv4/tcp.c | 29 +++++-
net/ipv4/udp.c | 6 ++
net/sched/Kconfig | 11 ---
net/sched/Makefile | 1 +
net/sched/cls_cgroup.c | 193 +++++++++++++++++++++++++++++++++++-
net/sched/cls_counter_holder.c | 144 +++++++++++++++++++++++++++
12 files changed, 612 insertions(+), 35 deletions(-)
create mode 100644 include/net/cls_counter_holder.h
create mode 100644 net/sched/cls_counter_holder.c
diff --git a/include/net/cls_cgroup.h b/include/net/cls_cgroup.h
index 2581638..304786d 100644
--- a/include/net/cls_cgroup.h
+++ b/include/net/cls_cgroup.h
@@ -13,54 +13,197 @@
#ifndef _NET_CLS_CGROUP_H
#define _NET_CLS_CGROUP_H
+#include <linux/atomic.h>
#include <linux/cgroup.h>
#include <linux/hardirq.h>
#include <linux/rcupdate.h>
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+#include <linux/nsproxy.h>
+#include <linux/string.h>
+#include <linux/skbuff.h>
+#include <net/cls_counter_holder.h>
+#include <net/sock.h>
+
+/*TODO hide all it to separate file*/
+
+struct cls_iface_cntrs {
+ char *dev_name;
+ atomic64_t snd_counter;
+ atomic64_t rcv_counter;
+ struct list_head link;
+};
+
+#endif /*CONFIG_NET_CLS_COUNTER*/
+
+
#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
struct cgroup_cls_state
{
struct cgroup_subsys_state css;
u32 classid;
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+ struct cls_iface_cntrs iface_stats;
+#endif /*CONFIG_NET_CLS_COUNTER*/
};
extern void sock_update_classid(struct sock *sk, struct task_struct *task);
-#if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
-static inline u32 task_cls_classid(struct task_struct *p)
+#if IS_MODULE(CONFIG_NET_CLS_CGROUP)
+static inline struct cgroup_cls_state *get_cls_cgroup(struct
task_struct *p)
{
- u32 classid;
+ struct cgroup_subsys_state *css = task_subsys_state(p,
+ net_cls_subsys_id);
+ if (css)
+ return container_of(css,
+ struct cgroup_cls_state, css);
+ return NULL;
+}
+#elif IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
+static inline struct cgroup_cls_state *get_cls_cgroup(struct
task_struct *p)
+{
+ return container_of(task_subsys_state(p, net_cls_subsys_id),
+ struct cgroup_cls_state, css);
+}
+#endif
- if (in_interrupt())
- return 0;
- rcu_read_lock();
- classid = container_of(task_subsys_state(p, net_cls_subsys_id),
- struct cgroup_cls_state, css)->classid;
- rcu_read_unlock();
+#endif /*CONFIG_NET_CLS_CGROUP*/
- return classid;
+#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+static inline u32 skb_cls_classid(const struct sk_buff *skb)
+{
+ return (skb && skb->sk) ? skb->sk->sk_classid : 0;
+}
+
+static inline int get_ifindex_from_skb(const struct sk_buff *skb)
+{
+ int ifindex = 0;
+ if (skb)
+ ifindex = skb->skb_iif;
+ return ifindex;
+}
+
+static struct cls_iface_cntrs *find_cls_counter(
+ struct cgroup_cls_state *cls_cgroup,
+ const char *dev_name,
+ bool create)
+{
+ /*TODO Add lock*/
+ struct cls_iface_cntrs *entry = NULL;
+
+ if (!dev_name) {
+ pr_err("cls please provide valid dev name");
+ return NULL;
+ }
+
+ list_for_each_entry(entry, &cls_cgroup->iface_stats.link, link)
+ if (!strcmp(entry->dev_name, dev_name))
+ return entry;
+
+ if (!create)
+ return entry;
+
+ /*not found, insert*/
+ entry = kmalloc(sizeof(struct cls_iface_cntrs), GFP_ATOMIC);
+ entry->dev_name = kstrdup(dev_name, GFP_ATOMIC);
+ atomic64_set(&entry->rcv_counter, 0);
+ atomic64_set(&entry->snd_counter, 0);
+ list_add_tail(&entry->link, &cls_cgroup->iface_stats.link);
+ return entry;
}
-#elif IS_MODULE(CONFIG_NET_CLS_CGROUP)
+
+static void charge_net_cls_snd(struct cgroup_cls_state *cls_cgroup,
+ const u32 copied, const char *dev_name)
+{
+ struct cls_iface_cntrs *cnt = find_cls_counter(cls_cgroup,
+ dev_name, true);
+
+ if (!cnt)
+ return;
+
+ atomic64_add(copied, &cnt->snd_counter);
+}
+
+static char *get_dev_name(const int ifindex)
+{
+ struct net *net = NULL;
+ struct nsproxy *nsproxy = NULL;
+ struct net_device *net_dev = NULL;
+
+ nsproxy = task_nsproxy(current);
+ if (!nsproxy) {
+ pr_debug("cls cant find task_nsproxy");
+ return NULL;
+ }
+
+ net = get_net(nsproxy->net_ns);
+ if (!net) {
+ pr_debug("cls cant find net");
+ return NULL;
+ }
+ net_dev = dev_get_by_index(net, ifindex);
+
+ return net_dev ? net_dev->name : NULL;
+}
+
+static void charge_net_cls_rcv(struct cgroup_cls_state *cls_cgroup,
+ const u32 copied, const int ifindex)
+{
+ char *dev_name = get_dev_name(ifindex);
+ struct cls_iface_cntrs *cnt = find_cls_counter(cls_cgroup,
+ dev_name, true);
+
+ if (!cnt)
+ return;
+
+ atomic64_add(copied, &cnt->rcv_counter);
+}
+
+static inline void count_cls_rcv(struct task_struct *p, const u32 copied,
+ const int ifindex)
+{
+ struct cgroup_cls_state *cls_cgroup;
+
+ cls_cgroup = get_cls_cgroup(p);
+
+ if (cls_cgroup)
+ charge_net_cls_rcv(cls_cgroup, copied, ifindex);
+}
+
+static inline void count_cls_snd(u32 classid, const u32 copied,
+ const char *dev_name)
+{
+ struct cgroup_cls_state *cls_cgroup;
+
+ cls_cgroup = find_cls_cgroup_by_classid(classid);
+
+ if (cls_cgroup)
+ charge_net_cls_snd(cls_cgroup, copied, dev_name);
+}
+#endif /*CONFIG_NET_CLS_COUNTER*/
+
static inline u32 task_cls_classid(struct task_struct *p)
{
- struct cgroup_subsys_state *css;
- u32 classid = 0;
+ int classid = 0;
+ struct cgroup_cls_state *cls_cgroup = NULL;
if (in_interrupt())
return 0;
rcu_read_lock();
- css = task_subsys_state(p, net_cls_subsys_id);
- if (css)
- classid = container_of(css,
- struct cgroup_cls_state, css)->classid;
+
+ cls_cgroup = get_cls_cgroup(p);
+ if (cls_cgroup)
+ classid = cls_cgroup->classid;
+
rcu_read_unlock();
return classid;
}
-#endif
-#else /* !CGROUP_NET_CLS_CGROUP */
+
+#else /* !CONFIG_NET_CLS_CGROUP */
static inline void sock_update_classid(struct sock *sk, struct
task_struct *task)
{
}
@@ -69,5 +212,22 @@ static inline u32 task_cls_classid(struct
task_struct *p)
{
return 0;
}
-#endif /* CGROUP_NET_CLS_CGROUP */
+#endif /* CONFIG_NET_CLS_CGROUP */
+
+#if !IS_ENABLED(CONFIG_NET_CLS_CGROUP) ||
!IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+static inline void count_cls_rcv(struct task_struct *p,
+ const u32 copied, const int ifindex)
+{
+}
+
+static inline void count_cls_snd(u32 classid,
+ const u32 copied, const char *dev_name)
+{
+}
+
+static inline u32 skb_cls_classid(const struct sk_buff *skb)
+{
+ return 0;
+}
+#endif
#endif /* _NET_CLS_CGROUP_H */
diff --git a/include/net/cls_counter_holder.h
b/include/net/cls_counter_holder.h
new file mode 100644
index 0000000..a129baa
--- /dev/null
+++ b/include/net/cls_counter_holder.h
@@ -0,0 +1,26 @@
+/*
+ * cls_counter_holder.c Interface for holding references of the
+ * net cls cgroup instances.
+ *
+ * Authors: Alexey Perevalov, <a.perevalov@samsung.com>
+ *
+ * Changes:
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#ifndef _NET_CLS_COUNTER_HOLDER_H_
+#define _NET_CLS_COUNTER_HOLDER_H_
+
+#include <net/cls_cgroup.h>
+
+struct cgroup_cls_state;
+
+void insert_cls_cgroup_entry(struct cgroup_cls_state *obj);
+void delete_cls_cgroup_entry(const u32 classid);
+struct cgroup_cls_state *find_cls_cgroup_by_classid(const u32 classid);
+
+
+#endif /* _NET_CLS_COUNTER_HOLDER_H_ */
diff --git a/init/Kconfig b/init/Kconfig
index 6fdd6e3..2e6af85 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -841,6 +841,31 @@ config CGROUP_HUGETLB
control group is tracked in the third page lru pointer. This means
that we cannot use the controller with huge page less than 3 pages.
+menuconfig NET_CLS_CGROUP
+ tristate "Control Group Classifier"
+ select NET_CLS
+ depends on CGROUPS
+ ---help---
+ Say Y here if you want to classify packets based on the control
+ cgroup of their process.
+
+ To compile this code as a module, choose M here: the
+ module will be called cls_cgroup.
+
+if NET_CLS_CGROUP
+config NET_CLS_COUNTER
+ bool "Network traffic counter for network Control Group Classifier"
+ select NET_CLS
+ default n
+ depends on NET_CLS_CGROUP && RESOURCE_COUNTERS
+ ---help---
+ Say Y here if you want to count traffic associate with the control
+ cgroup.
+
+ To add functionality to cls_cgroup select y.
+
+endif #NET_CLS_CGROUP
+
config CGROUP_PERF
bool "Enable perf_event per-cpu per-container group (cgroup) monitoring"
depends on PERF_EVENTS && CGROUPS
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 13774b3..68a4a53 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2966,6 +2966,8 @@ int cgroup_rm_cftypes(struct cgroup_subsys *ss,
struct cftype *cfts)
cgroup_cfts_commit(ss, NULL, false);
return -ENOENT;
}
+EXPORT_SYMBOL_GPL(cgroup_rm_cftypes);
+
/**
* cgroup_task_count - count the number of tasks in a cgroup.
diff --git a/kernel/res_counter.c b/kernel/res_counter.c
index ad581aa..f5767af 100644
--- a/kernel/res_counter.c
+++ b/kernel/res_counter.c
@@ -13,6 +13,8 @@
#include <linux/res_counter.h>
#include <linux/uaccess.h>
#include <linux/mm.h>
+#include <linux/export.h>
+
void res_counter_init(struct res_counter *counter, struct res_counter
*parent)
{
@@ -21,6 +23,7 @@ void res_counter_init(struct res_counter *counter,
struct res_counter *parent)
counter->soft_limit = RESOURCE_MAX;
counter->parent = parent;
}
+EXPORT_SYMBOL(res_counter_init);
int res_counter_charge_locked(struct res_counter *counter, unsigned long
val,
bool force)
@@ -170,6 +173,7 @@ u64 res_counter_read_u64(struct res_counter
*counter, int member)
return *res_counter_member(counter, member);
}
#endif
+EXPORT_SYMBOL(res_counter_read_u64);
int res_counter_memparse_write_strategy(const char *buf,
unsigned long long *res)
diff --git a/net/core/dev.c b/net/core/dev.c
index b4978e2..61c9a61 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -135,6 +135,7 @@
#include <linux/net_tstamp.h>
#include <linux/static_key.h>
#include <net/flow_keys.h>
+#include <net/cls_cgroup.h>
#include "net-sysfs.h"
@@ -2570,6 +2571,11 @@ int dev_queue_xmit(struct sk_buff *skb)
*/
rcu_read_lock_bh();
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+ if (dev)
+ count_cls_snd(skb_cls_classid(skb), skb->len, dev->name);
+#endif
+
skb_update_prio(skb);
txq = netdev_pick_tx(dev, skb);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index eace049..ba54577 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -276,6 +276,7 @@
#include <net/ip.h>
#include <net/netdma.h>
#include <net/sock.h>
+#include <net/cls_cgroup.h>
#include <asm/uaccess.h>
#include <asm/ioctls.h>
@@ -1467,6 +1468,9 @@ int tcp_read_sock(struct sock *sk,
read_descriptor_t *desc,
u32 seq = tp->copied_seq;
u32 offset;
int copied = 0;
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+ int ifindex = 0;
+#endif
if (sk->sk_state == TCP_LISTEN)
return -ENOTCONN;
@@ -1509,6 +1513,9 @@ int tcp_read_sock(struct sock *sk,
read_descriptor_t *desc,
++seq;
break;
}
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+ ifindex = get_ifindex_from_skb(skb);
+#endif
sk_eat_skb(sk, skb, false);
if (!desc->count)
break;
@@ -1519,8 +1526,12 @@ int tcp_read_sock(struct sock *sk,
read_descriptor_t *desc,
tcp_rcv_space_adjust(sk);
/* Clean up data we have read: This will do ACK frames. */
- if (copied > 0)
+ if (copied > 0) {
tcp_cleanup_rbuf(sk, copied);
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+ count_cls_rcv(current, copied, ifindex);
+#endif
+ }
return copied;
}
EXPORT_SYMBOL(tcp_read_sock);
@@ -1548,6 +1559,9 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock
*sk, struct msghdr *msg,
bool copied_early = false;
struct sk_buff *skb;
u32 urg_hole = 0;
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+ int ifindex = 0;
+#endif
lock_sock(sk);
@@ -1872,6 +1886,9 @@ skip_copy:
if (tcp_hdr(skb)->fin)
goto found_fin_ok;
if (!(flags & MSG_PEEK)) {
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+ ifindex = get_ifindex_from_skb(skb);
+#endif
sk_eat_skb(sk, skb, copied_early);
copied_early = false;
}
@@ -1881,6 +1898,9 @@ skip_copy:
/* Process the FIN. */
++*seq;
if (!(flags & MSG_PEEK)) {
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+ ifindex = get_ifindex_from_skb(skb);
+#endif
sk_eat_skb(sk, skb, copied_early);
copied_early = false;
}
@@ -1923,6 +1943,11 @@ skip_copy:
/* Clean up data we have read: This will do ACK frames. */
tcp_cleanup_rbuf(sk, copied);
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+ if (copied > 0)
+ count_cls_rcv(current, copied, ifindex);
+#endif
+
release_sock(sk);
return copied;
@@ -1932,6 +1957,8 @@ out:
recv_urg:
err = tcp_recv_urg(sk, msg, len, flags);
+ if (err > 0)
+ count_cls_rcv(current, err, ifindex);
goto out;
recv_sndq:
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 79c8dbe..a143629 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -101,6 +101,7 @@
#include <linux/skbuff.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
+#include <net/cls_cgroup.h>
#include <net/net_namespace.h>
#include <net/icmp.h>
#include <net/route.h>
@@ -1254,6 +1255,11 @@ try_again:
if (flags & MSG_TRUNC)
err = ulen;
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+ if (ulen > 0)
+ count_cls_rcv(current, ulen, get_ifindex_from_skb(skb));
+#endif
+
out_free:
skb_free_datagram_locked(sk, skb);
out:
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 62fb51f..926dedf 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -418,17 +418,6 @@ config NET_CLS_FLOW
To compile this code as a module, choose M here: the
module will be called cls_flow.
-config NET_CLS_CGROUP
- tristate "Control Group Classifier"
- select NET_CLS
- depends on CGROUPS
- ---help---
- Say Y here if you want to classify packets based on the control
- cgroup of their process.
-
- To compile this code as a module, choose M here: the
- module will be called cls_cgroup.
-
config NET_EMATCH
bool "Extended Matches"
select NET_CLS
diff --git a/net/sched/Makefile b/net/sched/Makefile
index 978cbf0..95dbb12 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_NET_CLS_RSVP6) += cls_rsvp6.o
obj-$(CONFIG_NET_CLS_BASIC) += cls_basic.o
obj-$(CONFIG_NET_CLS_FLOW) += cls_flow.o
obj-$(CONFIG_NET_CLS_CGROUP) += cls_cgroup.o
+obj-$(CONFIG_NET_CLS_COUNTER) += cls_counter_holder.o
obj-$(CONFIG_NET_EMATCH) += ematch.o
obj-$(CONFIG_NET_EMATCH_CMP) += em_cmp.o
obj-$(CONFIG_NET_EMATCH_NBYTE) += em_nbyte.o
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 709b0fb..5683120 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -9,6 +9,7 @@
* Authors: Thomas Graf <tgraf@suug.ch>
*/
+#include <linux/atomic.h>
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/types.h>
@@ -22,6 +23,15 @@
#include <net/pkt_cls.h>
#include <net/sock.h>
#include <net/cls_cgroup.h>
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+#include <linux/rbtree.h>
+#include <net/cls_counter_holder.h>
+
+static struct notifier_block counter_notifier;
+static const char *rcv_label = "rcv:";
+static const char *snd_label = "snd:";
+
+#endif
static inline struct cgroup_cls_state *cgrp_cls_state(struct cgroup *cgrp)
{
@@ -46,11 +56,47 @@ static struct cgroup_subsys_state
*cgrp_create(struct cgroup *cgrp)
if (cgrp->parent)
cs->classid = cgrp_cls_state(cgrp->parent)->classid;
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+ atomic64_set(&cs->iface_stats.snd_counter, 0);
+ atomic64_set(&cs->iface_stats.rcv_counter, 0);
+ cs->iface_stats.dev_name = 0;
+ INIT_LIST_HEAD(&cs->iface_stats.link);
+#endif
+
return &cs->css;
}
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+static inline void cgrp_counter_destroy(struct cgroup_cls_state *cs)
+{
+ struct list_head *pos, *q;
+ delete_cls_cgroup_entry(cs->classid);
+
+ list_for_each_safe(pos, q, &cs->iface_stats.link) {
+ struct cls_iface_cntrs *tmp = list_entry(
+ pos, struct cls_iface_cntrs, link);
+ list_del(pos);
+ if (!tmp)
+ continue;
+
+ if (!tmp->dev_name)
+ kfree(tmp->dev_name);
+ kfree(tmp);
+ }
+
+}
+#endif
+
static void cgrp_destroy(struct cgroup *cgrp)
{
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+
+ struct cgroup_cls_state *cs = cgrp_cls_state(cgrp);
+
+ if (!cs)
+ return;
+ cgrp_counter_destroy(cs);
+#endif
kfree(cgrp_cls_state(cgrp));
}
@@ -81,9 +127,57 @@ static u64 read_classid(struct cgroup *cgrp, struct
cftype *cft)
return cgrp_cls_state(cgrp)->classid;
}
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+static const char *extract_dev_name(const char *cgroup_file_name)
+{
+ const char *dot = strchr(cgroup_file_name, '.');
+ const size_t len = dot ? dot - cgroup_file_name :
+ strlen(cgroup_file_name);
+
+ return kstrndup(cgroup_file_name, len, GFP_KERNEL);
+}
+
+static int read_stat(struct cgroup *cgrp, struct cftype *cft,
+ struct cgroup_map_cb *cb)
+{
+ struct cgroup_cls_state *cs = cgrp_cls_state(cgrp);
+ const char *dev_name = extract_dev_name(cft->name);
+ struct cls_iface_cntrs *res = find_cls_counter(cs, dev_name, false);
+
+ if (!res) {
+ pr_debug("cls cant read for cls");
+ return -EINVAL;
+ }
+
+ cb->fill(cb, rcv_label,
+ atomic64_read(&res->rcv_counter));
+ cb->fill(cb, snd_label,
+ atomic64_read(&res->snd_counter));
+
+ kfree(dev_name);
+ return 0;
+}
+#endif /* CONFIG_NET_CLS_COUNTER */
+
static int write_classid(struct cgroup *cgrp, struct cftype *cft, u64 value)
{
- cgrp_cls_state(cgrp)->classid = (u32) value;
+ struct cgroup_cls_state *cgrp_cls = cgrp_cls_state(cgrp);
+ u32 *classid = &cgrp_cls->classid;
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+ u32 oldclassid = *classid;
+
+ if (find_cls_cgroup_by_classid(value)) {
+ pr_err("cls: classid %llu already exists\n", value);
+ return -EINVAL;
+ }
+
+ insert_cls_cgroup_entry(cgrp_cls);
+
+ if (oldclassid)
+ delete_cls_cgroup_entry(oldclassid);
+#endif /* CONFIG_NET_CLS_COUNTER */
+ *classid = (u32) value;
+
return 0;
}
@@ -307,17 +401,107 @@ static struct tcf_proto_ops cls_cgroup_ops
__read_mostly = {
.owner = THIS_MODULE,
};
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+static inline int add_cft_file_for_device(struct net_device *dev)
+{
+ struct cftype *cft;
+ int ret = 0;
+
+ if (!dev)
+ return ret;
+
+ cft = kmalloc(sizeof(struct cftype) * 2,
+ GFP_KERNEL);
+ /* *2 and last 0 fill for terminator */
+ memset(cft, 0, sizeof(struct cftype) * 2);
+
+ snprintf(cft->name, MAX_CFTYPE_NAME,
+ "%s.usage_in_bytes", dev->name);
+ cft->read_map = read_stat;
+ cft->private = RES_USAGE;
+ ret = cgroup_add_cftypes(&net_cls_subsys, cft);
+ if (ret)
+ pr_err("cls error adding cft for counting at " \
+ "cls_cgroup %d\n", ret);
+ return ret;
+}
+
+static int device_state_cb(struct notifier_block *nb,
+ unsigned long state, void *arg)
+{
+ struct net_device *net = (struct net_device *)arg;
+ if (!nb || !net) {
+ pr_err("Not valid arguments for net_device notifier cb\n");
+ return 0;
+ }
+
+ if (state == NETDEV_REGISTER) {
+ pr_info("cls New device %s\n", net->name);
+ return add_cft_file_for_device(net);
+ }
+ return 0;
+}
+
+static inline int init_cgroup_counter(void)
+{
+ int ret = 0;
+ struct net_device *dev;
+ counter_notifier.notifier_call = device_state_cb;
+
+ ret = register_netdevice_notifier(&counter_notifier);
+ if (ret)
+ pr_err("cls Cant register nofier\n");
+
+ for_each_netdev(&init_net, dev) {
+ ret = add_cft_file_for_device(dev);
+ if (ret)
+ goto unregister_notifier;
+ }
+
+ return ret;
+unregister_notifier:
+
+ unregister_netdevice_notifier(&counter_notifier);
+ return ret;
+}
+
+static void release_cft(void)
+{
+ struct list_head *pos, *q;
+ list_for_each_safe(pos, q, &net_cls_subsys.cftsets) {
+ struct cftype_set *set =
+ list_entry(pos, struct cftype_set, node);
+ int ret = cgroup_rm_cftypes(&net_cls_subsys, set->cfts);
+ if (!ret) {
+ pr_err("cls cant remove cftypes\n");
+ break;
+ }
+
+ kfree(set->cfts);
+ }
+}
+#endif
+
static int __init init_cgroup_cls(void)
{
int ret;
-
ret = cgroup_load_subsys(&net_cls_subsys);
if (ret)
goto out;
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+ ret = init_cgroup_counter();
+ if (ret)
+ goto unload;
+#endif
+
ret = register_tcf_proto_ops(&cls_cgroup_ops);
if (ret)
- cgroup_unload_subsys(&net_cls_subsys);
+ goto unload;
+
+ return 0;
+unload:
+ cgroup_unload_subsys(&net_cls_subsys);
out:
return ret;
@@ -327,6 +511,9 @@ static void __exit exit_cgroup_cls(void)
{
unregister_tcf_proto_ops(&cls_cgroup_ops);
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+ release_cft();
+#endif
cgroup_unload_subsys(&net_cls_subsys);
}
diff --git a/net/sched/cls_counter_holder.c b/net/sched/cls_counter_holder.c
new file mode 100644
index 0000000..94ab285
--- /dev/null
+++ b/net/sched/cls_counter_holder.c
@@ -0,0 +1,144 @@
+/*
+ * net/sched/cls_counter_holder.c Interface for holding references of the
+ * net cls cgroup instances.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Authors: Alexey Perevalov <a.perevalov@samsung.com>
+ */
+
+
+#include <linux/export.h>
+#include <linux/module.h>
+#include <net/cls_cgroup.h>
+#include <net/cls_counter_holder.h>
+
+static struct rb_root classid_tree = RB_ROOT;
+static DEFINE_SPINLOCK(classid_tree_lock);
+
+struct entry {
+ struct cgroup_cls_state *data;
+ struct rb_node node;
+};
+
+static struct entry *find_entry(struct rb_root *root, const u32 classid)
+{
+ struct rb_node *node = root->rb_node;
+
+ while (node) {
+ struct entry *cls_entry = rb_entry(node, struct entry, node);
+ int result = 0;
+ if (!cls_entry || !cls_entry->data)
+ break;
+ result = cls_entry->data->classid - classid;
+
+ if (result < 0)
+ node = node->rb_left;
+ else if (result > 0)
+ node = node->rb_right;
+ else
+ return cls_entry;
+ }
+ return NULL;
+}
+
+void insert_cls_cgroup_entry(struct cgroup_cls_state *obj)
+{
+ struct rb_node **new;
+ struct rb_node *parent = NULL;
+ struct entry *new_entry;
+ unsigned long irq_flags = 0;
+
+ struct rb_root *root = &classid_tree;
+
+ spin_lock_irqsave(&classid_tree_lock, irq_flags);
+
+ new = &root->rb_node;
+
+ while (*new) {
+ struct entry *this = rb_entry(*new, struct entry, node);
+ /* Sort by classid, then by ifindex */
+ int result =
+ (this->data->classid - obj->classid);
+ parent = *new;
+ if (result < 0)
+ new = &((*new)->rb_left);
+ else if (result > 0)
+ new = &((*new)->rb_right);
+ else
+ goto unlock;
+ }
+
+ /* If we here, we need to insert new entry into tree */
+ new_entry = kmalloc(sizeof(struct entry), GFP_ATOMIC);
+ if (!new_entry)
+ goto unlock;
+
+ new_entry->data = obj;
+ /* Add new node and rebalance tree */
+ rb_link_node(&new_entry->node, parent, new);
+ rb_insert_color(&new_entry->node, root);
+
+unlock:
+ spin_unlock_irqrestore(&classid_tree_lock, irq_flags);
+}
+EXPORT_SYMBOL(insert_cls_cgroup_entry);
+
+void delete_cls_cgroup_entry(const u32 classid)
+{
+ unsigned long irq_flags = 0;
+ struct entry *data = NULL;
+ struct rb_root *root = &classid_tree;
+ spin_lock_irqsave(&classid_tree_lock, irq_flags);
+
+ data = find_entry(root, classid);
+
+ if (data) {
+ rb_erase(&data->node, root);
+ kfree(data);
+ }
+ spin_unlock_irqrestore(&classid_tree_lock, irq_flags);
+}
+EXPORT_SYMBOL(delete_cls_cgroup_entry);
+
+static void free_node(struct rb_node *root)
+{
+ struct entry *cur_entry = rb_entry(root, struct entry, node);
+ if (root->rb_left)
+ free_node(root->rb_left);
+ if (root->rb_right)
+ free_node(root->rb_right);
+ kfree(cur_entry);
+}
+
+static void free_classid_tree(void)
+{
+ unsigned long irq_flags = 0;
+
+ spin_lock_irqsave(&classid_tree_lock, irq_flags);
+
+ free_node(classid_tree.rb_node);
+
+ spin_unlock_irqrestore(&classid_tree_lock, irq_flags);
+}
+
+struct cgroup_cls_state *find_cls_cgroup_by_classid(const u32 classid)
+{
+ struct entry *cls_entry = find_entry(&classid_tree, classid);
+ if (cls_entry)
+ return cls_entry->data;
+
+ return NULL;
+}
+EXPORT_SYMBOL(find_cls_cgroup_by_classid);
+
+static void __exit exit_cls_counter_holder(void)
+{
+ free_classid_tree();
+}
+
+module_exit(exit_cls_counter_holder);
+MODULE_LICENSE("GPL");
--
1.7.9.5
The performance measurement results
*Local* Communication latencies in microseconds smaller is better
2p/0K ctxsw Pipe AF Unix UDP RPC/UDP TCP RPC/TCP TCP conn
Kernel with patch: 6.641 14.83 13.68 43.715 25.005 31.82 26.845 31.45
Kernel without patch: 6.5815 14.64 13.6895 48.6 28.74 35.745 26.875 31.6
Kernel with patch and created cgroups 6.456 14.54 13.88 50.145 33.735
32.26 26.92 31
*Local* Communication bandwidths in MB/s bigger is better
Pipe AF Unix TCP File reread Mmap reread Bcopy (libc) Bcopy (hand) Mem
read Mem write
Kernel with patch: 724.8 6952.3 3366.8 4409.565 7534.86 6629.05 3490.055
5594.15 6701.1
Kernel without patch: 669.55 6839.85 3347.65 4412.215 7506.46 6579.53
3453.145 5579.5 6243.45
Kernel with patch and create cgroups 718.75 6882.65 3353.2 4422.95
7521.225 6629.665 3472.87 5591.05 6409.9
--
Best regards,
Alexey Perevalov
^ permalink raw reply related
* Re: [RFC PATCH v3] cgroup: net_cls: traffic counter based on classification control cgroup
From: Daniel Wagner @ 2013-01-14 8:09 UTC (permalink / raw)
To: Alexey Perevalov; +Cc: cgroups, Glauber Costa, Kyungmin Park, netdev
In-Reply-To: <50F04502.9090902@samsung.com>
Hi Alexey,
On 11.01.2013 17:59, Alexey Perevalov wrote:
> I'm sorry for previous email with attachments.
It seems something went wrong with the patch, e.g. indention is wrong
and also I see '^M$' line endings. I assume you are sending your patches
through an exchange server which is likely not to work.
> I would like to represent next version of patch I sent before
> cgroup: "net_cls: traffic counter based on classification control cgroup"
>
> The main idea is the same as was. It keeping counter in control groups,
> but now uses atomic instead resource_counters.
+#if IS_ENABLED(CONFIG_NET_CLS_COUNTER)
+ if (copied > 0)
+ count_cls_rcv(current, copied, ifindex);
+#endif
+
release_sock(sk);
return copied;
Normally, distros will enable most config flags. Maybe you could use
a jump label to reduce the cost for the users which have
CONFIG_NET_CLS_COUNTER enabled and do not use it?
> I have a performance measurement for this patch. It was done by lmbench
> on physical machine.
> Results are not so representative for 20 tests and some numbers are real
> weird.
Could you explain in the commit message how your patch is designed? I
see you are using a RB tree. What's the purpose of it?
> Daniel Wagner wrote what he is doing something similar, but using
> namespaces.
I am trying a different approach on this problem using iptables. I am
playing around with a few patches which allow to install a iptables rule
which matches on the security context, e.g.
iptables -t mangle -A OUTPUT -m secmark --secctx \
unconfined_u:unconfined_r:foo_t:s0-s0:c0.c1023 -j MARK --set-mark 1
So far it looks promising, but as I me previous networking experience
is, that something will not work eventually.
> Proposed by me approach is used in upcoming Tizen release, but little
> bit different version.
Thanks,
Daniel
^ permalink raw reply
* Re: PMTU discovery is broken on kernel 3.7.1 for UDP sockets
From: Yurij M. Plotnikov @ 2013-01-14 8:26 UTC (permalink / raw)
To: Steffen Klassert; +Cc: Ben Hutchings, netdev, Alexandra N. Kossovsky
In-Reply-To: <20121221102253.GO18940@secunet.com>
On 12/21/12 14:22, Steffen Klassert wrote:
> On Thu, Dec 20, 2012 at 01:35:35PM +0100, Steffen Klassert wrote:
>
>> On Thu, Dec 20, 2012 at 03:22:13PM +0400, Yurij M. Plotnikov wrote:
>>
>>> On 12/20/12 11:34, Steffen Klassert wrote:
>>>
>>>> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
>>>> index 3c9d208..1049ce0 100644
>>>> --- a/net/ipv4/ip_sockglue.c
>>>> +++ b/net/ipv4/ip_sockglue.c
>>>> @@ -1198,7 +1198,7 @@ static int do_ip_getsockopt(struct sock *sk, int level, int optname,
>>>> {
>>>> struct dst_entry *dst;
>>>> val = 0;
>>>> - dst = sk_dst_get(sk);
>>>> + dst = sk_dst_check(sk, 0);
>>>> if (dst) {
>>>> val = dst_mtu(dst);
>>>> dst_release(dst);
>>>>
>>> With this patch kernel 3.7.1 works perfect. All described problems
>>> are fixed.
>>>
>> Thanks for testing!
>>
>> I'm not sure if we can't use this as a fix. I think with this patch it
>> could happen that we return -ENOTCONN instead of a pmtu value on a
>> connected socket. Perhaps it is better to update the cached dst_entry in
>> ipv4_sk_update_pmtu() when we receive the -EMSGSIZE. I'll do some
>> investigation.
>>
>>
> It turned out that updating the cached dst_entry in ipv4_sk_update_pmtu()
> is not trivial. We need to implement proper socket locking and we need
> socket release calback functions for all protocols that use
> ipv4_sk_update_pmtu(), similar to tcp.
>
> Today is my last office day for this year, so we probably have to defer
> a solution to the next year.
>
> Thanks.
>
Hi Steffen,
Could you, please, tell me is there any news about this bug?
Thanks in advance,
Yurij.
^ permalink raw reply
* Re: [PATCH] tcp: fix a panic on UP machines in reqsk_fastopen_remove
From: Jerry Chu @ 2013-01-14 8:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Ketan Kulkarni, Yuchung Cheng
In-Reply-To: <1358137311.8744.1590.camel@edumazet-glaptop>
On Sun, Jan 13, 2013 at 8:21 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> spin_is_locked() on a non !SMP build is kind of useless.
>
> BUG_ON(!spin_is_locked(xx)) is guaranteed to crash.
>
> Just remove this check in reqsk_fastopen_remove() as
> the callers do hold the socket lock.
>
> Reported-by: Ketan Kulkarni <ketkulka@gmail.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Jerry Chu <hkchu@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> Cc: Dave Taht <dave.taht@gmail.com>
> ---
> net/core/request_sock.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/net/core/request_sock.c b/net/core/request_sock.c
> index c31d9e8..4425148 100644
> --- a/net/core/request_sock.c
> +++ b/net/core/request_sock.c
> @@ -186,8 +186,6 @@ void reqsk_fastopen_remove(struct sock *sk, struct request_sock *req,
> struct fastopen_queue *fastopenq =
> inet_csk(lsk)->icsk_accept_queue.fastopenq;
>
> - BUG_ON(!spin_is_locked(&sk->sk_lock.slock) && !sock_owned_by_user(sk));
> -
> tcp_sk(sk)->fastopen_rsk = NULL;
> spin_lock_bh(&fastopenq->lock);
> fastopenq->qlen--;
>
>
Thanks, Eric. (I thought the assertion would be transparent to UP...)
Acked-by: H.K. Jerry Chu <hkchu@google.com>
^ permalink raw reply
* R: RE: per route MTU settings
From: pupilla @ 2013-01-14 8:51 UTC (permalink / raw)
To: luky-37, netdev
luky-37@hotmail.com wrote:
>I guess 10.81.104.1 has a 576B MTU route/interface towards
>10.81.105.109,
no, 10.81.104.1 is only a L3 switch.
>Running from both hosts pings with:
>-s 548
>-s 549
>-s 1422
>-s 1423
>
>and then analyzing the results will probably give us a better
>idea of what actually happens.
here is the results:
root@10.81.104.254:~# ip route add 10.81.105.121/32 via 10.81.104.1 mtu lock
1450
root@10.81.104.254:~# ip r s | grep 10.81 | grep eth2
10.81.104.0/24 dev eth2 proto kernel scope link src 10.81.104.254
10.81.104.0/21 via 10.81.104.1 dev eth2
10.81.105.121 via 10.81.104.1 dev eth2 mtu lock 1450
root@10.81.104.254:~# ip route flush cache
root@10.81.104.254:~# ping -M do 10.81.105.121 -c 4 -s 548
PING 10.81.105.121 (10.81.105.121) 548(576) bytes of data.
556 bytes from 10.81.105.121: icmp_req=1 ttl=127 time=0.476 ms
556 bytes from 10.81.105.121: icmp_req=2 ttl=127 time=0.448 ms
556 bytes from 10.81.105.121: icmp_req=3 ttl=127 time=0.515 ms
root@10.81.104.254:~# ip route flush cache
root@10.81.104.254:~# ping -M do 10.81.105.121 -c 4 -s 549
PING 10.81.105.121 (10.81.105.121) 549(577) bytes of data.
557 bytes from 10.81.105.121: icmp_req=1 ttl=127 time=0.386 ms
557 bytes from 10.81.105.121: icmp_req=2 ttl=127 time=0.525 ms
557 bytes from 10.81.105.121: icmp_req=3 ttl=127 time=0.447 ms
root@10.81.104.254:~# ip route flush cache
root@10.81.104.254:~# ping -M do 10.81.105.121 -c 4 -s 1422
PING 10.81.105.121 (10.81.105.121) 1422(1450) bytes of data.
1430 bytes from 10.81.105.121: icmp_req=1 ttl=127 time=0.724 ms
1430 bytes from 10.81.105.121: icmp_req=2 ttl=127 time=0.785 ms
1430 bytes from 10.81.105.121: icmp_req=3 ttl=127 time=1.95 ms
root@10.81.104.254:~# ip route flush cache
root@10.81.104.254:~# ping -M do 10.81.105.121 -c 4 -s 1423
PING 10.81.105.121 (10.81.105.121) 1423(1451) bytes of data.
>From 10.81.104.254 icmp_seq=1 Frag needed and DF set (mtu = 1450)
>From 10.81.104.254 icmp_seq=1 Frag needed and DF set (mtu = 1450)
>From 10.81.104.254 icmp_seq=1 Frag needed and DF set (mtu = 1450)
root@10.81.104.126:~# ip r s
default via 10.81.104.254 dev eth0 metric 1
10.81.104.0/24 dev eth0 proto kernel scope link src 10.81.104.126
127.0.0.0/8 dev lo scope link
root@10.81.104.126:~# ping -M do 10.81.105.121 -c 4 -s 548
PING 10.81.105.121 (10.81.105.121) 548(576) bytes of data.
556 bytes from 10.81.105.121: icmp_req=1 ttl=127 time=0.352 ms
556 bytes from 10.81.105.121: icmp_req=2 ttl=127 time=0.372 ms
556 bytes from 10.81.105.121: icmp_req=3 ttl=127 time=0.368 ms
root@10.81.104.126:~# ip route flush cache
root@10.81.104.126:~# ping -M do 10.81.105.121 -c 4 -s 549
PING 10.81.105.121 (10.81.105.121) 549(577) bytes of data.
>From 10.81.104.254 icmp_seq=1 Frag needed and DF set (mtu = 576)
I don't undestand why 10.81.104.254 here, is reporting
that the mtu is 576 instead of 1450
>From 10.81.104.126 icmp_seq=2 Frag needed and DF set (mtu = 576)
>From 10.81.104.126 icmp_seq=2 Frag needed and DF set (mtu = 576)
root@10.81.104.126:~# ip route flush cache
root@10.81.104.126:~# ping -M do 10.81.105.121 -c 4 -s 1422
PING 10.81.105.121 (10.81.105.121) 1422(1450) bytes of data.
>From 10.81.104.254 icmp_seq=1 Frag needed and DF set (mtu = 576)
>From 10.81.104.126 icmp_seq=2 Frag needed and DF set (mtu = 576)
>From 10.81.104.126 icmp_seq=2 Frag needed and DF set (mtu = 576)
root@10.81.104.126:~# ip route flush cache
root@10.81.104.126:~# ping -M do 10.81.105.121 -c 4 -s 1423
PING 10.81.105.121 (10.81.105.121) 1423(1451) bytes of data.
>From 10.81.104.254 icmp_seq=1 Frag needed and DF set (mtu = 576)
>From 10.81.104.126 icmp_seq=2 Frag needed and DF set (mtu = 576)
>From 10.81.104.126 icmp_seq=2 Frag needed and DF set (mtu = 576)
^ permalink raw reply
* [Patch net-next] netpoll: fix a rtnl lock assertion failure
From: Cong Wang @ 2013-01-14 8:55 UTC (permalink / raw)
To: netdev; +Cc: Jiri Pirko, David S. Miller, Cong Wang
From: Cong Wang <amwang@redhat.com>
This patch fixes the following warning:
[ 72.013864] RTNL: assertion failed at net/core/dev.c (4955)
[ 72.017758] Pid: 668, comm: netpoll-prep-v6 Not tainted 3.8.0-rc1+ #474
[ 72.019582] Call Trace:
[ 72.020295] [<ffffffff8176653d>] netdev_master_upper_dev_get+0x35/0x58
[ 72.022545] [<ffffffff81784edd>] netpoll_setup+0x61/0x340
[ 72.024846] [<ffffffff815d837e>] store_enabled+0x82/0xc3
[ 72.027466] [<ffffffff815d7e51>] netconsole_target_attr_store+0x35/0x37
[ 72.029348] [<ffffffff811c3479>] configfs_write_file+0xe2/0x10c
[ 72.030959] [<ffffffff8115d239>] vfs_write+0xaf/0xf6
[ 72.032359] [<ffffffff81978a05>] ? sysret_check+0x22/0x5d
[ 72.033824] [<ffffffff8115d453>] sys_write+0x5c/0x84
[ 72.035328] [<ffffffff819789d9>] system_call_fastpath+0x16/0x1b
by holding the rtnl_lock. And as we just want test if the device
has any upper device, so I think netdev_has_any_upper_dev() is enough.
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 9f05067..dd28cdd 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -1055,7 +1055,9 @@ int netpoll_setup(struct netpoll *np)
return -ENODEV;
}
- if (netdev_master_upper_dev_get(ndev)) {
+ rtnl_lock();
+ if (netdev_has_any_upper_dev(ndev)) {
+ rtnl_unlock();
np_err(np, "%s is a slave device, aborting\n", np->dev_name);
err = -EBUSY;
goto put;
@@ -1066,7 +1068,6 @@ int netpoll_setup(struct netpoll *np)
np_info(np, "device %s not up yet, forcing it\n", np->dev_name);
- rtnl_lock();
err = dev_open(ndev);
rtnl_unlock();
@@ -1094,7 +1095,8 @@ int netpoll_setup(struct netpoll *np)
np_notice(np, "carrier detect appears untrustworthy, waiting 4 seconds\n");
msleep(4000);
}
- }
+ } else
+ rtnl_unlock();
if (!np->local_ip.ip) {
if (!np->ipv6) {
^ permalink raw reply related
* Re: [Patch net-next] netpoll: fix a rtnl lock assertion failure
From: Jiri Pirko @ 2013-01-14 9:15 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, David S. Miller
In-Reply-To: <1358153708-9099-1-git-send-email-amwang@redhat.com>
Mon, Jan 14, 2013 at 09:55:08AM CET, amwang@redhat.com wrote:
>From: Cong Wang <amwang@redhat.com>
>
>This patch fixes the following warning:
>
>[ 72.013864] RTNL: assertion failed at net/core/dev.c (4955)
>[ 72.017758] Pid: 668, comm: netpoll-prep-v6 Not tainted 3.8.0-rc1+ #474
>[ 72.019582] Call Trace:
>[ 72.020295] [<ffffffff8176653d>] netdev_master_upper_dev_get+0x35/0x58
>[ 72.022545] [<ffffffff81784edd>] netpoll_setup+0x61/0x340
>[ 72.024846] [<ffffffff815d837e>] store_enabled+0x82/0xc3
>[ 72.027466] [<ffffffff815d7e51>] netconsole_target_attr_store+0x35/0x37
>[ 72.029348] [<ffffffff811c3479>] configfs_write_file+0xe2/0x10c
>[ 72.030959] [<ffffffff8115d239>] vfs_write+0xaf/0xf6
>[ 72.032359] [<ffffffff81978a05>] ? sysret_check+0x22/0x5d
>[ 72.033824] [<ffffffff8115d453>] sys_write+0x5c/0x84
>[ 72.035328] [<ffffffff819789d9>] system_call_fastpath+0x16/0x1b
>
>by holding the rtnl_lock. And as we just want test if the device
>has any upper device, so I think netdev_has_any_upper_dev() is enough.
>
>Cc: Jiri Pirko <jiri@resnulli.us>
>Cc: David S. Miller <davem@davemloft.net>
>Signed-off-by: Cong Wang <amwang@redhat.com>
>
>---
>diff --git a/net/core/netpoll.c b/net/core/netpoll.c
>index 9f05067..dd28cdd 100644
>--- a/net/core/netpoll.c
>+++ b/net/core/netpoll.c
>@@ -1055,7 +1055,9 @@ int netpoll_setup(struct netpoll *np)
> return -ENODEV;
> }
>
>- if (netdev_master_upper_dev_get(ndev)) {
>+ rtnl_lock();
>+ if (netdev_has_any_upper_dev(ndev)) {
This would prevent from using dev with for example vlan dev attached to
it. Is it desirable? I suppose not.
Also I think in this situation, netdev_master_upper_dev_get_rcu() would
be probably better to use. Not sure though.
>+ rtnl_unlock();
> np_err(np, "%s is a slave device, aborting\n", np->dev_name);
> err = -EBUSY;
> goto put;
>@@ -1066,7 +1068,6 @@ int netpoll_setup(struct netpoll *np)
>
> np_info(np, "device %s not up yet, forcing it\n", np->dev_name);
>
>- rtnl_lock();
> err = dev_open(ndev);
> rtnl_unlock();
>
>@@ -1094,7 +1095,8 @@ int netpoll_setup(struct netpoll *np)
> np_notice(np, "carrier detect appears untrustworthy, waiting 4 seconds\n");
> msleep(4000);
> }
>- }
>+ } else
>+ rtnl_unlock();
>
> if (!np->local_ip.ip) {
> if (!np->ipv6) {
^ permalink raw reply
* Re: [PATCH V3 2/2] vhost: handle polling errors
From: Michael S. Tsirkin @ 2013-01-14 9:17 UTC (permalink / raw)
To: Jason Wang; +Cc: kvm, eric.dumazet, netdev, linux-kernel, virtualization, davem
In-Reply-To: <50F3B5D6.4070405@redhat.com>
On Mon, Jan 14, 2013 at 03:37:58PM +0800, Jason Wang wrote:
> On 01/14/2013 02:57 PM, Michael S. Tsirkin wrote:
> > On Mon, Jan 14, 2013 at 10:59:02AM +0800, Jason Wang wrote:
> >> On 01/13/2013 07:10 PM, Michael S. Tsirkin wrote:
> >>> On Mon, Jan 07, 2013 at 11:04:32PM +0800, Jason Wang wrote:
> >>>> On 01/07/2013 10:55 PM, Michael S. Tsirkin wrote:
> >>>>> On Mon, Jan 07, 2013 at 12:38:17PM +0800, Jason Wang wrote:
> >>>>>> On 01/06/2013 09:22 PM, Michael S. Tsirkin wrote:
> >>>>>>> On Sun, Jan 06, 2013 at 03:18:38PM +0800, Jason Wang wrote:
> >>>>>>>> Polling errors were ignored by vhost/vhost_net, this may lead to crash when
> >>>>>>>> trying to remove vhost from waitqueue when after the polling is failed. Solve
> >>>>>>>> this problem by:
> >>>>>>>>
> >>>>>>>> - checking the poll->wqh before trying to remove from waitqueue
> >>>>>>>> - report an error when poll() returns a POLLERR in vhost_start_poll()
> >>>>>>>> - report an error when vhost_start_poll() fails in
> >>>>>>>> vhost_vring_ioctl()/vhost_net_set_backend() which is used to notify the
> >>>>>>>> failure to userspace.
> >>>>>>>> - report an error in the data path in vhost_net when meet polling errors.
> >>>>>>>>
> >>>>>>>> After those changes, we can safely drop the tx polling state in vhost_net since
> >>>>>>>> it was replaced by the checking of poll->wqh.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> >>>>>>>> ---
> >>>>>>>> drivers/vhost/net.c | 74 ++++++++++++++++--------------------------------
> >>>>>>>> drivers/vhost/vhost.c | 31 +++++++++++++++-----
> >>>>>>>> drivers/vhost/vhost.h | 2 +-
> >>>>>>>> 3 files changed, 49 insertions(+), 58 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> >>>>>>>> index d10ad6f..125c1e5 100644
> >>>>>>>> --- a/drivers/vhost/net.c
> >>>>>>>> +++ b/drivers/vhost/net.c
> >>>>>>>> @@ -64,20 +64,10 @@ enum {
> >>>>>>>> VHOST_NET_VQ_MAX = 2,
> >>>>>>>> };
> >>>>>>>>
> >>>>>>>> -enum vhost_net_poll_state {
> >>>>>>>> - VHOST_NET_POLL_DISABLED = 0,
> >>>>>>>> - VHOST_NET_POLL_STARTED = 1,
> >>>>>>>> - VHOST_NET_POLL_STOPPED = 2,
> >>>>>>>> -};
> >>>>>>>> -
> >>>>>>>> struct vhost_net {
> >>>>>>>> struct vhost_dev dev;
> >>>>>>>> struct vhost_virtqueue vqs[VHOST_NET_VQ_MAX];
> >>>>>>>> struct vhost_poll poll[VHOST_NET_VQ_MAX];
> >>>>>>>> - /* Tells us whether we are polling a socket for TX.
> >>>>>>>> - * We only do this when socket buffer fills up.
> >>>>>>>> - * Protected by tx vq lock. */
> >>>>>>>> - enum vhost_net_poll_state tx_poll_state;
> >>>>>>>> /* Number of TX recently submitted.
> >>>>>>>> * Protected by tx vq lock. */
> >>>>>>>> unsigned tx_packets;
> >>>>>>>> @@ -155,24 +145,6 @@ static void copy_iovec_hdr(const struct iovec *from, struct iovec *to,
> >>>>>>>> }
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> -/* Caller must have TX VQ lock */
> >>>>>>>> -static void tx_poll_stop(struct vhost_net *net)
> >>>>>>>> -{
> >>>>>>>> - if (likely(net->tx_poll_state != VHOST_NET_POLL_STARTED))
> >>>>>>>> - return;
> >>>>>>>> - vhost_poll_stop(net->poll + VHOST_NET_VQ_TX);
> >>>>>>>> - net->tx_poll_state = VHOST_NET_POLL_STOPPED;
> >>>>>>>> -}
> >>>>>>>> -
> >>>>>>>> -/* Caller must have TX VQ lock */
> >>>>>>>> -static void tx_poll_start(struct vhost_net *net, struct socket *sock)
> >>>>>>>> -{
> >>>>>>>> - if (unlikely(net->tx_poll_state != VHOST_NET_POLL_STOPPED))
> >>>>>>>> - return;
> >>>>>>>> - vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
> >>>>>>>> - net->tx_poll_state = VHOST_NET_POLL_STARTED;
> >>>>>>>> -}
> >>>>>>>> -
> >>>>>>>> /* In case of DMA done not in order in lower device driver for some reason.
> >>>>>>>> * upend_idx is used to track end of used idx, done_idx is used to track head
> >>>>>>>> * of used idx. Once lower device DMA done contiguously, we will signal KVM
> >>>>>>>> @@ -227,6 +199,7 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
> >>>>>>>> static void handle_tx(struct vhost_net *net)
> >>>>>>>> {
> >>>>>>>> struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX];
> >>>>>>>> + struct vhost_poll *poll = net->poll + VHOST_NET_VQ_TX;
> >>>>>>>> unsigned out, in, s;
> >>>>>>>> int head;
> >>>>>>>> struct msghdr msg = {
> >>>>>>>> @@ -252,7 +225,8 @@ static void handle_tx(struct vhost_net *net)
> >>>>>>>> wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> >>>>>>>> if (wmem >= sock->sk->sk_sndbuf) {
> >>>>>>>> mutex_lock(&vq->mutex);
> >>>>>>>> - tx_poll_start(net, sock);
> >>>>>>>> + if (vhost_poll_start(poll, sock->file))
> >>>>>>>> + vq_err(vq, "Fail to start TX polling\n");
> >>>>>>> s/Fail/Failed/
> >>>>>>>
> >>>>>>> A question though: how can this happen? Could you clarify please?
> >>>>>>> Maybe we can find a way to prevent this error?
> >>>>>> Two conditions I think this can happen:
> >>>>>>
> >>>>>> 1) a buggy userspace disable a queue through TUNSETQUEUE
> >>>>>> 2) the net device were gone
> >>>>>>
> >>>>>> For 1, looks like we can delay the disabling until the refcnt goes to
> >>>>>> zero. For 2 may needs more changes.
> >>>>> I'd expect keeping a socket reference would prevent both issues.
> >>>>> Doesn't it?
> >>>> Doesn't work for 2 I think, the socket didn't hold a refcnt of the
> >>>> device, so the device can go away at anytime. Although we can change
> >>>> this, but it's the behaviour before multiqueue support.
> >>> Hmm there's one scenario that does seem to
> >>> trigger this: queue can get disabled
> >>> and then poll fails.
> >>>
> >>> Is this the only issue?
> >> Another one I think we can trigger is:
> >>
> >> - start vhost thread
> >> - do ip link del link dev tap0 to delete the tap device
> >>
> >> In this case, the netdevice is unregistered but the file/socket still exist.
> > Yes but in this case poll_wait is called so apparently no issue
> > with existing code? We only have an issue if poll_wait is
> > not called right?
>
> Right.
> > Maybe the simplest fix is to invoke poll_wait before
> > checking if (!tun) ?
>
> True, but we this depends on the assumption that each kind of backend
> must make sure poll_wait() is called in .poll(). And we need to change
> macvtap_poll() as well. And we'd better also pass those polling errors
> to notify user about this wrong state.
For macvtap at least, I agree checking at the time where backend is set
makes sense, since the error can be reported to user.
Checking during vring operation can't properly be reported.
So my suggestion is to make polling of disable queue in tun
simply work, and check wqh when backend is set to handle
the macvtap case.
Makes sense?
> >
> >>>>>> Not sure it's worth to do this work,
> >>>>>> maybe a warning is enough just like other failure.
> >>>>> With other failures, you normally can correct the error then
> >>>>> kick to have it restart. This is soomething thagt would not
> >>>>> work here.
> >>>> If userspace is wrote correctly, (e.g passing a fd with correct state)
> >>>> it can also be corrected.
> >>>>>>>> mutex_unlock(&vq->mutex);
> >>>>>>>> return;
> >>>>>>>> }
> >>>>>>>> @@ -261,7 +235,7 @@ static void handle_tx(struct vhost_net *net)
> >>>>>>>> vhost_disable_notify(&net->dev, vq);
> >>>>>>>>
> >>>>>>>> if (wmem < sock->sk->sk_sndbuf / 2)
> >>>>>>>> - tx_poll_stop(net);
> >>>>>>>> + vhost_poll_stop(poll);
> >>>>>>>> hdr_size = vq->vhost_hlen;
> >>>>>>>> zcopy = vq->ubufs;
> >>>>>>>>
> >>>>>>>> @@ -283,8 +257,10 @@ static void handle_tx(struct vhost_net *net)
> >>>>>>>>
> >>>>>>>> wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> >>>>>>>> if (wmem >= sock->sk->sk_sndbuf * 3 / 4) {
> >>>>>>>> - tx_poll_start(net, sock);
> >>>>>>>> - set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >>>>>>>> + if (vhost_poll_start(poll, sock->file))
> >>>>>>>> + vq_err(vq, "Fail to start TX polling\n");
> >>>>>>>> + else
> >>>>>>>> + set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >>>>>>>> break;
> >>>>>>>> }
> >>>>>>>> /* If more outstanding DMAs, queue the work.
> >>>>>>>> @@ -294,8 +270,10 @@ static void handle_tx(struct vhost_net *net)
> >>>>>>>> (vq->upend_idx - vq->done_idx) :
> >>>>>>>> (vq->upend_idx + UIO_MAXIOV - vq->done_idx);
> >>>>>>>> if (unlikely(num_pends > VHOST_MAX_PEND)) {
> >>>>>>>> - tx_poll_start(net, sock);
> >>>>>>>> - set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >>>>>>>> + if (vhost_poll_start(poll, sock->file))
> >>>>>>>> + vq_err(vq, "Fail to start TX polling\n");
> >>>>>>>> + else
> >>>>>>>> + set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >>>>>>>> break;
> >>>>>>>> }
> >>>>>>>> if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> >>>>>>>> @@ -360,7 +338,8 @@ static void handle_tx(struct vhost_net *net)
> >>>>>>>> }
> >>>>>>>> vhost_discard_vq_desc(vq, 1);
> >>>>>>>> if (err == -EAGAIN || err == -ENOBUFS)
> >>>>>>>> - tx_poll_start(net, sock);
> >>>>>>>> + if (vhost_poll_start(poll, sock->file))
> >>>>>>>> + vq_err(vq, "Fail to start TX polling\n");
> >>>>>>>> break;
> >>>>>>>> }
> >>>>>>>> if (err != len)
> >>>>>>>> @@ -623,7 +602,6 @@ static int vhost_net_open(struct inode *inode, struct file *f)
> >>>>>>>>
> >>>>>>>> vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT, dev);
> >>>>>>>> vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN, dev);
> >>>>>>>> - n->tx_poll_state = VHOST_NET_POLL_DISABLED;
> >>>>>>>>
> >>>>>>>> f->private_data = n;
> >>>>>>>>
> >>>>>>>> @@ -633,29 +611,25 @@ static int vhost_net_open(struct inode *inode, struct file *f)
> >>>>>>>> static void vhost_net_disable_vq(struct vhost_net *n,
> >>>>>>>> struct vhost_virtqueue *vq)
> >>>>>>>> {
> >>>>>>>> + struct vhost_poll *poll = n->poll + (vq - n->vqs);
> >>>>>>>> +
> >>>>>>>> if (!vq->private_data)
> >>>>>>>> return;
> >>>>>>>> - if (vq == n->vqs + VHOST_NET_VQ_TX) {
> >>>>>>>> - tx_poll_stop(n);
> >>>>>>>> - n->tx_poll_state = VHOST_NET_POLL_DISABLED;
> >>>>>>>> - } else
> >>>>>>>> - vhost_poll_stop(n->poll + VHOST_NET_VQ_RX);
> >>>>>>>> + vhost_poll_stop(poll);
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> -static void vhost_net_enable_vq(struct vhost_net *n,
> >>>>>>>> +static int vhost_net_enable_vq(struct vhost_net *n,
> >>>>>>>> struct vhost_virtqueue *vq)
> >>>>>>>> {
> >>>>>>>> struct socket *sock;
> >>>>>>>> + struct vhost_poll *poll = n->poll + (vq - n->vqs);
> >>>>>>>>
> >>>>>>>> sock = rcu_dereference_protected(vq->private_data,
> >>>>>>>> lockdep_is_held(&vq->mutex));
> >>>>>>>> if (!sock)
> >>>>>>>> - return;
> >>>>>>>> - if (vq == n->vqs + VHOST_NET_VQ_TX) {
> >>>>>>>> - n->tx_poll_state = VHOST_NET_POLL_STOPPED;
> >>>>>>>> - tx_poll_start(n, sock);
> >>>>>>>> - } else
> >>>>>>>> - vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
> >>>>>>>> + return 0;
> >>>>>>>> +
> >>>>>>>> + return vhost_poll_start(poll, sock->file);
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> static struct socket *vhost_net_stop_vq(struct vhost_net *n,
> >>>>>>>> @@ -833,7 +807,9 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> >>>>>>>> r = vhost_init_used(vq);
> >>>>>>>> if (r)
> >>>>>>>> goto err_used;
> >>>>>>>> - vhost_net_enable_vq(n, vq);
> >>>>>>>> + r = vhost_net_enable_vq(n, vq);
> >>>>>>>> + if (r)
> >>>>>>>> + goto err_used;
> >>>>>>>>
> >>>>>>>> oldubufs = vq->ubufs;
> >>>>>>>> vq->ubufs = ubufs;
> >>>>>>>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >>>>>>>> index 34389f7..5c7a466 100644
> >>>>>>>> --- a/drivers/vhost/vhost.c
> >>>>>>>> +++ b/drivers/vhost/vhost.c
> >>>>>>>> @@ -77,26 +77,41 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> >>>>>>>> init_poll_funcptr(&poll->table, vhost_poll_func);
> >>>>>>>> poll->mask = mask;
> >>>>>>>> poll->dev = dev;
> >>>>>>>> + poll->wqh = NULL;
> >>>>>>>>
> >>>>>>>> vhost_work_init(&poll->work, fn);
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> +/* Stop polling a file. After this function returns, it becomes safe to drop the
> >>>>>>>> + * file reference. You must also flush afterwards. */
> >>>>>>>> +void vhost_poll_stop(struct vhost_poll *poll)
> >>>>>>>> +{
> >>>>>>>> + if (poll->wqh) {
> >>>>>>>> + remove_wait_queue(poll->wqh, &poll->wait);
> >>>>>>>> + poll->wqh = NULL;
> >>>>>>>> + }
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>> /* Start polling a file. We add ourselves to file's wait queue. The caller must
> >>>>>>>> * keep a reference to a file until after vhost_poll_stop is called. */
> >>>>>>>> -void vhost_poll_start(struct vhost_poll *poll, struct file *file)
> >>>>>>>> +int vhost_poll_start(struct vhost_poll *poll, struct file *file)
> >>>>>>>> {
> >>>>>>>> unsigned long mask;
> >>>>>>>> + int ret = 0;
> >>>>>>>> +
> >>>>>>>> + if (poll->wqh)
> >>>>>>>> + return -EBUSY;
> >>>>>>>>
> >>>>>>> I think this should return success: we are already polling.
> >>>>>>> Otherwise this would trigger a bug below I think.
> >>>>>> Ok.
> >>>>>>>> mask = file->f_op->poll(file, &poll->table);
> >>>>>>>> if (mask)
> >>>>>>>> vhost_poll_wakeup(&poll->wait, 0, 0, (void *)mask);
> >>>>>>>> -}
> >>>>>>>>
> >>>>>>>> -/* Stop polling a file. After this function returns, it becomes safe to drop the
> >>>>>>>> - * file reference. You must also flush afterwards. */
> >>>>>>>> -void vhost_poll_stop(struct vhost_poll *poll)
> >>>>>>>> -{
> >>>>>>>> - remove_wait_queue(poll->wqh, &poll->wait);
> >>>>>>>> + if (mask & POLLERR) {
> >>>>>>>> + ret = -EINVAL;
> >>>>>>>> + vhost_poll_stop(poll);
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + return ret;
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
> >>>>>>>> @@ -792,7 +807,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
> >>>>>>>> fput(filep);
> >>>>>>>>
> >>>>>>>> if (pollstart && vq->handle_kick)
> >>>>>>>> - vhost_poll_start(&vq->poll, vq->kick);
> >>>>>>>> + r = vhost_poll_start(&vq->poll, vq->kick);
> >>>>>>>>
> >>>>>>>> mutex_unlock(&vq->mutex);
> >>>>>>>>
> >>>>>>>> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> >>>>>>>> index 2639c58..17261e2 100644
> >>>>>>>> --- a/drivers/vhost/vhost.h
> >>>>>>>> +++ b/drivers/vhost/vhost.h
> >>>>>>>> @@ -42,7 +42,7 @@ void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
> >>>>>>>>
> >>>>>>>> void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> >>>>>>>> unsigned long mask, struct vhost_dev *dev);
> >>>>>>>> -void vhost_poll_start(struct vhost_poll *poll, struct file *file);
> >>>>>>>> +int vhost_poll_start(struct vhost_poll *poll, struct file *file);
> >>>>>>>> void vhost_poll_stop(struct vhost_poll *poll);
> >>>>>>>> void vhost_poll_flush(struct vhost_poll *poll);
> >>>>>>>> void vhost_poll_queue(struct vhost_poll *poll);
> >>>>>>>> --
> >>>>>>>> 1.7.1
> >>>>>>> --
> >>>>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
> >>>>>>> the body of a message to majordomo@vger.kernel.org
> >>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>>>> --
> >>>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
> >>>>> the body of a message to majordomo@vger.kernel.org
> >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe kvm" in
> >>> the body of a message to majordomo@vger.kernel.org
> >>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
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