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

* [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: 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

* 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

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

* 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

* 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: 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: 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: 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: 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: 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: [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: 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: r8169 :  always copying the rx buffer to new skb
From: Francois Romieu @ 2011-06-28  7:55 UTC (permalink / raw)
  To: John Lumby; +Cc: netdev
In-Reply-To: <COL116-W30238D0821823E764BBB53A3570@phx.gbl>

John, please ask your mail user agent to avoid really, really long lines.

John Lumby <johnlumby@hotmail.com> :
[...]
> I don't plan to do any more on this but can provide my patch (currently
> one monolithic one based on DaveM 3.0.0-rc1netnext-110615) and detailed
> results if anyone wants.

If you want your work to stand a chance to be of any use, you should
send it. Please read the "12) Sign your work" part of
"Documentation/SubmittingPatches" - even if it's a giant pile of mildly
standard code - so that I can store it at kernel.org.

Thanks.

-- 
Ueimor

^ permalink raw reply

* [PATCH]rionet: fix NULL pointer dereference in rionet_remove
From: Yinglin Luan @ 2011-06-26  4:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, mporter, jj, cmdkhh, synmyth

Function rionet_remove initializes local variable 'ndev' to NULL
and do nothing changes before the call to unregister_netdev(ndev),
this could cause a NULL pointer dereference.

Reported-by: Jesper Juhl <jj@chaosbits.net>
Signed-off-by: Yinglin Luan <synmyth@gmail.com>
---
 drivers/net/rionet.c |   28 +++++++++++++++-------------
 1 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/net/rionet.c b/drivers/net/rionet.c
index 77c5092..5d3436d 100644
--- a/drivers/net/rionet.c
+++ b/drivers/net/rionet.c
@@ -378,7 +378,7 @@ static int rionet_close(struct net_device *ndev)
 
 static void rionet_remove(struct rio_dev *rdev)
 {
-	struct net_device *ndev = NULL;
+	struct net_device *ndev = rio_get_drvdata(rdev);
 	struct rionet_peer *peer, *tmp;
 
 	free_pages((unsigned long)rionet_active, rdev->net->hport->sys_size ?
@@ -433,22 +433,12 @@ static const struct net_device_ops rionet_netdev_ops = {
 	.ndo_set_mac_address	= eth_mac_addr,
 };
 
-static int rionet_setup_netdev(struct rio_mport *mport)
+static int rionet_setup_netdev(struct rio_mport *mport, struct net_device *ndev)
 {
 	int rc = 0;
-	struct net_device *ndev = NULL;
 	struct rionet_private *rnet;
 	u16 device_id;
 
-	/* Allocate our net_device structure */
-	ndev = alloc_etherdev(sizeof(struct rionet_private));
-	if (ndev == NULL) {
-		printk(KERN_INFO "%s: could not allocate ethernet device.\n",
-		       DRV_NAME);
-		rc = -ENOMEM;
-		goto out;
-	}
-
 	rionet_active = (struct rio_dev **)__get_free_pages(GFP_KERNEL,
 			mport->sys_size ? __fls(sizeof(void *)) + 4 : 0);
 	if (!rionet_active) {
@@ -504,11 +494,21 @@ static int rionet_probe(struct rio_dev *rdev, const struct rio_device_id *id)
 	int rc = -ENODEV;
 	u32 lpef, lsrc_ops, ldst_ops;
 	struct rionet_peer *peer;
+	struct net_device *ndev = NULL;
 
 	/* If local device is not rionet capable, give up quickly */
 	if (!rionet_capable)
 		goto out;
 
+	/* Allocate our net_device structure */
+	ndev = alloc_etherdev(sizeof(struct rionet_private));
+	if (ndev == NULL) {
+		printk(KERN_INFO "%s: could not allocate ethernet device.\n",
+		       DRV_NAME);
+		rc = -ENOMEM;
+		goto out;
+	}
+
 	/*
 	 * First time through, make sure local device is rionet
 	 * capable, setup netdev,  and set flags so this is skipped
@@ -529,7 +529,7 @@ static int rionet_probe(struct rio_dev *rdev, const struct rio_device_id *id)
 			goto out;
 		}
 
-		rc = rionet_setup_netdev(rdev->net->hport);
+		rc = rionet_setup_netdev(rdev->net->hport, ndev);
 		rionet_check = 1;
 	}
 
@@ -546,6 +546,8 @@ static int rionet_probe(struct rio_dev *rdev, const struct rio_device_id *id)
 		list_add_tail(&peer->node, &rionet_peers);
 	}
 
+	rio_set_drvdata(rdev, ndev);
+
       out:
 	return rc;
 }
-- 
1.7.0.4

^ permalink raw reply related

* RE: [PATCH net-next]  benet: convert to 64 bit stats
From: Sathya.Perla @ 2011-06-28  7:37 UTC (permalink / raw)
  To: shemminger, davem; +Cc: netdev
In-Reply-To: <20110627114313.1d1b9680@nehalam.ftrdhcpuser.net>

>-----Original Message-----
>From: Stephen Hemminger [mailto:shemminger@vyatta.com]
>Sent: Tuesday, June 28, 2011 12:13 AM
>To: David Miller
>Cc: Perla, Sathya; netdev@vger.kernel.org
>Subject: [PATCH net-next] benet: convert to 64 bit stats
>
>This changes how the benet driver does statistics:
>  * use 64 bit statistics interface (old api was only 32 bit on 32 bit
>platform)
>  * use u64_stats_sync to ensure atomic 64 bit on 32 bit SMP
>  * only update statistics when needed
>
>Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
>---
>
> drivers/net/benet/be.h      |    4 -
> drivers/net/benet/be_cmds.c |    1
> drivers/net/benet/be_main.c |  161 ++++++++++++++++++++++++----------------
>----
> 3 files changed, 93 insertions(+), 73 deletions(-)

Thanks for the patch!
be_ethtool.c:be_get_ethtool_stats() currently accesses old netdev->stats. This needs to be fixed.
I guess it would be OK to drop netdev stats from being reported via ethtool.
The routine also accesses per-ring rx_bytes/rx_pkts. I'm OK with dropping these too in ethtool for
the scope of this patch as the aggregates are reported in netdev stats.

Can you incorporate the following changes...

diff --git a/drivers/net/benet/be_ethtool.c b/drivers/net/benet/be_ethtool.c
index 30c1386..9a89caa 100644
--- a/drivers/net/benet/be_ethtool.c
+++ b/drivers/net/benet/be_ethtool.c
@@ -26,13 +26,9 @@ struct be_ethtool_stat {
 	int offset;
 };
 
-enum {NETSTAT, DRVSTAT_TX, DRVSTAT_RX, ERXSTAT,
-			DRVSTAT};
+enum {DRVSTAT_TX, DRVSTAT_RX, ERXSTAT, DRVSTAT};
 #define FIELDINFO(_struct, field) FIELD_SIZEOF(_struct, field), \
 					offsetof(_struct, field)
-#define NETSTAT_INFO(field) 	#field, NETSTAT,\
-					FIELDINFO(struct net_device_stats,\
-						field)
 #define DRVSTAT_TX_INFO(field)	#field, DRVSTAT_TX,\
 					FIELDINFO(struct be_tx_stats, field)
 #define DRVSTAT_RX_INFO(field)	#field, DRVSTAT_RX,\
@@ -44,14 +40,6 @@ enum {NETSTAT, DRVSTAT_TX, DRVSTAT_RX, ERXSTAT,
 						field)
 
 static const struct be_ethtool_stat et_stats[] = {
-	{NETSTAT_INFO(rx_packets)},
-	{NETSTAT_INFO(tx_packets)},
-	{NETSTAT_INFO(rx_bytes)},
-	{NETSTAT_INFO(tx_bytes)},
-	{NETSTAT_INFO(rx_errors)},
-	{NETSTAT_INFO(tx_errors)},
-	{NETSTAT_INFO(rx_dropped)},
-	{NETSTAT_INFO(tx_dropped)},
 	{DRVSTAT_INFO(be_tx_events)},
 	{DRVSTAT_INFO(rx_crc_errors)},
 	{DRVSTAT_INFO(rx_alignment_symbol_errors)},
@@ -94,8 +82,6 @@ static const struct be_ethtool_stat et_stats[] = {
 
 /* Stats related to multi RX queues */
 static const struct be_ethtool_stat et_rx_stats[] = {
-	{DRVSTAT_RX_INFO(rx_bytes)},
-	{DRVSTAT_RX_INFO(rx_pkts)},
 	{DRVSTAT_RX_INFO(rx_rate)},
 	{DRVSTAT_RX_INFO(rx_polls)},
 	{DRVSTAT_RX_INFO(rx_events)},
@@ -263,16 +249,7 @@ be_get_ethtool_stats(struct net_device *netdev,
 	int i, j, base;
 
 	for (i = 0; i < ETHTOOL_STATS_NUM; i++) {
-		switch (et_stats[i].type) {
-		case NETSTAT:
-			p = &netdev->stats;
-			break;
-		case DRVSTAT:
-			p = &adapter->drv_stats;
-			break;
-		}
-
-		p = (u8 *)p + et_stats[i].offset;
+		p = (u8 *)&adapter->drv_stats + et_stats[i].offset;
 		data[i] = (et_stats[i].size == sizeof(u64)) ?
 				*(u64 *)p: *(u32 *)p;
 	}
@@ -282,7 +259,7 @@ be_get_ethtool_stats(struct net_device *netdev,
 		for (i = 0; i < ETHTOOL_RXSTATS_NUM; i++) {
 			switch (et_rx_stats[i].type) {
 			case DRVSTAT_RX:
-				p = (u8 *)&rxo->stats + et_rx_stats[i].offset;
+				p = (u8 *)rx_stats(rxo) + et_rx_stats[i].offset;
 				break;
 			case ERXSTAT:
 				p = (u32 *)be_erx_stats_from_cmd(adapter) +
@@ -298,7 +275,7 @@ be_get_ethtool_stats(struct net_device *netdev,
 	base = ETHTOOL_STATS_NUM + adapter->num_rx_qs * ETHTOOL_RXSTATS_NUM;
 	for_all_tx_queues(adapter, txo, j) {
 		for (i = 0; i < ETHTOOL_TXSTATS_NUM; i++) {
-			p = (u8 *)&txo->stats + et_tx_stats[i].offset;
+			p = (u8 *)tx_stats(txo) + et_tx_stats[i].offset;
 			data[base + j * ETHTOOL_TXSTATS_NUM + i] =
 				(et_tx_stats[i].size == sizeof(u64)) ?
 					*(u64 *)p: *(u32 *)p;




^ permalink raw reply related

* [PATCH NEXT 2/2] qlcnic: add external loopback support
From: amit.salecha @ 2011-06-28  6:40 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman, Amit Kumar Salecha, Sucheta Chakraborty
In-Reply-To: <1309243247-15950-1-git-send-email-amit.salecha@qlogic.com>

From: Amit Kumar Salecha <amit.salecha@qlogic.com>

o Add external loopback test in self test:
  - Send set external loopback mode request to fw.
     To quiscent other storage functions.
  - Perform test
  - Send unset loopback mode request to fw.

o Rename ilb to lb.
o Update driver version 5.0.20.

Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/qlcnic/qlcnic.h         |    5 +++--
 drivers/net/qlcnic/qlcnic_ethtool.c |   25 ++++++++++++++++---------
 drivers/net/qlcnic/qlcnic_init.c    |    3 ++-
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/net/qlcnic/qlcnic.h b/drivers/net/qlcnic/qlcnic.h
index e545450..9899a79 100644
--- a/drivers/net/qlcnic/qlcnic.h
+++ b/drivers/net/qlcnic/qlcnic.h
@@ -36,8 +36,8 @@
 
 #define _QLCNIC_LINUX_MAJOR 5
 #define _QLCNIC_LINUX_MINOR 0
-#define _QLCNIC_LINUX_SUBVERSION 19
-#define QLCNIC_LINUX_VERSIONID  "5.0.19"
+#define _QLCNIC_LINUX_SUBVERSION 20
+#define QLCNIC_LINUX_VERSIONID  "5.0.20"
 #define QLCNIC_DRV_IDC_VER  0x01
 #define QLCNIC_DRIVER_VERSION  ((_QLCNIC_LINUX_MAJOR << 16) |\
 		 (_QLCNIC_LINUX_MINOR << 8) | (_QLCNIC_LINUX_SUBVERSION))
@@ -782,6 +782,7 @@ struct qlcnic_mac_list_s {
 #define QLCNIC_IP_DOWN		3
 
 #define QLCNIC_ILB_MODE		0x1
+#define QLCNIC_ELB_MODE		0x2
 
 #define QLCNIC_LINKEVENT	0x1
 #define QLCNIC_LB_RESPONSE	0x2
diff --git a/drivers/net/qlcnic/qlcnic_ethtool.c b/drivers/net/qlcnic/qlcnic_ethtool.c
index 743035e..821d8a7 100644
--- a/drivers/net/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/qlcnic/qlcnic_ethtool.c
@@ -85,7 +85,8 @@ static const char qlcnic_gstrings_test[][ETH_GSTRING_LEN] = {
 	"Register_Test_on_offline",
 	"Link_Test_on_offline",
 	"Interrupt_Test_offline",
-	"Loopback_Test_offline"
+	"Internal_Loopback_offline",
+	"External_Loopback_offline"
 };
 
 #define QLCNIC_TEST_LEN	ARRAY_SIZE(qlcnic_gstrings_test)
@@ -709,7 +710,7 @@ int qlcnic_check_loopback_buff(unsigned char *data, u8 mac[])
 	return memcmp(data, buff, QLCNIC_ILB_PKT_SIZE);
 }
 
-static int qlcnic_do_ilb_test(struct qlcnic_adapter *adapter)
+static int qlcnic_do_lb_test(struct qlcnic_adapter *adapter)
 {
 	struct qlcnic_recv_context *recv_ctx = adapter->recv_ctx;
 	struct qlcnic_host_sds_ring *sds_ring = &recv_ctx->sds_rings[0];
@@ -735,19 +736,19 @@ static int qlcnic_do_ilb_test(struct qlcnic_adapter *adapter)
 		dev_kfree_skb_any(skb);
 
 		if (!adapter->diag_cnt)
-			dev_warn(&adapter->pdev->dev, "ILB Test: %dth packet"
+			dev_warn(&adapter->pdev->dev, "LB Test: %dth packet"
 				" not recevied\n", i + 1);
 		else
 			cnt++;
 	}
 	if (cnt != i) {
-		dev_warn(&adapter->pdev->dev, "ILB Test failed\n");
+		dev_warn(&adapter->pdev->dev, "LB Test failed\n");
 		return -1;
 	}
 	return 0;
 }
 
-static int qlcnic_iloopback_test(struct net_device *netdev)
+static int qlcnic_loopback_test(struct net_device *netdev, u8 mode)
 {
 	struct qlcnic_adapter *adapter = netdev_priv(netdev);
 	int max_sds_rings = adapter->max_sds_rings;
@@ -755,7 +756,8 @@ static int qlcnic_iloopback_test(struct net_device *netdev)
 	int loop = 0;
 	int ret;
 
-	netdev_info(netdev, "%s:  in progress\n", __func__);
+	netdev_info(netdev, "%s loopback test in progress\n",
+		   mode == QLCNIC_ILB_MODE ? "internal" : "external");
 	if (adapter->op_mode == QLCNIC_NON_PRIV_FUNC) {
 		netdev_warn(netdev, "Loopback test not supported for non "
 				"privilege function\n");
@@ -772,7 +774,7 @@ static int qlcnic_iloopback_test(struct net_device *netdev)
 
 	sds_ring = &adapter->recv_ctx->sds_rings[0];
 
-	ret = qlcnic_set_lb_mode(adapter, QLCNIC_ILB_MODE);
+	ret = qlcnic_set_lb_mode(adapter, mode);
 	if (ret)
 		goto free_res;
 
@@ -790,7 +792,7 @@ static int qlcnic_iloopback_test(struct net_device *netdev)
 		goto free_res;
 	}
 
-	ret = qlcnic_do_ilb_test(adapter);
+	ret = qlcnic_do_lb_test(adapter);
 
 	qlcnic_clear_lb_mode(adapter);
 
@@ -822,10 +824,15 @@ qlcnic_diag_test(struct net_device *dev, struct ethtool_test *eth_test,
 		if (data[2])
 			eth_test->flags |= ETH_TEST_FL_FAILED;
 
-		data[3] = qlcnic_iloopback_test(dev);
+		data[3] = qlcnic_loopback_test(dev, QLCNIC_ILB_MODE);
 		if (data[3])
 			eth_test->flags |= ETH_TEST_FL_FAILED;
 
+		if (eth_test->flags & ETH_TEST_FL_EXTERNAL_LB) {
+			data[4] = qlcnic_loopback_test(dev, QLCNIC_ELB_MODE);
+			if (data[4])
+				eth_test->flags |= ETH_TEST_FL_FAILED;
+		}
 	}
 }
 
diff --git a/drivers/net/qlcnic/qlcnic_init.c b/drivers/net/qlcnic/qlcnic_init.c
index 9d5bee0..6ec1baa 100644
--- a/drivers/net/qlcnic/qlcnic_init.c
+++ b/drivers/net/qlcnic/qlcnic_init.c
@@ -1303,7 +1303,8 @@ qlcnic_handle_linkevent(struct qlcnic_adapter *adapter,
 		dev_info(&netdev->dev, "unsupported cable length %d\n",
 				cable_len);
 
-	if (!link_status && (lb_status == 1))
+	if (!link_status && (lb_status == QLCNIC_ILB_MODE ||
+	    lb_status == QLCNIC_ELB_MODE))
 		adapter->ahw->loopback_state |= QLCNIC_LINKEVENT;
 
 	qlcnic_advert_link_change(adapter, link_status);
-- 
1.7.3.3


^ permalink raw reply related

* [PATCH NEXT 1/2] net: add external loopback test in ethtool self test
From: amit.salecha @ 2011-06-28  6:40 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman, Amit Kumar Salecha
In-Reply-To: <1309243247-15950-1-git-send-email-amit.salecha@qlogic.com>

From: Amit Kumar Salecha <amit.salecha@qlogic.com>

External loopback test can be performed by application without any driver
support on normal Ethernet cards.
But on CNA devices, where multiple functions share same physical port.
Here internal loopback test and external loopback test can be initiated by
different function at same time. To co exist all functions, firmware need
to regulate what test can be run by which function. So before performing external
loopback test, command need to send to firmware, which will quiescent other functions.

User may not want to run external loopback test always. As special cable need to be
connected for this test.

So adding explicit flag in ethtool self test, which will specify interface
to perform external loopback test.

Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 include/linux/ethtool.h |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 048d0fa..c2ba287 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -310,9 +310,18 @@ struct ethtool_sset_info {
 				   __u32's, etc. */
 };
 
+/*
+ * Flags definition of ethtool_test
+ *
+ * ETH_TEST_FL_OFFLINE:  online / offline
+ * ETH_TEST_FL_FAILED: test passed / failed
+ * ETH_TEST_FL_EXTERNAL_LB: perform external loopback test
+ */
+
 enum ethtool_test_flags {
-	ETH_TEST_FL_OFFLINE	= (1 << 0),	/* online / offline */
-	ETH_TEST_FL_FAILED	= (1 << 1),	/* test passed / failed */
+	ETH_TEST_FL_OFFLINE	= (1 << 0),
+	ETH_TEST_FL_FAILED	= (1 << 1),
+	ETH_TEST_FL_EXTERNAL_LB	= (1 << 2),
 };
 
 /* for requesting NIC test and getting results*/
-- 
1.7.3.3


^ permalink raw reply related

* [PATCH NEXT 0/2]net:external loopback support in ethtool
From: amit.salecha @ 2011-06-28  6:40 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman


Hi,
	Adding external loopback support in ethtool self test. External loopback test
	requires special cable need to be connected, so it can not be part of default test.
	Adding separate flag to specify external loopback test.

-Amit

^ permalink raw reply

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

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>
---
 drivers/net/jme.c |   31 +++++++++++++++++++------------
 drivers/net/jme.h |    2 ++
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index 2ce9339..8887e2d 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
@@ -2529,8 +2527,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;
 }
@@ -3002,6 +2999,10 @@ jme_init_one(struct pci_dev *pdev,
 	jme->reg_pmcs = PMCS_MFEN;
 	jme->reg_gpreg1 = GPREG1_DEFAULT;
 
+	jme_clear_pm(jme);
+	pci_set_power_state(jme->pdev, PCI_D0);
+	device_set_wakeup_enable(&pdev->dev, true);
+
 	if (jme->reg_rxmcs & RXMCS_CHECKSUM)
 		netdev->features |= NETIF_F_RXCSUM;
 
@@ -3057,7 +3058,6 @@ jme_init_one(struct pci_dev *pdev,
 	jme->mii_if.mdio_read = jme_mdio_read;
 	jme->mii_if.mdio_write = jme_mdio_write;
 
-	jme_clear_pm(jme);
 	jme_set_phyfifo_5level(jme);
 	jme->pcirev = pdev->revision;
 	if (!jme->fpgaver)
@@ -3131,12 +3131,18 @@ jme_shutdown(struct pci_dev *pdev)
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct jme_adapter *jme = netdev_priv(netdev);
 
-	jme_powersave_phy(jme);
-	pci_pme_active(pdev, true);
+	if (jme->reg_pmcs) {
+		jme_powersave_phy(jme);
+		jme_clear_pm(jme);
+		pci_pme_active(pdev, true);
+	} else {
+		jme_phy_off(jme);
+	}
 }
 
 #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);
@@ -3171,18 +3177,19 @@ static int jme_suspend(struct device *dev)
 	tasklet_hi_enable(&jme->rxempty_task);
 
 	jme_powersave_phy(jme);
+	jme_clear_pm(jme);
 
 	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

* [PATCH net-next-2.6 1/2] jme: Fix compile warning introduced by new pm macro
From: Guo-Fu Tseng @ 2011-06-28  5:23 UTC (permalink / raw)
  To: David Miller; +Cc: Guo-Fu Tseng, linux-netdev, Aries Lee, Devinchiu

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

SIMPLE_DEV_PM_OPS is using SET_SYSTEM_SLEEP_PM_OPS
and SET_SYSTEM_SLEEP_PM_OPS is empty when CONFIG_PM_SLEEP
is not defined.

Switching #ifdef CONFIG_PM to #ifdef CONFIG_PM_SLEEP

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

diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index b5b174a..2ce9339 100644
--- a/drivers/net/jme.c
+++ b/drivers/net/jme.c
@@ -3135,7 +3135,7 @@ jme_shutdown(struct pci_dev *pdev)
 	pci_pme_active(pdev, true);
 }
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
 static int jme_suspend(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
-- 
1.7.3.4


^ permalink raw reply related

* Re: [PATCH 00/12] Fix various section mismatches and build errors.
From: David Miller @ 2011-06-28  5:12 UTC (permalink / raw)
  To: ralf
  Cc: akpm, alan, bcasavan, airlied, grundler, JBottomley, perex,
	rpurdie, klassert, tj, dri-devel, linux-kernel, linux-mips,
	linux-scsi, linux-serial, netdev
In-Reply-To: <17dd5038b15d7135791aadbe80464a13c80758d3.1309182742.git.ralf@linux-mips.org>


See commit:

commit 948252cb9e01d65a89ecadf67be5018351eee15e
Author: David S. Miller <davem@davemloft.net>
Date:   Tue May 31 19:27:48 2011 -0700

    Revert "net: fix section mismatches"
    
    This reverts commit e5cb966c0838e4da43a3b0751bdcac7fe719f7b4.
    
    It causes new build regressions with gcc-4.2 which is
    pretty common on non-x86 platforms.
    
    Reported-by: James Bottomley <James.Bottomley@HansenPartnership.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

and postings that led to this revert including:

http://marc.info/?l=linux-netdev&m=130653748205263&w=2

^ permalink raw reply

* [PATCH net-2.6] jme: Fix compile warning introduced by new pm macro
From: Guo-Fu Tseng @ 2011-06-28  5:12 UTC (permalink / raw)
  To: David Miller; +Cc: Guo-Fu Tseng, linux-netdev, Aries Lee, Devinchiu, stable

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

SIMPLE_DEV_PM_OPS is using SET_SYSTEM_SLEEP_PM_OPS
and SET_SYSTEM_SLEEP_PM_OPS is empty when CONFIG_PM_SLEEP
is not defined.

Switching #ifdef CONFIG_PM to #ifdef CONFIG_PM_SLEEP

Signed-off-by: Guo-Fu Tseng <cooldavid@cooldavid.org>
Cc: stable@kernel.org
---
 drivers/net/jme.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index b5b174a..2ce9339 100644
--- a/drivers/net/jme.c
+++ b/drivers/net/jme.c
@@ -3135,7 +3135,7 @@ jme_shutdown(struct pci_dev *pdev)
 	pci_pme_active(pdev, true);
 }
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
 static int jme_suspend(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
-- 
1.7.3.4


^ 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