* [PATCH] ehea: Fixing statistics
@ 2010-10-26 18:03 leitao
2010-10-26 18:48 ` Eric Dumazet
0 siblings, 1 reply; 8+ messages in thread
From: leitao @ 2010-10-26 18:03 UTC (permalink / raw)
To: davem; +Cc: netdev, Breno Leitao
Currently ehea stats are broken. The bytes counters are got from
the hardware, while the packets counters are got from the device
driver. Also, the device driver counters are resetted during the
the down process, and the hardware aren't, causing some weird
numbers.
This patch just consolidates the packets and bytes on the device
driver.
Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>
---
drivers/net/ehea/ehea.h | 2 ++
drivers/net/ehea/ehea_main.c | 29 ++++++++++++++++++++++++-----
2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ehea/ehea.h b/drivers/net/ehea/ehea.h
index 1321cb6..8e745e7 100644
--- a/drivers/net/ehea/ehea.h
+++ b/drivers/net/ehea/ehea.h
@@ -396,7 +396,9 @@ struct ehea_port_res {
int swqe_ll_count;
u32 swqe_id_counter;
u64 tx_packets;
+ u64 tx_bytes;
u64 rx_packets;
+ u64 rx_bytes;
u32 poll_counter;
struct net_lro_mgr lro_mgr;
struct net_lro_desc lro_desc[MAX_LRO_DESCRIPTORS];
diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index bb7d306..86ac44d 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -330,7 +330,7 @@ static struct net_device_stats *ehea_get_stats(struct net_device *dev)
struct ehea_port *port = netdev_priv(dev);
struct net_device_stats *stats = &port->stats;
struct hcp_ehea_port_cb2 *cb2;
- u64 hret, rx_packets, tx_packets;
+ u64 hret, rx_packets, tx_packets, rx_bytes = 0, tx_bytes = 0;
int i;
memset(stats, 0, sizeof(*stats));
@@ -353,18 +353,22 @@ static struct net_device_stats *ehea_get_stats(struct net_device *dev)
ehea_dump(cb2, sizeof(*cb2), "net_device_stats");
rx_packets = 0;
- for (i = 0; i < port->num_def_qps; i++)
+ for (i = 0; i < port->num_def_qps; i++) {
rx_packets += port->port_res[i].rx_packets;
+ rx_bytes += port->port_res[i].rx_bytes;
+ }
tx_packets = 0;
- for (i = 0; i < port->num_def_qps + port->num_add_tx_qps; i++)
+ for (i = 0; i < port->num_def_qps + port->num_add_tx_qps; i++) {
tx_packets += port->port_res[i].tx_packets;
+ tx_bytes += port->port_res[i].tx_bytes;
+ }
stats->tx_packets = tx_packets;
stats->multicast = cb2->rxmcp;
stats->rx_errors = cb2->rxuerr;
- stats->rx_bytes = cb2->rxo;
- stats->tx_bytes = cb2->txo;
+ stats->rx_bytes = rx_bytes;
+ stats->tx_bytes = tx_bytes;
stats->rx_packets = rx_packets;
out_herr:
@@ -703,6 +707,7 @@ static int ehea_proc_rwqes(struct net_device *dev,
int skb_arr_rq2_len = pr->rq2_skba.len;
int skb_arr_rq3_len = pr->rq3_skba.len;
int processed, processed_rq1, processed_rq2, processed_rq3;
+ u64 processed_bytes = 0;
int wqe_index, last_wqe_index, rq, port_reset;
processed = processed_rq1 = processed_rq2 = processed_rq3 = 0;
@@ -760,6 +765,7 @@ static int ehea_proc_rwqes(struct net_device *dev,
processed_rq3++;
}
+ processed_bytes += skb->len;
ehea_proc_skb(pr, cqe, skb);
} else {
pr->p_stats.poll_receive_errors++;
@@ -775,6 +781,7 @@ static int ehea_proc_rwqes(struct net_device *dev,
lro_flush_all(&pr->lro_mgr);
pr->rx_packets += processed;
+ pr->rx_bytes += processed_bytes;
ehea_refill_rq1(pr, last_wqe_index, processed_rq1);
ehea_refill_rq2(pr, processed_rq2);
@@ -1509,9 +1516,20 @@ static int ehea_init_port_res(struct ehea_port *port, struct ehea_port_res *pr,
enum ehea_eq_type eq_type = EHEA_EQ;
struct ehea_qp_init_attr *init_attr = NULL;
int ret = -EIO;
+ u64 tx_bytes, rx_bytes, tx_packets, rx_packets;
+
+ tx_bytes = pr->tx_bytes;
+ tx_packets = pr->tx_packets;
+ rx_bytes = pr->rx_bytes;
+ rx_packets = pr->rx_packets;
memset(pr, 0, sizeof(struct ehea_port_res));
+ pr->tx_bytes = rx_bytes;
+ pr->tx_packets = tx_packets;
+ pr->rx_bytes = rx_bytes;
+ pr->rx_packets = rx_packets;
+
pr->port = port;
spin_lock_init(&pr->xmit_lock);
spin_lock_init(&pr->netif_queue);
@@ -2296,6 +2314,7 @@ static int ehea_start_xmit(struct sk_buff *skb, struct net_device *dev)
ehea_post_swqe(pr->qp, swqe);
pr->tx_packets++;
+ pr->tx_bytes += skb->len;
if (unlikely(atomic_read(&pr->swqe_avail) <= 1)) {
spin_lock_irqsave(&pr->netif_queue, flags);
--
1.7.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] ehea: Fixing statistics
2010-10-26 18:03 [PATCH] ehea: Fixing statistics leitao
@ 2010-10-26 18:48 ` Eric Dumazet
2010-10-27 5:21 ` [PATCH] ehea: fix use after free Eric Dumazet
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2010-10-26 18:48 UTC (permalink / raw)
To: leitao; +Cc: davem, netdev
Le mardi 26 octobre 2010 à 14:03 -0400, leitao@linux.vnet.ibm.com a
écrit :
> @@ -2296,6 +2314,7 @@ static int ehea_start_xmit(struct sk_buff *skb, struct net_device *dev)
>
> ehea_post_swqe(pr->qp, swqe);
> pr->tx_packets++;
> + pr->tx_bytes += skb->len;
>
> if (unlikely(atomic_read(&pr->swqe_avail) <= 1)) {
> spin_lock_irqsave(&pr->netif_queue, flags);
This seems very suspicious to me. Lets take a look at this driver.
ehea_xmit3() frees the skb.
Yet, you happily use skb after it... kaboom...
Note: driver already uses skb after its freeing, before your patch.
if (vlan_tx_tag_present(skb)) {
swqe->tx_control |= EHEA_SWQE_VLAN_INSERT;
swqe->vlan_tag = vlan_tx_tag_get(skb);
}
How can it works ?
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] ehea: fix use after free
2010-10-26 18:48 ` Eric Dumazet
@ 2010-10-27 5:21 ` Eric Dumazet
2010-10-27 16:05 ` Breno Leitao
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2010-10-27 5:21 UTC (permalink / raw)
To: leitao; +Cc: davem, netdev
Le mardi 26 octobre 2010 à 20:48 +0200, Eric Dumazet a écrit :
> Note: driver already uses skb after its freeing, before your patch.
>
> if (vlan_tx_tag_present(skb)) {
> swqe->tx_control |= EHEA_SWQE_VLAN_INSERT;
> swqe->vlan_tag = vlan_tx_tag_get(skb);
> }
>
Could you please test following patch ?
Thanks
[PATCH] ehea: fix use after free
ehea_start_xmit() dereferences skb after its freeing in ehea_xmit3() to
get vlan tags.
Move the offending block before the potential ehea_xmit3() call.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
drivers/net/ehea/ehea_main.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index bb7d306..e59d386 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -2249,6 +2249,11 @@ static int ehea_start_xmit(struct sk_buff *skb, struct net_device *dev)
memset(swqe, 0, SWQE_HEADER_SIZE);
atomic_dec(&pr->swqe_avail);
+ if (vlan_tx_tag_present(skb)) {
+ swqe->tx_control |= EHEA_SWQE_VLAN_INSERT;
+ swqe->vlan_tag = vlan_tx_tag_get(skb);
+ }
+
if (skb->len <= SWQE3_MAX_IMM) {
u32 sig_iv = port->sig_comp_iv;
u32 swqe_num = pr->swqe_id_counter;
@@ -2279,11 +2284,6 @@ static int ehea_start_xmit(struct sk_buff *skb, struct net_device *dev)
}
pr->swqe_id_counter += 1;
- if (vlan_tx_tag_present(skb)) {
- swqe->tx_control |= EHEA_SWQE_VLAN_INSERT;
- swqe->vlan_tag = vlan_tx_tag_get(skb);
- }
-
if (netif_msg_tx_queued(port)) {
ehea_info("post swqe on QP %d", pr->qp->init_attr.qp_nr);
ehea_dump(swqe, 512, "swqe");
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] ehea: fix use after free
2010-10-27 5:21 ` [PATCH] ehea: fix use after free Eric Dumazet
@ 2010-10-27 16:05 ` Breno Leitao
2010-10-27 18:39 ` David Miller
0 siblings, 1 reply; 8+ messages in thread
From: Breno Leitao @ 2010-10-27 16:05 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, netdev
> [PATCH] ehea: fix use after free
>
> ehea_start_xmit() dereferences skb after its freeing in ehea_xmit3() to
> get vlan tags.
>
> Move the offending block before the potential ehea_xmit3() call.
>
> Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com>
Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ehea: fix use after free
2010-10-27 16:05 ` Breno Leitao
@ 2010-10-27 18:39 ` David Miller
2010-10-27 18:45 ` [PATCH v2] ehea: Fixing statistics Breno Leitao
0 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2010-10-27 18:39 UTC (permalink / raw)
To: leitao; +Cc: eric.dumazet, netdev
From: Breno Leitao <leitao@linux.vnet.ibm.com>
Date: Wed, 27 Oct 2010 14:05:35 -0200
>> [PATCH] ehea: fix use after free
>>
>> ehea_start_xmit() dereferences skb after its freeing in ehea_xmit3()
>> to
>> get vlan tags.
>>
>> Move the offending block before the potential ehea_xmit3() call.
>>
>> Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com>
> Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] ehea: Fixing statistics
2010-10-27 18:39 ` David Miller
@ 2010-10-27 18:45 ` Breno Leitao
2010-10-27 18:53 ` Eric Dumazet
0 siblings, 1 reply; 8+ messages in thread
From: Breno Leitao @ 2010-10-27 18:45 UTC (permalink / raw)
To: davem; +Cc: eric.dumazet, netdev, Breno Leitao
(Applied over Eric's "ehea: fix use after free" patch)
Currently ehea stats are broken. The bytes counters are got from
the hardware, while the packets counters are got from the device
driver. Also, the device driver counters are resetted during the
the down process, and the hardware aren't, causing some weird
numbers.
This patch just consolidates the packets and bytes on the device
driver.
Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>
---
drivers/net/ehea/ehea.h | 2 ++
drivers/net/ehea/ehea_main.c | 32 ++++++++++++++++++++++++++------
2 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ehea/ehea.h b/drivers/net/ehea/ehea.h
index 1321cb6..8e745e7 100644
--- a/drivers/net/ehea/ehea.h
+++ b/drivers/net/ehea/ehea.h
@@ -396,7 +396,9 @@ struct ehea_port_res {
int swqe_ll_count;
u32 swqe_id_counter;
u64 tx_packets;
+ u64 tx_bytes;
u64 rx_packets;
+ u64 rx_bytes;
u32 poll_counter;
struct net_lro_mgr lro_mgr;
struct net_lro_desc lro_desc[MAX_LRO_DESCRIPTORS];
diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index e59d386..182b2a7 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -330,7 +330,7 @@ static struct net_device_stats *ehea_get_stats(struct net_device *dev)
struct ehea_port *port = netdev_priv(dev);
struct net_device_stats *stats = &port->stats;
struct hcp_ehea_port_cb2 *cb2;
- u64 hret, rx_packets, tx_packets;
+ u64 hret, rx_packets, tx_packets, rx_bytes = 0, tx_bytes = 0;
int i;
memset(stats, 0, sizeof(*stats));
@@ -353,18 +353,22 @@ static struct net_device_stats *ehea_get_stats(struct net_device *dev)
ehea_dump(cb2, sizeof(*cb2), "net_device_stats");
rx_packets = 0;
- for (i = 0; i < port->num_def_qps; i++)
+ for (i = 0; i < port->num_def_qps; i++) {
rx_packets += port->port_res[i].rx_packets;
+ rx_bytes += port->port_res[i].rx_bytes;
+ }
tx_packets = 0;
- for (i = 0; i < port->num_def_qps + port->num_add_tx_qps; i++)
+ for (i = 0; i < port->num_def_qps + port->num_add_tx_qps; i++) {
tx_packets += port->port_res[i].tx_packets;
+ tx_bytes += port->port_res[i].tx_bytes;
+ }
stats->tx_packets = tx_packets;
stats->multicast = cb2->rxmcp;
stats->rx_errors = cb2->rxuerr;
- stats->rx_bytes = cb2->rxo;
- stats->tx_bytes = cb2->txo;
+ stats->rx_bytes = rx_bytes;
+ stats->tx_bytes = tx_bytes;
stats->rx_packets = rx_packets;
out_herr:
@@ -703,6 +707,7 @@ static int ehea_proc_rwqes(struct net_device *dev,
int skb_arr_rq2_len = pr->rq2_skba.len;
int skb_arr_rq3_len = pr->rq3_skba.len;
int processed, processed_rq1, processed_rq2, processed_rq3;
+ u64 processed_bytes = 0;
int wqe_index, last_wqe_index, rq, port_reset;
processed = processed_rq1 = processed_rq2 = processed_rq3 = 0;
@@ -760,6 +765,7 @@ static int ehea_proc_rwqes(struct net_device *dev,
processed_rq3++;
}
+ processed_bytes += skb->len;
ehea_proc_skb(pr, cqe, skb);
} else {
pr->p_stats.poll_receive_errors++;
@@ -775,6 +781,7 @@ static int ehea_proc_rwqes(struct net_device *dev,
lro_flush_all(&pr->lro_mgr);
pr->rx_packets += processed;
+ pr->rx_bytes += processed_bytes;
ehea_refill_rq1(pr, last_wqe_index, processed_rq1);
ehea_refill_rq2(pr, processed_rq2);
@@ -1509,9 +1516,20 @@ static int ehea_init_port_res(struct ehea_port *port, struct ehea_port_res *pr,
enum ehea_eq_type eq_type = EHEA_EQ;
struct ehea_qp_init_attr *init_attr = NULL;
int ret = -EIO;
+ u64 tx_bytes, rx_bytes, tx_packets, rx_packets;
+
+ tx_bytes = pr->tx_bytes;
+ tx_packets = pr->tx_packets;
+ rx_bytes = pr->rx_bytes;
+ rx_packets = pr->rx_packets;
memset(pr, 0, sizeof(struct ehea_port_res));
+ pr->tx_bytes = rx_bytes;
+ pr->tx_packets = tx_packets;
+ pr->rx_bytes = rx_bytes;
+ pr->rx_packets = rx_packets;
+
pr->port = port;
spin_lock_init(&pr->xmit_lock);
spin_lock_init(&pr->netif_queue);
@@ -2254,6 +2272,9 @@ static int ehea_start_xmit(struct sk_buff *skb, struct net_device *dev)
swqe->vlan_tag = vlan_tx_tag_get(skb);
}
+ pr->tx_packets++;
+ pr->tx_bytes += skb->len;
+
if (skb->len <= SWQE3_MAX_IMM) {
u32 sig_iv = port->sig_comp_iv;
u32 swqe_num = pr->swqe_id_counter;
@@ -2295,7 +2316,6 @@ static int ehea_start_xmit(struct sk_buff *skb, struct net_device *dev)
}
ehea_post_swqe(pr->qp, swqe);
- pr->tx_packets++;
if (unlikely(atomic_read(&pr->swqe_avail) <= 1)) {
spin_lock_irqsave(&pr->netif_queue, flags);
--
1.7.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ehea: Fixing statistics
2010-10-27 18:45 ` [PATCH v2] ehea: Fixing statistics Breno Leitao
@ 2010-10-27 18:53 ` Eric Dumazet
2010-10-27 21:21 ` David Miller
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2010-10-27 18:53 UTC (permalink / raw)
To: Breno Leitao; +Cc: davem, netdev
Le mercredi 27 octobre 2010 à 14:45 -0400, Breno Leitao a écrit :
> (Applied over Eric's "ehea: fix use after free" patch)
>
> Currently ehea stats are broken. The bytes counters are got from
> the hardware, while the packets counters are got from the device
> driver. Also, the device driver counters are resetted during the
> the down process, and the hardware aren't, causing some weird
> numbers.
>
> This patch just consolidates the packets and bytes on the device
> driver.
>
> Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>
Reviewed-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ehea: Fixing statistics
2010-10-27 18:53 ` Eric Dumazet
@ 2010-10-27 21:21 ` David Miller
0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2010-10-27 21:21 UTC (permalink / raw)
To: eric.dumazet; +Cc: leitao, netdev
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 27 Oct 2010 20:53:30 +0200
> Le mercredi 27 octobre 2010 à 14:45 -0400, Breno Leitao a écrit :
>> (Applied over Eric's "ehea: fix use after free" patch)
>>
>> Currently ehea stats are broken. The bytes counters are got from
>> the hardware, while the packets counters are got from the device
>> driver. Also, the device driver counters are resetted during the
>> the down process, and the hardware aren't, causing some weird
>> numbers.
>>
>> This patch just consolidates the packets and bytes on the device
>> driver.
>>
>> Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>
>
> Reviewed-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-10-27 21:21 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-26 18:03 [PATCH] ehea: Fixing statistics leitao
2010-10-26 18:48 ` Eric Dumazet
2010-10-27 5:21 ` [PATCH] ehea: fix use after free Eric Dumazet
2010-10-27 16:05 ` Breno Leitao
2010-10-27 18:39 ` David Miller
2010-10-27 18:45 ` [PATCH v2] ehea: Fixing statistics Breno Leitao
2010-10-27 18:53 ` Eric Dumazet
2010-10-27 21:21 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).