* RE: [net-next 1/8] pci: Add flag indicating device has been assigned by KVM [and Xen]
From: Rose, Gregory V @ 2011-09-26 17:01 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Kirsher, Jeffrey T, davem@davemloft.net, Ian Campbell,
Jesse Barnes, netdev@vger.kernel.org, gospo@redhat.com,
linux-pci@vger.kernel.org
In-Reply-To: <20110926164238.GA4667@phenom.oracle.com>
> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Monday, September 26, 2011 9:43 AM
> To: Rose, Gregory V
> Cc: Kirsher, Jeffrey T; davem@davemloft.net; Ian Campbell; Jesse Barnes;
> netdev@vger.kernel.org; gospo@redhat.com; linux-pci@vger.kernel.org
> Subject: Re: [net-next 1/8] pci: Add flag indicating device has been
> assigned by KVM [and Xen]
>
> > > Any plans to support bnx4 and igb with this feature?
> >
> > I'm updating the igb driver to support this feature right now. I would
> hesitate to get into other drivers and muck with them because the changes
> to support this feature are not trivial and also involve policy decisions
> that I'd prefer to leave up to the folks who maintain those drivers. Plus
> I have no other vendor hardware available to test the changes.
> >
>
> Sounds fair. Did you CC them?
I'll grep through the drivers and find who calls pci_disable_sriov() and send the respective driver maintainers a notification letting them know of the patches.
>
> > > Any ETA?
> >
> > I should have the igb driver work done in the next week or so.
>
> Ok, please CC me on the code when you post it.
Will do.
- Greg
^ permalink raw reply
* [PATCH 1/2] net: check return value for dst_alloc
From: Madalin Bucur @ 2011-09-26 17:04 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, davem, timo.teras, Madalin Bucur
return value of dst_alloc must be checked before use
Signed-off-by: Madalin Bucur <madalin.bucur@freescale.com>
---
net/xfrm/xfrm_policy.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 94fdcc7..552df27 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1349,14 +1349,16 @@ static inline struct xfrm_dst *xfrm_alloc_dst(struct net *net, int family)
BUG();
}
xdst = dst_alloc(dst_ops, NULL, 0, 0, 0);
- memset(&xdst->u.rt6.rt6i_table, 0, sizeof(*xdst) - sizeof(struct dst_entry));
- xfrm_policy_put_afinfo(afinfo);
- if (likely(xdst))
+ if (likely(xdst)) {
+ memset(&xdst->u.rt6.rt6i_table, 0,
+ sizeof(*xdst) - sizeof(struct dst_entry));
xdst->flo.ops = &xfrm_bundle_fc_ops;
- else
+ } else
xdst = ERR_PTR(-ENOBUFS);
+ xfrm_policy_put_afinfo(afinfo);
+
return xdst;
}
--
1.7.0.1
^ permalink raw reply related
* [PATCH 2/2] net: check return value for dst_alloc
From: Madalin Bucur @ 2011-09-26 17:04 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, davem, timo.teras, Madalin Bucur
return value of dst_alloc must be checked before use
Signed-off-by: Madalin Bucur <madalin.bucur@freescale.com>
---
net/ipv6/route.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 1250f90..fb545ed 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -244,7 +244,9 @@ static inline struct rt6_info *ip6_dst_alloc(struct dst_ops *ops,
{
struct rt6_info *rt = dst_alloc(ops, dev, 0, 0, flags);
- memset(&rt->rt6i_table, 0, sizeof(*rt) - sizeof(struct dst_entry));
+ if (rt != NULL)
+ memset(&rt->rt6i_table, 0,
+ sizeof(*rt) - sizeof(struct dst_entry));
return rt;
}
--
1.7.0.1
^ permalink raw reply related
* [PATCH] net/flow: remove sleeping and deferral mechanism from flow_cache_flush
From: Madalin Bucur @ 2011-09-26 17:09 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, davem, timo.teras, Madalin Bucur
flow_cache_flush must not sleep as it can be called in atomic context;
removed the schedule_work as the deferred processing lead to the flow
cache gc never being actually run under heavy network load
Signed-off-by: Madalin Bucur <madalin.bucur@freescale.com>
---
net/core/flow.c | 21 +++++++++------------
1 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/net/core/flow.c b/net/core/flow.c
index 555a456..0950f97 100644
--- a/net/core/flow.c
+++ b/net/core/flow.c
@@ -14,7 +14,6 @@
#include <linux/init.h>
#include <linux/slab.h>
#include <linux/smp.h>
-#include <linux/completion.h>
#include <linux/percpu.h>
#include <linux/bitops.h>
#include <linux/notifier.h>
@@ -49,7 +48,6 @@ struct flow_cache_percpu {
struct flow_flush_info {
struct flow_cache *cache;
atomic_t cpuleft;
- struct completion completion;
};
struct flow_cache {
@@ -100,7 +98,7 @@ static void flow_entry_kill(struct flow_cache_entry *fle)
kmem_cache_free(flow_cachep, fle);
}
-static void flow_cache_gc_task(struct work_struct *work)
+static void flow_cache_gc_task(void)
{
struct list_head gc_list;
struct flow_cache_entry *fce, *n;
@@ -113,7 +111,6 @@ static void flow_cache_gc_task(struct work_struct *work)
list_for_each_entry_safe(fce, n, &gc_list, u.gc_list)
flow_entry_kill(fce);
}
-static DECLARE_WORK(flow_cache_gc_work, flow_cache_gc_task);
static void flow_cache_queue_garbage(struct flow_cache_percpu *fcp,
int deleted, struct list_head *gc_list)
@@ -123,7 +120,7 @@ static void flow_cache_queue_garbage(struct flow_cache_percpu *fcp,
spin_lock_bh(&flow_cache_gc_lock);
list_splice_tail(gc_list, &flow_cache_gc_list);
spin_unlock_bh(&flow_cache_gc_lock);
- schedule_work(&flow_cache_gc_work);
+ flow_cache_gc_task();
}
}
@@ -320,8 +317,7 @@ static void flow_cache_flush_tasklet(unsigned long data)
flow_cache_queue_garbage(fcp, deleted, &gc_list);
- if (atomic_dec_and_test(&info->cpuleft))
- complete(&info->completion);
+ atomic_dec(&info->cpuleft);
}
static void flow_cache_flush_per_cpu(void *data)
@@ -339,22 +335,23 @@ static void flow_cache_flush_per_cpu(void *data)
void flow_cache_flush(void)
{
struct flow_flush_info info;
- static DEFINE_MUTEX(flow_flush_sem);
+ static DEFINE_SPINLOCK(flow_flush_lock);
/* Don't want cpus going down or up during this. */
get_online_cpus();
- mutex_lock(&flow_flush_sem);
+ spin_lock_bh(&flow_flush_lock);
info.cache = &flow_cache_global;
atomic_set(&info.cpuleft, num_online_cpus());
- init_completion(&info.completion);
local_bh_disable();
smp_call_function(flow_cache_flush_per_cpu, &info, 0);
flow_cache_flush_tasklet((unsigned long)&info);
local_bh_enable();
- wait_for_completion(&info.completion);
- mutex_unlock(&flow_flush_sem);
+ while (atomic_read(&info.cpuleft) != 0)
+ cpu_relax();
+
+ spin_unlock_bh(&flow_flush_lock);
put_online_cpus();
}
--
1.7.0.1
^ permalink raw reply related
* [PATCH] ehea: Remove sleep at .ndo_get_stats
From: brenohl @ 2011-09-26 17:15 UTC (permalink / raw)
To: eric.dumazet; +Cc: davem, netdev, Breno Leitao
In-Reply-To: <1316704497.29549.10.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Currently ehea ndo_get_stats can sleep in two places, in a hcall
and in a GFP_KERNEL alloc, which is not correct.
This patch creates a delayed workqueue that grabs the information each 1
sec from the hardware, and place it into the device structure, so that,
.ndo_get_stats quickly returns the device structure statistics block.
Signed-off-by: Breno Leitao <brenohl@br.ibm.com>
---
drivers/net/ethernet/ibm/ehea/ehea.h | 1 +
drivers/net/ethernet/ibm/ehea/ehea_main.c | 26 ++++++++++++++++++++------
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ehea/ehea.h b/drivers/net/ethernet/ibm/ehea/ehea.h
index 7dd5e6a..0b8e6a9 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea.h
+++ b/drivers/net/ethernet/ibm/ehea/ehea.h
@@ -459,6 +459,7 @@ struct ehea_port {
struct ehea_mc_list *mc_list; /* Multicast MAC addresses */
struct ehea_eq *qp_eq;
struct work_struct reset_task;
+ struct delayed_work stats_work;
struct mutex port_lock;
char int_aff_name[EHEA_IRQ_NAME_SIZE];
int allmulti; /* Indicates IFF_ALLMULTI state */
diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
index 583bcd3..a10f6b3 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -330,17 +330,24 @@ out:
static struct net_device_stats *ehea_get_stats(struct net_device *dev)
{
struct ehea_port *port = netdev_priv(dev);
+
+ return &port->stats;
+}
+
+static void ehea_update_stats(struct work_struct *work)
+{
+ struct ehea_port *port =
+ container_of(work, struct ehea_port, stats_work.work);
+ struct net_device *dev = port->netdev;
struct net_device_stats *stats = &port->stats;
struct hcp_ehea_port_cb2 *cb2;
u64 hret, rx_packets, tx_packets, rx_bytes = 0, tx_bytes = 0;
int i;
- memset(stats, 0, sizeof(*stats));
-
cb2 = (void *)get_zeroed_page(GFP_KERNEL);
if (!cb2) {
- netdev_err(dev, "no mem for cb2\n");
- goto out;
+ netdev_err(dev, "No mem for cb2. The interface statistics was not be updated\n");
+ goto resched;
}
hret = ehea_h_query_ehea_port(port->adapter->handle,
@@ -375,8 +382,10 @@ static struct net_device_stats *ehea_get_stats(struct net_device *dev)
out_herr:
free_page((unsigned long)cb2);
-out:
- return stats;
+
+resched:
+ schedule_delayed_work(&port->stats_work, msecs_to_jiffies(1000));
+
}
static void ehea_refill_rq1(struct ehea_port_res *pr, int index, int nr_of_wqes)
@@ -2651,6 +2660,7 @@ static int ehea_open(struct net_device *dev)
}
mutex_unlock(&port->port_lock);
+ schedule_delayed_work(&port->stats_work, msecs_to_jiffies(1000));
return ret;
}
@@ -2690,6 +2700,7 @@ static int ehea_stop(struct net_device *dev)
set_bit(__EHEA_DISABLE_PORT_RESET, &port->flags);
cancel_work_sync(&port->reset_task);
+ cancel_delayed_work_sync(&port->stats_work);
mutex_lock(&port->port_lock);
netif_stop_queue(dev);
port_napi_disable(port);
@@ -3235,10 +3246,12 @@ struct ehea_port *ehea_setup_single_port(struct ehea_adapter *adapter,
dev->features |= NETIF_F_LRO;
INIT_WORK(&port->reset_task, ehea_reset_port);
+ INIT_DELAYED_WORK(&port->stats_work, ehea_update_stats);
init_waitqueue_head(&port->swqe_avail_wq);
init_waitqueue_head(&port->restart_wq);
+ memset(&port->stats, 0, sizeof(struct net_device_stats));
ret = register_netdev(dev);
if (ret) {
pr_err("register_netdev failed. ret=%d\n", ret);
@@ -3278,6 +3291,7 @@ static void ehea_shutdown_single_port(struct ehea_port *port)
struct ehea_adapter *adapter = port->adapter;
cancel_work_sync(&port->reset_task);
+ cancel_delayed_work_sync(&port->stats_work);
unregister_netdev(port->netdev);
ehea_unregister_port(port);
kfree(port->mc_list);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 1/4] IPVS: Add documentation for new sysctl entries
From: Randy Dunlap @ 2011-09-26 17:16 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netdev, netfilter-devel, netfilter, Wensong Zhang,
Julian Anastasov, Patrick McHardy
In-Reply-To: <1317000233-11169-2-git-send-email-horms@verge.net.au>
On 09/25/2011 06:23 PM, Simon Horman wrote:
> Add missing documentation for conntrack, snat_reroute and sync_version.
>
> Also fix up a typo, IPVS_DEBUG should be IP_VS_DEBUG.
>
> Signed-off-by: Simon Horman <horms@verge.net.au>
>
> conntrack
> ---
> Documentation/networking/ipvs-sysctl.txt | 53 +++++++++++++++++++++++++++++-
> 1 files changed, 52 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/networking/ipvs-sysctl.txt b/Documentation/networking/ipvs-sysctl.txt
> index 4ccdbca..5214339 100644
> --- a/Documentation/networking/ipvs-sysctl.txt
> +++ b/Documentation/networking/ipvs-sysctl.txt
> @@ -15,6 +15,23 @@ amemthresh - INTEGER
> enabled and the variable is automatically set to 2, otherwise
> the strategy is disabled and the variable is set to 1.
>
> +conntrack - BOOLEAN
> + 0 - disabled (default)
> + not 0 - enabled
> +
> + If set, maintain connection tracking entries for
> + connections handled by IPVS.
> +
> + This should be enabled if connections handled by IPVS are to be
> + also handled by stateful firewall rules. That is, iptables rules
> + that make use of connection tracking. It is a performance
> + optimisation to disable this setting otherwise.
> +
> + Connections handled by the IPVS FTP application module
> + will have connection tracking entries regardless of this setting.
> +
> + Only available when IPVS is compiled with the CONFIG_IP_VS_NFCT
seems to be missing something. Maybe
Only available when IPVS is compiled with CONFIG_IP_VS_NFCT enabled.
or
Only available when IPVS is compiled with the CONFIG_IP_VS_NFCT
symbol enabled.
> +
> cache_bypass - BOOLEAN
> 0 - disabled (default)
> not 0 - enabled
> @@ -39,7 +56,7 @@ debug_level - INTEGER
> 11 - IPVS packet handling (ip_vs_in/ip_vs_out)
> 12 or more - packet traversal
>
> - Only available when IPVS is compiled with the CONFIG_IPVS_DEBUG
> + Only available when IPVS is compiled with the CONFIG_IP_VS_DEBUG
with CONFIG_IP_VS_DEBUG enabled.
>
> Higher debugging levels include the messages for lower debugging
> levels, so setting debug level 2, includes level 0, 1 and 2
--
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply
* Re: [E1000-devel] intel 82599 multi-port performance
From: Ben Greear @ 2011-09-26 17:24 UTC (permalink / raw)
To: Chris Friesen
Cc: Alexander Duyck, e1000-devel@lists.sourceforge.net, netdev,
Brandeburg, Jesse, J.Hwan.Kim, frog1120
In-Reply-To: <4E80AAF8.1000806@genband.com>
On 09/26/2011 09:40 AM, Chris Friesen wrote:
> On 09/26/2011 10:04 AM, Alexander Duyck wrote:
>
>> It sounds like you are using a single card, would that be correct? If
>> you are running close to line rate on both ports this could be causing
>> you to saturate the PCIe x8 link.
>
> According to
> "http://communities.intel.com/community/wired/blog/2009/06/08/understanding-pci-express-bandwidth"
> 8x PCIe should have a bandwidth of 4GB/s. 2 10Gigabit ports is 2.5GB/s.
>
> The 82599 only goes up to 8x, so I'd expect that it should be sufficient
> to handle the full traffic.
>
> To any of the Intel guys out there...any ideas? Can an 82599 on an 8x
> bus handle max line rate with minimum size packets?
Rick Jones sent me an interesting link related to this. Short answer seems
to be 'yes', but it seems not for any normal off-the-shelf software stack.
> This: http://comments.gmane.org/gmane.linux.network/203602 should lead you to some slide.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* [PATCH 1/2] virtio-net: Verify page list size before fitting into skb
From: Sasha Levin @ 2011-09-26 17:41 UTC (permalink / raw)
To: linux-kernel
Cc: Sasha Levin, Rusty Russell, Michael S. Tsirkin, virtualization,
netdev, kvm
This patch verifies that the length of a buffer stored in a linked list
of pages is small enough to fit into a skb.
If the size is larger than a max size of a skb, it means that we shouldn't
go ahead building skbs anyway since we won't be able to send the buffer as
the user requested.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
Cc: kvm@vger.kernel.org
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
drivers/net/virtio_net.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0c7321c..64e0717 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -165,6 +165,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
unsigned int copy, hdr_len, offset;
char *p;
+ if (len > MAX_SKB_FRAGS * PAGE_SIZE)
+ return NULL;
+
p = page_address(page);
/* copy small packet so we can reuse these pages for small data */
--
1.7.6.1
^ permalink raw reply related
* [PATCH 2/2] virtio-net: Prevent NULL dereference
From: Sasha Levin @ 2011-09-26 17:41 UTC (permalink / raw)
To: linux-kernel
Cc: Sasha Levin, Rusty Russell, Michael S. Tsirkin, virtualization,
netdev, kvm
In-Reply-To: <1317058869-19276-1-git-send-email-levinsasha928@gmail.com>
This patch prevents a NULL dereference when the user has passed a length
longer than an actual buffer to virtio-net.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
Cc: kvm@vger.kernel.org
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
drivers/net/virtio_net.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 64e0717..8d32c1e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -198,7 +198,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
len -= copy;
offset += copy;
- while (len) {
+ while (len && page) {
set_skb_frag(skb, page, offset, &len);
page = (struct page *)page->private;
offset = 0;
--
1.7.6.1
^ permalink raw reply related
* Re: [E1000-devel] intel 82599 multi-port performance
From: Chris Friesen @ 2011-09-26 17:46 UTC (permalink / raw)
To: Ben Greear
Cc: Alexander Duyck, e1000-devel@lists.sourceforge.net, netdev,
Brandeburg, Jesse, J.Hwan.Kim, frog1120
In-Reply-To: <4E80B551.1040609@candelatech.com>
On 09/26/2011 11:24 AM, Ben Greear wrote:
> On 09/26/2011 09:40 AM, Chris Friesen wrote:
>> To any of the Intel guys out there...any ideas? Can an 82599 on an 8x
>> bus handle max line rate with minimum size packets?
>
> Rick Jones sent me an interesting link related to this. Short answer seems
> to be 'yes', but it seems not for any normal off-the-shelf software stack.
>
> > This: http://comments.gmane.org/gmane.linux.network/203602 should
> lead you to some slide.
Interesting. I wonder if Intel's DPDK will be the only way to handle
those sorts of packet rates.
Chris
--
Chris Friesen
Software Developer
GENBAND
chris.friesen@genband.com
www.genband.com
^ permalink raw reply
* Re: intel 82599 multi-port performance
From: Ben Greear @ 2011-09-26 17:57 UTC (permalink / raw)
To: Chris Friesen
Cc: e1000-devel@lists.sourceforge.net, netdev, Brandeburg, Jesse,
J.Hwan.Kim, frog1120
In-Reply-To: <4E80BA91.2040803@genband.com>
On 09/26/2011 10:46 AM, Chris Friesen wrote:
> On 09/26/2011 11:24 AM, Ben Greear wrote:
>> On 09/26/2011 09:40 AM, Chris Friesen wrote:
>
>>> To any of the Intel guys out there...any ideas? Can an 82599 on an 8x
>>> bus handle max line rate with minimum size packets?
>>
>> Rick Jones sent me an interesting link related to this. Short answer seems
>> to be 'yes', but it seems not for any normal off-the-shelf software stack.
>>
>> > This: http://comments.gmane.org/gmane.linux.network/203602 should
>> lead you to some slide.
>
> Interesting. I wonder if Intel's DPDK will be the only way to handle those sorts of packet rates.
Pktgen is probably still the fastest general code that I know of,
but we had some interesting results setting the TCP_MAXSEGS to
88, which creates around 150 byte packets, and let the NICs offload
chop up large TCP writes into small packets on the wire.
Using core-I7 980x CPU, and dual-port 82599, we could send
around 4Mpps and receive around 2Mpps between two machines.
We were using a single port on each NIC/machine for this test. Connection
was a bit asymmetric, seems one side would over-power the other...so if
we twiddled a bit, we could get around 3Mpps in each direction.
Our user-space app has some over-head as well, but we can send
at least 5Gbps full duplex on two ports using normal sized frames, so I
think the bottleneck in this case is the TCP offload in the NIC.
Still, pretty impressive for stateful TCP packets per second :)
Top-of-tree netperf just learned to do the TCP_MAXSEG trick as well,
so it might be fun to play with that. It probably has less overhead
than our tool, so might run even faster.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure contains a
definitive record of customers, application performance, security
threats, fraudulent activity and more. Splunk takes this data and makes
sense of it. Business sense. IT sense. Common sense.
http://p.sf.net/sfu/splunk-d2dcopy1
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH] ehea: Remove sleep at .ndo_get_stats
From: Eric Dumazet @ 2011-09-26 18:02 UTC (permalink / raw)
To: brenohl; +Cc: davem, netdev
In-Reply-To: <1317057325-7410-1-git-send-email-brenohl@br.ibm.com>
Le lundi 26 septembre 2011 à 14:15 -0300, brenohl@br.ibm.com a écrit :
> Currently ehea ndo_get_stats can sleep in two places, in a hcall
> and in a GFP_KERNEL alloc, which is not correct.
> This patch creates a delayed workqueue that grabs the information each 1
> sec from the hardware, and place it into the device structure, so that,
> .ndo_get_stats quickly returns the device structure statistics block.
>
> Signed-off-by: Breno Leitao <brenohl@br.ibm.com>
> ---
> drivers/net/ethernet/ibm/ehea/ehea.h | 1 +
> drivers/net/ethernet/ibm/ehea/ehea_main.c | 26 ++++++++++++++++++++------
> 2 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ehea/ehea.h b/drivers/net/ethernet/ibm/ehea/ehea.h
> index 7dd5e6a..0b8e6a9 100644
> --- a/drivers/net/ethernet/ibm/ehea/ehea.h
> +++ b/drivers/net/ethernet/ibm/ehea/ehea.h
> @@ -459,6 +459,7 @@ struct ehea_port {
> struct ehea_mc_list *mc_list; /* Multicast MAC addresses */
> struct ehea_eq *qp_eq;
> struct work_struct reset_task;
> + struct delayed_work stats_work;
> struct mutex port_lock;
> char int_aff_name[EHEA_IRQ_NAME_SIZE];
> int allmulti; /* Indicates IFF_ALLMULTI state */
> diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
> index 583bcd3..a10f6b3 100644
> --- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
> +++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
> @@ -330,17 +330,24 @@ out:
> static struct net_device_stats *ehea_get_stats(struct net_device *dev)
> {
> struct ehea_port *port = netdev_priv(dev);
> +
> + return &port->stats;
> +}
Hmm...
You should move in ehea_get_stats() all software-computed stats.
Only the hardware assisted stats should be gathered/changed from your
ehea_update_stats() helper.
This way, SNMP readers get accurate stats for all fields but the
stats->multicast and stats->rx_errors that are updated once per second.
^ permalink raw reply
* Re: intel 82599 multi-port performance
From: Rick Jones @ 2011-09-26 18:16 UTC (permalink / raw)
To: frog1120; +Cc: J.Hwan.Kim, netdev, netdev
In-Reply-To: <4E809D59.10103@gmail.com>
On 09/26/2011 08:42 AM, J.Hwan.Kim wrote:
> On 2011년 09월 26일 23:20, Chris Friesen wrote:
>> On 09/26/2011 04:26 AM, J.Hwan Kim wrote:
>>> Hi, everyone
>>>
>>> Now, I'm testing a network card including intel 82599.
>>> In our experiment, with the driver modified with ixgbe and multi-port
>>> enabled,
>>
>> What do you mean by "modified with ixgbe and multi-port enabled"? You
>> shouldn't need to do anything special to use both ports.
>>
>>> rx performance of each port with 10Gbps of 64bytes frame is
>>> a half than when only 1 port is used.
>>
>> Sounds like a cpu limitation. What is your cpu usage? How are your
>> interrupts routed? Are you using multiple rx queues?
>>
>
> Our server is XEON 2.4GHz with 8 cores.
> I'm using 4 RSS queues for each port and distributed it's interrupts to
> different cores respectively.
> I checked the CPU utilization with TOP, I guess ,it is not cpu imitation
> problem.
99 times out of 10, by default top will show the average CPU utilization
across all the "CPUs" of the system. So I will ask the pedantic question
- Did you check per-CPU utilization or just overall?
rick jones
^ permalink raw reply
* Re: tg3: BMC stops responding in 3.0
From: Matt Carlson @ 2011-09-26 18:22 UTC (permalink / raw)
To: Arkadiusz Mi??kiewicz
Cc: Matthew Carlson, Michael Chan, netdev@vger.kernel.org
In-Reply-To: <201109232145.50449.a.miskiewicz@gmail.com>
On Fri, Sep 23, 2011 at 12:45:50PM -0700, Arkadiusz Mi??kiewicz wrote:
>
> Hi,
>
> I was using 2.6.38.8 and recently tried to switch to 3.0.4 on Tyan S2891
> platform.
>
> This platform uses tg3:
> tg3 0000:0a:09.1: eth1: Tigon3 [partno(BCM95704) rev 2003] (PCIX:133MHz:64-
> bit) MAC address 00:e0:81:33:5e:af
> tg3 0000:0a:09.1: eth1: attached PHY is 5704 (10/100/1000Base-T Ethernet)
> (WireSpeed[1], EEE[0])
> tg3 0000:0a:09.1: eth1: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] TSOcap[1]
> tg3 0000:0a:09.1: eth1: dma_rwctrl[769f4000] dma_mask[64-bit]
>
> With 2.6.38.8 everything was working fine. With 3.0.4 there is a problem. As
> soon as tg3 module is loaded or eth0 configured (can't tell which one since
> the machine is 400km away from me and I have no way to play with it other than
> ipmi or ssh) BMC stops responding (so all ipmitool commands over LAN stop
> working). Normal tg3 activity is not affected - I can ssh-in without a problem
> etc but ipmi over lan doesn't work.
>
> From ssh console "ipmitool lan print" works, shows data but for example after
> "ipmitool mc reset cold" it doesn't recover - ipmitool returns "Invalid
> channel: 255". I have to reboot to 2.6.38.8 and then issue "ipmitool mc reset
> cold" to recover.
>
> Any idea which tg3 change could break this? Can't bisect this due remote
> access only.
>
> I was hoping that maybe 9e975cc291d80d5e4562d6bed15ec171e896d69b
> "tg3: Fix io failures after chip reset" will fix things for me but no - this
> doesn't help.
What version of the tg3 driver are you working with?
^ permalink raw reply
* Re: tg3: BMC stops responding in 3.0
From: Arkadiusz Miśkiewicz @ 2011-09-26 18:31 UTC (permalink / raw)
To: Matt Carlson; +Cc: Michael Chan, netdev@vger.kernel.org
In-Reply-To: <20110926182217.GB2440@mcarlson.broadcom.com>
On Monday 26 of September 2011, Matt Carlson wrote:
> On Fri, Sep 23, 2011 at 12:45:50PM -0700, Arkadiusz Mi??kiewicz wrote:
> > Hi,
> >
> > I was using 2.6.38.8 and recently tried to switch to 3.0.4 on Tyan S2891
> > platform.
> >
> > This platform uses tg3:
> > tg3 0000:0a:09.1: eth1: Tigon3 [partno(BCM95704) rev 2003]
> > (PCIX:133MHz:64- bit) MAC address 00:e0:81:33:5e:af
> > tg3 0000:0a:09.1: eth1: attached PHY is 5704 (10/100/1000Base-T Ethernet)
> > (WireSpeed[1], EEE[0])
> > tg3 0000:0a:09.1: eth1: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0]
> > TSOcap[1] tg3 0000:0a:09.1: eth1: dma_rwctrl[769f4000] dma_mask[64-bit]
> >
> > With 2.6.38.8 everything was working fine. With 3.0.4 there is a problem.
> > As soon as tg3 module is loaded or eth0 configured (can't tell which one
> > since the machine is 400km away from me and I have no way to play with
> > it other than ipmi or ssh) BMC stops responding (so all ipmitool
> > commands over LAN stop working). Normal tg3 activity is not affected - I
> > can ssh-in without a problem etc but ipmi over lan doesn't work.
> >
> > From ssh console "ipmitool lan print" works, shows data but for example
> > after "ipmitool mc reset cold" it doesn't recover - ipmitool returns
> > "Invalid channel: 255". I have to reboot to 2.6.38.8 and then issue
> > "ipmitool mc reset cold" to recover.
> >
> > Any idea which tg3 change could break this? Can't bisect this due remote
> > access only.
> >
> > I was hoping that maybe 9e975cc291d80d5e4562d6bed15ec171e896d69b
> > "tg3: Fix io failures after chip reset" will fix things for me but no -
> > this doesn't help.
>
> What version of the tg3 driver are you working with?
The one in 3.0.4 kernel. I think it's 3.119 (at least modinfo says so).
--
Arkadiusz Miśkiewicz PLD/Linux Team
arekm / maven.pl http://ftp.pld-linux.org/
^ permalink raw reply
* Re: [PATCH 1/2] virtio-net: Verify page list size before fitting into skb
From: Michael S. Tsirkin @ 2011-09-26 18:44 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
In-Reply-To: <1317058869-19276-1-git-send-email-levinsasha928@gmail.com>
On Mon, Sep 26, 2011 at 08:41:08PM +0300, Sasha Levin wrote:
> This patch verifies that the length of a buffer stored in a linked list
> of pages is small enough to fit into a skb.
>
> If the size is larger than a max size of a skb, it means that we shouldn't
> go ahead building skbs anyway since we won't be able to send the buffer as
> the user requested.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Cc: kvm@vger.kernel.org
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
Interesting. This is a theoretical issue, correct?
Not a crash you actually see.
This crash would mean device is giving us packets
that are way too large. Avoiding crashes even in the face of
a misbehaved device is a good idea, but should
we print a diagnostic to a system log?
Maybe rate-limited or print once to avoid filling
up the disk. Other places in driver print with pr_debug
I'm not sure that's right but better than nothing.
> ---
> drivers/net/virtio_net.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 0c7321c..64e0717 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -165,6 +165,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> unsigned int copy, hdr_len, offset;
> char *p;
>
> + if (len > MAX_SKB_FRAGS * PAGE_SIZE)
unlikely()?
Also, this seems too aggressive: at this point len includes the header
and the linear part. The right place for this
test is probably where we fill in the frags, just before
while (len)
The whole can only happen when mergeable buffers
are disabled, right?
> + return NULL;
> +
> p = page_address(page);
>
> /* copy small packet so we can reuse these pages for small data */
> --
> 1.7.6.1
^ permalink raw reply
* Re: [PATCH 2/2] virtio-net: Prevent NULL dereference
From: Michael S. Tsirkin @ 2011-09-26 18:47 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
In-Reply-To: <1317058869-19276-2-git-send-email-levinsasha928@gmail.com>
On Mon, Sep 26, 2011 at 08:41:09PM +0300, Sasha Levin wrote:
> This patch prevents a NULL dereference when the user has passed a length
> longer than an actual buffer to virtio-net.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Cc: kvm@vger.kernel.org
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
Hmm, another protection against a buggy device, right?
No problem with that, but let's discard the packet
and print a disgnostic, so that the user can discover
what happened.
> ---
> drivers/net/virtio_net.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 64e0717..8d32c1e 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -198,7 +198,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> len -= copy;
> offset += copy;
>
> - while (len) {
> + while (len && page) {
> set_skb_frag(skb, page, offset, &len);
> page = (struct page *)page->private;
> offset = 0;
> --
> 1.7.6.1
^ permalink raw reply
* Re: [PATCH] ehea: Remove sleep at .ndo_get_stats
From: Breno Leitao @ 2011-09-26 18:56 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, netdev
In-Reply-To: <1317060132.2796.4.camel@edumazet-laptop>
On 09/26/2011 03:02 PM, Eric Dumazet wrote:
> You should move in ehea_get_stats() all software-computed stats.
Agreed. I am going to resend the patch.
Thanks
^ permalink raw reply
* Re: [PATCH 01/15] add Documentation/namespaces/user_namespace.txt (v3)
From: Vasiliy Kulikov @ 2011-09-26 19:17 UTC (permalink / raw)
To: Serge Hallyn
Cc: akpm, linux-kernel, netdev, containers, dhowells, ebiederm,
rdunlap, Serge E. Hallyn, kernel-hardening
In-Reply-To: <1314993400-6910-4-git-send-email-serge@hallyn.com>
(cc'ed kernel-hardening)
Hi Serge,
I didn't deeply studied the patches yet (sorry!), but I have some
long-term question about the technique in general. I couldn't find
answers to the questions in the documentation.
First, the patches by design expose much kernel code to unprivileged
userspace processes. This code doesn't expect malformed data (e.g. VFS,
specific filesystems, block layer, char drivers, sysadmin part of LSMs,
etc. etc.). By relaxing permission rules you greatly increase attack
surface of the kernel from unprivileged users. Are you (or somebody
else) planning to audit this code?
Also, will it be possible to somehow restrict what specific kernel
facilities are accessible from users (IOW, what root emulation
limitations are in action)? It is userful from both points of sysadmin,
who might not want to allow users to do such things, and from the
security POV in sense of attack surface reduction.
The patches explicitly enable some features for users on white list
basis. It's possible to do it for simple cases, but what are you going
to do with multiplexing functions where there is a permission check
before the actual multiplexing? FS, networking drivers, etc. Are you
going to do the same thing as net_namespace does? - For each multiplexed
entity create bool ->ns_aware which is false by default for all
"untrusted"/not prepared protocols and is true for audited/prepared
protocols. Or probably you have something else in mind?
Thanks,
On Fri, Sep 02, 2011 at 19:56 +0000, Serge Hallyn wrote:
> From: "Serge E. Hallyn" <serge@hallyn.com>
>
> Quoting David Howells (dhowells@redhat.com):
> > Randy Dunlap <rdunlap@xenotime.net> wrote:
> >
> > > > +Any task in or resource belonging to the initial user namespace will, to this
> > > > +new task, appear to belong to UID and GID -1 - which is usually known as
> > >
> > > that extra hyphen is confusing. how about:
> > >
> > > to UID and GID -1, which is
> >
> > 'which are'.
> >
> > David
>
> This will hold some info about the design. Currently it contains
> future todos, issues and questions.
>
> Changelog:
> jul 26: incorporate feedback from David Howells.
> jul 29: incorporate feedback from Randy Dunlap.
>
> Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Randy Dunlap <rdunlap@xenotime.net>
> ---
> Documentation/namespaces/user_namespace.txt | 107 +++++++++++++++++++++++++++
> 1 files changed, 107 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/namespaces/user_namespace.txt
>
> diff --git a/Documentation/namespaces/user_namespace.txt b/Documentation/namespaces/user_namespace.txt
> new file mode 100644
> index 0000000..b0bc480
> --- /dev/null
> +++ b/Documentation/namespaces/user_namespace.txt
> @@ -0,0 +1,107 @@
> +Description
> +===========
> +
> +Traditionally, each task is owned by a user ID (UID) and belongs to one or more
> +groups (GID). Both are simple numeric IDs, though userspace usually translates
> +them to names. The user namespace allows tasks to have different views of the
> +UIDs and GIDs associated with tasks and other resources. (See 'UID mapping'
> +below for more.)
> +
> +The user namespace is a simple hierarchical one. The system starts with all
> +tasks belonging to the initial user namespace. A task creates a new user
> +namespace by passing the CLONE_NEWUSER flag to clone(2). This requires the
> +creating task to have the CAP_SETUID, CAP_SETGID, and CAP_CHOWN capabilities,
> +but it does not need to be running as root. The clone(2) call will result in a
> +new task which to itself appears to be running as UID and GID 0, but to its
> +creator seems to have the creator's credentials.
> +
> +To this new task, any resource belonging to the initial user namespace will
> +appear to belong to user and group 'nobody', which are UID and GID -1.
> +Permission to open such files will be granted according to world access
> +permissions. UID comparisons and group membership checks will return false,
> +and privilege will be denied.
> +
> +When a task belonging to (for example) userid 500 in the initial user namespace
> +creates a new user namespace, even though the new task will see itself as
> +belonging to UID 0, any task in the initial user namespace will see it as
> +belonging to UID 500. Therefore, UID 500 in the initial user namespace will be
> +able to kill the new task. Files created by the new user will (eventually) be
> +seen by tasks in its own user namespace as belonging to UID 0, but to tasks in
> +the initial user namespace as belonging to UID 500.
> +
> +Note that this userid mapping for the VFS is not yet implemented, though the
> +lkml and containers mailing list archives will show several previous
> +prototypes. In the end, those got hung up waiting on the concept of targeted
> +capabilities to be developed, which, thanks to the insight of Eric Biederman,
> +they finally did.
> +
> +Relationship between the User namespace and other namespaces
> +============================================================
> +
> +Other namespaces, such as UTS and network, are owned by a user namespace. When
> +such a namespace is created, it is assigned to the user namespace of the task
> +by which it was created. Therefore, attempts to exercise privilege to
> +resources in, for instance, a particular network namespace, can be properly
> +validated by checking whether the caller has the needed privilege (i.e.
> +CAP_NET_ADMIN) targeted to the user namespace which owns the network namespace.
> +This is done using the ns_capable() function.
> +
> +As an example, if a new task is cloned with a private user namespace but
> +no private network namespace, then the task's network namespace is owned
> +by the parent user namespace. The new task has no privilege to the
> +parent user namespace, so it will not be able to create or configure
> +network devices. If, instead, the task were cloned with both private
> +user and network namespaces, then the private network namespace is owned
> +by the private user namespace, and so root in the new user namespace
> +will have privilege targeted to the network namespace. It will be able
> +to create and configure network devices.
> +
> +UID Mapping
> +===========
> +The current plan (see 'flexible UID mapping' at
> +https://wiki.ubuntu.com/UserNamespace) is:
> +
> +The UID/GID stored on disk will be that in the init_user_ns. Most likely
> +UID/GID in other namespaces will be stored in xattrs. But Eric was advocating
> +(a few years ago) leaving the details up to filesystems while providing a lib/
> +stock implementation. See the thread around here:
> +http://www.mail-archive.com/devel@openvz.org/msg09331.html
> +
> +
> +Working notes
> +=============
> +Capability checks for actions related to syslog must be against the
> +init_user_ns until syslog is containerized.
> +
> +Same is true for reboot and power, control groups, devices, and time.
> +
> +Perf actions (kernel/event/core.c for instance) will always be constrained to
> +init_user_ns.
> +
> +Q:
> +Is accounting considered properly containerized with respect to pidns? (it
> +appears to be). If so, then we can change the capable() check in
> +kernel/acct.c to 'ns_capable(current_pid_ns()->user_ns, CAP_PACCT)'
> +
> +Q:
> +For things like nice and schedaffinity, we could allow root in a container to
> +control those, and leave only cgroups to constrain the container. I'm not sure
> +whether that is right, or whether it violates admin expectations.
> +
> +I deferred some of commoncap.c. I'm punting on xattr stuff as they take
> +dentries, not inodes.
> +
> +For drivers/tty/tty_io.c and drivers/tty/vt/vt.c, we'll want to (for some of
> +them) target the capability checks at the user_ns owning the tty. That will
> +have to wait until we get userns owning files straightened out.
> +
> +We need to figure out how to label devices. Should we just toss a user_ns
> +right into struct device?
> +
> +capable(CAP_MAC_ADMIN) checks are always to be against init_user_ns, unless
> +some day LSMs were to be containerized, near zero chance.
> +
> +inode_owner_or_capable() should probably take an optional ns and cap parameter.
> +If cap is 0, then CAP_FOWNER is checked. If ns is NULL, we derive the ns from
> +inode. But if ns is provided, then callers who need to derive
> +inode_userns(inode) anyway can save a few cycles.
> --
> 1.7.5.4
--
Vasiliy
^ permalink raw reply
* [PATCH] ehea: Remove sleep at .ndo_get_stats
From: brenohl @ 2011-09-26 19:21 UTC (permalink / raw)
To: eric.dumazet; +Cc: davem, netdev, Breno Leitao
In-Reply-To: <1317060132.2796.4.camel@edumazet-laptop>
Currently ehea ndo_get_stats can sleep in two places, in a hcall
and in a GFP_KERNEL alloc, which is not correct.
This patch creates a delayed workqueue that grabs the information each 1
sec from the hardware, and place it into the device structure, so that,
.ndo_get_stats quickly returns the device structure statistics block.
Signed-off-by: Breno Leitao <brenohl@br.ibm.com>
---
drivers/net/ethernet/ibm/ehea/ehea.h | 1 +
drivers/net/ethernet/ibm/ehea/ehea_main.c | 53 ++++++++++++++++------------
2 files changed, 31 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ehea/ehea.h b/drivers/net/ethernet/ibm/ehea/ehea.h
index 7dd5e6a..0b8e6a9 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea.h
+++ b/drivers/net/ethernet/ibm/ehea/ehea.h
@@ -459,6 +459,7 @@ struct ehea_port {
struct ehea_mc_list *mc_list; /* Multicast MAC addresses */
struct ehea_eq *qp_eq;
struct work_struct reset_task;
+ struct delayed_work stats_work;
struct mutex port_lock;
char int_aff_name[EHEA_IRQ_NAME_SIZE];
int allmulti; /* Indicates IFF_ALLMULTI state */
diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
index 583bcd3..976988d 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -331,16 +331,34 @@ 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, rx_bytes = 0, tx_bytes = 0;
int i;
- memset(stats, 0, sizeof(*stats));
+ for (i = 0; i < port->num_def_qps; i++) {
+ stats->rx_packets += port->port_res[i].rx_packets;
+ stats->rx_bytes += port->port_res[i].rx_bytes;
+ }
+
+ for (i = 0; i < port->num_def_qps + port->num_add_tx_qps; i++) {
+ stats->tx_packets += port->port_res[i].tx_packets;
+ stats->tx_bytes += port->port_res[i].tx_bytes;
+ }
+
+ return &port->stats;
+}
+
+static void ehea_update_stats(struct work_struct *work)
+{
+ struct ehea_port *port =
+ container_of(work, struct ehea_port, stats_work.work);
+ struct net_device *dev = port->netdev;
+ struct net_device_stats *stats = &port->stats;
+ struct hcp_ehea_port_cb2 *cb2;
+ u64 hret;
cb2 = (void *)get_zeroed_page(GFP_KERNEL);
if (!cb2) {
- netdev_err(dev, "no mem for cb2\n");
- goto out;
+ netdev_err(dev, "No mem for cb2. Some interface statistics were not updated\n");
+ goto resched;
}
hret = ehea_h_query_ehea_port(port->adapter->handle,
@@ -354,29 +372,13 @@ static struct net_device_stats *ehea_get_stats(struct net_device *dev)
if (netif_msg_hw(port))
ehea_dump(cb2, sizeof(*cb2), "net_device_stats");
- rx_packets = 0;
- 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++) {
- 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 = rx_bytes;
- stats->tx_bytes = tx_bytes;
- stats->rx_packets = rx_packets;
out_herr:
free_page((unsigned long)cb2);
-out:
- return stats;
+resched:
+ schedule_delayed_work(&port->stats_work, msecs_to_jiffies(1000));
}
static void ehea_refill_rq1(struct ehea_port_res *pr, int index, int nr_of_wqes)
@@ -2651,6 +2653,7 @@ static int ehea_open(struct net_device *dev)
}
mutex_unlock(&port->port_lock);
+ schedule_delayed_work(&port->stats_work, msecs_to_jiffies(1000));
return ret;
}
@@ -2690,6 +2693,7 @@ static int ehea_stop(struct net_device *dev)
set_bit(__EHEA_DISABLE_PORT_RESET, &port->flags);
cancel_work_sync(&port->reset_task);
+ cancel_delayed_work_sync(&port->stats_work);
mutex_lock(&port->port_lock);
netif_stop_queue(dev);
port_napi_disable(port);
@@ -3235,10 +3239,12 @@ struct ehea_port *ehea_setup_single_port(struct ehea_adapter *adapter,
dev->features |= NETIF_F_LRO;
INIT_WORK(&port->reset_task, ehea_reset_port);
+ INIT_DELAYED_WORK(&port->stats_work, ehea_update_stats);
init_waitqueue_head(&port->swqe_avail_wq);
init_waitqueue_head(&port->restart_wq);
+ memset(&port->stats, 0, sizeof(struct net_device_stats));
ret = register_netdev(dev);
if (ret) {
pr_err("register_netdev failed. ret=%d\n", ret);
@@ -3278,6 +3284,7 @@ static void ehea_shutdown_single_port(struct ehea_port *port)
struct ehea_adapter *adapter = port->adapter;
cancel_work_sync(&port->reset_task);
+ cancel_delayed_work_sync(&port->stats_work);
unregister_netdev(port->netdev);
ehea_unregister_port(port);
kfree(port->mc_list);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 1/2] virtio-net: Verify page list size before fitting into skb
From: Sasha Levin @ 2011-09-26 19:37 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
In-Reply-To: <20110926184445.GA22278@redhat.com>
On Mon, 2011-09-26 at 21:44 +0300, Michael S. Tsirkin wrote:
> On Mon, Sep 26, 2011 at 08:41:08PM +0300, Sasha Levin wrote:
> > This patch verifies that the length of a buffer stored in a linked list
> > of pages is small enough to fit into a skb.
> >
> > If the size is larger than a max size of a skb, it means that we shouldn't
> > go ahead building skbs anyway since we won't be able to send the buffer as
> > the user requested.
> >
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: netdev@vger.kernel.org
> > Cc: kvm@vger.kernel.org
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
>
> Interesting. This is a theoretical issue, correct?
> Not a crash you actually see.
Actually it was an actual crash caused when our virtio-net driver in kvm
tools did funny things and passed '(u32)-1' length as a buffer length to
the guest kernel.
> This crash would mean device is giving us packets
> that are way too large. Avoiding crashes even in the face of
> a misbehaved device is a good idea, but should
> we print a diagnostic to a system log?
> Maybe rate-limited or print once to avoid filling
> up the disk. Other places in driver print with pr_debug
> I'm not sure that's right but better than nothing.
Yup, I'll add some debug info.
> > ---
> > drivers/net/virtio_net.c | 3 +++
> > 1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 0c7321c..64e0717 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -165,6 +165,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> > unsigned int copy, hdr_len, offset;
> > char *p;
> >
> > + if (len > MAX_SKB_FRAGS * PAGE_SIZE)
>
> unlikely()?
>
> Also, this seems too aggressive: at this point len includes the header
> and the linear part. The right place for this
> test is probably where we fill in the frags, just before
> while (len)
>
> The whole can only happen when mergeable buffers
> are disabled, right?
>From what I understand it can happen whenever you're going to build a
skb longer than PAGE_SIZE.
--
Sasha.
^ permalink raw reply
* [PATCH] pch_gbe: remove unused variable
From: Jon Mason @ 2011-09-26 19:37 UTC (permalink / raw)
To: David S. Miller; +Cc: toshiharu-linux, netdev
netdev is unused in pch_gbe_setup_rctl. Remove this declaration to
avoid a compiler warning.
Signed-off-by: Jon Mason <jdmason@kudzu.us>
---
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 35a7c21..5dc61b4 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -698,7 +698,6 @@ static void pch_gbe_configure_tx(struct pch_gbe_adapter *adapter)
*/
static void pch_gbe_setup_rctl(struct pch_gbe_adapter *adapter)
{
- struct net_device *netdev = adapter->netdev;
struct pch_gbe_hw *hw = &adapter->hw;
u32 rx_mode, tcpip;
--
1.7.6.2
^ permalink raw reply related
* Re: [PATCH] ehea: Remove sleep at .ndo_get_stats
From: Eric Dumazet @ 2011-09-26 19:42 UTC (permalink / raw)
To: brenohl; +Cc: davem, netdev
In-Reply-To: <1317064918-1481-1-git-send-email-brenohl@br.ibm.com>
Le lundi 26 septembre 2011 à 16:21 -0300, brenohl@br.ibm.com a écrit :
> Currently ehea ndo_get_stats can sleep in two places, in a hcall
> and in a GFP_KERNEL alloc, which is not correct.
> This patch creates a delayed workqueue that grabs the information each 1
> sec from the hardware, and place it into the device structure, so that,
> .ndo_get_stats quickly returns the device structure statistics block.
>
> Signed-off-by: Breno Leitao <brenohl@br.ibm.com>
> ---
> drivers/net/ethernet/ibm/ehea/ehea.h | 1 +
> drivers/net/ethernet/ibm/ehea/ehea_main.c | 53 ++++++++++++++++------------
> 2 files changed, 31 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ehea/ehea.h b/drivers/net/ethernet/ibm/ehea/ehea.h
> index 7dd5e6a..0b8e6a9 100644
> --- a/drivers/net/ethernet/ibm/ehea/ehea.h
> +++ b/drivers/net/ethernet/ibm/ehea/ehea.h
> @@ -459,6 +459,7 @@ struct ehea_port {
> struct ehea_mc_list *mc_list; /* Multicast MAC addresses */
> struct ehea_eq *qp_eq;
> struct work_struct reset_task;
> + struct delayed_work stats_work;
> struct mutex port_lock;
> char int_aff_name[EHEA_IRQ_NAME_SIZE];
> int allmulti; /* Indicates IFF_ALLMULTI state */
> diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
> index 583bcd3..976988d 100644
> --- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
> +++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
> @@ -331,16 +331,34 @@ 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, rx_bytes = 0, tx_bytes = 0;
> int i;
>
> - memset(stats, 0, sizeof(*stats));
> + for (i = 0; i < port->num_def_qps; i++) {
> + stats->rx_packets += port->port_res[i].rx_packets;
> + stats->rx_bytes += port->port_res[i].rx_bytes;
> + }
> +
No.
You _really_ need the temporary variables, and set stats->field once
final result is known. Right now, you are adding data over and over.
Please test your patches :(
> + for (i = 0; i < port->num_def_qps + port->num_add_tx_qps; i++) {
> + stats->tx_packets += port->port_res[i].tx_packets;
> + stats->tx_bytes += port->port_res[i].tx_bytes;
> + }
> +
> + return &port->stats;
return stats;
> +}
> +
^ permalink raw reply
* Re: [PATCH 1/2] virtio-net: Verify page list size before fitting into skb
From: Pekka Enberg @ 2011-09-26 19:45 UTC (permalink / raw)
To: Sasha Levin
Cc: Michael S. Tsirkin, linux-kernel, Rusty Russell, virtualization,
netdev, kvm
In-Reply-To: <1317065842.20885.3.camel@lappy>
On Mon, Sep 26, 2011 at 10:37 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
>> Interesting. This is a theoretical issue, correct?
>> Not a crash you actually see.
>
> Actually it was an actual crash caused when our virtio-net driver in kvm
> tools did funny things and passed '(u32)-1' length as a buffer length to
> the guest kernel.
I'm not sure what Michael means with "theoretical issue" here. Can the guest
driver assume that the hypervisor doesn't attempt to do nasty things?
Pekka
^ permalink raw reply
* Re: Question about memory leak detector giving false positive report for net/core/flow.c
From: Eric Dumazet @ 2011-09-26 19:46 UTC (permalink / raw)
To: Catalin Marinas
Cc: Huajun Li, linux-mm@kvack.org, netdev, linux-kernel, Tejun Heo,
Christoph Lameter
In-Reply-To: <20110926165024.GA21617@e102109-lin.cambridge.arm.com>
Le lundi 26 septembre 2011 à 17:50 +0100, Catalin Marinas a écrit :
> On Mon, Sep 26, 2011 at 05:32:54PM +0100, Eric Dumazet wrote:
> > Le lundi 26 septembre 2011 à 23:17 +0800, Huajun Li a écrit :
> > > Memory leak detector gives following memory leak report, it seems the
> > > report is triggered by net/core/flow.c, but actually, it should be a
> > > false positive report.
> > > So, is there any idea from kmemleak side to fix/disable this false
> > > positive report like this?
> > > Yes, kmemleak_not_leak(...) could disable it, but is it suitable for this case ?
> ...
> > CC lkml and percpu maintainers (Tejun Heo & Christoph Lameter ) as well
> >
> > AFAIK this false positive only occurs if percpu data is allocated
> > outside of embedded pcu space.
> >
> > (grep pcpu_get_vm_areas /proc/vmallocinfo)
> >
> > I suspect this is a percpu/kmemleak cooperation problem (a missing
> > kmemleak_alloc() ?)
> >
> > I am pretty sure kmemleak_not_leak() is not the right answer to this
> > problem.
>
> kmemleak_not_leak() definitely not the write answer. The alloc_percpu()
> call does not have any kmemleak_alloc() callback, so it doesn't scan
> them.
>
> Huajun, could you please try the patch below:
>
> 8<--------------------------------
> kmemleak: Handle percpu memory allocation
>
> From: Catalin Marinas <catalin.marinas@arm.com>
>
> This patch adds kmemleak callbacks from the percpu allocator, reducing a
> number of false positives caused by kmemleak not scanning such memory
> blocks.
>
> Reported-by: Huajun Li <huajun.li.lee@gmail.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> mm/percpu.c | 11 +++++++++--
> 1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/mm/percpu.c b/mm/percpu.c
> index bf80e55..c47a90b 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -67,6 +67,7 @@
> #include <linux/spinlock.h>
> #include <linux/vmalloc.h>
> #include <linux/workqueue.h>
> +#include <linux/kmemleak.h>
>
> #include <asm/cacheflush.h>
> #include <asm/sections.h>
> @@ -833,7 +834,9 @@ fail_unlock_mutex:
> */
> void __percpu *__alloc_percpu(size_t size, size_t align)
> {
> - return pcpu_alloc(size, align, false);
> + void __percpu *ptr = pcpu_alloc(size, align, false);
> + kmemleak_alloc(ptr, size, 1, GFP_KERNEL);
> + return ptr;
> }
> EXPORT_SYMBOL_GPL(__alloc_percpu);
>
> @@ -855,7 +858,9 @@ EXPORT_SYMBOL_GPL(__alloc_percpu);
> */
> void __percpu *__alloc_reserved_percpu(size_t size, size_t align)
> {
> - return pcpu_alloc(size, align, true);
> + void __percpu *ptr = pcpu_alloc(size, align, true);
> + kmemleak_alloc(ptr, size, 1, GFP_KERNEL);
> + return ptr;
> }
>
> /**
> @@ -915,6 +920,8 @@ void free_percpu(void __percpu *ptr)
> if (!ptr)
> return;
>
> + kmemleak_free(ptr);
> +
> addr = __pcpu_ptr_to_addr(ptr);
>
> spin_lock_irqsave(&pcpu_lock, flags);
>
Hmm, you need to call kmemleak_alloc() for each chunk allocated per
possible cpu.
Here is the (untested) patch for the allocation phase, need the same at
freeing time
diff --git a/mm/percpu-km.c b/mm/percpu-km.c
index 89633fe..5061ac5 100644
--- a/mm/percpu-km.c
+++ b/mm/percpu-km.c
@@ -37,9 +37,12 @@ static int pcpu_populate_chunk(struct pcpu_chunk *chunk, int off, int size)
{
unsigned int cpu;
- for_each_possible_cpu(cpu)
- memset((void *)pcpu_chunk_addr(chunk, cpu, 0) + off, 0, size);
+ for_each_possible_cpu(cpu) {
+ void *chunk_addr = (void *)pcpu_chunk_addr(chunk, cpu, 0) + off;
+ kmemleak_alloc(chunk_addr, size, 1, GFP_KERNEL);
+ memset(chunk_addr, 0, size);
+ }
return 0;
}
diff --git a/mm/percpu-vm.c b/mm/percpu-vm.c
index ea53496..0d397cc 100644
--- a/mm/percpu-vm.c
+++ b/mm/percpu-vm.c
@@ -342,8 +342,12 @@ static int pcpu_populate_chunk(struct pcpu_chunk *chunk, int off, int size)
/* commit new bitmap */
bitmap_copy(chunk->populated, populated, pcpu_unit_pages);
clear:
- for_each_possible_cpu(cpu)
- memset((void *)pcpu_chunk_addr(chunk, cpu, 0) + off, 0, size);
+ for_each_possible_cpu(cpu) {
+ void *chunk_addr = (void *)pcpu_chunk_addr(chunk, cpu, 0) + off;
+
+ kmemleak_alloc(chunk_addr, size, 1, GFP_KERNEL);
+ memset(chunk_addr, 0, size);
+ }
return 0;
err_unmap:
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
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