Re: can: make the number of echo skb's configurable

[Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]

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

Wolfgang Grandegger wrote:
> This patch allows the CAN controller driver to define the number of echo
> skb's used for the local loopback (echo), as suggested by Kurt Van
> Dijck, with the function:
> 
>   struct net_device *alloc_candev(int sizeof_priv,
>                                   unsigned int echo_skb_max);
> 
> The CAN drivers have been adapted accordingly. For the ems_usb driver,
> as suggested by Sebastian Haas, the number of echo skb's has been
> increased to 10, which improves the transmission performance a lot.
> 
> Signed-off-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> Signed-off-by: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public.gmane.org>
> ---
>  drivers/net/can/at91_can.c        |    2 +-
>  drivers/net/can/dev.c             |   32 ++++++++++++++++++++++++++------
>  drivers/net/can/sja1000/sja1000.c |    3 ++-
>  drivers/net/can/sja1000/sja1000.h |    2 ++
>  drivers/net/can/ti_hecc.c         |    6 +-----
>  drivers/net/can/usb/ems_usb.c     |    4 ++--
>  include/linux/can/dev.h           |   16 ++++++++--------
>  7 files changed, 42 insertions(+), 23 deletions(-)
> 
> Index: net-next-2.6/drivers/net/can/dev.c
> ===================================================================
> --- net-next-2.6.orig/drivers/net/can/dev.c
> +++ net-next-2.6/drivers/net/can/dev.c
> @@ -245,7 +245,7 @@ static void can_flush_echo_skb(struct ne
>  	struct net_device_stats *stats = &dev->stats;
>  	int i;
>  
> -	for (i = 0; i < CAN_ECHO_SKB_MAX; i++) {
> +	for (i = 0; i < priv->echo_skb_max; i++) {
>  		if (priv->echo_skb[i]) {
>  			kfree_skb(priv->echo_skb[i]);
>  			priv->echo_skb[i] = NULL;
> @@ -262,10 +262,13 @@ static void can_flush_echo_skb(struct ne
>   * of the device driver. The driver must protect access to
>   * priv->echo_skb, if necessary.
>   */
> -void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev, int idx)
> +void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
> +		      unsigned int idx)
>  {
>  	struct can_priv *priv = netdev_priv(dev);
>  
> +	BUG_ON(idx >= priv->echo_skb_max);
> +
>  	/* check flag whether this packet has to be looped back */
>  	if (!(dev->flags & IFF_ECHO) || skb->pkt_type != PACKET_LOOPBACK) {
>  		kfree_skb(skb);
> @@ -311,10 +314,12 @@ EXPORT_SYMBOL_GPL(can_put_echo_skb);
>   * is handled in the device driver. The driver must protect
>   * access to priv->echo_skb, if necessary.
>   */
> -void can_get_echo_skb(struct net_device *dev, int idx)
> +void can_get_echo_skb(struct net_device *dev, unsigned int idx)
>  {
>  	struct can_priv *priv = netdev_priv(dev);
>  
> +	BUG_ON(idx >= priv->echo_skb_max);
> +
>  	if (priv->echo_skb[idx]) {
>  		netif_rx(priv->echo_skb[idx]);
>  		priv->echo_skb[idx] = NULL;
> @@ -327,10 +332,12 @@ EXPORT_SYMBOL_GPL(can_get_echo_skb);
>    *
>    * The function is typically called when TX failed.
>    */
> -void can_free_echo_skb(struct net_device *dev, int idx)
> +void can_free_echo_skb(struct net_device *dev, unsigned int idx)
>  {
>  	struct can_priv *priv = netdev_priv(dev);
>  
> +	BUG_ON(idx >= priv->echo_skb_max);
> +
>  	if (priv->echo_skb[idx]) {
>  		kfree_skb(priv->echo_skb[idx]);
>  		priv->echo_skb[idx] = NULL;
> @@ -445,17 +452,30 @@ static void can_setup(struct net_device 
>  /*
>   * Allocate and setup space for the CAN network device
>   */
> -struct net_device *alloc_candev(int sizeof_priv)
> +struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max)
>  {
>  	struct net_device *dev;
>  	struct can_priv *priv;
> +	int size;
>  
> -	dev = alloc_netdev(sizeof_priv, "can%d", can_setup);
> +	if (echo_skb_max)
> +		size = ALIGN(sizeof_priv, sizeof(struct sk_buff *)) +
> +			echo_skb_max * sizeof(struct sk_buff *);
> +	else
> +		size = sizeof_priv;
> +
> +	dev = alloc_netdev(size, "can%d", can_setup);
>  	if (!dev)
>  		return NULL;
>  
>  	priv = netdev_priv(dev);
>  
> +	if (echo_skb_max) {
> +		priv->echo_skb_max = echo_skb_max;
> +		priv->echo_skb = (void *)priv +
> +			ALIGN(sizeof_priv, sizeof(struct sk_buff *));
> +	}
> +
>  	priv->state = CAN_STATE_STOPPED;
>  
>  	init_timer(&priv->restart_timer);
> Index: net-next-2.6/include/linux/can/dev.h
> ===================================================================
> --- net-next-2.6.orig/include/linux/can/dev.h
> +++ net-next-2.6/include/linux/can/dev.h
> @@ -29,8 +29,6 @@ enum can_mode {
>  /*
>   * CAN common private data
>   */
> -#define CAN_ECHO_SKB_MAX  4
> -
>  struct can_priv {
>  	struct can_device_stats can_stats;
>  
> @@ -44,15 +42,16 @@ struct can_priv {
>  	int restart_ms;
>  	struct timer_list restart_timer;
>  
> -	struct sk_buff *echo_skb[CAN_ECHO_SKB_MAX];
> -
>  	int (*do_set_bittiming)(struct net_device *dev);
>  	int (*do_set_mode)(struct net_device *dev, enum can_mode mode);
>  	int (*do_get_state)(const struct net_device *dev,
>  			    enum can_state *state);
> +
> +	unsigned int echo_skb_max;
> +	struct sk_buff **echo_skb;
>  };
>  
> -struct net_device *alloc_candev(int sizeof_priv);
> +struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max);
>  void free_candev(struct net_device *dev);
>  
>  int open_candev(struct net_device *dev);
> @@ -64,8 +63,9 @@ void unregister_candev(struct net_device
>  int can_restart_now(struct net_device *dev);
>  void can_bus_off(struct net_device *dev);
>  
> -void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev, int idx);
> -void can_get_echo_skb(struct net_device *dev, int idx);
> -void can_free_echo_skb(struct net_device *dev, int idx);
> +void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
> +		      unsigned int idx);
> +void can_get_echo_skb(struct net_device *dev, unsigned int idx);
> +void can_free_echo_skb(struct net_device *dev, unsigned int idx);
>  
>  #endif /* CAN_DEV_H */
> Index: net-next-2.6/drivers/net/can/at91_can.c
> ===================================================================
> --- net-next-2.6.orig/drivers/net/can/at91_can.c
> +++ net-next-2.6/drivers/net/can/at91_can.c
> @@ -1087,7 +1087,7 @@ static int __init at91_can_probe(struct 
>  		goto exit_release;
>  	}
>  
> -	dev = alloc_candev(sizeof(struct at91_priv));
> +	dev = alloc_candev(sizeof(struct at91_priv), AT91_MB_TX_NUM);
>  	if (!dev) {
>  		err = -ENOMEM;
>  		goto exit_iounmap;

The at91 part looks okay.

Marc

-- 
Pengutronix e.K.                         | Marc Kleine-Budde           |
Linux Solutions for Science and Industry | 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: 260 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