* Re: [PATCH v3 6/7] tcp buffer limitation: per-cgroup limit
From: KAMEZAWA Hiroyuki @ 2011-09-26 11:02 UTC (permalink / raw)
To: Glauber Costa
Cc: Greg Thelen, linux-kernel, paul, lizf, ebiederm, davem, netdev,
linux-mm, kirill
In-Reply-To: <4E7DDB82.3030802@parallels.com>
On Sat, 24 Sep 2011 10:30:42 -0300
Glauber Costa <glommer@parallels.com> wrote:
> On 09/22/2011 03:01 AM, Greg Thelen wrote:
> > On Sun, Sep 18, 2011 at 5:56 PM, Glauber Costa<glommer@parallels.com> wrote:
> >> +static inline bool mem_cgroup_is_root(struct mem_cgroup *mem)
> >> +{
> >> + return (mem == root_mem_cgroup);
> >> +}
> >> +
> >
> > Why are you adding a copy of mem_cgroup_is_root(). I see one already
> > in v3.0. Was it deleted in a previous patch?
>
> Already answered by another good samaritan.
>
> >> +static int tcp_write_maxmem(struct cgroup *cgrp, struct cftype *cft, u64 val)
> >> +{
> >> + struct mem_cgroup *sg = mem_cgroup_from_cont(cgrp);
> >> + struct mem_cgroup *parent = parent_mem_cgroup(sg);
> >> + struct net *net = current->nsproxy->net_ns;
> >> + int i;
> >> +
> >> + if (!cgroup_lock_live_group(cgrp))
> >> + return -ENODEV;
> >
> > Why is cgroup_lock_live_cgroup() needed here? Does it protect updates
> > to sg->tcp_prot_mem[*]?
> >
> >> +static u64 tcp_read_maxmem(struct cgroup *cgrp, struct cftype *cft)
> >> +{
> >> + struct mem_cgroup *sg = mem_cgroup_from_cont(cgrp);
> >> + u64 ret;
> >> +
> >> + if (!cgroup_lock_live_group(cgrp))
> >> + return -ENODEV;
> >
> > Why is cgroup_lock_live_cgroup() needed here? Does it protect updates
> > to sg->tcp_max_memory?
>
> No, that is not my understanding. My understanding is this lock is
> needed to protect against the cgroup just disappearing under our nose.
>
Hm. reference count of dentry for cgroup isn't enough ?
Thanks,
-Kame
--
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
* Re: [PATCH] tcp: ECN blackhole should not force quickack mode
From: jamal @ 2011-09-26 12:00 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, netdev, Jerry Chu, Ilpo Järvinen, Jim Gettys,
Dave Taht
In-Reply-To: <1317025614.2557.7.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
On Mon, 2011-09-26 at 10:26 +0200, Eric Dumazet wrote:
> This refers to additions to RFC 2481 : This was refined by RFC 3168, and
> the retransmitted TCP packets requirement is part of the final RFC :
Ah, yes - thats where it went.
>
> BTW, the ECN+ proposal (RFC 5562 : Adding Explicit Congestion
> Notification (ECN) Capability to TCP's SYN/ACK Packets) would allow the
> client (receiving SYNACK message with ECT flags) to set the TCP_ECN_SEEN
> I added in my patch, allowing even the first (retransmitted) data packet
> to trigger quickack mode.
Excellent ;->
Are you going to add 5562 support?
cheers,
jamal
^ permalink raw reply
* Re: [PATCH] tcp: ECN blackhole should not force quickack mode
From: Eric Dumazet @ 2011-09-26 12:44 UTC (permalink / raw)
To: jhs
Cc: David Miller, netdev, Jerry Chu, Ilpo Järvinen, Jim Gettys,
Dave Taht
In-Reply-To: <1317038443.1865.40.camel@mojatatu>
Le lundi 26 septembre 2011 à 08:00 -0400, jamal a écrit :
> On Mon, 2011-09-26 at 10:26 +0200, Eric Dumazet wrote:
>
> > This refers to additions to RFC 2481 : This was refined by RFC 3168, and
> > the retransmitted TCP packets requirement is part of the final RFC :
>
> Ah, yes - thats where it went.
>
> >
> > BTW, the ECN+ proposal (RFC 5562 : Adding Explicit Congestion
> > Notification (ECN) Capability to TCP's SYN/ACK Packets) would allow the
> > client (receiving SYNACK message with ECT flags) to set the TCP_ECN_SEEN
> > I added in my patch, allowing even the first (retransmitted) data packet
> > to trigger quickack mode.
>
> Excellent ;->
> Are you going to add 5562 support?
I sent a private mail to Adam Langley this morning asking him if he
planned an official submission of a prior patch :
http://www.ietf.org/mail-archive/web/tcpm/current/msg03988.html
If Adam is too busy or not anymore interested, I would like to spend
some cycles on this.
Thanks
^ permalink raw reply
* re: [SCSI] cxgb3i: convert cdev->l2opt to use rcu to prevent NULL dereference (v2)
From: Dan Carpenter @ 2011-09-26 13:40 UTC (permalink / raw)
To: nhorman; +Cc: netdev, linux-scsi
Hi Neil,
c98bc57ee65b6 "[SCSI] cxgb3i: convert cdev->l2opt to use rcu to
prevent NULL dereference (v2)" from linux-next introduces a usinging
uninitialized variable bug.
struct l2t_entry *t3_l2t_get(struct t3cdev *cdev, struct neighbour *neigh,
struct net_device *dev)
{
- struct l2t_entry *e;
- struct l2t_data *d = L2DATA(cdev);
+ struct l2t_entry *e = NULL;
+ struct l2t_data *d;
u32 addr = *(u32 *) neigh->primary_key;
int ifidx = neigh->dev->ifindex;
int hash = arp_hash(addr, ifidx, d);
^
Uninitialized variable.
struct port_info *p = netdev_priv(dev);
int smt_idx = p->port_id;
+ rcu_read_lock();
+ d = L2DATA(cdev);
+ if (!d)
+ goto done_rcu;
+
regards,
dan carpenter
^ permalink raw reply
* Re: [SCSI] cxgb3i: convert cdev->l2opt to use rcu to prevent NULL dereference (v2)
From: Neil Horman @ 2011-09-26 14:20 UTC (permalink / raw)
To: Dan Carpenter; +Cc: netdev, linux-scsi
In-Reply-To: <20110926134048.GA10999@longonot.mountain>
On Mon, Sep 26, 2011 at 04:40:48PM +0300, Dan Carpenter wrote:
> Hi Neil,
>
> c98bc57ee65b6 "[SCSI] cxgb3i: convert cdev->l2opt to use rcu to
> prevent NULL dereference (v2)" from linux-next introduces a usinging
> uninitialized variable bug.
>
> struct l2t_entry *t3_l2t_get(struct t3cdev *cdev, struct neighbour *neigh,
> struct net_device *dev)
> {
> - struct l2t_entry *e;
> - struct l2t_data *d = L2DATA(cdev);
> + struct l2t_entry *e = NULL;
> + struct l2t_data *d;
> u32 addr = *(u32 *) neigh->primary_key;
> int ifidx = neigh->dev->ifindex;
> int hash = arp_hash(addr, ifidx, d);
> ^
> Uninitialized variable.
>
> struct port_info *p = netdev_priv(dev);
> int smt_idx = p->port_id;
>
> + rcu_read_lock();
> + d = L2DATA(cdev);
> + if (!d)
> + goto done_rcu;
> +
>
> regards,
> dan carpenter
>
Yup, thanks, you need this to9 fix the uninitalized var.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
l2t.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/cxgb3/l2t.c b/drivers/net/cxgb3/l2t.c
index 3808f99..4154097 100644
--- a/drivers/net/cxgb3/l2t.c
+++ b/drivers/net/cxgb3/l2t.c
@@ -302,9 +302,9 @@ struct l2t_entry *t3_l2t_get(struct t3cdev *cdev, struct neighbour *neigh,
{
struct l2t_entry *e = NULL;
struct l2t_data *d;
+ int hash;
u32 addr = *(u32 *) neigh->primary_key;
int ifidx = neigh->dev->ifindex;
- int hash = arp_hash(addr, ifidx, d);
struct port_info *p = netdev_priv(dev);
int smt_idx = p->port_id;
@@ -313,6 +313,8 @@ struct l2t_entry *t3_l2t_get(struct t3cdev *cdev, struct neighbour *neigh,
if (!d)
goto done_rcu;
+ hash = arp_hash(addr, ifidx, d);
+
write_lock_bh(&d->lock);
for (e = d->l2tab[hash].first; e; e = e->next)
if (e->addr == addr && e->ifindex == ifidx &&
^ permalink raw reply related
* Re: intel 82599 multi-port performance
From: Chris Friesen @ 2011-09-26 14:20 UTC (permalink / raw)
To: J.Hwan Kim; +Cc: netdev
In-Reply-To: <4E805359.2080600@gmail.com>
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?
Chris
--
Chris Friesen
Software Developer
GENBAND
chris.friesen@genband.com
www.genband.com
^ permalink raw reply
* Re: [SCSI] cxgb3i: convert cdev->l2opt to use rcu to prevent NULL dereference (v2)
From: James Bottomley @ 2011-09-26 14:31 UTC (permalink / raw)
To: Neil Horman; +Cc: Dan Carpenter, netdev, linux-scsi
In-Reply-To: <20110926142052.GA18283@hmsreliant.think-freely.org>
On Mon, 2011-09-26 at 10:20 -0400, Neil Horman wrote:
> On Mon, Sep 26, 2011 at 04:40:48PM +0300, Dan Carpenter wrote:
> > Hi Neil,
> >
> > c98bc57ee65b6 "[SCSI] cxgb3i: convert cdev->l2opt to use rcu to
> > prevent NULL dereference (v2)" from linux-next introduces a usinging
> > uninitialized variable bug.
> >
> > struct l2t_entry *t3_l2t_get(struct t3cdev *cdev, struct neighbour *neigh,
> > struct net_device *dev)
> > {
> > - struct l2t_entry *e;
> > - struct l2t_data *d = L2DATA(cdev);
> > + struct l2t_entry *e = NULL;
> > + struct l2t_data *d;
> > u32 addr = *(u32 *) neigh->primary_key;
> > int ifidx = neigh->dev->ifindex;
> > int hash = arp_hash(addr, ifidx, d);
> > ^
> > Uninitialized variable.
> >
> > struct port_info *p = netdev_priv(dev);
> > int smt_idx = p->port_id;
> >
> > + rcu_read_lock();
> > + d = L2DATA(cdev);
> > + if (!d)
> > + goto done_rcu;
> > +
> >
> > regards,
> > dan carpenter
> >
>
> Yup, thanks, you need this to9 fix the uninitalized var.
I rolled this into the original.
James
^ permalink raw reply
* Re: [PATCH v3 4/7] per-cgroup tcp buffers control
From: Andrew Vagin @ 2011-09-26 14:39 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-kernel, paul, lizf, kamezawa.hiroyu, ebiederm, davem,
gthelen, netdev, linux-mm, kirill
In-Reply-To: <1316393805-3005-5-git-send-email-glommer@parallels.com>
We can't change net.ipv4.tcp_mem if a cgroup with memory controller
isn't mounted.
[root@dhcp-10-30-20-19 ~]# sysctl -w net.ipv4.tcp_mem="3 2 3"
error: "Invalid argument" setting key "net.ipv4.tcp_mem"
It's because tcp_max_memory is initialized in mem_cgroup_populate:
mem_cgroup_populate->register_kmem_files->sockets_populate->tcp_init_cgroup
> +int sockets_populate(struct cgroup *cgrp, struct cgroup_subsys *ss)
> +{
> + struct proto *proto;
> + int ret = 0;
> +
> + read_lock(&proto_list_lock);
> + list_for_each_entry(proto,&proto_list, node) {
> + if (proto->init_cgroup)
> + ret |= proto->init_cgroup(proto, cgrp, ss);
> + }
> + if (!ret)
> + goto out;
> +
> + list_for_each_entry_continue_reverse(proto,&proto_list, node)
> + if (proto->destroy_cgroup)
> + proto->destroy_cgroup(proto, cgrp, ss);
> +
> +out:
> + read_unlock(&proto_list_lock);
> + return ret;
> +}
> @@ -198,6 +203,21 @@ static int ipv4_tcp_mem(ctl_table *ctl, int write,
> if (ret)
> return ret;
>
> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM
> + rcu_read_lock();
> + cg = mem_cgroup_from_task(current);
> + for (i = 0; i< 3; i++)
> + if (vec[i]> tcp_max_memory(cg)) {
> + rcu_read_unlock();
> + return -EINVAL;
> + }
--
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
* Re: [SCSI] cxgb3i: convert cdev->l2opt to use rcu to prevent NULL dereference (v2)
From: Neil Horman @ 2011-09-26 15:02 UTC (permalink / raw)
To: James Bottomley; +Cc: Dan Carpenter, netdev, linux-scsi
In-Reply-To: <1317047476.9560.0.camel@dabdike>
On Mon, Sep 26, 2011 at 09:31:15AM -0500, James Bottomley wrote:
> On Mon, 2011-09-26 at 10:20 -0400, Neil Horman wrote:
> > On Mon, Sep 26, 2011 at 04:40:48PM +0300, Dan Carpenter wrote:
> > > Hi Neil,
> > >
> > > c98bc57ee65b6 "[SCSI] cxgb3i: convert cdev->l2opt to use rcu to
> > > prevent NULL dereference (v2)" from linux-next introduces a usinging
> > > uninitialized variable bug.
> > >
> > > struct l2t_entry *t3_l2t_get(struct t3cdev *cdev, struct neighbour *neigh,
> > > struct net_device *dev)
> > > {
> > > - struct l2t_entry *e;
> > > - struct l2t_data *d = L2DATA(cdev);
> > > + struct l2t_entry *e = NULL;
> > > + struct l2t_data *d;
> > > u32 addr = *(u32 *) neigh->primary_key;
> > > int ifidx = neigh->dev->ifindex;
> > > int hash = arp_hash(addr, ifidx, d);
> > > ^
> > > Uninitialized variable.
> > >
> > > struct port_info *p = netdev_priv(dev);
> > > int smt_idx = p->port_id;
> > >
> > > + rcu_read_lock();
> > > + d = L2DATA(cdev);
> > > + if (!d)
> > > + goto done_rcu;
> > > +
> > >
> > > regards,
> > > dan carpenter
> > >
> >
> > Yup, thanks, you need this to9 fix the uninitalized var.
>
> I rolled this into the original.
>
Thank you!
Neil
> James
>
>
>
^ permalink raw reply
* Question about memory leak detector giving false positive report for net/core/flow.c
From: Huajun Li @ 2011-09-26 15:17 UTC (permalink / raw)
To: Catalin Marinas, linux-mm; +Cc: "Eric Dumaze"t, netdev, Huajun Li
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 ?
BTW, I wrote a simple test code to emulate net/core/flow.c behavior at
this stage which triggers the report, and it could also make kmemleak
give similar report, please check below test code:
kernel version:
#uname -a
Linux 3.1.0-rc7 #22 SMP Tue Sep 26 05:43:01 CST 2011 x86_64 x86_64
x86_64 GNU/Linux
memory leak report:
-------------------------------------------------------------------------------------------
unreferenced object 0xffff880073a70000 (size 8192):
comm "swapper", pid 1, jiffies 4294937832 (age 445.740s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<ffffffff8124db64>] create_object+0x144/0x360
[<ffffffff8191192e>] kmemleak_alloc+0x7e/0x110
[<ffffffff81235b26>] __kmalloc_node+0x156/0x3a0
[<ffffffff81935512>] flow_cache_cpu_prepare.clone.1+0x58/0xc0
[<ffffffff8214c361>] flow_cache_init_global+0xb6/0x1af
[<ffffffff8100225d>] do_one_initcall+0x4d/0x260
[<ffffffff820ec2e9>] kernel_init+0x161/0x23a
[<ffffffff8194ab04>] kernel_thread_helper+0x4/0x10
[<ffffffffffffffff>] 0xffffffffffffffff
unreferenced object 0xffff880073a74290 (size 8192):
comm "swapper", pid 1, jiffies 4294937832 (age 445.740s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<ffffffff8124db64>] create_object+0x144/0x360
[<ffffffff8191192e>] kmemleak_alloc+0x7e/0x110
[<ffffffff81235b26>] __kmalloc_node+0x156/0x3a0
[<ffffffff81935512>] flow_cache_cpu_prepare.clone.1+0x58/0xc0
[<ffffffff8214c361>] flow_cache_init_global+0xb6/0x1af
[<ffffffff8100225d>] do_one_initcall+0x4d/0x260
[<ffffffff820ec2e9>] kernel_init+0x161/0x23a
[<ffffffff8194ab04>] kernel_thread_helper+0x4/0x10
[<ffffffffffffffff>] 0xffffffffffffffff
Simple test code to reproduce a similar report:
-----------------------------------------------------------------------------------------
MODULE_LICENSE("GPL");
struct test {
int *pt;
};
static struct test __percpu *percpu;
static int __init test_init(void)
{
int i;
percpu = alloc_percpu(struct test);
if (!percpu)
return -ENOMEM;
for_each_online_cpu(i) {
struct test *p = per_cpu_ptr(percpu, i);
p->pt = kmalloc(sizeof(int), GFP_KERNEL);
}
return 0;
}
static void __exit test_exit(void)
{
int i;
for_each_possible_cpu(i) {
struct test *p = per_cpu_ptr(percpu, i);
if (p->pt)
kfree(p->pt);
}
if (percpu)
free_percpu(percpu);
}
module_init(test_init);
module_exit(test_exit);
^ permalink raw reply
* Re: intel 82599 multi-port performance
From: J.Hwan.Kim @ 2011-09-26 15:42 UTC (permalink / raw)
Cc: netdev
In-Reply-To: <4E808A41.8040902@genband.com>
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.
^ permalink raw reply
* Re: Bridge stays down until a port is added
From: Stephen Hemminger @ 2011-09-26 15:47 UTC (permalink / raw)
To: Marc Haber; +Cc: netdev
In-Reply-To: <20110925082504.GA32712@torres.zugschlus.de>
The root of this whole problem is really that IPv6 reports addresses
in a tentative state to applications that can not be passed to the bind() system call.
For most cases, this problem never happens because the tentative addresses are
resolved by Duplicate Address Detection before the application starts. But
I have seen (and fixed) this happen before this whole discussion started.
1. The problem is not unique to bridges. It happens with bridge, macvtap,
even on wireless networks where the device is available but carrier is
not asserted.
2. Any change to what the kernel does (like not reporting tentative addresses)
would break applications even worse.
3. When the bridge was always reporting carrier, it was in effect breaking
IPv6 Duplicate Address Detection. And that is bad.
^ permalink raw reply
* Re: Why not refresh ipv6 local link address of a bridge when a new interface added to the bridge?
From: Stephen Hemminger @ 2011-09-26 15:51 UTC (permalink / raw)
To: Eric Xiong; +Cc: netdev
In-Reply-To: <CA+UXGK3yQzUuZwHWooxd91G77ACvYzwTQUOoDdVc-=CCEv4POQ@mail.gmail.com>
On Tue, 20 Sep 2011 10:39:31 +0800
Eric Xiong <bottomofstack@gmail.com> wrote:
> Hi all:
> Sorry for disturb! when adding a interface to bridge, maybe the bridge
> mac address will be changed. the bridge code don't invoke
> call_netdevice_notifiers to notify ipv6 code to refresh local link
> address. I don't know why. Thanks for your help!
>
> BRs.
This change went in for v3.1
Bridge: Always send NETDEV_CHANGEADDR up on br MAC change.
This ensures the neighbor entries associated with the bridge
dev are flushed, also invalidating the associated cached L2 headers.
This means we br_add_if/br_del_if ports to implement hand-over and
not wind up with bridge packets going out with stale MAC.
This means we can also change MAC of port device and also not wind
up with bridge packets going out with stale MAC.
This builds on Stephen Hemminger's patch, also handling the br_del_if
case and the port MAC change case.
Cc: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* RE: [net-next 02/10] ixgbevf: Fix broken trunk vlan
From: Rose, Gregory V @ 2011-09-26 15:57 UTC (permalink / raw)
To: Jesse Gross, Kirsher, Jeffrey T
Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com,
Jiri Pirko
In-Reply-To: <CAEP_g=93GcR7yE7N+GtaB6dpjPuGGtCBXznEmV6Sx=BX-kSbmg@mail.gmail.com>
> -----Original Message-----
> From: Jesse Gross [mailto:jesse@nicira.com]
> Sent: Saturday, September 24, 2011 9:33 AM
> To: Kirsher, Jeffrey T
> Cc: davem@davemloft.net; Rose, Gregory V; netdev@vger.kernel.org;
> gospo@redhat.com; Jiri Pirko
> Subject: Re: [net-next 02/10] ixgbevf: Fix broken trunk vlan
>
> On Sat, Sep 24, 2011 at 2:17 AM, Jeff Kirsher
> <jeffrey.t.kirsher@intel.com> wrote:
> > From: Greg Rose <gregory.v.rose@intel.com>
> >
> > Changes to clean up the vlan rx path broke trunk vlan. Trunk vlans in
> > a VF driver are those set using:
> >
> > "ip link set <pfdev> vf <n> <vlanid>"
> >
> > Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> > CC: Jiri Pirko <jpirko@redhat.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> > drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 6 ++----
> > 1 files changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> > index d72905b..4930c46 100644
> > --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> > @@ -293,12 +293,10 @@ static void ixgbevf_receive_skb(struct
> ixgbevf_q_vector *q_vector,
> > {
> > struct ixgbevf_adapter *adapter = q_vector->adapter;
> > bool is_vlan = (status & IXGBE_RXD_STAT_VP);
> > + u16 tag = le16_to_cpu(rx_desc->wb.upper.vlan);
> >
> > - if (is_vlan) {
> > - u16 tag = le16_to_cpu(rx_desc->wb.upper.vlan);
> > -
> > + if (is_vlan && test_bit(tag, adapter->active_vlans))
> > __vlan_hwaccel_put_tag(skb, tag);
> > - }
>
> What happens if you run tcpdump without configuring vlan devices?
> Shouldn't you see tagged packets for the vlans that are being trunked
> to you? I think this will strip tags in that case. The apparent
> behavior of vlan filters here is also surprising to me because on one
> hand if they're truly filtering this test shouldn't be needed and on
> the other hand they don't seem to be disabled in promiscuous mode.
I think you're not quite understanding the action the HW is taking here. In the physical function driver we have put the VF on a VLAN without it knowing that it's on a VLAN. Once that is done by the PF, the VF is not allowed to configure its own VLANs anymore. However, the descriptor still includes a bit for the VLAN tag indicating it was a packet that arrived on a VLAN. The HW is inserting and stripping the VLAN tag though without any awareness of that by the VF driver.
It's a security measure to allow an administrator to put a VF on a VLAN to provide another level of isolation for the VF.
Intel VFs don't support promiscuous mode. If you ran tcpdump you wouldn't see the VLAN tags because they've been stripped by the HW. The VF has no choice in this.
- Greg
^ permalink raw reply
* Re: intel 82599 multi-port performance
From: Alexander Duyck @ 2011-09-26 16:04 UTC (permalink / raw)
To: frog1120; +Cc: J.Hwan.Kim, 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.
What kind of rates are you seeing on a single port versus multiple
ports? There are multiple possibilities in terms of what could be
limiting your performance.
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. If you have a second card available
you may want to try installing that in a secondary Gen2 PCIe slot and
seeing if you can improve the performance by using 2 PCIe slots instead
of one.
Also could you include your kernel config? Certain features such as
Netfilter and IOMMU can have a significant impact on performance.
Thanks,
Alex
^ permalink raw reply
* RE: [net-next 05/10] if_link: Add additional parameter to IFLA_VF_INFO for spoof checking
From: Rose, Gregory V @ 2011-09-26 16:06 UTC (permalink / raw)
To: Ben Hutchings, Kirsher, Jeffrey T
Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com
In-Reply-To: <1316882459.4122.83.camel@deadeye>
> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Sent: Saturday, September 24, 2011 9:41 AM
> To: Kirsher, Jeffrey T
> Cc: davem@davemloft.net; Rose, Gregory V; netdev@vger.kernel.org;
> gospo@redhat.com
> Subject: Re: [net-next 05/10] if_link: Add additional parameter to
> IFLA_VF_INFO for spoof checking
>
> On Sat, 2011-09-24 at 02:17 -0700, Jeff Kirsher wrote:
> > From: Greg Rose <gregory.v.rose@intel.com>
> >
> > Add configuration setting for drivers to turn spoof checking on or off
> > for discrete VFs.
> >
> > Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> > include/linux/if_link.h | 7 +++++++
> > include/linux/netdevice.h | 3 +++
> > net/core/rtnetlink.c | 25 ++++++++++++++++++++++---
> > 3 files changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/if_link.h b/include/linux/if_link.h
> > index 0ee969a..8bd6d6d 100644
> > --- a/include/linux/if_link.h
> > +++ b/include/linux/if_link.h
> > @@ -279,6 +279,7 @@ enum {
> > IFLA_VF_MAC, /* Hardware queue specific attributes */
> > IFLA_VF_VLAN,
> > IFLA_VF_TX_RATE, /* TX Bandwidth Allocation */
> > + IFLA_VF_SPOOFCHK, /* Spoof Checking on/off switch */
> > __IFLA_VF_MAX,
> > };
> >
> > @@ -300,12 +301,18 @@ struct ifla_vf_tx_rate {
> > __u32 rate; /* Max TX bandwidth in Mbps, 0 disables throttling */
> > };
> >
> > +struct ifla_vf_spoofchk {
> > + __u32 vf;
> > + __u32 setting;
> > +};
> > +
> > struct ifla_vf_info {
> > __u32 vf;
> > __u8 mac[32];
> > __u32 vlan;
> > __u32 qos;
> > __u32 tx_rate;
> > + __u32 spoofchk;
> > };
>
> Not something you need to change now, but shouldn't this last struct
> definition be #ifdef __KERNEL__?
It's not done for any of the other ifla_vf_* structs in if_link.h. Which doesn't mean it shouldn't have been done but I was merely following custom. If it needs doing though then I'd be glad to do it.
>
> > /* VF ports management section
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index 43b3298..a2951a0 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -781,6 +781,7 @@ struct netdev_tc_txq {
> > * int (*ndo_set_vf_mac)(struct net_device *dev, int vf, u8* mac);
> > * int (*ndo_set_vf_vlan)(struct net_device *dev, int vf, u16 vlan, u8
> qos);
> > * int (*ndo_set_vf_tx_rate)(struct net_device *dev, int vf, int rate);
> > + * int (*ndo_set_vf_spoofchk)(struct net_device *dev, int vf, u8
> setting);
> > * int (*ndo_get_vf_config)(struct net_device *dev,
> > * int vf, struct ifla_vf_info *ivf);
> > * int (*ndo_set_vf_port)(struct net_device *dev, int vf,
> > @@ -900,6 +901,8 @@ struct net_device_ops {
> > int queue, u16 vlan, u8 qos);
> > int (*ndo_set_vf_tx_rate)(struct net_device *dev,
> > int vf, int rate);
> > + int (*ndo_set_vf_spoofchk)(struct net_device *dev,
> > + int vf, u8 setting);
>
> Why u8 and not bool?
I didn't see the bool type used in any of the other parameter lists for the net device ops so I hesitated to use it myself. If it's not a problem doing so then I'll go ahead and change it.
>
> > int (*ndo_get_vf_config)(struct net_device *dev,
> > int vf,
> > struct ifla_vf_info *ivf);
> > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > index 39f8dd6..6535810 100644
> > --- a/net/core/rtnetlink.c
> > +++ b/net/core/rtnetlink.c
> > @@ -731,7 +731,8 @@ static inline int rtnl_vfinfo_size(const struct
> net_device *dev)
> > size += num_vfs *
> > (nla_total_size(sizeof(struct ifla_vf_mac)) +
> > nla_total_size(sizeof(struct ifla_vf_vlan)) +
> > - nla_total_size(sizeof(struct ifla_vf_tx_rate)));
> > + nla_total_size(sizeof(struct ifla_vf_tx_rate)) +
> > + nla_total_size(sizeof(struct ifla_vf_spoofchk)));
> > return size;
> > } else
> > return 0;
> > @@ -954,13 +955,19 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
> struct net_device *dev,
> > struct ifla_vf_mac vf_mac;
> > struct ifla_vf_vlan vf_vlan;
> > struct ifla_vf_tx_rate vf_tx_rate;
> > + struct ifla_vf_spoofchk vf_spoofchk;
> > if (dev->netdev_ops->ndo_get_vf_config(dev, i, &ivi))
> > break;
> > - vf_mac.vf = vf_vlan.vf = vf_tx_rate.vf = ivi.vf;
> > + vf_mac.vf =
> > + vf_vlan.vf =
> > + vf_tx_rate.vf =
> > + vf_spoofchk.vf = ivi.vf;
> > +
> [...]
>
> The continuation lines should be indented. Or you could just write the
> assignments as multiple statements.
OK, sure. I'll do that.
Thanks,
- Greg
^ permalink raw reply
* Re: [net-next 05/10] if_link: Add additional parameter to IFLA_VF_INFO for spoof checking
From: Stephen Hemminger @ 2011-09-26 16:14 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Jeff Kirsher, davem, Greg Rose, netdev, gospo
In-Reply-To: <1316981213.4122.106.camel@deadeye>
On Sun, 25 Sep 2011 21:06:49 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:
> On Sun, 2011-09-25 at 10:22 -0700, Stephen Hemminger wrote:
> > On Sat, 24 Sep 2011 02:17:38 -0700
> > Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> >
> > > From: Greg Rose <gregory.v.rose@intel.com>
> > >
> > > Add configuration setting for drivers to turn spoof checking on or off
> > > for discrete VFs.
> > >
> > > Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> > > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > > ---
> > > include/linux/if_link.h | 7 +++++++
> > > include/linux/netdevice.h | 3 +++
> > > net/core/rtnetlink.c | 25 ++++++++++++++++++++++---
> > > 3 files changed, 32 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/include/linux/if_link.h b/include/linux/if_link.h
> > > index 0ee969a..8bd6d6d 100644
> > > --- a/include/linux/if_link.h
> > > +++ b/include/linux/if_link.h
> > > @@ -279,6 +279,7 @@ enum {
> > > IFLA_VF_MAC, /* Hardware queue specific attributes */
> > > IFLA_VF_VLAN,
> > > IFLA_VF_TX_RATE, /* TX Bandwidth Allocation */
> > > + IFLA_VF_SPOOFCHK, /* Spoof Checking on/off switch */
> > > __IFLA_VF_MAX,
> > > };
> > >
> > > @@ -300,12 +301,18 @@ struct ifla_vf_tx_rate {
> > > __u32 rate; /* Max TX bandwidth in Mbps, 0 disables throttling */
> > > };
> > >
> > > +struct ifla_vf_spoofchk {
> > > + __u32 vf;
> > > + __u32 setting;
> > > +};
> > > +
> > > struct ifla_vf_info {
> > > __u32 vf;
> > > __u8 mac[32];
> > > __u32 vlan;
> > > __u32 qos;
> > > __u32 tx_rate;
> > > + __u32 spoofchk;
> > > };
> >
> > This breaks ABI compatibility, unless you add some special case code
> > to handle the case of tools sending the old ifla_vf_info. Users may have older version
> > of ip utilities that send smaller size structure.
>
> Unless I'm missing something, that structure is not sent or received by
> userland; that's why I thought it should be #ifdef __KERNEL__.
The struct ifla_vf_info is exposed to userland, it is not inside the #ifdef
and therefore exposed.
But it is probably okay to change this structure because the ifla_vf_info
is not used/supported by any released version iproute2. There may have been
some patches to use this, but they never made it into the git or released
code.
^ permalink raw reply
* RE: [net-next 05/10] if_link: Add additional parameter to IFLA_VF_INFO for spoof checking
From: Rose, Gregory V @ 2011-09-26 16:18 UTC (permalink / raw)
To: Stephen Hemminger, Kirsher, Jeffrey T
Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com
In-Reply-To: <20110925102238.55ba72eb@s6510.linuxnetplumber.net>
> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> Sent: Sunday, September 25, 2011 10:23 AM
> To: Kirsher, Jeffrey T
> Cc: davem@davemloft.net; Rose, Gregory V; netdev@vger.kernel.org;
> gospo@redhat.com
> Subject: Re: [net-next 05/10] if_link: Add additional parameter to
> IFLA_VF_INFO for spoof checking
>
> On Sat, 24 Sep 2011 02:17:38 -0700
> Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
>
> > From: Greg Rose <gregory.v.rose@intel.com>
> >
> > Add configuration setting for drivers to turn spoof checking on or off
> > for discrete VFs.
> >
> > Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> > include/linux/if_link.h | 7 +++++++
> > include/linux/netdevice.h | 3 +++
> > net/core/rtnetlink.c | 25 ++++++++++++++++++++++---
> > 3 files changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/if_link.h b/include/linux/if_link.h
> > index 0ee969a..8bd6d6d 100644
> > --- a/include/linux/if_link.h
> > +++ b/include/linux/if_link.h
> > @@ -279,6 +279,7 @@ enum {
> > IFLA_VF_MAC, /* Hardware queue specific attributes */
> > IFLA_VF_VLAN,
> > IFLA_VF_TX_RATE, /* TX Bandwidth Allocation */
> > + IFLA_VF_SPOOFCHK, /* Spoof Checking on/off switch */
> > __IFLA_VF_MAX,
> > };
> >
> > @@ -300,12 +301,18 @@ struct ifla_vf_tx_rate {
> > __u32 rate; /* Max TX bandwidth in Mbps, 0 disables throttling */
> > };
> >
> > +struct ifla_vf_spoofchk {
> > + __u32 vf;
> > + __u32 setting;
> > +};
> > +
> > struct ifla_vf_info {
> > __u32 vf;
> > __u8 mac[32];
> > __u32 vlan;
> > __u32 qos;
> > __u32 tx_rate;
> > + __u32 spoofchk;
> > };
>
> This breaks ABI compatibility, unless you add some special case code
> to handle the case of tools sending the old ifla_vf_info. Users may have
> older version
> of ip utilities that send smaller size structure.
The structure is not sent directly to the kernel from user space. The kernel will get the information and stuff it into individual data units using NLA_PUT. If the older tool doesn't ask for the info then that's fine so far as I can tell.
The only issue I've seen is using the new ip tool on older kernels that don't supply the data. You'll get a segmentation fault and core dump. However, I was under the impression that the general rule was to use a release of the ip tool only on the kernel it was released for or on newer kernels.
- Greg
^ permalink raw reply
* Re: [net-next 05/10] if_link: Add additional parameter to IFLA_VF_INFO for spoof checking
From: Stephen Hemminger @ 2011-09-26 16:21 UTC (permalink / raw)
To: Rose, Gregory V
Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
gospo@redhat.com
In-Reply-To: <43F901BD926A4E43B106BF17856F0755019C519376@orsmsx508.amr.corp.intel.com>
On Mon, 26 Sep 2011 09:18:34 -0700
"Rose, Gregory V" <gregory.v.rose@intel.com> wrote:
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> > Sent: Sunday, September 25, 2011 10:23 AM
> > To: Kirsher, Jeffrey T
> > Cc: davem@davemloft.net; Rose, Gregory V; netdev@vger.kernel.org;
> > gospo@redhat.com
> > Subject: Re: [net-next 05/10] if_link: Add additional parameter to
> > IFLA_VF_INFO for spoof checking
> >
> > On Sat, 24 Sep 2011 02:17:38 -0700
> > Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> >
> > > From: Greg Rose <gregory.v.rose@intel.com>
> > >
> > > Add configuration setting for drivers to turn spoof checking on or off
> > > for discrete VFs.
> > >
> > > Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> > > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > > ---
> > > include/linux/if_link.h | 7 +++++++
> > > include/linux/netdevice.h | 3 +++
> > > net/core/rtnetlink.c | 25 ++++++++++++++++++++++---
> > > 3 files changed, 32 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/include/linux/if_link.h b/include/linux/if_link.h
> > > index 0ee969a..8bd6d6d 100644
> > > --- a/include/linux/if_link.h
> > > +++ b/include/linux/if_link.h
> > > @@ -279,6 +279,7 @@ enum {
> > > IFLA_VF_MAC, /* Hardware queue specific attributes */
> > > IFLA_VF_VLAN,
> > > IFLA_VF_TX_RATE, /* TX Bandwidth Allocation */
> > > + IFLA_VF_SPOOFCHK, /* Spoof Checking on/off switch */
> > > __IFLA_VF_MAX,
> > > };
> > >
> > > @@ -300,12 +301,18 @@ struct ifla_vf_tx_rate {
> > > __u32 rate; /* Max TX bandwidth in Mbps, 0 disables throttling */
> > > };
> > >
> > > +struct ifla_vf_spoofchk {
> > > + __u32 vf;
> > > + __u32 setting;
> > > +};
> > > +
> > > struct ifla_vf_info {
> > > __u32 vf;
> > > __u8 mac[32];
> > > __u32 vlan;
> > > __u32 qos;
> > > __u32 tx_rate;
> > > + __u32 spoofchk;
> > > };
> >
> > This breaks ABI compatibility, unless you add some special case code
> > to handle the case of tools sending the old ifla_vf_info. Users may have
> > older version
> > of ip utilities that send smaller size structure.
>
> The structure is not sent directly to the kernel from user space. The kernel will get the information and stuff it into individual data units using NLA_PUT. If the older tool doesn't ask for the info then that's fine so far as I can tell.
>
> The only issue I've seen is using the new ip tool on older kernels that don't supply the data. You'll get a segmentation fault and core dump. However, I was under the impression that the general rule was to use a release of the ip tool only on the kernel it was released for or on newer kernels.
>
> - Greg
The tools need to run on older kernels. Think of Debian and other distributions which want to
ship newer ip tools but run on old kernel. In this case, what is expected is:
# ip li some new option
RTNETLINK: Invalid ...
^ permalink raw reply
* RE: [net-next] iproute2: Add new command to IP link to enable/disable VF spoof check
From: Rose, Gregory V @ 2011-09-26 16:21 UTC (permalink / raw)
To: Stephen Hemminger, Kirsher, Jeffrey T
Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com
In-Reply-To: <20110925102353.602d239b@s6510.linuxnetplumber.net>
> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> Sent: Sunday, September 25, 2011 10:24 AM
> To: Kirsher, Jeffrey T
> Cc: Rose, Gregory V; davem@davemloft.net; netdev@vger.kernel.org;
> gospo@redhat.com
> Subject: Re: [net-next] iproute2: Add new command to IP link to
> enable/disable VF spoof check
>
> On Sun, 25 Sep 2011 01:23:18 -0700
> Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
>
> > From: Greg Rose <gregory.v.rose@intel.com>
> >
> > Add IP link command parsing for VF spoof checking enable/disable
> >
> > Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>
> See my comments about ABI compatibility. Can't just grow size of
> structure.
> Need to have new version of iproute work with older kernels.
In this case I have no idea how to proceed. I'll go back and stare at it some more.
- Greg
^ permalink raw reply
* Re: [net-next 1/8] pci: Add flag indicating device has been assigned by KVM [and Xen]
From: Konrad Rzeszutek Wilk @ 2011-09-26 16:25 UTC (permalink / raw)
To: Jeff Kirsher, davem@davemloft.net
Cc: Rose, Gregory V, Ian Campbell, davem@davemloft.net, Jesse Barnes,
netdev@vger.kernel.org, gospo@redhat.com,
linux-pci@vger.kernel.org
In-Reply-To: <1316839300.2182.134.camel@jtkirshe-mobl>
On Fri, Sep 23, 2011 at 09:41:39PM -0700, Jeff Kirsher wrote:
> On Fri, 2011-09-23 at 12:05 -0700, Konrad Rzeszutek Wilk wrote:
> > > > > Version 2 of this patch with the modified comment was just posted to
> > > > netdev. Ian Campbell had mentioned that the Xen pciback driver should be
> > > > modified to use this flag but I don't believe any patches for it have been
> > > > created yet.
> > > >
> > > > http://marc.info/?i=20110729165446.GA6731@dumpdata.com
..
Any plans to support bnx4 and igb with this feature? Any ETA?
Dave,
Please also add this patch to your net-next queue.
>From 53cada5f5f614a856f6ea5a09dc565d6a455d7c2 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Mon, 26 Sep 2011 12:22:01 -0400
Subject: [PATCH] xen/pciback: Add flag indicating device has been assigned by Xen
Device drivers that create and destroy SR-IOV virtual functions via
calls to pci_enable_sriov() and pci_disable_sriov can cause catastrophic
failures if they attempt to destroy VFs while they are assigned to
guest virtual machines. By adding a flag for use by the Xen PCI back
to indicate that a device is assigned a device driver can check that
flag and avoid destroying VFs while they are assigned and avoid system
failures.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/xen/xen-pciback/xenbus.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
index 978d2c6..18db31f 100644
--- a/drivers/xen/xen-pciback/xenbus.c
+++ b/drivers/xen/xen-pciback/xenbus.c
@@ -249,6 +249,7 @@ static int xen_pcibk_export_device(struct xen_pcibk_device *pdev,
goto out;
dev_dbg(&dev->dev, "registering for %d\n", pdev->xdev->otherend_id);
+ dev->dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
if (xen_register_device_domain_owner(dev,
pdev->xdev->otherend_id) != 0) {
dev_err(&dev->dev, "device has been assigned to another " \
@@ -288,6 +289,7 @@ static int xen_pcibk_remove_device(struct xen_pcibk_device *pdev,
}
dev_dbg(&dev->dev, "unregistering for %d\n", pdev->xdev->otherend_id);
+ dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
xen_unregister_device_domain_owner(dev);
xen_pcibk_release_pci_dev(pdev, dev);
--
1.7.4.1
^ permalink raw reply related
* RE: [net-next 1/8] pci: Add flag indicating device has been assigned by KVM [and Xen]
From: Rose, Gregory V @ 2011-09-26 16:31 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, Kirsher, Jeffrey T, davem@davemloft.net
Cc: Ian Campbell, davem@davemloft.net, Jesse Barnes,
netdev@vger.kernel.org, gospo@redhat.com,
linux-pci@vger.kernel.org
In-Reply-To: <20110926162552.GA4175@phenom.oracle.com>
> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Monday, September 26, 2011 9:26 AM
> To: Kirsher, Jeffrey T; davem@davemloft.net
> Cc: Rose, Gregory V; Ian Campbell; davem@davemloft.net; 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]
>
> On Fri, Sep 23, 2011 at 09:41:39PM -0700, Jeff Kirsher wrote:
> > On Fri, 2011-09-23 at 12:05 -0700, Konrad Rzeszutek Wilk wrote:
> > > > > > Version 2 of this patch with the modified comment was just
> posted to
> > > > > netdev. Ian Campbell had mentioned that the Xen pciback driver
> should be
> > > > > modified to use this flag but I don't believe any patches for it
> have been
> > > > > created yet.
> > > > >
> > > > > http://marc.info/?i=20110729165446.GA6731@dumpdata.com
> ..
> 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.
> Any ETA?
I should have the igb driver work done in the next week or so.
- Greg
^ permalink raw reply
* Re: [PATCH] net: change capability used by socket options IP{,V6}_TRANSPARENT
From: Casey Schaufler @ 2011-09-26 16:31 UTC (permalink / raw)
To: Maciej Żenczykowski
Cc: netdev, linux-security-module, James Morris, Casey Schaufler
In-Reply-To: <CAHo-OozcFULbDBHoWsQPoun-HAaHLBM=pRzMJjsv1cJF77zSYg@mail.gmail.com>
On 9/23/2011 12:33 PM, Maciej Żenczykowski wrote:
>> Under what circumstances would a process that requires the
>> new capability not require CAP_NET_ADMIN? Is there a real
>> case where a process would be expected to require only this
>> new capability? Adding new capability values is somewhat
>> perilous and the granularity you are proposing, that of
>> controlling a single bit, would explode the list of
>> capabilities into the hundreds if it were applied throughout
>> the kernel.
> CAP_NET_ADMIN is a huge hammer, it allows one to totally
> reconfigure the networking subsystem.
>
> In a containerized multi-user/job environment, you do not want
> something like an instance of a load-balanced web server, proxy
> or dns server being able to do that - policy/configuration decisions
> should be left up to the administrator and/or machine management
> daemon(s). Each of these can make use of transparent sockets
> (in various ways, mostly in coordination with large scale load balancing).
>
> You also do not want one user running in one container being able
> to sniff (CAP_NET_RAW) traffic from another user (hence CAP_NET_RAW
> isn't an acceptable substitute).
>
> One could conceivably use network namespaces for seperation, but
> in this particular case they are _way_ too overkill (and also add too
> much overhead).
>
> This might be *just* a single bit in the socket, but this bit effectively
> controls whether you can do certain types of privileged operations
> on the socket in question - and it gets tested in various places throughout
> the networking stack.
>
> Hopefully, this answers your question.
It is an important argument, but no, it does not address my issue.
The problem is that you can make that same argument for breaking up
just about every capability. CAP_SYSADMIN could easily be broken into
a hundred separate capabilities and CAP_NET_ADMIN, as you point out,
into dozens. My point is that with the potential to create so many
capabilities, what makes this particular action so much more important
than the other things already covered by CAP_NET_ADMIN?
If we introduce dozens of new capabilities we will end up with
the exact same problem that has been so clearly demonstrated
by the SELinux reference policy. Excessive granularity will kill
and security facility. Capabilities are already more granular than
most people are comfortable with.
For a facility to warrant a new capability it must be completely
unreasonable to fit it into an existing capability, sufficiently
general in its use that the bit won't show up unused when networking
fashions change in a year or two, and it needs to protect something
that is obviously very important. You have a variant of a somewhat
obscure facility that will only be used in edge cases in support
of an unproven (if promising) technology that is targeted to a
disappearing use case.
>
> - Maciej
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" 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 linux-security-module" 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
* RE: [net-next 05/10] if_link: Add additional parameter to IFLA_VF_INFO for spoof checking
From: Rose, Gregory V @ 2011-09-26 16:32 UTC (permalink / raw)
To: Stephen Hemminger, Ben Hutchings
Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
gospo@redhat.com
In-Reply-To: <20110926091426.5c83a4d7@nehalam.linuxnetplumber.net>
> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> Sent: Monday, September 26, 2011 9:14 AM
> To: Ben Hutchings
> Cc: Kirsher, Jeffrey T; davem@davemloft.net; Rose, Gregory V;
> netdev@vger.kernel.org; gospo@redhat.com
> Subject: Re: [net-next 05/10] if_link: Add additional parameter to
> IFLA_VF_INFO for spoof checking
>
> On Sun, 25 Sep 2011 21:06:49 +0100
> Ben Hutchings <bhutchings@solarflare.com> wrote:
>
> > On Sun, 2011-09-25 at 10:22 -0700, Stephen Hemminger wrote:
> > > On Sat, 24 Sep 2011 02:17:38 -0700
> > > Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> > >
> > > > From: Greg Rose <gregory.v.rose@intel.com>
> > > >
> > > > Add configuration setting for drivers to turn spoof checking on or
> off
> > > > for discrete VFs.
> > > >
> > > > Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> > > > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > > > ---
> > > > include/linux/if_link.h | 7 +++++++
> > > > include/linux/netdevice.h | 3 +++
> > > > net/core/rtnetlink.c | 25 ++++++++++++++++++++++---
> > > > 3 files changed, 32 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/include/linux/if_link.h b/include/linux/if_link.h
> > > > index 0ee969a..8bd6d6d 100644
> > > > --- a/include/linux/if_link.h
> > > > +++ b/include/linux/if_link.h
> > > > @@ -279,6 +279,7 @@ enum {
> > > > IFLA_VF_MAC, /* Hardware queue specific attributes
> */
> > > > IFLA_VF_VLAN,
> > > > IFLA_VF_TX_RATE, /* TX Bandwidth Allocation */
> > > > + IFLA_VF_SPOOFCHK, /* Spoof Checking on/off switch */
> > > > __IFLA_VF_MAX,
> > > > };
> > > >
> > > > @@ -300,12 +301,18 @@ struct ifla_vf_tx_rate {
> > > > __u32 rate; /* Max TX bandwidth in Mbps, 0 disables throttling
> */
> > > > };
> > > >
> > > > +struct ifla_vf_spoofchk {
> > > > + __u32 vf;
> > > > + __u32 setting;
> > > > +};
> > > > +
> > > > struct ifla_vf_info {
> > > > __u32 vf;
> > > > __u8 mac[32];
> > > > __u32 vlan;
> > > > __u32 qos;
> > > > __u32 tx_rate;
> > > > + __u32 spoofchk;
> > > > };
> > >
> > > This breaks ABI compatibility, unless you add some special case code
> > > to handle the case of tools sending the old ifla_vf_info. Users may
> have older version
> > > of ip utilities that send smaller size structure.
> >
> > Unless I'm missing something, that structure is not sent or received by
> > userland; that's why I thought it should be #ifdef __KERNEL__.
>
> The struct ifla_vf_info is exposed to userland, it is not inside the
> #ifdef
> and therefore exposed.
>
> But it is probably okay to change this structure because the ifla_vf_info
> is not used/supported by any released version iproute2. There may have
> been
> some patches to use this, but they never made it into the git or released
> code.
>
I will respin the patches with *ifdef __KERNEL__ wrapper.
Thanks,
- Greg
^ permalink raw reply
* Re: Question about memory leak detector giving false positive report for net/core/flow.c
From: Eric Dumazet @ 2011-09-26 16:32 UTC (permalink / raw)
To: Huajun Li
Cc: Catalin Marinas, linux-mm, netdev, linux-kernel, Tejun Heo,
Christoph Lameter
In-Reply-To: <CA+v9cxadZzWr35Q9RFzVgk_NZsbZ8PkVLJNxjBAMpargW9Lm4Q@mail.gmail.com>
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 ?
>
> BTW, I wrote a simple test code to emulate net/core/flow.c behavior at
> this stage which triggers the report, and it could also make kmemleak
> give similar report, please check below test code:
>
> kernel version:
> #uname -a
> Linux 3.1.0-rc7 #22 SMP Tue Sep 26 05:43:01 CST 2011 x86_64 x86_64
> x86_64 GNU/Linux
>
> memory leak report:
> -------------------------------------------------------------------------------------------
> unreferenced object 0xffff880073a70000 (size 8192):
> comm "swapper", pid 1, jiffies 4294937832 (age 445.740s)
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<ffffffff8124db64>] create_object+0x144/0x360
> [<ffffffff8191192e>] kmemleak_alloc+0x7e/0x110
> [<ffffffff81235b26>] __kmalloc_node+0x156/0x3a0
> [<ffffffff81935512>] flow_cache_cpu_prepare.clone.1+0x58/0xc0
> [<ffffffff8214c361>] flow_cache_init_global+0xb6/0x1af
> [<ffffffff8100225d>] do_one_initcall+0x4d/0x260
> [<ffffffff820ec2e9>] kernel_init+0x161/0x23a
> [<ffffffff8194ab04>] kernel_thread_helper+0x4/0x10
> [<ffffffffffffffff>] 0xffffffffffffffff
> unreferenced object 0xffff880073a74290 (size 8192):
> comm "swapper", pid 1, jiffies 4294937832 (age 445.740s)
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<ffffffff8124db64>] create_object+0x144/0x360
> [<ffffffff8191192e>] kmemleak_alloc+0x7e/0x110
> [<ffffffff81235b26>] __kmalloc_node+0x156/0x3a0
> [<ffffffff81935512>] flow_cache_cpu_prepare.clone.1+0x58/0xc0
> [<ffffffff8214c361>] flow_cache_init_global+0xb6/0x1af
> [<ffffffff8100225d>] do_one_initcall+0x4d/0x260
> [<ffffffff820ec2e9>] kernel_init+0x161/0x23a
> [<ffffffff8194ab04>] kernel_thread_helper+0x4/0x10
> [<ffffffffffffffff>] 0xffffffffffffffff
>
>
>
> Simple test code to reproduce a similar report:
> -----------------------------------------------------------------------------------------
> MODULE_LICENSE("GPL");
>
> struct test {
> int *pt;
char spaceholder[30000];
> };
>
> static struct test __percpu *percpu;
>
> static int __init test_init(void)
> {
> int i;
>
> percpu = alloc_percpu(struct test);
> if (!percpu)
> return -ENOMEM;
>
> for_each_online_cpu(i) {
> struct test *p = per_cpu_ptr(percpu, i);
> p->pt = kmalloc(sizeof(int), GFP_KERNEL);
> }
>
> return 0;
> }
>
> static void __exit test_exit(void)
> {
> int i;
>
> for_each_possible_cpu(i) {
> struct test *p = per_cpu_ptr(percpu, i);
> if (p->pt)
> kfree(p->pt);
> }
>
> if (percpu)
> free_percpu(percpu);
> }
> module_init(test_init);
> module_exit(test_exit);
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.
--
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
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;
as well as URLs for NNTP newsgroup(s).