Netdev List
 help / color / mirror / Atom feed
* Re: what's causing "ip_rt_bug"?
From: Julian Anastasov @ 2011-06-28  8:13 UTC (permalink / raw)
  To: David Miller; +Cc: mangoo, eric.dumazet, netdev, bazsi, hidden
In-Reply-To: <20110627.205544.111681152997205782.davem@davemloft.net>


	Hello,

On Mon, 27 Jun 2011, David Miller wrote:

> From: Julian Anastasov <ja@ssi.bg>
> Date: Sat, 18 Jun 2011 20:53:59 +0300 (EEST)
> 
> > 	Hm, if it happens "sometimes", can it be some
> > problem with tproxy and TIME_WAIT sockets? I see that
> > tproxy_sk_is_transparent has special treatment for TW
> > sockets while ip_route_me_harder is different. As result,
> > may be input route is assigned for TW packets.
> > 
> > 	May be inet_sk_flowi_flags() needs fixing, not
> > sure. But following patch is first step to fix this
> > problem. I don't have setup to test this patch.
> 
> TPROXY has special code to make sure that time-wait sockets
> are not assigned to skb->sk, as explained in commit
> d503b30bd648b3cb4e5f50b65d27e389960cc6d9, that would cause
> all kinds of crashes in nfnetlink_log etc.
> 
> Therefore we would see skb->sk==NULL at ip_route_me_harder()
> in that case.

	Aha, after this clarification other changes should not
be needed. If saddr is translated, now we will use
FLOWI_FLAG_ANYSRC. As result, if SNAT happens one day in
LOCAL_OUT, the new saddr can be unicast because RTN_UNSPEC
is provided for addr_type. If saddr is not changed, it
should be already validated when the first route for skb is
performed, so TPROXY should work.

> > ===========================================================
> > 
> > 	Avoid creating input routes with ip_route_me_harder.
> > It does not work for locally generated packets. Instead,
> > restrict sockets to provide valid saddr for output route (or
> > unicast saddr for transparent proxy). For other traffic
> > allow saddr to be unicast or local but if callers forget
> > to check saddr type use 0 for the output route.
> > 
> > 	The resulting handling should be:
> > 
> > - REJECT TCP:
> > 	- in INPUT we can provide addr_type = RTN_LOCAL but
> > 	better allow rejecting traffic delivered with
> > 	local route (no IP address => use RTN_UNSPEC to
> > 	allow also RTN_UNICAST).
> > 	- FORWARD: RTN_UNSPEC => allow RTN_LOCAL/RTN_UNICAST
> > 	saddr, add fix to ignore RTN_BROADCAST and RTN_MULTICAST
> > 	- OUTPUT: RTN_UNSPEC
> > 
> > - NAT, mangle, ip_queue, nf_ip_reroute: RTN_UNSPEC in LOCAL_OUT
> > 
> > - IPVS:
> > 	- use RTN_LOCAL in LOCAL_OUT and FORWARD after SNAT
> > 	to restrict saddr to be local
> > 
> > Signed-off-by: Julian Anastasov <ja@ssi.bg>
> 
> Unless someone gives some negative feedback soon I'm going to
> apply this.

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* Re: [RFC 02/72] 3c*/acenic/typhoon: Move 3Com Ethernet drivers
From: Alan Cox @ 2011-06-28  8:22 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: davem, netdev, Philip Blundell, Steffen Klassert, David Dillow,
	Jes Sorensen, Donald Becker, Craig Southeren, David Hinds
In-Reply-To: <1309010363-22750-3-git-send-email-jeffrey.t.kirsher@intel.com>

On Sat, 25 Jun 2011 06:58:13 -0700
Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:

> Moves the 3Com drivers into drivers/net/ethernet/3com/ and the
> necessary Kconfig and Makefile changes.

This seems a bizarre way to carve stuff up.

To start with several of the things you put in 3com are just branded
items, or are boards using non 3com devices which are currently with
the other drivers that do so, some even use shared midlayers that you
move them away from.

You also pull stuff out of pcmcia which makes it harder to scan stuff
grouped logically.

So this all seems a bit daft to me.

Surely it would make sense to put all the Intel 825xx stuff together,
all the 8390 stuff together and so on.

Alan

^ permalink raw reply

* Re: what's causing "ip_rt_bug"?
From: Tomasz Chmielewski @ 2011-06-28  8:30 UTC (permalink / raw)
  To: David Miller; +Cc: ja, eric.dumazet, netdev, bazsi, hidden
In-Reply-To: <20110627.205544.111681152997205782.davem@davemloft.net>

On 28.06.2011 05:55, David Miller wrote:

>> 	The resulting handling should be:
>>
>> - REJECT TCP:
>> 	- in INPUT we can provide addr_type = RTN_LOCAL but
>> 	better allow rejecting traffic delivered with
>> 	local route (no IP address =>  use RTN_UNSPEC to
>> 	allow also RTN_UNICAST).
>> 	- FORWARD: RTN_UNSPEC =>  allow RTN_LOCAL/RTN_UNICAST
>> 	saddr, add fix to ignore RTN_BROADCAST and RTN_MULTICAST
>> 	- OUTPUT: RTN_UNSPEC
>>
>> - NAT, mangle, ip_queue, nf_ip_reroute: RTN_UNSPEC in LOCAL_OUT
>>
>> - IPVS:
>> 	- use RTN_LOCAL in LOCAL_OUT and FORWARD after SNAT
>> 	to restrict saddr to be local
>>
>> Signed-off-by: Julian Anastasov<ja@ssi.bg>
>
> Unless someone gives some negative feedback soon I'm going to
> apply this.

Can you tell me where it will be pushed?

I.e. 3.x kernels only, or does it have a chance to go into 2.6.39.x?


-- 
Tomasz Chmielewski
http://wpkg.org

^ permalink raw reply

* Re: what's causing "ip_rt_bug"?
From: David Miller @ 2011-06-28  8:40 UTC (permalink / raw)
  To: mangoo; +Cc: ja, eric.dumazet, netdev, bazsi, hidden
In-Reply-To: <4E099113.6000801@wpkg.org>

From: Tomasz Chmielewski <mangoo@wpkg.org>
Date: Tue, 28 Jun 2011 10:30:11 +0200

> Can you tell me where it will be pushed?
> 
> I.e. 3.x kernels only, or does it have a chance to go into 2.6.39.x?

I'll apply it for 3.0.0 and also queue it up for -stable.

^ permalink raw reply

* Re: what's causing "ip_rt_bug"?
From: David Miller @ 2011-06-28  8:41 UTC (permalink / raw)
  To: ja; +Cc: mangoo, eric.dumazet, netdev, bazsi, hidden
In-Reply-To: <alpine.LFD.2.00.1106281029490.1880@ja.ssi.bg>

From: Julian Anastasov <ja@ssi.bg>
Date: Tue, 28 Jun 2011 11:13:25 +0300 (EEST)

> On Mon, 27 Jun 2011, David Miller wrote:
> 
>> TPROXY has special code to make sure that time-wait sockets
>> are not assigned to skb->sk, as explained in commit
>> d503b30bd648b3cb4e5f50b65d27e389960cc6d9, that would cause
>> all kinds of crashes in nfnetlink_log etc.
>> 
>> Therefore we would see skb->sk==NULL at ip_route_me_harder()
>> in that case.
> 
> 	Aha, after this clarification other changes should not
> be needed.

By this do you mean that you think your patch in this thread
is completely sufficient?

^ permalink raw reply

* Re: what's causing "ip_rt_bug"?
From: Julian Anastasov @ 2011-06-28  9:05 UTC (permalink / raw)
  To: David Miller; +Cc: mangoo, eric.dumazet, netdev, bazsi, hidden
In-Reply-To: <20110628.014117.92807428115239742.davem@davemloft.net>


	Hello,

On Tue, 28 Jun 2011, David Miller wrote:

> From: Julian Anastasov <ja@ssi.bg>
> Date: Tue, 28 Jun 2011 11:13:25 +0300 (EEST)
> 
> > On Mon, 27 Jun 2011, David Miller wrote:
> > 
> >> TPROXY has special code to make sure that time-wait sockets
> >> are not assigned to skb->sk, as explained in commit
> >> d503b30bd648b3cb4e5f50b65d27e389960cc6d9, that would cause
> >> all kinds of crashes in nfnetlink_log etc.
> >> 
> >> Therefore we would see skb->sk==NULL at ip_route_me_harder()
> >> in that case.
> > 
> > 	Aha, after this clarification other changes should not
> > be needed.
> 
> By this do you mean that you think your patch in this thread
> is completely sufficient?

	Yes. My worry was for the skb->sk != NULL not being
handled by inet_sk_flowi_flags for TW sockets. But it seems
it is not needed, so the patch in this form should be ok.

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* Re: SKB paged fragment lifecycle on receive
From: Ian Campbell @ 2011-06-28 10:24 UTC (permalink / raw)
  To: David Miller
  Cc: netdev@vger.kernel.org, jeremy@goop.org,
	xen-devel@lists.xensource.com, eric.dumazet@gmail.com,
	rusty@rustcorp.com.au
In-Reply-To: <20110627.154921.1544030815492828408.davem@davemloft.net>

On Mon, 2011-06-27 at 23:49 +0100, David Miller wrote:
> From: Ian Campbell <Ian.Campbell@eu.citrix.com>
> Date: Mon, 27 Jun 2011 15:42:04 +0100
> 
> > However it seems like this might still have a problem if your SKBs are
> > ever cloned. What happens in this case, e.g if a user of AF_PACKET sends
> > a broadcast via a device associated with a bridge[1] (where it would be
> > flooded)?
> 
> You don't need a bridge to get a clone on transmit, the packet
> scheduler can do clones.  Just grep for skb_clone in the packet
> action handlers net/sched/act_*.c

Are you sure? I only see skb_cloned() and skb_clone_writeable() under
there )(3.0-rc4) and not any actual skb_clone()s.

The only actual clone I see under there is in net/sched/sch_netem.c.

However it sounds like it is expected that a clone can happen on pretty
any skb which makes the frag lifecycle issue seem like one which could
effect anything which sends a page to the network without relinquishing
complete control of it (common in any kind of zero-copy scenario).

Ian.

^ permalink raw reply

* Re: SKB paged fragment lifecycle on receive
From: Ian Campbell @ 2011-06-28 10:25 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: netdev@vger.kernel.org, rusty@rustcorp.com.au,
	xen-devel@lists.xensource.com, David Miller,
	eric.dumazet@gmail.com
In-Reply-To: <4E08ED69.3010709@goop.org>

On Mon, 2011-06-27 at 21:51 +0100, Jeremy Fitzhardinge wrote:
> On 06/25/2011 12:58 PM, Ian Campbell wrote:
> > On Fri, 2011-06-24 at 13:11 -0700, Jeremy Fitzhardinge wrote:
> >> On 06/24/2011 12:46 PM, David Miller wrote:
> >>> Pages get transferred between different SKBs all the time.
> >>>
> >>> For example, GRO makes extensive use of this technique.
> >>> See net/core/skbuff.c:skb_gro_receive().
> >>>
> >>> It is just one example.
> >> I see, and the new skb doesn't get a destructor copied from the
> >> original, so there'd be no second callback.
> > What about if we were to have a per-shinfo destructor (called once for
> > each page as its refcount goes 1->0, from whichever skb ends up with the
> > last ref) as well as the skb-destructors.
> 
> We never want the refcount for granted pages to go from 1 -> 0.  The
> safest thing is to make sure we always elevate the refcount to make sure
> that nothing else can ever drop the last ref.

I guess I meant called just after the refcount goes 2->1 or something.
_But_... thinking about it some more I don't think this scheme works in
the general case because the entity injecting into the network stack may
not be the only reference count holder.

Consider the NFS case -- in that case there is already a reference
because the page belongs to userspace. You certainly don't want to wait
for the process to exit before considering the page done with.

You may also have multiple reference counts due to multiple threads
doing I/O on (overlapping or not) portions of the same page etc.

So now we're into the realms of a shinfo per frag reference count and
destructor, which reintroduces the whole question of what happens if a
page is copied/moved to another shinfo.

Could we add a per-frag pointer to the shinfo pointing to:
	struct {
		atomic_t ref;
		void (destructor*)(struct skb, int fragnr, /*TBD*/);
		void *data; /* user data, maybe */
	};
This data structure is owned by whoever injected the page into the stack
and the pointer to it is propagated everywhere that page goes, including
across skb splits and pages moving or copied between skbs etc etc.

The split between ref counting on this and struct page needs a bit of
careful thought but perhaps it's IFF this struct exists the stack will
hold a single struct page refcount and use this new refcount for
everything else, dropping the struct page refcount when the frag
refcount hits 0. Otherwise it simply uses struct page refcount as
normal.

> If we can trust the network stack to always do the last release (and not
> hand it off to something else to do it), then we could have a destructor
> which gets called before the last ref drop (or leaves the ref drop to
> the destructor to do), and do everything required that way.  But it
> seems pretty fragile.  At the very least it would need a thorough code
> audit to make sure that everything handles page lifetimes in the
> expected way - but then I'd still worry about out-of-tree patches
> breaking something in subtle ways.
> 
> >  This already handles the
> > cloning case but when pages are moved between shinfo then would it make
> > sense for that to be propagated between skb's under these circumstances
> > and/or require them to be the same? Since in the case of something like
> > skb_gro_receive the skbs (and hence the frag array pages) are all from
> > the same 'owner' (even if the skb is actually created by the stack on
> > their behalf) I suspect this could work?
> >
> > But I bet this assumption isn't valid in all cases.
> 
> Hm.

e.g. if you have some sort of tunnel protocol which is encapsulating
multiple skb streams into a single one might you get an skb with pages
from multiple sources?

> > In which case I end up wondering about a destructor per page in the frag
> > array. At which point we might as well consider it as a part of the core
> > mm stuff rather than something net specific?
> 
> Doing it generically still needs some kind of marker that the page has a
> special-case destructor (and the destructor pointer itself).

True. you only really need the destructor pointer (which can be NULL)
but yes, it would add stuff to struct page which may not be useful
elsewhere.

Ian.

^ permalink raw reply

* Re: [PATCH net-next-2.6 2/2] jme: Cleanup PM operations after using new PM API
From: Guo-Fu Tseng @ 2011-06-28 11:19 UTC (permalink / raw)
  To: Guo-Fu Tseng, David Miller; +Cc: linux-netdev, Aries Lee, Devinchiu
In-Reply-To: <1309238615-25590-2-git-send-email-cooldavid@cooldavid.org>

On Tue, 28 Jun 2011 13:23:35 +0800, Guo-Fu Tseng wrote
> From: Guo-Fu Tseng <cooldavid@cooldavid.org>
> 
> 	1. Using enum name instead of numeric value.
> 	2. device_set_wakeup_enable expect bool argument
> 	   adding !!() to the argument to be passed.
> 	3. Remove non-hardware related operations from
> 	   jme_clear_pm()
> 	4. Reuse jme_clear_pm() in jme_resume()
> 	5. Clear wakeup event indicator bits(call jme_clear_pm())
> 	   before going to sleep.
> 	6. Check for wakeup setting while shutdown
> 		Turn off PHY if wakeup is not enabled.
> 		Power-safe PHY(lower speed) if wakeup is enabled.
> 
> Signed-off-by: Guo-Fu Tseng <cooldavid@cooldavid.org>
Dear Davem:

Please ignore this patch.
I'll resend a cleaner version later.

Sorry for the inconvenience.

Guo-Fu Tseng


^ permalink raw reply

* [PATCH net-next-2.6] jme: Cleanup PM operations after using new PM API
From: Guo-Fu Tseng @ 2011-06-28 11:38 UTC (permalink / raw)
  To: David Miller; +Cc: Guo-Fu Tseng, linux-netdev, Aries Lee, Devinchiu

From: Guo-Fu Tseng <cooldavid@cooldavid.org>

	1. Using enum name instead of numeric value.
	2. device_set_wakeup_enable expect bool argument
	   adding !!() to the argument to be passed.
	3. Remove non-jme-hardware related operations from
	   jme_clear_pm()
	4. Reuse jme_clear_pm() in jme_resume() and jme_powersave_phy()

Signed-off-by: Guo-Fu Tseng <cooldavid@cooldavid.org>
---
 drivers/net/jme.c |   23 +++++++++++------------
 drivers/net/jme.h |    2 ++
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index 2ce9339..6b2a5e7 100644
--- a/drivers/net/jme.c
+++ b/drivers/net/jme.c
@@ -271,9 +271,7 @@ jme_reset_mac_processor(struct jme_adapter *jme)
 static inline void
 jme_clear_pm(struct jme_adapter *jme)
 {
-	jwrite32(jme, JME_PMCS, 0xFFFF0000 | jme->reg_pmcs);
-	pci_set_power_state(jme->pdev, PCI_D0);
-	device_set_wakeup_enable(&jme->pdev->dev, false);
+	jwrite32(jme, JME_PMCS, PMCS_STMASK | jme->reg_pmcs);
 }
 
 static int
@@ -1817,11 +1815,9 @@ jme_powersave_phy(struct jme_adapter *jme)
 {
 	if (jme->reg_pmcs) {
 		jme_set_100m_half(jme);
-
 		if (jme->reg_pmcs & (PMCS_LFEN | PMCS_LREN))
 			jme_wait_link(jme);
-
-		jwrite32(jme, JME_PMCS, jme->reg_pmcs);
+		jme_clear_pm(jme);
 	} else {
 		jme_phy_off(jme);
 	}
@@ -2529,8 +2525,7 @@ jme_set_wol(struct net_device *netdev,
 		jme->reg_pmcs |= PMCS_MFEN;
 
 	jwrite32(jme, JME_PMCS, jme->reg_pmcs);
-
-	device_set_wakeup_enable(&jme->pdev->dev, jme->reg_pmcs);
+	device_set_wakeup_enable(&jme->pdev->dev, !!(jme->reg_pmcs));
 
 	return 0;
 }
@@ -3058,6 +3053,9 @@ jme_init_one(struct pci_dev *pdev,
 	jme->mii_if.mdio_write = jme_mdio_write;
 
 	jme_clear_pm(jme);
+	pci_set_power_state(jme->pdev, PCI_D0);
+	device_set_wakeup_enable(&pdev->dev, true);
+
 	jme_set_phyfifo_5level(jme);
 	jme->pcirev = pdev->revision;
 	if (!jme->fpgaver)
@@ -3136,7 +3134,8 @@ jme_shutdown(struct pci_dev *pdev)
 }
 
 #ifdef CONFIG_PM_SLEEP
-static int jme_suspend(struct device *dev)
+static int
+jme_suspend(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct net_device *netdev = pci_get_drvdata(pdev);
@@ -3175,14 +3174,14 @@ static int jme_suspend(struct device *dev)
 	return 0;
 }
 
-static int jme_resume(struct device *dev)
+static int
+jme_resume(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct jme_adapter *jme = netdev_priv(netdev);
 
-	jwrite32(jme, JME_PMCS, 0xFFFF0000 | jme->reg_pmcs);
-
+	jme_clear_pm(jme);
 	jme_phy_on(jme);
 	if (test_bit(JME_FLAG_SSET, &jme->flags))
 		jme_set_settings(netdev, &jme->old_ecmd);
diff --git a/drivers/net/jme.h b/drivers/net/jme.h
index 0d5da06..1481a62 100644
--- a/drivers/net/jme.h
+++ b/drivers/net/jme.h
@@ -852,6 +852,7 @@ enum jme_ghc_txmac_clk {
  * Power management control and status register
  */
 enum jme_pmcs_bit_masks {
+	PMCS_STMASK	= 0xFFFF0000,
 	PMCS_WF7DET	= 0x80000000,
 	PMCS_WF6DET	= 0x40000000,
 	PMCS_WF5DET	= 0x20000000,
@@ -863,6 +864,7 @@ enum jme_pmcs_bit_masks {
 	PMCS_LFDET	= 0x00040000,
 	PMCS_LRDET	= 0x00020000,
 	PMCS_MFDET	= 0x00010000,
+	PMCS_ENMASK	= 0x0000FFFF,
 	PMCS_WF7EN	= 0x00008000,
 	PMCS_WF6EN	= 0x00004000,
 	PMCS_WF5EN	= 0x00002000,
-- 
1.7.3.4


^ permalink raw reply related

* lost gARP after live migration
From: Laszlo Ersek @ 2011-06-28 13:01 UTC (permalink / raw)
  To: xen-devel@lists.xensource.com; +Cc: netdev, Paolo Bonzini

Hi,

with reference to RHBZ#713585:

It seems when a RHEL-6.1 or F-15 Xen PV guest is live migrated, the 
gratuitous ARP packet is not forwarded to the affected "networking 
equipment". The netback vif is added to a routed bridge in the host(s) 
and external hosts are expeted to have connection to the guest at all 
times, no matter the current Xen host.

I experimented a bit with tcpdump, and the gARP does appear on the 
netfront interface. It also appears on the host bridge if sufficient 
time passes between completing the xenbus handshake and sending the gARP.

When the guest queues eg. three gARPs in rapid succession, a variable 
number of them gets lost. (When all such packets disappear, then the 
migrated guest becomes invisible to the outside world, until it 
initiates network traffic on its own.)

When the guest waits for about half a second before sending (queueing), 
the very first gARP packet successfully appears on the host bridge.

I suspect it's a timing race against the netback vif being added to the 
host bridge. What would be a good countermeasure?

- Adding two modparams to xen-netfront (gARP requeue count & number of 
msecs to wait between queueing the gARPs).
- (Paolo's idea:) watching the "hotplug-status" xenstore node and 
sending a single gARP when the watch fires with "connected". This node 
belongs to the backend xenstore subtree, thus watching it from the guest 
doesn't please the architecture astronaut in me.
- Something else.

Sorry for the naivety / verbiage.

Thanks,
lacos

^ permalink raw reply

* Re: lost gARP after live migration
From: Paolo Bonzini @ 2011-06-28 13:03 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: netdev, xen-devel@lists.xensource.com
In-Reply-To: <4E09D0A0.1080107@redhat.com>

On 06/28/2011 03:01 PM, Laszlo Ersek wrote:
> - (Paolo's idea:) watching the "hotplug-status" xenstore node and
> sending a single gARP when the watch fires with "connected". This node
> belongs to the backend xenstore subtree, thus watching it from the guest
> doesn't please the architecture astronaut in me.

Note that watching the backend and reading its information is quite 
common.  In fact, that's how the state of the backend is observed in the 
first place.  Of course you cannot write to the backend tree, but you do 
not have to do that.

Paolo

^ permalink raw reply

* [PATCH] Exclude duplicated checking for iface-up. This flags is checked in 'is_skb_forwardable' function, which is subroutine of 'dev_forward_skb'.
From: alex.bluesman.smirnov @ 2011-06-28 13:30 UTC (permalink / raw)
  To: davem; +Cc: netdev, Alexander Smirnov

From: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>

Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
---
 drivers/net/veth.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 3b99f64..fb2fd37 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -163,9 +163,6 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 	stats = this_cpu_ptr(priv->stats);
 	rcv_stats = this_cpu_ptr(rcv_priv->stats);
 
-	if (!(rcv->flags & IFF_UP))
-		goto tx_drop;
-
 	/* don't change ip_summed == CHECKSUM_PARTIAL, as that
 	   will cause bad checksum on forwarded packets */
 	if (skb->ip_summed == CHECKSUM_NONE)
-- 
1.7.2.3


^ permalink raw reply related

* Re: lost gARP after live migration
From: Ian Campbell @ 2011-06-28 13:33 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: netdev@vger.kernel.org, xen-devel@lists.xensource.com,
	Paolo Bonzini
In-Reply-To: <4E09D0A0.1080107@redhat.com>

On Tue, 2011-06-28 at 14:01 +0100, Laszlo Ersek wrote:
> Hi,
> 
> with reference to RHBZ#713585:
> 
> It seems when a RHEL-6.1 or F-15 Xen PV guest is live migrated, the 
> gratuitous ARP packet is not forwarded to the affected "networking 
> equipment". The netback vif is added to a routed bridge in the host(s) 
> and external hosts are expeted to have connection to the guest at all 
> times, no matter the current Xen host.
> 
> I experimented a bit with tcpdump, and the gARP does appear on the 
> netfront interface. It also appears on the host bridge if sufficient 
> time passes between completing the xenbus handshake and sending the gARP.
> 
> When the guest queues eg. three gARPs in rapid succession, a variable 
> number of them gets lost. (When all such packets disappear, then the 
> migrated guest becomes invisible to the outside world, until it 
> initiates network traffic on its own.)
> 
> When the guest waits for about half a second before sending (queueing), 
> the very first gARP packet successfully appears on the host bridge.
> 
> I suspect it's a timing race against the netback vif being added to the 
> host bridge. What would be a good countermeasure?
> 
> - Adding two modparams to xen-netfront (gARP requeue count & number of 
> msecs to wait between queueing the gARPs).
> - (Paolo's idea:) watching the "hotplug-status" xenstore node and 
> sending a single gARP when the watch fires with "connected". This node 
> belongs to the backend xenstore subtree, thus watching it from the guest 
> doesn't please the architecture astronaut in me.

netback already waits (or should...) for hotplug-status to fire with
"connected" before moving to state XenbusStateConnected. See
hotplug_status_changed in drivers/net/xen-netback/xenbus.c. You need
either the netback in upstream or something newer than 43223efd9bfd (C
Feb 2010) if you are using e.g. xen.git#xen/next-2.6.32. That commit
fixes pretty much the issue you describe.

I expected that netfront waited for the backend to hit
XenbusStateConnected before sending the grat ARP but instead I find it
happens when the backend hits XenbusStateInitWait. I'm not sure if that
is a problem -- it appears to have been done this way since forever
(even back in the classic Xen kernels) and I've never noticed a gARP go
missing in the way you describe, but perhaps something isn't quite
matching up any more.

Ian.

> - Something else.
> 
> Sorry for the naivety / verbiage.
> 
> Thanks,
> lacos
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

^ permalink raw reply

* Re: lost gARP after live migration
From: Ben Hutchings @ 2011-06-28 14:14 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: netdev, xen-devel@lists.xensource.com, Paolo Bonzini
In-Reply-To: <4E09D0A0.1080107@redhat.com>

On Tue, 2011-06-28 at 15:01 +0200, Laszlo Ersek wrote:
[...]
> When the guest waits for about half a second before sending (queueing), 
> the very first gARP packet successfully appears on the host bridge.
> 
> I suspect it's a timing race against the netback vif being added to the 
> host bridge. What would be a good countermeasure?
> 
> - Adding two modparams to xen-netfront (gARP requeue count & number of 
> msecs to wait between queueing the gARPs).

Note that peer notifications are indirected through netdev notifiers and
now include IPv6 NAs as well as ARPs.  If repeated notifications are
commonly necessary then this should probably be handled in the protocol
(or in the networking core).  However this sounds like a workaround
whereas your other option would be a proper fix:

> - (Paolo's idea:) watching the "hotplug-status" xenstore node and 
> sending a single gARP when the watch fires with "connected". This node 
> belongs to the backend xenstore subtree, thus watching it from the guest 
> doesn't please the architecture astronaut in me.
[...]

-- 
Ben Hutchings, Senior Software 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: linux-next: manual merge of the net tree with the wireless-current tree
From: Guy, Wey-Yi @ 2011-06-28 14:57 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: David Miller, netdev@vger.kernel.org, linux-next@vger.kernel.org,
	linux-kernel@vger.kernel.org, Berg, Johannes, John W. Linville,
	Grumbach, Emmanuel
In-Reply-To: <20110628140219.1480dc7a.sfr@canb.auug.org.au>

On Mon, 2011-06-27 at 21:02 -0700, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the net tree got a conflict in
> drivers/net/wireless/iwlwifi/iwl-tx.c between commits 1107a08a1a3e
> ("iwlagn: fix cmd queue unmap"), e815407d395e ("iwlagn: map command
> buffers BIDI") and 2627c002cbed ("iwlagn: use PCI_DMA_* for pci_*
> operations") from the wireless-current tree and commit 795414db8607
> ("iwlagn: don't use the PCI wrappers for DMA operation") from the net
> tree.
> 
> I fixed it up (I think - see below) and can carry the fix as necessary.

Looks right to me. Thanks for fixing it.

Wey

^ permalink raw reply

* Re: [PATCH] bridge: Forward EAPOL Kconfig option BRIDGE_PAE_FORWARD
From: Stephen Hemminger @ 2011-06-28 15:10 UTC (permalink / raw)
  To: David Lamparter; +Cc: Nick Carter, netdev, davem
In-Reply-To: <20110628150257.GB126252@jupiter.n2.diac24.net>

On Tue, 28 Jun 2011 17:02:57 +0200
David Lamparter <equinox@diac24.net> wrote:

> On Sat, Jun 25, 2011 at 12:33:05AM +0100, Nick Carter wrote:
> > @@ -98,6 +98,14 @@ int br_handle_frame_finish(struct sk_buff *skb)
> >  	}
> > 
> >  	if (skb) {
> > +		/* Prevent Crosstalk where a Supplicant on one Port attempts to
> > +		 * interfere with authentications occurring on another Port.
> > +		 * (IEEE Std 802.1X-2001 C.3.3)
> > +		 */
> > +		if (unlikely(!br->pae_forward &&
> > +		    skb->protocol == htons(ETH_P_PAE)))
> > +			goto drop;
> > +
> >  		if (dst)
> >  			br_forward(dst->dst, skb, skb2);
> >  		else
> > @@ -166,6 +174,10 @@ struct sk_buff *br_handle_frame(struct sk_buff *skb)
> >  		if (p->br->stp_enabled == BR_NO_STP && dest[5] == 0)
> >  			goto forward;
> > 
> > +		/* Check if PAE frame should be forwarded */
> > +		if (p->br->pae_forward && skb->protocol == htons(ETH_P_PAE))
> > +			goto forward;
> > +
> >  		if (NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
> >  			    NULL, br_handle_local_finish))
> >  			return NULL;	/* frame consumed by filter */
> 
> No, please don't.
> 
> Linux bridging has two "grand" modes: dumb and STP enabled.
> 
> If we're running a dumb bridge, we behave like an ethernet hub without
> any intelligence, and in that case we should absolutely forward 802.1X
> frames. We may have (e.g. VM) client(s) that want to authenticate with a
> physical switch.
> (For the spec, this counts as "repeater", not "bridge"/"switch")
> 
> If we're running with STP enabled, then 802.1X traffic should already be
> caught by the general ethernet link-local multicast drop (which applies
> to 01:80:c2:/24 and therefore catches 802.1X too.)

The problem is that STP is not enabled by default, and most people don't
know how to enable it.

^ permalink raw reply

* Re: [PATCH] bridge: Forward EAPOL Kconfig option BRIDGE_PAE_FORWARD
From: David Lamparter @ 2011-06-28 15:02 UTC (permalink / raw)
  To: Nick Carter; +Cc: Stephen Hemminger, netdev, davem
In-Reply-To: <BANLkTimD+fvhrhpc+kPXt1qMnQvVi2dw=Q@mail.gmail.com>

On Sat, Jun 25, 2011 at 12:33:05AM +0100, Nick Carter wrote:
> @@ -98,6 +98,14 @@ int br_handle_frame_finish(struct sk_buff *skb)
>  	}
> 
>  	if (skb) {
> +		/* Prevent Crosstalk where a Supplicant on one Port attempts to
> +		 * interfere with authentications occurring on another Port.
> +		 * (IEEE Std 802.1X-2001 C.3.3)
> +		 */
> +		if (unlikely(!br->pae_forward &&
> +		    skb->protocol == htons(ETH_P_PAE)))
> +			goto drop;
> +
>  		if (dst)
>  			br_forward(dst->dst, skb, skb2);
>  		else
> @@ -166,6 +174,10 @@ struct sk_buff *br_handle_frame(struct sk_buff *skb)
>  		if (p->br->stp_enabled == BR_NO_STP && dest[5] == 0)
>  			goto forward;
> 
> +		/* Check if PAE frame should be forwarded */
> +		if (p->br->pae_forward && skb->protocol == htons(ETH_P_PAE))
> +			goto forward;
> +
>  		if (NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
>  			    NULL, br_handle_local_finish))
>  			return NULL;	/* frame consumed by filter */

No, please don't.

Linux bridging has two "grand" modes: dumb and STP enabled.

If we're running a dumb bridge, we behave like an ethernet hub without
any intelligence, and in that case we should absolutely forward 802.1X
frames. We may have (e.g. VM) client(s) that want to authenticate with a
physical switch.
(For the spec, this counts as "repeater", not "bridge"/"switch")

If we're running with STP enabled, then 802.1X traffic should already be
caught by the general ethernet link-local multicast drop (which applies
to 01:80:c2:/24 and therefore catches 802.1X too.)


-David


^ permalink raw reply

* Re: [PATCH] bridge: Forward EAPOL Kconfig option BRIDGE_PAE_FORWARD
From: David Lamparter @ 2011-06-28 16:00 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Lamparter, Nick Carter, netdev, davem
In-Reply-To: <20110628081015.1b06a3f0@nehalam.ftrdhcpuser.net>

On Tue, Jun 28, 2011 at 08:10:15AM -0700, Stephen Hemminger wrote:
> On Tue, 28 Jun 2011 17:02:57 +0200
> David Lamparter <equinox@diac24.net> wrote:
> > >  	if (skb) {
> > > +		/* Prevent Crosstalk where a Supplicant on one Port attempts to
> > > +		 * interfere with authentications occurring on another Port.
> > > +		 * (IEEE Std 802.1X-2001 C.3.3)
> > > +		 */
> > > +		if (unlikely(!br->pae_forward &&
> > > +		    skb->protocol == htons(ETH_P_PAE)))
> > 
> > No, please don't.
> > 
> > Linux bridging has two "grand" modes: dumb and STP enabled.
> > 
> > If we're running a dumb bridge, we behave like an ethernet hub without
> > any intelligence, and in that case we should absolutely forward 802.1X
> > frames. We may have (e.g. VM) client(s) that want to authenticate with a
> > physical switch.
> > (For the spec, this counts as "repeater", not "bridge"/"switch")
> > 
> > If we're running with STP enabled, then 802.1X traffic should already be
> > caught by the general ethernet link-local multicast drop (which applies
> > to 01:80:c2:/24 and therefore catches 802.1X too.)
> 
> The problem is that STP is not enabled by default, and most people don't
> know how to enable it.

Yes, the default is a dumb hub (IMHO correctly so). And a dumb hub will
forward 802.1X packets (IMHO also correctly so).

Why should we specifically add a knob for EAPOL? Next we're adding one
for STP itself, then one for LLDP, then one for Cisco's deprecated
crap (CDP, DTP, ...) etc.

If you want a dumb hub that drops EAPOL, use ebtables.

-David


^ permalink raw reply

* Re: RFT: virtio_net: limit xmit polling
From: Tom Lendacky @ 2011-06-28 16:08 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Krishna Kumar2, habanero, lguest, Shirley Ma, kvm, Carsten Otte,
	linux-s390, Heiko Carstens, linux-kernel, virtualization, steved,
	Christian Borntraeger, netdev, Martin Schwidefsky, linux390
In-Reply-To: <20110619102700.GA11198@redhat.com>

[-- Attachment #1: Type: Text/Plain, Size: 6384 bytes --]

On Sunday, June 19, 2011 05:27:00 AM Michael S. Tsirkin wrote:
> OK, different people seem to test different trees.  In the hope to get
> everyone on the same page, I created several variants of this patch so
> they can be compared. Whoever's interested, please check out the
> following, and tell me how these compare:
> 
> kernel:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git
> 
> virtio-net-limit-xmit-polling/base - this is net-next baseline to test
> against virtio-net-limit-xmit-polling/v0 - fixes checks on out of capacity
> virtio-net-limit-xmit-polling/v1 - previous revision of the patch
> 		this does xmit,free,xmit,2*free,free
> virtio-net-limit-xmit-polling/v2 - new revision of the patch
> 		this does free,xmit,2*free,free
> 

Here's a summary of the results.  I've also attached an ODS format spreadsheet
(30 KB in size) that might be easier to analyze and also has some pinned VM
results data.  I broke the tests down into a local guest-to-guest scenario
and a remote host-to-guest scenario.

Within the local guest-to-guest scenario I ran:
  - TCP_RR tests using two different messsage sizes and four different
    instance counts among 1 pair of VMs and 2 pairs of VMs.
  - TCP_STREAM tests using four different message sizes and two different
    instance counts among 1 pair of VMs and 2 pairs of VMs.

Within the remote host-to-guest scenario I ran:
  - TCP_RR tests using two different messsage sizes and four different
    instance counts to 1 VM and 4 VMs.
  - TCP_STREAM and TCP_MAERTS tests using four different message sizes and
    two different instance counts to 1 VM and 4 VMs.
over a 10GbE link.

*** Local Guest-to-Guest ***

Here's the local guest-to-guest summary for 1 VM pair doing TCP_RR with
256/256 request/response message size in transactions per second:

Instances	Base		V0		V1		V2
1		 8,151.56	 8,460.72	 8,439.16	 9,990.37
25		48,761.74	51,032.62	51,103.25	49,533.52
50		55,687.38	55,974.18	56,854.10	54,888.65
100		58,255.06	58,255.86	60,380.90	59,308.36

Here's the local guest-to-guest summary for 2 VM pairs doing TCP_RR with
256/256 request/response message size in transactions per second:

Instances	Base		V0		V1		V2
1		18,758.48	19,112.50	18,597.07	19,252.04
25		80,500.50	78,801.78	80,590.68	78,782.07
50		80,594.20	77,985.44	80,431.72	77,246.90
100		82,023.23	81,325.96	81,303.32	81,727.54

Here's the local guest-to-guest summary for 1 VM pair doing TCP_STREAM with
256, 1K, 4K and 16K message size in Mbps:

256:
Instances	Base		V0		V1		V2
1		   961.78	 1,115.92	   794.02	   740.37
4		 2,498.33	 2,541.82	 2,441.60	 2,308.26

1K:					
1		 3,476.61	 3,522.02	 2,170.86	 1,395.57
4		 6,344.30	 7,056.57	 7,275.16	 7,174.09

4K:					
1		 9,213.57	10,647.44	 9,883.42	 9,007.29
4		11,070.66	11,300.37	11,001.02	12,103.72

16K:
1		12,065.94	 9,437.78	11,710.60	 6,989.93
4		12,755.28	13,050.78	12,518.06	13,227.33

Here's the local guest-to-guest summary for 2 VM pairs doing TCP_STREAM with
256, 1K, 4K and 16K message size in Mbps:

256:
Instances	Base		V0		V1		V2
1		 2,434.98	 2,403.23	 2,308.69	 2,261.35
4		 5,973.82	 5,729.48	 5,956.76	 5,831.86

1K:
1		 5,305.99	 5,148.72	 4,960.67	 5,067.76
4		10,628.38	10,649.49	10,098.90	10,380.09

4K:
1		11,577.03	10,710.33	11,700.53	10,304.09
4		14,580.66	14,881.38	14,551.17	15,053.02

16K:
1		16,801.46	16,072.50	15,773.78	15,835.66
4		17,194.00	17,294.02	17,319.78	17,121.09


*** Remote Host-to-Guest ***

Here's the remote host-to-guest summary for 1 VM doing TCP_RR with
256/256 request/response message size in transactions per second:

Instances	Base		V0		V1		V2
1		 9,732.99	10,307.98	10,529.82	 8,889.28
25		43,976.18	49,480.50	46,536.66	45,682.38
50		63,031.33	67,127.15	60,073.34	65,748.62
100		64,778.43	65,338.07	66,774.12	69,391.22

Here's the remote host-to-guest summary for 4 VMs doing TCP_RR with
256/256 request/response message size in transactions per second:

Instances	Base		V0		V1		V2
1		 39,270.42	 38,253.60	 39,353.10	 39,566.33
25		207,120.91	207,964.50	211,539.70	213,882.21
50		218,801.54	221,490.56	220,529.48	223,594.25
100		218,432.62	215,061.44	222,011.61	223,480.47

Here's the remote host-to-guest summary for 1 VM doing TCP_STREAM with
256, 1K, 4K and 16K message size in Mbps:

256:
Instances	Base		V0		V1		V2
1		2,274.74	2,220.38	2,245.26	2,212.30
4		5,689.66	5,953.86	5,984.80	5,827.94

1K:
1		7,804.38	7,236.29	6,716.58	7,485.09
4		7,722.42	8,070.38	7,700.45	7,856.76

4K:
1		8,976.14	9,026.77	9,147.32	9,095.58
4		7,532.25	7,410.80	7,683.81	7,524.94

16K:
1		8,991.61	9,045.10	9,124.58	9,238.34
4		7,406.10	7,626.81	7,711.62	7,345.37

Here's the remote host-to-guest summary for 1 VM doing TCP_MAERTS with
256, 1K, 4K and 16K message size in Mbps:

256:
Instances	Base		V0		V1		V2
1		1,165.69	1,181.92	1,152.20	1,104.68
4		2,580.46	2,545.22	2,436.30	2,601.74

1K:
1		2,393.34	2,457.22	2,128.86	2,258.92
4		7,152.57	7,606.60	8,004.64	7,576.85

4K:
1		9,258.93	8,505.06	9,309.78	9,215.05
4		9,374.20	9,363.48	9,372.53	9,352.00

16K:
1		9,244.70	9,287.72	9,298.60	9,322.28
4		9,380.02	9,347.50	9,377.46	9,372.98

Here's the remote host-to-guest summary for 4 VMs doing TCP_STREAM with
256, 1K, 4K and 16K message size in Mbps:

256:
Instances	Base		V0		V1		V2
1		9,392.37	9,390.74	9,395.58	9,392.46
4		9,394.24	9,394.46	9,395.42	9,394.05

1K:
1		9,396.34	9,397.46	9,396.64	9,443.26
4		9,397.14	9,402.25	9,398.67	9,391.09

4K:
1		9,397.16	9,398.07	9,397.30	9,396.33
4		9,395.64	9,400.25	9,397.54	9,397.75

16K:
1		9,396.58	9,397.01	9,397.58	9,397.70
4		9,399.15	9,400.02	9,399.66	9,400.16


Here's the remote host-to-guest summary for 4 VMs doing TCP_MAERTS with
256, 1K, 4K and 16K message size in Mbps:

256:
Instances	Base		V0		V1		V2
1		5,048.66	5,007.26	5,074.98	4,974.86
4		9,217.23	9,245.14	9,263.97	9,294.23

1K:
1		9,378.32	9,387.12	9,386.21	9,361.55
4		9,384.42	9,384.02	9,385.50	9,385.55

4K:
1		9,391.10	9,390.28	9,389.70	9,391.02
4		9,384.38	9,383.39	9,384.74	9,384.19

16K:
1		9,390.77	9,389.62	9,388.07	9,388.19
4		9,381.86	9,382.37	9,385.54	9,383.88


Tom

> There's also this on top:
> virtio-net-limit-xmit-polling/v3 -> don't delay avail index update
> I don't think it's important to test this one, yet
> 
> Userspace to use: event index work is not yet merged upstream
> so the revision to use is still this:
> git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu-kvm.git
> virtio-net-event-idx-v3

[-- Attachment #2: MST-Request.ods --]
[-- Type: application/vnd.oasis.opendocument.spreadsheet, Size: 31012 bytes --]

[-- Attachment #3: Type: text/plain, Size: 184 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* RE: [PATCH] [net][bna] Fix call trace when interrupts are disabled while sleeping function kzalloc is called
From: Shyam_Iyer @ 2011-06-28 16:29 UTC (permalink / raw)
  To: davem, shyam.iyer.t; +Cc: netdev, rmody, ddutt
In-Reply-To: <20110627.160706.1453191829412045606.davem@davemloft.net>

[-- Attachment #1: Type: text/plain, Size: 1112 bytes --]

(My replies bounced of netdev for some reason..resending)

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Monday, June 27, 2011 7:07 PM
> To: shyam.iyer.t@gmail.com
> Cc: netdev@vger.kernel.org; rmody@brocade.com; ddutt@brocadel.com;
> Iyer, Shyam
> Subject: Re: [PATCH] [net][bna] Fix call trace when interrupts are
> disabled while sleeping function kzalloc is called
> 
> From: Shyam Iyer <shyam.iyer.t@gmail.com>
> Date: Mon, 27 Jun 2011 16:21:32 -0400
> 
> > The kzalloc sleeps and disabling interrupts(spin_lock_irqsave)
> > causes oops like the one.
> 
> What if ->cfg_flags changes while you have dropped the lock?
> 
> If the lock doesn't protect those flags, what was it being
> taken for in the first place?

Oops yes!

Here is try 2.

I found that I had to initialize the flags variable outside the spin_lock_irqsave in the following way to fix the problem. The sleeping kzalloc function is in fact in the request_threaded_irq call which allocates the irq handler function and its flags arguments.

Attached patch fixes the issue.

[-- Attachment #2: 0001-Fix-call-trace-in-request_irq.patch --]
[-- Type: application/octet-stream, Size: 1042 bytes --]

From 1d46f1847fc000d335f3b3e7875ba88a360174a4 Mon Sep 17 00:00:00 2001
From: Shyam Iyer <shyam_iyer@dell.com>
Date: Tue, 28 Jun 2011 10:02:13 -0400
Subject: [PATCH] Fix call trace in request_irq.

Signed-off-by: Shyam Iyer <shyam_iyer@dell.com>
---
 drivers/net/bna/bnad.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bna/bnad.c b/drivers/net/bna/bnad.c
index 7d25a97..d4e8672 100644
--- a/drivers/net/bna/bnad.c
+++ b/drivers/net/bna/bnad.c
@@ -1111,7 +1111,7 @@ bnad_mbox_irq_alloc(struct bnad *bnad,
 		    struct bna_intr_info *intr_info)
 {
 	int 		err = 0;
-	unsigned long 	flags;
+	unsigned long 	flags = 0;
 	u32	irq;
 	irq_handler_t 	irq_handler;
 
@@ -1125,7 +1125,6 @@ bnad_mbox_irq_alloc(struct bnad *bnad,
 	if (bnad->cfg_flags & BNAD_CF_MSIX) {
 		irq_handler = (irq_handler_t)bnad_msix_mbox_handler;
 		irq = bnad->msix_table[bnad->msix_num - 1].vector;
-		flags = 0;
 		intr_info->intr_type = BNA_INTR_T_MSIX;
 		intr_info->idl[0].vector = bnad->msix_num - 1;
 	} else {
-- 
1.7.5.4


^ permalink raw reply related

* RE: [PATCH] [net][bna] Fix call trace when interrupts are disabled while sleeping function  kzalloc is called
From: Shyam_Iyer @ 2011-06-28 16:32 UTC (permalink / raw)
  To: rmody, shyam.iyer.t, netdev; +Cc: ddutt, huangj
In-Reply-To: <E5313AF6F2BFD14293E5FD0F94750F86A82D66F4FF@HQ1-EXCH01.corp.brocade.com>

(Resending the reply for this as well.)

> -----Original Message-----
> From: Rasesh Mody [mailto:rmody@brocade.com]
> Sent: Monday, June 27, 2011 8:51 PM
> To: Shyam Iyer; netdev@vger.kernel.org
> Cc: ddutt@brocadel.com; Iyer, Shyam; Jing Huang
> Subject: RE: [PATCH] [net][bna] Fix call trace when interrupts are
> disabled while sleeping function kzalloc is called
> 
> 
> >From: Shyam Iyer [mailto:shyam.iyer.t@gmail.com]
> >Sent: Monday, June 27, 2011 1:22 PM
> >Subject: [PATCH] [net][bna] Fix call trace when interrupts are
> disabled
> >while sleeping function kzalloc is called
> >
> >The kzalloc sleeps and disabling interrupts(spin_lock_irqsave) causes
> >oops like the one.
> 
> Hi Shyam,
> 
> We are not calling any sleeping function while holding the lock in
> bnad_mbox_irq_alloc(). How would your patch fix the call trace? Also
> can you tell which conditions led to this trace.
> 
> Thanks,
> --Rasesh

Hi Rasesh,

Please review my reply/patch2 to Dave. I guess the condition to reproduce is to simply insmod/modprobe the driver on the Brocade 1020 CNA . I don't have cables attached to the card if that helps.

Thanks,
Shyam

^ permalink raw reply

* Re: [PATCH V7 2/4 net-next] skbuff: Add userspace zero-copy buffers in skb
From: Shirley Ma @ 2011-06-28 16:51 UTC (permalink / raw)
  To: David Miller; +Cc: mst, eric.dumazet, avi, arnd, netdev, kvm, linux-kernel
In-Reply-To: <20110627.155426.51839633424542723.davem@davemloft.net>

On Mon, 2011-06-27 at 15:54 -0700, David Miller wrote:
> From: Shirley Ma <mashirle@us.ibm.com>
> Date: Mon, 27 Jun 2011 08:45:10 -0700
> 
> > To support skb zero-copy, a pointer is needed to add to skb share
> info.
> > Do you agree with this approach? If not, do you have any other
> > suggestions?
> 
> I really can't form an opinion unless I am shown the complete
> implementation, what this give us in return, what the impact is, etc. 

zero-copy skb buffers can save significant CPUs. Right now, I only
implements macvtap/vhost zero-copy between KVM guest and host. The
performance is as follow:

Single TCP_STREAM 120 secs test results 2.6.39-rc3 over ixgbe 10Gb NIC
results:

Message BW(Gb/s)qemu-kvm (NumCPU)vhost-net(NumCPU) PerfTop irq/s
4K      7408.57         92.1%           22.6%           1229
4K(Orig)4913.17         118.1%          84.1%           2086    

8K      9129.90         89.3%           23.3%           1141
8K(Orig)7094.55         115.9%          84.7%           2157

16K     9178.81         89.1%           23.3%           1139
16K(Orig)8927.1         118.7%          83.4%           2262

64K     9171.43         88.4%           24.9%           1253
64K(Orig)9085.85        115.9%          82.4%           2229

You can see the overall CPU saved 50% w/i zero-copy.

The impact is every skb allocation consumed one more pointer in skb
share info, and a pointer check in skb release when last reference is
gone.

For skb clone, skb expand private head and skb copy, it still keeps copy
the buffers to kernel, so we can avoid user application, like tcpdump to
hold the user-space buffers too long.

Thanks
Shirley

^ permalink raw reply

* Re: [PATCH V7 2/4 net-next] skbuff: Add userspace zero-copy buffers in skb
From: Rick Jones @ 2011-06-28 17:19 UTC (permalink / raw)
  To: Shirley Ma
  Cc: David Miller, mst, eric.dumazet, avi, arnd, netdev, kvm,
	linux-kernel
In-Reply-To: <1309279892.3559.6.camel@localhost.localdomain>

On 06/28/2011 09:51 AM, Shirley Ma wrote:
> On Mon, 2011-06-27 at 15:54 -0700, David Miller wrote:
>> From: Shirley Ma<mashirle@us.ibm.com>
>> Date: Mon, 27 Jun 2011 08:45:10 -0700
>>
>>> To support skb zero-copy, a pointer is needed to add to skb share
>> info.
>>> Do you agree with this approach? If not, do you have any other
>>> suggestions?
>>
>> I really can't form an opinion unless I am shown the complete
>> implementation, what this give us in return, what the impact is, etc.
>
> zero-copy skb buffers can save significant CPUs. Right now, I only
> implements macvtap/vhost zero-copy between KVM guest and host. The
> performance is as follow:
>
> Single TCP_STREAM 120 secs test results 2.6.39-rc3 over ixgbe 10Gb NIC
> results:
>
> Message BW(Gb/s)qemu-kvm (NumCPU)vhost-net(NumCPU) PerfTop irq/s
> 4K      7408.57         92.1%           22.6%           1229
> 4K(Orig)4913.17         118.1%          84.1%           2086
>
> 8K      9129.90         89.3%           23.3%           1141
> 8K(Orig)7094.55         115.9%          84.7%           2157
>
> 16K     9178.81         89.1%           23.3%           1139
> 16K(Orig)8927.1         118.7%          83.4%           2262
>
> 64K     9171.43         88.4%           24.9%           1253
> 64K(Orig)9085.85        115.9%          82.4%           2229
>
> You can see the overall CPU saved 50% w/i zero-copy.

While this isn't the copy between netperf and the stack, at some point 
you may want to enable netperf's "DIRTY" mode (./configure 
--enable-dirty) to cause it to start either dirtying buffers before 
send, or reading from buffers after receive.  I cannot guarantee that 
there hasn't been bitrot in that area of netperf though :)  Particularly 
in a TCP_MAERTS test.  The "DIRTY" mode code will not do anything in a 
TCP_SENDFILE test.

A simple sanity check of the effect of the changes on a TCP_RR test 
would probably be goodness as well.

happy benchmarking,

rick jones
one of these days I'll have to find a good way to get accurate overall 
CPU utilization from within a guest and teach netperf about it.

>
> The impact is every skb allocation consumed one more pointer in skb
> share info, and a pointer check in skb release when last reference is
> gone.
>
> For skb clone, skb expand private head and skb copy, it still keeps copy
> the buffers to kernel, so we can avoid user application, like tcpdump to
> hold the user-space buffers too long.
>
> Thanks
> Shirley
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: [PATCH] [net][bna] Fix call trace when interrupts are disabled while sleeping function kzalloc is called
From: Rasesh Mody @ 2011-06-28 17:27 UTC (permalink / raw)
  To: Shyam_Iyer@Dell.com, davem@davemloft.net, shyam.iyer.t@gmail.com
  Cc: netdev@vger.kernel.org, Debashis Dutt, Jing Huang
In-Reply-To: <DBFB1B45AF80394ABD1C807E9F28D1570453626FF0@BLRX7MCDC203.AMER.DELL.COM>

>From: Shyam_Iyer@Dell.com [mailto:Shyam_Iyer@Dell.com]
>Sent: Tuesday, June 28, 2011 9:30 AM
>
>Here is try 2.
>
>I found that I had to initialize the flags variable outside the
>spin_lock_irqsave in the following way to fix the problem. The sleeping
>kzalloc function is in fact in the request_threaded_irq call which
>allocates the irq handler function and its flags arguments.
>
>Attached patch fixes the issue.

The attached patch may fix the issue in MSIX mode, however we'll continue to see the issue in INTX mode. Using a separate irq_flags may be a good idea.


^ permalink raw reply


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