Netdev List
 help / color / mirror / Atom feed
* 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

* Re: [net-next 05/10] if_link: Add additional parameter to IFLA_VF_INFO for spoof checking
From: Stephen Hemminger @ 2011-09-26 16:37 UTC (permalink / raw)
  To: Rose, Gregory V
  Cc: Ben Hutchings, Kirsher, Jeffrey T, davem@davemloft.net,
	netdev@vger.kernel.org, gospo@redhat.com
In-Reply-To: <43F901BD926A4E43B106BF17856F0755019C5193B4@orsmsx508.amr.corp.intel.com>

On Mon, 26 Sep 2011 09:32:35 -0700
"Rose, Gregory V" <gregory.v.rose@intel.com> wrote:

> > -----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
> 

Just make spoofchk a new nested attribute.

^ permalink raw reply

* RE: [net-next] iproute2: Add new command to IP link to enable/disable VF spoof check
From: Ben Hutchings @ 2011-09-26 16:41 UTC (permalink / raw)
  To: Rose, Gregory V
  Cc: Stephen Hemminger, Kirsher, Jeffrey T, davem@davemloft.net,
	netdev@vger.kernel.org, gospo@redhat.com
In-Reply-To: <43F901BD926A4E43B106BF17856F0755019C519385@orsmsx508.amr.corp.intel.com>

On Mon, 2011-09-26 at 09:21 -0700, Rose, Gregory V wrote:
> > -----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.

Where is this structure used on the wire?  I just don't see it.

If the modified iproute2 crashes on older kernels, that's presumably
because it's not correctly handling the case where IFLA_VF_SPOOFCHK is
missing.  Nothing to do with the size of this structure.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: intel 82599 multi-port performance
From: Chris Friesen @ 2011-09-26 16:40 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: frog1120, J.Hwan.Kim, netdev, Kirsher, Jeffrey T,
	Brandeburg, Jesse, e1000-devel@lists.sourceforge.net
In-Reply-To: <4E80A2AB.2040206@intel.com>

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?

Chris

-- 
Chris Friesen
Software Developer
GENBAND
chris.friesen@genband.com
www.genband.com

^ 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:42 UTC (permalink / raw)
  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
In-Reply-To: <43F901BD926A4E43B106BF17856F0755019C5193AF@orsmsx508.amr.corp.intel.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.
> 

Sounds fair. Did you CC them?

> > 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.
> 
> - Greg

^ permalink raw reply

* RE: [net-next 1/7] bna: Brocade 1860 IOC PLL, Reg Defs and ASIC Mode Changes
From: Rasesh Mody @ 2011-09-26 16:43 UTC (permalink / raw)
  To: Joe Perches
  Cc: davem@davemloft.net, netdev@vger.kernel.org,
	Adapter Linux Open SRC Team, Gurunatha Karaje
In-Reply-To: <1316831427.28330.17.camel@Joe-Laptop>

>From: Joe Perches [mailto:joe@perches.com]
>Sent: Friday, September 23, 2011 7:30 PM
>
>trivia...

Hi Joe,
We'll re-submit this patch-set addressing the comments.
Thanks,
Rasesh


^ permalink raw reply

* Re: [net-next] iproute2: Add new command to IP link to enable/disable VF spoof check
From: Stephen Hemminger @ 2011-09-26 16:47 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Rose, Gregory V, Kirsher, Jeffrey T, davem@davemloft.net,
	netdev@vger.kernel.org, gospo@redhat.com
In-Reply-To: <1317055319.2773.41.camel@bwh-desktop>

On Mon, 26 Sep 2011 17:41:59 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:

> On Mon, 2011-09-26 at 09:21 -0700, Rose, Gregory V wrote:
> > > -----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.
> 
> Where is this structure used on the wire?  I just don't see it.
> 
> If the modified iproute2 crashes on older kernels, that's presumably
> because it's not correctly handling the case where IFLA_VF_SPOOFCHK is
> missing.  Nothing to do with the size of this structure.

The proposed code is fine because the vf_info is not used by any code
in iproute right now and I doubt that any other tools are using it either.

^ permalink raw reply

* Re: Question about memory leak detector giving false positive report for net/core/flow.c
From: Catalin Marinas @ 2011-09-26 16:50 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Huajun Li, linux-mm@kvack.org, netdev, linux-kernel, Tejun Heo,
	Christoph Lameter
In-Reply-To: <1317054774.6363.9.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>

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);

-- 
Catalin

--
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

* 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:57 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
	gospo@redhat.com
In-Reply-To: <20110926092114.05e99094@nehalam.linuxnetplumber.net>

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Stephen Hemminger
> Sent: Monday, September 26, 2011 9:21 AM
> To: Rose, Gregory V
> Cc: Kirsher, Jeffrey T; davem@davemloft.net; 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 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 ...

That is what happens when you set the option on older kernels that don't support it.  In the do_setvfinfo() function in ../net/core/rtnetlink.c the default return value is EINVAL.  If the case for IFLA_VF_SPOOFCHK doesn't exist then it falls through and returns with the error value.

It's when you do the ip link show <dev> command on an older kernel that doesn't report the spoof check value that you get the segmentation fault in the ip tool.  I'll look at that and see what I can do to fix that up and respin the patch.

- Greg

^ 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:58 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: <20110926094719.0cb6c296@nehalam.linuxnetplumber.net>

> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> Sent: Monday, September 26, 2011 9:47 AM
> To: Ben Hutchings
> Cc: Rose, Gregory V; Kirsher, Jeffrey T; 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 Mon, 26 Sep 2011 17:41:59 +0100
> Ben Hutchings <bhutchings@solarflare.com> wrote:
> 
> > On Mon, 2011-09-26 at 09:21 -0700, Rose, Gregory V wrote:
> > > > -----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.
> >
> > Where is this structure used on the wire?  I just don't see it.
> >
> > If the modified iproute2 crashes on older kernels, that's presumably
> > because it's not correctly handling the case where IFLA_VF_SPOOFCHK is
> > missing.  Nothing to do with the size of this structure.
> 
> The proposed code is fine because the vf_info is not used by any code
> in iproute right now and I doubt that any other tools are using it either.

As I mentioned in another post to this thread the ip tool works fine on older kernels when you do the set operation.  It's when you run the ip link show command that you can get a seg fault because the ip tool is looking for a spoof check value set by the kernel that doesn't exist.  I'll see what I can do to fix that up and respin the patch.

- Greg

^ permalink raw reply

* RE: [RFC/PATCH] net: add deferred flow cache flush function to be
From: Madalin Bucur @ 2011-09-26 16:58 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, davem, timo.teras, Madalin Bucur

I've investigated this issue further and come up with a different fix -
modify the flow_cache_flush to prevent it from sleeping and also remove the
deferral of the actual gc work through schedule_work. That caused the gc never
to happen on a highly loaded system that had the cpus occupied servicing
network softirqs.

During the investigation I've also found two places the return value of 
dst_alloc was used without checking, leading to a crash. 

The 3 patches will follow.


> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On 
> Behalf Of Eric Dumazet
>
> Problem is sometime we dont want to defer and are in process context.
> 
> xfrm_dev_event(  event == NETDEV_DOWN) is probably a place we dont want
> to defer the garbage collection.

^ permalink raw reply

* 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox