Netdev List
 help / color / mirror / Atom feed
* Re: [net-next PATCH v1 1/7] net: add generic PF_BRIDGE:RTM_ FDB hooks
From: Ben Hutchings @ 2012-04-11 16:05 UTC (permalink / raw)
  To: John Fastabend
  Cc: roprabhu, mst, stephen.hemminger, davem, hadi, jeffrey.t.kirsher,
	netdev, gregory.v.rose, krkumar2, sri
In-Reply-To: <4F85991A.5030808@intel.com>

On Wed, 2012-04-11 at 07:45 -0700, John Fastabend wrote:
> On 4/10/2012 8:23 PM, Ben Hutchings wrote:
> > On Mon, 2012-04-09 at 15:00 -0700, John Fastabend wrote:
> > [...]
> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> >> index 1f77540..05822e5 100644
> >> --- a/include/linux/netdevice.h
> >> +++ b/include/linux/netdevice.h
> > [...]
> >> @@ -905,6 +906,19 @@ struct netdev_fcoe_hbainfo {
> >>   *	feature set might be less than what was returned by ndo_fix_features()).
> >>   *	Must return >0 or -errno if it changed dev->features itself.
> >>   *
> >> + * int (*ndo_fdb_add)(struct ndmsg *ndm, struct net_device *dev,
> >> + *		      unsigned char *addr, u16 flags)
> >> + *	Adds an FDB entry to dev for addr. The ndmsg contains flags to indicate
> >> + *	if the dev->master FDB should be updated or the devices internal FDB.
> > 
> > I don't think the second sentence is helpful, as rtnl_fdb_add() will
> > take care of those flags.
> > 
> >> + * int (*ndo_fdb_del)(struct ndmsg *ndm, struct net_device *dev,
> >> + *		      unsigned char *addr)
> >> + *	Deletes the FDB entry from dev coresponding to addr. The ndmsg
> >> + *	contains flags to indicate if the dev->master FDB should be
> >> + *	updated or the devices internal FDB.
> > 
> > Similarly here.
> 
> agreed neither seem particularly helpful I'll remove them.
> 
> > 
> >> + * int (*ndo_fdb_dump)(struct sk_buff *skb, struct netlink_callback *cb,
> >> + *		       struct net_device *dev, int idx)
> >> + *	Used to add FDB entries to dump requests. Implementers should add
> >> + *	entries to skb and update idx with the number of entries.
> >>   */
> > [...
> >> --- a/net/core/rtnetlink.c
> >> +++ b/net/core/rtnetlink.c
> > [...]
> >> +static int rtnl_fdb_add(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
> >> +{
> > [...]
> >> +	err = -EOPNOTSUPP;
> > 
> > So if NTF_MASTER and NTF_SELF are both set, we can quietly fall back to
> > just setting one FDB?  Not sure that's really desirable though 
> > 
> 
> It makes it easier to keep an embedded agent and the sw bridge in
> sync if setting both flags adds the entry to both the SW bridge and
> embedded bridge.

Yes, I agree with that.

[...]
> > Wonder what we should do on error here if we've already successfully
> > called ndo_fdb_add on the master?  Should we try to roll back the first
> > addition?
> > 
> 
> The problem with rolling back is the table is likely already updated and
> traffic may already be being forwarded. So I think in this case the user
> will have to query the device to learn what failed. It seems like the
> simplest way to handle this. I think it is unwanted to have traffic being
> forwarded one way for a short period of time then rolled back.
> 
> The other idea I just had is we could clear the NTF_ bit in ndm_flags after
> the successful add, del command. I believe the nlmsg gets sent back to the
> user on error I would need to check on this.
[...]

That sounds like a good way of doing it, assuming there's no
compatibility issue.

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 v2] Lockd: pass network namespace to creation and destruction routines
From: J. Bruce Fields @ 2012-04-11 16:11 UTC (permalink / raw)
  To: Stanislav Kinsbursky
  Cc: Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA, xemul-bzQdu9zFT3WakBO8gow8eQ,
	neilb-l3A5Bk7waGM, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	jbottomley-bzQdu9zFT3WakBO8gow8eQ, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	devel-GEFAQzZX7r8dnm+yROfE0A
In-Reply-To: <20120329145421.31911.65512.stgit-bi+AKbBUZKagILUCTcTcHdKyNwTtLsGr@public.gmane.org>

On Thu, Mar 29, 2012 at 06:54:33PM +0400, Stanislav Kinsbursky wrote:
> v2: dereference of most probably already released nlm_host removed in
> nlmclnt_done() and reclaimer().

Did you want this in Trond's tree or mine?

--b.

> 
> These routines are called from locks reclaimer() kernel thread. This thread
> works in "init_net" network context and currently relays on persence on lockd
> thread and it's per-net resources. Thus lockd_up() and lockd_down() can't relay
> on current network context. So let's pass corrent one into them.
> 
> Signed-off-by: Stanislav Kinsbursky <skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> 
> Signed-off-by: Stanislav Kinsbursky <skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> 
> ---
>  fs/lockd/clntlock.c        |   13 ++++++++-----
>  fs/lockd/svc.c             |    7 +++----
>  fs/nfsd/nfssvc.c           |    6 +++---
>  include/linux/lockd/bind.h |    4 ++--
>  4 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/lockd/clntlock.c b/fs/lockd/clntlock.c
> index ba1dc2e..ca0a080 100644
> --- a/fs/lockd/clntlock.c
> +++ b/fs/lockd/clntlock.c
> @@ -56,7 +56,7 @@ struct nlm_host *nlmclnt_init(const struct nlmclnt_initdata *nlm_init)
>  	u32 nlm_version = (nlm_init->nfs_version == 2) ? 1 : 4;
>  	int status;
>  
> -	status = lockd_up();
> +	status = lockd_up(nlm_init->net);
>  	if (status < 0)
>  		return ERR_PTR(status);
>  
> @@ -65,7 +65,7 @@ struct nlm_host *nlmclnt_init(const struct nlmclnt_initdata *nlm_init)
>  				   nlm_init->hostname, nlm_init->noresvport,
>  				   nlm_init->net);
>  	if (host == NULL) {
> -		lockd_down();
> +		lockd_down(nlm_init->net);
>  		return ERR_PTR(-ENOLCK);
>  	}
>  
> @@ -80,8 +80,10 @@ EXPORT_SYMBOL_GPL(nlmclnt_init);
>   */
>  void nlmclnt_done(struct nlm_host *host)
>  {
> +	struct net *net = host->net;
> +
>  	nlmclnt_release_host(host);
> -	lockd_down();
> +	lockd_down(net);
>  }
>  EXPORT_SYMBOL_GPL(nlmclnt_done);
>  
> @@ -220,11 +222,12 @@ reclaimer(void *ptr)
>  	struct nlm_wait	  *block;
>  	struct file_lock *fl, *next;
>  	u32 nsmstate;
> +	struct net *net = host->net;
>  
>  	allow_signal(SIGKILL);
>  
>  	down_write(&host->h_rwsem);
> -	lockd_up();	/* note: this cannot fail as lockd is already running */
> +	lockd_up(net);	/* note: this cannot fail as lockd is already running */
>  
>  	dprintk("lockd: reclaiming locks for host %s\n", host->h_name);
>  
> @@ -275,6 +278,6 @@ restart:
>  
>  	/* Release host handle after use */
>  	nlmclnt_release_host(host);
> -	lockd_down();
> +	lockd_down(net);
>  	return 0;
>  }
> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> index b34100e..ce4c80e 100644
> --- a/fs/lockd/svc.c
> +++ b/fs/lockd/svc.c
> @@ -295,11 +295,10 @@ static void lockd_down_net(struct net *net)
>  /*
>   * Bring up the lockd process if it's not already up.
>   */
> -int lockd_up(void)
> +int lockd_up(struct net *net)
>  {
>  	struct svc_serv *serv;
>  	int		error = 0;
> -	struct net *net = current->nsproxy->net_ns;
>  
>  	mutex_lock(&nlmsvc_mutex);
>  	/*
> @@ -377,12 +376,12 @@ EXPORT_SYMBOL_GPL(lockd_up);
>   * Decrement the user count and bring down lockd if we're the last.
>   */
>  void
> -lockd_down(void)
> +lockd_down(struct net *net)
>  {
>  	mutex_lock(&nlmsvc_mutex);
>  	if (nlmsvc_users) {
>  		if (--nlmsvc_users) {
> -			lockd_down_net(current->nsproxy->net_ns);
> +			lockd_down_net(net);
>  			goto out;
>  		}
>  	} else {
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index fce472f..0f3e35b 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -220,7 +220,7 @@ static int nfsd_startup(unsigned short port, int nrservs)
>  	ret = nfsd_init_socks(port);
>  	if (ret)
>  		goto out_racache;
> -	ret = lockd_up();
> +	ret = lockd_up(&init_net);
>  	if (ret)
>  		goto out_racache;
>  	ret = nfs4_state_start();
> @@ -229,7 +229,7 @@ static int nfsd_startup(unsigned short port, int nrservs)
>  	nfsd_up = true;
>  	return 0;
>  out_lockd:
> -	lockd_down();
> +	lockd_down(&init_net);
>  out_racache:
>  	nfsd_racache_shutdown();
>  	return ret;
> @@ -246,7 +246,7 @@ static void nfsd_shutdown(void)
>  	if (!nfsd_up)
>  		return;
>  	nfs4_state_shutdown();
> -	lockd_down();
> +	lockd_down(&init_net);
>  	nfsd_racache_shutdown();
>  	nfsd_up = false;
>  }
> diff --git a/include/linux/lockd/bind.h b/include/linux/lockd/bind.h
> index 11a966e..4d24d64 100644
> --- a/include/linux/lockd/bind.h
> +++ b/include/linux/lockd/bind.h
> @@ -54,7 +54,7 @@ extern void	nlmclnt_done(struct nlm_host *host);
>  
>  extern int	nlmclnt_proc(struct nlm_host *host, int cmd,
>  					struct file_lock *fl);
> -extern int	lockd_up(void);
> -extern void	lockd_down(void);
> +extern int	lockd_up(struct net *net);
> +extern void	lockd_down(struct net *net);
>  
>  #endif /* LINUX_LOCKD_BIND_H */
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2] Lockd: pass network namespace to creation and destruction routines
From: Stanislav Kinsbursky @ 2012-04-11 16:12 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Pavel Emelianov, neilb-l3A5Bk7waGM@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	James Bottomley, davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
	devel-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org
In-Reply-To: <20120411161114.GB28928-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>

11.04.2012 20:11, J. Bruce Fields пишет:
> On Thu, Mar 29, 2012 at 06:54:33PM +0400, Stanislav Kinsbursky wrote:
>> v2: dereference of most probably already released nlm_host removed in
>> nlmclnt_done() and reclaimer().
>
> Did you want this in Trond's tree or mine?
>

Your tree is preferred since I'm working with it.

> --b.
>
>>
>> These routines are called from locks reclaimer() kernel thread. This thread
>> works in "init_net" network context and currently relays on persence on lockd
>> thread and it's per-net resources. Thus lockd_up() and lockd_down() can't relay
>> on current network context. So let's pass corrent one into them.
>>
>> Signed-off-by: Stanislav Kinsbursky<skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>>
>> Signed-off-by: Stanislav Kinsbursky<skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>>
>> ---
>>   fs/lockd/clntlock.c        |   13 ++++++++-----
>>   fs/lockd/svc.c             |    7 +++----
>>   fs/nfsd/nfssvc.c           |    6 +++---
>>   include/linux/lockd/bind.h |    4 ++--
>>   4 files changed, 16 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/lockd/clntlock.c b/fs/lockd/clntlock.c
>> index ba1dc2e..ca0a080 100644
>> --- a/fs/lockd/clntlock.c
>> +++ b/fs/lockd/clntlock.c
>> @@ -56,7 +56,7 @@ struct nlm_host *nlmclnt_init(const struct nlmclnt_initdata *nlm_init)
>>   	u32 nlm_version = (nlm_init->nfs_version == 2) ? 1 : 4;
>>   	int status;
>>
>> -	status = lockd_up();
>> +	status = lockd_up(nlm_init->net);
>>   	if (status<  0)
>>   		return ERR_PTR(status);
>>
>> @@ -65,7 +65,7 @@ struct nlm_host *nlmclnt_init(const struct nlmclnt_initdata *nlm_init)
>>   				   nlm_init->hostname, nlm_init->noresvport,
>>   				   nlm_init->net);
>>   	if (host == NULL) {
>> -		lockd_down();
>> +		lockd_down(nlm_init->net);
>>   		return ERR_PTR(-ENOLCK);
>>   	}
>>
>> @@ -80,8 +80,10 @@ EXPORT_SYMBOL_GPL(nlmclnt_init);
>>    */
>>   void nlmclnt_done(struct nlm_host *host)
>>   {
>> +	struct net *net = host->net;
>> +
>>   	nlmclnt_release_host(host);
>> -	lockd_down();
>> +	lockd_down(net);
>>   }
>>   EXPORT_SYMBOL_GPL(nlmclnt_done);
>>
>> @@ -220,11 +222,12 @@ reclaimer(void *ptr)
>>   	struct nlm_wait	  *block;
>>   	struct file_lock *fl, *next;
>>   	u32 nsmstate;
>> +	struct net *net = host->net;
>>
>>   	allow_signal(SIGKILL);
>>
>>   	down_write(&host->h_rwsem);
>> -	lockd_up();	/* note: this cannot fail as lockd is already running */
>> +	lockd_up(net);	/* note: this cannot fail as lockd is already running */
>>
>>   	dprintk("lockd: reclaiming locks for host %s\n", host->h_name);
>>
>> @@ -275,6 +278,6 @@ restart:
>>
>>   	/* Release host handle after use */
>>   	nlmclnt_release_host(host);
>> -	lockd_down();
>> +	lockd_down(net);
>>   	return 0;
>>   }
>> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
>> index b34100e..ce4c80e 100644
>> --- a/fs/lockd/svc.c
>> +++ b/fs/lockd/svc.c
>> @@ -295,11 +295,10 @@ static void lockd_down_net(struct net *net)
>>   /*
>>    * Bring up the lockd process if it's not already up.
>>    */
>> -int lockd_up(void)
>> +int lockd_up(struct net *net)
>>   {
>>   	struct svc_serv *serv;
>>   	int		error = 0;
>> -	struct net *net = current->nsproxy->net_ns;
>>
>>   	mutex_lock(&nlmsvc_mutex);
>>   	/*
>> @@ -377,12 +376,12 @@ EXPORT_SYMBOL_GPL(lockd_up);
>>    * Decrement the user count and bring down lockd if we're the last.
>>    */
>>   void
>> -lockd_down(void)
>> +lockd_down(struct net *net)
>>   {
>>   	mutex_lock(&nlmsvc_mutex);
>>   	if (nlmsvc_users) {
>>   		if (--nlmsvc_users) {
>> -			lockd_down_net(current->nsproxy->net_ns);
>> +			lockd_down_net(net);
>>   			goto out;
>>   		}
>>   	} else {
>> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
>> index fce472f..0f3e35b 100644
>> --- a/fs/nfsd/nfssvc.c
>> +++ b/fs/nfsd/nfssvc.c
>> @@ -220,7 +220,7 @@ static int nfsd_startup(unsigned short port, int nrservs)
>>   	ret = nfsd_init_socks(port);
>>   	if (ret)
>>   		goto out_racache;
>> -	ret = lockd_up();
>> +	ret = lockd_up(&init_net);
>>   	if (ret)
>>   		goto out_racache;
>>   	ret = nfs4_state_start();
>> @@ -229,7 +229,7 @@ static int nfsd_startup(unsigned short port, int nrservs)
>>   	nfsd_up = true;
>>   	return 0;
>>   out_lockd:
>> -	lockd_down();
>> +	lockd_down(&init_net);
>>   out_racache:
>>   	nfsd_racache_shutdown();
>>   	return ret;
>> @@ -246,7 +246,7 @@ static void nfsd_shutdown(void)
>>   	if (!nfsd_up)
>>   		return;
>>   	nfs4_state_shutdown();
>> -	lockd_down();
>> +	lockd_down(&init_net);
>>   	nfsd_racache_shutdown();
>>   	nfsd_up = false;
>>   }
>> diff --git a/include/linux/lockd/bind.h b/include/linux/lockd/bind.h
>> index 11a966e..4d24d64 100644
>> --- a/include/linux/lockd/bind.h
>> +++ b/include/linux/lockd/bind.h
>> @@ -54,7 +54,7 @@ extern void	nlmclnt_done(struct nlm_host *host);
>>
>>   extern int	nlmclnt_proc(struct nlm_host *host, int cmd,
>>   					struct file_lock *fl);
>> -extern int	lockd_up(void);
>> -extern void	lockd_down(void);
>> +extern int	lockd_up(struct net *net);
>> +extern void	lockd_down(struct net *net);
>>
>>   #endif /* LINUX_LOCKD_BIND_H */
>>


-- 
Best regards,
Stanislav Kinsbursky
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] atl1: fix kernel panic in case of DMA errors
From: Tony Zelenoff @ 2012-04-11 16:15 UTC (permalink / raw)
  To: davem; +Cc: antonz, netdev

Problem:
There was two separate work_struct structures which share one
handler. Unfortunately getting atl1_adapter structure from
work_struct in case of DMA error was done from incorrect
offset which cause kernel panics.

Solution:
The useless work_struct for DMA error removed and
handler name changed to more generic one.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c |   12 +++++-------
 drivers/net/ethernet/atheros/atlx/atl1.h |    3 +--
 drivers/net/ethernet/atheros/atlx/atlx.c |    2 +-
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index 40ac414..c926857 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -2476,7 +2476,7 @@ static irqreturn_t atl1_intr(int irq, void *data)
 					"pcie phy link down %x\n", status);
 			if (netif_running(adapter->netdev)) {	/* reset MAC */
 				iowrite32(0, adapter->hw.hw_addr + REG_IMR);
-				schedule_work(&adapter->pcie_dma_to_rst_task);
+				schedule_work(&adapter->reset_dev_task);
 				return IRQ_HANDLED;
 			}
 		}
@@ -2488,7 +2488,7 @@ static irqreturn_t atl1_intr(int irq, void *data)
 					"pcie DMA r/w error (status = 0x%x)\n",
 					status);
 			iowrite32(0, adapter->hw.hw_addr + REG_IMR);
-			schedule_work(&adapter->pcie_dma_to_rst_task);
+			schedule_work(&adapter->reset_dev_task);
 			return IRQ_HANDLED;
 		}
 
@@ -2633,10 +2633,10 @@ static void atl1_down(struct atl1_adapter *adapter)
 	atl1_clean_rx_ring(adapter);
 }
 
-static void atl1_tx_timeout_task(struct work_struct *work)
+static void atl1_reset_dev_task(struct work_struct *work)
 {
 	struct atl1_adapter *adapter =
-		container_of(work, struct atl1_adapter, tx_timeout_task);
+		container_of(work, struct atl1_adapter, reset_dev_task);
 	struct net_device *netdev = adapter->netdev;
 
 	netif_device_detach(netdev);
@@ -3038,12 +3038,10 @@ static int __devinit atl1_probe(struct pci_dev *pdev,
 		    (unsigned long)adapter);
 	adapter->phy_timer_pending = false;
 
-	INIT_WORK(&adapter->tx_timeout_task, atl1_tx_timeout_task);
+	INIT_WORK(&adapter->reset_dev_task, atl1_reset_dev_task);
 
 	INIT_WORK(&adapter->link_chg_task, atlx_link_chg_task);
 
-	INIT_WORK(&adapter->pcie_dma_to_rst_task, atl1_tx_timeout_task);
-
 	err = register_netdev(netdev);
 	if (err)
 		goto err_common;
diff --git a/drivers/net/ethernet/atheros/atlx/atl1.h b/drivers/net/ethernet/atheros/atlx/atl1.h
index 109d6da..e04bf4d 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.h
+++ b/drivers/net/ethernet/atheros/atlx/atl1.h
@@ -758,9 +758,8 @@ struct atl1_adapter {
 	u16 link_speed;
 	u16 link_duplex;
 	spinlock_t lock;
-	struct work_struct tx_timeout_task;
+	struct work_struct reset_dev_task;
 	struct work_struct link_chg_task;
-	struct work_struct pcie_dma_to_rst_task;
 
 	struct timer_list phy_config_timer;
 	bool phy_timer_pending;
diff --git a/drivers/net/ethernet/atheros/atlx/atlx.c b/drivers/net/ethernet/atheros/atlx/atlx.c
index 3cd8837..c9e9dc5 100644
--- a/drivers/net/ethernet/atheros/atlx/atlx.c
+++ b/drivers/net/ethernet/atheros/atlx/atlx.c
@@ -194,7 +194,7 @@ static void atlx_tx_timeout(struct net_device *netdev)
 {
 	struct atlx_adapter *adapter = netdev_priv(netdev);
 	/* Do the reset outside of interrupt context */
-	schedule_work(&adapter->tx_timeout_task);
+	schedule_work(&adapter->reset_dev_task);
 }
 
 /*
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH 05/10] net: move destructor_arg to the front of sk_buff.
From: Alexander Duyck @ 2012-04-11 16:31 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Eric Dumazet, netdev@vger.kernel.org, David Miller,
	Michael S. Tsirkin, Wei Liu (Intern), xen-devel@lists.xen.org
In-Reply-To: <1334131241.12209.199.camel@dagon.hellion.org.uk>

On 04/11/2012 01:00 AM, Ian Campbell wrote:
> On Tue, 2012-04-10 at 20:15 +0100, Alexander Duyck wrote:
>> On 04/10/2012 11:41 AM, Eric Dumazet wrote:
>>> On Tue, 2012-04-10 at 11:33 -0700, Alexander Duyck wrote:
>>>
>>>> Have you checked this for 32 bit as well as 64?  Based on my math your
>>>> next patch will still mess up the memset on 32 bit with the structure
>>>> being split somewhere just in front of hwtstamps.
>>>>
>>>> Why not just take frags and move it to the start of the structure?  It
>>>> is already an unknown value because it can be either 16 or 17 depending
>>>> on the value of PAGE_SIZE, and since you are making changes to frags the
>>>> changes wouldn't impact the alignment of the other values later on since
>>>> you are aligning the end of the structure.  That way you would be
>>>> guaranteed that all of the fields that will be memset would be in the
>>>> last 64 bytes.
>>>>
>>> Now when a fragmented packet is copied in pskb_expand_head(), you access
>>> two separate zones of memory to copy the shinfo. But its supposed to be
>>> slow path.
>>>
>>> Problem with this is that the offsets of often used fields will be big
>>> (instead of being < 127) and code will be bigger on x86.
>> Actually now that I think about it my concerns go much further than the
>> memset.  I'm convinced that this is going to cause a pretty significant
>> performance regression on multiple drivers, especially on non x86_64
>> architecture.  What we have right now on most platforms is a
>> skb_shared_info structure in which everything up to and including frag 0
>> is all in one cache line.  This gives us pretty good performance for igb
>> and ixgbe since that is our common case when jumbo frames are not
>> enabled is to split the head and place the data in a page.
> With all the changes in this series it is still possible to fit a
> maximum standard MTU frame and the shinfo on the same 4K page while also
> have the skb_shared_info up to and including frag [0] aligned to the
> same 64 byte cache line. 
>
> The only exception is destructor_arg on 64 bit which is on the preceding
> cache line but that is not a field used in any hot path.
The problem I have is that this is only true on x86_64.  Proper work
hasn't been done to guarantee this on any other architectures.

I think what I would like to see is instead of just setting things up
and hoping it comes out cache aligned on nr_frags why not take steps to
guarantee it?  You could do something like place and size the structure
based on:
SKB_DATA_ALIGN(sizeof(skb_shared_info) - offsetof(struct
skb_shared_info, nr_frags)) + offsetof(struct skb_shared_info, nr_frags)

That way you would have your alignment still guaranteed based off of the
end of the structure, but anything placed before nr_frags would be
placed on the end of the previous cache line.

>> However the change being recommend here only resolves the issue for one
>> specific architecture, and that is what I don't agree with.  What we
>> need is a solution that also works for 64K pages or 32 bit pointers and
>> I am fairly certain this current solution does not.
> I think it does work for 32 bit pointers. What issue to do you see with
> 64K pages?
>
> Ian.
With 64K pages the MAX_SKB_FRAGS value drops from 17 to 16.  That will
undoubtedly mess up the alignment.

Thanks,

Alex

^ permalink raw reply

* Re: [RFC PATCH 1/2] net: ethtool: Add capability to retrieve plug-in module EEPROM
From: Stuart Hodgson @ 2012-04-11 16:50 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: netdev, bruce.w.allan, mirq-linux, decot, amit.salecha,
	alexander.h.duyck, davem, linux-kernel
In-Reply-To: <1333389160.2623.30.camel@bwh-desktop.uk.solarflarecom.com>

On 02/04/12 18:52, Ben Hutchings wrote:
> We previously discussed the need for this in person, so I'm going to
> review this as a submission rather than an RFC.
>
> On Tue, 2012-03-27 at 18:51 +0100, Stuart Hodgson wrote:
>> Added extensions to the ethtool API to obtain plugin module eeprom data
>> This is useful for end users to be able to determine what the capabilities
>> of the in use module are.
>>
>> The first provides a new struct ethtool_modinfo that will return the
>> type and size of plug-in module eeprom (such as SFP+) for parsing
>> by userland program.
>>
>> The second provides the API to get the raw eeprom information
>> using the existing ethtool_eeprom structture to return the data
>>
>> Signed-off-by: Stuart Hodgson<smhodgson@solarflare.com>
>> ---
>>    include/linux/ethtool.h |   20 ++++++++++++
>>    net/core/ethtool.c      |   79
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>    2 files changed, 99 insertions(+), 0 deletions(-)
>
> This is line-wrapped; you'll need to take care to avoid that when
> submitting the patch for real.  Tabs have been converted to spaces,
> which you also need to avoid.  See Documentation/email-clients.txt.
>
>> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
>> index da5b2de..50eaf35 100644
>> --- a/include/linux/ethtool.h
>> +++ b/include/linux/ethtool.h
>> @@ -117,6 +117,14 @@ struct ethtool_eeprom {
>>        __u8    data[0];
>>    };
>>
>> +/* for passing plug-in module information */
>> +struct ethtool_modinfo {
>> +    __u32   cmd;
>> +    __u32   type;
>> +    __u32   eeprom_len;
>> +    __u32   reserved[8];
>> +};
>> +
>
> This needs a proper kernel-doc comment with a description of each of the
> fields (except reserved).
>
>>    /**
>>     * struct ethtool_coalesce - coalescing parameters for IRQs and stats
>> updates
>>     * @cmd: ETHTOOL_{G,S}COALESCE
>> @@ -936,6 +944,10 @@ struct ethtool_ops {
>>        int    (*get_dump_data)(struct net_device *,
>>                     struct ethtool_dump *, void *);
>>        int    (*set_dump)(struct net_device *, struct ethtool_dump *);
>> +    int    (*get_module_info)(struct net_device *,
>> +                   struct ethtool_modinfo *);
>> +    int    (*get_module_eeprom)(struct net_device *,
>> +                     struct ethtool_eeprom *, u8 *);
>
> These need to be described in the kernel-doc comment for this structure.
>
>>    };
>>    #endif /* __KERNEL__ */
>> @@ -1010,6 +1022,8 @@ struct ethtool_ops {
>>    #define ETHTOOL_SET_DUMP    0x0000003e /* Set dump settings */
>>    #define ETHTOOL_GET_DUMP_FLAG    0x0000003f /* Get dump settings */
>>    #define ETHTOOL_GET_DUMP_DATA    0x00000040 /* Get dump data */
>> +#define ETHTOOL_GMODULEINFO    0x00000041 /* Get plug-in module
>> information */
>> +#define ETHTOOL_GMODULEEEPROM    0x00000042 /* Get plug-in module eeprom */
>>
>>    /* compatibility with older code */
>>    #define SPARC_ETH_GSET        ETHTOOL_GSET
>> @@ -1159,6 +1173,12 @@ struct ethtool_ops {
>>    #define RX_CLS_LOC_FIRST    0xfffffffe
>>    #define RX_CLS_LOC_LAST        0xfffffffd
>>
>> +/* EEPROM Standards for plug in modules */
>> +#define SFF_8079        0x1
>> +#define SFF_8079_LEN        256
>> +#define SFF_8472        0x2
>> +#define SFF_8472_LEN        512
>> +
>
> I think it's best to include a prefix of 'ethtool_' 'ETHTOOL_' or 'ETH_'
> in new definitions in ethtool.h.  Since these are specific to modules I
> would suggest a prefix of 'ETH_MODULE_'.
>
>>    /* Reset flags */
>>    /* The reset() operation must clear the flags for the components which
>>     * were actually reset.  On successful return, the flags indicate the
>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>> index 3f79db1..1e86bd9 100644
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
> [...]
>> +static int ethtool_get_module_eeprom(struct net_device *dev,
>> +        void __user *useraddr)
>> +{
>> +    int ret;
>> +    struct ethtool_eeprom eeprom;
>> +    struct ethtool_modinfo modinfo;
>> +    const struct ethtool_ops *ops = dev->ethtool_ops;
>> +    void __user *userbuf = useraddr + sizeof(eeprom);
>> +    u8 *data;
>> +
>> +    if (!ops->get_module_info || !ops->get_module_eeprom)
>> +        return -EOPNOTSUPP;
>> +
>> +    if (copy_from_user(&eeprom, useraddr, sizeof(eeprom)))
>> +        return -EFAULT;
>> +
>> +    /* Check for wrap and zero */
>> +    if (eeprom.offset + eeprom.len<= eeprom.offset)
>> +        return -EINVAL;
>> +
>> +    /* Get the modinfo to get the length */
>> +    ret = ops->get_module_info(dev,&modinfo);
>> +    if (ret)
>> +        return ret;
>> +
>> +    if (eeprom.offset + eeprom.len>  modinfo.eeprom_len)
>> +        return -EINVAL;
>> +
>> +    data = kmalloc(PAGE_SIZE, GFP_USER);
>> +    if (!data)
>> +        return -ENOMEM;
>
> What if some device has a larger EEPROM?  Surely this length should be
> eeprom.len.
>

Do you mean what if the eeprom length in te device is larger than
PAGE_SIZE? If so then it should really use modinfo.eeprom_len since
this the size of the data. eeprom.len could be arbitary.

>> +    ret = ops->get_module_eeprom(dev,&eeprom, data);
>> +    if (ret)
>> +        goto out;
>> +
>> +
>> +    if (copy_to_user(userbuf, data, eeprom.len)) {
>> +        ret = -EFAULT;
>> +        goto out;
>> +    }
>> +
>> +    if (copy_to_user(useraddr,&eeprom, sizeof(eeprom)))
>> +        ret = -EFAULT;
> [...]
>
> I think you can drop this last copy as there's no information to return
> in the eeprom structure itself.
>
> Ben.
>

Other comments have been addressed and will be re-submitted.

Stu

^ permalink raw reply

* Re: [PATCH 05/10] net: move destructor_arg to the front of sk_buff.
From: Ian Campbell @ 2012-04-11 17:00 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Eric Dumazet, netdev@vger.kernel.org, David Miller,
	Michael S. Tsirkin, Wei Liu (Intern), xen-devel@lists.xen.org
In-Reply-To: <4F85B1EA.9000600@intel.com>

On Wed, 2012-04-11 at 17:31 +0100, Alexander Duyck wrote:
> On 04/11/2012 01:00 AM, Ian Campbell wrote:
> > On Tue, 2012-04-10 at 20:15 +0100, Alexander Duyck wrote:
> >> On 04/10/2012 11:41 AM, Eric Dumazet wrote:
> >>> On Tue, 2012-04-10 at 11:33 -0700, Alexander Duyck wrote:
> >>>
> >>>> Have you checked this for 32 bit as well as 64?  Based on my math your
> >>>> next patch will still mess up the memset on 32 bit with the structure
> >>>> being split somewhere just in front of hwtstamps.
> >>>>
> >>>> Why not just take frags and move it to the start of the structure?  It
> >>>> is already an unknown value because it can be either 16 or 17 depending
> >>>> on the value of PAGE_SIZE, and since you are making changes to frags the
> >>>> changes wouldn't impact the alignment of the other values later on since
> >>>> you are aligning the end of the structure.  That way you would be
> >>>> guaranteed that all of the fields that will be memset would be in the
> >>>> last 64 bytes.
> >>>>
> >>> Now when a fragmented packet is copied in pskb_expand_head(), you access
> >>> two separate zones of memory to copy the shinfo. But its supposed to be
> >>> slow path.
> >>>
> >>> Problem with this is that the offsets of often used fields will be big
> >>> (instead of being < 127) and code will be bigger on x86.
> >> Actually now that I think about it my concerns go much further than the
> >> memset.  I'm convinced that this is going to cause a pretty significant
> >> performance regression on multiple drivers, especially on non x86_64
> >> architecture.  What we have right now on most platforms is a
> >> skb_shared_info structure in which everything up to and including frag 0
> >> is all in one cache line.  This gives us pretty good performance for igb
> >> and ixgbe since that is our common case when jumbo frames are not
> >> enabled is to split the head and place the data in a page.
> > With all the changes in this series it is still possible to fit a
> > maximum standard MTU frame and the shinfo on the same 4K page while also
> > have the skb_shared_info up to and including frag [0] aligned to the
> > same 64 byte cache line. 
> >
> > The only exception is destructor_arg on 64 bit which is on the preceding
> > cache line but that is not a field used in any hot path.
> The problem I have is that this is only true on x86_64.  Proper work
> hasn't been done to guarantee this on any other architectures.

FWIW I did also explicitly cover i386 (see
<1334130984.12209.195.camel@dagon.hellion.org.uk>)

> I think what I would like to see is instead of just setting things up
> and hoping it comes out cache aligned on nr_frags why not take steps to
> guarantee it?  You could do something like place and size the structure
> based on:
> SKB_DATA_ALIGN(sizeof(skb_shared_info) - offsetof(struct
> skb_shared_info, nr_frags)) + offsetof(struct skb_shared_info, nr_frags)
> 
> That way you would have your alignment still guaranteed based off of the
> end of the structure, but anything placed before nr_frags would be
> placed on the end of the previous cache line.
> 
> >> However the change being recommend here only resolves the issue for one
> >> specific architecture, and that is what I don't agree with.  What we
> >> need is a solution that also works for 64K pages or 32 bit pointers and
> >> I am fairly certain this current solution does not.
> > I think it does work for 32 bit pointers. What issue to do you see with
> > 64K pages?
> >
> > Ian.
> With 64K pages the MAX_SKB_FRAGS value drops from 17 to 16.  That will
> undoubtedly mess up the alignment.

Oh, I see. Need to think about this some more but your suggestion above
is an interesting one, I'll see what I can do with that.

Ian.

^ permalink raw reply

* [PATCH 3/8] r6040: add a MAC operation timeout define
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
  To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1334164723-9627-1-git-send-email-florian@openwrt.org>

2048 is the usual value for busy-waiting on a register r/w, define it
as MAC_DEF_TIMEOUT and use it where it is appropriate.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 drivers/net/ethernet/rdc/r6040.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index fa29596..9ffbf6e 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -137,6 +137,8 @@
 #define MBCR_DEFAULT	0x012A	/* MAC Bus Control Register */
 #define MCAST_MAX	3	/* Max number multicast addresses to filter */
 
+#define MAC_DEF_TIMEOUT	2048	/* Default MAC read/write operation timeout */
+
 /* Descriptor status */
 #define DSC_OWNER_MAC	0x8000	/* MAC is the owner of this descriptor */
 #define DSC_RX_OK	0x4000	/* RX was successful */
@@ -204,7 +206,7 @@ static char version[] __devinitdata = DRV_NAME
 /* Read a word data from PHY Chip */
 static int r6040_phy_read(void __iomem *ioaddr, int phy_addr, int reg)
 {
-	int limit = 2048;
+	int limit = MAC_DEF_TIMEOUT;
 	u16 cmd;
 
 	iowrite16(MDIO_READ + reg + (phy_addr << 8), ioaddr + MMDIO);
@@ -222,7 +224,7 @@ static int r6040_phy_read(void __iomem *ioaddr, int phy_addr, int reg)
 static void r6040_phy_write(void __iomem *ioaddr,
 					int phy_addr, int reg, u16 val)
 {
-	int limit = 2048;
+	int limit = MAC_DEF_TIMEOUT;
 	u16 cmd;
 
 	iowrite16(val, ioaddr + MMWD);
@@ -361,7 +363,7 @@ err_exit:
 static void r6040_reset_mac(struct r6040_private *lp)
 {
 	void __iomem *ioaddr = lp->base;
-	int limit = 2048;
+	int limit = MAC_DEF_TIMEOUT;
 	u16 cmd;
 
 	iowrite16(MAC_RST, ioaddr + MCR1);
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 1/8] r6040: consolidate MAC reset to its own function
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
  To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1334164723-9627-1-git-send-email-florian@openwrt.org>

The reset of the MAC is currently done identically from two places
and one place is not waiting for the MAC_SM bit to be set after reset.
Everytime the MAC is software resetted a state machine is also needed
so consolidate the reset to its own function.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 drivers/net/ethernet/rdc/r6040.c |   37 ++++++++++++++++++-------------------
 1 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index a26307f..c10b0b8 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -358,27 +358,35 @@ err_exit:
 	return rc;
 }
 
-static void r6040_init_mac_regs(struct net_device *dev)
+static void r6040_reset_mac(struct r6040_private *lp)
 {
-	struct r6040_private *lp = netdev_priv(dev);
 	void __iomem *ioaddr = lp->base;
 	int limit = 2048;
 	u16 cmd;
 
-	/* Mask Off Interrupt */
-	iowrite16(MSK_INT, ioaddr + MIER);
-
-	/* Reset RDC MAC */
 	iowrite16(MAC_RST, ioaddr + MCR1);
 	while (limit--) {
 		cmd = ioread16(ioaddr + MCR1);
 		if (cmd & MAC_RST)
 			break;
 	}
+
 	/* Reset internal state machine */
 	iowrite16(MAC_SM_RST, ioaddr + MAC_SM);
 	iowrite16(0, ioaddr + MAC_SM);
 	mdelay(5);
+}
+
+static void r6040_init_mac_regs(struct net_device *dev)
+{
+	struct r6040_private *lp = netdev_priv(dev);
+	void __iomem *ioaddr = lp->base;
+
+	/* Mask Off Interrupt */
+	iowrite16(MSK_INT, ioaddr + MIER);
+
+	/* Reset RDC MAC */
+	r6040_reset_mac(lp);
 
 	/* MAC Bus Control Register */
 	iowrite16(MBCR_DEFAULT, ioaddr + MBCR);
@@ -445,18 +453,13 @@ static void r6040_down(struct net_device *dev)
 {
 	struct r6040_private *lp = netdev_priv(dev);
 	void __iomem *ioaddr = lp->base;
-	int limit = 2048;
 	u16 *adrp;
-	u16 cmd;
 
 	/* Stop MAC */
 	iowrite16(MSK_INT, ioaddr + MIER);	/* Mask Off Interrupt */
-	iowrite16(MAC_RST, ioaddr + MCR1);	/* Reset RDC MAC */
-	while (limit--) {
-		cmd = ioread16(ioaddr + MCR1);
-		if (cmd & MAC_RST)
-			break;
-	}
+
+	/* Reset RDC MAC */
+	r6040_reset_mac(lp);
 
 	/* Restore MAC Address to MIDx */
 	adrp = (u16 *) dev->dev_addr;
@@ -736,11 +739,7 @@ static void r6040_mac_address(struct net_device *dev)
 	u16 *adrp;
 
 	/* Reset MAC */
-	iowrite16(MAC_RST, ioaddr + MCR1);
-	/* Reset internal state machine */
-	iowrite16(MAC_SM_RST, ioaddr + MAC_SM);
-	iowrite16(0, ioaddr + MAC_SM);
-	mdelay(5);
+	r6040_reset_mac(lp);
 
 	/* Restore MAC Address */
 	adrp = (u16 *) dev->dev_addr;
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 0/8] r6040: misc cleanups
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
  To: davem; +Cc: netdev, Florian Fainelli

Hi David,

This series contains some cleanups and one small fix, they are targetted
at net-next.

Thanks!

Florian Fainelli (8):
  r6040: consolidate MAC reset to its own function
  r6040: remove unused variable mcr1 from private structure
  r6040: add a MAC operation timeout define
  r6040: fix typo on stats update in tx path
  r6040: define and use MLSR register bits
  r6040: define and use MTPR transmit enable bit
  r6040: define and use bits of register PHY_CC
  r6040: update copyright from 2007 to now

 drivers/net/ethernet/rdc/r6040.c |   75 ++++++++++++++++++++++----------------
 1 files changed, 44 insertions(+), 31 deletions(-)

-- 
1.7.5.4

^ permalink raw reply

* [PATCH 5/8] r6040: define and use MLSR register bits
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
  To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1334164723-9627-1-git-send-email-florian@openwrt.org>

Define the MLSR (MAC Last Status Register bits) for:
- tx fifo under-run
- tx exceed collision
- tx late collision

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 drivers/net/ethernet/rdc/r6040.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index db33573..1a365ae 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -77,6 +77,9 @@
 #define MR_BSR		0x18	/* RX buffer size */
 #define MR_DCR		0x1A	/* RX descriptor control */
 #define MLSR		0x1C	/* Last status */
+#define  TX_FIFO_UNDR	0x0200	/* TX FIFO under-run */
+#define	 TX_EXCEEDC	0x2000	/* Transmit exceed collision */
+#define  TX_LATEC	0x4000	/* Transmit late collision */
 #define MMDIO		0x20	/* MDIO control register */
 #define  MDIO_WRITE	0x4000	/* MDIO write */
 #define  MDIO_READ	0x2000	/* MDIO read */
@@ -604,9 +607,9 @@ static void r6040_tx(struct net_device *dev)
 		/* Check for errors */
 		err = ioread16(ioaddr + MLSR);
 
-		if (err & 0x0200)
+		if (err & TX_FIFO_UNDR)
 			dev->stats.tx_fifo_errors++;
-		if (err & (0x2000 | 0x4000))
+		if (err & (TX_EXCEEDC | TX_LATEC))
 			dev->stats.tx_carrier_errors++;
 
 		if (descptr->status & DSC_OWNER_MAC)
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 2/8] r6040: remove unused variable mcr1 from private structure
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
  To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1334164723-9627-1-git-send-email-florian@openwrt.org>

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 drivers/net/ethernet/rdc/r6040.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index c10b0b8..fa29596 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -187,7 +187,7 @@ struct r6040_private {
 	dma_addr_t rx_ring_dma;
 	dma_addr_t tx_ring_dma;
 	u16	tx_free_desc;
-	u16	mcr0, mcr1;
+	u16	mcr0;
 	struct net_device *dev;
 	struct mii_bus *mii_bus;
 	struct napi_struct napi;
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 7/8] r6040: define and use bits of register PHY_CC
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
  To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1334164723-9627-1-git-send-email-florian@openwrt.org>

Define and use the bits of the PHY_CC (status change configuration) register.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 drivers/net/ethernet/rdc/r6040.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index afa4186..050cf59 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -128,6 +128,9 @@
 #define MID_3M		0x82	/* MID3 Medium */
 #define MID_3H		0x84	/* MID3 High */
 #define PHY_CC		0x88	/* PHY status change configuration register */
+#define  SCEN		0x8000	/* PHY status change enable */
+#define  PHYAD_SHIFT	8	/* PHY address shift */
+#define  TMRDIV_SHIFT	0	/* Timer divider shift */
 #define PHY_ST		0x8A	/* PHY status register */
 #define MAC_SM		0xAC	/* MAC status machine */
 #define  MAC_SM_RST	0x0002	/* MAC status machine reset */
@@ -1132,10 +1135,15 @@ static int __devinit r6040_init_one(struct pci_dev *pdev,
 		err = -EIO;
 		goto err_out_free_res;
 	}
+
 	/* If PHY status change register is still set to zero it means the
-	 * bootloader didn't initialize it */
+	 * bootloader didn't initialize it, so we set it to:
+	 * - enable phy status change
+	 * - enable all phy addresses
+	 * - set to lowest timer divider */
 	if (ioread16(ioaddr + PHY_CC) == 0)
-		iowrite16(0x9f07, ioaddr + PHY_CC);
+		iowrite16(SCEN | PHY_MAX_ADDR << PHYAD_SHIFT |
+				7 << TMRDIV_SHIFT, ioaddr + PHY_CC);
 
 	/* Init system & device */
 	lp->base = ioaddr;
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 6/8] r6040: define and use MTPR transmit enable bit
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
  To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1334164723-9627-1-git-send-email-florian@openwrt.org>

Define MTPR bit 0 of the register and use it where it is appropriate.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 drivers/net/ethernet/rdc/r6040.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index 1a365ae..afa4186 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -74,6 +74,7 @@
 #define MT_ICR		0x0C	/* TX interrupt control */
 #define MR_ICR		0x10	/* RX interrupt control */
 #define MTPR		0x14	/* TX poll command register */
+#define  TM2TX		0x0001	/* Trigger MAC to transmit */
 #define MR_BSR		0x18	/* RX buffer size */
 #define MR_DCR		0x1A	/* RX descriptor control */
 #define MLSR		0x1C	/* Last status */
@@ -420,7 +421,7 @@ static void r6040_init_mac_regs(struct net_device *dev)
 	/* Let TX poll the descriptors
 	 * we may got called by r6040_tx_timeout which has left
 	 * some unsent tx buffers */
-	iowrite16(0x01, ioaddr + MTPR);
+	iowrite16(TM2TX, ioaddr + MTPR);
 }
 
 static void r6040_tx_timeout(struct net_device *dev)
@@ -844,7 +845,7 @@ static netdev_tx_t r6040_start_xmit(struct sk_buff *skb,
 	skb_tx_timestamp(skb);
 
 	/* Trigger the MAC to check the TX descriptor */
-	iowrite16(0x01, ioaddr + MTPR);
+	iowrite16(TM2TX, ioaddr + MTPR);
 	lp->tx_insert_ptr = descptr->vndescp;
 
 	/* If no tx resource, stop */
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 4/8] r6040: fix typo on stats update in tx path
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
  To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1334164723-9627-1-git-send-email-florian@openwrt.org>

We are currently updating the rx fifo error counter in the tx path while
it should have been the tx fifo error counter, fix that.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 drivers/net/ethernet/rdc/r6040.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index 9ffbf6e..db33573 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -605,7 +605,7 @@ static void r6040_tx(struct net_device *dev)
 		err = ioread16(ioaddr + MLSR);
 
 		if (err & 0x0200)
-			dev->stats.rx_fifo_errors++;
+			dev->stats.tx_fifo_errors++;
 		if (err & (0x2000 | 0x4000))
 			dev->stats.tx_carrier_errors++;
 
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 8/8] r6040: update copyright from 2007 to now
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
  To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1334164723-9627-1-git-send-email-florian@openwrt.org>

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
 drivers/net/ethernet/rdc/r6040.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index 050cf59..4de7364 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -4,7 +4,7 @@
  * Copyright (C) 2004 Sten Wang <sten.wang@rdc.com.tw>
  * Copyright (C) 2007
  *	Daniel Gimpelevich <daniel@gimpelevich.san-francisco.ca.us>
- *	Florian Fainelli <florian@openwrt.org>
+ * Copyright (C) 2007-2012 Florian Fainelli <florian@openwrt.org>
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
-- 
1.7.5.4

^ permalink raw reply related

* Re: [net-next PATCH v1 1/7] net: add generic PF_BRIDGE:RTM_ FDB hooks
From: John Fastabend @ 2012-04-11 17:22 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: roprabhu, mst, stephen.hemminger, davem, hadi, jeffrey.t.kirsher,
	netdev, gregory.v.rose, krkumar2, sri
In-Reply-To: <1334160350.2552.2.camel@bwh-desktop.uk.solarflarecom.com>

On 4/11/2012 9:05 AM, Ben Hutchings wrote:
> On Wed, 2012-04-11 at 07:45 -0700, John Fastabend wrote:
>> On 4/10/2012 8:23 PM, Ben Hutchings wrote:
>>> On Mon, 2012-04-09 at 15:00 -0700, John Fastabend wrote:
>>> [...]
>>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>>> index 1f77540..05822e5 100644
>>>> --- a/include/linux/netdevice.h
>>>> +++ b/include/linux/netdevice.h
>>> [...]
>>>> @@ -905,6 +906,19 @@ struct netdev_fcoe_hbainfo {
>>>>   *	feature set might be less than what was returned by ndo_fix_features()).
>>>>   *	Must return >0 or -errno if it changed dev->features itself.
>>>>   *
>>>> + * int (*ndo_fdb_add)(struct ndmsg *ndm, struct net_device *dev,
>>>> + *		      unsigned char *addr, u16 flags)
>>>> + *	Adds an FDB entry to dev for addr. The ndmsg contains flags to indicate
>>>> + *	if the dev->master FDB should be updated or the devices internal FDB.
>>>
>>> I don't think the second sentence is helpful, as rtnl_fdb_add() will
>>> take care of those flags.
>>>
>>>> + * int (*ndo_fdb_del)(struct ndmsg *ndm, struct net_device *dev,
>>>> + *		      unsigned char *addr)
>>>> + *	Deletes the FDB entry from dev coresponding to addr. The ndmsg
>>>> + *	contains flags to indicate if the dev->master FDB should be
>>>> + *	updated or the devices internal FDB.
>>>
>>> Similarly here.
>>
>> agreed neither seem particularly helpful I'll remove them.
>>
>>>
>>>> + * int (*ndo_fdb_dump)(struct sk_buff *skb, struct netlink_callback *cb,
>>>> + *		       struct net_device *dev, int idx)
>>>> + *	Used to add FDB entries to dump requests. Implementers should add
>>>> + *	entries to skb and update idx with the number of entries.
>>>>   */
>>> [...
>>>> --- a/net/core/rtnetlink.c
>>>> +++ b/net/core/rtnetlink.c
>>> [...]
>>>> +static int rtnl_fdb_add(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
>>>> +{
>>> [...]
>>>> +	err = -EOPNOTSUPP;
>>>
>>> So if NTF_MASTER and NTF_SELF are both set, we can quietly fall back to
>>> just setting one FDB?  Not sure that's really desirable though 
>>>
>>
>> It makes it easier to keep an embedded agent and the sw bridge in
>> sync if setting both flags adds the entry to both the SW bridge and
>> embedded bridge.
> 
> Yes, I agree with that.
> 
> [...]
>>> Wonder what we should do on error here if we've already successfully
>>> called ndo_fdb_add on the master?  Should we try to roll back the first
>>> addition?
>>>
>>
>> The problem with rolling back is the table is likely already updated and
>> traffic may already be being forwarded. So I think in this case the user
>> will have to query the device to learn what failed. It seems like the
>> simplest way to handle this. I think it is unwanted to have traffic being
>> forwarded one way for a short period of time then rolled back.
>>
>> The other idea I just had is we could clear the NTF_ bit in ndm_flags after
>> the successful add, del command. I believe the nlmsg gets sent back to the
>> user on error I would need to check on this.
> [...]
> 
> That sounds like a good way of doing it, assuming there's no
> compatibility issue.
> 
> Ben.
> 

I don't see any compat issues. Current applications shouldn't be setting
any flags and shouldn't be expecting any flags in the ack.

Also I think I need to add some notify hooks to help management. The
bridge code has fdb_notify() so I'll add a similar rtnl_fdb_notify() hook
here. Caught this while implementing some user space daemon code.

Thanks! v2 coming soon.

^ permalink raw reply

* Re: [RFC PATCH 2/2] net: ethtool: Add capability to retrieve plug-in module EEPROM
From: Stuart Hodgson @ 2012-04-11 17:41 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev, davem, linux-kernel
In-Reply-To: <1333390698.2623.47.camel@bwh-desktop.uk.solarflarecom.com>

On 02/04/12 19:18, Ben Hutchings wrote:
> On Tue, 2012-03-27 at 18:51 +0100, Stuart Hodgson wrote:
>> Implementation in sfc driver to return the plugin module eeprom
>>
>> Currently allows for SFP+ eeprom to be returned using the ethtool API.
>> This can be extended in future to handle different eeprom formats
>> and sizes.
>>
>> Signed-off-by: Stuart Hodgson<smhodgson@solarflare.com>
>> ---
>>    drivers/net/ethernet/sfc/ethtool.c    |   36 +++++++++++
>>    drivers/net/ethernet/sfc/mcdi_phy.c   |  105
>> +++++++++++++++++++++++++++++++++
>>    drivers/net/ethernet/sfc/net_driver.h |    5 ++
>>    3 files changed, 146 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/sfc/ethtool.c
>> b/drivers/net/ethernet/sfc/ethtool.c
>> index f22f45f..e77895f 100644
>> --- a/drivers/net/ethernet/sfc/ethtool.c
>> +++ b/drivers/net/ethernet/sfc/ethtool.c
>> @@ -1108,6 +1108,40 @@ static int efx_ethtool_set_rxfh_indir(struct
>> net_device *net_dev,
>>        return 0;
>>    }
>>
>> +static int efx_ethtool_get_module_eeprom(struct net_device *net_dev,
>> +                     struct ethtool_eeprom *ee,
>> +                     u8 *data)
>> +{
>> +    struct efx_nic *efx = netdev_priv(net_dev);
>> +    int ret;
>> +
>> +    if (!efx->phy_op ||
>> +        !efx->phy_op->get_module_eeprom)
>
> No need to break that line.
>
> [...]
>> --- a/drivers/net/ethernet/sfc/mcdi_phy.c
>> +++ b/drivers/net/ethernet/sfc/mcdi_phy.c
>> @@ -304,6 +304,17 @@ static u32 mcdi_to_ethtool_media(u32 media)
>>        }
>>    }
>>
>> +static u32 mcdi_to_module_eeprom_len(u32 media)
>> +{
>> +    switch (media) {
>> +    case MC_CMD_MEDIA_SFP_PLUS:
>> +        return SFF_8079_LEN;
>> +    case MC_CMD_MEDIA_XFP:
>
> Why split out XFP if we're not going to treat it any differently?
>
>> +    default:
>> +        return 0;
>> +    }
>> +}
>> +
>>    static int efx_mcdi_phy_probe(struct efx_nic *efx)
>>    {
>>        struct efx_mcdi_phy_data *phy_data;
>> @@ -739,6 +750,98 @@ static const char *efx_mcdi_phy_test_name(struct
>> efx_nic *efx,
>>        return NULL;
>>    }
>>
>> +#define SFP_PAGE_SIZE    128
>> +#define NUM_PAGES    2
>
> SFP_NUM_PAGES?
>
>> +#define OFF_TO_BUFF(x)    (x + MC_CMD_GET_PHY_MEDIA_INFO_OUT_DATA_OFST)
>
> This is not really worth defining for just the one use.
>
>> +static int efx_mcdi_phy_get_module_eeprom(struct efx_nic *efx,
>> +                      struct ethtool_eeprom *ee, u8 *data)
>> +{
>> +    u8 outbuf[MC_CMD_GET_PHY_MEDIA_INFO_OUT_LENMAX];
>> +    u8 inbuf[MC_CMD_GET_PHY_MEDIA_INFO_IN_LEN];
>> +    size_t outlen;
>> +    int rc;
>> +    int payload_len;
>> +    int copied = 0;
>> +    int space_remaining = ee->len;
>> +    int page;
>> +    int page_off;
>> +    int to_copy;
>
> None of payload_len..to_copy should be signed.
>
>> +    u8 *user_data = data;
>> +
>> +    if (!data || !ee)
>> +        return -EINVAL;
>
> Neither of these is allowed to be NULL; don't bother checking.
>
>> +    if (ee->offset>  (SFP_PAGE_SIZE * NUM_PAGES)) {
>> +        rc = -EINVAL;
>> +        goto fail;
>> +    }
>> +
>> +    page_off = (ee->offset % SFP_PAGE_SIZE);
>> +    page = (ee->offset>  SFP_PAGE_SIZE) ? 1 : 0;
>
> Why not ee->offset / SFP_PAGE_SIZE?
>
>> +    while (space_remaining&&  (page<  NUM_PAGES)) {
>> +
>> +        MCDI_SET_DWORD(inbuf, GET_PHY_MEDIA_INFO_IN_PAGE, page);
>> +
>> +        rc = efx_mcdi_rpc(efx, MC_CMD_GET_PHY_MEDIA_INFO,
>> +                  inbuf, sizeof(inbuf),
>> +                  outbuf, sizeof(outbuf),
>> +&outlen);
>> +
>> +        if (rc)
>> +            goto fail;
>> +
>> +        /* Copy as much as we can into data */
>> +        if (outlen<  MC_CMD_GET_PHY_MEDIA_INFO_OUT_LENMIN ||
>> +            outlen>  MC_CMD_GET_PHY_MEDIA_INFO_OUT_LENMAX) {
>> +            rc = -EIO;
>> +            goto fail;
>> +        }
>> +
>> +        payload_len = MCDI_DWORD(outbuf,
>> +                     GET_PHY_MEDIA_INFO_OUT_DATALEN);
>> +
>> +        to_copy = (space_remaining<  payload_len) ?
>> +                space_remaining : payload_len;
>> +
>> +        to_copy -= page_off;
>
> page_off is the number of bytes we need to discard from payload_len, but
> we don't want do discard that from space_remaining.  I think the last
> two statements should be changed to:
>
> 	payload_len -= page_off;
> 	to_copy = (space_remaining<  payload_len) ?
> 		space_remaining : payload_len;
>

I am pretty sure that these two pieces of code are the same

>> +        memcpy(user_data,
>> +               (outbuf + OFF_TO_BUFF(page_off)),
>> +               to_copy);
>> +
>> +        space_remaining -= to_copy;
>> +        user_data += to_copy;
>> +        copied += to_copy;
>> +        page_off = 0;
>> +        page++;
>> +    }
>> +
>> +    ee->len = copied;
>> +
>> +    return 0;
>> +fail:
>> +    return rc;
>
> Nothing to clean up here, so you might as well return errors directly.
>
>> +}
>> +
>> +static int efx_mcdi_phy_get_module_info(struct efx_nic *efx,
>> +                    struct ethtool_modinfo *modinfo)
>> +{
>> +    /* This will return a length of the eeprom
>> +     * type of the module that was detected during the probe,
>> +     * if not modules inserted then phy_data will be NULL */
>> +    struct efx_mcdi_phy_data *phy_cfg;
>> +
>> +    if (!efx || !efx->phy_data)
>> +        return -EOPNOTSUPP;
>
> efx certainly can't be NULL here and I don't believe efx->phy_data can
> either.  (None of the other MCDI PHY operations check it.)
>
>> +    phy_cfg = efx->phy_data;
>> +    modinfo->eeprom_len = mcdi_to_module_eeprom_len(phy_cfg->media);
>> +    modinfo->type = SFF_8079;
> [...]
>
> I don't think this makes sense.  If we're fixing the type as SFF_8079
> then why are we calling a function to get the length?
>
> Ben.
>

What about adding an mcdi_to_module_eeprom_type in the same manner
as mcdi_to_module_eeprom_len and the other mapping functions?

Stu

^ permalink raw reply

* [PATCH 05/10] mac80211: Add more ethtools stats: survey, rates, etc
From: greearb-my8/4N5VtI7c+919tysfdA @ 2012-04-11 17:52 UTC (permalink / raw)
  To: linux-wireless-u79uwXL29TY76Z2rM5mHXA
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Ben Greear
In-Reply-To: <1334166738-28243-1-git-send-email-greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>

From: Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>

The signal and noise are forced to be positive since ethtool
deals in unsigned 64-bit values and this number should be human
readable.  This gives easy access to some of the data formerly
exposed in the deprecated /proc/net/wireless file.

Signed-off-by: Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
---
:100644 100644 ea2e995... bbaf564... M	net/mac80211/cfg.c
 net/mac80211/cfg.c |  156 ++++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 115 insertions(+), 41 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index ea2e995..bbaf564 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -19,6 +19,7 @@
 #include "cfg.h"
 #include "rate.h"
 #include "mesh.h"
+#include "../wireless/core.h"
 
 static struct net_device *ieee80211_add_iface(struct wiphy *wiphy, char *name,
 					      enum nl80211_iftype type,
@@ -117,7 +118,9 @@ static const char ieee80211_gstrings_sta_stats[][ETH_GSTRING_LEN] = {
 	"rx_duplicates", "rx_fragments", "rx_dropped",
 	"tx_packets", "tx_bytes", "tx_fragments",
 	"tx_filtered", "tx_retry_failed", "tx_retries",
-	"beacon_loss"
+	"beacon_loss", "txrate", "rxrate", "signal",
+	"channel", "noise", "ch_time", "ch_time_busy",
+	"ch_time_ext_busy", "ch_time_rx", "ch_time_tx"
 };
 #define STA_STATS_LEN	ARRAY_SIZE(ieee80211_gstrings_sta_stats)
 
@@ -138,46 +141,6 @@ static int ieee80211_get_et_sset_count(struct wiphy *wiphy,
 	return rv;
 }
 
-static void ieee80211_get_et_stats(struct wiphy *wiphy,
-				   struct net_device *dev,
-				   struct ethtool_stats *stats,
-				   u64 *data)
-{
-	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
-	struct sta_info *sta;
-	struct ieee80211_local *local = sdata->local;
-
-	memset(data, 0, sizeof(u64) * STA_STATS_LEN);
-
-	rcu_read_lock();
-	list_for_each_entry_rcu(sta, &local->sta_list, list) {
-		int i = 0;
-
-		/* Make sure this station belongs to the proper dev */
-		if (sta->sdata->dev != dev)
-			continue;
-
-		data[i++] += sta->rx_packets;
-		data[i++] += sta->rx_bytes;
-		data[i++] += sta->wep_weak_iv_count;
-		data[i++] += sta->num_duplicates;
-		data[i++] += sta->rx_fragments;
-		data[i++] += sta->rx_dropped;
-
-		data[i++] += sta->tx_packets;
-		data[i++] += sta->tx_bytes;
-		data[i++] += sta->tx_fragments;
-		data[i++] += sta->tx_filtered_count;
-		data[i++] += sta->tx_retry_failed;
-		data[i++] += sta->tx_retry_count;
-		data[i++] += sta->beacon_loss_count;
-		BUG_ON(i != STA_STATS_LEN);
-	}
-	rcu_read_unlock();
-
-	drv_get_et_stats(sdata, stats, &(data[STA_STATS_LEN]));
-}
-
 static void ieee80211_get_et_strings(struct wiphy *wiphy,
 				     struct net_device *dev,
 				     u32 sset, u8 *data)
@@ -531,6 +494,117 @@ static void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo)
 		sinfo->sta_flags.set |= BIT(NL80211_STA_FLAG_TDLS_PEER);
 }
 
+static void ieee80211_get_et_stats(struct wiphy *wiphy,
+				   struct net_device *dev,
+				   struct ethtool_stats *stats,
+				   u64 *data)
+{
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	struct sta_info *sta;
+	struct ieee80211_local *local = sdata->local;
+	struct station_info sinfo;
+	struct survey_info survey;
+	bool do_once = true;
+	int i;
+#define STA_STATS_SURVEY_LEN 7
+
+	memset(data, 0, sizeof(u64) * STA_STATS_LEN);
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(sta, &local->sta_list, list) {
+		i = 0;
+
+		/* Make sure this station belongs to the proper dev */
+		if (sta->sdata->dev != dev)
+			continue;
+
+		data[i++] += sta->rx_packets;
+		data[i++] += sta->rx_bytes;
+		data[i++] += sta->wep_weak_iv_count;
+		data[i++] += sta->num_duplicates;
+		data[i++] += sta->rx_fragments;
+		data[i++] += sta->rx_dropped;
+
+		data[i++] += sta->tx_packets;
+		data[i++] += sta->tx_bytes;
+		data[i++] += sta->tx_fragments;
+		data[i++] += sta->tx_filtered_count;
+		data[i++] += sta->tx_retry_failed;
+		data[i++] += sta->tx_retry_count;
+		data[i++] += sta->beacon_loss_count;
+
+		if (!do_once) {
+			i += 3;
+			goto after_once;
+		}
+
+		do_once = false;
+		sinfo.filled = 0;
+		sta_set_sinfo(sta, &sinfo);
+
+		if (sinfo.filled | STATION_INFO_TX_BITRATE)
+			data[i] = 100000 *
+				cfg80211_calculate_bitrate(&sinfo.txrate);
+		i++;
+		if (sinfo.filled | STATION_INFO_RX_BITRATE)
+			data[i] = 100000 *
+				cfg80211_calculate_bitrate(&sinfo.rxrate);
+		i++;
+
+		if (sinfo.filled | STATION_INFO_SIGNAL_AVG)
+			data[i] = abs(sinfo.signal_avg);
+		i++;
+
+after_once:
+		if (WARN_ON(i != (STA_STATS_LEN - STA_STATS_SURVEY_LEN))) {
+			rcu_read_unlock();
+			return;
+		}
+	}
+
+	i = STA_STATS_LEN - STA_STATS_SURVEY_LEN;
+	/* Get survey stats for current channel only */
+	survey.filled = 0;
+	if (drv_get_survey(local, 0, &survey) != 0) {
+		survey.filled = 0;
+		data[i++] = 0;
+	} else {
+		data[i++] = survey.channel->center_freq;
+	}
+
+	if (survey.filled & SURVEY_INFO_NOISE_DBM)
+		data[i++] = abs(survey.noise);
+	else
+		data[i++] = -1LL;
+	if (survey.filled & SURVEY_INFO_CHANNEL_TIME)
+		data[i++] = survey.channel_time;
+	else
+		data[i++] = -1LL;
+	if (survey.filled & SURVEY_INFO_CHANNEL_TIME_BUSY)
+		data[i++] = survey.channel_time_busy;
+	else
+		data[i++] = -1LL;
+	if (survey.filled & SURVEY_INFO_CHANNEL_TIME_EXT_BUSY)
+		data[i++] = survey.channel_time_ext_busy;
+	else
+		data[i++] = -1LL;
+	if (survey.filled & SURVEY_INFO_CHANNEL_TIME_RX)
+		data[i++] = survey.channel_time_rx;
+	else
+		data[i++] = -1LL;
+	if (survey.filled & SURVEY_INFO_CHANNEL_TIME_TX)
+		data[i++] = survey.channel_time_tx;
+	else
+		data[i++] = -1LL;
+
+	if (WARN_ON(i != STA_STATS_LEN)) {
+		rcu_read_unlock();
+		return;
+	}
+
+	drv_get_et_stats(sdata, stats, &(data[STA_STATS_LEN]));
+}
+
 
 static int ieee80211_dump_station(struct wiphy *wiphy, struct net_device *dev,
 				 int idx, u8 *mac, struct station_info *sinfo)
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 06/10] mac80211: Add sta_state to ethtool stats.
From: greearb-my8/4N5VtI7c+919tysfdA @ 2012-04-11 17:52 UTC (permalink / raw)
  To: linux-wireless-u79uwXL29TY76Z2rM5mHXA
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Ben Greear
In-Reply-To: <1334166738-28243-1-git-send-email-greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>

From: Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>

Helps to know how the station is doing in it's association
attempt.

Signed-off-by: Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
---
:100644 100644 bbaf564... a63834d... M	net/mac80211/cfg.c
 net/mac80211/cfg.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index bbaf564..a63834d 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -118,7 +118,7 @@ static const char ieee80211_gstrings_sta_stats[][ETH_GSTRING_LEN] = {
 	"rx_duplicates", "rx_fragments", "rx_dropped",
 	"tx_packets", "tx_bytes", "tx_fragments",
 	"tx_filtered", "tx_retry_failed", "tx_retries",
-	"beacon_loss", "txrate", "rxrate", "signal",
+	"beacon_loss", "sta_state", "txrate", "rxrate", "signal",
 	"channel", "noise", "ch_time", "ch_time_busy",
 	"ch_time_ext_busy", "ch_time_rx", "ch_time_tx"
 };
@@ -534,10 +534,12 @@ static void ieee80211_get_et_stats(struct wiphy *wiphy,
 		data[i++] += sta->beacon_loss_count;
 
 		if (!do_once) {
-			i += 3;
+			i += 4;
 			goto after_once;
 		}
 
+		data[i++] = sta->sta_state;
+
 		do_once = false;
 		sinfo.filled = 0;
 		sta_set_sinfo(sta, &sinfo);
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 08/10] mac80211: Support on-channel scan option.
From: greearb-my8/4N5VtI7c+919tysfdA @ 2012-04-11 17:52 UTC (permalink / raw)
  To: linux-wireless-u79uwXL29TY76Z2rM5mHXA
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Ben Greear
In-Reply-To: <1334166738-28243-1-git-send-email-greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>

From: Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>

This based on an idea posted by Stanislaw Gruszka,
though I accept full blame for the implementation!

This is compile-tested only at this point.

The idea is to let users scan on the current operating
channel without interrupting normal traffic more than
absolutely necessary (changing power level might reset
some hardware, for instance).

Signed-off-by: Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
---
:100644 100644 ea9623c... 82da2c9... M	net/mac80211/ieee80211_i.h
:100644 100644 d019f0d... dff9c22... M	net/mac80211/main.c
:100644 100644 54a0491... dd2fbec... M	net/mac80211/rx.c
:100644 100644 c70e176... 5c7a332... M	net/mac80211/scan.c
 net/mac80211/ieee80211_i.h |    3 +
 net/mac80211/main.c        |    4 +-
 net/mac80211/rx.c          |    2 +
 net/mac80211/scan.c        |   92 +++++++++++++++++++++++++++++++-------------
 4 files changed, 73 insertions(+), 28 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ea9623c..82da2c9 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -803,6 +803,8 @@ struct tpt_led_trigger {
  *	well be on the operating channel
  * @SCAN_HW_SCANNING: The hardware is scanning for us, we have no way to
  *	determine if we are on the operating channel or not
+ * @SCAN_ONCHANNEL_SCANNING:  Do a software scan on only the current operating
+ *	channel. This should not interrupt normal traffic.
  * @SCAN_COMPLETED: Set for our scan work function when the driver reported
  *	that the scan completed.
  * @SCAN_ABORTED: Set for our scan work function when the driver reported
@@ -811,6 +813,7 @@ struct tpt_led_trigger {
 enum {
 	SCAN_SW_SCANNING,
 	SCAN_HW_SCANNING,
+	SCAN_ONCHANNEL_SCANNING,
 	SCAN_COMPLETED,
 	SCAN_ABORTED,
 };
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index d019f0d..dff9c22 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -47,7 +47,8 @@ void ieee80211_configure_filter(struct ieee80211_local *local)
 	if (atomic_read(&local->iff_allmultis))
 		new_flags |= FIF_ALLMULTI;
 
-	if (local->monitors || test_bit(SCAN_SW_SCANNING, &local->scanning))
+	if (local->monitors || test_bit(SCAN_SW_SCANNING, &local->scanning) ||
+	    test_bit(SCAN_ONCHANNEL_SCANNING, &local->scanning))
 		new_flags |= FIF_BCN_PRBRESP_PROMISC;
 
 	if (local->fif_probe_req || local->probe_req_reg)
@@ -148,6 +149,7 @@ int ieee80211_hw_config(struct ieee80211_local *local, u32 changed)
 	}
 
 	if (test_bit(SCAN_SW_SCANNING, &local->scanning) ||
+	    test_bit(SCAN_ONCHANNEL_SCANNING, &local->scanning) ||
 	    test_bit(SCAN_HW_SCANNING, &local->scanning))
 		power = chan->max_power;
 	else
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 54a0491..dd2fbec 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -425,6 +425,7 @@ ieee80211_rx_h_passive_scan(struct ieee80211_rx_data *rx)
 
 	if (test_bit(SCAN_HW_SCANNING, &local->scanning) ||
 	    test_bit(SCAN_SW_SCANNING, &local->scanning) ||
+	    test_bit(SCAN_ONCHANNEL_SCANNING, &local->scanning) ||
 	    local->sched_scanning)
 		return ieee80211_scan_rx(rx->sdata, skb);
 
@@ -2915,6 +2916,7 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
 		local->dot11ReceivedFragmentCount++;
 
 	if (unlikely(test_bit(SCAN_HW_SCANNING, &local->scanning) ||
+		     test_bit(SCAN_ONCHANNEL_SCANNING, &local->scanning) ||
 		     test_bit(SCAN_SW_SCANNING, &local->scanning)))
 		status->rx_flags |= IEEE80211_RX_IN_SCAN;
 
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index c70e176..5c7a332 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -387,6 +387,29 @@ static int ieee80211_start_sw_scan(struct ieee80211_local *local)
 	return 0;
 }
 
+static void ieee80211_scan_state_send_probe(struct ieee80211_local *local,
+					    unsigned long *next_delay)
+{
+	int i;
+	struct ieee80211_sub_if_data *sdata = local->scan_sdata;
+	enum ieee80211_band band = local->hw.conf.channel->band;
+
+	for (i = 0; i < local->scan_req->n_ssids; i++)
+		ieee80211_send_probe_req(
+			sdata, NULL,
+			local->scan_req->ssids[i].ssid,
+			local->scan_req->ssids[i].ssid_len,
+			local->scan_req->ie, local->scan_req->ie_len,
+			local->scan_req->rates[band], false,
+			local->scan_req->no_cck);
+
+	/*
+	 * After sending probe requests, wait for probe responses
+	 * on the channel.
+	 */
+	*next_delay = IEEE80211_CHANNEL_TIME;
+	local->next_scan_state = SCAN_DECISION;
+}
 
 static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
 				  struct cfg80211_scan_request *req)
@@ -438,10 +461,43 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
 	local->scan_req = req;
 	local->scan_sdata = sdata;
 
-	if (local->ops->hw_scan)
+	if (local->ops->hw_scan) {
 		__set_bit(SCAN_HW_SCANNING, &local->scanning);
-	else
-		__set_bit(SCAN_SW_SCANNING, &local->scanning);
+	} else {
+		/* If we are scanning only on the current channel, then
+		 * we do not need to stop normal activities
+		 */
+		if ((req->n_channels == 1) &&
+		    (req->channels[0]->center_freq ==
+		     local->hw.conf.channel->center_freq)) {
+			unsigned long next_delay;
+			__set_bit(SCAN_ONCHANNEL_SCANNING, &local->scanning);
+			ieee80211_recalc_idle(local);
+			/* accept probe-responses */
+			ieee80211_configure_filter(local);
+			/* We need to set power level to max for scanning. */
+			ieee80211_hw_config(local, 0);
+
+			if ((req->channels[0]->flags &
+			     IEEE80211_CHAN_PASSIVE_SCAN) ||
+			    !local->scan_req->n_ssids) {
+				next_delay = IEEE80211_PASSIVE_CHANNEL_TIME;
+			} else {
+				ieee80211_scan_state_send_probe(local,
+								&next_delay);
+				next_delay = IEEE80211_CHANNEL_TIME;
+			}
+
+			/* Now, just wait a bit and we are all done! */
+			ieee80211_queue_delayed_work(&local->hw,
+						     &local->scan_work,
+						     next_delay);
+			return 0;
+		}
+		else {
+			__set_bit(SCAN_SW_SCANNING, &local->scanning);
+		}
+	}
 
 	ieee80211_recalc_idle(local);
 
@@ -598,30 +654,6 @@ static void ieee80211_scan_state_set_channel(struct ieee80211_local *local,
 	local->next_scan_state = SCAN_SEND_PROBE;
 }
 
-static void ieee80211_scan_state_send_probe(struct ieee80211_local *local,
-					    unsigned long *next_delay)
-{
-	int i;
-	struct ieee80211_sub_if_data *sdata = local->scan_sdata;
-	enum ieee80211_band band = local->hw.conf.channel->band;
-
-	for (i = 0; i < local->scan_req->n_ssids; i++)
-		ieee80211_send_probe_req(
-			sdata, NULL,
-			local->scan_req->ssids[i].ssid,
-			local->scan_req->ssids[i].ssid_len,
-			local->scan_req->ie, local->scan_req->ie_len,
-			local->scan_req->rates[band], false,
-			local->scan_req->no_cck);
-
-	/*
-	 * After sending probe requests, wait for probe responses
-	 * on the channel.
-	 */
-	*next_delay = IEEE80211_CHANNEL_TIME;
-	local->next_scan_state = SCAN_DECISION;
-}

^ permalink raw reply related

* [PATCH 09/10] ath9k: Add more recv stats.
From: greearb-my8/4N5VtI7c+919tysfdA @ 2012-04-11 17:52 UTC (permalink / raw)
  To: linux-wireless-u79uwXL29TY76Z2rM5mHXA
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Ben Greear
In-Reply-To: <1334166738-28243-1-git-send-email-greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>

From: Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>

This adds counters in various places that can drop packets on
rx without otherwise incrementing a counter.  It also counts
some non-error cases, such as becons and fragments received.

Should help with figuring out where packets are (and are not)
dropped.

Signed-off-by: Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
---
:100644 100644 186efc5... 189ac7f... M	drivers/net/wireless/ath/ath9k/debug.c
:100644 100644 fe2b487... 17f6cc2... M	drivers/net/wireless/ath/ath9k/debug.h
:100644 100644 8588017... 301ef3e... M	drivers/net/wireless/ath/ath9k/recv.c
 drivers/net/wireless/ath/ath9k/debug.c |   53 ++++++++++++++------------------
 drivers/net/wireless/ath/ath9k/debug.h |   18 +++++++++++
 drivers/net/wireless/ath/ath9k/recv.c  |   35 +++++++++++++++-----
 3 files changed, 67 insertions(+), 39 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 186efc5..189ac7f 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -881,36 +881,27 @@ static ssize_t read_file_recv(struct file *file, char __user *user_buf,
 	len += snprintf(buf + len, size - len, "%22s : %10u\n", s, \
 			sc->debug.stats.rxstats.phy_err_stats[p]);
 
+#define RXS_ERR(s, e)				\
+	len += snprintf(buf + len, size - len,	\
+			"%22s : %10u\n", s, \
+			sc->debug.stats.rxstats.e);
+
 	struct ath_softc *sc = file->private_data;
 	char *buf;
-	unsigned int len = 0, size = 1600;
+	unsigned int len = 0, size = 3000;
 	ssize_t retval = 0;
 
 	buf = kzalloc(size, GFP_KERNEL);
 	if (buf == NULL)
 		return -ENOMEM;
 
-	len += snprintf(buf + len, size - len,
-			"%22s : %10u\n", "CRC ERR",
-			sc->debug.stats.rxstats.crc_err);
-	len += snprintf(buf + len, size - len,
-			"%22s : %10u\n", "DECRYPT CRC ERR",
-			sc->debug.stats.rxstats.decrypt_crc_err);
-	len += snprintf(buf + len, size - len,
-			"%22s : %10u\n", "PHY ERR",
-			sc->debug.stats.rxstats.phy_err);
-	len += snprintf(buf + len, size - len,
-			"%22s : %10u\n", "MIC ERR",
-			sc->debug.stats.rxstats.mic_err);
-	len += snprintf(buf + len, size - len,
-			"%22s : %10u\n", "PRE-DELIM CRC ERR",
-			sc->debug.stats.rxstats.pre_delim_crc_err);
-	len += snprintf(buf + len, size - len,
-			"%22s : %10u\n", "POST-DELIM CRC ERR",
-			sc->debug.stats.rxstats.post_delim_crc_err);
-	len += snprintf(buf + len, size - len,
-			"%22s : %10u\n", "DECRYPT BUSY ERR",
-			sc->debug.stats.rxstats.decrypt_busy_err);
+	RXS_ERR("CRC ERR", crc_err);
+	RXS_ERR("DECRYPT CRC ERR", decrypt_crc_err);
+	RXS_ERR("PHY ERR", phy_err);
+	RXS_ERR("MIC ERR", mic_err);
+	RXS_ERR("PRE-DELIM CRC ERR", pre_delim_crc_err);
+	RXS_ERR("POST-DELIM CRC ERR", post_delim_crc_err);
+	RXS_ERR("DECRYPT BUSY ERR", decrypt_busy_err);
 
 	PHY_ERR("UNDERRUN ERR", ATH9K_PHYERR_UNDERRUN);
 	PHY_ERR("TIMING ERR", ATH9K_PHYERR_TIMING);
@@ -939,12 +930,16 @@ static ssize_t read_file_recv(struct file *file, char __user *user_buf,
 	PHY_ERR("HT-LENGTH ERR", ATH9K_PHYERR_HT_LENGTH_ILLEGAL);
 	PHY_ERR("HT-RATE ERR", ATH9K_PHYERR_HT_RATE_ILLEGAL);
 
-	len += snprintf(buf + len, size - len,
-			"%22s : %10u\n", "RX-Pkts-All",
-			sc->debug.stats.rxstats.rx_pkts_all);
-	len += snprintf(buf + len, size - len,
-			"%22s : %10u\n", "RX-Bytes-All",
-			sc->debug.stats.rxstats.rx_bytes_all);
+	RXS_ERR("DECRYPT BUSY ERR", decrypt_busy_err);
+	RXS_ERR("RX-LENGTH-ERR", rx_len_err);
+	RXS_ERR("RX-OOM-ERR", rx_oom_err);
+	RXS_ERR("RX-RATE-ERR", rx_rate_err);
+	RXS_ERR("RX-DROP-RXFLUSH", rx_drop_rxflush);
+	RXS_ERR("RX-TOO-MANY-FRAGS", rx_too_many_frags_err);
+	RXS_ERR("RX-Pkts-All", rx_pkts_all);
+	RXS_ERR("RX-Bytes-All", rx_bytes_all);
+	RXS_ERR("RX-Beacons", rx_beacons);
+	RXS_ERR("RX-Frags", rx_frags);
 
 	if (len > size)
 		len = size;
@@ -959,7 +954,6 @@ static ssize_t read_file_recv(struct file *file, char __user *user_buf,
 
 void ath_debug_stat_rx(struct ath_softc *sc, struct ath_rx_status *rs)
 {
-#define RX_STAT_INC(c) sc->debug.stats.rxstats.c++
 #define RX_PHY_ERR_INC(c) sc->debug.stats.rxstats.phy_err_stats[c]++
 #define RX_SAMP_DBG(c) (sc->debug.bb_mac_samp[sc->debug.sampidx].rs\
 			[sc->debug.rsidx].c)
@@ -1005,7 +999,6 @@ void ath_debug_stat_rx(struct ath_softc *sc, struct ath_rx_status *rs)
 
 #endif
 
-#undef RX_STAT_INC
 #undef RX_PHY_ERR_INC
 #undef RX_SAMP_DBG
 }
diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index fe2b487..17f6cc2 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -139,6 +139,8 @@ struct ath_tx_stats {
 	u32 txfailed;
 };
 
+#define RX_STAT_INC(c) (sc->debug.stats.rxstats.c++)
+
 /**
  * struct ath_rx_stats - RX Statistics
  * @rx_pkts_all:  No. of total frames received, including ones that
@@ -155,6 +157,13 @@ struct ath_tx_stats {
  * @post_delim_crc_err: Post-Frame delimiter CRC error detections
  * @decrypt_busy_err: Decryption interruptions counter
  * @phy_err_stats: Individual PHY error statistics
+ * @rx_len_err:  No. of frames discarded due to bad length.
+ * @rx_oom_err:  No. of frames dropped due to OOM issues.
+ * @rx_rate_err:  No. of frames dropped due to rate errors.
+ * @rx_too_many_frags_err:  Frames dropped due to too-many-frags received.
+ * @rx_drop_rxflush: No. of frames dropped due to RX-FLUSH.
+ * @rx_beacons:  No. of beacons received.
+ * @rx_frags:  No. of rx-fragements received.
  */
 struct ath_rx_stats {
 	u32 rx_pkts_all;
@@ -167,6 +176,13 @@ struct ath_rx_stats {
 	u32 post_delim_crc_err;
 	u32 decrypt_busy_err;
 	u32 phy_err_stats[ATH9K_PHYERR_MAX];
+	u32 rx_len_err;
+	u32 rx_oom_err;
+	u32 rx_rate_err;
+	u32 rx_too_many_frags_err;
+	u32 rx_drop_rxflush;
+	u32 rx_beacons;
+	u32 rx_frags;
 };
 
 enum ath_reset_type {
@@ -250,6 +266,8 @@ void ath_debug_stat_rx(struct ath_softc *sc, struct ath_rx_status *rs);
 
 #else
 
+#define RX_STAT_INC(c) /* NOP */
+
 static inline int ath9k_init_debug(struct ath_hw *ah)
 {
 	return 0;
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 8588017..301ef3e 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -824,15 +824,20 @@ static bool ath9k_rx_accept(struct ath_common *common,
 	if (rx_stats->rs_keyix == ATH9K_RXKEYIX_INVALID)
 		rx_stats->rs_status &= ~ATH9K_RXERR_KEYMISS;
 
-	if (!rx_stats->rs_datalen)
+	if (!rx_stats->rs_datalen) {
+		RX_STAT_INC(rx_len_err);
 		return false;
+	}
+
         /*
          * rs_status follows rs_datalen so if rs_datalen is too large
          * we can take a hint that hardware corrupted it, so ignore
          * those frames.
          */
-	if (rx_stats->rs_datalen > (common->rx_bufsize - rx_status_len))
+	if (rx_stats->rs_datalen > (common->rx_bufsize - rx_status_len)) {
+		RX_STAT_INC(rx_len_err);
 		return false;
+	}
 
 	/* Only use error bits from the last fragment */
 	if (rx_stats->rs_more)
@@ -902,6 +907,7 @@ static int ath9k_process_rate(struct ath_common *common,
 	struct ieee80211_supported_band *sband;
 	enum ieee80211_band band;
 	unsigned int i = 0;
+	struct ath_softc *sc = (struct ath_softc *) common->priv;
 
 	band = hw->conf.channel->band;
 	sband = hw->wiphy->bands[band];
@@ -936,7 +942,7 @@ static int ath9k_process_rate(struct ath_common *common,
 	ath_dbg(common, ANY,
 		"unsupported hw bitrate detected 0x%02x using 1 Mbit\n",
 		rx_stats->rs_rate);
-
+	RX_STAT_INC(rx_rate_err);
 	return -EINVAL;
 }
 
@@ -1823,10 +1829,14 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
 
 		hdr = (struct ieee80211_hdr *) (hdr_skb->data + rx_status_len);
 		rxs = IEEE80211_SKB_RXCB(hdr_skb);
-		if (ieee80211_is_beacon(hdr->frame_control) &&
-		    !is_zero_ether_addr(common->curbssid) &&
-		    !compare_ether_addr(hdr->addr3, common->curbssid))
-			rs.is_mybeacon = true;
+		if (ieee80211_is_beacon(hdr->frame_control)) {
+			RX_STAT_INC(rx_beacons);
+			if (!is_zero_ether_addr(common->curbssid) &&
+			    !compare_ether_addr(hdr->addr3, common->curbssid))
+				rs.is_mybeacon = true;
+			else
+				rs.is_mybeacon = false;
+		}
 		else
 			rs.is_mybeacon = false;
 
@@ -1836,8 +1846,10 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
 		 * If we're asked to flush receive queue, directly
 		 * chain it back at the queue without processing it.
 		 */
-		if (sc->sc_flags & SC_OP_RXFLUSH)
+		if (sc->sc_flags & SC_OP_RXFLUSH) {
+			RX_STAT_INC(rx_drop_rxflush);
 			goto requeue_drop_frag;
+		}
 
 		memset(rxs, 0, sizeof(struct ieee80211_rx_status));
 
@@ -1867,8 +1879,10 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
 		 * tell hardware it can give us a new frame using the old
 		 * skb and put it at the tail of the sc->rx.rxbuf list for
 		 * processing. */
-		if (!requeue_skb)
+		if (!requeue_skb) {
+			RX_STAT_INC(rx_oom_err);
 			goto requeue_drop_frag;
+		}
 
 		/* Unmap the frame */
 		dma_unmap_single(sc->dev, bf->bf_buf_addr,
@@ -1899,6 +1913,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
 		}
 
 		if (rs.rs_more) {
+			RX_STAT_INC(rx_frags);
 			/*
 			 * rs_more indicates chained descriptors which can be
 			 * used to link buffers together for a sort of
@@ -1908,6 +1923,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
 				/* too many fragments - cannot handle frame */
 				dev_kfree_skb_any(sc->rx.frag);
 				dev_kfree_skb_any(skb);
+				RX_STAT_INC(rx_too_many_frags_err);
 				skb = NULL;
 			}
 			sc->rx.frag = skb;
@@ -1919,6 +1935,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
 
 			if (pskb_expand_head(hdr_skb, 0, space, GFP_ATOMIC) < 0) {
 				dev_kfree_skb(skb);
+				RX_STAT_INC(rx_oom_err);
 				goto requeue_drop_frag;
 			}
 
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 00/10] Wireless patches for wireless-testing
From: greearb @ 2012-04-11 17:52 UTC (permalink / raw)
  To: linux-wireless; +Cc: netdev, Ben Greear

From: Ben Greear <greearb@candelatech.com>

Support Ethtool stats for Wireless, including in-depth
stats for ath9k, and add some additional ath9k stats
to debugfs.  Support scanning on-channel.

The basic ethtool patches are little changed from previous
posted patches.  The others are tweaked at least some, and
some have not been posted before at all.

Ben Greear (10):
  cfg80211: Add framework to support ethtool stats.
  mac80211: Support getting sta_info stats via ethtool.
  mac80211: Framework to get wifi-driver stats via ethtool.
  ath9k: Support ethtool getstats api.
  mac80211: Add more ethtools stats: survey, rates, etc
  mac80211: Add sta_state to ethtool stats.
  ath9k: Add tx-failed counter.
  mac80211: Support on-channel scan option.
  ath9k: Add more recv stats.
  ath9k: Gather and report IRQ sync_cause errors.

 drivers/net/wireless/ath/ath9k/ar9002_mac.c |    1 +
 drivers/net/wireless/ath/ath9k/ar9003_mac.c |    2 +
 drivers/net/wireless/ath/ath9k/debug.c      |  167 ++++++++++++++-------------
 drivers/net/wireless/ath/ath9k/debug.h      |   43 +++++++
 drivers/net/wireless/ath/ath9k/hw.c         |   49 ++++++++
 drivers/net/wireless/ath/ath9k/hw.h         |    6 +
 drivers/net/wireless/ath/ath9k/main.c       |  135 +++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/recv.c       |   35 ++++--
 include/net/cfg80211.h                      |   17 +++
 include/net/mac80211.h                      |   17 +++
 net/mac80211/cfg.c                          |  160 +++++++++++++++++++++++++
 net/mac80211/driver-ops.h                   |   37 ++++++
 net/mac80211/driver-trace.h                 |   15 +++
 net/mac80211/ieee80211_i.h                  |    3 +
 net/mac80211/main.c                         |    4 +-
 net/mac80211/rx.c                           |    2 +
 net/mac80211/scan.c                         |   92 +++++++++++-----
 net/wireless/ethtool.c                      |   29 +++++
 18 files changed, 695 insertions(+), 119 deletions(-)

-- 
1.7.3.4

^ permalink raw reply

* [PATCH 01/10] cfg80211: Add framework to support ethtool stats.
From: greearb @ 2012-04-11 17:52 UTC (permalink / raw)
  To: linux-wireless; +Cc: netdev, Ben Greear
In-Reply-To: <1334166738-28243-1-git-send-email-greearb@candelatech.com>

From: Ben Greear <greearb@candelatech.com>

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 ae3a3bb... 8bf68f3... M	include/net/cfg80211.h
:100644 100644 9bde4d1... 7eecdf4... M	net/wireless/ethtool.c
 include/net/cfg80211.h |   17 +++++++++++++++++
 net/wireless/ethtool.c |   29 +++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index ae3a3bb..8bf68f3 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1511,6 +1511,16 @@ struct cfg80211_gtk_rekey_data {
  *	later passes to cfg80211_probe_status().
  *
  * @set_noack_map: Set the NoAck Map for the TIDs.
+ *
+ * @get_et_sset_count:  Ethtool API to get string-set count.
+ *	See @ethtool_ops.get_sset_count
+ *
+ * @get_et_stats:  Ethtool API to get a set of u64 stats.
+ *	See @ethtool_ops.get_ethtool_stats
+ *
+ * @get_et_strings:  Ethtool API to get a set of strings to describe stats
+ *	and perhaps other supported types of ethtool data-sets.
+ *	See @ethtool_ops.get_strings
  */
 struct cfg80211_ops {
 	int	(*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -1707,6 +1717,13 @@ struct cfg80211_ops {
 				  u16 noack_map);
 
 	struct ieee80211_channel *(*get_channel)(struct wiphy *wiphy);
+
+	int	(*get_et_sset_count)(struct wiphy *wiphy,
+				     struct net_device *dev, int sset);
+	void	(*get_et_stats)(struct wiphy *wiphy, struct net_device *dev,
+				struct ethtool_stats *stats, u64 *data);
+	void	(*get_et_strings)(struct wiphy *wiphy, struct net_device *dev,
+				  u32 sset, u8 *data);
 };
 
 /*
diff --git a/net/wireless/ethtool.c b/net/wireless/ethtool.c
index 9bde4d1..7eecdf4 100644
--- a/net/wireless/ethtool.c
+++ b/net/wireless/ethtool.c
@@ -68,6 +68,32 @@ static int cfg80211_set_ringparam(struct net_device *dev,
 	return -ENOTSUPP;
 }
 
+static int cfg80211_get_sset_count(struct net_device *dev, int sset)
+{
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	struct cfg80211_registered_device *rdev = wiphy_to_dev(wdev->wiphy);
+	if (rdev->ops->get_et_sset_count)
+		return rdev->ops->get_et_sset_count(wdev->wiphy, dev, sset);
+	return -EOPNOTSUPP;
+}
+
+static void cfg80211_get_stats(struct net_device *dev,
+			       struct ethtool_stats *stats, u64 *data)
+{
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	struct cfg80211_registered_device *rdev = wiphy_to_dev(wdev->wiphy);
+	if (rdev->ops->get_et_stats)
+		rdev->ops->get_et_stats(wdev->wiphy, dev, stats, data);
+}
+
+static void cfg80211_get_strings(struct net_device *dev, u32 sset, u8 *data)
+{
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	struct cfg80211_registered_device *rdev = wiphy_to_dev(wdev->wiphy);
+	if (rdev->ops->get_et_strings)
+		rdev->ops->get_et_strings(wdev->wiphy, dev, sset, data);
+}
+
 const struct ethtool_ops cfg80211_ethtool_ops = {
 	.get_drvinfo = cfg80211_get_drvinfo,
 	.get_regs_len = cfg80211_get_regs_len,
@@ -75,4 +101,7 @@ const struct ethtool_ops cfg80211_ethtool_ops = {
 	.get_link = ethtool_op_get_link,
 	.get_ringparam = cfg80211_get_ringparam,
 	.set_ringparam = cfg80211_set_ringparam,
+	.get_strings = cfg80211_get_strings,
+	.get_ethtool_stats = cfg80211_get_stats,
+	.get_sset_count = cfg80211_get_sset_count,
 };
-- 
1.7.3.4

^ permalink raw reply related

* [PATCH 02/10] mac80211: Support getting sta_info stats via ethtool.
From: greearb @ 2012-04-11 17:52 UTC (permalink / raw)
  To: linux-wireless; +Cc: netdev, Ben Greear
In-Reply-To: <1334166738-28243-1-git-send-email-greearb@candelatech.com>

From: Ben Greear <greearb@candelatech.com>

This lets ethtool print out stats related to stations
connected to the interface.  Does not yet get stats
from the underlying driver.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 83e08dc... d008666... M	net/mac80211/cfg.c
 net/mac80211/cfg.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 83e08dc..d008666 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -112,6 +112,74 @@ static int ieee80211_set_noack_map(struct wiphy *wiphy,
 	return 0;
 }
 
+static const char ieee80211_gstrings_sta_stats[][ETH_GSTRING_LEN] = {
+	"rx_packets", "rx_bytes", "wep_weak_iv_count",
+	"rx_duplicates", "rx_fragments", "rx_dropped",
+	"tx_packets", "tx_bytes", "tx_fragments",
+	"tx_filtered", "tx_retry_failed", "tx_retries",
+	"beacon_loss"
+};
+#define STA_STATS_LEN	ARRAY_SIZE(ieee80211_gstrings_sta_stats)
+
+static int ieee80211_get_et_sset_count(struct wiphy *wiphy,
+				       struct net_device *dev,
+				       int sset)
+{
+	if (sset == ETH_SS_STATS)
+		return STA_STATS_LEN;
+
+	return -EOPNOTSUPP;
+}
+
+static void ieee80211_get_et_stats(struct wiphy *wiphy,
+				   struct net_device *dev,
+				   struct ethtool_stats *stats,
+				   u64 *data)
+{
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	struct sta_info *sta;
+	struct ieee80211_local *local = sdata->local;
+
+	memset(data, 0, sizeof(u64) * STA_STATS_LEN);
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(sta, &local->sta_list, list) {
+		int i = 0;
+
+		/* Make sure this station belongs to the proper dev */
+		if (sta->sdata->dev != dev)
+			continue;
+
+		data[i++] += sta->rx_packets;
+		data[i++] += sta->rx_bytes;
+		data[i++] += sta->wep_weak_iv_count;
+		data[i++] += sta->num_duplicates;
+		data[i++] += sta->rx_fragments;
+		data[i++] += sta->rx_dropped;
+
+		data[i++] += sta->tx_packets;
+		data[i++] += sta->tx_bytes;
+		data[i++] += sta->tx_fragments;
+		data[i++] += sta->tx_filtered_count;
+		data[i++] += sta->tx_retry_failed;
+		data[i++] += sta->tx_retry_count;
+		data[i++] += sta->beacon_loss_count;
+		BUG_ON(i != STA_STATS_LEN);
+	}
+	rcu_read_unlock();
+}
+
+static void ieee80211_get_et_strings(struct wiphy *wiphy,
+				     struct net_device *dev,
+				     u32 sset, u8 *data)
+{
+	if (sset == ETH_SS_STATS) {
+		int sz_sta_stats = sizeof(ieee80211_gstrings_sta_stats);
+		memcpy(data, *ieee80211_gstrings_sta_stats, sz_sta_stats);
+	}
+}
+
+
 static int ieee80211_add_key(struct wiphy *wiphy, struct net_device *dev,
 			     u8 key_idx, bool pairwise, const u8 *mac_addr,
 			     struct key_params *params)
@@ -2749,4 +2817,7 @@ struct cfg80211_ops mac80211_config_ops = {
 	.probe_client = ieee80211_probe_client,
 	.get_channel = ieee80211_wiphy_get_channel,
 	.set_noack_map = ieee80211_set_noack_map,
+	.get_et_sset_count = ieee80211_get_et_sset_count,
+	.get_et_stats = ieee80211_get_et_stats,
+	.get_et_strings = ieee80211_get_et_strings,
 };
-- 
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