Netdev List
 help / color / mirror / Atom feed
* [PATCH 0/5] Export the sock's security context to proc
From: rongqing.li @ 2011-08-05  8:58 UTC (permalink / raw)
  To: netdev, selinux

-------
    Any review would be much appreciated.
 
Comments:
--------
    Export the sock's security context to proc.
    
    The element sk_security of struct sock represents the socket
    security context ID, which is inheriting from the process when
    creates this socket on most of the time.
    
    but when SELinux type_transition rule is applied to socket, or
    application sets /proc/xxx/attr/createsock, the socket security
    context would be different from the creating process. on this
    condition, the "netstat -Z" will return wrong value, since
    "netstat -Z" only returns the process security context as socket
    process security.
    
    Export the raw sock's security context to proc, so that "netstat -Z"
    could be fixed by reading procfs.

Test:
--------
1. When Enable SELinux.


1.1 check the socket security context has been exported in procfs

root@qemu-host:/root> head -n 3 /proc/net/tcp 
  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt uid  timeout inode   scontext                                          
   0: 00000000:05FE 00000000:0000 0A 00000000:00000000 00:00000000 00000000 0        0 5029 1 ffff88001b8ecc00 100 0 0 10 -1 system_u:system_r:initrc_t:s0-s15:c0.c1023                                                                
   1: 00000000:DBE2 00000000:0000 0A 00000000:00000000 00:00000000 00000000 0        0 4915 1 ffff88001b8ec600 100 0 0 10 -1 system_u:system_r:rpcd_t:s0-s15:c0.c1023                                                              


root@qemu-host:/root> head -n 3 /proc/net/udp 
  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt uid  timeout inode ref pointer drops  scontext                          
   53: 00000000:89F1 00000000:0000 07 00000000:00000000 00:00000000 00000000 0        0 4912 2 ffff88001e3b49c0 0 system_u:system_r:rpcd_t:s0-s15:c0.c1023        
  172: 00000000:0268 00000000:0000 07 00000000:00000000 00:00000000 00000000 0        0 4851 2 ffff88001e3b4340 0 system_u:system_r:rpcbind_t:s0-s15:c0.c1023           


root@qemu-host:/root> head -n 3 /proc/net/unix 
Num       RefCount Protocol Flags    Type St Inode Path      scontext
ffff88001ea1cc00: 00000002 00000000 00000000 0002 01   976 @/org/kernel/udev/udevd               system_u:system_r:udev_t:s0-s15:c0.c1023
ffff88001bbe6600: 0000000A 00000000 00000000 0002 01  4740 /dev/log                              system_u:system_r:syslogd_s_t:s15:c0.c1023
root@qemu-host:/root> 


root@qemu-host:/root> head -n 3 /proc/net/raw  
  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt uid  timeout inode ref pointer drops   scontext
root@qemu-host:/root> 

1.2 check these patches do not affect the netstat, it can still work
root@qemu-host:/root> netstat -a
Active Internet connections (servers and established) Proto Recv-Q Send-Q Local Address               Foreign Address State      
tcp        0      0 *:1534                      *:* LISTEN      
tcp        0      0 *:56290                     *:* LISTEN      
tcp        0      0 localhost:submission        *:* LISTEN      
tcp        0      0 *:sunrpc                    *:* LISTEN
...

1.3 When syslog creates socket, and type transition has been applied on them, the security context of
socket would be syslogd_s_t, not same as its own process security context
syslogd_t, the "netstat -Z" returns wrong value, but the security context in procfs is correct

root@qemu-host:/etc> cat /proc/net/unix |grep syslog
ffff88001f856000: 00000002 00000000 00010000 0001 01  6385 /var/lib/syslog-ng/syslog-ng.ctl      system_u:system_r:syslogd_t:s15:c0.c1023
ffff88001f856300: 00000002 00000000 00000000 0002 01  6383 /dev/log                              system_u:system_r:syslogd_s_t:s15:c0.c1023
root@qemu-host:/etc> 

root@qemu-host:/etc> netstat -aZ|grep 6383
unix  2      [ ]         DGRAM                    6383   793/syslog-ng
system_u:system_r:syslogd_t:s15:c0.c1023          /dev/log
root@qemu-host:/etc> 



2. When SElinux is disabled

2.1 check the /proc/net/udp information are same as no these patches

root@qemu-host:/root> head -n 3 /proc/net/raw  
  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt uid  timeout inode ref pointer drops 

root@qemu-host:/root> head -n 3 /proc/net/unix 
Num       RefCount Protocol Flags    Type St Inode Path    
ffff88001d226000: 0000000A 00000000 00000000 0002 01  2661 /dev/log                              
ffff88001ea1cc00: 00000002 00000000 00000000 0002 01   897 @/org/kernel/udev/udevd               

root@qemu-host:/root> head -n 3 /proc/net/tcp  
  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt uid  timeout inode                                                     
   0: 00000000:05FE 00000000:0000 0A 00000000:00000000 00:00000000 00000000 0        0 2950 1 ffff88001d294c00 100 0 0 10 -1                     
   1: 0100007F:024B 00000000:0000 0A 00000000:00000000 00:00000000 00000000 0        0 3217 1 ffff88001d295e00 100 0 0 10 -1                     

root@qemu-host:/root> head -n 3 /proc/net/udp 
  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt uid  timeout inode ref pointer drops                                    
   57: 00000000:03F5 00000000:0000 07 00000000:00000000 00:00000000 00000000 0        0 2772 2 ffff88001d2ac340 0                                 
  122: 00000000:D936 00000000:0000 07 00000000:00000000 00:00000000 00000000 0        0 2831 2 ffff88001d2acd00 0                                 
root@qemu-host:/root>



^ permalink raw reply

* Re: [RFC 2/4] [flexcan] Introduce a flexcan_clk set of functions.
From: Marc Kleine-Budde @ 2011-08-05  8:34 UTC (permalink / raw)
  To: Robin Holt
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, Wolfgang Grandegger
In-Reply-To: <1312509979-13226-3-git-send-email-holt-sJ/iWh9BUns@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 6442 bytes --]

On 08/05/2011 04:06 AM, Robin Holt wrote:
> The freescale P1010RDB board does not have a
> clk_{get,put,get_rate,enable,disable} set of functions.  Wrap these with a
> flexcan_ #define for arm, and implement a more complete function for ppc.

Some codingstyle nitpicks inline. I hope we'll find a cleaner solution
than this patch.

Marc

> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  drivers/net/can/flexcan.c |  114 +++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 105 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 74b1706..3417d0b 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -220,6 +220,102 @@ static inline void flexcan_write(u32 val, void __iomem *addr)
>  }
>  #endif
>  
> +#if defined(__powerpc__)
> +struct flexcan_clk {
> +	unsigned long rate;
> +	void *data;
> +};
> +
> +static struct clk *flexcan_clk_get(struct device *dev, const char *id)
> +{
> +	struct flexcan_clk *clock;
> +	u32 *clock_freq;
> +	u32 *clock_divider;
> +	int err;
> +
> +	clock = kmalloc(sizeof(struct flexcan_clk), GFP_KERNEL);
> +	if (!clock) {
> +		dev_err(dev, "Cannot allocate memory\n");
> +		err = -ENOMEM;
> +		goto failed_clock;
> +	}
> +	clock_freq = (u32 *)of_get_property(dev->of_node, "clock_freq", NULL);
> +	if (!clock_freq) {
> +		dev_err(dev, "Cannot find clock_freq property\n");
> +		err = -EINVAL;
> +		goto failed_clock;
> +	}
> +
> +	clock_divider = (u32 *) of_get_property(dev->of_node,
                               ^

remove this space, please
> +					"fsl,flexcan-clock-divider", NULL);
> +	if (clock_divider == NULL) {

!clock_divider

> +		dev_err(dev, "Cannot find fsl,flexcan-clock-divider property\n");
> +		err = -EINVAL;
> +		goto failed_clock;
> +	}
> +
> +	clock->rate = DIV_ROUND_CLOSEST(*clock_freq / *clock_divider, 1000);
> +	clock->rate *= 1000;
> +
> +	return (struct clk *)clock;
> +
> + failed_clock:
> +	kfree(clock);
> +	return ERR_PTR(err);
> +}
> +
> +static inline void flexcan_clk_put(struct clk *_clk)
> +{
> +	struct flexcan_clk *clk = (struct flexcan_clk *)_clk;

that cast is not needed.

> +
> +	kfree(clk);
> +}
> +
> +static inline int flexcan_clk_enable(struct clk *clk)
> +{
> +	return 0;
> +}
> +
> +static inline void flexcan_clk_disable(struct clk *clk)
> +{
> +	return;
> +}
> +
> +static inline unsigned long flexcan_clk_get_rate(struct clk *_clk)
> +{
> +	struct flexcan_clk *clk = (struct flexcan_clk *)_clk;
> +
> +	return clk->rate;
> +}
> +
> +#else
> +static inline struct clk *flexcan_clk_get(struct device *dev, const char *id)
> +{
> +	return clk_get(dev, id);
> +}
> +
> +static inline void flexcan_clk_put(struct clk *clk)
> +{
> +	clk_put(clk);
> +}
> +
> +static inline int flexcan_clk_enable(struct clk *clk)
> +{
> +	return clk_enable(clk);
> +}
> +
> +static inline void flexcan_clk_disable(struct clk *clk)
> +{
> +	clk_disable(clk);
> +}
> +
> +static inline unsigned long flexcan_clk_get_rate(struct clk *clk)
> +{
> +	return clk_get_rate(clk);
> +}
> +
> +#endif
> +
>  /*
>   * Swtich transceiver on or off
>   */
> @@ -807,7 +903,7 @@ static int flexcan_open(struct net_device *dev)
>  	struct flexcan_priv *priv = netdev_priv(dev);
>  	int err;
>  
> -	clk_enable(priv->clk);
> +	flexcan_clk_enable(priv->clk);
>  
>  	err = open_candev(dev);
>  	if (err)
> @@ -829,7 +925,7 @@ static int flexcan_open(struct net_device *dev)
>   out_close:
>  	close_candev(dev);
>   out:
> -	clk_disable(priv->clk);
> +	flexcan_clk_disable(priv->clk);
>  
>  	return err;
>  }
> @@ -843,7 +939,7 @@ static int flexcan_close(struct net_device *dev)
>  	flexcan_chip_stop(dev);
>  
>  	free_irq(dev->irq, dev);
> -	clk_disable(priv->clk);
> +	flexcan_clk_disable(priv->clk);
>  
>  	close_candev(dev);
>  
> @@ -882,7 +978,7 @@ static int __devinit register_flexcandev(struct net_device *dev)
>  	struct flexcan_regs __iomem *regs = priv->base;
>  	u32 reg, err;
>  
> -	clk_enable(priv->clk);
> +	flexcan_clk_enable(priv->clk);
>  
>  	/* select "bus clock", chip must be disabled */
>  	flexcan_chip_disable(priv);
> @@ -916,7 +1012,7 @@ static int __devinit register_flexcandev(struct net_device *dev)
>   out:
>  	/* disable core and turn off clocks */
>  	flexcan_chip_disable(priv);
> -	clk_disable(priv->clk);
> +	flexcan_clk_disable(priv->clk);
>  
>  	return err;
>  }
> @@ -936,7 +1032,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>  	resource_size_t mem_size;
>  	int err, irq;
>  
> -	clk = clk_get(&pdev->dev, NULL);
> +	clk = flexcan_clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(clk)) {
>  		dev_err(&pdev->dev, "no clock defined\n");
>  		err = PTR_ERR(clk);
> @@ -973,7 +1069,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>  	dev->flags |= IFF_ECHO; /* we support local echo in hardware */
>  
>  	priv = netdev_priv(dev);
> -	priv->can.clock.freq = clk_get_rate(clk);
> +	priv->can.clock.freq = flexcan_clk_get_rate(clk);
>  	priv->can.bittiming_const = &flexcan_bittiming_const;
>  	priv->can.do_set_mode = flexcan_set_mode;
>  	priv->can.do_get_berr_counter = flexcan_get_berr_counter;
> @@ -1008,7 +1104,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>   failed_map:
>  	release_mem_region(mem->start, mem_size);
>   failed_get:
> -	clk_put(clk);
> +	flexcan_clk_put(clk);
>   failed_clock:
>  	return err;
>  }
> @@ -1026,7 +1122,7 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	release_mem_region(mem->start, resource_size(mem));
>  
> -	clk_put(priv->clk);
> +	flexcan_clk_put(priv->clk);
>  
>  	free_candev(dev);
>  


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [RFC 1/4] [flexcan] Abstract off read/write for big/little endian.
From: Marc Kleine-Budde @ 2011-08-05  8:32 UTC (permalink / raw)
  To: Robin Holt
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, Wolfgang Grandegger
In-Reply-To: <1312509979-13226-2-git-send-email-holt-sJ/iWh9BUns@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 858 bytes --]

On 08/05/2011 04:06 AM, Robin Holt wrote:
> First step in converting the flexcan driver from supporting just arm to
> supporting both arm and powerpc architectures.
> 
> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Acked-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [PATCH 1/3] net: sendmmsg should only return an error if no messages were sent
From: Steven Whitehouse @ 2011-08-05  8:20 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: acme, rdenis, netdev
In-Reply-To: <201108050357.p753vtpO022773@www262.sakura.ne.jp>

Hi,

On Fri, 2011-08-05 at 12:57 +0900, Tetsuo Handa wrote:
> Anton Blanchard wrote:
> > sendmmsg uses a similar error return strategy as recvmmsg but it
> > turns out to be a confusing way to communicate errors.
> > 
> > The current code stores the error code away and returns it on the next
> > sendmmsg call. This means a call with completely valid arguments could
> > get an error from a previous call.
> > 
> > Change things so we only return an error if no datagrams could be sent.
> > If less than the requested number of messages were sent, the application
> > must retry starting at the first failed one and if the problem is
> > persistent the error will be returned.
> > 
> > This matches the behaviour of other syscalls like read/write - it
> > is not an error if less than the requested number of elements are sent.
> 
> OK. David S. Miller suggested this behavior and Anton Blanchard agreed with
> this behavior.
> 
> Quoting from commit a2e27255 "net: Introduce recvmmsg socket syscall":
> | . R?mi Denis-Courmont & Steven Whitehouse: If we receive N < vlen
> |   datagrams and then recvmsg returns an error, recvmmsg will return
> |   the successfully received datagrams, store the error and return it
> |   in the next call.
> 
> R?mi Denis-Courmont, Steven Whitehouse and Arnaldo Carvalho de Melo, do you
> want to change recvmmsg()'s behaviour as well?

Since I've joined this part way through it seems, I'm assuming that if
something was sent/received then that will be returned and the error
stored until the next call. If nothing was sent/received then the error
can be returned immediately.

That is what I'd expect to be the case, since otherwise it is impossible
to know how much has been successfully sent/received in the partial
failure case, I think. Also it means that sendmmesg/recvmmsg matches
sendmsg/recvmsg in terms of expected return values and thus the
principle of least surprise.

So if thats what is being proposed, then it sounds good to me,

Steve.



^ permalink raw reply

* Re: return of ip_rt_bug()
From: Julian Anastasov @ 2011-08-05  7:56 UTC (permalink / raw)
  To: Tom London; +Cc: Dave Jones, netdev
In-Reply-To: <CAFiZG+UY+Rn=wqi6b9CM0CcARHJUGiqh_jpfEveGLhm+D2hNhA@mail.gmail.com>


	Hello,

On Thu, 4 Aug 2011, Tom London wrote:

> So, my router is just giving my wired NIC different addresses....
> 
> More I can provide?

	Thanks! Can you check in dmesg or in another log
about the first IP address in such line:

printk(KERN_DEBUG "ip_rt_bug: %pI4 -> %pI4, %s\n"

	Is it now 192.168.2.6 ?

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* Re: [RFC 4/4] [flexcan] Add support for FLEXCAN_DEBUG
From: Marc Kleine-Budde @ 2011-08-05  7:16 UTC (permalink / raw)
  To: Robin Holt
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, Wolfgang Grandegger
In-Reply-To: <1312509979-13226-5-git-send-email-holt-sJ/iWh9BUns@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 3634 bytes --]

On 08/05/2011 04:06 AM, Robin Holt wrote:
> Add a wrapper function for a register dump when a developer defines
> FLEXCAN_DEBUG

Comments inline..however I'm not sure if we need this patch.

> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  drivers/net/can/flexcan.c |   40 ++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 40 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index fbb61c6..96684ca 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -316,6 +316,38 @@ static inline unsigned long flexcan_clk_get_rate(struct clk *clk)
>  
>  #endif
>  
> +#if defined(FLEXCAN_DEBUG)
> +void _flexcan_reg_dump(struct net_device *dev, const char *file, int line,
> +		       const char *func)
> +{
> +	const struct flexcan_priv *priv = netdev_priv(dev);
> +	struct flexcan_regs __iomem *regs = priv->base;
> +
> +	printk(KERN_INFO "flexcan_reg_dump:%s:%d:%s()\n", file, line, func);

Use netdev_$LEVEL, please.
If you use dbg, you can remove the ifdef altogether.

> +	printk(KERN_INFO
> +		"\t  mcr 0x%08x  ctrl 0x%08x timer 0x%08x   rxg 0x%08x",
> +		flexcan_read(&regs->mcr),
> +		flexcan_read(&regs->ctrl),
> +		flexcan_read(&regs->timer),
> +		flexcan_read(&regs->rxgmask));
> +	printk(KERN_INFO
> +		"\t rx14 0x%08x  rx15 0x%08x   ecr 0x%08x   esr 0x%08x",
> +		flexcan_read(&regs->rx14mask),
> +		flexcan_read(&regs->rx15mask),
> +		flexcan_read(&regs->ecr),
> +		flexcan_read(&regs->esr));
> +	printk(KERN_INFO
> +		"\timsk2 0x%08x imsk1 0x%08x iflg2 0x%08x iflg1 0x%08x",
> +		flexcan_read(&regs->imask2),
> +		flexcan_read(&regs->imask1),
> +		flexcan_read(&regs->iflag2),
> +		flexcan_read(&regs->iflag1));
> +}
> +#define flexcan_reg_dump(_d) _flexcan_reg_dump(_d, __FILE__, __LINE__, __func__)
> +#else
> +#define flexcan_reg_dump(_d)
> +#endif
> +
>  /*
>   * Swtich transceiver on or off
>   */
> @@ -376,6 +408,8 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	u32 can_id;
>  	u32 ctrl = FLEXCAN_MB_CNT_CODE(0xc) | (cf->can_dlc << 16);
>  
> +	flexcan_reg_dump(dev);
> +
>  	if (can_dropped_invalid_skb(dev, skb))
>  		return NETDEV_TX_OK;
>  
> @@ -408,6 +442,8 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	/* tx_packets is incremented in flexcan_irq */
>  	stats->tx_bytes += cf->can_dlc;
>  
> +	flexcan_reg_dump(dev);
> +
>  	return NETDEV_TX_OK;
>  }
>  
> @@ -676,6 +712,8 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  	struct flexcan_regs __iomem *regs = priv->base;
>  	u32 reg_iflag1, reg_esr;
>  
> +	flexcan_reg_dump(dev);
> +
>  	reg_iflag1 = flexcan_read(&regs->iflag1);
>  	reg_esr = flexcan_read(&regs->esr);
>  	flexcan_write(FLEXCAN_ESR_ERR_INT, &regs->esr);	/* ACK err IRQ */
> @@ -716,6 +754,8 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  		netif_wake_queue(dev);
>  	}
>  
> +	flexcan_reg_dump(dev);
> +
>  	return IRQ_HANDLED;
>  }
>  

cheers, Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [RFC 3/4] [flexcan] Add of_match to platform_device definition.
From: Marc Kleine-Budde @ 2011-08-05  7:13 UTC (permalink / raw)
  To: Robin Holt
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, Wolfgang Grandegger
In-Reply-To: <1312509979-13226-4-git-send-email-holt-sJ/iWh9BUns@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 1747 bytes --]

On 08/05/2011 04:06 AM, Robin Holt wrote:
> It looks like the of_device stuff got moved under the
> platform_device->driver and all we should need to do is define an of_match
> to get a flexcan_probe call out.

Please rephrase this commit message, at it goes into the git tree.
Otherwise the patche looks good.

Marc
> 
> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  drivers/net/can/flexcan.c |   13 ++++++++++++-
>  1 files changed, 12 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 3417d0b..fbb61c6 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -1129,8 +1129,19 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static struct of_device_id flexcan_of_match[] = {
> +	{
> +		.compatible = "fsl,flexcan",
> +	},
> +	{},
> +};
> +
>  static struct platform_driver flexcan_driver = {
> -	.driver.name = DRV_NAME,
> +	.driver = {
> +		.name = DRV_NAME,
> +		.owner = THIS_MODULE,
> +		.of_match_table = flexcan_of_match,
> +	},
>  	.probe = flexcan_probe,
>  	.remove = __devexit_p(flexcan_remove),
>  };


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [PATCH 2/3] net: Cap number of elements for sendmmsg
From: Tetsuo Handa @ 2011-08-05  5:50 UTC (permalink / raw)
  To: anton, acme, davem; +Cc: netdev, linux-security-module, eparis, casey, mjt
In-Reply-To: <20110805144619.729c631a@kryten>

Anton Blanchard wrote:
> > Apart from the upper limit for vlen argument of sendmmsg()/recvmmsg(),
> > we need to deal with stall problem (described below).
> 
> Capping vlen at 1024 should prevent that. Your patch does a signed
> comparison which just reduces the maximum value by 1 bit doesn't it?

Just for avoiding returning IS_ERR_VALUE() value upon success.

> Keep in mind each element could have up to 1024 iovec entries at worst
> case, so I think 1024 is a sane upper max.

OK. Please take Anton's version.

Arnaldo, please consider copying this change to recvmmsg() too.

^ permalink raw reply

* Re: [uclinux-dist-devel] [net-next v2 55/71] bfin_mac: Move the Analog Devices Inc driver
From: Bob Liu @ 2011-08-05  5:18 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, netdev, uclinux-dist-devel, sassmann, gospo
In-Reply-To: <1312082850-24914-56-git-send-email-jeffrey.t.kirsher@intel.com>

On Sun, Jul 31, 2011 at 11:27 AM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> Move the Analog Devices Inc driver into drivers/net/ethernet/adi/ and
> make the necessary Kconfig and Makefile changes.
>
> CC: <uclinux-dist-devel@blackfin.uclinux.org>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Looks good for me, Thanks.

Acked-by: Bob Liu <bob.liu@analog.com>

> ---
>  MAINTAINERS                               |    2 +-
>  drivers/net/Kconfig                       |   46 --------------------
>  drivers/net/Makefile                      |    1 -
>  drivers/net/ethernet/Kconfig              |    1 +
>  drivers/net/ethernet/Makefile             |    1 +
>  drivers/net/ethernet/adi/Kconfig          |   65 +++++++++++++++++++++++++++++
>  drivers/net/ethernet/adi/Makefile         |    5 ++
>  drivers/net/{ => ethernet/adi}/bfin_mac.c |    0
>  drivers/net/{ => ethernet/adi}/bfin_mac.h |    0
>  9 files changed, 73 insertions(+), 48 deletions(-)
>  create mode 100644 drivers/net/ethernet/adi/Kconfig
>  create mode 100644 drivers/net/ethernet/adi/Makefile
>  rename drivers/net/{ => ethernet/adi}/bfin_mac.c (100%)
>  rename drivers/net/{ => ethernet/adi}/bfin_mac.h (100%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a91e8fb..83a51ad 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1440,7 +1440,7 @@ BLACKFIN EMAC DRIVER
>  L:     uclinux-dist-devel@blackfin.uclinux.org
>  W:     http://blackfin.uclinux.org
>  S:     Supported
> -F:     drivers/net/bfin_mac.*
> +F:     drivers/net/ethernet/adi/
>
>  BLACKFIN RTC DRIVER
>  M:     Mike Frysinger <vapier.adi@gmail.com>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 61e71d0..bb4bc4b 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -252,52 +252,6 @@ config SH_ETH
>          This driver supporting CPUs are:
>                - SH7710, SH7712, SH7763, SH7619, SH7724, and SH7757.
>
> -config BFIN_MAC
> -       tristate "Blackfin on-chip MAC support"
> -       depends on NET_ETHERNET && (BF516 || BF518 || BF526 || BF527 || BF536 || BF537)
> -       select CRC32
> -       select MII
> -       select PHYLIB
> -       select BFIN_MAC_USE_L1 if DMA_UNCACHED_NONE
> -       help
> -         This is the driver for Blackfin on-chip mac device. Say Y if you want it
> -         compiled into the kernel. This driver is also available as a module
> -         ( = code which can be inserted in and removed from the running kernel
> -         whenever you want). The module will be called bfin_mac.
> -
> -config BFIN_MAC_USE_L1
> -       bool "Use L1 memory for rx/tx packets"
> -       depends on BFIN_MAC && (BF527 || BF537)
> -       default y
> -       help
> -         To get maximum network performance, you should use L1 memory as rx/tx buffers.
> -         Say N here if you want to reserve L1 memory for other uses.
> -
> -config BFIN_TX_DESC_NUM
> -       int "Number of transmit buffer packets"
> -       depends on BFIN_MAC
> -       range 6 10 if BFIN_MAC_USE_L1
> -       range 10 100
> -       default "10"
> -       help
> -         Set the number of buffer packets used in driver.
> -
> -config BFIN_RX_DESC_NUM
> -       int "Number of receive buffer packets"
> -       depends on BFIN_MAC
> -       range 20 100 if BFIN_MAC_USE_L1
> -       range 20 800
> -       default "20"
> -       help
> -         Set the number of buffer packets used in driver.
> -
> -config BFIN_MAC_USE_HWSTAMP
> -       bool "Use IEEE 1588 hwstamp"
> -       depends on BFIN_MAC && BF518
> -       default y
> -       help
> -         To support the IEEE 1588 Precision Time Protocol (PTP), select y here
> -
>  config NET_NETX
>        tristate "NetX Ethernet support"
>        select MII
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 8e6dbd7..d249d76 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -57,7 +57,6 @@ obj-$(CONFIG_EQUALIZER) += eql.o
>  obj-$(CONFIG_TUN) += tun.o
>  obj-$(CONFIG_VETH) += veth.o
>  obj-$(CONFIG_NET_NETX) += netx-eth.o
> -obj-$(CONFIG_BFIN_MAC) += bfin_mac.o
>  obj-$(CONFIG_DM9000) += dm9000.o
>  obj-$(CONFIG_ENC28J60) += enc28j60.o
>  obj-$(CONFIG_ETHOC) += ethoc.o
> diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
> index 6b3b3dc..e087337 100644
> --- a/drivers/net/ethernet/Kconfig
> +++ b/drivers/net/ethernet/Kconfig
> @@ -15,6 +15,7 @@ source "drivers/net/ethernet/3com/Kconfig"
>  source "drivers/net/ethernet/amd/Kconfig"
>  source "drivers/net/ethernet/apple/Kconfig"
>  source "drivers/net/ethernet/atheros/Kconfig"
> +source "drivers/net/ethernet/adi/Kconfig"
>  source "drivers/net/ethernet/broadcom/Kconfig"
>  source "drivers/net/ethernet/brocade/Kconfig"
>  source "drivers/net/ethernet/chelsio/Kconfig"
> diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
> index 2a1cbce..826db27 100644
> --- a/drivers/net/ethernet/Makefile
> +++ b/drivers/net/ethernet/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_NET_VENDOR_8390) += 8390/
>  obj-$(CONFIG_NET_VENDOR_AMD) += amd/
>  obj-$(CONFIG_NET_VENDOR_APPLE) += apple/
>  obj-$(CONFIG_NET_VENDOR_ATHEROS) += atheros/
> +obj-$(CONFIG_NET_BFIN) += adi/
>  obj-$(CONFIG_NET_VENDOR_BROADCOM) += broadcom/
>  obj-$(CONFIG_NET_VENDOR_BROCADE) += brocade/
>  obj-$(CONFIG_NET_VENDOR_CHELSIO) += chelsio/
> diff --git a/drivers/net/ethernet/adi/Kconfig b/drivers/net/ethernet/adi/Kconfig
> new file mode 100644
> index 0000000..a2c8a3b
> --- /dev/null
> +++ b/drivers/net/ethernet/adi/Kconfig
> @@ -0,0 +1,65 @@
> +#
> +# Blackfin device configuration
> +#
> +
> +config NET_BFIN
> +       bool "Blackfin devices"
> +       depends on BF516 || BF518 || BF526 || BF527 || BF536 || BF537
> +       ---help---
> +         If you have a network (Ethernet) card belonging to this class, say Y.
> +         Make sure you know the name of your card. Read the Ethernet-HOWTO,
> +         available from <http://www.tldp.org/docs.html#howto>.
> +
> +         If unsure, say Y.
> +
> +         Note that the answer to this question doesn't directly affect the
> +         kernel: saying N will just cause the configurator to skip all
> +         the remaining Blackfin card questions. If you say Y, you will be
> +         asked for your specific card in the following questions.
> +
> +config BFIN_MAC
> +       tristate "Blackfin on-chip MAC support"
> +       depends on NET_BFIN && (BF516 || BF518 || BF526 || BF527 || \
> +                BF536 || BF537)
> +       select CRC32
> +       select MII
> +       select PHYLIB
> +       select BFIN_MAC_USE_L1 if DMA_UNCACHED_NONE
> +       ---help---
> +         This is the driver for Blackfin on-chip mac device. Say Y if you want
> +         it compiled into the kernel. This driver is also available as a
> +         module ( = code which can be inserted in and removed from the running
> +         kernel whenever you want). The module will be called bfin_mac.
> +
> +config BFIN_MAC_USE_L1
> +       bool "Use L1 memory for rx/tx packets"
> +       depends on BFIN_MAC && (BF527 || BF537)
> +       default y
> +       ---help---
> +         To get maximum network performance, you should use L1 memory as rx/tx
> +         buffers. Say N here if you want to reserve L1 memory for other uses.
> +
> +config BFIN_TX_DESC_NUM
> +       int "Number of transmit buffer packets"
> +       depends on BFIN_MAC
> +       range 6 10 if BFIN_MAC_USE_L1
> +       range 10 100
> +       default "10"
> +       ---help---
> +         Set the number of buffer packets used in driver.
> +
> +config BFIN_RX_DESC_NUM
> +       int "Number of receive buffer packets"
> +       depends on BFIN_MAC
> +       range 20 100 if BFIN_MAC_USE_L1
> +       range 20 800
> +       default "20"
> +       ---help---
> +         Set the number of buffer packets used in driver.
> +
> +config BFIN_MAC_USE_HWSTAMP
> +       bool "Use IEEE 1588 hwstamp"
> +       depends on BFIN_MAC && BF518
> +       default y
> +       ---help---
> +         To support the IEEE 1588 Precision Time Protocol (PTP), select y here
> diff --git a/drivers/net/ethernet/adi/Makefile b/drivers/net/ethernet/adi/Makefile
> new file mode 100644
> index 0000000..b1fbe19
> --- /dev/null
> +++ b/drivers/net/ethernet/adi/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# Makefile for the Blackfin device drivers.
> +#
> +
> +obj-$(CONFIG_BFIN_MAC) += bfin_mac.o
> diff --git a/drivers/net/bfin_mac.c b/drivers/net/ethernet/adi/bfin_mac.c
> similarity index 100%
> rename from drivers/net/bfin_mac.c
> rename to drivers/net/ethernet/adi/bfin_mac.c
> diff --git a/drivers/net/bfin_mac.h b/drivers/net/ethernet/adi/bfin_mac.h
> similarity index 100%
> rename from drivers/net/bfin_mac.h
> rename to drivers/net/ethernet/adi/bfin_mac.h
> --
> 1.7.6
>
> _______________________________________________
> Uclinux-dist-devel mailing list
> Uclinux-dist-devel@blackfin.uclinux.org
> https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel
>



-- 
Regards,
--Bob

^ permalink raw reply

* Re: [PATCH] Add pch ieee1588 driver for Intel EG20T PCH
From: Toshiharu Okada @ 2011-08-05  4:51 UTC (permalink / raw)
  To: Richard Cochran
  Cc: richard.cochran, netdev, linux-kernel, qi.wang, yong.y.wang,
	joel.clark, kok.howg.ewe, tomoya-linux
In-Reply-To: <20110803192331.GA11166@riccoc20.at.omicron.at>

Hi Richard

Thank you for your comments.
I will confirm them.

Toshiharu Okada
----- Original Message ----- 
From: "Richard Cochran" <richardcochran@gmail.com>
To: "Toshiharu Okada" <toshiharu-linux@dsn.okisemi.com>
Cc: <richard.cochran@omicron.at>; <netdev@vger.kernel.org>; 
<linux-kernel@vger.kernel.org>; <qi.wang@intel.com>; 
<yong.y.wang@intel.com>; <joel.clark@intel.com>; <kok.howg.ewe@intel.com>; 
<tomoya-linux@dsn.okisemi.com>
Sent: Thursday, August 04, 2011 4:23 AM
Subject: Re: [PATCH] Add pch ieee1588 driver for Intel EG20T PCH


On Wed, Aug 03, 2011 at 03:27:41PM +0900, Toshiharu Okada wrote:
> This patch is for IEEE1588 driver of Intel EG20T PCH.
> EG20T PCH is the platform controller hub that is used in Intel's
> general embedded platform.
> This driver adds support for using the EG20T PCH as a PTP clock.
>
> Would you review this patch, although this driver has not been tested yet?

Thanks for offering this patch. Please see my comments, below.

>
> Signed-off-by: Toshiharu Okada <toshiharu-linux@dsn.okisemi.com>
> ---
>  drivers/ptp/Kconfig   |   13 +
>  drivers/ptp/Makefile  |    1 +
>  drivers/ptp/ptp_pch.c |  629 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 643 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/ptp/ptp_pch.c
>
> diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
> index 68d7201..cd9bc3b 100644
> --- a/drivers/ptp/Kconfig
> +++ b/drivers/ptp/Kconfig
> @@ -72,4 +72,17 @@ config DP83640_PHY
>    In order for this to work, your MAC driver must also
>    implement the skb_tx_timetamp() function.
>
> +config PTP_1588_CLOCK_PCH
> + tristate "Intel PCH EG20T as PTP clock"
> + depends on PTP_1588_CLOCK
> + depends on PCH_GBE
> + help
> +   This driver adds support for using the PCH EG20T as a PTP
> +   clock. This clock is only useful if your PTP programs are
> +   getting hardware time stamps on the PTP Ethernet packets
> +   using the SO_TIMESTAMPING API.
> +
> +   To compile this driver as a module, choose M here: the module
> +   will be called ptp_pch.
> +
>  endmenu
> diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
> index f6933e8..8b58597 100644
> --- a/drivers/ptp/Makefile
> +++ b/drivers/ptp/Makefile
> @@ -5,3 +5,4 @@
>  ptp-y := ptp_clock.o ptp_chardev.o ptp_sysfs.o
>  obj-$(CONFIG_PTP_1588_CLOCK) += ptp.o
>  obj-$(CONFIG_PTP_1588_CLOCK_IXP46X) += ptp_ixp46x.o
> +obj-$(CONFIG_PTP_1588_CLOCK_PCH) += ptp_pch.o
> diff --git a/drivers/ptp/ptp_pch.c b/drivers/ptp/ptp_pch.c
> new file mode 100644
> index 0000000..0a804dc
> --- /dev/null
> +++ b/drivers/ptp/ptp_pch.c
> @@ -0,0 +1,629 @@
> +/*
> + * PTP 1588 clock using the EG20T PCH
> + *
> + * Copyright (C) 2010 OMICRON electronics GmbH
> + * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD.
> + *
> + * This code was derived from the IXP46X driver.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307, 
> USA.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/ptp_clock_kernel.h>
> +
> +#define STATION_ADDR_LEN 20
> +#define PCI_DEVICE_ID_PCH_1588 0x8819
> +#define IO_MEM_BAR 1
> +
> +
> +/* Register read/write macros */
> +#define PCH_BIT_SET_CHECK(addr, bitmask) \
> + ((ioread32(addr) & (bitmask)) == (bitmask))
> +#define PCH_SET_ADDR_BIT(addr, bitmask)\
> + iowrite32((ioread32(addr) | (bitmask)), (addr))
> +#define PCH_CLR_ADDR_BIT(addr, bitmask)\
> + iowrite32((ioread32(addr) & ~(bitmask)), (addr))

I don't really like macros of this sort, just to set and clear
register bits. It doesn't make any fewer lines nor does it clarify the
code. Every driver author will know what is meant by the plain old C,
like:

    x = x | mask;
    x = x & ~mask;

Also, you should consider whether you need to protect against
concurrent access on a register.

> +
> +#define DEFAULT_ADDEND 0xF0000029
> +#define TICKS_NS_SHIFT  4

This driver is based on my ixp46x driver, which is fine because,
looking at the data sheet, it appears that the time stamping unit in
the EG20T PCH is fairly similar.

However, I doubt that these ADDEND and SHIFT values will work for you,
since they were calculated for the frequency compensated clock on the
IXP. They reflect an input clock frequency of 66.666666 MHz and a
nominal frequency of 62.5 MHz (or a period of 16 nanoseconds, thus
SHIFT is 4).

Section 14.6.1.10 in the data sheet seems to imply that the input
clock is 50 MHz. In that case you could use a nominal frequency of
31.25 MHz (period 32 ns, SHIFT 5). However, you need to find out
the actual input clock frequency. If this can vary, then the driver
should allow changing these values.

> +#define N_EXT_TS 2
> +
> +enum pch_status {
> + PCH_SUCCESS,
> + PCH_INVALIDPARAM,
> + PCH_NOTIMESTAMP,
> + PCH_INTERRUPTMODEINUSE,
> + PCH_FAILED,
> + PCH_UNSUPPORTED,
> +};
> +/**
> + * struct pch_ts_regs - IEEE 1588 registers
> + */
> +struct pch_ts_regs {
> + u32 control;
> + u32 event;
> + u32 addend;
> + u32 accum;
> + u32 test;
> + u32 ts_compare;
> + u32 rsystime_lo;
> + u32 rsystime_hi;
> + u32 systime_lo;
> + u32 systime_hi;
> + u32 trgt_lo;
> + u32 trgt_hi;
> + u32 asms_lo;
> + u32 asms_hi;
> + u32 amms_lo;
> + u32 amms_hi;
> + u32 ch_control;

You never program this register. But I think the "Mode" field should
be set to 2. The other settings really make no sense at all.  Or do
you set this in the MAC driver?

(I wonder why Intel retained the very limited PTP V1 modes from the
IXP time stamping unit.)

> + u32 ch_event;
> + u32 tx_snap_lo;
> + u32 tx_snap_hi;
> + u32 rx_snap_lo;
> + u32 rx_snap_hi;
> + u32 src_uuid_lo;
> + u32 src_uuid_hi;
> + u32 can_status;
> + u32 can_snap_lo;
> + u32 can_snap_hi;
> + u32 ts_sel;
> + u32 ts_st[6];
> + u32 reserve1[15];
> + u32 stl_max_set_en;
> + u32 stl_max_set;
> + u32 reserve2[13];
> + u32 srst;
> +};
> +
> +#define PCH_TSC_RESET (1 << 0)
> +#define PCH_TSC_TTM_MASK (1 << 1)
> +#define PCH_TSC_ASMS_MASK (1 << 2)
> +#define PCH_TSC_AMMS_MASK (1 << 3)
> +#define PCH_TSC_PPSM_MASK (1 << 4)
> +#define PCH_TSE_TTIPEND (1 << 1)
> +#define PCH_TSE_SNS (1 << 2)
> +#define PCH_TSE_SNM (1 << 3)
> +#define PCH_TSE_PPS (1 << 4)
> +#define PCH_CC_MM (1 << 0)
> +#define PCH_CC_TA (1 << 1)
> +
> +#define PCH_CC_MODE_SHIFT 16
> +#define PCH_CC_MODE_MASK 0x001F0000
> +#define PCH_CC_VERSION (1 << 31)
> +#define PCH_CE_TXS (1 << 0)
> +#define PCH_CE_RXS (1 << 1)
> +#define PCH_CE_OVR (1 << 0)
> +#define PCH_CE_VAL (1 << 1)
> +#define PCH_ECS_ETH (1 << 0)
> +
> +#define PCH_ECS_CAN (1 << 1)
> +#define PCH_STATION_BYTES 6
> +
> +#define PCH_IEEE1588_ETH (1 << 0)
> +#define PCH_IEEE1588_CAN (1 << 1)
> +/**
> + * struct pch_dev - Driver private data
> + */
> +struct pch_dev {
> + struct pch_ts_regs *regs;
> + struct ptp_clock *ptp_clock;
> + struct ptp_clock_info caps;
> + int exts0_enabled;
> + int exts1_enabled;
> +
> + u32 mem_base;
> + u32 mem_size;
> + u32 irq;
> + u32 suspend:1;
> + u32 initialized:1;
> + struct pci_dev *pdev;
> + spinlock_t lock;

It would be nice to have a short comment telling what this lock
protects. (Yes, I know what it protects, but still that is good
practice to have a comment.)

> +};
> +
> +static inline void pch_eth_enable_set(struct pch_dev *chip)
> +{
> + /* SET the eth_enable bit */
> + PCH_SET_ADDR_BIT(&chip->regs->ts_sel, PCH_ECS_ETH);
> +}

I really don't like this or the other similar helper functions,
below. They don't make the driver more understandable or shorter.
This function is only used in one place. You need at least two callers
to justify this.

> +
> +/*
> + * Register access functions
> + */
> +
> +static u64 pch_systime_read(struct pch_ts_regs *regs)
> +{
> + u64 ns;
> + u32 lo, hi;
> +
> + lo = ioread32(&regs->systime_lo);
> + hi = ioread32(&regs->systime_hi);
> +
> + ns = ((u64) hi) << 32;
> + ns |= lo;
> + ns <<= TICKS_NS_SHIFT;

Here 'ns' will only be in nanoseconds when the ADDEND and SHIFT macros
are correct for the actual machine, as I mentioned above.

> +
> + return ns;
> +}
> +
> +static void pch_systime_write(struct pch_ts_regs *regs, u64 ns)
> +{
> + u32 hi, lo;
> +
> + ns >>= TICKS_NS_SHIFT;
> + hi = ns >> 32;
> + lo = ns & 0xffffffff;
> +
> + iowrite32(lo, &regs->systime_lo);
> + iowrite32(hi, &regs->systime_hi);
> +}
> +

I think the following helper functions ...

> +static inline u32 pch_pps_evt_get(struct pch_dev *chip)
> +{
> + /* Poll for PPS event */
> + return PCH_BIT_SET_CHECK(&chip->regs->event, PCH_TSE_PPS);
> +}
> +
> +static inline u32 pch_amms_evt_get(struct pch_dev *chip)
> +{
> + /* Poll for Auxiliary Master Mode Snapshot Captured event */
> + return PCH_BIT_SET_CHECK(&chip->regs->event, PCH_TSE_SNM);
> +}
> +
> +static inline u32 pch_asms_evt_get(struct pch_dev *chip)
> +{
> + /* Poll for Auxiliary Slave Mode Snapshot Captured event */
> + return PCH_BIT_SET_CHECK(&chip->regs->event, PCH_TSE_SNS);
> +}
> +
> +static inline u32 pch_ttm_evt_get(struct pch_dev *chip)
> +{
> + /* Poll for Target Time Reached event */
> + return PCH_BIT_SET_CHECK(&chip->regs->event, PCH_TSE_TTIPEND);
> +}
> +
> +static inline void pch_pps_evt_clear(struct pch_dev *chip)
> +{
> + /* Clear PPS event */
> + PCH_SET_ADDR_BIT(&chip->regs->event, PCH_TSE_PPS);
> +}
> +
> +static inline void pch_amms_evt_clear(struct pch_dev *chip)
> +{
> + /* Clear Auxiliary Master Mode Snapshot Captured event */
> + PCH_SET_ADDR_BIT(&chip->regs->event, PCH_TSE_SNM);
> +}
> +
> +static inline void pch_asms_evt_clear(struct pch_dev *chip)
> +{
> + /* Clear Auxiliary Slave Mode Snapshot Captured event */
> + PCH_SET_ADDR_BIT(&chip->regs->event, PCH_TSE_SNS);
> +}
> +
> +static inline void pch_ttm_evt_clear(struct pch_dev *chip)
> +{
> + /* Clear Target Time Reached event */
> + PCH_SET_ADDR_BIT(&chip->regs->event, PCH_TSE_TTIPEND);
> +}

... are just noise and add nothing useful to the driver.

> +
> +static inline void pch_block_reset(struct pch_dev *chip)
> +{
> + /* Reset Hardware Assist block */
> + PCH_SET_ADDR_BIT(&chip->regs->control, PCH_TSC_RESET);
> + PCH_CLR_ADDR_BIT(&chip->regs->control, PCH_TSC_RESET);
> +}
> +
> +/* This function enables all 64 bits in system time registers [high & 
> low].
> +This is a work-around for non continuous value in the SystemTime 
> Register*/
> +static void pch_set_system_time_count(struct pch_dev *chip)
> +{
> + iowrite32(0x01, &chip->regs->stl_max_set_en);
> + iowrite32(0xFFFFFFFF, &chip->regs->stl_max_set);
> + iowrite32(0x00, &chip->regs->stl_max_set_en);
> +}
> +
> +static void pch_reset(struct pch_dev *chip)
> +{
> + /* Reset Hardware Assist */
> + pch_block_reset(chip);
> +
> + /* enable all 32 bits in system time registers */
> + pch_set_system_time_count(chip);
> +}

These three, above, are okay, since they encapsulate one logical
operation that takes more than one register access.

However, you might consider whether you need locking here.

> +
> +static void pch_eth_enable(struct pch_dev *chip)
> +{
> + pch_eth_enable_set(chip);
> +}

Again, this helper only has one caller. Why not just set the bit that
you need in line?

> +
> +/*
> + * Interrupt service routine
> + */
> +static irqreturn_t isr(int irq, void *priv)
> +{
> + struct pch_dev *pch_dev = priv;
> + struct pch_ts_regs *regs = pch_dev->regs;
> + struct ptp_clock_event event;
> + u32 ack = 0, lo, hi, val;
> +
> + val = ioread32(&regs->event);
> +
> + if (val & PCH_TSE_SNS) {
> + ack |= PCH_TSE_SNS;
> + if (pch_dev->exts0_enabled) {
> + hi = ioread32(&regs->asms_hi);
> + lo = ioread32(&regs->asms_lo);
> + event.type = PTP_CLOCK_EXTTS;
> + event.index = 0;
> + event.timestamp = ((u64) hi) << 32;
> + event.timestamp |= lo;
> + event.timestamp <<= TICKS_NS_SHIFT;
> + ptp_clock_event(pch_dev->ptp_clock, &event);
> + }
> + }
> +
> + if (val & PCH_TSE_SNM) {
> + ack |= PCH_TSE_SNM;
> + if (pch_dev->exts1_enabled) {
> + hi = ioread32(&regs->amms_hi);
> + lo = ioread32(&regs->amms_lo);
> + event.type = PTP_CLOCK_EXTTS;
> + event.index = 1;
> + event.timestamp = ((u64) hi) << 32;
> + event.timestamp |= lo;
> + event.timestamp <<= TICKS_NS_SHIFT;
> + ptp_clock_event(pch_dev->ptp_clock, &event);
> + }
> + }
> +
> + if (val & PCH_TSE_TTIPEND)
> + ack |= PCH_TSE_TTIPEND; /* this bit seems to be always set */

This ISR code (and much of the rest of the driver) is copied from the
IXP driver. It would be nice to know if it actually works on the atom
hardware. Do have any hardware to try it on?

> +
> + if (ack) {
> + iowrite32(ack, &regs->event);
> + return IRQ_HANDLED;
> + } else
> + return IRQ_NONE;
> +}
> +
> +/*
> + * PTP clock operations
> + */
> +
> +static int ptp_pch_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
> +{
> + u64 adj;
> + u32 diff, addend;
> + int neg_adj = 0;
> + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps);
> + struct pch_ts_regs *regs = pch_dev->regs;
> +
> + if (ppb < 0) {
> + neg_adj = 1;
> + ppb = -ppb;
> + }
> + addend = DEFAULT_ADDEND;
> + adj = addend;
> + adj *= ppb;
> + diff = div_u64(adj, 1000000000ULL);
> +
> + addend = neg_adj ? addend - diff : addend + diff;
> +
> + iowrite32(addend, &regs->addend);
> +
> + return 0;
> +}
> +
> +static int ptp_pch_adjtime(struct ptp_clock_info *ptp, s64 delta)
> +{
> + s64 now;
> + unsigned long flags;
> + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps);
> + struct pch_ts_regs *regs = pch_dev->regs;
> +
> + spin_lock_irqsave(&pch_dev->lock, flags);
> + now = pch_systime_read(regs);
> + now += delta;
> + pch_systime_write(regs, now);
> + spin_unlock_irqrestore(&pch_dev->lock, flags);
> +
> + return 0;
> +}
> +
> +static int ptp_pch_gettime(struct ptp_clock_info *ptp, struct timespec 
> *ts)
> +{
> + u64 ns;
> + u32 remainder;
> + unsigned long flags;
> + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps);
> + struct pch_ts_regs *regs = pch_dev->regs;
> +
> + spin_lock_irqsave(&pch_dev->lock, flags);
> + ns = pch_systime_read(regs);
> + spin_unlock_irqrestore(&pch_dev->lock, flags);
> +
> + ts->tv_sec = div_u64_rem(ns, 1000000000, &remainder);
> + ts->tv_nsec = remainder;
> + return 0;
> +}
> +
> +static int ptp_pch_settime(struct ptp_clock_info *ptp,
> +    const struct timespec *ts)
> +{
> + u64 ns;
> + unsigned long flags;
> + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps);
> + struct pch_ts_regs *regs = pch_dev->regs;
> +
> + ns = ts->tv_sec * 1000000000ULL;
> + ns += ts->tv_nsec;
> +
> + spin_lock_irqsave(&pch_dev->lock, flags);
> + pch_systime_write(regs, ns);
> + spin_unlock_irqrestore(&pch_dev->lock, flags);
> +
> + return 0;
> +}
> +
> +static int ptp_pch_enable(struct ptp_clock_info *ptp,
> +   struct ptp_clock_request *rq, int on)
> +{
> + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps);
> +
> + switch (rq->type) {
> + case PTP_CLK_REQ_EXTTS:
> + switch (rq->extts.index) {
> + case 0:
> + pch_dev->exts0_enabled = on ? 1 : 0;
> + break;
> + case 1:
> + pch_dev->exts1_enabled = on ? 1 : 0;
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> + default:
> + break;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static struct ptp_clock_info ptp_pch_caps = {
> + .owner = THIS_MODULE,
> + .name = "PCH timer",
> + .max_adj = 66666655,

This should be recalculated once you figure out the input clock and
nominal frequency.

> + .n_ext_ts = N_EXT_TS,
> + .pps = 0,
> + .adjfreq = ptp_pch_adjfreq,
> + .adjtime = ptp_pch_adjtime,
> + .gettime = ptp_pch_gettime,
> + .settime = ptp_pch_settime,
> + .enable = ptp_pch_enable,
> +};
> +
> +
> +#ifdef CONFIG_PM
> +static s32 pch_suspend(struct pci_dev *pdev, pm_message_t state)
> +{
> + struct pch_dev *chip = pci_get_drvdata(pdev);
> +
> + chip->suspend = 1;

You set this flag here ...

> + pci_disable_device(pdev);
> + pci_enable_wake(pdev, PCI_D3hot, 0);
> +
> + if (pci_save_state(pdev) != 0) {
> + dev_err(&pdev->dev,
> + "%s: could not save PCI config state\n", __func__);
> + return -ENOMEM;
> + }
> + pci_set_power_state(pdev, pci_choose_state(pdev, state));
> +
> + return 0;
> +}
> +
> +static s32 pch_resume(struct pci_dev *pdev)
> +{
> + s32 ret;
> + struct pch_dev *chip = pci_get_drvdata(pdev);
> +
> + pci_set_power_state(pdev, PCI_D0);
> + pci_restore_state(pdev);
> + ret = pci_enable_device(pdev);
> + if (ret) {
> + dev_err(&pdev->dev, "%s: pci_enable_device failed\n", __func__);
> + return ret;
> + }
> + pci_enable_wake(pdev, PCI_D3hot, 0);
> + chip->suspend = 0;

... and clear it again here. Why?

> + return 0;
> +}
> +#else
> +#define pch_suspend NULL
> +#define pch_resume NULL
> +#endif
> +
> +static void __devexit pch_remove(struct pci_dev *pdev)
> +{
> + struct pch_dev *chip = pci_get_drvdata(pdev);
> +
> + ptp_clock_unregister(chip->ptp_clock);
> + /* free the interrupt */
> + if (pdev->irq != 0)
> + free_irq(pdev->irq, chip);
> +
> + /* unmap the virtual IO memory space */
> + if (chip->regs != 0) {
> + iounmap(chip->regs);
> + chip->regs = 0;
> + }
> + /* release the reserved IO memory space */
> + if (chip->mem_base != 0) {
> + release_mem_region(chip->mem_base, chip->mem_size);
> + chip->mem_base = 0;
> + }
> + pci_disable_device(pdev);
> + kfree(chip);
> + dev_info(&pdev->dev, "%s: complete\n", __func__);
> +}
> +
> +static s32 __devinit
> +pch_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> + s32 ret;
> + struct pch_dev *chip;
> +
> + chip = kzalloc(sizeof(struct pch_dev), GFP_KERNEL);
> + if (chip == NULL)
> + return -ENOMEM;
> +
> + /* enable the 1588 pci device */
> + ret = pci_enable_device(pdev);
> + if (ret != 0) {
> + dev_err(&pdev->dev,
> + "%s:could not enable the pci device\n", __func__);
> + goto err_pci_en;
> + }
> +
> + chip->mem_base = pci_resource_start(pdev, IO_MEM_BAR);
> + if (!chip->mem_base) {
> + dev_err(&pdev->dev,
> + "%s: could not locate IO memory address\n", __func__);
> + ret = -ENODEV;
> + goto err_pci_start;
> + }
> +
> + /* retreive the available length of the IO memory space */
> + chip->mem_size = pci_resource_len(pdev, IO_MEM_BAR);
> +
> + /* allocate the memory for the device registers */
> + if (!request_mem_region
> +     (chip->mem_base, chip->mem_size, "1588_regs")) {

Poor statement break (and this would fit all on one line).

> + dev_err(&pdev->dev,
> +     "%s: could not allocate register memory space\n", __func__);

Bad indentation.

> + ret = -EBUSY;
> + goto err_req_mem_region;
> + }
> +
> + /* get the virtual address to the 1588 registers */
> + chip->regs = ioremap(chip->mem_base, chip->mem_size);
> +
> + if (!chip->regs) {
> + dev_err(&pdev->dev,
> + "%s: Could not get virtual address\n", __func__);
> + ret = -ENOMEM;
> + goto err_ioremap;
> + }
> +
> + chip->caps = ptp_pch_caps;
> + chip->ptp_clock = ptp_clock_register(&chip->caps);
> +
> + if (IS_ERR(chip->ptp_clock))
> + return PTR_ERR(chip->ptp_clock);
> +
> + spin_lock_init(&chip->lock);
> +
> + ret = request_irq(pdev->irq, &isr, IRQF_SHARED, KBUILD_MODNAME, chip);
> + if (ret != 0) {
> + dev_err(&pdev->dev,
> + "%s: failed to get irq %d\n", __func__, pdev->irq);
> + goto err_req_irq;
> + }
> +
> + chip->initialized = 1;

You set this flag, but never use it.

> + /* indicate success */
> + chip->irq = pdev->irq;
> + chip->pdev = pdev;
> + pci_set_drvdata(pdev, chip);
> +
> + /* reset the ieee1588 h/w */
> + pch_reset(chip);
> +
> + iowrite32(DEFAULT_ADDEND, &chip->regs->addend);
> + iowrite32(1, &chip->regs->trgt_lo);
> + iowrite32(0, &chip->regs->trgt_hi);
> + iowrite32(PCH_TSE_TTIPEND, &chip->regs->event);
> + pch_eth_enable(chip);
> +
> + return 0;
> +
> +err_req_irq:
> + ptp_clock_unregister(chip->ptp_clock);
> + iounmap(chip->regs);
> + chip->regs = 0;
> +
> +err_ioremap:
> + release_mem_region(chip->mem_base, chip->mem_size);
> +
> +err_req_mem_region:
> + chip->mem_base = 0;
> +
> +err_pci_start:
> + pci_disable_device(pdev);
> +
> +err_pci_en:
> + kfree(chip);
> + dev_err(&pdev->dev, "%s: probe failed(ret=0x%x)\n", __func__, ret);
> +
> + return ret;
> +}
> +
> +static DEFINE_PCI_DEVICE_TABLE(pch_gbe_pcidev_id) = {
> + {.vendor = PCI_VENDOR_ID_INTEL,

Needs a space (or newline) before the dot.

> + .device = PCI_DEVICE_ID_PCH_1588
> + },
> + {0}
> +};
> +
> +static struct pci_driver pch_pcidev = {
> + .name = KBUILD_MODNAME,
> + .id_table = pch_pcidev_id,

Here you meant "pch_gbe_pcidev_id" instead (or no "gbe" in the PCI
device table).

> + .probe = pch_probe,
> + .remove = pch_remove,
> + .suspend = pch_suspend,
> + .resume = pch_resume,
> +};
> +
> +static void __exit ptp_pch_exit(void)
> +{
> + pci_unregister_driver(&pch_pcidev);
> +}
> +
> +static s32 __init ptp_pch_init(void)
> +{
> + s32 ret;
> +
> + /* register the driver with the pci core */
> + ret = pci_register_driver(&pch_pcidev);
> +
> + return ret;
> +}
> +
> +module_init(ptp_pch_init);
> +module_exit(ptp_pch_exit);
> +
> +MODULE_AUTHOR("OKI SEMICONDUCTOR, <toshiharu-linux@dsn.okisemi.com>");
> +MODULE_DESCRIPTION("PTP clock using the EG20T timer");
> +MODULE_LICENSE("GPL");

Overall, the driver looks okay. I would appreciate if you would take a
look at the comments and submit a revised patch.

I would also like to see how the time stamps are done in the MAC
driver. Have you already posted that?

Feature request: I notice in the data sheet that the time stamping
unit can produce a PPS output. Any chance that you could program this
feature?

Thanks,

Richard


> +
> -- 
> 1.7.4.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply

* Re: [PATCH 2/3] net: Cap number of elements for sendmmsg
From: Anton Blanchard @ 2011-08-05  4:46 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: acme, netdev, linux-security-module, davem, eparis, casey, mjt
In-Reply-To: <201108050429.p754TTBa030939@www262.sakura.ne.jp>


Hi,

> I think 1024 is a reasonable value.
> But I also worry that programmers may wish to send more.
> 
> 
> Apart from the upper limit for vlen argument of sendmmsg()/recvmmsg(),
> we need to deal with stall problem (described below).

Capping vlen at 1024 should prevent that. Your patch does a signed
comparison which just reduces the maximum value by 1 bit doesn't it?

Keep in mind each element could have up to 1024 iovec entries at worst
case, so I think 1024 is a sane upper max.

Anton

> It may be possible to abuse sendmmsg() which was introduced in Linux
> 3.0 and recvmmsg() which was introduced in Linux 2.6.33 for
> triggering CPU stall warning.
> 
> I ran below program
> 
> ---------- Test program start ----------
> #include <string.h>
> #include <stdlib.h>
> #include <stdio.h>
> #include <unistd.h>
> #include <netdb.h>
> #include <sys/types.h>
> #include <sys/ioctl.h>
> #include <sys/socket.h>
> #include <asm/unistd.h>
> #include <errno.h>
> 
> #ifndef __NR_sendmmsg
> #if defined( __PPC__)
> #define __NR_sendmmsg	349
> #elif defined(__x86_64__)
> #define __NR_sendmmsg	307
> #elif defined(__i386__)
> #define __NR_sendmmsg	345
> #else
> #error __NR_sendmmsg not defined
> #endif
> #endif
> 
> struct mmsghdr {
> 	struct msghdr msg_hdr;
> 	unsigned int msg_len;
> };
> 
> static inline int sendmmsg(int fd, struct mmsghdr *mmsg, unsigned
> vlen, unsigned flags)
> {
> 	return syscall(__NR_sendmmsg, fd, mmsg, vlen, flags, NULL);
> }
> 
> #define NUMBATCH (1048576 * 64)
> 
> int main(int argc, char *argv[])
> {
> 	const int fd = socket(AF_INET, SOCK_DGRAM, 0);
> 	struct mmsghdr *datagrams;
> 	unsigned int i;
> 	struct iovec iovec = { };
> 	struct sockaddr_in addr = { };
> 	addr.sin_family = AF_INET;
> 	addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
> 	addr.sin_port = htons(10000);
> 	datagrams = calloc(sizeof(*datagrams), NUMBATCH);
> 	for (i = 0; i < NUMBATCH; ++i) {
> 		datagrams[i].msg_hdr.msg_iov = &iovec;
> 		datagrams[i].msg_hdr.msg_iovlen = 1;
> 		datagrams[i].msg_hdr.msg_name = &addr;
> 		datagrams[i].msg_hdr.msg_namelen = sizeof(addr);
> 	}
> 	printf("Calling sendmmsg()\n");
> 	printf("%d\n", sendmmsg(fd, datagrams, NUMBATCH, 0));
> 	printf("Done\n");
> 	return 0;
> }
> ---------- Test program end ----------
> 
> and got below output.
> 
> # time ./a.out
> Calling sendmmsg()
> INFO: rcu_sched_state detected stall on CPU 0 (t=15000 jiffies)
> 67108864
> Done
> 
> real    2m48.736s
> user    0m0.128s
> sys     0m16.489s
> 
> 
> 
> If this application created threads that matches number of CPUs
> available, and entered into sendmmsg(), the machine will hang for
> many seconds.
> 
> Also, signals are ignored when this application is in sendmmsg().
> That is, if this application is holding much RAM (like above program)
> and is selected by OOM-killer, this application cannot be killed
> until returns from sendmmsg().
> 
> Applying below patch solves the stall message and signal delaying
> problem. ----------------------------------------
> [PATCH] net: Fix sendmmsg() stall problem.
> 
> If the caller passed a huge value (e.g. 64M) to vlen argument of
> sendmmsg(), the caller triggers "INFO: rcu_sched_state detected stall
> on CPU X" message because there is no chance to call scheduler. Thus
> give a chance to call scheduler and also check for pending signal.
> 
> Also, if the caller passed a value where IS_ERR() returns true (e.g.
> UINT_MAX), the caller will get EOF and errno will be set to (e.g.)
> EPERM when all datagrams are successfully sent. Thus, limit the max
> value of vlen to INT_MAX.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: stable <stable@kernel.org> [3.0+]
> ---
>  net/socket.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> --- linux-3.0.orig/net/socket.c
> +++ linux-3.0/net/socket.c
> @@ -1999,6 +1999,8 @@ int __sys_sendmmsg(int fd, struct mmsghd
>  	struct compat_mmsghdr __user *compat_entry;
>  	struct msghdr msg_sys;
>  
> +	if ((int) vlen < 0)
> +		return -EINVAL;
>  	datagrams = 0;
>  
>  	sock = sockfd_lookup_light(fd, &err, &fput_needed);
> @@ -2035,6 +2037,9 @@ int __sys_sendmmsg(int fd, struct mmsghd
>  		if (err)
>  			break;
>  		++datagrams;
> +		cond_resched();
> +		if (signal_pending(current))
> +			break;
>  	}
>  
>  out_put:
> ----------------------------------------
> 
> The situation is similar regaring recvmmsg(). Although recvmmsg()
> less likely stalls than sendmmsg() does, it could happen if a huge
> number of datagrams are in the socket's receive queue and the caller
> attempted to fetch all of them at once. Thus, we may want below patch
> as well.
> 
> ----------------------------------------
> [PATCH] net: Fix recvmmsg() stall problem.
> 
> If the caller passed a huge value to vlen argument of recvmmsg() and
> there are enough datagrams in the socket's receive queue, trying to
> pick up all at once may trigger "INFO: rcu_sched_state detected stall
> on CPU X" message because there is no chance to call scheduler. Thus
> give a chance to call scheduler and also check for pending signal.
> 
> Also, if the caller passed a value where IS_ERR() returns true (e.g.
> UINT_MAX), the caller will get EOF and errno will be set to (e.g.)
> EPERM when all datagrams are successfully received. Thus, limit the
> max value of vlen to INT_MAX.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: stable <stable@kernel.org> [2.6.33+]
> ---
>  net/socket.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> --- linux-3.0.orig/net/socket.c
> +++ linux-3.0/net/socket.c
> @@ -2204,6 +2204,8 @@ int __sys_recvmmsg(int fd, struct mmsghd
>  	struct msghdr msg_sys;
>  	struct timespec end_time;
>  
> +	if ((int) vlen < 0)
> +		return -EINVAL;
>  	if (timeout &&
>  	    poll_select_set_timeout(&end_time, timeout->tv_sec,
>  				    timeout->tv_nsec))
> @@ -2247,6 +2249,9 @@ int __sys_recvmmsg(int fd, struct mmsghd
>  		if (err)
>  			break;
>  		++datagrams;
> +		cond_resched();
> +		if (signal_pending(current))
> +			break;
>  
>  		/* MSG_WAITFORONE turns on MSG_DONTWAIT after one
> packet */ if (flags & MSG_WAITFORONE)
> ----------------------------------------
> 


^ permalink raw reply

* Re: [net-next-2.6 PATCH] enic: Add timestamp to network interface stats
From: Stephen Hemminger @ 2011-08-05  4:42 UTC (permalink / raw)
  To: Danny Guo; +Cc: davem, netdev
In-Reply-To: <20110805001124.32402.42919.stgit@savbu-pc100.cisco.com>

On Thu, 04 Aug 2011 17:11:24 -0700
Danny Guo <dannguo@cisco.com> wrote:

> From: Danny Guo <dannguo@cisco.com>
> 
> This patch adds timestamps in ethtool stats. It makes it easier to provide scripts to users to calculate throughput, etc. It also allows software to synchronize timestamps with host time for correlating host events with stats collection.
> 
> Signed-off-by: Danny Guo <dannguo@cisco.com>
> Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
> Signed-off-by: Roopa Prabhu <roprabhu@cisco.com>
> Signed-off-by: David Wang <dwang2@cisco.com>


The concept is okay, but don't like drivers doing device specific
things all the time.

> ---
>  drivers/net/enic/enic.h       |    2 +-
>  drivers/net/enic/enic_main.c  |    7 ++++++-
>  drivers/net/enic/vnic_dev.c   |    1 +
>  drivers/net/enic/vnic_stats.h |    1 +
>  4 files changed, 9 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
> index ce76d9a..2e58bdc 100644
> --- a/drivers/net/enic/enic.h
> +++ b/drivers/net/enic/enic.h
> @@ -32,7 +32,7 @@
>  
>  #define DRV_NAME		"enic"
>  #define DRV_DESCRIPTION		"Cisco VIC Ethernet NIC Driver"
> -#define DRV_VERSION		"2.1.1.24"
> +#define DRV_VERSION		"2.1.1.25"
>  #define DRV_COPYRIGHT		"Copyright 2008-2011 Cisco Systems, Inc"
>  
>  #define ENIC_BARS_MAX		6
> diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
> index 67a27cd..a3f61c1 100644
> --- a/drivers/net/enic/enic_main.c
> +++ b/drivers/net/enic/enic_main.c
> @@ -121,6 +121,8 @@ static const struct enic_stat enic_rx_stats[] = {
>  static const unsigned int enic_n_tx_stats = ARRAY_SIZE(enic_tx_stats);
>  static const unsigned int enic_n_rx_stats = ARRAY_SIZE(enic_rx_stats);
>  
> +#define ENIC_TIMESTAMP_STAT_NAME	"timestamp(ns)"
> +
>  static int enic_is_dynamic(struct enic *enic)
>  {
>  	return enic->pdev->device == PCI_DEVICE_ID_CISCO_VIC_ENET_DYN;
> @@ -224,6 +226,8 @@ static void enic_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
>  			memcpy(data, enic_rx_stats[i].name, ETH_GSTRING_LEN);
>  			data += ETH_GSTRING_LEN;
>  		}
> +		memcpy(data, ENIC_TIMESTAMP_STAT_NAME, ETH_GSTRING_LEN);
> +		data += ETH_GSTRING_LEN;
>  		break;
>  	}
>  }
> @@ -232,7 +236,7 @@ static int enic_get_sset_count(struct net_device *netdev, int sset)
>  {
>  	switch (sset) {
>  	case ETH_SS_STATS:
> -		return enic_n_tx_stats + enic_n_rx_stats;
> +		return enic_n_tx_stats + enic_n_rx_stats + 1;
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> @@ -251,6 +255,7 @@ static void enic_get_ethtool_stats(struct net_device *netdev,
>  		*(data++) = ((u64 *)&vstats->tx)[enic_tx_stats[i].offset];
>  	for (i = 0; i < enic_n_rx_stats; i++)
>  		*(data++) = ((u64 *)&vstats->rx)[enic_rx_stats[i].offset];
> +	*(data++) = vstats->timestamp_ns;
>  }
>  
>  static u32 enic_get_msglevel(struct net_device *netdev)
> diff --git a/drivers/net/enic/vnic_dev.c b/drivers/net/enic/vnic_dev.c
> index 8c4c8cf..656871a 100644
> --- a/drivers/net/enic/vnic_dev.c
> +++ b/drivers/net/enic/vnic_dev.c
> @@ -467,6 +467,7 @@ int vnic_dev_stats_dump(struct vnic_dev *vdev, struct vnic_stats **stats)
>  		if (!vdev->stats)
>  			return -ENOMEM;
>  	}
> +	memset(vdev->stats, 0, sizeof(*vdev->stats));

No, reading stats should not clear them!


>  	*stats = vdev->stats;
>  	a0 = vdev->stats_pa;
> diff --git a/drivers/net/enic/vnic_stats.h b/drivers/net/enic/vnic_stats.h
> index 77750ec..9d5158d 100644
> --- a/drivers/net/enic/vnic_stats.h
> +++ b/drivers/net/enic/vnic_stats.h
> @@ -65,6 +65,7 @@ struct vnic_rx_stats {
>  struct vnic_stats {
>  	struct vnic_tx_stats tx;
>  	struct vnic_rx_stats rx;
> +	u64 timestamp_ns;
>  };

Two problems: one is that you are exposing a timestamp in a random unit, and
the other problem is that is never set (at least in this code).


^ permalink raw reply

* Re: [PATCHv3] Bridge: Always send NETDEV_CHANGEADDR up on br MAC change.
From: Stephen Hemminger @ 2011-08-05  4:36 UTC (permalink / raw)
  To: Andrei Warkentin; +Cc: netdev
In-Reply-To: <1312510625-26596-2-git-send-email-andreiw@motorola.com>

On Thu,  4 Aug 2011 21:17:05 -0500
Andrei Warkentin <andreiw@motorola.com> wrote:

Half ok, half not.

> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
> index cf09fe5..ef18070 100644
> --- a/net/bridge/br_device.c
> +++ b/net/bridge/br_device.c
> @@ -162,6 +162,7 @@ static int br_set_mac_address(struct net_device *dev, void *p)
>  	br->flags |= BR_SET_MAC_ADDR;
>  	spin_unlock_bh(&br->lock);
>  
> +	call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
>  	return 0;
>  }

This is unnecessary since already done by dev_set_mac_address.

> @@ -492,10 +493,13 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
>  	del_nbp(p);
>  
>  	spin_lock_bh(&br->lock);
> -	br_stp_recalculate_bridge_id(br);
> +	changed_addr = br_stp_recalculate_bridge_id(br);
>  	br_features_recompute(br);
>  	spin_unlock_bh(&br->lock);
>  
> +	if (changed_addr)
> +		call_netdevice_notifiers(NETDEV_CHANGEADDR, br->dev);
> +
>  	return 0;
>  }
>  
Good, I forgot that case.

> diff --git a/net/bridge/br_notify.c b/net/bridge/br_notify.c
> index 404d4e1..651eac3 100644
> --- a/net/bridge/br_notify.c
> +++ b/net/bridge/br_notify.c
> @@ -34,6 +34,7 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
>  	struct net_device *dev = ptr;
>  	struct net_bridge_port *p = br_port_get(dev);
>  	struct net_bridge *br;
> +	bool changed_addr;
>  	int err;
>  
>  	/* not a port of a bridge */
> @@ -51,8 +52,11 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
>  	case NETDEV_CHANGEADDR:
>  		spin_lock_bh(&br->lock);
>  		br_fdb_changeaddr(p, dev->dev_addr);
> -		br_stp_recalculate_bridge_id(br);
> +		changed_addr = br_stp_recalculate_bridge_id(br);
>  		spin_unlock_bh(&br->lock);
> +
> +		if (changed_addr)
> +			call_netdevice_notifiers(NETDEV_CHANGEADDR, br->dev);
>  		break;
>  
>  	case NETDEV_CHANGE:

This is also fine.

> iff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
> index c0990ba..4528e9a 100644
> --- a/net/bridge/br_stp_if.c
> +++ b/net/bridge/br_stp_if.c
> @@ -213,7 +213,7 @@ bool br_stp_recalculate_bridge_id(struct net_bridge *br)
>  
>  	/* user has chosen a value so keep it */
>  	if (br->flags & BR_SET_MAC_ADDR)
> -		return;
> +		return false;
>  
>  	list_for_each_entry(p, &br->port_list, list) {
>  		if (addr == br_mac_zero ||

This is already in net-next.





^ permalink raw reply

* Re: [PATCH 2/3] net: Cap number of elements for sendmmsg
From: Tetsuo Handa @ 2011-08-05  4:29 UTC (permalink / raw)
  To: anton, acme; +Cc: netdev, linux-security-module, davem, eparis, casey, mjt
In-Reply-To: <20110805000822.327910762@samba.org>

Anton Blanchard wrote:
> To limit the amount of time we can spend in sendmmsg, cap the
> number of elements to UIO_MAXIOV (currently 1024). 
> 
> For error handling an application using sendmmsg needs to retry at
> the first unsent message, so capping is simpler and requires less
> application logic than returning EINVAL.
> 
> Signed-off-by: Anton Blanchard <anton@samba.org>
> Cc: stable <stable@kernel.org> [3.0+]

I think 1024 is a reasonable value.
But I also worry that programmers may wish to send more.


Apart from the upper limit for vlen argument of sendmmsg()/recvmmsg(),
we need to deal with stall problem (described below).



It may be possible to abuse sendmmsg() which was introduced in Linux 3.0 and
recvmmsg() which was introduced in Linux 2.6.33 for triggering CPU stall
warning.

I ran below program

---------- Test program start ----------
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <netdb.h>
#include <sys/types.h>
#include <sys/ioctl.h>
#include <sys/socket.h>
#include <asm/unistd.h>
#include <errno.h>

#ifndef __NR_sendmmsg
#if defined( __PPC__)
#define __NR_sendmmsg	349
#elif defined(__x86_64__)
#define __NR_sendmmsg	307
#elif defined(__i386__)
#define __NR_sendmmsg	345
#else
#error __NR_sendmmsg not defined
#endif
#endif

struct mmsghdr {
	struct msghdr msg_hdr;
	unsigned int msg_len;
};

static inline int sendmmsg(int fd, struct mmsghdr *mmsg, unsigned vlen,
			   unsigned flags)
{
	return syscall(__NR_sendmmsg, fd, mmsg, vlen, flags, NULL);
}

#define NUMBATCH (1048576 * 64)

int main(int argc, char *argv[])
{
	const int fd = socket(AF_INET, SOCK_DGRAM, 0);
	struct mmsghdr *datagrams;
	unsigned int i;
	struct iovec iovec = { };
	struct sockaddr_in addr = { };
	addr.sin_family = AF_INET;
	addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
	addr.sin_port = htons(10000);
	datagrams = calloc(sizeof(*datagrams), NUMBATCH);
	for (i = 0; i < NUMBATCH; ++i) {
		datagrams[i].msg_hdr.msg_iov = &iovec;
		datagrams[i].msg_hdr.msg_iovlen = 1;
		datagrams[i].msg_hdr.msg_name = &addr;
		datagrams[i].msg_hdr.msg_namelen = sizeof(addr);
	}
	printf("Calling sendmmsg()\n");
	printf("%d\n", sendmmsg(fd, datagrams, NUMBATCH, 0));
	printf("Done\n");
	return 0;
}
---------- Test program end ----------

and got below output.

# time ./a.out
Calling sendmmsg()
INFO: rcu_sched_state detected stall on CPU 0 (t=15000 jiffies)
67108864
Done

real    2m48.736s
user    0m0.128s
sys     0m16.489s



If this application created threads that matches number of CPUs available, and
entered into sendmmsg(), the machine will hang for many seconds.

Also, signals are ignored when this application is in sendmmsg(). That is,
if this application is holding much RAM (like above program) and is selected by
OOM-killer, this application cannot be killed until returns from sendmmsg().

Applying below patch solves the stall message and signal delaying problem.
----------------------------------------
[PATCH] net: Fix sendmmsg() stall problem.

If the caller passed a huge value (e.g. 64M) to vlen argument of sendmmsg(),
the caller triggers "INFO: rcu_sched_state detected stall on CPU X" message
because there is no chance to call scheduler. Thus give a chance to call
scheduler and also check for pending signal.

Also, if the caller passed a value where IS_ERR() returns true (e.g. UINT_MAX),
the caller will get EOF and errno will be set to (e.g.) EPERM when all
datagrams are successfully sent. Thus, limit the max value of vlen to INT_MAX.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: stable <stable@kernel.org> [3.0+]
---
 net/socket.c |    5 +++++
 1 file changed, 5 insertions(+)

--- linux-3.0.orig/net/socket.c
+++ linux-3.0/net/socket.c
@@ -1999,6 +1999,8 @@ int __sys_sendmmsg(int fd, struct mmsghd
 	struct compat_mmsghdr __user *compat_entry;
 	struct msghdr msg_sys;
 
+	if ((int) vlen < 0)
+		return -EINVAL;
 	datagrams = 0;
 
 	sock = sockfd_lookup_light(fd, &err, &fput_needed);
@@ -2035,6 +2037,9 @@ int __sys_sendmmsg(int fd, struct mmsghd
 		if (err)
 			break;
 		++datagrams;
+		cond_resched();
+		if (signal_pending(current))
+			break;
 	}
 
 out_put:
----------------------------------------

The situation is similar regaring recvmmsg(). Although recvmmsg() less likely
stalls than sendmmsg() does, it could happen if a huge number of datagrams are
in the socket's receive queue and the caller attempted to fetch all of them at
once. Thus, we may want below patch as well.

----------------------------------------
[PATCH] net: Fix recvmmsg() stall problem.

If the caller passed a huge value to vlen argument of recvmmsg() and there are
enough datagrams in the socket's receive queue, trying to pick up all at once
may trigger "INFO: rcu_sched_state detected stall on CPU X" message because
there is no chance to call scheduler. Thus give a chance to call scheduler and
also check for pending signal.

Also, if the caller passed a value where IS_ERR() returns true (e.g. UINT_MAX),
the caller will get EOF and errno will be set to (e.g.) EPERM when all
datagrams are successfully received. Thus, limit the max value of vlen to
INT_MAX.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: stable <stable@kernel.org> [2.6.33+]
---
 net/socket.c |    5 +++++
 1 file changed, 5 insertions(+)

--- linux-3.0.orig/net/socket.c
+++ linux-3.0/net/socket.c
@@ -2204,6 +2204,8 @@ int __sys_recvmmsg(int fd, struct mmsghd
 	struct msghdr msg_sys;
 	struct timespec end_time;
 
+	if ((int) vlen < 0)
+		return -EINVAL;
 	if (timeout &&
 	    poll_select_set_timeout(&end_time, timeout->tv_sec,
 				    timeout->tv_nsec))
@@ -2247,6 +2249,9 @@ int __sys_recvmmsg(int fd, struct mmsghd
 		if (err)
 			break;
 		++datagrams;
+		cond_resched();
+		if (signal_pending(current))
+			break;
 
 		/* MSG_WAITFORONE turns on MSG_DONTWAIT after one packet */
 		if (flags & MSG_WAITFORONE)
----------------------------------------

^ permalink raw reply

* Re: PHY down?
From: Edgar E. Iglesias @ 2011-08-05  4:13 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: Ben Hutchings, netdev
In-Reply-To: <OFEC77B7FA.1BCB17E8-ONC12578E0.0043754B-C12578E0.00441F36@transmode.se>

On Tue, Aug 02, 2011 at 02:24:04PM +0200, Joakim Tjernlund wrote:
> Ben Hutchings <bhutchings@solarflare.com> wrote on 2011/08/02 13:01:08:
> >
> > On Tue, 2011-08-02 at 11:24 +0200, Joakim Tjernlund wrote:
> > > I must be missing something obvious but I cannot find how
> > > to bring eth0's PHY down (link down) from user space.
> > > Tried various settings with ethtool and ifconfig eth0 down but it didn't help.
> >
> > If you configure an interface down, and if the interface is not used for
> > remote management, then some drivers will turn the PHY off.  But there
> > is no general way to control this explicitly.
> 
> OK, when to stop the PHY doesn't seem standardized. Seem logical to me
> to also turn off the PHY when interface is configured down. Perhaps this could
> be agreed upon?
> What does remote management mean? How do I identify if the I/F is used for remote management?
> 
>  Jocke

I'd rather see an explicit way of turning off the PHY. Downing the interface might be
useful without killing the PHY. I've seen PoE switches that kill you when you bring
the link down or reset the PHY after once beeing up, Down/up of interface might become
impossible in some environments if we would standardize on PHY off for interface down.

Maybe an explicit ethtool option?

Cheers

^ permalink raw reply

* Re: Fw: [Bug 39132] Starting with 3.0.0-rc6, masquerading seems to be broken.
From: David Hill @ 2011-08-05  4:09 UTC (permalink / raw)
  To: Julian Anastasov, Florian Mickler; +Cc: netdev, David Miller
In-Reply-To: <alpine.LFD.2.00.1108042131040.1495@ja.ssi.bg>

Hello Julian,

    I'm not using TPROXY and I've used a blank firewall with only 
masquerading and reproduced the issue.
Nothing is in NAT/mangle nor OUTPUT  but the rules mentionned in the 
attached files to this bug.

 Francis Whittle  (Comment #18) has the same issue.

Thank you ,

Dave


----- Original Message ----- 
From: "Julian Anastasov" <ja@ssi.bg>
To: "Florian Mickler" <florian@mickler.org>
Cc: <hilld@binarystorm.net>; <netdev@vger.kernel.org>; "David Miller" 
<davem@davemloft.net>
Sent: Thursday, August 04, 2011 2:38 PM
Subject: Re: Fw: [Bug 39132] Starting with 3.0.0-rc6, masquerading seems to 
be broken.


>
> Hello,
>
> On Thu, 4 Aug 2011, Florian Mickler wrote:
>
>> Can someone take a look at this regression?
>>
>> Begin forwarded message:
>>
>> Date: Thu, 28 Jul 2011 04:51:12 GMT
>> From: bugzilla-daemon@bugzilla.kernel.org
>> To: florian@mickler.org
>> Subject: [Bug 39132] Starting with 3.0.0-rc6, masquerading seems to be
>> broken.
>>
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=39132
>
> So, problem points again to
> "Fix ip_route_me_harder triggering ip_rt_bug" ? May be
> David C. Hill or Florian can provide some information, eg. is
> tproxy used, what NAT rules are used, any rules in OUTPUT
> hooks (NAT/mangle) and which packets are dropped.
>
> Regards
>
> --
> Julian Anastasov <ja@ssi.bg>
>
> -- 
> This message has been scanned for viruses and
> dangerous content by MailScanner, and is
> believed to be clean.
>
>
>
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 10.0.1390 / Virus Database: 1518/3810 - Release Date: 08/04/11
> 


-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.


^ permalink raw reply

* Re: [PATCH 1/3] net: sendmmsg should only return an error if no messages were sent
From: Tetsuo Handa @ 2011-08-05  3:57 UTC (permalink / raw)
  To: acme, rdenis, swhiteho; +Cc: netdev
In-Reply-To: <20110805000822.240895823@samba.org>

Anton Blanchard wrote:
> sendmmsg uses a similar error return strategy as recvmmsg but it
> turns out to be a confusing way to communicate errors.
> 
> The current code stores the error code away and returns it on the next
> sendmmsg call. This means a call with completely valid arguments could
> get an error from a previous call.
> 
> Change things so we only return an error if no datagrams could be sent.
> If less than the requested number of messages were sent, the application
> must retry starting at the first failed one and if the problem is
> persistent the error will be returned.
> 
> This matches the behaviour of other syscalls like read/write - it
> is not an error if less than the requested number of elements are sent.

OK. David S. Miller suggested this behavior and Anton Blanchard agreed with
this behavior.

Quoting from commit a2e27255 "net: Introduce recvmmsg socket syscall":
| . R?mi Denis-Courmont & Steven Whitehouse: If we receive N < vlen
|   datagrams and then recvmsg returns an error, recvmmsg will return
|   the successfully received datagrams, store the error and return it
|   in the next call.

R?mi Denis-Courmont, Steven Whitehouse and Arnaldo Carvalho de Melo, do you
want to change recvmmsg()'s behaviour as well?

^ permalink raw reply

* Re: return of ip_rt_bug()
From: Tom London @ 2011-08-05  2:45 UTC (permalink / raw)
  To: Julian Anastasov; +Cc: Dave Jones, netdev
In-Reply-To: <CAFiZG+W9-vV6o9EbuVEwztW4sQ1QV-MxD+xCHFw7TbNAryzN6A@mail.gmail.com>

On Thu, Aug 4, 2011 at 10:48 AM, Tom London <selinux@gmail.com> wrote:
> On Thu, Aug 4, 2011 at 10:37 AM, Julian Anastasov <ja@ssi.bg> wrote:
>>
>>        Hello,
>>
>> On Thu, 4 Aug 2011, Tom London wrote:
>>
>>> How else can I help?
>>
>>        From your bug report at
>> https://bugzilla.redhat.com/show_bug.cgi?id=712632 I see that
>> the application is xsane, "Manufacturer: EPSON". I downloaded
>> some sources and in sane-backends-git20110804/sanei/sanei_udp.c
>> I see some UDP usage.
>>
>>        For example, sane-backends-git20110804/backend/epson2.c
>> calls sanei_udp_open_broadcast (UDP socket with SO_BROADCAST).
>> The socket is not bound, not connected, application sends packet to
>> 255.255.255.255:3289 in blocking mode and waits for reply for
>> 1 second. It is done for "net autodiscovery" config. As the socket
>> is not bound, kernel should search source address for every packet.
>> Nothing special so far. Not sure why your report has 2 oopses in
>> period of 1 second, may be config has 2 lines "net autodiscovery"
>> and 2 packets are sent?
>>
>>        Your first report was for 192.168.2.5 but
>> I don't see the IP from your last report that is with
>> kernel-3.1.0-0.rc0.git12.1.fc17.x86_64. Now you show local IP is
>> 192.168.2.6. Do you have 192.168.2.5 as local IP, what shows
>> 'ip addr' ?
>>
>>        Can you confirm that the IP you see in oops is always
>> configured (ip addr). Or may be it comes from DHCP and now is
>> 192.168.2.6?
>>
>>        Can you start 'ip monitor' in one console while
>> attaching the USB device, so that we can know if any IP
>> addresses are reconfigured due to some events. For example,
>> script that restarts DHCP.
>>
>> Regards
>>
>> --
>> Julian Anastasov <ja@ssi.bg>
>>
>
> Sure.  I'll set this up when I get back home this evening.
>
> Not sure about the 192.168.2.5  vs 192.168.2.6 confusion.  My laptop
> is connected to a Belkin router, and uses DHCP.  I sometimes have the
> wireless interface connected as well, so perhaps this sometimes occurs
> when only the wired NIC is connected and sometimes when both the wired
> and wireless NICs are connected?  I'll also see if I can uncover any
> DHCP history
>
> I'll try to follow the above instructions, and will report out about 8PM PDT.
>
> tom

OK.  Booted up.  Here is what 'ifconfig' says:

[root@tlondon ~]# ifconfig
eth0      Link encap:Ethernet  HWaddr 00:1F:16:0B:56:A8
          inet addr:192.168.2.6  Bcast:192.168.2.255  Mask:255.255.255.0
          inet6 addr: fe80::21f:16ff:fe0b:56a8/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:2101 errors:0 dropped:0 overruns:0 frame:0
          TX packets:1814 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000
          RX bytes:1719912 (1.6 MiB)  TX bytes:268153 (261.8 KiB)
          Interrupt:20 Memory:f2600000-f2620000

lo        Link encap:Local Loopback
          inet addr:127.0.0.1  Mask:255.0.0.0
          inet6 addr: ::1/128 Scope:Host
          UP LOOPBACK RUNNING  MTU:16436  Metric:1
          RX packets:79 errors:0 dropped:0 overruns:0 frame:0
          TX packets:79 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0
          RX bytes:19162 (18.7 KiB)  TX bytes:19162 (18.7 KiB)

virbr0    Link encap:Ethernet  HWaddr 52:54:00:B9:89:30
          inet addr:192.168.122.1  Bcast:192.168.122.255  Mask:255.255.255.0
          UP BROADCAST MULTICAST  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0
          RX bytes:0 (0.0 b)  TX bytes:0 (0.0 b)

wlan0     Link encap:Ethernet  HWaddr 00:21:5D:AC:C6:92
          inet addr:192.168.2.9  Bcast:192.168.2.255  Mask:255.255.255.0
          inet6 addr: fe80::221:5dff:feac:c692/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:22 errors:0 dropped:0 overruns:0 frame:0
          TX packets:26 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000
          RX bytes:2767 (2.7 KiB)  TX bytes:5705 (5.5 KiB)

[root@tlondon ~]#

'ip addr' says:
[root@tlondon ~]# ip addr
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
    inet6 ::1/128 scope host
       valid_lft forever preferred_lft forever
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast
state UP qlen 1000
    link/ether 00:1f:16:0b:56:a8 brd ff:ff:ff:ff:ff:ff
    inet 192.168.2.6/24 brd 192.168.2.255 scope global eth0
    inet6 fe80::21f:16ff:fe0b:56a8/64 scope link
       valid_lft forever preferred_lft forever
3: wlan0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP qlen 1000
    link/ether 00:21:5d:ac:c6:92 brd ff:ff:ff:ff:ff:ff
    inet 192.168.2.9/24 brd 192.168.2.255 scope global wlan0
    inet6 fe80::221:5dff:feac:c692/64 scope link
       valid_lft forever preferred_lft forever
4: virbr0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue
state DOWN
    link/ether 52:54:00:b9:89:30 brd ff:ff:ff:ff:ff:ff
    inet 192.168.122.1/24 brd 192.168.122.255 scope global virbr0
5: virbr0-nic: <BROADCAST,MULTICAST> mtu 1500 qdisc noop master virbr0
state DOWN qlen 500
    link/ether 52:54:00:b9:89:30 brd ff:ff:ff:ff:ff:ff
[root@tlondon ~]#


 I ran 'ip monitor' in one terminal window, ran 'inotail -f
/var/log/messages' in another, started 'gimp', and
did a 'create from usb:epson'.

I got this in the 'ip monitor' window:

[root@tlondon ~]# ip monitor
3: wlan0: <BROADCAST,MULTICAST,UP,LOWER_UP>
    link/ether
3: wlan0: <BROADCAST,MULTICAST,UP,LOWER_UP>
    link/ether
3: wlan0: <BROADCAST,MULTICAST,UP,LOWER_UP>
    link/ether
192.168.2.1 dev eth0 lladdr 00:1c:df:e2:3e:e9 STALE


I got this in the 'inotify -f /var/log/messages' window:

Aug  4 19:29:27 tlondon kernel: [  305.997223] usb 3-1: new full speed
USB device number 2 using uhci_hcd
Aug  4 19:29:28 tlondon kernel: [  306.581320] usb 3-1: New USB device
found, idVendor=04b8, idProduct=010a
Aug  4 19:29:28 tlondon kernel: [  306.581332] usb 3-1: New USB device
strings: Mfr=1, Product=2, SerialNumber=0
Aug  4 19:29:28 tlondon kernel: [  306.581340] usb 3-1: Product: Perfection1640
Aug  4 19:29:28 tlondon kernel: [  306.581346] usb 3-1: Manufacturer: EPSON
Aug  4 19:29:28 tlondon mtp-probe: checking bus 3, device 2:
"/sys/devices/pci0000:00/0000:00:1a.0/usb3/3-1"
Aug  4 19:29:28 tlondon mtp-probe: bus: 3, device: 2 was not an MTP device
Aug  4 19:30:07 tlondon kernel: [  345.300960] ------------[ cut here
]------------
Aug  4 19:30:07 tlondon kernel: [  345.300977] WARNING: at
net/ipv4/route.c:1714 ip_rt_bug+0x5c/0x62()
Aug  4 19:30:07 tlondon kernel: [  345.300984] Hardware name: 74585FU
Aug  4 19:30:07 tlondon kernel: [  345.300989] Modules linked in: fuse
ip6table_filter ip6_tables ebtable_nat ebtables ipt_MASQUERADE
iptable_nat nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_state
nf_conntrack xt_CHECKSUM ppdev iptable_mangle parport_pc lp parport
tun bridge stp llc rfcomm bnep usblp arc4 snd_usb_audio
snd_usbmidi_lib snd_hda_codec_conexant snd_rawmidi uvcvideo videodev
snd_hda_intel snd_hda_codec media v4l2_compat_ioctl32 snd_hwdep
snd_seq snd_seq_device iwlagn snd_pcm btusb microcode iTCO_wdt
i2c_i801 iTCO_vendor_support thinkpad_acpi mac80211 snd_timer
bluetooth cfg80211 snd_page_alloc rfkill snd soundcore e1000e
virtio_net kvm_intel kvm uinput wmi i915 drm_kms_helper drm
i2c_algo_bit i2c_core video [last unloaded: scsi_wait_scan]
Aug  4 19:30:07 tlondon kernel: [  345.301272] Pid: 2348, comm: xsane
Not tainted 3.1.0-0.rc0.git17.1.fc17.x86_64 #1
Aug  4 19:30:07 tlondon kernel: [  345.301280] Call Trace:
Aug  4 19:30:07 tlondon kernel: [  345.301300]  [<ffffffff8105c470>]
warn_slowpath_common+0x83/0x9b
Aug  4 19:30:07 tlondon kernel: [  345.301315]  [<ffffffff8105c4a2>]
warn_slowpath_null+0x1a/0x1c
Aug  4 19:30:07 tlondon kernel: [  345.301329]  [<ffffffff8142f625>]
ip_rt_bug+0x5c/0x62
Aug  4 19:30:07 tlondon kernel: [  345.301342]  [<ffffffff81437231>]
dst_output+0x19/0x1d
Aug  4 19:30:07 tlondon kernel: [  345.301355]  [<ffffffff81438952>]
ip_local_out+0x20/0x25
Aug  4 19:30:07 tlondon kernel: [  345.301369]  [<ffffffff81439819>]
ip_send_skb+0x19/0x3e
Aug  4 19:30:07 tlondon kernel: [  345.301385]  [<ffffffff81456016>]
udp_send_skb+0x239/0x29b
Aug  4 19:30:07 tlondon kernel: [  345.301399]  [<ffffffff814577b3>]
udp_sendmsg+0x5a1/0x7d4
Aug  4 19:30:07 tlondon kernel: [  345.301415]  [<ffffffff813f69f7>] ?
release_sock+0x35/0x155
Aug  4 19:30:07 tlondon kernel: [  345.301428]  [<ffffffff8143732c>] ?
ip_select_ident+0x3d/0x3d
Aug  4 19:30:07 tlondon kernel: [  345.301443]  [<ffffffff81062587>] ?
local_bh_enable_ip+0xe/0x10
Aug  4 19:30:07 tlondon kernel: [  345.301457]  [<ffffffff814f1519>] ?
_raw_spin_unlock_bh+0x40/0x44
Aug  4 19:30:07 tlondon kernel: [  345.301470]  [<ffffffff813f6b0e>] ?
release_sock+0x14c/0x155
Aug  4 19:30:07 tlondon kernel: [  345.301485]  [<ffffffff8145eccc>]
inet_sendmsg+0x66/0x6f
Aug  4 19:30:07 tlondon kernel: [  345.301498]  [<ffffffff813f1fc2>]
sock_sendmsg+0xe6/0x109
Aug  4 19:30:07 tlondon kernel: [  345.301513]  [<ffffffff8108f078>] ?
lock_acquire+0x10f/0x13e
Aug  4 19:30:07 tlondon kernel: [  345.301528]  [<ffffffff8110d89e>] ?
might_fault+0x5c/0xac
Aug  4 19:30:07 tlondon kernel: [  345.301542]  [<ffffffff8108ef3c>] ?
lock_release+0x1a4/0x1d1
Aug  4 19:30:07 tlondon kernel: [  345.301556]  [<ffffffff8110d8e7>] ?
might_fault+0xa5/0xac
Aug  4 19:30:07 tlondon kernel: [  345.301569]  [<ffffffff813f2d07>] ?
copy_from_user+0x2f/0x31
Aug  4 19:30:07 tlondon kernel: [  345.301582]  [<ffffffff813f4b9d>]
sys_sendto+0x132/0x174
AugAug  4 19:30:08 tlondon kernel: [  346.314606] ------------[ cut
here ]------------
Aug  4 19:30:08 tlondon kernel: [  346.314612] WARNING: at
net/ipv4/route.c:1714 ip_rt_bug+0x5c/0x62()
Aug  4 19:30:08 tlondon kernel: [  346.314615] Hardware name: 74585FU
Aug  4 19:30:08 tlondon kernel: [  346.314616] Modules linked in: fuse
ip6table_filter ip6_tables ebtable_nat ebtables ipt_MASQUERADE
iptable_nat nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_state
nf_conntrack xt_CHECKSUM ppdev iptable_mangle parport_pc lp parport
tun bridge stp llc rfcomm bnep usblp arc4 snd_usb_audio
snd_usbmidi_lib snd_hda_codec_conexant snd_rawmidi uvcvideo videodev
snd_hda_intel snd_hda_codec media v4l2_compat_ioctl32 snd_hwdep
snd_seq snd_seq_device iwlagn snd_pcm btusb microcode iTCO_wdt
i2c_i801 iTCO_vendor_support thinkpad_acpi mac80211 snd_timer
bluetooth cfg80211 snd_page_alloc rfkill snd soundcore e1000e
virtio_net kvm_intel kvm uinput wmi i915 drm_kms_helper drm
i2c_algo_bit i2c_core video [last unloaded: scsi_wait_scan]
Aug  4 19:30:08 tlondon kernel: [  346.314693] Pid: 2348, comm: xsane
Tainted: G        W   3.1.0-0.rc0.git17.1.fc17.x86_64 #1
Aug  4 19:30:08 tlondon kernel: [  346.314695] Call Trace:
Aug  4 19:30:08 tlondon kernel: [  346.314701]  [<ffffffff8105c470>]
warn_slowpath_common+0x83/0x9b
Aug  4 19:30:08 tlondon kernel: [  346.314704]  [<ffffffff8105c4a2>]
warn_slowpath_null+0x1a/0x1c
Aug  4 19:30:08 tlondon kernel: [  346.314708]  [<ffffffff8142f625>]
ip_rt_bug+0x5c/0x62
Aug  4 19:30:08 tlondon kernel: [  346.314711]  [<ffffffff81437231>]
dst_output+0x19/0x1d
Aug  4 19:30:08 tlondon kernel: [  346.314714]  [<ffffffff81438952>]
ip_local_out+0x20/0x25
Aug  4 19:30:08 tlondon kernel: [  346.314717]  [<ffffffff81439819>]
ip_send_skb+0x19/0x3e
Aug  4 19:30:08 tlondon kernel: [  346.314721]  [<ffffffff81456016>]
udp_send_skb+0x239/0x29b
Aug  4 19:30:08 tlondon kernel: [  346.314725]  [<ffffffff814577b3>]
udp_sendmsg+0x5a1/0x7d4
Aug  4 19:30:08 tlondon kernel: [  346.314729]  [<ffffffff813f69f7>] ?
release_sock+0x35/0x155
Aug  4 19:30:08 tlondon kernel: [  346.314732]  [<ffffffff8143732c>] ?
ip_select_ident+0x3d/0x3d
Aug  4 19:30:08 tlondon kernel: [  346.314736]  [<ffffffff81062587>] ?
local_bh_enable_ip+0xe/0x10
Aug  4 19:30:08 tlondon kernel: [  346.314740]  [<ffffffff814f1519>] ?
_raw_spin_unlock_bh+0x40/0x44
Aug  4 19:30:08 tlondon kernel: [  346.314743]  [<ffffffff813f6b0e>] ?
release_sock+0x14c/0x155
Aug  4 19:30:08 tlondon kernel: [  346.314747]  [<ffffffff8145eccc>]
inet_sendmsg+0x66/0x6f
Aug  4 19:30:08 tlondon kernel: [  346.314750]  [<ffffffff813f1fc2>]
sock_sendmsg+0xe6/0x109
Aug  4 19:30:08 tlondon kernel: [  346.314754]  [<ffffffff8108f078>] ?
lock_acquire+0x10f/0x13e
Aug  4 19:30:08 tlondon kernel: [  346.314758]  [<ffffffff8110d89e>] ?
might_fault+0x5c/0xac
Aug  4 19:30:08 tlondon kernel: [  346.314761]  [<ffffffff8108ef3c>] ?
lock_release+0x1a4/0x1d1
Aug  4 19:30:08 tlondon kernel: [  346.314765]  [<ffffffff8110d8e7>] ?
might_fault+0xa5/0xac
Aug  4 19:30:08 tlondon kernel: [  346.314768]  [<ffffffff813f2d07>] ?
copy_from_user+0x2f/0x31
Aug  4 19:30:08 tlondon kernel: [  346.314771]  [<ffffffff813f4b9d>]
sys_sendto+0x132/0x174
Aug  4 19:30:08 tlondon kernel: [  346.314775]  [<ffffffff810b29eb>] ?
audit_syscall_entry+0x11c/0x148
Aug  4 19:30:08 tlondon kernel: [  346.314780]  [<ffffffff8124f03e>] ?
trace_hardirqs_on_thunk+0x3a/0x3f
Aug  4 19:30:08 tlondon kernel: [  346.314784]  [<ffffffff814f8382>]
system_call_fastpath+0x16/0x1b
Aug  4 19:30:08 tlondon kernel: [  346.314786] ---[ end trace
97e7c0a8de097c51 ]---

Regarding the 'movable IP Address' issue (.5 vs. .6), I found the
following in /var/lib/dhclient/dhclient-5fb06bd0-0bb0-7ffb-45f1-d6edd65f3e03-eth0.lease

lease {
  interface "eth0";
  fixed-address 192.168.2.5;
  option subnet-mask 255.255.255.0;
  option dhcp-lease-time 4294967295;
  option routers 192.168.2.1;
  option dhcp-message-type 5;
  option dhcp-server-identifier 192.168.2.1;
  option domain-name-servers 192.168.2.1;
  option domain-name "TintonFalls";
  renew 4 2079/07/06 21:52:00;
  rebind 1 2130/10/30 06:17:29;
  expire 6 2147/11/04 01:06:07;
}
lease {
  interface "eth0";
  fixed-address 192.168.2.6;
  option subnet-mask 255.255.255.0;
  option dhcp-lease-time 4294967295;
  option routers 192.168.2.1;
  option dhcp-message-type 5;
  option dhcp-server-identifier 192.168.2.1;
  option domain-name-servers 192.168.2.1;
  option domain-name "TintonFalls";
  renew 2 2079/08/22 16:15:42;
  rebind 4 2130/09/07 00:41:11;
  expire 1 2147/09/11 19:29:49;
}

So, my router is just giving my wired NIC different addresses....

More I can provide?

tom
-- 
Tom London

^ permalink raw reply

* [PATCHv3] Bridge: Always send NETDEV_CHANGEADDR up on br MAC change.
From: Andrei Warkentin @ 2011-08-05  2:17 UTC (permalink / raw)
  To: netdev; +Cc: Andrei Warkentin, Stephen Hemminger
In-Reply-To: <1312509349-23363-1-git-send-email-andreiw@motorola.com>

This ensures the neighbor entries associated with the bridge
dev are flushed, also invalidating the associated cached L2 headers.

This means we br_add_if/br_del_if ports to implement hand-over and
not wind up with bridge packets going out with stale MAC.

This means we can also change MAC of port device and also not wind
up with bridge packets going out with stale MAC.

This builds on Stephen Hemminger's patch, also handling the br_del_if
case, port MAC change case, and bridge MAC manual assignment case.

Cc: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 net/bridge/br_device.c |    1 +
 net/bridge/br_if.c     |    6 +++++-
 net/bridge/br_notify.c |    6 +++++-
 net/bridge/br_stp_if.c |    2 +-
 4 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index cf09fe5..ef18070 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -162,6 +162,7 @@ static int br_set_mac_address(struct net_device *dev, void *p)
 	br->flags |= BR_SET_MAC_ADDR;
 	spin_unlock_bh(&br->lock);
 
+	call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
 	return 0;
 }
 
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 204e542..36ad887 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -481,6 +481,7 @@ put_back:
 int br_del_if(struct net_bridge *br, struct net_device *dev)
 {
 	struct net_bridge_port *p;
+	bool changed_addr;
 
 	if (!br_port_exists(dev))
 		return -EINVAL;
@@ -492,10 +493,13 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
 	del_nbp(p);
 
 	spin_lock_bh(&br->lock);
-	br_stp_recalculate_bridge_id(br);
+	changed_addr = br_stp_recalculate_bridge_id(br);
 	br_features_recompute(br);
 	spin_unlock_bh(&br->lock);
 
+	if (changed_addr)
+		call_netdevice_notifiers(NETDEV_CHANGEADDR, br->dev);
+
 	return 0;
 }
 
diff --git a/net/bridge/br_notify.c b/net/bridge/br_notify.c
index 404d4e1..651eac3 100644
--- a/net/bridge/br_notify.c
+++ b/net/bridge/br_notify.c
@@ -34,6 +34,7 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
 	struct net_device *dev = ptr;
 	struct net_bridge_port *p = br_port_get(dev);
 	struct net_bridge *br;
+	bool changed_addr;
 	int err;
 
 	/* not a port of a bridge */
@@ -51,8 +52,11 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
 	case NETDEV_CHANGEADDR:
 		spin_lock_bh(&br->lock);
 		br_fdb_changeaddr(p, dev->dev_addr);
-		br_stp_recalculate_bridge_id(br);
+		changed_addr = br_stp_recalculate_bridge_id(br);
 		spin_unlock_bh(&br->lock);
+
+		if (changed_addr)
+			call_netdevice_notifiers(NETDEV_CHANGEADDR, br->dev);
 		break;
 
 	case NETDEV_CHANGE:
diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index c0990ba..4528e9a 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -213,7 +213,7 @@ bool br_stp_recalculate_bridge_id(struct net_bridge *br)
 
 	/* user has chosen a value so keep it */
 	if (br->flags & BR_SET_MAC_ADDR)
-		return;
+		return false;
 
 	list_for_each_entry(p, &br->port_list, list) {
 		if (addr == br_mac_zero ||
-- 
1.7.0.4


^ permalink raw reply related

* Always send NETDEV_CHANGEADDR up
From: Andrei Warkentin @ 2011-08-05  2:17 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1312509349-23363-1-git-send-email-andreiw@motorola.com>

Hi,

I apologize for spamming. I removed the meaningless Change-Id from
the patch.

ToC:
[PATCHv3] Bridge: Always send NETDEV_CHANGEADDR up on br MAC change.

A

^ permalink raw reply

* [PATCHv2] Bridge: Always send NETDEV_CHANGEADDR up on br MAC change.
From: Andrei Warkentin @ 2011-08-05  2:11 UTC (permalink / raw)
  To: netdev; +Cc: Andrei Warkentin, Stephen Hemminger
In-Reply-To: <1312509349-23363-1-git-send-email-andreiw@motorola.com>

This ensures the neighbor entries associated with the bridge
dev are flushed, also invalidating the associated cached L2 headers.

This means we br_add_if/br_del_if ports to implement hand-over and
not wind up with bridge packets going out with stale MAC.

This means we can also change MAC of port device and also not wind
up with bridge packets going out with stale MAC.

This builds on Stephen Hemminger's patch, also handling the br_del_if
case, port MAC change case, and bridge MAC manual assignment case.

Change-Id: I6039ba021006f854e0e7e83dd1c4261c500aeab7
Cc: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Andrei Warkentin <andreiw@motorola.com>
---
 net/bridge/br_device.c |    1 +
 net/bridge/br_if.c     |    6 +++++-
 net/bridge/br_notify.c |    6 +++++-
 net/bridge/br_stp_if.c |    2 +-
 4 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index cf09fe5..ef18070 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -162,6 +162,7 @@ static int br_set_mac_address(struct net_device *dev, void *p)
 	br->flags |= BR_SET_MAC_ADDR;
 	spin_unlock_bh(&br->lock);
 
+	call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
 	return 0;
 }
 
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 204e542..36ad887 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -481,6 +481,7 @@ put_back:
 int br_del_if(struct net_bridge *br, struct net_device *dev)
 {
 	struct net_bridge_port *p;
+	bool changed_addr;
 
 	if (!br_port_exists(dev))
 		return -EINVAL;
@@ -492,10 +493,13 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
 	del_nbp(p);
 
 	spin_lock_bh(&br->lock);
-	br_stp_recalculate_bridge_id(br);
+	changed_addr = br_stp_recalculate_bridge_id(br);
 	br_features_recompute(br);
 	spin_unlock_bh(&br->lock);
 
+	if (changed_addr)
+		call_netdevice_notifiers(NETDEV_CHANGEADDR, br->dev);
+
 	return 0;
 }
 
diff --git a/net/bridge/br_notify.c b/net/bridge/br_notify.c
index 404d4e1..651eac3 100644
--- a/net/bridge/br_notify.c
+++ b/net/bridge/br_notify.c
@@ -34,6 +34,7 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
 	struct net_device *dev = ptr;
 	struct net_bridge_port *p = br_port_get(dev);
 	struct net_bridge *br;
+	bool changed_addr;
 	int err;
 
 	/* not a port of a bridge */
@@ -51,8 +52,11 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
 	case NETDEV_CHANGEADDR:
 		spin_lock_bh(&br->lock);
 		br_fdb_changeaddr(p, dev->dev_addr);
-		br_stp_recalculate_bridge_id(br);
+		changed_addr = br_stp_recalculate_bridge_id(br);
 		spin_unlock_bh(&br->lock);
+
+		if (changed_addr)
+			call_netdevice_notifiers(NETDEV_CHANGEADDR, br->dev);
 		break;
 
 	case NETDEV_CHANGE:
diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index c0990ba..4528e9a 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -213,7 +213,7 @@ bool br_stp_recalculate_bridge_id(struct net_bridge *br)
 
 	/* user has chosen a value so keep it */
 	if (br->flags & BR_SET_MAC_ADDR)
-		return;
+		return false;
 
 	list_for_each_entry(p, &br->port_list, list) {
 		if (addr == br_mac_zero ||
-- 
1.7.0.4


^ permalink raw reply related

* Always send NETDEV_CHANGEADDR up
From: Andrei Warkentin @ 2011-08-05  2:11 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1312509349-23363-1-git-send-email-andreiw@motorola.com>

Minor fix in br_notify, such that notification only is sent if 
bridge MAC address is updated.

ToC:
[PATCHv2] Bridge: Always send NETDEV_CHANGEADDR up on br MAC change.

A

^ permalink raw reply

* [RFC 1/4] [flexcan] Abstract off read/write for big/little endian.
From: Robin Holt @ 2011-08-05  2:06 UTC (permalink / raw)
  To: Robin Holt, Marc Kleine-Budde, Wolfgang Grandegger
  Cc: Robin Holt, socketcan-core, netdev
In-Reply-To: <1312509979-13226-1-git-send-email-holt@sgi.com>

First step in converting the flexcan driver from supporting just arm to
supporting both arm and powerpc architectures.

Signed-off-by: Robin Holt <holt@sgi.com>
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Wolfgang Grandegger <wg@grandegger.com>
Cc: socketcan-core@lists.berlios.de
Cc: netdev@vger.kernel.org
---
 drivers/net/can/flexcan.c |  140 ++++++++++++++++++++++++++------------------
 1 files changed, 83 insertions(+), 57 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 67d9fc0..74b1706 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -196,6 +196,31 @@ static struct can_bittiming_const flexcan_bittiming_const = {
 };
 
 /*
+ * Abstract off the read/write for arm versus ppc.
+ */
+#if defined(__BIG_ENDIAN)
+static inline u32 flexcan_read(void __iomem *addr)
+{
+	return in_be32(addr);
+}
+
+static inline void flexcan_write(u32 val, void __iomem *addr)
+{
+	out_be32(addr, val);
+}
+#else
+static inline u32 flexcan_read(void __iomem *addr)
+{
+	return readl(addr);
+}
+
+static inline void flexcan_write(u32 val, void __iomem *addr)
+{
+	writel(val, addr);
+}
+#endif
+
+/*
  * Swtich transceiver on or off
  */
 static void flexcan_transceiver_switch(const struct flexcan_priv *priv, int on)
@@ -216,9 +241,9 @@ static inline void flexcan_chip_enable(struct flexcan_priv *priv)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg;
 
-	reg = readl(&regs->mcr);
+	reg = flexcan_read(&regs->mcr);
 	reg &= ~FLEXCAN_MCR_MDIS;
-	writel(reg, &regs->mcr);
+	flexcan_write(reg, &regs->mcr);
 
 	udelay(10);
 }
@@ -228,9 +253,9 @@ static inline void flexcan_chip_disable(struct flexcan_priv *priv)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg;
 
-	reg = readl(&regs->mcr);
+	reg = flexcan_read(&regs->mcr);
 	reg |= FLEXCAN_MCR_MDIS;
-	writel(reg, &regs->mcr);
+	flexcan_write(reg, &regs->mcr);
 }
 
 static int flexcan_get_berr_counter(const struct net_device *dev,
@@ -238,7 +263,7 @@ static int flexcan_get_berr_counter(const struct net_device *dev,
 {
 	const struct flexcan_priv *priv = netdev_priv(dev);
 	struct flexcan_regs __iomem *regs = priv->base;
-	u32 reg = readl(&regs->ecr);
+	u32 reg = flexcan_read(&regs->ecr);
 
 	bec->txerr = (reg >> 0) & 0xff;
 	bec->rxerr = (reg >> 8) & 0xff;
@@ -272,15 +297,15 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	if (cf->can_dlc > 0) {
 		u32 data = be32_to_cpup((__be32 *)&cf->data[0]);
-		writel(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
+		flexcan_write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
 	}
 	if (cf->can_dlc > 3) {
 		u32 data = be32_to_cpup((__be32 *)&cf->data[4]);
-		writel(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
+		flexcan_write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
 	}
 
-	writel(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
-	writel(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
+	flexcan_write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
+	flexcan_write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
 
 	kfree_skb(skb);
 
@@ -468,8 +493,8 @@ static void flexcan_read_fifo(const struct net_device *dev,
 	struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
 	u32 reg_ctrl, reg_id;
 
-	reg_ctrl = readl(&mb->can_ctrl);
-	reg_id = readl(&mb->can_id);
+	reg_ctrl = flexcan_read(&mb->can_ctrl);
+	reg_id = flexcan_read(&mb->can_id);
 	if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
 		cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG;
 	else
@@ -479,12 +504,12 @@ static void flexcan_read_fifo(const struct net_device *dev,
 		cf->can_id |= CAN_RTR_FLAG;
 	cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);
 
-	*(__be32 *)(cf->data + 0) = cpu_to_be32(readl(&mb->data[0]));
-	*(__be32 *)(cf->data + 4) = cpu_to_be32(readl(&mb->data[1]));
+	*(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb->data[0]));
+	*(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb->data[1]));
 
 	/* mark as read */
-	writel(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
-	readl(&regs->timer);
+	flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
+	flexcan_read(&regs->timer);
 }
 
 static int flexcan_read_frame(struct net_device *dev)
@@ -520,17 +545,17 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
 	 * The error bits are cleared on read,
 	 * use saved value from irq handler.
 	 */
-	reg_esr = readl(&regs->esr) | priv->reg_esr;
+	reg_esr = flexcan_read(&regs->esr) | priv->reg_esr;
 
 	/* handle state changes */
 	work_done += flexcan_poll_state(dev, reg_esr);
 
 	/* handle RX-FIFO */
-	reg_iflag1 = readl(&regs->iflag1);
+	reg_iflag1 = flexcan_read(&regs->iflag1);
 	while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
 	       work_done < quota) {
 		work_done += flexcan_read_frame(dev);
-		reg_iflag1 = readl(&regs->iflag1);
+		reg_iflag1 = flexcan_read(&regs->iflag1);
 	}
 
 	/* report bus errors */
@@ -540,8 +565,8 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
 	if (work_done < quota) {
 		napi_complete(napi);
 		/* enable IRQs */
-		writel(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
-		writel(priv->reg_ctrl_default, &regs->ctrl);
+		flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+		flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
 	}
 
 	return work_done;
@@ -555,9 +580,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg_iflag1, reg_esr;
 
-	reg_iflag1 = readl(&regs->iflag1);
-	reg_esr = readl(&regs->esr);
-	writel(FLEXCAN_ESR_ERR_INT, &regs->esr);	/* ACK err IRQ */
+	reg_iflag1 = flexcan_read(&regs->iflag1);
+	reg_esr = flexcan_read(&regs->esr);
+	flexcan_write(FLEXCAN_ESR_ERR_INT, &regs->esr);	/* ACK err IRQ */
 
 	/*
 	 * schedule NAPI in case of:
@@ -573,16 +598,16 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 		 * save them for later use.
 		 */
 		priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
-		writel(FLEXCAN_IFLAG_DEFAULT & ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE,
-		       &regs->imask1);
-		writel(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
+		flexcan_write(FLEXCAN_IFLAG_DEFAULT &
+			~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);
+		flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
 		       &regs->ctrl);
 		napi_schedule(&priv->napi);
 	}
 
 	/* FIFO overflow */
 	if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
-		writel(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
+		flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
 		dev->stats.rx_over_errors++;
 		dev->stats.rx_errors++;
 	}
@@ -591,7 +616,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	if (reg_iflag1 & (1 << FLEXCAN_TX_BUF_ID)) {
 		/* tx_bytes is incremented in flexcan_start_xmit */
 		stats->tx_packets++;
-		writel((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
+		flexcan_write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
 		netif_wake_queue(dev);
 	}
 
@@ -605,7 +630,7 @@ static void flexcan_set_bittiming(struct net_device *dev)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg;
 
-	reg = readl(&regs->ctrl);
+	reg = flexcan_read(&regs->ctrl);
 	reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) |
 		 FLEXCAN_CTRL_RJW(0x3) |
 		 FLEXCAN_CTRL_PSEG1(0x7) |
@@ -629,11 +654,11 @@ static void flexcan_set_bittiming(struct net_device *dev)
 		reg |= FLEXCAN_CTRL_SMP;
 
 	dev_info(dev->dev.parent, "writing ctrl=0x%08x\n", reg);
-	writel(reg, &regs->ctrl);
+	flexcan_write(reg, &regs->ctrl);
 
 	/* print chip status */
 	dev_dbg(dev->dev.parent, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__,
-		readl(&regs->mcr), readl(&regs->ctrl));
+		flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
 }
 
 /*
@@ -654,10 +679,10 @@ static int flexcan_chip_start(struct net_device *dev)
 	flexcan_chip_enable(priv);
 
 	/* soft reset */
-	writel(FLEXCAN_MCR_SOFTRST, &regs->mcr);
+	flexcan_write(FLEXCAN_MCR_SOFTRST, &regs->mcr);
 	udelay(10);
 
-	reg_mcr = readl(&regs->mcr);
+	reg_mcr = flexcan_read(&regs->mcr);
 	if (reg_mcr & FLEXCAN_MCR_SOFTRST) {
 		dev_err(dev->dev.parent,
 			"Failed to softreset can module (mcr=0x%08x)\n",
@@ -679,12 +704,12 @@ static int flexcan_chip_start(struct net_device *dev)
 	 * choose format C
 	 *
 	 */
-	reg_mcr = readl(&regs->mcr);
+	reg_mcr = flexcan_read(&regs->mcr);
 	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
 		FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
 		FLEXCAN_MCR_IDAM_C;
 	dev_dbg(dev->dev.parent, "%s: writing mcr=0x%08x", __func__, reg_mcr);
-	writel(reg_mcr, &regs->mcr);
+	flexcan_write(reg_mcr, &regs->mcr);
 
 	/*
 	 * CTRL
@@ -702,7 +727,7 @@ static int flexcan_chip_start(struct net_device *dev)
 	 * (FLEXCAN_CTRL_ERR_MSK), too. Otherwise we don't get any
 	 * warning or bus passive interrupts.
 	 */
-	reg_ctrl = readl(&regs->ctrl);
+	reg_ctrl = flexcan_read(&regs->ctrl);
 	reg_ctrl &= ~FLEXCAN_CTRL_TSYN;
 	reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF |
 		FLEXCAN_CTRL_ERR_STATE | FLEXCAN_CTRL_ERR_MSK;
@@ -710,38 +735,39 @@ static int flexcan_chip_start(struct net_device *dev)
 	/* save for later use */
 	priv->reg_ctrl_default = reg_ctrl;
 	dev_dbg(dev->dev.parent, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
-	writel(reg_ctrl, &regs->ctrl);
+	flexcan_write(reg_ctrl, &regs->ctrl);
 
 	for (i = 0; i < ARRAY_SIZE(regs->cantxfg); i++) {
-		writel(0, &regs->cantxfg[i].can_ctrl);
-		writel(0, &regs->cantxfg[i].can_id);
-		writel(0, &regs->cantxfg[i].data[0]);
-		writel(0, &regs->cantxfg[i].data[1]);
+		flexcan_write(0, &regs->cantxfg[i].can_ctrl);
+		flexcan_write(0, &regs->cantxfg[i].can_id);
+		flexcan_write(0, &regs->cantxfg[i].data[0]);
+		flexcan_write(0, &regs->cantxfg[i].data[1]);
 
 		/* put MB into rx queue */
-		writel(FLEXCAN_MB_CNT_CODE(0x4), &regs->cantxfg[i].can_ctrl);
+		flexcan_write(FLEXCAN_MB_CNT_CODE(0x4),
+			&regs->cantxfg[i].can_ctrl);
 	}
 
 	/* acceptance mask/acceptance code (accept everything) */
-	writel(0x0, &regs->rxgmask);
-	writel(0x0, &regs->rx14mask);
-	writel(0x0, &regs->rx15mask);
+	flexcan_write(0x0, &regs->rxgmask);
+	flexcan_write(0x0, &regs->rx14mask);
+	flexcan_write(0x0, &regs->rx15mask);
 
 	flexcan_transceiver_switch(priv, 1);
 
 	/* synchronize with the can bus */
-	reg_mcr = readl(&regs->mcr);
+	reg_mcr = flexcan_read(&regs->mcr);
 	reg_mcr &= ~FLEXCAN_MCR_HALT;
-	writel(reg_mcr, &regs->mcr);
+	flexcan_write(reg_mcr, &regs->mcr);
 
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
 	/* enable FIFO interrupts */
-	writel(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+	flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
 
 	/* print chip status */
 	dev_dbg(dev->dev.parent, "%s: reading mcr=0x%08x ctrl=0x%08x\n",
-		__func__, readl(&regs->mcr), readl(&regs->ctrl));
+		__func__, flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
 
 	return 0;
 
@@ -763,12 +789,12 @@ static void flexcan_chip_stop(struct net_device *dev)
 	u32 reg;
 
 	/* Disable all interrupts */
-	writel(0, &regs->imask1);
+	flexcan_write(0, &regs->imask1);
 
 	/* Disable + halt module */
-	reg = readl(&regs->mcr);
+	reg = flexcan_read(&regs->mcr);
 	reg |= FLEXCAN_MCR_MDIS | FLEXCAN_MCR_HALT;
-	writel(reg, &regs->mcr);
+	flexcan_write(reg, &regs->mcr);
 
 	flexcan_transceiver_switch(priv, 0);
 	priv->can.state = CAN_STATE_STOPPED;
@@ -860,24 +886,24 @@ static int __devinit register_flexcandev(struct net_device *dev)
 
 	/* select "bus clock", chip must be disabled */
 	flexcan_chip_disable(priv);
-	reg = readl(&regs->ctrl);
+	reg = flexcan_read(&regs->ctrl);
 	reg |= FLEXCAN_CTRL_CLK_SRC;
-	writel(reg, &regs->ctrl);
+	flexcan_write(reg, &regs->ctrl);
 
 	flexcan_chip_enable(priv);
 
 	/* set freeze, halt and activate FIFO, restrict register access */
-	reg = readl(&regs->mcr);
+	reg = flexcan_read(&regs->mcr);
 	reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
 		FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
-	writel(reg, &regs->mcr);
+	flexcan_write(reg, &regs->mcr);
 
 	/*
 	 * Currently we only support newer versions of this core
 	 * featuring a RX FIFO. Older cores found on some Coldfire
 	 * derivates are not yet supported.
 	 */
-	reg = readl(&regs->mcr);
+	reg = flexcan_read(&regs->mcr);
 	if (!(reg & FLEXCAN_MCR_FEN)) {
 		dev_err(dev->dev.parent,
 			"Could not enable RX FIFO, unsupported core\n");
-- 
1.7.2.1


^ permalink raw reply related

* [RFC 4/4] [flexcan] Add support for FLEXCAN_DEBUG
From: Robin Holt @ 2011-08-05  2:06 UTC (permalink / raw)
  To: Robin Holt, Marc Kleine-Budde, Wolfgang Grandegger
  Cc: Robin Holt, socketcan-core, netdev
In-Reply-To: <1312509979-13226-1-git-send-email-holt@sgi.com>

Add a wrapper function for a register dump when a developer defines
FLEXCAN_DEBUG.

Signed-off-by: Robin Holt <holt@sgi.com>
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Wolfgang Grandegger <wg@grandegger.com>
Cc: socketcan-core@lists.berlios.de
Cc: netdev@vger.kernel.org
---
 drivers/net/can/flexcan.c |   40 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index fbb61c6..96684ca 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -316,6 +316,38 @@ static inline unsigned long flexcan_clk_get_rate(struct clk *clk)
 
 #endif
 
+#if defined(FLEXCAN_DEBUG)
+void _flexcan_reg_dump(struct net_device *dev, const char *file, int line,
+		       const char *func)
+{
+	const struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+
+	printk(KERN_INFO "flexcan_reg_dump:%s:%d:%s()\n", file, line, func);
+	printk(KERN_INFO
+		"\t  mcr 0x%08x  ctrl 0x%08x timer 0x%08x   rxg 0x%08x",
+		flexcan_read(&regs->mcr),
+		flexcan_read(&regs->ctrl),
+		flexcan_read(&regs->timer),
+		flexcan_read(&regs->rxgmask));
+	printk(KERN_INFO
+		"\t rx14 0x%08x  rx15 0x%08x   ecr 0x%08x   esr 0x%08x",
+		flexcan_read(&regs->rx14mask),
+		flexcan_read(&regs->rx15mask),
+		flexcan_read(&regs->ecr),
+		flexcan_read(&regs->esr));
+	printk(KERN_INFO
+		"\timsk2 0x%08x imsk1 0x%08x iflg2 0x%08x iflg1 0x%08x",
+		flexcan_read(&regs->imask2),
+		flexcan_read(&regs->imask1),
+		flexcan_read(&regs->iflag2),
+		flexcan_read(&regs->iflag1));
+}
+#define flexcan_reg_dump(_d) _flexcan_reg_dump(_d, __FILE__, __LINE__, __func__)
+#else
+#define flexcan_reg_dump(_d)
+#endif
+
 /*
  * Swtich transceiver on or off
  */
@@ -376,6 +408,8 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	u32 can_id;
 	u32 ctrl = FLEXCAN_MB_CNT_CODE(0xc) | (cf->can_dlc << 16);
 
+	flexcan_reg_dump(dev);
+
 	if (can_dropped_invalid_skb(dev, skb))
 		return NETDEV_TX_OK;
 
@@ -408,6 +442,8 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* tx_packets is incremented in flexcan_irq */
 	stats->tx_bytes += cf->can_dlc;
 
+	flexcan_reg_dump(dev);
+
 	return NETDEV_TX_OK;
 }
 
@@ -676,6 +712,8 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg_iflag1, reg_esr;
 
+	flexcan_reg_dump(dev);
+
 	reg_iflag1 = flexcan_read(&regs->iflag1);
 	reg_esr = flexcan_read(&regs->esr);
 	flexcan_write(FLEXCAN_ESR_ERR_INT, &regs->esr);	/* ACK err IRQ */
@@ -716,6 +754,8 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 		netif_wake_queue(dev);
 	}
 
+	flexcan_reg_dump(dev);
+
 	return IRQ_HANDLED;
 }
 
-- 
1.7.2.1


^ permalink raw reply related

* [RFC 2/4] [flexcan] Introduce a flexcan_clk set of functions.
From: Robin Holt @ 2011-08-05  2:06 UTC (permalink / raw)
  To: Robin Holt, Marc Kleine-Budde, Wolfgang Grandegger
  Cc: Robin Holt, socketcan-core, netdev
In-Reply-To: <1312509979-13226-1-git-send-email-holt@sgi.com>

The freescale P1010RDB board does not have a
clk_{get,put,get_rate,enable,disable} set of functions.  Wrap these with a
flexcan_ #define for arm, and implement a more complete function for ppc.

Signed-off-by: Robin Holt <holt@sgi.com>
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Wolfgang Grandegger <wg@grandegger.com>
Cc: socketcan-core@lists.berlios.de
Cc: netdev@vger.kernel.org
---
 drivers/net/can/flexcan.c |  114 +++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 105 insertions(+), 9 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 74b1706..3417d0b 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -220,6 +220,102 @@ static inline void flexcan_write(u32 val, void __iomem *addr)
 }
 #endif
 
+#if defined(__powerpc__)
+struct flexcan_clk {
+	unsigned long rate;
+	void *data;
+};
+
+static struct clk *flexcan_clk_get(struct device *dev, const char *id)
+{
+	struct flexcan_clk *clock;
+	u32 *clock_freq;
+	u32 *clock_divider;
+	int err;
+
+	clock = kmalloc(sizeof(struct flexcan_clk), GFP_KERNEL);
+	if (!clock) {
+		dev_err(dev, "Cannot allocate memory\n");
+		err = -ENOMEM;
+		goto failed_clock;
+	}
+	clock_freq = (u32 *)of_get_property(dev->of_node, "clock_freq", NULL);
+	if (!clock_freq) {
+		dev_err(dev, "Cannot find clock_freq property\n");
+		err = -EINVAL;
+		goto failed_clock;
+	}
+
+	clock_divider = (u32 *) of_get_property(dev->of_node,
+					"fsl,flexcan-clock-divider", NULL);
+	if (clock_divider == NULL) {
+		dev_err(dev, "Cannot find fsl,flexcan-clock-divider property\n");
+		err = -EINVAL;
+		goto failed_clock;
+	}
+
+	clock->rate = DIV_ROUND_CLOSEST(*clock_freq / *clock_divider, 1000);
+	clock->rate *= 1000;
+
+	return (struct clk *)clock;
+
+ failed_clock:
+	kfree(clock);
+	return ERR_PTR(err);
+}
+
+static inline void flexcan_clk_put(struct clk *_clk)
+{
+	struct flexcan_clk *clk = (struct flexcan_clk *)_clk;
+
+	kfree(clk);
+}
+
+static inline int flexcan_clk_enable(struct clk *clk)
+{
+	return 0;
+}
+
+static inline void flexcan_clk_disable(struct clk *clk)
+{
+	return;
+}
+
+static inline unsigned long flexcan_clk_get_rate(struct clk *_clk)
+{
+	struct flexcan_clk *clk = (struct flexcan_clk *)_clk;
+
+	return clk->rate;
+}
+
+#else
+static inline struct clk *flexcan_clk_get(struct device *dev, const char *id)
+{
+	return clk_get(dev, id);
+}
+
+static inline void flexcan_clk_put(struct clk *clk)
+{
+	clk_put(clk);
+}
+
+static inline int flexcan_clk_enable(struct clk *clk)
+{
+	return clk_enable(clk);
+}
+
+static inline void flexcan_clk_disable(struct clk *clk)
+{
+	clk_disable(clk);
+}
+
+static inline unsigned long flexcan_clk_get_rate(struct clk *clk)
+{
+	return clk_get_rate(clk);
+}
+
+#endif
+
 /*
  * Swtich transceiver on or off
  */
@@ -807,7 +903,7 @@ static int flexcan_open(struct net_device *dev)
 	struct flexcan_priv *priv = netdev_priv(dev);
 	int err;
 
-	clk_enable(priv->clk);
+	flexcan_clk_enable(priv->clk);
 
 	err = open_candev(dev);
 	if (err)
@@ -829,7 +925,7 @@ static int flexcan_open(struct net_device *dev)
  out_close:
 	close_candev(dev);
  out:
-	clk_disable(priv->clk);
+	flexcan_clk_disable(priv->clk);
 
 	return err;
 }
@@ -843,7 +939,7 @@ static int flexcan_close(struct net_device *dev)
 	flexcan_chip_stop(dev);
 
 	free_irq(dev->irq, dev);
-	clk_disable(priv->clk);
+	flexcan_clk_disable(priv->clk);
 
 	close_candev(dev);
 
@@ -882,7 +978,7 @@ static int __devinit register_flexcandev(struct net_device *dev)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg, err;
 
-	clk_enable(priv->clk);
+	flexcan_clk_enable(priv->clk);
 
 	/* select "bus clock", chip must be disabled */
 	flexcan_chip_disable(priv);
@@ -916,7 +1012,7 @@ static int __devinit register_flexcandev(struct net_device *dev)
  out:
 	/* disable core and turn off clocks */
 	flexcan_chip_disable(priv);
-	clk_disable(priv->clk);
+	flexcan_clk_disable(priv->clk);
 
 	return err;
 }
@@ -936,7 +1032,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	resource_size_t mem_size;
 	int err, irq;
 
-	clk = clk_get(&pdev->dev, NULL);
+	clk = flexcan_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(clk)) {
 		dev_err(&pdev->dev, "no clock defined\n");
 		err = PTR_ERR(clk);
@@ -973,7 +1069,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	dev->flags |= IFF_ECHO; /* we support local echo in hardware */
 
 	priv = netdev_priv(dev);
-	priv->can.clock.freq = clk_get_rate(clk);
+	priv->can.clock.freq = flexcan_clk_get_rate(clk);
 	priv->can.bittiming_const = &flexcan_bittiming_const;
 	priv->can.do_set_mode = flexcan_set_mode;
 	priv->can.do_get_berr_counter = flexcan_get_berr_counter;
@@ -1008,7 +1104,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
  failed_map:
 	release_mem_region(mem->start, mem_size);
  failed_get:
-	clk_put(clk);
+	flexcan_clk_put(clk);
  failed_clock:
 	return err;
 }
@@ -1026,7 +1122,7 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(mem->start, resource_size(mem));
 
-	clk_put(priv->clk);
+	flexcan_clk_put(priv->clk);
 
 	free_candev(dev);
 
-- 
1.7.2.1


^ permalink raw reply related


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