[...]
> > I was in the process of preparing a patch with alternate approach.
>
> I started to implement the alloc_candev(int sizeof_priv, int
> echo_skb_max), which is quit simple. Any alternate approach is welcome,
> of course.
>
By trying that, I proved myself being wrong.
adding an extra function call keeps the existing function proper,
but in the end, drivers just need updating
Trying your approach in the end is
simpler in coding, just extra docs.
I attached my work, maybe you find it usefull :-).
it compiles, I _believe_ it would run.
[...]
>
> Currently we use 4 echo skbs and for that amount it was not worth to
> implement a more sophisticated approach. So far, nobody proved that
> there is a benefit of using a higher number and for that reason I was
> not willing to change it. For devices with slow queuing, like USB I see
> some benefit and Sebastian provided even figures.
Ack. I definitely understand that 4 is not worth the pain, and 16 does.
>
> Wolfgang.
Index: include/linux/can/dev.h
===================================================================
--- include/linux/can/dev.h (revision 1053)
+++ include/linux/can/dev.h (working copy)
@@ -32,8 +32,6 @@
/*
* CAN common private data
*/
-#define CAN_ECHO_SKB_MAX 4
-
struct can_priv {
#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,23)
struct net_device_stats net_stats;
@@ -50,7 +48,8 @@
int restart_ms;
struct timer_list restart_timer;
- struct sk_buff *echo_skb[CAN_ECHO_SKB_MAX];
+ struct sk_buff **echo_skb;
+ int echo_skb_max;
int (*do_set_bittiming)(struct net_device *dev);
int (*do_set_mode)(struct net_device *dev, enum can_mode mode);
@@ -72,7 +71,7 @@
struct net_device_stats *can_get_stats(struct net_device *dev);
#endif
-struct net_device *alloc_candev(int sizeof_priv);
+struct net_device *alloc_candev(int sizeof_priv, int echo_skb_max);
void free_candev(struct net_device *dev);
int open_candev(struct net_device *dev);
Index: include/socketcan/can/dev.h
===================================================================
--- include/socketcan/can/dev.h (revision 1053)
+++ include/socketcan/can/dev.h (working copy)
@@ -32,8 +32,6 @@
/*
* CAN common private data
*/
-#define CAN_ECHO_SKB_MAX 4
-
struct can_priv {
#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,23)
struct net_device_stats net_stats;
@@ -50,7 +48,8 @@
int restart_ms;
struct timer_list restart_timer;
- struct sk_buff *echo_skb[CAN_ECHO_SKB_MAX];
+ struct sk_buff **echo_skb;
+ int echo_skb_max;
int (*do_set_bittiming)(struct net_device *dev);
int (*do_set_mode)(struct net_device *dev, enum can_mode mode);
@@ -72,7 +71,7 @@
struct net_device_stats *can_get_stats(struct net_device *dev);
#endif
-struct net_device *alloc_candev(int sizeof_priv);
+struct net_device *alloc_candev(int sizeof_priv, int echo_skb_max);
void free_candev(struct net_device *dev);
int open_candev(struct net_device *dev);
Index: drivers/net/can/at91_can.c
===================================================================
--- drivers/net/can/at91_can.c (revision 1053)
+++ drivers/net/can/at91_can.c (working copy)
@@ -1025,7 +1025,11 @@
goto exit_release;
}
- dev = alloc_candev(sizeof(struct at91_priv));
+ /*
+ * I think at91_CAN can use up to 16 tx buffs
+ * can someone verify
+ */
+ dev = alloc_candev(sizeof(struct at91_priv), 16);
if (!dev) {
err = -ENOMEM;
goto exit_iounmap;
Index: drivers/net/can/mcp251x.c
===================================================================
--- drivers/net/can/mcp251x.c (revision 1053)
+++ drivers/net/can/mcp251x.c (working copy)
@@ -941,7 +941,7 @@
struct net_device *net;
struct mcp251x_priv *priv;
- net = alloc_candev(sizeof_priv);
+ net = alloc_candev(sizeof_priv, 1);
if (!net)
return NULL;
Index: drivers/net/can/cc770/cc770.c
===================================================================
--- drivers/net/can/cc770/cc770.c (revision 1053)
+++ drivers/net/can/cc770/cc770.c (working copy)
@@ -854,7 +854,7 @@
struct net_device *dev;
struct cc770_priv *priv;
- dev = alloc_candev(sizeof(struct cc770_priv) + sizeof_priv);
+ dev = alloc_candev(sizeof(struct cc770_priv) + sizeof_priv, 1);
if (!dev)
return NULL;
Index: drivers/net/can/dev.c
===================================================================
--- drivers/net/can/dev.c (revision 1053)
+++ drivers/net/can/dev.c (working copy)
@@ -272,7 +272,7 @@
#endif
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;
@@ -298,6 +298,12 @@
kfree_skb(skb);
return;
}
+ if (idx > priv->echo_skb_max) {
+ dev_err(ND2D(dev), "%s: BUG! echo_skb[%i] requested"
+ ", %i available\n", __func__, idx, priv->echo_skb_max);
+ kfree_skb(skb);
+ return;
+ }
if (!priv->echo_skb[idx]) {
struct sock *srcsk = skb->sk;
@@ -342,6 +348,11 @@
{
struct can_priv *priv = netdev_priv(dev);
+ if (idx > priv->echo_skb_max) {
+ dev_err(ND2D(dev), "%s: BUG! echo_skb[%i] requested"
+ ", %i available\n", __func__, idx, priv->echo_skb_max);
+ return;
+ }
if (priv->echo_skb[idx]) {
netif_rx(priv->echo_skb[idx]);
priv->echo_skb[idx] = NULL;
@@ -358,6 +369,11 @@
{
struct can_priv *priv = netdev_priv(dev);
+ if (idx > priv->echo_skb_max) {
+ dev_err(ND2D(dev), "%s: BUG! echo_skb[%i] requested"
+ ", %i available\n", __func__, idx, priv->echo_skb_max);
+ return;
+ }
if (priv->echo_skb[idx]) {
kfree_skb(priv->echo_skb[idx]);
priv->echo_skb[idx] = NULL;
@@ -366,6 +382,36 @@
EXPORT_SYMBOL_GPL(can_free_echo_skb);
/*
+ * allocate a echo_skb
+ *
+ * The function is typically called shortly afer alloc_candev
+ */
+int candev_set_echo_skb_max(struct net_device *dev, int max)
+{
+ struct can_priv *priv = netdev_priv(dev);
+ struct sk_buff **skbs;
+
+ if (max <= 0)
+ return -EINVAL;
+ /*
+ * don't do this while running
+ */
+ if (dev->flags & IFF_UP)
+ return -EBUSY;
+ skbs = krealloc(priv->echo_skb, max * sizeof(skbs[0]), GFP_KERNEL);
+ if (!skbs)
+ return -ENOMEM;
+ // clean the extra space
+ if (max > priv->echo_skb_max)
+ memset(&skbs[priv->echo_skb_max], 0,
+ sizeof(skbs[0])*(max - priv->echo_skb_max));
+ priv->echo_skb_max = max;
+ priv->echo_skb = skbs;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(candev_set_echo_skb_max);
+
+/*
* CAN device restart for bus-off recovery
*/
void can_restart(unsigned long data)
@@ -490,12 +536,17 @@
/*
* 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, int echo_skb_max)
{
struct net_device *dev;
struct can_priv *priv;
+ int echo_skb_size;
- dev = alloc_netdev(sizeof_priv, "can%d", can_setup);
+ if (echo_skb_max <= 0)
+ return NULL;
+
+ echo_skb_size = sizeof(priv->echo_skb[0]) * echo_skb_max;
+ dev = alloc_netdev(sizeof_priv + echo_skb_size, "can%d", can_setup);
if (!dev)
return NULL;
@@ -503,6 +554,12 @@
priv->state = CAN_STATE_STOPPED;
+ /*
+ * prepare echo_skb array, behind netdev_priv
+ */
+ priv->echo_skb = &((char *)priv)[sizeof_priv];
+ priv->echo_skb_max = echo_skb_max;
+
init_timer(&priv->restart_timer);
return dev;
@@ -514,6 +571,7 @@
*/
void free_candev(struct net_device *dev)
{
+ struct can_priv *priv = netdev_priv(dev);
free_netdev(dev);
}
EXPORT_SYMBOL_GPL(free_candev);
Index: drivers/net/can/softing/softing_main.c
===================================================================
--- drivers/net/can/softing/softing_main.c (revision 1053)
+++ drivers/net/can/softing/softing_main.c (working copy)
@@ -69,7 +69,7 @@
goto xmit_done;
if (card->tx.pending >= TXMAX)
goto xmit_done;
- if (priv->tx.pending >= CAN_ECHO_SKB_MAX)
+ if (priv->tx.pending >= priv->can.echo_skb_max)
goto xmit_done;
fifo_wr = card->dpram.tx->wr;
if (fifo_wr == card->dpram.tx->rd)
@@ -107,7 +107,7 @@
++priv->tx.pending;
can_put_echo_skb(skb, dev, priv->tx.echo_put);
++priv->tx.echo_put;
- if (priv->tx.echo_put >= CAN_ECHO_SKB_MAX)
+ if (priv->tx.echo_put >= priv->can.echo_skb_max)
priv->tx.echo_put = 0;
/* can_put_echo_skb() saves the skb, safe to return TX_OK */
ret = NETDEV_TX_OK;
@@ -319,7 +319,7 @@
skb->tstamp = ktime;
can_get_echo_skb(bus->netdev, bus->tx.echo_get);
++bus->tx.echo_get;
- if (bus->tx.echo_get >= CAN_ECHO_SKB_MAX)
+ if (bus->tx.echo_get >= bus->can.echo_skb_max)
bus->tx.echo_get = 0;
if (bus->tx.pending)
--bus->tx.pending;
@@ -360,7 +360,7 @@
if (!canif_is_active(bus->netdev))
/* it makes no sense to wake dead busses */
continue;
- if (bus->tx.pending >= CAN_ECHO_SKB_MAX)
+ if (bus->tx.pending >= bus->can.echo_skb_max)
continue;
netif_wake_queue(bus->netdev);
}
@@ -625,7 +625,7 @@
struct net_device *ndev;
struct softing_priv *priv;
- ndev = alloc_candev(sizeof(*priv));
+ ndev = alloc_candev(sizeof(*priv), 8);
if (!ndev) {
dev_alert(card->dev, "alloc_candev failed\n");
return 0;
Index: drivers/net/can/sja1000/sja1000.c
===================================================================
--- drivers/net/can/sja1000/sja1000.c (revision 1053)
+++ drivers/net/can/sja1000/sja1000.c (working copy)
@@ -596,7 +596,7 @@
struct net_device *dev;
struct sja1000_priv *priv;
- dev = alloc_candev(sizeof(struct sja1000_priv) + sizeof_priv);
+ dev = alloc_candev(sizeof(struct sja1000_priv) + sizeof_priv, 1);
if (!dev)
return NULL;
Index: drivers/net/can/esd_pci331.c
===================================================================
--- drivers/net/can/esd_pci331.c (revision 1053)
+++ drivers/net/can/esd_pci331.c (working copy)
@@ -776,7 +776,7 @@
struct esd331_priv *priv;
int err;
- dev = alloc_candev(sizeof(*priv));
+ dev = alloc_candev(sizeof(*priv), 1);
if (dev == NULL)
return ERR_PTR(-ENOMEM);
Index: drivers/net/can/ems_usb.c
===================================================================
--- drivers/net/can/ems_usb.c (revision 1053)
+++ drivers/net/can/ems_usb.c (working copy)
@@ -226,7 +226,7 @@
#define CPC_HEADER_SIZE 4
#define MAX_RX_URBS 10
-#define MAX_TX_URBS CAN_ECHO_SKB_MAX
+#define MAX_TX_URBS 10
struct ems_usb;
@@ -241,8 +241,6 @@
struct can_priv can; /* must be the first member */
int open_time;
- struct sk_buff *echo_skb[MAX_TX_URBS];
-
struct usb_device *udev;
struct net_device *netdev;
@@ -1009,7 +1007,7 @@
struct ems_usb *dev;
int i, err;
- netdev = alloc_candev(sizeof(struct ems_usb));
+ netdev = alloc_candev(sizeof(struct ems_usb), MAX_TX_URBS);
if (!netdev) {
dev_err(ND2D(netdev), "Couldn't alloc candev\n");
return -ENOMEM;
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
