Netdev List
 help / color / mirror / Atom feed
* Re: [patch net-next-2.6] net: allow notifier subscribers to forbid device from closing
From: Jiri Pirko @ 2011-08-31 16:21 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev, davem, eric.dumazet, shemminger
In-Reply-To: <1314804352.2741.4.camel@bwh-desktop>

Wed, Aug 31, 2011 at 05:25:51PM CEST, bhutchings@solarflare.com wrote:
>On Wed, 2011-08-31 at 17:15 +0200, Jiri Pirko wrote:
>> In some situations, like when the device is used as slave device in
>> bond/br/etc it is not nice if someone closes the device. This allows
>> it's masters to forbid this closure.
>
>No it doesn't.
It does

>
>[...]
>> @@ -1269,9 +1282,12 @@ static int dev_close_many(struct list_head *head)
>>  	struct net_device *dev, *tmp;
>>  	LIST_HEAD(tmp_list);
>>  
>> -	list_for_each_entry_safe(dev, tmp, head, unreg_list)
>> +	list_for_each_entry_safe(dev, tmp, head, unreg_list) {
>>  		if (!(dev->flags & IFF_UP))
>>  			list_move(&dev->unreg_list, &tmp_list);
>> +		else
>> +			__dev_pre_close(dev);
>> +	}
>>  
>>  	__dev_close_many(head);
>
>The return value is ignored here.

That's intended. The reason is this is called from
rollback_registered_many - refuse should be ignored in that case
>
>And this is called from dev_close(), where you are adding the
>notification as well.  So the notifier will usually be called twice.
>
Indeed. Anyway I thought about it and we probably do not need this patch
as Stephen said.

>[...]
>> @@ -1397,6 +1418,7 @@ rollback:
>>  				break;
>>  
>>  			if (dev->flags & IFF_UP) {
>> +				nb->notifier_call(nb, NETDEV_PRE_DOWN, dev);
>>  				nb->notifier_call(nb, NETDEV_GOING_DOWN, dev);
>>  				nb->notifier_call(nb, NETDEV_DOWN, dev);
>>  			}
>[...]
>
>The return value has to be ignored here.  Not sure it makes any sense to
>call the notifier at all.
>
>Ben.
>
>-- 
>Ben Hutchings, Staff Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>

^ permalink raw reply

* Re: [patch net-next-2.6] net: allow notifier subscribers to forbid device from closing
From: Jiri Pirko @ 2011-08-31 16:22 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Ben Hutchings, netdev, davem, eric.dumazet
In-Reply-To: <20110831085338.04c96e70@nehalam.ftrdhcpuser.net>

Wed, Aug 31, 2011 at 05:53:38PM CEST, shemminger@vyatta.com wrote:
>On Wed, 31 Aug 2011 16:25:51 +0100
>Ben Hutchings <bhutchings@solarflare.com> wrote:
>
>> On Wed, 2011-08-31 at 17:15 +0200, Jiri Pirko wrote:
>> > In some situations, like when the device is used as slave device in
>> > bond/br/etc it is not nice if someone closes the device. This allows
>> > it's masters to forbid this closure.
>> 
>> No it doesn't.
>> 
>> [...]
>> > @@ -1269,9 +1282,12 @@ static int dev_close_many(struct list_head *head)
>> >  	struct net_device *dev, *tmp;
>> >  	LIST_HEAD(tmp_list);
>> >  
>> > -	list_for_each_entry_safe(dev, tmp, head, unreg_list)
>> > +	list_for_each_entry_safe(dev, tmp, head, unreg_list) {
>> >  		if (!(dev->flags & IFF_UP))
>> >  			list_move(&dev->unreg_list, &tmp_list);
>> > +		else
>> > +			__dev_pre_close(dev);
>> > +	}
>> >  
>> >  	__dev_close_many(head);
>> 
>> The return value is ignored here.
>> 
>> And this is called from dev_close(), where you are adding the
>> notification as well.  So the notifier will usually be called twice.
>> 
>> [...]
>> > @@ -1397,6 +1418,7 @@ rollback:
>> >  				break;
>> >  
>> >  			if (dev->flags & IFF_UP) {
>> > +				nb->notifier_call(nb, NETDEV_PRE_DOWN, dev);
>> >  				nb->notifier_call(nb, NETDEV_GOING_DOWN, dev);
>> >  				nb->notifier_call(nb, NETDEV_DOWN, dev);
>> >  			}
>> [...]
>> 
>> The return value has to be ignored here.  Not sure it makes any sense to
>> call the notifier at all.
>> 
>> Ben.
>> 
>
>Also we need to allow rmmod'ing a network device even it is
>part of a bridge and that implicitly
>calls close.


this is not a problem because when it's called from
rollback_registered_many, return value if pre_down is ignored.

^ permalink raw reply

* [PATCH] net: Make flow cache namespace-aware
From: David Ward @ 2011-08-31 16:05 UTC (permalink / raw)
  To: netdev; +Cc: David Ward, Andrew Dickinson
In-Reply-To: <AANLkTinZJ9REmG0pHCVzEZ=tN-R4WuiQunuCBhmYtzyb@mail.gmail.com>

flow_cache_lookup will return a cached object (or null pointer) that the
resolver (i.e. xfrm_policy_lookup) previously found for another namespace
using the same key/family/dir.  Instead, make the namespace part of what
identifies entries in the cache.

As before, flow_entry_valid will return 0 for entries where the namespace
has been deleted, and they will be removed from the cache the next time
flow_cache_gc_task is run.

Reported-by: Andrew Dickinson <whydna@whydna.net>
Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
 net/core/flow.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/net/core/flow.c b/net/core/flow.c
index bf32c33..47b6d26 100644
--- a/net/core/flow.c
+++ b/net/core/flow.c
@@ -30,6 +30,7 @@ struct flow_cache_entry {
 		struct hlist_node	hlist;
 		struct list_head	gc_list;
 	} u;
+	struct net			*net;
 	u16				family;
 	u8				dir;
 	u32				genid;
@@ -232,7 +233,8 @@ flow_cache_lookup(struct net *net, const struct flowi *key, u16 family, u8 dir,
 
 	hash = flow_hash_code(fc, fcp, key);
 	hlist_for_each_entry(tfle, entry, &fcp->hash_table[hash], u.hlist) {
-		if (tfle->family == family &&
+		if (tfle->net == net &&
+		    tfle->family == family &&
 		    tfle->dir == dir &&
 		    flow_key_compare(key, &tfle->key) == 0) {
 			fle = tfle;
@@ -246,6 +248,7 @@ flow_cache_lookup(struct net *net, const struct flowi *key, u16 family, u8 dir,
 
 		fle = kmem_cache_alloc(flow_cachep, GFP_ATOMIC);
 		if (fle) {
+			fle->net = net;
 			fle->family = family;
 			fle->dir = dir;
 			memcpy(&fle->key, key, sizeof(*key));
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH] net: Initialize entire flowi struct
From: David Ward @ 2011-08-31 16:05 UTC (permalink / raw)
  To: netdev; +Cc: David Ward

The entire flowi struct needs to be initialized by afinfo->decode_session,
because flow_hash_code operates over the entire struct and may otherwise
return different hash values for what is intended to be the same key.

Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
 net/ipv4/xfrm4_policy.c |    2 +-
 net/ipv6/xfrm6_policy.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index fc5368a..afce24d 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -114,7 +114,7 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
 	u8 *xprth = skb_network_header(skb) + iph->ihl * 4;
 	struct flowi4 *fl4 = &fl->u.ip4;
 
-	memset(fl4, 0, sizeof(struct flowi4));
+	memset(fl, 0, sizeof(struct flowi));
 	fl4->flowi4_mark = skb->mark;
 
 	if (!ip_is_fragment(iph)) {
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index d879f7e..9088d38 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -129,7 +129,7 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
 	const unsigned char *nh = skb_network_header(skb);
 	u8 nexthdr = nh[IP6CB(skb)->nhoff];
 
-	memset(fl6, 0, sizeof(struct flowi6));
+	memset(fl, 0, sizeof(struct flowi));
 	fl6->flowi6_mark = skb->mark;
 
 	ipv6_addr_copy(&fl6->daddr, reverse ? &hdr->saddr : &hdr->daddr);
-- 
1.7.4.1

^ permalink raw reply related

* Re: [PATCH] virtio-net: Read MAC only after initializing MSI-X
From: Sasha Levin @ 2011-08-31 16:24 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Rusty Russell, linux-kernel, virtualization, netdev, kvm
In-Reply-To: <20110822083613.GA18556@redhat.com>

I'm wondering if we can switch to using a linked list 'capabilities'
structure similar to whats being done with PCI capabilities.

Here are the pros and the cons as I see them:

Pros:
 * Simpler code - currently before each access to the virtio config
space we have to check whether MSI-X is on and whether the device has
64bit features. This isn't necessarily slow, but it complicates the
code.

 * Future proof - code will be a mess once 5-6 features can change the
config space.

 * 'Concept reuse' - using same concepts in virtio-pci as the ones used
in PCI ('PCI Capabilities list') would make it easier to understand, and
would implement the same method to use optional features as in the layer
above.


Cons:
 * MSI-X is already moving the config space, we'll need to keep
supporting it for a while, but it would mean that it's the only thing we
need to keep backwards compatible.

 * 64bit features also move config space, but they're brand new in the
spec and aren't implemented in the kernel yet - I doubt anyone
implemented it anywhere else yet.

On Mon, 2011-08-22 at 11:36 +0300, Michael S. Tsirkin wrote:
> On Mon, Aug 22, 2011 at 09:54:50AM +0930, Rusty Russell wrote:
> > On Fri, 19 Aug 2011 18:23:35 +0300, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > On Sat, Aug 13, 2011 at 11:51:01AM +0300, Sasha Levin wrote:
> > > > The MAC of a virtio-net device is located at the first field of the device
> > > > specific header. This header is located at offset 20 if the device doesn't
> > > > support MSI-X or offset 24 if it does.
> > > > 
> > > > Current code in virtnet_probe() used to probe the MAC before checking for
> > > > MSI-X, which means that the read was always made from offset 20 regardless
> > > > of whether MSI-X in enabled or not.
> > > > 
> > > > This patch moves the MAC probe to after the detection of whether MSI-X is
> > > > enabled. This way the MAC will be read from offset 24 if the device indeed
> > > > supports MSI-X.
> > > > 
> > > > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > > Cc: virtualization@lists.linux-foundation.org
> > > > Cc: netdev@vger.kernel.org
> > > > Cc: kvm@vger.kernel.org
> > > > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > > 
> > > I am not sure I see a bug in virtio: the config pace layout simply
> > > changes as msix is enabled and disabled (and if you look at the latest
> > > draft, also on whether 64 bit features are enabled).
> > > It doesn't depend on msix capability being present in device.
> > > 
> > > The spec seems to be explicit enough:
> > > 	If MSI-X is enabled for the device, two additional fields immediately
> > > 	follow this header.
> > > 
> > > So I'm guessing the bug is in kvm tools which assume
> > > same layout for when msix is enabled and disabled.
> > > qemu-kvm seems to do the right thing so the device
> > > seems to get the correct mac.
> > 
> > So, the config space moves once MSI-X is enabled?  In which case, it
> > should say "ONCE MSI-X is enabled..."
> > 
> > Thanks,
> > Rusty.
> 
> Yes. Or maybe 'WHEN' - since if MSI-X is disabled again, it moves back.
> Let's update the spec to make it clearer?
> 

-- 

Sasha.

^ permalink raw reply

* Re: [PATCH RFT v3] bnx2: don't request firmware when there's no userspace.
From: Michael Chan @ 2011-08-31 16:32 UTC (permalink / raw)
  To: Francois Romieu; +Cc: davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <20110831045943.GB24254@electric-eye.fr.zoreil.com>


On Tue, 2011-08-30 at 21:59 -0700, Francois Romieu wrote:
> The firmware is cached during the first successful call to open() and
> released once the network device is unregistered. The driver uses the
> cached firmware between open() and unregister_netdev().
> 
> It's similar to 953a12cc2889d1be92e80a2d0bab5ffef4942300 but the
> firmware is mandatory.
> 
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
> Reviewed-by: Michael Chan <mchan@broadcom.com>

This version looks good.  If this is for net-next, please re-diff
against net-next which has moved all net drivers to new locations.
Thanks.

> ---
>  Since v2: remove erroneous __devinit.
>  Since v1: conditional release in bnx2_release_firmware.
> 
>  drivers/net/bnx2.c |   64 ++++++++++++++++++++++++++++++++-------------------
>  1 files changed, 40 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
> index 4b2b570..87a21c1 100644
> --- a/drivers/net/bnx2.c
> +++ b/drivers/net/bnx2.c
> @@ -3649,8 +3649,17 @@ check_mips_fw_entry(const struct firmware *fw,
>  	return 0;
>  }
>  
> -static int __devinit
> -bnx2_request_firmware(struct bnx2 *bp)
> +static void bnx2_release_firmware(struct bnx2 *bp)
> +{
> +	if (bp->rv2p_firmware) {
> +		release_firmware(bp->mips_firmware);
> +		release_firmware(bp->rv2p_firmware);
> +		bp->rv2p_firmware = NULL;
> +	}
> +}
> +
> +static int
> +bnx2_request_uncached_firmware(struct bnx2 *bp)
>  {
>  	const char *mips_fw_file, *rv2p_fw_file;
>  	const struct bnx2_mips_fw_file *mips_fw;
> @@ -3672,13 +3681,13 @@ bnx2_request_firmware(struct bnx2 *bp)
>  	rc = request_firmware(&bp->mips_firmware, mips_fw_file, &bp->pdev->dev);
>  	if (rc) {
>  		pr_err("Can't load firmware file \"%s\"\n", mips_fw_file);
> -		return rc;
> +		goto out;
>  	}
>  
>  	rc = request_firmware(&bp->rv2p_firmware, rv2p_fw_file, &bp->pdev->dev);
>  	if (rc) {
>  		pr_err("Can't load firmware file \"%s\"\n", rv2p_fw_file);
> -		return rc;
> +		goto err_release_mips_firmware;
>  	}
>  	mips_fw = (const struct bnx2_mips_fw_file *) bp->mips_firmware->data;
>  	rv2p_fw = (const struct bnx2_rv2p_fw_file *) bp->rv2p_firmware->data;
> @@ -3689,16 +3698,30 @@ bnx2_request_firmware(struct bnx2 *bp)
>  	    check_mips_fw_entry(bp->mips_firmware, &mips_fw->tpat) ||
>  	    check_mips_fw_entry(bp->mips_firmware, &mips_fw->txp)) {
>  		pr_err("Firmware file \"%s\" is invalid\n", mips_fw_file);
> -		return -EINVAL;
> +		rc = -EINVAL;
> +		goto err_release_firmware;
>  	}
>  	if (bp->rv2p_firmware->size < sizeof(*rv2p_fw) ||
>  	    check_fw_section(bp->rv2p_firmware, &rv2p_fw->proc1.rv2p, 8, true) ||
>  	    check_fw_section(bp->rv2p_firmware, &rv2p_fw->proc2.rv2p, 8, true)) {
>  		pr_err("Firmware file \"%s\" is invalid\n", rv2p_fw_file);
> -		return -EINVAL;
> +		rc = -EINVAL;
> +		goto err_release_firmware;
>  	}
> +out:
> +	return rc;
>  
> -	return 0;
> +err_release_firmware:
> +	release_firmware(bp->rv2p_firmware);
> +	bp->rv2p_firmware = NULL;
> +err_release_mips_firmware:
> +	release_firmware(bp->mips_firmware);
> +	goto out;
> +}
> +
> +static int bnx2_request_firmware(struct bnx2 *bp)
> +{
> +	return bp->rv2p_firmware ? 0 : bnx2_request_uncached_firmware(bp);
>  }
>  
>  static u32
> @@ -6266,6 +6289,10 @@ bnx2_open(struct net_device *dev)
>  	struct bnx2 *bp = netdev_priv(dev);
>  	int rc;
>  
> +	rc = bnx2_request_firmware(bp);
> +	if (rc < 0)
> +		goto out;
> +
>  	netif_carrier_off(dev);
>  
>  	bnx2_set_power_state(bp, PCI_D0);
> @@ -6326,8 +6353,8 @@ bnx2_open(struct net_device *dev)
>  		netdev_info(dev, "using MSIX\n");
>  
>  	netif_tx_start_all_queues(dev);
> -
> -	return 0;
> +out:
> +	return rc;
>  
>  open_err:
>  	bnx2_napi_disable(bp);
> @@ -6335,7 +6362,8 @@ open_err:
>  	bnx2_free_irq(bp);
>  	bnx2_free_mem(bp);
>  	bnx2_del_napi(bp);
> -	return rc;
> +	bnx2_release_firmware(bp);
> +	goto out;
>  }
>  
>  static void
> @@ -8353,10 +8381,6 @@ bnx2_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	pci_set_drvdata(pdev, dev);
>  
> -	rc = bnx2_request_firmware(bp);
> -	if (rc)
> -		goto error;
> -
>  	memcpy(dev->dev_addr, bp->mac_addr, 6);
>  	memcpy(dev->perm_addr, bp->mac_addr, 6);
>  
> @@ -8387,11 +8411,6 @@ bnx2_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	return 0;
>  
>  error:
> -	if (bp->mips_firmware)
> -		release_firmware(bp->mips_firmware);
> -	if (bp->rv2p_firmware)
> -		release_firmware(bp->rv2p_firmware);
> -
>  	if (bp->regview)
>  		iounmap(bp->regview);
>  	pci_release_regions(pdev);
> @@ -8412,11 +8431,6 @@ bnx2_remove_one(struct pci_dev *pdev)
>  	del_timer_sync(&bp->timer);
>  	cancel_work_sync(&bp->reset_task);
>  
> -	if (bp->mips_firmware)
> -		release_firmware(bp->mips_firmware);
> -	if (bp->rv2p_firmware)
> -		release_firmware(bp->rv2p_firmware);
> -
>  	if (bp->regview)
>  		iounmap(bp->regview);
>  
> @@ -8427,6 +8441,8 @@ bnx2_remove_one(struct pci_dev *pdev)
>  		bp->flags &= ~BNX2_FLAG_AER_ENABLED;
>  	}
>  
> +	bnx2_release_firmware(bp);
> +
>  	free_netdev(dev);
>  
>  	pci_release_regions(pdev);

^ permalink raw reply

* Re: [PATCH 07/24] can: Remove unnecessary OOM logging messages
From: Oliver Hartkopp @ 2011-08-31 17:11 UTC (permalink / raw)
  To: Joe Perches; +Cc: Urs Thuermann, David S. Miller, netdev, linux-kernel
In-Reply-To: <d2773db0709a09bfe6d43a28ba1323c5d3f9a76b.1314650069.git.joe@perches.com>

On 29.08.2011 23:17, Joe Perches wrote:

> Removing unnecessary messages saves code and text.
> 
> Site specific OOM messages are duplications of a generic MM
> out of memory message and aren't really useful, so just
> delete them.
> 
> Signed-off-by: Joe Perches <joe@perches.com>


Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>

Tnx Joe.

> ---
>  net/can/af_can.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/net/can/af_can.c b/net/can/af_can.c
> index b9efa94..11300be 100644
> --- a/net/can/af_can.c
> +++ b/net/can/af_can.c
> @@ -770,11 +770,9 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
>  
>  		/* create new dev_rcv_lists for this device */
>  		d = kzalloc(sizeof(*d), GFP_KERNEL);
> -		if (!d) {
> -			printk(KERN_ERR
> -			       "can: allocation of receive list failed\n");
> +		if (!d)
>  			return NOTIFY_DONE;
> -		}
> +
>  		BUG_ON(dev->ml_priv);
>  		dev->ml_priv = d;
>  

^ permalink raw reply

* Re: [PATCH 1/1] net/can/af_can.c: Change del_timer to del_timer_sync
From: Oliver Hartkopp @ 2011-08-31 17:28 UTC (permalink / raw)
  To: Rajan Aggarwal; +Cc: Urs Thuermann, David S. Miller, netdev
In-Reply-To: <1314784658-23938-1-git-send-email-rajan.aggarwal85@gmail.com>

On 31.08.2011 11:57, Rajan Aggarwal wrote:

> From: Rajan Aggarwal <Rajan Aggarwal rajan.aggarwal85@gmail.com>
> 
> This is important for SMP platform to check if timer function is
> executing on other CPU with deleting the timer.
> 
> Signed-off-by: Rajan Aggarwal <Rajan Aggarwal rajan.aggarwal85@gmail.com>
> ---


Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>

Tnx Rajan.

>  net/can/af_can.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/can/af_can.c b/net/can/af_can.c
> index 8ce926d..9b0c32a 100644
> --- a/net/can/af_can.c
> +++ b/net/can/af_can.c
> @@ -857,7 +857,7 @@ static __exit void can_exit(void)
>  	struct net_device *dev;
>  
>  	if (stats_timer)
> -		del_timer(&can_stattimer);
> +		del_timer_sync(&can_stattimer);
>  
>  	can_remove_proc();
>  

^ permalink raw reply

* Re: [PATCH] mii: Remove references to DP83840 PHY in mii.h
From: Ben Hutchings @ 2011-08-31 17:34 UTC (permalink / raw)
  To: Mark Einon; +Cc: netdev, davem
In-Reply-To: <1314804928-7353-1-git-send-email-mark.einon@gmail.com>

On Wed, 2011-08-31 at 16:35 +0100, Mark Einon wrote:
> There are references to this PHY chip in the generic mii.h header, so removing them.
> 
> Signed-off-by: Mark Einon <mark.einon@gmail.com>
> ---
>  include/linux/mii.h |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/mii.h b/include/linux/mii.h
> index 103113a..4c3cfb5 100644
> --- a/include/linux/mii.h
> +++ b/include/linux/mii.h
> @@ -40,12 +40,12 @@
>  #define BMCR_CTST               0x0080  /* Collision test              */
>  #define BMCR_FULLDPLX           0x0100  /* Full duplex                 */
>  #define BMCR_ANRESTART          0x0200  /* Auto negotiation restart    */
> -#define BMCR_ISOLATE            0x0400  /* Disconnect DP83840 from MII */
> -#define BMCR_PDOWN              0x0800  /* Powerdown the DP83840       */
> +#define BMCR_ISOLATE            0x0400  /* Disconnect PHY from MII     */

Since you're trying to improve these comments, I think this could do
with further improvement.  When this bit is set, the PHY's data paths
are isolated from the MII (or other interface to the MAC).  The control
path is still connected to the management interface (MDIO), which is
important when we want to clear this bit!  So it would be better to say
something like 'Isolate data paths fromn MII'.

> +#define BMCR_PDOWN              0x0800  /* Powerdown                   */

This selects a low-power state (if implemented).  It doesn't entirely
turn the PHY off, and at least the management interface must stil be
functional.  So it would be better to say 'Request low-power state'.

>  #define BMCR_ANENABLE           0x1000  /* Enable auto negotiation     */
>  #define BMCR_SPEED100           0x2000  /* Select 100Mbps              */
>  #define BMCR_LOOPBACK           0x4000  /* TXD loopback bits           */
> -#define BMCR_RESET              0x8000  /* Reset the DP83840           */
> +#define BMCR_RESET              0x8000  /* Reset                       */
>  
>  /* Basic mode status register. */
>  #define BMSR_ERCAP              0x0001  /* Ext-reg capability          */
> @@ -55,9 +55,9 @@
>  #define BMSR_RFAULT             0x0010  /* Remote fault detected       */
>  #define BMSR_ANEGCOMPLETE       0x0020  /* Auto-negotiation complete   */
>  #define BMSR_RESV               0x00c0  /* Unused...                   */
> -#define BMSR_ESTATEN		0x0100	/* Extended Status in R15 */
> -#define BMSR_100HALF2           0x0200  /* Can do 100BASE-T2 HDX */
> -#define BMSR_100FULL2           0x0400  /* Can do 100BASE-T2 FDX */
> +#define BMSR_ESTATEN		0x0100	/* Extended Status in R15      */
> +#define BMSR_100HALF2           0x0200  /* Can do 100BASE-T2 HDX       */
> +#define BMSR_100FULL2           0x0400  /* Can do 100BASE-T2 FDX       */

This formatting change is unrelated.  If you're going to fix formatting
then please convert spaces to tabs after each name and value.

Ben.

>  #define BMSR_10HALF             0x0800  /* Can do 10mbps, half-duplex  */
>  #define BMSR_10FULL             0x1000  /* Can do 10mbps, full-duplex  */
>  #define BMSR_100HALF            0x2000  /* Can do 100mbps, half-duplex */

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

^ permalink raw reply

* Re: [PATCH 1/3] bnx2x: remove TPA_ENABLE_FLAG
From: Michal Schmidt @ 2011-08-31 16:55 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: Vlad Zolotarov, Dmitry Kravkov, Eilon Greenstein,
	mirqus@gmail.com
In-Reply-To: <201108311816.30468.vladz@broadcom.com>

On Wed, 31 Aug 2011 18:16:30 +0300 Vlad Zolotarov wrote:
> This is bogus - what if bnx2x_reload_if_running(dev)
> (bnx2x_nic_load()) failes? The original dev->features would be
> lost... Pls., see my latest response on your previouse patch series
> thread.

OK, so the conclusion in the other thread was to restore the features
before returning. Here's an updated patch.


Subject: [PATCH] bnx2x: remove TPA_ENABLE_FLAG

TPA_ENABLE_FLAG is unnecessary, because it mirrors NETIF_F_LRO.

"Cheating" in bnx2x_set_features() was suggested by Michał Mirosław.

Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x.h      |    2 +-
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c  |   31 ++++++++++++---------
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |    7 +---
 3 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
index 735e491..5d5f323 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
@@ -1174,7 +1174,7 @@ struct bnx2x {
 #define USING_MSIX_FLAG			(1 << 5)
 #define USING_MSI_FLAG			(1 << 6)
 #define DISABLE_MSI_FLAG		(1 << 7)
-#define TPA_ENABLE_FLAG			(1 << 8)
+
 #define NO_MCP_FLAG			(1 << 9)
 
 #define BP_NOMCP(bp)			(bp->flags & NO_MCP_FLAG)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index a08b101..399e71c 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -62,7 +62,7 @@ static inline void bnx2x_bz_fp(struct bnx2x *bp, int index)
 	 * set the tpa flag for each queue. The tpa flag determines the queue
 	 * minimal size so it must be set prior to queue memory allocation
 	 */
-	fp->disable_tpa = ((bp->flags & TPA_ENABLE_FLAG) == 0);
+	fp->disable_tpa = !(bp->dev->features & NETIF_F_LRO);
 
 #ifdef BCM_CNIC
 	/* We don't want TPA on an FCoE L2 ring */
@@ -3410,13 +3410,10 @@ u32 bnx2x_fix_features(struct net_device *dev, u32 features)
 int bnx2x_set_features(struct net_device *dev, u32 features)
 {
 	struct bnx2x *bp = netdev_priv(dev);
-	u32 flags = bp->flags;
 	bool bnx2x_reload = false;
 
-	if (features & NETIF_F_LRO)
-		flags |= TPA_ENABLE_FLAG;
-	else
-		flags &= ~TPA_ENABLE_FLAG;
+	if ((features ^ dev->features) & NETIF_F_LRO)
+		bnx2x_reload = true;
 
 	if (features & NETIF_F_LOOPBACK) {
 		if (bp->link_params.loopback_mode != LOOPBACK_BMAC) {
@@ -3430,14 +3427,22 @@ int bnx2x_set_features(struct net_device *dev, u32 features)
 		}
 	}
 
-	if (flags ^ bp->flags) {
-		bp->flags = flags;
-		bnx2x_reload = true;
-	}
-
 	if (bnx2x_reload) {
-		if (bp->recovery_state == BNX2X_RECOVERY_DONE)
-			return bnx2x_reload_if_running(dev);
+		if (bp->recovery_state == BNX2X_RECOVERY_DONE) {
+			/*
+			 * Cheat! Normally dev->features will be set after we
+			 * return, but that's too late. We need to know how to
+			 * configure the NIC when reloading it, so copy the
+			 * features beforehand. Restore them after reload to
+			 * avoid any possible confusion of the caller.
+			 */
+			int ret;
+			u32 orig_features = dev->features;
+			dev->features = features;
+			ret = bnx2x_reload_if_running(dev);
+			dev->features = orig_features;
+			return ret;
+		}
 		/* else: bnx2x_nic_load() will be called at end of recovery */
 	}
 
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index e7b584b..fd32c04 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -9757,13 +9757,10 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
 	bp->multi_mode = multi_mode;
 
 	/* Set TPA flags */
-	if (disable_tpa) {
-		bp->flags &= ~TPA_ENABLE_FLAG;
+	if (disable_tpa)
 		bp->dev->features &= ~NETIF_F_LRO;
-	} else {
-		bp->flags |= TPA_ENABLE_FLAG;
+	else
 		bp->dev->features |= NETIF_F_LRO;
-	}
 	bp->disable_tpa = disable_tpa;
 
 	if (CHIP_IS_E1(bp))
-- 
1.7.6

^ permalink raw reply related

* Re: [PATCH] ip route: don't implicitly filter address family
From: Stephen Hemminger @ 2011-08-31 17:48 UTC (permalink / raw)
  To: Dan McGee; +Cc: netdev
In-Reply-To: <1310755113-32492-1-git-send-email-dan@archlinux.org>

On Fri, 15 Jul 2011 13:38:33 -0500
Dan McGee <dan@archlinux.org> wrote:

> When looking at the output of 'ip route show' on an IPv6 capable
> machine, the default output doesn't show IPv6 routes. This is unlike any
> other ip subcommand, which show all address families by default.
> Furthermore, the behavior is totally bogus for different permutations of
> the command:
> 
>     ip route show : will show IPv4, not show IPv6
> 	ip route show table all : will show IPv4 and IPv6
> 	ip route -f link show : will show IPv4 and IPv6
> 
> Rectify this behavior by removing some 2004 code that tries to munge the
> address family if it is unspecified and fails bad at guessing the user's
> intention.
> 
> Signed-off-by: Dan McGee <dan@archlinux.org>
> ---
>  ip/iproute.c |    3 ---
>  1 files changed, 0 insertions(+), 3 deletions(-)
> 
> diff --git a/ip/iproute.c b/ip/iproute.c
> index ca09029..4458d9c 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -1236,9 +1236,6 @@ static int iproute_list_flush_or_save(int argc, char **argv, int action)
>  		argc--; argv++;
>  	}
>  
> -	if (do_ipv6 == AF_UNSPEC && filter.tb)
> -		do_ipv6 = AF_INET;
> -
>  	ll_init_map(&rth);
>  
>  	if (id || od)  {

No, this will change the output for unsuspecting users and likely
break their scripts.

^ permalink raw reply

* [PATCH net-next 0/8] tg3: Odds and ends
From: Matt Carlson @ 2011-08-31 21:44 UTC (permalink / raw)
  To: davem; +Cc: netdev, mcarlson

This patchset applies a few minor changes and cleans up some unnecessary function prototypes.

^ permalink raw reply

* [PATCH net-next 1/8] tg3: Check all adv bits when checking config
From: Matt Carlson @ 2011-08-31 21:44 UTC (permalink / raw)
  To: davem; +Cc: netdev, mcarlson

This patch makes sure the driver checks all advertisement bits when
checking the current hw advertisements.

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Benjamin Li <benli@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 0f81111..de98c10 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -3334,8 +3334,9 @@ static int tg3_copper_is_advertising_all(struct tg3 *tp, u32 mask)
 	if (tg3_readphy(tp, MII_ADVERTISE, &adv_reg))
 		return 0;
 
-	if ((adv_reg & all_mask) != all_mask)
+	if ((adv_reg & ADVERTISE_ALL) != all_mask)
 		return 0;
+
 	if (!(tp->phy_flags & TG3_PHYFLG_10_100_ONLY)) {
 		u32 tg3_ctrl;
 
@@ -3348,7 +3349,8 @@ static int tg3_copper_is_advertising_all(struct tg3 *tp, u32 mask)
 		if (tg3_readphy(tp, MII_CTRL1000, &tg3_ctrl))
 			return 0;
 
-		if ((tg3_ctrl & all_mask) != all_mask)
+		tg3_ctrl &= (ADVERTISE_1000HALF | ADVERTISE_1000FULL);
+		if (tg3_ctrl != all_mask)
 			return 0;
 	}
 	return 1;
-- 
1.7.3.4

^ permalink raw reply related

* [PATCH net-next 2/8] tg3: Fix missed MSI workaround
From: Matt Carlson @ 2011-08-31 21:44 UTC (permalink / raw)
  To: davem; +Cc: netdev, mcarlson

This patch fixes a minor counter initialization bug and makes the MSI
workaround slightly more efficient by attempting to service pending
interrupts before applying the workaround.

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Benjamin Li <benli@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index de98c10..ab4c598 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -8107,7 +8107,7 @@ static void tg3_rings_reset(struct tg3 *tp)
 				tw32_mailbox(tp->napi[i].prodmbox, 0);
 			tw32_rx_mbox(tp->napi[i].consmbox, 0);
 			tw32_mailbox_f(tp->napi[i].int_mbox, 1);
-			tp->napi[0].chk_msi_cnt = 0;
+			tp->napi[i].chk_msi_cnt = 0;
 			tp->napi[i].last_rx_cons = 0;
 			tp->napi[i].last_tx_cons = 0;
 		}
@@ -9187,8 +9187,7 @@ static void tg3_chk_missed_msi(struct tg3 *tp)
 					tnapi->chk_msi_cnt++;
 					return;
 				}
-				tw32_mailbox(tnapi->int_mbox,
-					     tnapi->last_tag << 24);
+				tg3_msi(0, tnapi);
 			}
 		}
 		tnapi->chk_msi_cnt = 0;
-- 
1.7.3.4

^ permalink raw reply related

* [PATCH net-next 7/8] tg3: Eliminate tg3_halt_cpu() prototype
From: Matt Carlson @ 2011-08-31 21:44 UTC (permalink / raw)
  To: davem; +Cc: netdev, mcarlson

This patch moves the implementatino of tg3_halt_cpu() earlier in the
file to eliminate its prototype.

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Benjamin Li <benli@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c |  443 ++++++++++++++++++-----------------
 1 files changed, 222 insertions(+), 221 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 272aa11..88426b2 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -2999,6 +2999,228 @@ static int tg3_nvram_read_be32(struct tg3 *tp, u32 offset, __be32 *val)
 	return res;
 }
 
+#define RX_CPU_SCRATCH_BASE	0x30000
+#define RX_CPU_SCRATCH_SIZE	0x04000
+#define TX_CPU_SCRATCH_BASE	0x34000
+#define TX_CPU_SCRATCH_SIZE	0x04000
+
+/* tp->lock is held. */
+static int tg3_halt_cpu(struct tg3 *tp, u32 offset)
+{
+	int i;
+
+	BUG_ON(offset == TX_CPU_BASE && tg3_flag(tp, 5705_PLUS));
+
+	if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5906) {
+		u32 val = tr32(GRC_VCPU_EXT_CTRL);
+
+		tw32(GRC_VCPU_EXT_CTRL, val | GRC_VCPU_EXT_CTRL_HALT_CPU);
+		return 0;
+	}
+	if (offset == RX_CPU_BASE) {
+		for (i = 0; i < 10000; i++) {
+			tw32(offset + CPU_STATE, 0xffffffff);
+			tw32(offset + CPU_MODE,  CPU_MODE_HALT);
+			if (tr32(offset + CPU_MODE) & CPU_MODE_HALT)
+				break;
+		}
+
+		tw32(offset + CPU_STATE, 0xffffffff);
+		tw32_f(offset + CPU_MODE,  CPU_MODE_HALT);
+		udelay(10);
+	} else {
+		for (i = 0; i < 10000; i++) {
+			tw32(offset + CPU_STATE, 0xffffffff);
+			tw32(offset + CPU_MODE,  CPU_MODE_HALT);
+			if (tr32(offset + CPU_MODE) & CPU_MODE_HALT)
+				break;
+		}
+	}
+
+	if (i >= 10000) {
+		netdev_err(tp->dev, "%s timed out, %s CPU\n",
+			   __func__, offset == RX_CPU_BASE ? "RX" : "TX");
+		return -ENODEV;
+	}
+
+	/* Clear firmware's nvram arbitration. */
+	if (tg3_flag(tp, NVRAM))
+		tw32(NVRAM_SWARB, SWARB_REQ_CLR0);
+	return 0;
+}
+
+struct fw_info {
+	unsigned int fw_base;
+	unsigned int fw_len;
+	const __be32 *fw_data;
+};
+
+/* tp->lock is held. */
+static int tg3_load_firmware_cpu(struct tg3 *tp, u32 cpu_base,
+				 u32 cpu_scratch_base, int cpu_scratch_size,
+				 struct fw_info *info)
+{
+	int err, lock_err, i;
+	void (*write_op)(struct tg3 *, u32, u32);
+
+	if (cpu_base == TX_CPU_BASE && tg3_flag(tp, 5705_PLUS)) {
+		netdev_err(tp->dev,
+			   "%s: Trying to load TX cpu firmware which is 5705\n",
+			   __func__);
+		return -EINVAL;
+	}
+
+	if (tg3_flag(tp, 5705_PLUS))
+		write_op = tg3_write_mem;
+	else
+		write_op = tg3_write_indirect_reg32;
+
+	/* It is possible that bootcode is still loading at this point.
+	 * Get the nvram lock first before halting the cpu.
+	 */
+	lock_err = tg3_nvram_lock(tp);
+	err = tg3_halt_cpu(tp, cpu_base);
+	if (!lock_err)
+		tg3_nvram_unlock(tp);
+	if (err)
+		goto out;
+
+	for (i = 0; i < cpu_scratch_size; i += sizeof(u32))
+		write_op(tp, cpu_scratch_base + i, 0);
+	tw32(cpu_base + CPU_STATE, 0xffffffff);
+	tw32(cpu_base + CPU_MODE, tr32(cpu_base+CPU_MODE)|CPU_MODE_HALT);
+	for (i = 0; i < (info->fw_len / sizeof(u32)); i++)
+		write_op(tp, (cpu_scratch_base +
+			      (info->fw_base & 0xffff) +
+			      (i * sizeof(u32))),
+			      be32_to_cpu(info->fw_data[i]));
+
+	err = 0;
+
+out:
+	return err;
+}
+
+/* tp->lock is held. */
+static int tg3_load_5701_a0_firmware_fix(struct tg3 *tp)
+{
+	struct fw_info info;
+	const __be32 *fw_data;
+	int err, i;
+
+	fw_data = (void *)tp->fw->data;
+
+	/* Firmware blob starts with version numbers, followed by
+	   start address and length. We are setting complete length.
+	   length = end_address_of_bss - start_address_of_text.
+	   Remainder is the blob to be loaded contiguously
+	   from start address. */
+
+	info.fw_base = be32_to_cpu(fw_data[1]);
+	info.fw_len = tp->fw->size - 12;
+	info.fw_data = &fw_data[3];
+
+	err = tg3_load_firmware_cpu(tp, RX_CPU_BASE,
+				    RX_CPU_SCRATCH_BASE, RX_CPU_SCRATCH_SIZE,
+				    &info);
+	if (err)
+		return err;
+
+	err = tg3_load_firmware_cpu(tp, TX_CPU_BASE,
+				    TX_CPU_SCRATCH_BASE, TX_CPU_SCRATCH_SIZE,
+				    &info);
+	if (err)
+		return err;
+
+	/* Now startup only the RX cpu. */
+	tw32(RX_CPU_BASE + CPU_STATE, 0xffffffff);
+	tw32_f(RX_CPU_BASE + CPU_PC, info.fw_base);
+
+	for (i = 0; i < 5; i++) {
+		if (tr32(RX_CPU_BASE + CPU_PC) == info.fw_base)
+			break;
+		tw32(RX_CPU_BASE + CPU_STATE, 0xffffffff);
+		tw32(RX_CPU_BASE + CPU_MODE,  CPU_MODE_HALT);
+		tw32_f(RX_CPU_BASE + CPU_PC, info.fw_base);
+		udelay(1000);
+	}
+	if (i >= 5) {
+		netdev_err(tp->dev, "%s fails to set RX CPU PC, is %08x "
+			   "should be %08x\n", __func__,
+			   tr32(RX_CPU_BASE + CPU_PC), info.fw_base);
+		return -ENODEV;
+	}
+	tw32(RX_CPU_BASE + CPU_STATE, 0xffffffff);
+	tw32_f(RX_CPU_BASE + CPU_MODE,  0x00000000);
+
+	return 0;
+}
+
+/* tp->lock is held. */
+static int tg3_load_tso_firmware(struct tg3 *tp)
+{
+	struct fw_info info;
+	const __be32 *fw_data;
+	unsigned long cpu_base, cpu_scratch_base, cpu_scratch_size;
+	int err, i;
+
+	if (tg3_flag(tp, HW_TSO_1) ||
+	    tg3_flag(tp, HW_TSO_2) ||
+	    tg3_flag(tp, HW_TSO_3))
+		return 0;
+
+	fw_data = (void *)tp->fw->data;
+
+	/* Firmware blob starts with version numbers, followed by
+	   start address and length. We are setting complete length.
+	   length = end_address_of_bss - start_address_of_text.
+	   Remainder is the blob to be loaded contiguously
+	   from start address. */
+
+	info.fw_base = be32_to_cpu(fw_data[1]);
+	cpu_scratch_size = tp->fw_len;
+	info.fw_len = tp->fw->size - 12;
+	info.fw_data = &fw_data[3];
+
+	if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5705) {
+		cpu_base = RX_CPU_BASE;
+		cpu_scratch_base = NIC_SRAM_MBUF_POOL_BASE5705;
+	} else {
+		cpu_base = TX_CPU_BASE;
+		cpu_scratch_base = TX_CPU_SCRATCH_BASE;
+		cpu_scratch_size = TX_CPU_SCRATCH_SIZE;
+	}
+
+	err = tg3_load_firmware_cpu(tp, cpu_base,
+				    cpu_scratch_base, cpu_scratch_size,
+				    &info);
+	if (err)
+		return err;
+
+	/* Now startup the cpu. */
+	tw32(cpu_base + CPU_STATE, 0xffffffff);
+	tw32_f(cpu_base + CPU_PC, info.fw_base);
+
+	for (i = 0; i < 5; i++) {
+		if (tr32(cpu_base + CPU_PC) == info.fw_base)
+			break;
+		tw32(cpu_base + CPU_STATE, 0xffffffff);
+		tw32(cpu_base + CPU_MODE,  CPU_MODE_HALT);
+		tw32_f(cpu_base + CPU_PC, info.fw_base);
+		udelay(1000);
+	}
+	if (i >= 5) {
+		netdev_err(tp->dev,
+			   "%s fails to set CPU PC, is %08x should be %08x\n",
+			   __func__, tr32(cpu_base + CPU_PC), info.fw_base);
+		return -ENODEV;
+	}
+	tw32(cpu_base + CPU_STATE, 0xffffffff);
+	tw32_f(cpu_base + CPU_MODE,  0x00000000);
+	return 0;
+}
+
+
 /* tp->lock is held. */
 static void __tg3_set_mac_addr(struct tg3 *tp, int skip_mac_1)
 {
@@ -7709,227 +7931,6 @@ static int tg3_halt(struct tg3 *tp, int kind, int silent)
 	return 0;
 }
 
-#define RX_CPU_SCRATCH_BASE	0x30000
-#define RX_CPU_SCRATCH_SIZE	0x04000
-#define TX_CPU_SCRATCH_BASE	0x34000
-#define TX_CPU_SCRATCH_SIZE	0x04000
-
-/* tp->lock is held. */
-static int tg3_halt_cpu(struct tg3 *tp, u32 offset)
-{
-	int i;
-
-	BUG_ON(offset == TX_CPU_BASE && tg3_flag(tp, 5705_PLUS));
-
-	if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5906) {
-		u32 val = tr32(GRC_VCPU_EXT_CTRL);
-
-		tw32(GRC_VCPU_EXT_CTRL, val | GRC_VCPU_EXT_CTRL_HALT_CPU);
-		return 0;
-	}
-	if (offset == RX_CPU_BASE) {
-		for (i = 0; i < 10000; i++) {
-			tw32(offset + CPU_STATE, 0xffffffff);
-			tw32(offset + CPU_MODE,  CPU_MODE_HALT);
-			if (tr32(offset + CPU_MODE) & CPU_MODE_HALT)
-				break;
-		}
-
-		tw32(offset + CPU_STATE, 0xffffffff);
-		tw32_f(offset + CPU_MODE,  CPU_MODE_HALT);
-		udelay(10);
-	} else {
-		for (i = 0; i < 10000; i++) {
-			tw32(offset + CPU_STATE, 0xffffffff);
-			tw32(offset + CPU_MODE,  CPU_MODE_HALT);
-			if (tr32(offset + CPU_MODE) & CPU_MODE_HALT)
-				break;
-		}
-	}
-
-	if (i >= 10000) {
-		netdev_err(tp->dev, "%s timed out, %s CPU\n",
-			   __func__, offset == RX_CPU_BASE ? "RX" : "TX");
-		return -ENODEV;
-	}
-
-	/* Clear firmware's nvram arbitration. */
-	if (tg3_flag(tp, NVRAM))
-		tw32(NVRAM_SWARB, SWARB_REQ_CLR0);
-	return 0;
-}
-
-struct fw_info {
-	unsigned int fw_base;
-	unsigned int fw_len;
-	const __be32 *fw_data;
-};
-
-/* tp->lock is held. */
-static int tg3_load_firmware_cpu(struct tg3 *tp, u32 cpu_base, u32 cpu_scratch_base,
-				 int cpu_scratch_size, struct fw_info *info)
-{
-	int err, lock_err, i;
-	void (*write_op)(struct tg3 *, u32, u32);
-
-	if (cpu_base == TX_CPU_BASE && tg3_flag(tp, 5705_PLUS)) {
-		netdev_err(tp->dev,
-			   "%s: Trying to load TX cpu firmware which is 5705\n",
-			   __func__);
-		return -EINVAL;
-	}
-
-	if (tg3_flag(tp, 5705_PLUS))
-		write_op = tg3_write_mem;
-	else
-		write_op = tg3_write_indirect_reg32;
-
-	/* It is possible that bootcode is still loading at this point.
-	 * Get the nvram lock first before halting the cpu.
-	 */
-	lock_err = tg3_nvram_lock(tp);
-	err = tg3_halt_cpu(tp, cpu_base);
-	if (!lock_err)
-		tg3_nvram_unlock(tp);
-	if (err)
-		goto out;
-
-	for (i = 0; i < cpu_scratch_size; i += sizeof(u32))
-		write_op(tp, cpu_scratch_base + i, 0);
-	tw32(cpu_base + CPU_STATE, 0xffffffff);
-	tw32(cpu_base + CPU_MODE, tr32(cpu_base+CPU_MODE)|CPU_MODE_HALT);
-	for (i = 0; i < (info->fw_len / sizeof(u32)); i++)
-		write_op(tp, (cpu_scratch_base +
-			      (info->fw_base & 0xffff) +
-			      (i * sizeof(u32))),
-			      be32_to_cpu(info->fw_data[i]));
-
-	err = 0;
-
-out:
-	return err;
-}
-
-/* tp->lock is held. */
-static int tg3_load_5701_a0_firmware_fix(struct tg3 *tp)
-{
-	struct fw_info info;
-	const __be32 *fw_data;
-	int err, i;
-
-	fw_data = (void *)tp->fw->data;
-
-	/* Firmware blob starts with version numbers, followed by
-	   start address and length. We are setting complete length.
-	   length = end_address_of_bss - start_address_of_text.
-	   Remainder is the blob to be loaded contiguously
-	   from start address. */
-
-	info.fw_base = be32_to_cpu(fw_data[1]);
-	info.fw_len = tp->fw->size - 12;
-	info.fw_data = &fw_data[3];
-
-	err = tg3_load_firmware_cpu(tp, RX_CPU_BASE,
-				    RX_CPU_SCRATCH_BASE, RX_CPU_SCRATCH_SIZE,
-				    &info);
-	if (err)
-		return err;
-
-	err = tg3_load_firmware_cpu(tp, TX_CPU_BASE,
-				    TX_CPU_SCRATCH_BASE, TX_CPU_SCRATCH_SIZE,
-				    &info);
-	if (err)
-		return err;
-
-	/* Now startup only the RX cpu. */
-	tw32(RX_CPU_BASE + CPU_STATE, 0xffffffff);
-	tw32_f(RX_CPU_BASE + CPU_PC, info.fw_base);
-
-	for (i = 0; i < 5; i++) {
-		if (tr32(RX_CPU_BASE + CPU_PC) == info.fw_base)
-			break;
-		tw32(RX_CPU_BASE + CPU_STATE, 0xffffffff);
-		tw32(RX_CPU_BASE + CPU_MODE,  CPU_MODE_HALT);
-		tw32_f(RX_CPU_BASE + CPU_PC, info.fw_base);
-		udelay(1000);
-	}
-	if (i >= 5) {
-		netdev_err(tp->dev, "%s fails to set RX CPU PC, is %08x "
-			   "should be %08x\n", __func__,
-			   tr32(RX_CPU_BASE + CPU_PC), info.fw_base);
-		return -ENODEV;
-	}
-	tw32(RX_CPU_BASE + CPU_STATE, 0xffffffff);
-	tw32_f(RX_CPU_BASE + CPU_MODE,  0x00000000);
-
-	return 0;
-}
-
-/* tp->lock is held. */
-static int tg3_load_tso_firmware(struct tg3 *tp)
-{
-	struct fw_info info;
-	const __be32 *fw_data;
-	unsigned long cpu_base, cpu_scratch_base, cpu_scratch_size;
-	int err, i;
-
-	if (tg3_flag(tp, HW_TSO_1) ||
-	    tg3_flag(tp, HW_TSO_2) ||
-	    tg3_flag(tp, HW_TSO_3))
-		return 0;
-
-	fw_data = (void *)tp->fw->data;
-
-	/* Firmware blob starts with version numbers, followed by
-	   start address and length. We are setting complete length.
-	   length = end_address_of_bss - start_address_of_text.
-	   Remainder is the blob to be loaded contiguously
-	   from start address. */
-
-	info.fw_base = be32_to_cpu(fw_data[1]);
-	cpu_scratch_size = tp->fw_len;
-	info.fw_len = tp->fw->size - 12;
-	info.fw_data = &fw_data[3];
-
-	if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5705) {
-		cpu_base = RX_CPU_BASE;
-		cpu_scratch_base = NIC_SRAM_MBUF_POOL_BASE5705;
-	} else {
-		cpu_base = TX_CPU_BASE;
-		cpu_scratch_base = TX_CPU_SCRATCH_BASE;
-		cpu_scratch_size = TX_CPU_SCRATCH_SIZE;
-	}
-
-	err = tg3_load_firmware_cpu(tp, cpu_base,
-				    cpu_scratch_base, cpu_scratch_size,
-				    &info);
-	if (err)
-		return err;
-
-	/* Now startup the cpu. */
-	tw32(cpu_base + CPU_STATE, 0xffffffff);
-	tw32_f(cpu_base + CPU_PC, info.fw_base);
-
-	for (i = 0; i < 5; i++) {
-		if (tr32(cpu_base + CPU_PC) == info.fw_base)
-			break;
-		tw32(cpu_base + CPU_STATE, 0xffffffff);
-		tw32(cpu_base + CPU_MODE,  CPU_MODE_HALT);
-		tw32_f(cpu_base + CPU_PC, info.fw_base);
-		udelay(1000);
-	}
-	if (i >= 5) {
-		netdev_err(tp->dev,
-			   "%s fails to set CPU PC, is %08x should be %08x\n",
-			   __func__, tr32(cpu_base + CPU_PC), info.fw_base);
-		return -ENODEV;
-	}
-	tw32(cpu_base + CPU_STATE, 0xffffffff);
-	tw32_f(cpu_base + CPU_MODE,  0x00000000);
-	return 0;
-}
-
-
 static int tg3_set_mac_addr(struct net_device *dev, void *p)
 {
 	struct tg3 *tp = netdev_priv(dev);
-- 
1.7.3.4

^ permalink raw reply related

* [PATCH net-next 8/8] tg3: Code movement
From: Matt Carlson @ 2011-08-31 21:44 UTC (permalink / raw)
  To: davem; +Cc: netdev, mcarlson

This patch just moves some code around for better organization.

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Benjamin Li <benli@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 88426b2..8caadc2 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -3819,6 +3819,7 @@ static int tg3_copper_is_advertising_all(struct tg3 *tp, u32 mask)
 		if (tg3_ctrl != all_mask)
 			return 0;
 	}
+
 	return 1;
 }
 
@@ -4119,8 +4120,8 @@ relink:
 			newlnkctl = oldlnkctl | PCI_EXP_LNKCTL_CLKREQ_EN;
 		if (newlnkctl != oldlnkctl)
 			pci_write_config_word(tp->pdev,
-					      pci_pcie_cap(tp->pdev) + PCI_EXP_LNKCTL,
-					      newlnkctl);
+					      pci_pcie_cap(tp->pdev) +
+					      PCI_EXP_LNKCTL, newlnkctl);
 	}
 
 	if (current_link_up != netif_carrier_ok(tp->dev)) {
@@ -6733,6 +6734,10 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		}
 	}
 
+	if (tg3_flag(tp, USE_JUMBO_BDFLAG) &&
+	    !mss && skb->len > VLAN_ETH_FRAME_LEN)
+		base_flags |= TXD_FLAG_JMB_PKT;
+
 #ifdef BCM_KERNEL_SUPPORTS_8021Q
 	if (vlan_tx_tag_present(skb)) {
 		base_flags |= TXD_FLAG_VLAN;
@@ -6740,10 +6745,6 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 #endif
 
-	if (tg3_flag(tp, USE_JUMBO_BDFLAG) &&
-	    !mss && skb->len > VLAN_ETH_FRAME_LEN)
-		base_flags |= TXD_FLAG_JMB_PKT;
-
 	len = skb_headlen(skb);
 
 	mapping = pci_map_single(tp->pdev, skb->data, len, PCI_DMA_TODEVICE);
@@ -14081,7 +14082,7 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
 		/* BCM5785 devices are effectively PCIe devices, and should
 		 * follow PCIe codepaths, but do not have a PCIe capabilities
 		 * section.
-		*/
+		 */
 		tg3_flag_set(tp, PCI_EXPRESS);
 	} else if (!tg3_flag(tp, 5705_PLUS) ||
 		   tg3_flag(tp, 5780_CLASS)) {
@@ -15541,7 +15542,7 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
 		tnapi->tx_pending = TG3_DEF_TX_RING_PENDING;
 
 		tnapi->int_mbox = intmbx;
-		if (i < 4)
+		if (i <= 4)
 			intmbx += 0x8;
 		else
 			intmbx += 0x4;
-- 
1.7.3.4

^ permalink raw reply related

* [PATCH net-next 6/8] tg3: Eliminate tg3_write_sig_post_reset() prototype
From: Matt Carlson @ 2011-08-31 21:44 UTC (permalink / raw)
  To: davem; +Cc: netdev, mcarlson

This patch moves the implementation of tg3_write_sig_post_reset()
earlier to eliminate its prototype.

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Benjamin Li <benli@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c |  458 +++++++++++++++++------------------
 1 files changed, 228 insertions(+), 230 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 333f370..272aa11 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -94,6 +94,10 @@ static inline void _tg3_flag_clear(enum TG3_FLAGS flag, unsigned long *bits)
 	__stringify(TG3_MAJ_NUM) "." __stringify(TG3_MIN_NUM)
 #define DRV_MODULE_RELDATE	"August 18, 2011"
 
+#define RESET_KIND_SHUTDOWN	0
+#define RESET_KIND_INIT		1
+#define RESET_KIND_SUSPEND	2
+
 #define TG3_DEF_RX_MODE		0
 #define TG3_DEF_TX_MODE		0
 #define TG3_DEF_MSG_ENABLE	  \
@@ -724,6 +728,103 @@ static void tg3_ape_unlock(struct tg3 *tp, int locknum)
 	tg3_ape_write32(tp, gnt + 4 * locknum, bit);
 }
 
+static void tg3_ape_send_event(struct tg3 *tp, u32 event)
+{
+	int i;
+	u32 apedata;
+
+	/* NCSI does not support APE events */
+	if (tg3_flag(tp, APE_HAS_NCSI))
+		return;
+
+	apedata = tg3_ape_read32(tp, TG3_APE_SEG_SIG);
+	if (apedata != APE_SEG_SIG_MAGIC)
+		return;
+
+	apedata = tg3_ape_read32(tp, TG3_APE_FW_STATUS);
+	if (!(apedata & APE_FW_STATUS_READY))
+		return;
+
+	/* Wait for up to 1 millisecond for APE to service previous event. */
+	for (i = 0; i < 10; i++) {
+		if (tg3_ape_lock(tp, TG3_APE_LOCK_MEM))
+			return;
+
+		apedata = tg3_ape_read32(tp, TG3_APE_EVENT_STATUS);
+
+		if (!(apedata & APE_EVENT_STATUS_EVENT_PENDING))
+			tg3_ape_write32(tp, TG3_APE_EVENT_STATUS,
+					event | APE_EVENT_STATUS_EVENT_PENDING);
+
+		tg3_ape_unlock(tp, TG3_APE_LOCK_MEM);
+
+		if (!(apedata & APE_EVENT_STATUS_EVENT_PENDING))
+			break;
+
+		udelay(100);
+	}
+
+	if (!(apedata & APE_EVENT_STATUS_EVENT_PENDING))
+		tg3_ape_write32(tp, TG3_APE_EVENT, APE_EVENT_1);
+}
+
+static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
+{
+	u32 event;
+	u32 apedata;
+
+	if (!tg3_flag(tp, ENABLE_APE))
+		return;
+
+	switch (kind) {
+	case RESET_KIND_INIT:
+		tg3_ape_write32(tp, TG3_APE_HOST_SEG_SIG,
+				APE_HOST_SEG_SIG_MAGIC);
+		tg3_ape_write32(tp, TG3_APE_HOST_SEG_LEN,
+				APE_HOST_SEG_LEN_MAGIC);
+		apedata = tg3_ape_read32(tp, TG3_APE_HOST_INIT_COUNT);
+		tg3_ape_write32(tp, TG3_APE_HOST_INIT_COUNT, ++apedata);
+		tg3_ape_write32(tp, TG3_APE_HOST_DRIVER_ID,
+			APE_HOST_DRIVER_ID_MAGIC(TG3_MAJ_NUM, TG3_MIN_NUM));
+		tg3_ape_write32(tp, TG3_APE_HOST_BEHAVIOR,
+				APE_HOST_BEHAV_NO_PHYLOCK);
+		tg3_ape_write32(tp, TG3_APE_HOST_DRVR_STATE,
+				    TG3_APE_HOST_DRVR_STATE_START);
+
+		event = APE_EVENT_STATUS_STATE_START;
+		break;
+	case RESET_KIND_SHUTDOWN:
+		/* With the interface we are currently using,
+		 * APE does not track driver state.  Wiping
+		 * out the HOST SEGMENT SIGNATURE forces
+		 * the APE to assume OS absent status.
+		 */
+		tg3_ape_write32(tp, TG3_APE_HOST_SEG_SIG, 0x0);
+
+		if (device_may_wakeup(&tp->pdev->dev) &&
+		    tg3_flag(tp, WOL_ENABLE)) {
+			tg3_ape_write32(tp, TG3_APE_HOST_WOL_SPEED,
+					    TG3_APE_HOST_WOL_SPEED_AUTO);
+			apedata = TG3_APE_HOST_DRVR_STATE_WOL;
+		} else
+			apedata = TG3_APE_HOST_DRVR_STATE_UNLOAD;
+
+		tg3_ape_write32(tp, TG3_APE_HOST_DRVR_STATE, apedata);
+
+		event = APE_EVENT_STATUS_STATE_UNLOAD;
+		break;
+	case RESET_KIND_SUSPEND:
+		event = APE_EVENT_STATUS_STATE_SUSPEND;
+		break;
+	default:
+		return;
+	}
+
+	event |= APE_EVENT_STATUS_DRIVER_EVNT | APE_EVENT_STATUS_STATE_CHNGE;
+
+	tg3_ape_send_event(tp, event);
+}
+
 static void tg3_disable_ints(struct tg3 *tp)
 {
 	int i;
@@ -1412,6 +1513,133 @@ static void tg3_stop_fw(struct tg3 *tp)
 	}
 }
 
+/* tp->lock is held. */
+static void tg3_write_sig_pre_reset(struct tg3 *tp, int kind)
+{
+	tg3_write_mem(tp, NIC_SRAM_FIRMWARE_MBOX,
+		      NIC_SRAM_FIRMWARE_MBOX_MAGIC1);
+
+	if (tg3_flag(tp, ASF_NEW_HANDSHAKE)) {
+		switch (kind) {
+		case RESET_KIND_INIT:
+			tg3_write_mem(tp, NIC_SRAM_FW_DRV_STATE_MBOX,
+				      DRV_STATE_START);
+			break;
+
+		case RESET_KIND_SHUTDOWN:
+			tg3_write_mem(tp, NIC_SRAM_FW_DRV_STATE_MBOX,
+				      DRV_STATE_UNLOAD);
+			break;
+
+		case RESET_KIND_SUSPEND:
+			tg3_write_mem(tp, NIC_SRAM_FW_DRV_STATE_MBOX,
+				      DRV_STATE_SUSPEND);
+			break;
+
+		default:
+			break;
+		}
+	}
+
+	if (kind == RESET_KIND_INIT ||
+	    kind == RESET_KIND_SUSPEND)
+		tg3_ape_driver_state_change(tp, kind);
+}
+
+/* tp->lock is held. */
+static void tg3_write_sig_post_reset(struct tg3 *tp, int kind)
+{
+	if (tg3_flag(tp, ASF_NEW_HANDSHAKE)) {
+		switch (kind) {
+		case RESET_KIND_INIT:
+			tg3_write_mem(tp, NIC_SRAM_FW_DRV_STATE_MBOX,
+				      DRV_STATE_START_DONE);
+			break;
+
+		case RESET_KIND_SHUTDOWN:
+			tg3_write_mem(tp, NIC_SRAM_FW_DRV_STATE_MBOX,
+				      DRV_STATE_UNLOAD_DONE);
+			break;
+
+		default:
+			break;
+		}
+	}
+
+	if (kind == RESET_KIND_SHUTDOWN)
+		tg3_ape_driver_state_change(tp, kind);
+}
+
+/* tp->lock is held. */
+static void tg3_write_sig_legacy(struct tg3 *tp, int kind)
+{
+	if (tg3_flag(tp, ENABLE_ASF)) {
+		switch (kind) {
+		case RESET_KIND_INIT:
+			tg3_write_mem(tp, NIC_SRAM_FW_DRV_STATE_MBOX,
+				      DRV_STATE_START);
+			break;
+
+		case RESET_KIND_SHUTDOWN:
+			tg3_write_mem(tp, NIC_SRAM_FW_DRV_STATE_MBOX,
+				      DRV_STATE_UNLOAD);
+			break;
+
+		case RESET_KIND_SUSPEND:
+			tg3_write_mem(tp, NIC_SRAM_FW_DRV_STATE_MBOX,
+				      DRV_STATE_SUSPEND);
+			break;
+
+		default:
+			break;
+		}
+	}
+}
+
+static int tg3_poll_fw(struct tg3 *tp)
+{
+	int i;
+	u32 val;
+
+	if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5906) {
+		/* Wait up to 20ms for init done. */
+		for (i = 0; i < 200; i++) {
+			if (tr32(VCPU_STATUS) & VCPU_STATUS_INIT_DONE)
+				return 0;
+			udelay(100);
+		}
+		return -ENODEV;
+	}
+
+	/* Wait for firmware initialization to complete. */
+	for (i = 0; i < 100000; i++) {
+		tg3_read_mem(tp, NIC_SRAM_FIRMWARE_MBOX, &val);
+		if (val == ~NIC_SRAM_FIRMWARE_MBOX_MAGIC1)
+			break;
+		udelay(10);
+	}
+
+	/* Chip might not be fitted with firmware.  Some Sun onboard
+	 * parts are configured like that.  So don't signal the timeout
+	 * of the above loop as an error, but do report the lack of
+	 * running firmware once.
+	 */
+	if (i >= 100000 && !tg3_flag(tp, NO_FWARE_REPORTED)) {
+		tg3_flag_set(tp, NO_FWARE_REPORTED);
+
+		netdev_info(tp->dev, "No firmware running\n");
+	}
+
+	if (tp->pci_chip_rev_id == CHIPREV_ID_57765_A0) {
+		/* The 57765 A0 needs a little more
+		 * time to do some important work.
+		 */
+		mdelay(10);
+	}
+
+	return 0;
+}
+
 static void tg3_link_report(struct tg3 *tp)
 {
 	if (!netif_carrier_ok(tp->dev)) {
@@ -2503,12 +2731,6 @@ static int tg3_5700_link_polarity(struct tg3 *tp, u32 speed)
 }
 
 static int tg3_setup_phy(struct tg3 *, int);
-
-#define RESET_KIND_SHUTDOWN	0
-#define RESET_KIND_INIT		1
-#define RESET_KIND_SUSPEND	2
-
-static void tg3_write_sig_post_reset(struct tg3 *, int);
 static int tg3_halt_cpu(struct tg3 *, u32);
 
 static void tg3_power_down_phy(struct tg3 *tp, bool do_low_power)
@@ -7147,230 +7369,6 @@ static int tg3_abort_hw(struct tg3 *tp, int silent)
 	return err;
 }
 
-static void tg3_ape_send_event(struct tg3 *tp, u32 event)
-{
-	int i;
-	u32 apedata;
-
-	/* NCSI does not support APE events */
-	if (tg3_flag(tp, APE_HAS_NCSI))
-		return;
-
-	apedata = tg3_ape_read32(tp, TG3_APE_SEG_SIG);
-	if (apedata != APE_SEG_SIG_MAGIC)
-		return;
-
-	apedata = tg3_ape_read32(tp, TG3_APE_FW_STATUS);
-	if (!(apedata & APE_FW_STATUS_READY))
-		return;
-
-	/* Wait for up to 1 millisecond for APE to service previous event. */
-	for (i = 0; i < 10; i++) {
-		if (tg3_ape_lock(tp, TG3_APE_LOCK_MEM))
-			return;
-
-		apedata = tg3_ape_read32(tp, TG3_APE_EVENT_STATUS);
-
-		if (!(apedata & APE_EVENT_STATUS_EVENT_PENDING))
-			tg3_ape_write32(tp, TG3_APE_EVENT_STATUS,
-					event | APE_EVENT_STATUS_EVENT_PENDING);
-
-		tg3_ape_unlock(tp, TG3_APE_LOCK_MEM);
-
-		if (!(apedata & APE_EVENT_STATUS_EVENT_PENDING))
-			break;
-
-		udelay(100);
-	}
-
-	if (!(apedata & APE_EVENT_STATUS_EVENT_PENDING))
-		tg3_ape_write32(tp, TG3_APE_EVENT, APE_EVENT_1);
-}
-
-static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
-{
-	u32 event;
-	u32 apedata;
-
-	if (!tg3_flag(tp, ENABLE_APE))
-		return;
-
-	switch (kind) {
-	case RESET_KIND_INIT:
-		tg3_ape_write32(tp, TG3_APE_HOST_SEG_SIG,
-				APE_HOST_SEG_SIG_MAGIC);
-		tg3_ape_write32(tp, TG3_APE_HOST_SEG_LEN,
-				APE_HOST_SEG_LEN_MAGIC);
-		apedata = tg3_ape_read32(tp, TG3_APE_HOST_INIT_COUNT);
-		tg3_ape_write32(tp, TG3_APE_HOST_INIT_COUNT, ++apedata);
-		tg3_ape_write32(tp, TG3_APE_HOST_DRIVER_ID,
-			APE_HOST_DRIVER_ID_MAGIC(TG3_MAJ_NUM, TG3_MIN_NUM));
-		tg3_ape_write32(tp, TG3_APE_HOST_BEHAVIOR,
-				APE_HOST_BEHAV_NO_PHYLOCK);
-		tg3_ape_write32(tp, TG3_APE_HOST_DRVR_STATE,
-				    TG3_APE_HOST_DRVR_STATE_START);
-
-		event = APE_EVENT_STATUS_STATE_START;
-		break;
-	case RESET_KIND_SHUTDOWN:
-		/* With the interface we are currently using,
-		 * APE does not track driver state.  Wiping
-		 * out the HOST SEGMENT SIGNATURE forces
-		 * the APE to assume OS absent status.
-		 */
-		tg3_ape_write32(tp, TG3_APE_HOST_SEG_SIG, 0x0);
-
-		if (device_may_wakeup(&tp->pdev->dev) &&
-		    tg3_flag(tp, WOL_ENABLE)) {
-			tg3_ape_write32(tp, TG3_APE_HOST_WOL_SPEED,
-					    TG3_APE_HOST_WOL_SPEED_AUTO);
-			apedata = TG3_APE_HOST_DRVR_STATE_WOL;
-		} else
-			apedata = TG3_APE_HOST_DRVR_STATE_UNLOAD;
-
-		tg3_ape_write32(tp, TG3_APE_HOST_DRVR_STATE, apedata);
-
-		event = APE_EVENT_STATUS_STATE_UNLOAD;
-		break;
-	case RESET_KIND_SUSPEND:
-		event = APE_EVENT_STATUS_STATE_SUSPEND;
-		break;
-	default:
-		return;
-	}
-
-	event |= APE_EVENT_STATUS_DRIVER_EVNT | APE_EVENT_STATUS_STATE_CHNGE;
-
-	tg3_ape_send_event(tp, event);
-}
-
-/* tp->lock is held. */
-static void tg3_write_sig_pre_reset(struct tg3 *tp, int kind)
-{
-	tg3_write_mem(tp, NIC_SRAM_FIRMWARE_MBOX,
-		      NIC_SRAM_FIRMWARE_MBOX_MAGIC1);
-
-	if (tg3_flag(tp, ASF_NEW_HANDSHAKE)) {
-		switch (kind) {
-		case RESET_KIND_INIT:
-			tg3_write_mem(tp, NIC_SRAM_FW_DRV_STATE_MBOX,
-				      DRV_STATE_START);
-			break;
-
-		case RESET_KIND_SHUTDOWN:
-			tg3_write_mem(tp, NIC_SRAM_FW_DRV_STATE_MBOX,
-				      DRV_STATE_UNLOAD);
-			break;
-
-		case RESET_KIND_SUSPEND:
-			tg3_write_mem(tp, NIC_SRAM_FW_DRV_STATE_MBOX,
-				      DRV_STATE_SUSPEND);
-			break;
-
-		default:
-			break;
-		}
-	}
-
-	if (kind == RESET_KIND_INIT ||
-	    kind == RESET_KIND_SUSPEND)
-		tg3_ape_driver_state_change(tp, kind);
-}
-
-/* tp->lock is held. */
-static void tg3_write_sig_post_reset(struct tg3 *tp, int kind)
-{
-	if (tg3_flag(tp, ASF_NEW_HANDSHAKE)) {
-		switch (kind) {
-		case RESET_KIND_INIT:
-			tg3_write_mem(tp, NIC_SRAM_FW_DRV_STATE_MBOX,
-				      DRV_STATE_START_DONE);
-			break;
-
-		case RESET_KIND_SHUTDOWN:
-			tg3_write_mem(tp, NIC_SRAM_FW_DRV_STATE_MBOX,
-				      DRV_STATE_UNLOAD_DONE);
-			break;
-
-		default:
-			break;
-		}
-	}
-
-	if (kind == RESET_KIND_SHUTDOWN)
-		tg3_ape_driver_state_change(tp, kind);
-}
-
-/* tp->lock is held. */
-static void tg3_write_sig_legacy(struct tg3 *tp, int kind)
-{
-	if (tg3_flag(tp, ENABLE_ASF)) {
-		switch (kind) {
-		case RESET_KIND_INIT:
-			tg3_write_mem(tp, NIC_SRAM_FW_DRV_STATE_MBOX,
-				      DRV_STATE_START);
-			break;
-
-		case RESET_KIND_SHUTDOWN:
-			tg3_write_mem(tp, NIC_SRAM_FW_DRV_STATE_MBOX,
-				      DRV_STATE_UNLOAD);
-			break;
-
-		case RESET_KIND_SUSPEND:
-			tg3_write_mem(tp, NIC_SRAM_FW_DRV_STATE_MBOX,
-				      DRV_STATE_SUSPEND);
-			break;
-
-		default:
-			break;
-		}
-	}
-}
-
-static int tg3_poll_fw(struct tg3 *tp)
-{
-	int i;
-	u32 val;
-
-	if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5906) {
-		/* Wait up to 20ms for init done. */
-		for (i = 0; i < 200; i++) {
-			if (tr32(VCPU_STATUS) & VCPU_STATUS_INIT_DONE)
-				return 0;
-			udelay(100);
-		}
-		return -ENODEV;
-	}
-
-	/* Wait for firmware initialization to complete. */
-	for (i = 0; i < 100000; i++) {
-		tg3_read_mem(tp, NIC_SRAM_FIRMWARE_MBOX, &val);
-		if (val == ~NIC_SRAM_FIRMWARE_MBOX_MAGIC1)
-			break;
-		udelay(10);
-	}
-
-	/* Chip might not be fitted with firmware.  Some Sun onboard
-	 * parts are configured like that.  So don't signal the timeout
-	 * of the above loop as an error, but do report the lack of
-	 * running firmware once.
-	 */
-	if (i >= 100000 && !tg3_flag(tp, NO_FWARE_REPORTED)) {
-		tg3_flag_set(tp, NO_FWARE_REPORTED);
-
-		netdev_info(tp->dev, "No firmware running\n");
-	}
-
-	if (tp->pci_chip_rev_id == CHIPREV_ID_57765_A0) {
-		/* The 57765 A0 needs a little more
-		 * time to do some important work.
-		 */
-		mdelay(10);
-	}
-
-	return 0;
-}
-
 /* Save PCI command register before chip reset */
 static void tg3_save_pci_state(struct tg3 *tp)
 {
-- 
1.7.3.4

^ permalink raw reply related

* [PATCH net-next 3/8] tg3: Remove tp->rx_offset term when unneeded
From: Matt Carlson @ 2011-08-31 21:44 UTC (permalink / raw)
  To: davem; +Cc: netdev, mcarlson

This patch removes the tp->rx_offset term if NET_IP_ALIGN is defined to
zero.

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Benjamin Li <benli@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index ab4c598..e2acf9a 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -187,6 +187,12 @@ static inline void _tg3_flag_clear(enum TG3_FLAGS flag, unsigned long *bits)
 	#define TG3_RX_COPY_THRESH(tp)	((tp)->rx_copy_thresh)
 #endif
 
+#if (NET_IP_ALIGN != 0)
+#define TG3_RX_OFFSET(tp)	((tp)->rx_offset)
+#else
+#define TG3_RX_OFFSET(tp)	0
+#endif
+
 /* minimum number of free TX descriptors required to wake up TX process */
 #define TG3_TX_WAKEUP_THRESH(tnapi)		((tnapi)->tx_pending / 4)
 #define TG3_TX_BD_DMA_MAX		4096
@@ -4984,11 +4990,11 @@ static int tg3_alloc_rx_skb(struct tg3 *tp, struct tg3_rx_prodring_set *tpr,
 	 * Callers depend upon this behavior and assume that
 	 * we leave everything unchanged if we fail.
 	 */
-	skb = netdev_alloc_skb(tp->dev, skb_size + tp->rx_offset);
+	skb = netdev_alloc_skb(tp->dev, skb_size + TG3_RX_OFFSET(tp));
 	if (skb == NULL)
 		return -ENOMEM;
 
-	skb_reserve(skb, tp->rx_offset);
+	skb_reserve(skb, TG3_RX_OFFSET(tp));
 
 	mapping = pci_map_single(tp->pdev, skb->data, skb_size,
 				 PCI_DMA_FROMDEVICE);
-- 
1.7.3.4

^ permalink raw reply related

* [PATCH net-next 5/8] tg3: Eliminate tg3_stop_fw() prototype
From: Matt Carlson @ 2011-08-31 21:44 UTC (permalink / raw)
  To: davem; +Cc: netdev, mcarlson

This patch moves tg3_stop_fw() earlier in the file to eliminate its
prototype.

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Benjamin Li <benli@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c |   34 ++++++++++++++++------------------
 1 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index dab2aa7..333f370 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -1396,6 +1396,22 @@ static void tg3_ump_link_report(struct tg3 *tp)
 	tg3_generate_fw_event(tp);
 }
 
+/* tp->lock is held. */
+static void tg3_stop_fw(struct tg3 *tp)
+{
+	if (tg3_flag(tp, ENABLE_ASF) && !tg3_flag(tp, ENABLE_APE)) {
+		/* Wait for RX cpu to ACK the previous event. */
+		tg3_wait_for_event_ack(tp);
+
+		tg3_write_mem(tp, NIC_SRAM_FW_CMD_MBOX, FWCMD_NICDRV_PAUSE_FW);
+
+		tg3_generate_fw_event(tp);
+
+		/* Wait for RX cpu to ACK this event. */
+		tg3_wait_for_event_ack(tp);
+	}
+}
+
 static void tg3_link_report(struct tg3 *tp)
 {
 	if (!netif_carrier_ok(tp->dev)) {
@@ -7426,8 +7442,6 @@ static void tg3_restore_pci_state(struct tg3 *tp)
 	}
 }
 
-static void tg3_stop_fw(struct tg3 *);
-
 /* tp->lock is held. */
 static int tg3_chip_reset(struct tg3 *tp)
 {
@@ -7675,22 +7689,6 @@ static int tg3_chip_reset(struct tg3 *tp)
 }
 
 /* tp->lock is held. */
-static void tg3_stop_fw(struct tg3 *tp)
-{
-	if (tg3_flag(tp, ENABLE_ASF) && !tg3_flag(tp, ENABLE_APE)) {
-		/* Wait for RX cpu to ACK the previous event. */
-		tg3_wait_for_event_ack(tp);
-
-		tg3_write_mem(tp, NIC_SRAM_FW_CMD_MBOX, FWCMD_NICDRV_PAUSE_FW);
-
-		tg3_generate_fw_event(tp);
-
-		/* Wait for RX cpu to ACK this event. */
-		tg3_wait_for_event_ack(tp);
-	}
-}
-
-/* tp->lock is held. */
 static int tg3_halt(struct tg3 *tp, int kind, int silent)
 {
 	int err;
-- 
1.7.3.4

^ permalink raw reply related

* [PATCH net-next 4/8] tg3: Add ability to turn off 1shot MSI
From: Matt Carlson @ 2011-08-31 21:44 UTC (permalink / raw)
  To: davem; +Cc: netdev, mcarlson

In older devices, 1-shot MSI mode had to be enabled by the code.  In
newer devices however, 1-shot MSI mode is enabled by default; code would
be needed to disable it.

Disabling 1-shot MSI mode is useful when debugging.  This patch changes
the code so that the TG3_FLAG_1SHOT_MSI accurately reflects (and
controls) the state of 1-shot MSI mode.

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Benjamin Li <benli@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index e2acf9a..dab2aa7 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -5712,7 +5712,7 @@ static irqreturn_t tg3_msi(int irq, void *dev_id)
 	 * NIC to stop sending us irqs, engaging "in-intr-handler"
 	 * event coalescing.
 	 */
-	tw32_mailbox(MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW, 0x00000001);
+	tw32_mailbox(tnapi->int_mbox, 0x00000001);
 	if (likely(!tg3_irq_sync(tp)))
 		napi_schedule(&tnapi->napi);
 
@@ -8809,6 +8809,8 @@ static int tg3_reset_hw(struct tg3 *tp, int reset_phy)
 	if (tg3_flag(tp, USING_MSIX) && tp->irq_cnt > 1) {
 		val = tr32(MSGINT_MODE);
 		val |= MSGINT_MODE_MULTIVEC_EN | MSGINT_MODE_ENABLE;
+		if (!tg3_flag(tp, 1SHOT_MSI))
+			val |= MSGINT_MODE_ONE_SHOT_DISABLE;
 		tw32(MSGINT_MODE, val);
 	}
 
@@ -9424,7 +9426,7 @@ static int tg3_test_interrupt(struct tg3 *tp)
 
 	if (intr_ok) {
 		/* Reenable MSI one shot mode. */
-		if (tg3_flag(tp, 57765_PLUS)) {
+		if (tg3_flag(tp, 57765_PLUS) && tg3_flag(tp, 1SHOT_MSI)) {
 			val = tr32(MSGINT_MODE) & ~MSGINT_MODE_ONE_SHOT_DISABLE;
 			tw32(MSGINT_MODE, val);
 		}
@@ -9602,6 +9604,8 @@ static void tg3_ints_init(struct tg3 *tp)
 		u32 msi_mode = tr32(MSGINT_MODE);
 		if (tg3_flag(tp, USING_MSIX) && tp->irq_cnt > 1)
 			msi_mode |= MSGINT_MODE_MULTIVEC_EN;
+		if (!tg3_flag(tp, 1SHOT_MSI))
+			msi_mode |= MSGINT_MODE_ONE_SHOT_DISABLE;
 		tw32(MSGINT_MODE, msi_mode | MSGINT_MODE_ENABLE);
 	}
 defcfg:
-- 
1.7.3.4

^ permalink raw reply related

* Re: [PATCH] iproute2: fix implicit declaration of function '__ALIGN_KERNEL'
From: Stephen Hemminger @ 2011-08-31 17:59 UTC (permalink / raw)
  To: Gilles Espinasse; +Cc: netdev
In-Reply-To: <1310312780-2323-1-git-send-email-g.esp@free.fr>

On Sun, 10 Jul 2011 17:46:20 +0200
Gilles Espinasse <g.esp@free.fr> wrote:

> Warning seen with 2.6.32 kernel headers
>  m_xt.c:85: warning: implicit declaration of function '__ALIGN_KERNEL'
> 
> Declaration lines borrowed from iptables-1.4.11.1
> 
> Signed-off-by: Gilles Espinasse <g.esp@free.fr>
> ---
>  include/linux/netfilter/x_tables.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
> index 4120970..a6614b0 100644
> --- a/include/linux/netfilter/x_tables.h
> +++ b/include/linux/netfilter/x_tables.h
> @@ -96,6 +96,8 @@ struct _xt_align {
>  	__u64 u64;
>  };
>  
> +#define __ALIGN_KERNEL(x, a)	__ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1)
> +#define __ALIGN_KERNEL_MASK(x, mask)	(((x) + (mask)) & ~(mask))
>  #define XT_ALIGN(s) __ALIGN_KERNEL((s), __alignof__(struct _xt_align))
>  
>  /* Standard return verdict, or do jump. */

This was a problem already fixed upstream by the iptables folks in later
version.  The files in include/linux are automatically generated by
a the kernel header script, and I don't want to start a bad precedent
by making a special case for this.

^ permalink raw reply

* [PATCH net-2.6] net: copy userspace buffers on device forwarding
From: Michael S. Tsirkin @ 2011-08-31 18:03 UTC (permalink / raw)
  To: Herbert Xu
  Cc: David S. Miller, Eric Dumazet, Michał Mirosław,
	Tom Herbert, Jiri Pirko, Jesse Gross, Shirley Ma, linux-kernel,
	netdev

dev_forward_skb loops an skb back into host networking
stack which might hang on the memory indefinitely.
In particular, this can happen in macvtap in bridged mode.
Copy the userspace fragments to avoid blocking the
sender in that case.

As this patch makes skb_copy_ubufs extern now,
I also added some documentation and made it clear
the SKBTX_DEV_ZEROCOPY flag automatically instead
of doing it in all callers. This can be made into a separate
patch if people feel it's worth it.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Shirley, any thoughts on this?
Another alternative is to disable zero copy for
bridged mode devices.

 include/linux/skbuff.h |    1 +
 net/core/dev.c         |    8 ++++++++
 net/core/skbuff.c      |   22 +++++++++++++++++-----
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 7b996ed..8bd383c 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -524,6 +524,7 @@ static inline struct sk_buff *alloc_skb_fclone(unsigned int size,
 extern bool skb_recycle_check(struct sk_buff *skb, int skb_size);
 
 extern struct sk_buff *skb_morph(struct sk_buff *dst, struct sk_buff *src);
+extern int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask);
 extern struct sk_buff *skb_clone(struct sk_buff *skb,
 				 gfp_t priority);
 extern struct sk_buff *skb_copy(const struct sk_buff *skb,
diff --git a/net/core/dev.c b/net/core/dev.c
index 17d67b5..b10ff0a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1515,6 +1515,14 @@ static inline bool is_skb_forwardable(struct net_device *dev,
  */
 int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
 {
+	if (skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) {
+		if (skb_copy_ubufs(skb, GFP_ATOMIC)) {
+			atomic_long_inc(&dev->rx_dropped);
+			kfree_skb(skb);
+			return NET_RX_DROP;
+		}
+	}
+
 	skb_orphan(skb);
 	nf_reset(skb);
 
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 27002df..387703f 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -611,8 +611,21 @@ struct sk_buff *skb_morph(struct sk_buff *dst, struct sk_buff *src)
 }
 EXPORT_SYMBOL_GPL(skb_morph);
 
-/* skb frags copy userspace buffers to kernel */
-static int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask)
+/*	skb_copy_ubufs	-	copy userspace skb frags buffers to kernel
+ *	@skb: the skb to modify
+ *	@gfp_mask: allocation priority
+ *
+ *	This must be called on SKBTX_DEV_ZEROCOPY skb.
+ *	It will copy all frags into kernel and drop the reference
+ *	to userspace pages.
+ *
+ *	If this function is called from an interrupt gfp_mask() must be
+ *	%GFP_ATOMIC.
+ *
+ *	Returns 0 on success or a negative error code on failure
+ *	to allocate kernel memory to copy to.
+ */
+int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask)
 {
 	int i;
 	int num_frags = skb_shinfo(skb)->nr_frags;
@@ -652,6 +665,8 @@ static int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask)
 		skb_shinfo(skb)->frags[i - 1].page = head;
 		head = (struct page *)head->private;
 	}
+
+	skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
 	return 0;
 }
 
@@ -677,7 +692,6 @@ struct sk_buff *skb_clone(struct sk_buff *skb, gfp_t gfp_mask)
 	if (skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) {
 		if (skb_copy_ubufs(skb, gfp_mask))
 			return NULL;
-		skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
 	}
 
 	n = skb + 1;
@@ -803,7 +817,6 @@ struct sk_buff *pskb_copy(struct sk_buff *skb, gfp_t gfp_mask)
 				n = NULL;
 				goto out;
 			}
-			skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
 		}
 		for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
 			skb_shinfo(n)->frags[i] = skb_shinfo(skb)->frags[i];
@@ -896,7 +909,6 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
 		if (skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) {
 			if (skb_copy_ubufs(skb, gfp_mask))
 				goto nofrags;
-			skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
 		}
 		for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
 			get_page(skb_shinfo(skb)->frags[i].page);
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* Re: [PATCH] iproute2: Fail "ip netns add" on existing network namespaces.
From: Stephen Hemminger @ 2011-08-31 18:05 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: netdev
In-Reply-To: <m1hb6nkqyx.fsf@fess.ebiederm.org>

On Fri, 15 Jul 2011 17:29:41 -0700
ebiederm@xmission.com (Eric W. Biederman) wrote:

> 
> Use O_EXCL so that we only create and mount a new network namespace
> if there is no chance an existing network namespace is present.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>

Both setns patches applied

^ permalink raw reply

* Re: [PATCH 1/3] bnx2x: remove TPA_ENABLE_FLAG
From: Michał Mirosław @ 2011-08-31 18:05 UTC (permalink / raw)
  To: Vladislav Zolotarov
  Cc: Michal Schmidt, netdev@vger.kernel.org, Dmitry Kravkov,
	Eilon Greenstein
In-Reply-To: <8628FE4E7912BF47A96AE7DD7BAC0AAD01067097ADE0@SJEXCHCCR02.corp.ad.broadcom.com>

2011/8/31 Vladislav Zolotarov <vladz@broadcom.com>:
>> -----Original Message-----
>> From: Michal Schmidt [mailto:mschmidt@redhat.com]
>> Sent: Wednesday, August 31, 2011 6:59 PM
>> To: Vladislav Zolotarov
>> Cc: netdev@vger.kernel.org; Dmitry Kravkov; Eilon Greenstein;
>> mirqus@gmail.com
>> Subject: Re: [PATCH 1/3] bnx2x: remove TPA_ENABLE_FLAG
>>
>> On Wed, 31 Aug 2011 18:16:30 +0300 Vlad Zolotarov wrote:
>> > On Wednesday 31 August 2011 18:00:34 Michal Schmidt wrote:
>> > >   if (bnx2x_reload) {
>> > > -         if (bp->recovery_state == BNX2X_RECOVERY_DONE)
>> > > +         if (bp->recovery_state == BNX2X_RECOVERY_DONE) {
>> > > +                 /*
>> > > +                  * Cheat! Normally dev->features will be
>> > > set after we
>> > > +                  * return, but that's too late. We need to
>> > > know how to
>> > > +                  * configure the NIC when reloading it, so
>> > > update
>> > > +                  * the features early.
>> > > +                  */
>> > > +                 dev->features = features;
>> > >                   return bnx2x_reload_if_running(dev);
>> >
>> > NACK
>> >
>> > This is bogus - what if bnx2x_reload_if_running(dev)
>> > (bnx2x_nic_load()) failes? The original dev->features would be
>> > lost...
>>
>> Well, yes, but since the NIC would be now not working, do we really
>> care about its features? :-)
>
> U r kidding, right? ;)
> We care about the consistency in the netdev features state - if we failed
> to configure the requested feature and returned an error on e.g. "ethtool -K ethX lro on"
> call, it's expected that a subsequent ethtool -k ethX call won't report the requested
> feature (LRO) as set.

If bnx2x_reload_if_running() failure means that NIC is disabled, then
Michal is right that there's no point in caring about dev->features,
sice 'load' operation (NIC configuration) needs to be done again
anyway.

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [PATCH 7/7] bnx2x: expose HW RX VLAN stripping toggle
From: Michał Mirosław @ 2011-08-31 18:11 UTC (permalink / raw)
  To: Michal Schmidt
  Cc: Vlad Zolotarov, netdev@vger.kernel.org, Dmitry Kravkov,
	Eilon Greenstein
In-Reply-To: <20110831175120.739bd5c2@alice>

2011/8/31 Michal Schmidt <mschmidt@redhat.com>:
> On Wed, 31 Aug 2011 17:37:49 +0200 Michal Schmidt wrote:
>> I could restore dev->features before
>> returning if bnx2x_reload_if_running() fails.
>
> Or even safer - restore them always:
>        ...
>        u32 orig_features = dev->features;
>        dev->features = features;
>        ret = bnx2x_reload_if_running(dev);
>        dev->features = orig_features;
>        return ret;
>        ...
> This way we don't have to assume anything about
> __netdev_update_features().

The assignment in __netdev_update_features() is just to save copying
that assignment all over the drivers' code. This won't change unless
someone wants to break^Wchange the design.

Best Regards,
Michał Mirosław

^ 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