* Re: [PATCH 1/2] net: Allow to create links with given ifindex
From: Eric Dumazet @ 2012-07-30 11:51 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: Pavel Emelyanov, Linux Netdev List, David Miller
In-Reply-To: <87pq7dh6b2.fsf@xmission.com>
On Mon, 2012-07-30 at 03:49 -0700, Eric W. Biederman wrote:
> Pavel Emelyanov <xemul@parallels.com> writes:
>
> > Currently the RTM_NEWLINK results in -EOPNOTSUPP if the ifinfomsg->ifi_index
> > is not zero. I propose to allow requesting ifindices on link creation. This
> > is required by the checkpoint-restore to correctly restore a net namespace
> > (i.e. -- a container). The question what to do with pre-created devices such
> > as lo or sit fbdev is open, but for manually created devices this can be
> > solved by this patch.
>
> Have you walked through and found the locations where we still rely on
> ifindex being globally unique?
>
> Last time I was working in this area there were serveral places where
> things were indexed by just the interface index.
Really ? This would be very strange.
AFAIK dev_new_index() is always called, even in the
dev_change_net_namespace() case if there is a conflict.
And dev_new_index() could use a pernet net->ifindex instead of a
shared/static one.
^ permalink raw reply
* Re: [net-next,v3] GRE over IPv6
From: Timo Teras @ 2012-07-30 11:38 UTC (permalink / raw)
To: xeb; +Cc: netdev
In-Reply-To: <1368610.TAsHNfv2Eb@comp1>
On Sat, 28 Jul 2012 22:12:42 -0000 xeb@mail.ru wrote:
> GRE over IPv6 implementation.
>
> Signed-off-by: Dmitry Kozlov <xeb@mail.ru>
>
> ---
> Changes:
> Initialize nt->dev before calling ip6gre_tnl_link_config in
> ip6gre_newlink.
> Add missing ip6gre.c
>
> include/linux/if_arp.h | 1 +
> include/linux/if_tunnel.h | 3 +
> include/linux/ip6_tunnel.h | 18 +
> include/net/ip6_tunnel.h | 40 +-
> include/net/ipv6.h | 1 +
> net/ipv6/Kconfig | 16 +
> net/ipv6/Makefile | 1 +
> net/ipv6/ip6_gre.c | 1817
> ++++++++++++++++++++++++++++++++++++++++++++
> net/ipv6/ip6_tunnel.c | 86 ++- 9 files changed, 1958
> insertions(+), 25 deletions(-)
Would it be possible and/or feasible to instead modify ip_gre to support
also ipv6 as outer protocol?
It already has ipv6 stuff in it for the inner protocol support. And it
would avoid duplicating most of the code.
And I would especially love that approach, since I could then on
per-target basis say if it should be contacted with IPv4 or IPv6. As in:
ip addr add 10.0.0.1/24 dev gre1
ip neigh add 10.0.0.2 lladdr 192.168.x.x dev gre1 nud permanent
ip neigh add 10.0.0.3 lladdr fe80::xxxx:xxxx:xxxx:xxxx/64 dev gre1 nud permanent
- Timo
^ permalink raw reply
* Re: [PATCH] can: kvaser_usb: Add support for Kvaser CAN/USB devices
From: Marc Kleine-Budde @ 2012-07-30 11:11 UTC (permalink / raw)
To: Olivier Sobrie; +Cc: Wolfgang Grandegger, linux-can, netdev
In-Reply-To: <1343626352-24760-1-git-send-email-olivier@sobrie.be>
[-- Attachment #1: Type: text/plain, Size: 43005 bytes --]
On 07/30/2012 07:32 AM, Olivier Sobrie wrote:
> This driver provides support for several Kvaser CAN/USB devices.
> Such kind of devices supports up to three can network interfaces.
>
> It has been tested with a Kvaser USB Leaf Light (one network interface)
> connected to a pch_can interface.
> The firmware version of the Kvaser device was 2.5.205.
Please add linux-usb@vger.kernel.org to Cc for review of the USB part.
Please combine .h and .c file. Please make use of netdev_LEVEL() for
error printing, not dev_LEVEL().
Please review if all members of the struct kvaser_msg are properly
aligned. You never access the struct kvaser_msg_* members directly, as
they are unaligned. Please check for le16 and le32 access. You missed to
convert the bitrate.
Please check if your driver survives hot-unplugging while sending and
receiving CAN frames at maximum laod.
More comments inline,
regards, Marc
> List of Kvaser devices supported by the driver:
> - Kvaser Leaf prototype (P010v2 and v3)
> - Kvaser Leaf Light (P010v3)
> - Kvaser Leaf Professional HS
> - Kvaser Leaf SemiPro HS
> - Kvaser Leaf Professional LS
> - Kvaser Leaf Professional SWC
> - Kvaser Leaf Professional LIN
> - Kvaser Leaf SemiPro LS
> - Kvaser Leaf SemiPro SWC
> - Kvaser Memorator II, Prototype
> - Kvaser Memorator II HS/HS
> - Kvaser USBcan Professional HS/HS
> - Kvaser Leaf Light GI
> - Kvaser Leaf Professional HS (OBD-II connector)
> - Kvaser Memorator Professional HS/LS
> - Kvaser Leaf Light "China"
> - Kvaser BlackBird SemiPro
> - Kvaser OEM Mercury
> - Kvaser OEM Leaf
> - Kvaser USBcan R
>
> Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
> ---
> drivers/net/can/usb/Kconfig | 33 ++
> drivers/net/can/usb/Makefile | 1 +
> drivers/net/can/usb/kvaser_usb.c | 1062 ++++++++++++++++++++++++++++++++++++++
> drivers/net/can/usb/kvaser_usb.h | 237 +++++++++
> 4 files changed, 1333 insertions(+)
> create mode 100644 drivers/net/can/usb/kvaser_usb.c
> create mode 100644 drivers/net/can/usb/kvaser_usb.h
>
> diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
> index 0a68768..578955f 100644
> --- a/drivers/net/can/usb/Kconfig
> +++ b/drivers/net/can/usb/Kconfig
> @@ -13,6 +13,39 @@ config CAN_ESD_USB2
> This driver supports the CAN-USB/2 interface
> from esd electronic system design gmbh (http://www.esd.eu).
>
> +config CAN_KVASER_USB
> + tristate "Kvaser CAN/USB interface"
> + ---help---
> + This driver adds support for Kvaser CAN/USB devices like Kvaser
> + Leaf Light.
> +
> + The driver gives support for the following devices:
> + - Kvaser Leaf prototype (P010v2 and v3)
> + - Kvaser Leaf Light (P010v3)
> + - Kvaser Leaf Professional HS
> + - Kvaser Leaf SemiPro HS
> + - Kvaser Leaf Professional LS
> + - Kvaser Leaf Professional SWC
> + - Kvaser Leaf Professional LIN
> + - Kvaser Leaf SemiPro LS
> + - Kvaser Leaf SemiPro SWC
> + - Kvaser Memorator II, Prototype
> + - Kvaser Memorator II HS/HS
> + - Kvaser USBcan Professional HS/HS
> + - Kvaser Leaf Light GI
> + - Kvaser Leaf Professional HS (OBD-II connector)
> + - Kvaser Memorator Professional HS/LS
> + - Kvaser Leaf Light "China"
> + - Kvaser BlackBird SemiPro
> + - Kvaser OEM Mercury
> + - Kvaser OEM Leaf
> + - Kvaser USBcan R
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called kvaser_usb.
> +
> config CAN_PEAK_USB
> tristate "PEAK PCAN-USB/USB Pro interfaces"
> ---help---
> diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
> index da6d1d3..80a2ee4 100644
> --- a/drivers/net/can/usb/Makefile
> +++ b/drivers/net/can/usb/Makefile
> @@ -4,6 +4,7 @@
>
> obj-$(CONFIG_CAN_EMS_USB) += ems_usb.o
> obj-$(CONFIG_CAN_ESD_USB2) += esd_usb2.o
> +obj-$(CONFIG_CAN_KVASER_USB) += kvaser_usb.o
> obj-$(CONFIG_CAN_PEAK_USB) += peak_usb/
>
> ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> new file mode 100644
> index 0000000..4965480
> --- /dev/null
> +++ b/drivers/net/can/usb/kvaser_usb.c
> @@ -0,0 +1,1062 @@
> +/*
Please add a license statement and probably your copyright:
* 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.
You also should copy the copyright from the drivers you used:
> + * Parts of this driver are based on the following:
> + * - Kvaser linux leaf driver (version 4.78)
I just downloaded their driver and noticed that it's quite sparse on
stating the license the code is released under.
"doc/HTMLhelp/copyright.htx" is quite restrictive, the word GPL occurs 3
times, all in MODULE_LICENSE("GPL"). Running modinfo on the usbcan.ko
shows "license: GPL"
> + * - CAN driver for esd CAN-USB/2
> + */
> +
> +#include <linux/init.h>
> +#include <linux/completion.h>
> +#include <linux/module.h>
> +#include <linux/netdevice.h>
> +#include <linux/usb.h>
> +
> +#include <linux/can.h>
> +#include <linux/can/dev.h>
> +#include <linux/can/error.h>
> +
> +#include "kvaser_usb.h"
> +
> +struct kvaser_usb_tx_urb_context {
> + struct kvaser_usb_net_priv *priv;
Huh - how does this work without forward declaration?
> + u32 echo_index;
> + int dlc;
> +};
> +
> +struct kvaser_usb {
> + struct usb_device *udev;
> + struct kvaser_usb_net_priv *nets[MAX_NET_DEVICES];
> +
> + struct usb_anchor rx_submitted;
> +
> + u32 fw_version;
> + unsigned int nchannels;
> +
> + bool rxinitdone;
> +};
> +
> +struct kvaser_usb_net_priv {
> + struct can_priv can;
> +
> + atomic_t active_tx_urbs;
> + struct usb_anchor tx_submitted;
> + struct kvaser_usb_tx_urb_context tx_contexts[MAX_TX_URBS];
> +
> + int open_time;
please remove open_time
> + struct completion start_stop_comp;
> +
> + struct kvaser_usb *dev;
> + struct net_device *netdev;
> + int channel;
> + struct can_berr_counter bec;
> +};
> +
> +static struct usb_device_id kvaser_usb_table[] = {
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_DEVEL_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LS_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_SWC_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LIN_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_LS_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_SWC_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_DEVEL_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSHS_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_UPRO_HSHS_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_GI_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_OBDII_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSLS_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_CH_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_BLACKBIRD_SPRO_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_MERCURY_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_LEAF_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_CAN_R_PRODUCT_ID) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(usb, kvaser_usb_table);
> +
> +static int kvaser_usb_send_msg(const struct kvaser_usb *dev,
> + struct kvaser_msg *msg)
inline?
> +{
> + int actual_len;
> +
> + return usb_bulk_msg(dev->udev, usb_sndbulkpipe(dev->udev, 1),
^
Can you please introduce a #define for this.
> + msg, msg->len, &actual_len, USB_SEND_TIMEOUT);
> +}
> +
> +static int kvaser_usb_wait_msg(const struct kvaser_usb *dev, u8 id,
> + struct kvaser_msg *msg)
> +{
> + struct kvaser_msg *tmp;
> + void *buf;
> + int actual_len;
> + int err;
> + int pos = 0;
> +
> + buf = kzalloc(RX_BUFFER_SIZE, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + err = usb_bulk_msg(dev->udev, usb_rcvbulkpipe(dev->udev, 129),
^^^
dito
> + buf, RX_BUFFER_SIZE, &actual_len,
> + USB_RECEIVE_TIMEOUT);
> + if (err < 0) {
> + kfree(buf);
> + return err;
> + }
> +
> + while (pos < actual_len) {
Please check that pos + sizeof(*msg) is < actual_len, as you fill access
it later.
> + tmp = buf + pos;
> +
> + if (!tmp->len)
> + break;
> +
> + if (tmp->id == id) {
> + memcpy(msg, tmp, sizeof(*msg));
> + kfree(buf);
> + return 0;
> + }
> +
> + pos += tmp->len;
> + }
> +
> + kfree(buf);
> +
> + return -EINVAL;
> +}
> +
> +static int kvaser_usb_send_simple_msg(const struct kvaser_usb *dev,
> + u8 msg_id, int channel)
> +{
> + struct kvaser_msg msg;
> + int err;
> +
> + msg.len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple);
> + msg.id = msg_id;
> + msg.u.simple.channel = channel;
> + msg.u.simple.tid = 0xff;
Please use C99 struct initializer.
struct kvaser_msg msg = {
.len = ,
.id =,
};
> +
> + err = kvaser_usb_send_msg(dev, &msg);
just:
return err;
> + if (err)
> + return err;
> +
> + return 0;
> +}
> +
> +static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
> +{
> + struct kvaser_msg msg;
> + int err;
> +
> + err = kvaser_usb_send_simple_msg(dev, CMD_GET_SOFTWARE_INFO, 0);
> + if (err)
> + return err;
> +
> + err = kvaser_usb_wait_msg(dev, CMD_GET_SOFTWARE_INFO_REPLY, &msg);
> + if (err)
> + return err;
> +
> + dev->fw_version = le32_to_cpu(msg.u.softinfo.fw_version);
> +
> + return 0;
> +}
> +
> +static int kvaser_usb_get_card_info(struct kvaser_usb *dev)
> +{
> + struct kvaser_msg msg;
> + int err;
> +
> + err = kvaser_usb_send_simple_msg(dev, CMD_GET_CARD_INFO, 0);
> + if (err)
> + return err;
> +
> + err = kvaser_usb_wait_msg(dev, CMD_GET_CARD_INFO_REPLY, &msg);
> + if (err)
> + return err;
> +
> + dev->nchannels = msg.u.cardinfo.nchannels;
> +
> + return 0;
> +}
> +
> +static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
> + const struct kvaser_msg *msg)
> +{
> + struct net_device_stats *stats;
> + struct kvaser_usb_tx_urb_context *context;
> + struct kvaser_usb_net_priv *priv;
> + u8 channel = msg->u.tx_acknowledge.channel;
> + u8 tid = msg->u.tx_acknowledge.tid;
> +
> + if (channel >= dev->nchannels) {
> + dev_err(dev->udev->dev.parent,
> + "Invalid channel number (%d)\n", channel);
> + return;
> + }
can you do a check for (channel >= dev->nchannels), in a central place?
e.g. kvaser_usb_handle_message()?
> +
> + priv = dev->nets[channel];
> +
> + if (!netif_device_present(priv->netdev))
> + return;
> +
> + stats = &priv->netdev->stats;
> +
> + context = &priv->tx_contexts[tid % MAX_TX_URBS];
> +
> + /*
> + * It looks like the firmware never sets the flags field of the
> + * tx_acknowledge frame and never reports a transmit failure.
> + * If the can message can't be transmited (e.g. incompatible
> + * bitrates), a frame CMD_CAN_ERROR_EVENT is sent (with a null
> + * tid) and the firmware tries to transmit again the packet until
> + * it succeeds. Once the packet is successfully transmitted, then
> + * the tx_acknowledge frame is sent.
> + */
> +
> + stats->tx_packets++;
> + stats->tx_bytes += context->dlc;
> + can_get_echo_skb(priv->netdev, context->echo_index);
> +
> + context->echo_index = MAX_TX_URBS;
> + atomic_dec(&priv->active_tx_urbs);
> +
> + netif_wake_queue(priv->netdev);
> +}
> +
> +static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> + const struct kvaser_msg *msg)
> +{
> + struct can_frame *cf;
> + struct sk_buff *skb;
> + struct net_device_stats *stats;
> + struct kvaser_usb_net_priv *priv;
> + u8 channel, status, txerr, rxerr;
> +
> + if (msg->id == CMD_CAN_ERROR_EVENT) {
> + channel = msg->u.error_event.channel;
> + status = msg->u.error_event.status;
> + txerr = msg->u.error_event.tx_errors_count;
> + rxerr = msg->u.error_event.rx_errors_count;
> + } else {
> + channel = msg->u.chip_state_event.channel;
> + status = msg->u.chip_state_event.status;
> + txerr = msg->u.chip_state_event.tx_errors_count;
> + rxerr = msg->u.chip_state_event.rx_errors_count;
> + }
> +
> + if (channel >= dev->nchannels) {
> + dev_err(dev->udev->dev.parent,
> + "Invalid channel number (%d)\n", channel);
> + return;
> + }
> +
> + priv = dev->nets[channel];
> + stats = &priv->netdev->stats;
> +
> + skb = alloc_can_err_skb(priv->netdev, &cf);
> + if (!skb) {
> + stats->rx_dropped++;
> + return;
> + }
> +
> + if ((status & M16C_STATE_BUS_OFF) ||
> + (status & M16C_STATE_BUS_RESET)) {
> + priv->can.state = CAN_STATE_BUS_OFF;
> + cf->can_id |= CAN_ERR_BUSOFF;
> + can_bus_off(priv->netdev);
> + } else if (status & M16C_STATE_BUS_ERROR) {
> + priv->can.state = CAN_STATE_ERROR_WARNING;
> + priv->can.can_stats.error_warning++;
> + } else if (status & M16C_STATE_BUS_PASSIVE) {
> + priv->can.state = CAN_STATE_ERROR_PASSIVE;
> + priv->can.can_stats.error_passive++;
> + } else {
> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> + cf->can_id |= CAN_ERR_PROT;
> + cf->data[2] = CAN_ERR_PROT_ACTIVE;
> + }
> +
> + if (msg->id == CMD_CAN_ERROR_EVENT) {
> + u8 error_factor = msg->u.error_event.error_factor;
> +
> + priv->can.can_stats.bus_error++;
> + stats->rx_errors++;
> +
> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> +
> + if ((priv->can.state == CAN_STATE_ERROR_WARNING) ||
> + (priv->can.state == CAN_STATE_ERROR_PASSIVE)) {
> + cf->data[1] = (txerr > rxerr) ?
> + CAN_ERR_CRTL_TX_PASSIVE
> + : CAN_ERR_CRTL_RX_PASSIVE;
> + }
> +
> + if (error_factor & M16C_EF_ACKE)
> + cf->data[3] |= (CAN_ERR_PROT_LOC_ACK |
> + CAN_ERR_PROT_LOC_ACK_DEL);
> + if (error_factor & M16C_EF_CRCE)
> + cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> + CAN_ERR_PROT_LOC_CRC_DEL);
> + if (error_factor & M16C_EF_FORME)
> + cf->data[2] |= CAN_ERR_PROT_FORM;
> + if (error_factor & M16C_EF_STFE)
> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> + if (error_factor & M16C_EF_BITE0)
> + cf->data[2] |= CAN_ERR_PROT_BIT0;
> + if (error_factor & M16C_EF_BITE1)
> + cf->data[2] |= CAN_ERR_PROT_BIT1;
> + if (error_factor & M16C_EF_TRE)
> + cf->data[2] |= CAN_ERR_PROT_TX;
> + }
> +
> + cf->data[6] = txerr;
> + cf->data[7] = rxerr;
> +
> + netif_rx(skb);
> +
> + priv->bec.txerr = txerr;
> + priv->bec.rxerr = rxerr;
> +
> + stats->rx_packets++;
> + stats->rx_bytes += cf->can_dlc;
> +}
> +
> +static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
> + const struct kvaser_msg *msg)
> +{
> + struct kvaser_usb_net_priv *priv;
> + struct can_frame *cf;
> + struct sk_buff *skb;
> + struct net_device_stats *stats;
> + u8 channel = msg->u.rx_can.channel;
> +
> + if (channel >= dev->nchannels) {
> + dev_err(dev->udev->dev.parent,
> + "Invalid channel number (%d)\n", channel);
> + return;
> + }
> +
> + priv = dev->nets[channel];
> + stats = &priv->netdev->stats;
> +
> + skb = alloc_can_skb(priv->netdev, &cf);
> + if (skb == NULL) {
> + stats->tx_dropped++;
> + return;
> + }
> +
> + cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) |
> + (msg->u.rx_can.msg[1] & 0x3f);
> + cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]);
> +
> + if (msg->id == CMD_RX_EXT_MESSAGE) {
> + cf->can_id <<= 18;
> + cf->can_id += ((msg->u.rx_can.msg[2] & 0x0f) << 14) |
|=
is more appropriate here
> + ((msg->u.rx_can.msg[3] & 0xff) << 6) |
> + (msg->u.rx_can.msg[4] & 0x3f);
> + cf->can_id |= CAN_EFF_FLAG;
> + }
> +
> + if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME) {
> + cf->can_id |= CAN_RTR_FLAG;
> + } else if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
> + MSG_FLAG_NERR)) {
> + cf->can_id |= CAN_ERR_FLAG;
> + cf->can_dlc = CAN_ERR_DLC;
What kind of error is this? Can you set cf->data? What about the
original cd->can_id? What about the stats->rx_*error* stats?
> + } else if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
> + cf->can_id = CAN_ERR_FLAG | CAN_ERR_CRTL;
> + cf->can_dlc = CAN_ERR_DLC;
> + cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> +
> + stats->rx_over_errors++;
> + stats->rx_errors++;
> + } else if (!msg->u.rx_can.flag) {
> + memcpy(cf->data, &msg->u.rx_can.msg[6], cf->can_dlc);
> + } else {
> + kfree_skb(skb);
> + return;
> + }
> +
> + netif_rx(skb);
> +
> + stats->rx_packets++;
> + stats->rx_bytes += cf->can_dlc;
> +}
> +
> +static void kvaser_usb_start_stop_chip_reply(const struct kvaser_usb *dev,
> + const struct kvaser_msg *msg)
> +{
> + struct kvaser_usb_net_priv *priv;
> + u8 channel = msg->u.simple.channel;
> +
> + if (channel >= dev->nchannels) {
> + dev_err(dev->udev->dev.parent,
> + "Invalid channel number (%d)\n", channel);
> + return;
> + }
> +
> + priv = dev->nets[channel];
> +
> + complete(&priv->start_stop_comp);
> +}
> +
> +static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
> + const struct kvaser_msg *msg)
> +{
> + switch (msg->id) {
> + case CMD_START_CHIP_REPLY:
> + case CMD_STOP_CHIP_REPLY:
> + kvaser_usb_start_stop_chip_reply(dev, msg);
> + break;
> +
> + case CMD_RX_STD_MESSAGE:
> + case CMD_RX_EXT_MESSAGE:
> + kvaser_usb_rx_can_msg(dev, msg);
> + break;
> +
> + case CMD_CHIP_STATE_EVENT:
> + case CMD_CAN_ERROR_EVENT:
> + kvaser_usb_rx_error(dev, msg);
> + break;
> +
> + case CMD_TX_ACKNOWLEDGE:
> + kvaser_usb_tx_acknowledge(dev, msg);
> + break;
> +
> + default:
> + dev_warn(dev->udev->dev.parent,
> + "Unhandled message (%d)\n", msg->id);
> + break;
> + }
> +}
> +
> +static void kvaser_usb_read_bulk_callback(struct urb *urb)
> +{
> + struct kvaser_usb *dev = urb->context;
> + struct kvaser_msg *msg;
> + int pos = 0;
> + int err, i;
> +
> + switch (urb->status) {
> + case 0:
> + break;
> + case -ENOENT:
> + case -ESHUTDOWN:
> + return;
> + default:
> + dev_info(dev->udev->dev.parent, "Rx URB aborted (%d)\n",
> + urb->status);
> + goto resubmit_urb;
> + }
> +
> + while (pos < urb->actual_length) {
please check here for pos + sizeof(*msg), too
> + msg = urb->transfer_buffer + pos;
> +
> + if (!msg->len)
> + break;
> +
> + kvaser_usb_handle_message(dev, msg);
> +
> + if (pos > urb->actual_length) {
> + dev_err(dev->udev->dev.parent, "Format error\n");
> + break;
> + }
> +
> + pos += msg->len;
> + }
> +
> +resubmit_urb:
> + usb_fill_bulk_urb(urb, dev->udev, usb_rcvbulkpipe(dev->udev, 129),
^^^
use #define
> + urb->transfer_buffer, RX_BUFFER_SIZE,
> + kvaser_usb_read_bulk_callback, dev);
> +
> + err = usb_submit_urb(urb, GFP_ATOMIC);
> + if (err == -ENODEV) {
> + for (i = 0; i < dev->nchannels; i++) {
> + if (!dev->nets[i])
> + continue;
> +
> + netif_device_detach(dev->nets[i]->netdev);
> + }
> + } else if (err) {
> + dev_err(dev->udev->dev.parent,
> + "Failed resubmitting read bulk urb: %d\n", err);
> + }
> +
> + return;
> +}
> +
> +static int kvaser_usb_setup_rx_urbs(struct kvaser_usb *dev)
> +{
> + int i, err = 0;
> +
> + if (dev->rxinitdone)
> + return 0;
> +
> + for (i = 0; i < MAX_RX_URBS; i++) {
> + struct urb *urb = NULL;
> + u8 *buf = NULL;
> +
> + urb = usb_alloc_urb(0, GFP_KERNEL);
> + if (!urb) {
> + dev_warn(dev->udev->dev.parent,
> + "No memory left for URBs\n");
> + err = -ENOMEM;
> + break;
> + }
> +
> + buf = usb_alloc_coherent(dev->udev, RX_BUFFER_SIZE,
> + GFP_KERNEL, &urb->transfer_dma);
> + if (!buf) {
> + dev_warn(dev->udev->dev.parent,
> + "No memory left for USB buffer\n");
> + usb_free_urb(urb);
> + err = -ENOMEM;
> + break;
> + }
> +
> + usb_fill_bulk_urb(urb, dev->udev,
> + usb_rcvbulkpipe(dev->udev, 129),
use #define
> + buf, RX_BUFFER_SIZE,
> + kvaser_usb_read_bulk_callback, dev);
> + urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> + usb_anchor_urb(urb, &dev->rx_submitted);
> +
> + err = usb_submit_urb(urb, GFP_KERNEL);
> + if (err) {
> + usb_unanchor_urb(urb);
> + usb_free_coherent(dev->udev, RX_BUFFER_SIZE, buf,
> + urb->transfer_dma);
> + break;
> + }
> +
> + usb_free_urb(urb);
> + }
> +
> + if (i == 0) {
> + dev_warn(dev->udev->dev.parent,
> + "Cannot setup read URBs, error %d\n", err);
> + return err;
> + } else if (i < MAX_RX_URBS) {
> + dev_warn(dev->udev->dev.parent,
> + "RX performances may be slow\n");
> + }
> +
> + dev->rxinitdone = true;
> +
> + return 0;
> +}
> +
> +static int kvaser_usb_set_opt_mode(const struct kvaser_usb_net_priv *priv)
> +{
> + struct kvaser_msg msg;
> +
> + memset(&msg, 0x00, sizeof(msg));
> + msg.id = CMD_SET_CTRL_MODE;
> + msg.len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_ctrl_mode);
> + msg.u.ctrl_mode.tid = 0xff;
> + msg.u.ctrl_mode.channel = priv->channel;
please use C99 struct initializers
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
> + msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT;
> + else
> + msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL;
> +
> + return kvaser_usb_send_msg(priv->dev, &msg);
> +}
> +
> +static int kvaser_usb_start_chip(struct kvaser_usb_net_priv *priv)
> +{
> + int err;
> +
> + init_completion(&priv->start_stop_comp);
> +
> + err = kvaser_usb_send_simple_msg(priv->dev, CMD_START_CHIP,
> + priv->channel);
> + if (err)
> + return err;
> +
> + if (!wait_for_completion_timeout(&priv->start_stop_comp,
> + msecs_to_jiffies(START_TIMEOUT)))
> + return -ETIMEDOUT;
> +
> + return 0;
> +}
> +
> +static int kvaser_usb_open(struct net_device *netdev)
> +{
> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> + struct kvaser_usb *dev = priv->dev;
> + int err;
> +
> + err = open_candev(netdev);
> + if (err)
> + return err;
> +
> + err = kvaser_usb_setup_rx_urbs(dev);
> + if (err)
> + return err;
> +
> + err = kvaser_usb_set_opt_mode(priv);
> + if (err)
> + return err;
> +
> + err = kvaser_usb_start_chip(priv);
> + if (err) {
> + netdev_warn(netdev, "Cannot start device, error %d\n", err);
> + close_candev(netdev);
> + return err;
> + }
> +
> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> + priv->open_time = jiffies;
> + netif_start_queue(netdev);
> +
> + return 0;
> +}
> +
> +static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
> +{
> + int i;
> +
> + usb_kill_anchored_urbs(&priv->tx_submitted);
> + atomic_set(&priv->active_tx_urbs, 0);
> +
> + for (i = 0; i < MAX_TX_URBS; i++)
ARRAY_SIZE(priv->tx_contexts) instead of MAX_TX_URBS
> + priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> +}
> +
> +static void kvaser_usb_unlink_all_urbs(struct kvaser_usb *dev)
> +{
> + int i;
> +
> + usb_kill_anchored_urbs(&dev->rx_submitted);
> +
> + for (i = 0; i < MAX_NET_DEVICES; i++) {
ARRAY_SIZE()
> + struct kvaser_usb_net_priv *priv = dev->nets[i];
> +
> + if (priv)
> + kvaser_usb_unlink_tx_urbs(priv);
> + }
> +}
> +
> +static int kvaser_usb_stop_chip(struct kvaser_usb_net_priv *priv)
> +{
> + int err;
> +
> + init_completion(&priv->start_stop_comp);
> +
> + err = kvaser_usb_send_simple_msg(priv->dev, CMD_STOP_CHIP,
> + priv->channel);
> + if (err)
> + return err;
> +
> + if (!wait_for_completion_timeout(&priv->start_stop_comp,
> + msecs_to_jiffies(STOP_TIMEOUT)))
> + return -ETIMEDOUT;
> +
> + return 0;
> +}
> +
> +static int kvaser_usb_flush_queue(struct kvaser_usb_net_priv *priv)
> +{
> + struct kvaser_msg msg;
> +
> + memset(&msg, 0x00, sizeof(msg));
> + msg.id = CMD_FLUSH_QUEUE;
> + msg.len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_flush_queue);
> + msg.u.flush_queue.channel = priv->channel;
C99 initialziers, please
> +
> + return kvaser_usb_send_msg(priv->dev, &msg);
> +}
> +
> +static int kvaser_usb_close(struct net_device *netdev)
> +{
> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> + int err;
> +
> + netif_stop_queue(netdev);
> +
> + err = kvaser_usb_flush_queue(priv);
> + if (err)
> + netdev_warn(netdev, "Cannot flush queue, error %d\n", err);
> +
> + err = kvaser_usb_stop_chip(priv);
> + if (err) {
> + netdev_warn(netdev, "Cannot stop device, error %d\n", err);
> + return err;
> + }
> +
> + kvaser_usb_unlink_tx_urbs(priv);
> +
> + priv->can.state = CAN_STATE_STOPPED;
> + close_candev(priv->netdev);
> + priv->open_time = 0;
> +
> + return 0;
> +}
> +
> +static void kvaser_usb_write_bulk_callback(struct urb *urb)
> +{
> + struct kvaser_usb_tx_urb_context *context = urb->context;
> + struct kvaser_usb_net_priv *priv;
> + struct net_device *netdev;
> +
> + if (WARN_ON(!context))
> + return;
> +
> + priv = context->priv;
> + netdev = priv->netdev;
> +
> + usb_free_coherent(urb->dev, urb->transfer_buffer_length,
> + urb->transfer_buffer, urb->transfer_dma);
> +
> + if (!netif_device_present(netdev))
> + return;
> +
> + if (urb->status)
> + netdev_info(netdev, "Tx URB aborted (%d)\n", urb->status);
> +
> + netdev->trans_start = jiffies;
Is trans_start needed? at least for non-usb devices it works without.
> +}
> +
> +static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
> + struct net_device *netdev)
> +{
> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> + struct kvaser_usb *dev = priv->dev;
> + struct net_device_stats *stats = &netdev->stats;
> + struct can_frame *cf = (struct can_frame *)skb->data;
> + struct kvaser_usb_tx_urb_context *context = NULL;
> + struct urb *urb;
> + void *buf;
> + struct kvaser_msg *msg;
> + int i, err;
> + int ret = NETDEV_TX_OK;
> +
> + if (can_dropped_invalid_skb(netdev, skb))
> + return NETDEV_TX_OK;
> +
> + urb = usb_alloc_urb(0, GFP_ATOMIC);
> + if (!urb) {
> + netdev_err(netdev, "No memory left for URBs\n");
> + stats->tx_dropped++;
> + dev_kfree_skb(skb);
> + goto nourbmem;
> + }
> +
> + buf = usb_alloc_coherent(dev->udev, sizeof(struct kvaser_msg),
> + GFP_ATOMIC, &urb->transfer_dma);
> + if (!buf) {
> + netdev_err(netdev, "No memory left for USB buffer\n");
> + stats->tx_dropped++;
> + dev_kfree_skb(skb);
> + goto nobufmem;
> + }
> +
> + msg = (struct kvaser_msg *)buf;
> + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_tx_can);
> + msg->u.tx_can.flags = 0;
> + msg->u.tx_can.channel = priv->channel;
> +
> + if (cf->can_id & CAN_EFF_FLAG) {
> + msg->id = CMD_TX_EXT_MESSAGE;
> + msg->u.tx_can.msg[0] = (cf->can_id >> 24) & 0x1f;
> + msg->u.tx_can.msg[1] = (cf->can_id >> 18) & 0x3f;
> + msg->u.tx_can.msg[2] = (cf->can_id >> 14) & 0x0f;
> + msg->u.tx_can.msg[3] = (cf->can_id >> 6) & 0xff;
> + msg->u.tx_can.msg[4] = cf->can_id & 0x3f;
> + } else {
> + msg->id = CMD_TX_STD_MESSAGE;
> + msg->u.tx_can.msg[0] = (cf->can_id >> 6) & 0x1f;
> + msg->u.tx_can.msg[1] = cf->can_id & 0x3f;
> + }
> +
> + msg->u.tx_can.msg[5] = cf->can_dlc;
> + memcpy(&msg->u.tx_can.msg[6], cf->data, cf->can_dlc);
> +
> + if (cf->can_id & CAN_RTR_FLAG)
> + msg->u.tx_can.flags |= MSG_FLAG_REMOTE_FRAME;
> +
> + for (i = 0; i < MAX_TX_URBS; i++) {
ARRAY_SIZE
> + if (priv->tx_contexts[i].echo_index == MAX_TX_URBS) {
> + context = &priv->tx_contexts[i];
> + break;
> + }
> + }
> +
> + if (!context) {
> + netdev_warn(netdev, "cannot find free context\n");
> + ret = NETDEV_TX_BUSY;
> + goto releasebuf;
> + }
> +
> + context->priv = priv;
> + context->echo_index = i;
> + context->dlc = cf->can_dlc;
> +
> + msg->u.tx_can.tid = context->echo_index;
> +
> + usb_fill_bulk_urb(urb, dev->udev, usb_sndbulkpipe(dev->udev, 1),
> + buf, msg->len,
> + kvaser_usb_write_bulk_callback, context);
> + urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> + usb_anchor_urb(urb, &priv->tx_submitted);
> +
> + can_put_echo_skb(skb, netdev, context->echo_index);
> +
> + atomic_inc(&priv->active_tx_urbs);
> +
> + if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS)
> + netif_stop_queue(netdev);
> +
> + err = usb_submit_urb(urb, GFP_ATOMIC);
> + if (unlikely(err)) {
> + can_free_echo_skb(netdev, context->echo_index);
> +
> + atomic_dec(&priv->active_tx_urbs);
> + usb_unanchor_urb(urb);
> +
> + stats->tx_dropped++;
> +
> + if (err == -ENODEV)
> + netif_device_detach(netdev);
> + else
> + netdev_warn(netdev, "Failed tx_urb %d\n", err);
> +
> + goto releasebuf;
> + }
> +
> + netdev->trans_start = jiffies;
> +
> + usb_free_urb(urb);
> +
> + return NETDEV_TX_OK;
> +
> +releasebuf:
> + usb_free_coherent(dev->udev, sizeof(struct kvaser_msg),
> + buf, urb->transfer_dma);
> +nobufmem:
> + usb_free_urb(urb);
> +nourbmem:
> + return ret;
> +}
> +
> +static const struct net_device_ops kvaser_usb_netdev_ops = {
> + .ndo_open = kvaser_usb_open,
> + .ndo_stop = kvaser_usb_close,
> + .ndo_start_xmit = kvaser_usb_start_xmit,
> +};
> +
> +static struct can_bittiming_const kvaser_usb_bittiming_const = {
> + .name = "kvaser_usb",
> + .tseg1_min = KVASER_USB_TSEG1_MIN,
> + .tseg1_max = KVASER_USB_TSEG1_MAX,
> + .tseg2_min = KVASER_USB_TSEG2_MIN,
> + .tseg2_max = KVASER_USB_TSEG2_MAX,
> + .sjw_max = KVASER_USB_SJW_MAX,
> + .brp_min = KVASER_USB_BRP_MIN,
> + .brp_max = KVASER_USB_BRP_MAX,
> + .brp_inc = KVASER_USB_BRP_INC,
> +};
> +
> +static int kvaser_usb_set_bittiming(struct net_device *netdev)
> +{
> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> + struct can_bittiming *bt = &priv->can.bittiming;
> + struct kvaser_usb *dev = priv->dev;
> + struct kvaser_msg msg;
> +
> + msg.id = CMD_SET_BUS_PARAMS;
> + msg.len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_busparams);
> + msg.u.busparams.channel = priv->channel;
> + msg.u.busparams.tid = 0xff;
> + msg.u.busparams.bitrate = bt->bitrate;
bitrate is le32
> + msg.u.busparams.sjw = bt->sjw;
> + msg.u.busparams.tseg1 = bt->prop_seg + bt->phase_seg1;
> + msg.u.busparams.tseg2 = bt->phase_seg2;
C99 initializers, please
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
> + msg.u.busparams.no_samp = 3;
> + else
> + msg.u.busparams.no_samp = 1;
> +
> + return kvaser_usb_send_msg(dev, &msg);
> +}
> +
> +static int kvaser_usb_set_mode(struct net_device *netdev,
> + enum can_mode mode)
> +{
> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> +
> + if (!priv->open_time)
> + return -EINVAL;
> +
> + switch (mode) {
> + case CAN_MODE_START:
> + if (netif_queue_stopped(netdev))
> + netif_wake_queue(netdev);
No need to restart your USB device?
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int kvaser_usb_get_berr_counter(const struct net_device *netdev,
> + struct can_berr_counter *bec)
> +{
> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> +
> + bec->txerr = priv->bec.txerr;
> + bec->rxerr = priv->bec.rxerr;
> +
> + return 0;
> +}
> +
> +static int kvaser_usb_init_one(struct usb_interface *intf,
> + const struct usb_device_id *id, int channel)
> +{
> + struct kvaser_usb *dev = usb_get_intfdata(intf);
> + struct net_device *netdev;
> + struct kvaser_usb_net_priv *priv;
> + int i, err;
> +
> + netdev = alloc_candev(sizeof(*priv), MAX_TX_URBS);
> + if (!netdev) {
> + dev_err(&intf->dev, "Cannot alloc candev\n");
> + return -ENOMEM;
> + }
> +
> + priv = netdev_priv(netdev);
> +
> + init_usb_anchor(&priv->tx_submitted);
> + atomic_set(&priv->active_tx_urbs, 0);
> +
> + for (i = 0; i < MAX_TX_URBS; i++)
> + priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> +
> + priv->dev = dev;
> + priv->netdev = netdev;
> + priv->channel = channel;
> +
> + priv->can.state = CAN_STATE_STOPPED;
> + priv->can.clock.freq = CAN_USB_CLOCK;
> + priv->can.bittiming_const = &kvaser_usb_bittiming_const;
> + priv->can.do_set_bittiming = kvaser_usb_set_bittiming;
> + priv->can.do_set_mode = kvaser_usb_set_mode;
> + priv->can.do_get_berr_counter = kvaser_usb_get_berr_counter;
> + priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;
> + if (id->driver_info & KVASER_HAS_SILENT_MODE)
> + priv->can.ctrlmode_supported |= CAN_CTRLMODE_LISTENONLY;
> +
> + netdev->flags |= IFF_ECHO;
> +
> + netdev->netdev_ops = &kvaser_usb_netdev_ops;
> +
> + SET_NETDEV_DEV(netdev, &intf->dev);
> +
> + err = register_candev(netdev);
> + if (err) {
> + dev_err(&intf->dev, "Failed to register can device\n");
> + free_candev(netdev);
> + return err;
> + }
> +
> + dev->nets[channel] = priv;
> + netdev_info(netdev, "device %s registered\n", netdev->name);
netdev_info should take care of printing the device's name.
> +
> + return 0;
> +}
> +
> +static int kvaser_usb_probe(struct usb_interface *intf,
> + const struct usb_device_id *id)
> +{
> + struct kvaser_usb *dev;
> + int err = -ENOMEM;
> + int i;
> +
> + dev = kzalloc(sizeof(*dev), GFP_KERNEL);
Who will free dev on driver unload? Please make use of devm_kzalloc().
> + if (!dev)
> + return -ENOMEM;
> +
> + dev->udev = interface_to_usbdev(intf);
> +
> + init_usb_anchor(&dev->rx_submitted);
> +
> + usb_set_intfdata(intf, dev);
> +
> + if (kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, 0)) {
> + dev_err(&intf->dev, "Cannot reset kvaser\n");
> + goto error;
> + }
> +
> + if (kvaser_usb_get_software_info(dev)) {
> + dev_err(&intf->dev, "Cannot get software infos\n");
> + goto error;
> + }
> +
> + if (kvaser_usb_get_card_info(dev)) {
> + dev_err(&intf->dev, "Cannot get card infos\n");
> + goto error;
> + }
> +
> + dev_dbg(&intf->dev, "Firmware version: %d.%d.%d\n",
> + ((dev->fw_version >> 24) & 0xff),
> + ((dev->fw_version >> 16) & 0xff),
> + (dev->fw_version & 0xffff));
> +
> + for (i = 0; i < dev->nchannels; i++)
> + kvaser_usb_init_one(intf, id, i);
> +
> + return 0;
> +
> +error:
> + kfree(dev);
> + return err;
> +}
> +
> +static void kvaser_usb_disconnect(struct usb_interface *intf)
> +{
> + struct kvaser_usb *dev = usb_get_intfdata(intf);
> + int i;
> +
> + usb_set_intfdata(intf, NULL);
> +
> + if (!dev)
> + return;
> +
> + for (i = 0; i < dev->nchannels; i++) {
> + if (!dev->nets[i])
> + continue;
> +
> + unregister_netdev(dev->nets[i]->netdev);
> + free_candev(dev->nets[i]->netdev);
> + }
> +
> + kvaser_usb_unlink_all_urbs(dev);
> +}
> +
> +static struct usb_driver kvaser_usb_driver = {
> + .name = "kvaser_usb",
> + .probe = kvaser_usb_probe,
> + .disconnect = kvaser_usb_disconnect,
> + .id_table = kvaser_usb_table
> +};
> +
> +module_usb_driver(kvaser_usb_driver);
> +
> +MODULE_AUTHOR("Olivier Sobrie <olivier@sobrie.be>");
> +MODULE_DESCRIPTION("Can driver for Kvaser CAN/USB devices");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/net/can/usb/kvaser_usb.h b/drivers/net/can/usb/kvaser_usb.h
> new file mode 100644
> index 0000000..8e0b6ab
> --- /dev/null
> +++ b/drivers/net/can/usb/kvaser_usb.h
> @@ -0,0 +1,237 @@
> +#ifndef _KVASER_USB_H_
> +#define _KVASER_USB_H_
> +
> +#define MAX_TX_URBS 16
Please no tab between define and macro name
> +#define MAX_RX_URBS 4
> +#define START_TIMEOUT 1000
> +#define STOP_TIMEOUT 1000
> +#define USB_SEND_TIMEOUT 1000
> +#define USB_RECEIVE_TIMEOUT 1000
> +#define RX_BUFFER_SIZE 3072
> +#define CAN_USB_CLOCK 8000000
> +#define MAX_NET_DEVICES 3
> +
> +/* Kvaser USB devices */
> +#define KVASER_VENDOR_ID 0x0bfd
> +#define USB_LEAF_DEVEL_PRODUCT_ID 10
> +#define USB_LEAF_LITE_PRODUCT_ID 11
> +#define USB_LEAF_PRO_PRODUCT_ID 12
> +#define USB_LEAF_SPRO_PRODUCT_ID 14
> +#define USB_LEAF_PRO_LS_PRODUCT_ID 15
> +#define USB_LEAF_PRO_SWC_PRODUCT_ID 16
> +#define USB_LEAF_PRO_LIN_PRODUCT_ID 17
> +#define USB_LEAF_SPRO_LS_PRODUCT_ID 18
> +#define USB_LEAF_SPRO_SWC_PRODUCT_ID 19
> +#define USB_MEMO2_DEVEL_PRODUCT_ID 22
> +#define USB_MEMO2_HSHS_PRODUCT_ID 23
> +#define USB_UPRO_HSHS_PRODUCT_ID 24
> +#define USB_LEAF_LITE_GI_PRODUCT_ID 25
> +#define USB_LEAF_PRO_OBDII_PRODUCT_ID 26
> +#define USB_MEMO2_HSLS_PRODUCT_ID 27
> +#define USB_LEAF_LITE_CH_PRODUCT_ID 28
> +#define USB_BLACKBIRD_SPRO_PRODUCT_ID 29
> +#define USB_OEM_MERCURY_PRODUCT_ID 34
> +#define USB_OEM_LEAF_PRODUCT_ID 35
> +#define USB_CAN_R_PRODUCT_ID 39
> +
> +/* USB devices features */
> +#define KVASER_HAS_SILENT_MODE (1 << 0)
pleae use BIT(0)
> +
> +/* Message header size */
> +#define MSG_HEADER_LEN 2
> +
> +/* Can message flags */
> +#define MSG_FLAG_ERROR_FRAME (1 << 0)
> +#define MSG_FLAG_OVERRUN (1 << 1)
> +#define MSG_FLAG_NERR (1 << 2)
> +#define MSG_FLAG_WAKEUP (1 << 3)
> +#define MSG_FLAG_REMOTE_FRAME (1 << 4)
> +#define MSG_FLAG_RESERVED (1 << 5)
> +#define MSG_FLAG_TX_ACK (1 << 6)
> +#define MSG_FLAG_TX_REQUEST (1 << 7)
> +
> +/* Can states */
> +#define M16C_STATE_BUS_RESET (1 << 0)
> +#define M16C_STATE_BUS_ERROR (1 << 4)
> +#define M16C_STATE_BUS_PASSIVE (1 << 5)
> +#define M16C_STATE_BUS_OFF (1 << 6)
> +
> +/* Can msg ids */
> +#define CMD_RX_STD_MESSAGE 12
> +#define CMD_TX_STD_MESSAGE 13
> +#define CMD_RX_EXT_MESSAGE 14
> +#define CMD_TX_EXT_MESSAGE 15
> +#define CMD_SET_BUS_PARAMS 16
> +#define CMD_GET_BUS_PARAMS 17
> +#define CMD_GET_BUS_PARAMS_REPLY 18
> +#define CMD_GET_CHIP_STATE 19
> +#define CMD_CHIP_STATE_EVENT 20
> +#define CMD_SET_CTRL_MODE 21
> +#define CMD_GET_CTRL_MODE 22
> +#define CMD_GET_CTRL_MODE_REPLY 23
> +#define CMD_RESET_CHIP 24
> +#define CMD_RESET_CHIP_REPLY 25
> +#define CMD_START_CHIP 26
> +#define CMD_START_CHIP_REPLY 27
> +#define CMD_STOP_CHIP 28
> +#define CMD_STOP_CHIP_REPLY 29
> +#define CMD_GET_CARD_INFO2 32
> +#define CMD_GET_CARD_INFO 34
> +#define CMD_GET_CARD_INFO_REPLY 35
> +#define CMD_GET_SOFTWARE_INFO 38
> +#define CMD_GET_SOFTWARE_INFO_REPLY 39
> +#define CMD_ERROR_EVENT 45
> +#define CMD_FLUSH_QUEUE 48
> +#define CMD_TX_ACKNOWLEDGE 50
> +#define CMD_CAN_ERROR_EVENT 51
> +#define CMD_USB_THROTTLE 77
> +
> +/* error factors */
> +#define M16C_EF_ACKE (1 << 0)
> +#define M16C_EF_CRCE (1 << 1)
> +#define M16C_EF_FORME (1 << 2)
> +#define M16C_EF_STFE (1 << 3)
> +#define M16C_EF_BITE0 (1 << 4)
> +#define M16C_EF_BITE1 (1 << 5)
> +#define M16C_EF_RCVE (1 << 6)
> +#define M16C_EF_TRE (1 << 7)
> +
> +/* bittiming parameters */
> +#define KVASER_USB_TSEG1_MIN 1
> +#define KVASER_USB_TSEG1_MAX 16
> +#define KVASER_USB_TSEG2_MIN 1
> +#define KVASER_USB_TSEG2_MAX 8
> +#define KVASER_USB_SJW_MAX 4
> +#define KVASER_USB_BRP_MIN 1
> +#define KVASER_USB_BRP_MAX 64
> +#define KVASER_USB_BRP_INC 1
> +
> +/* ctrl modes */
> +#define KVASER_CTRL_MODE_NORMAL 1
> +#define KVASER_CTRL_MODE_SILENT 2
> +#define KVASER_CTRL_MODE_SELFRECEPTION 3
> +#define KVASER_CTRL_MODE_OFF 4
> +
> +struct kvaser_msg_simple {
> + u8 tid;
> + u8 channel;
> +} __packed;
> +
> +struct kvaser_msg_cardinfo {
> + u8 tid;
> + u8 nchannels;
> + __le32 serial_number;
> + __le32 padding;
> + __le32 clock_resolution;
> + __le32 mfgdate;
> + u8 ean[8];
> + u8 hw_revision;
> + u8 usb_hs_mode;
> + __le16 padding2;
> +} __packed;
> +
> +struct kvaser_msg_cardinfo2 {
> + u8 tid;
> + u8 channel;
> + u8 pcb_id[24];
> + __le32 oem_unlock_code;
> +} __packed;
> +
> +struct kvaser_msg_softinfo {
> + u8 tid;
> + u8 channel;
> + __le32 sw_options;
> + __le32 fw_version;
> + __le16 max_outstanding_tx;
> + __le16 padding[9];
> +} __packed;
> +
> +struct kvaser_msg_busparams {
> + u8 tid;
> + u8 channel;
> + __le32 bitrate;
> + u8 tseg1;
> + u8 tseg2;
> + u8 sjw;
> + u8 no_samp;
> +} __packed;
> +
> +struct kvaser_msg_tx_can {
> + u8 channel;
> + u8 tid;
> + u8 msg[14];
> + u8 padding;
> + u8 flags;
> +} __packed;
> +
> +struct kvaser_msg_rx_can {
> + u8 channel;
> + u8 flag;
> + __le16 time[3];
> + u8 msg[14];
> +} __packed;
> +
> +struct kvaser_msg_chip_state_event {
> + u8 tid;
> + u8 channel;
> + __le16 time[3];
> + u8 tx_errors_count;
> + u8 rx_errors_count;
> + u8 status;
> + u8 padding[3];
> +} __packed;
> +
> +struct kvaser_msg_tx_acknowledge {
> + u8 channel;
> + u8 tid;
> + __le16 time[3];
> + u8 flags;
> + u8 time_offset;
> +} __packed;
> +
> +struct kvaser_msg_error_event {
> + u8 tid;
> + u8 flags;
> + __le16 time[3];
> + u8 channel;
> + u8 padding;
> + u8 tx_errors_count;
> + u8 rx_errors_count;
> + u8 status;
> + u8 error_factor;
> +} __packed;
> +
> +struct kvaser_msg_ctrl_mode {
> + u8 tid;
> + u8 channel;
> + u8 ctrl_mode;
> + u8 padding[3];
> +} __packed;
> +
> +struct kvaser_msg_flush_queue {
> + u8 tid;
> + u8 channel;
> + u8 flags;
> + u8 padding[3];
> +} __packed;
> +
> +struct kvaser_msg {
> + u8 len;
> + u8 id;
> + union {
> + struct kvaser_msg_simple simple;
> + struct kvaser_msg_cardinfo cardinfo;
> + struct kvaser_msg_cardinfo2 cardinfo2;
> + struct kvaser_msg_softinfo softinfo;
> + struct kvaser_msg_busparams busparams;
> + struct kvaser_msg_tx_can tx_can;
> + struct kvaser_msg_rx_can rx_can;
> + struct kvaser_msg_chip_state_event chip_state_event;
> + struct kvaser_msg_tx_acknowledge tx_acknowledge;
> + struct kvaser_msg_error_event error_event;
> + struct kvaser_msg_ctrl_mode ctrl_mode;
> + struct kvaser_msg_ctrl_mode flush_queue;
> + } u;
> +} __packed;
> +
> +#endif
>
--
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] net: Allow to create links with given ifindex
From: Eric W. Biederman @ 2012-07-30 10:56 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: Linux Netdev List, David Miller
In-Reply-To: <87pq7dh6b2.fsf@xmission.com>
ebiederm@xmission.com (Eric W. Biederman) writes:
> Pavel Emelyanov <xemul@parallels.com> writes:
>
>> Currently the RTM_NEWLINK results in -EOPNOTSUPP if the ifinfomsg->ifi_index
>> is not zero. I propose to allow requesting ifindices on link creation. This
>> is required by the checkpoint-restore to correctly restore a net namespace
>> (i.e. -- a container). The question what to do with pre-created devices such
>> as lo or sit fbdev is open, but for manually created devices this can be
>> solved by this patch.
>
> Have you walked through and found the locations where we still rely on
> ifindex being globally unique?
>
> Last time I was working in this area there were serveral places where
> things were indexed by just the interface index.
If it is really safe to make ifindex per network namespace at this
point you can make dev_new_ifindex have a per network namespace base
counter, and that will fix your problems with the loopback device.
Unless you have done the work to root out the last of dependencies on
ifindex being globally unique I think you will run into some operational
problems.
Eric
^ permalink raw reply
* Re: [PATCH 1/2] net: Allow to create links with given ifindex
From: Eric W. Biederman @ 2012-07-30 10:49 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: Linux Netdev List, David Miller
In-Reply-To: <50160EEF.6050406@parallels.com>
Pavel Emelyanov <xemul@parallels.com> writes:
> Currently the RTM_NEWLINK results in -EOPNOTSUPP if the ifinfomsg->ifi_index
> is not zero. I propose to allow requesting ifindices on link creation. This
> is required by the checkpoint-restore to correctly restore a net namespace
> (i.e. -- a container). The question what to do with pre-created devices such
> as lo or sit fbdev is open, but for manually created devices this can be
> solved by this patch.
Have you walked through and found the locations where we still rely on
ifindex being globally unique?
Last time I was working in this area there were serveral places where
things were indexed by just the interface index.
I susepct it might be easier to generate hotplug events at restart time
saying someone removed and added an identical set of network devices.
Certainly for physical hardware that needs to happen, because things
like mac addresses will change.
Eric
> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
>
> ---
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 0ebaea1..5966e2f 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5533,7 +5533,12 @@ int register_netdevice(struct net_device *dev)
> }
> }
>
> - dev->ifindex = dev_new_index(net);
> + ret = -EBUSY;
> + if (!dev->ifindex)
> + dev->ifindex = dev_new_index(net);
> + else if (__dev_get_by_index(net, dev->ifindex))
> + goto err_uninit;
> +
> if (dev->iflink == -1)
> dev->iflink = dev->ifindex;
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 334b930..76e19aa 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1801,8 +1801,6 @@ replay:
> return -ENODEV;
> }
>
> - if (ifm->ifi_index)
> - return -EOPNOTSUPP;
> if (tb[IFLA_MAP] || tb[IFLA_MASTER] || tb[IFLA_PROTINFO])
> return -EOPNOTSUPP;
>
> @@ -1828,10 +1826,14 @@ replay:
> return PTR_ERR(dest_net);
>
> dev = rtnl_create_link(net, dest_net, ifname, ops, tb);
> -
> - if (IS_ERR(dev))
> + if (IS_ERR(dev)) {
> err = PTR_ERR(dev);
> - else if (ops->newlink)
> + goto out;
> + }
> +
> + dev->ifindex = ifm->ifi_index;
> +
> + if (ops->newlink)
> err = ops->newlink(net, dev, tb, data);
> else
> err = register_netdevice(dev);
> --
> 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 net-next 4/7] sfc: Add support for IEEE-1588 PTP
From: Stuart Hodgson @ 2012-07-30 10:03 UTC (permalink / raw)
To: Richard Cochran
Cc: Ben Hutchings, David Miller, netdev, linux-net-drivers,
Andrew Jackson
In-Reply-To: <20120720153016.GB2771@netboy.at.omicron.at>
On 20/07/12 16:30, Richard Cochran wrote:
> On Fri, Jul 20, 2012 at 10:15:46AM +0100, Stuart Hodgson wrote:
>>
>> Do you mean using the PPS kernel consumer to govern the system time?
>
> Well, I meant just using the PPS subsystem, which does not necessarily
> mean that the kernel consumer has to be used. In my experience, it is
> better to handle the servo in user space, but in any case, the user
> has the choice of what to do.
>
>>>>>> + ptp_pps_evt.type = PTP_CLOCK_EXTTS;
>>>>>> + ptp_pps_evt.timestamp = ktime_to_ns(gen_time_host);
>>>>>> + ptp_clock_event(ptp->phc_clock, &ptp_pps_evt);
>
>> In order for a PPS to arrive at the kernel consumer ptp_clock_event
>> needs to be called with PTP_CLOCK_PPS. This then calls pps_get_ts
>> and stamps the event with the current system time, not the time
>> that was put into the event.
>
> Oops, I meant PTP_CLOCK_PPS. I overlooked that your code is making an
> external timestamp event, but the basic idea is similar.
>
>> Using PTP_CLOCK_EXTTS the PPS is visible to userspace via a read
>> on the phc device and can then be used in our modified ptpd2.
>
> How does your program use this information?
>
We have a second servo that governs the system time.
>>> ... why can't you also just set the time?
>>
>> Our hardware can only have an offset applied to the clock. In order to set time
>> we need to know the time now, then work out and offset to get to the target time.
>> At the point that we apply this offset the clock will have moved on and not be
>> set to the target time. We can apply some measured average times to the offset
>> to get closer but with this hardware settime will not leave the NIC clock at the
>> desired time.
>
> It does not matter if setting the time introduces a small error. That
> usually happens, but it is no big deal, since the servo in the PTP
> stack will correct the error.
>
I will add the ability to set the time to the patch.
> Thanks,
> Richard
^ permalink raw reply
* [PATCH net-next] bluetooth: Replace list_for_each with list_for_each_entry() helper
From: Denis, Kirjanov @ 2012-07-30 9:47 UTC (permalink / raw)
To: davem; +Cc: netdev, Denis Kirjanov
Signed-off-by: Denis Kirjanov <kirjanov@gmail.com>
---
include/net/bluetooth/hci_core.h | 20 ++++++++------------
1 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 475b8c0..fcf839f 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -852,7 +852,7 @@ struct hci_cb {
static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status)
{
- struct list_head *p;
+ struct hci_cb *cb;
__u8 encrypt;
hci_proto_auth_cfm(conn, status);
@@ -863,8 +863,7 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status)
encrypt = (conn->link_mode & HCI_LM_ENCRYPT) ? 0x01 : 0x00;
read_lock(&hci_cb_list_lock);
- list_for_each(p, &hci_cb_list) {
- struct hci_cb *cb = list_entry(p, struct hci_cb, list);
+ list_for_each_entry(cb, &hci_cb_list, list) {
if (cb->security_cfm)
cb->security_cfm(conn, status, encrypt);
}
@@ -874,7 +873,7 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status)
static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status,
__u8 encrypt)
{
- struct list_head *p;
+ struct hci_cb *cb;
if (conn->sec_level == BT_SECURITY_SDP)
conn->sec_level = BT_SECURITY_LOW;
@@ -885,8 +884,7 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status,
hci_proto_encrypt_cfm(conn, status, encrypt);
read_lock(&hci_cb_list_lock);
- list_for_each(p, &hci_cb_list) {
- struct hci_cb *cb = list_entry(p, struct hci_cb, list);
+ list_for_each_entry(cb, &hci_cb_list, list) {
if (cb->security_cfm)
cb->security_cfm(conn, status, encrypt);
}
@@ -895,11 +893,10 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status,
static inline void hci_key_change_cfm(struct hci_conn *conn, __u8 status)
{
- struct list_head *p;
+ struct hci_cb *cb;
read_lock(&hci_cb_list_lock);
- list_for_each(p, &hci_cb_list) {
- struct hci_cb *cb = list_entry(p, struct hci_cb, list);
+ list_for_each_entry(cb, &hci_cb_list, list) {
if (cb->key_change_cfm)
cb->key_change_cfm(conn, status);
}
@@ -909,11 +906,10 @@ static inline void hci_key_change_cfm(struct hci_conn *conn, __u8 status)
static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
__u8 role)
{
- struct list_head *p;
+ struct hci_cb *cb;
read_lock(&hci_cb_list_lock);
- list_for_each(p, &hci_cb_list) {
- struct hci_cb *cb = list_entry(p, struct hci_cb, list);
+ list_for_each_entry(cb, &hci_cb_list, list) {
if (cb->role_switch_cfm)
cb->role_switch_cfm(conn, status, role);
}
--
1.7.1
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
^ permalink raw reply related
* [PATCH] net: ipv4: fix RCU races on dst refcounts
From: Eric Dumazet @ 2012-07-30 9:20 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
commit c6cffba4ffa2 (ipv4: Fix input route performance regression.)
added various fatal races with dst refcounts.
crashes happen on tcp workloads if routes are added/deleted at the same
time.
The dst_free() calls from free_fib_info_rcu() are clearly racy.
We need instead regular dst refcounting (dst_release()) and make
sure dst_release() is aware of RCU grace periods :
Add DST_RCU_FREE flag so that dst_release() respects an RCU grace period
before dst destruction for cached dst
Introduce a new inet_sk_rx_dst_set() helper, using atomic_inc_not_zero()
to make sure we dont increase a zero refcount (On a dst currently
waiting an rcu grace period before destruction)
rt_cache_route() must take a reference on the new cached route, and
release it if was not able to install it.
With this patch, my machines survive various benchmarks.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/dst.h | 7 +------
include/net/inet_sock.h | 13 +++++++++++++
net/core/dst.c | 26 +++++++++++++++++++++-----
net/decnet/dn_route.c | 6 ++++++
net/ipv4/fib_semantics.c | 4 ++--
net/ipv4/route.c | 16 ++++------------
net/ipv4/tcp_input.c | 3 +--
net/ipv4/tcp_ipv4.c | 12 ++++++------
net/ipv4/tcp_minisocks.c | 3 +--
9 files changed, 55 insertions(+), 35 deletions(-)
diff --git a/include/net/dst.h b/include/net/dst.h
index baf5978..31a9fd3 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -61,6 +61,7 @@ struct dst_entry {
#define DST_NOPEER 0x0040
#define DST_FAKE_RTABLE 0x0080
#define DST_XFRM_TUNNEL 0x0100
+#define DST_RCU_FREE 0x0200
unsigned short pending_confirm;
@@ -382,12 +383,6 @@ static inline void dst_free(struct dst_entry *dst)
__dst_free(dst);
}
-static inline void dst_rcu_free(struct rcu_head *head)
-{
- struct dst_entry *dst = container_of(head, struct dst_entry, rcu_head);
- dst_free(dst);
-}
-
static inline void dst_confirm(struct dst_entry *dst)
{
dst->pending_confirm = 1;
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index 613cfa4..e3fd34c 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -249,4 +249,17 @@ static inline __u8 inet_sk_flowi_flags(const struct sock *sk)
return flags;
}
+static inline void inet_sk_rx_dst_set(struct sock *sk, const struct sk_buff *skb)
+{
+ struct dst_entry *dst = skb_dst(skb);
+
+ if (atomic_inc_not_zero(&dst->__refcnt)) {
+ if (!(dst->flags & DST_RCU_FREE))
+ dst->flags |= DST_RCU_FREE;
+
+ sk->sk_rx_dst = dst;
+ inet_sk(sk)->rx_dst_ifindex = skb->skb_iif;
+ }
+}
+
#endif /* _INET_SOCK_H */
diff --git a/net/core/dst.c b/net/core/dst.c
index 069d51d..d9e33eb 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -258,6 +258,15 @@ again:
}
EXPORT_SYMBOL(dst_destroy);
+static void dst_rcu_destroy(struct rcu_head *head)
+{
+ struct dst_entry *dst = container_of(head, struct dst_entry, rcu_head);
+
+ dst = dst_destroy(dst);
+ if (dst)
+ __dst_free(dst);
+}
+
void dst_release(struct dst_entry *dst)
{
if (dst) {
@@ -265,10 +274,14 @@ void dst_release(struct dst_entry *dst)
newrefcnt = atomic_dec_return(&dst->__refcnt);
WARN_ON(newrefcnt < 0);
- if (unlikely(dst->flags & DST_NOCACHE) && !newrefcnt) {
- dst = dst_destroy(dst);
- if (dst)
- __dst_free(dst);
+ if (unlikely(dst->flags & (DST_NOCACHE | DST_RCU_FREE)) && !newrefcnt) {
+ if (dst->flags & DST_RCU_FREE) {
+ call_rcu_bh(&dst->rcu_head, dst_rcu_destroy);
+ } else {
+ dst = dst_destroy(dst);
+ if (dst)
+ __dst_free(dst);
+ }
}
}
}
@@ -320,11 +333,14 @@ EXPORT_SYMBOL(__dst_destroy_metrics_generic);
*/
void skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst)
{
+ bool hold;
+
WARN_ON(!rcu_read_lock_held() && !rcu_read_lock_bh_held());
/* If dst not in cache, we must take a reference, because
* dst_release() will destroy dst as soon as its refcount becomes zero
*/
- if (unlikely(dst->flags & DST_NOCACHE)) {
+ hold = (dst->flags & (DST_NOCACHE | DST_RCU_FREE)) == DST_NOCACHE;
+ if (unlikely(hold)) {
dst_hold(dst);
skb_dst_set(skb, dst);
} else {
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 85a3604..2671977 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -184,6 +184,12 @@ static __inline__ unsigned int dn_hash(__le16 src, __le16 dst)
return dn_rt_hash_mask & (unsigned int)tmp;
}
+static inline void dst_rcu_free(struct rcu_head *head)
+{
+ struct dst_entry *dst = container_of(head, struct dst_entry, rcu_head);
+ dst_free(dst);
+}
+
static inline void dnrt_free(struct dn_route *rt)
{
call_rcu_bh(&rt->dst.rcu_head, dst_rcu_free);
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index da0cc2e..e55171f 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -172,9 +172,9 @@ static void free_fib_info_rcu(struct rcu_head *head)
if (nexthop_nh->nh_exceptions)
free_nh_exceptions(nexthop_nh);
if (nexthop_nh->nh_rth_output)
- dst_free(&nexthop_nh->nh_rth_output->dst);
+ dst_release(&nexthop_nh->nh_rth_output->dst);
if (nexthop_nh->nh_rth_input)
- dst_free(&nexthop_nh->nh_rth_input->dst);
+ dst_release(&nexthop_nh->nh_rth_input->dst);
} endfor_nexthops(fi);
release_net(fi->fib_net);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index fc1a81c..d6eabcf 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1199,11 +1199,6 @@ restart:
fnhe->fnhe_stamp = jiffies;
}
-static inline void rt_free(struct rtable *rt)
-{
- call_rcu_bh(&rt->dst.rcu_head, dst_rcu_free);
-}
-
static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
{
struct rtable *orig, *prev, **p = &nh->nh_rth_output;
@@ -1213,17 +1208,14 @@ static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
orig = *p;
+ rt->dst.flags |= DST_RCU_FREE;
+ dst_hold(&rt->dst);
prev = cmpxchg(p, orig, rt);
if (prev == orig) {
if (orig)
- rt_free(orig);
+ dst_release(&orig->dst);
} else {
- /* Routes we intend to cache in the FIB nexthop have
- * the DST_NOCACHE bit clear. However, if we are
- * unsuccessful at storing this route into the cache
- * we really need to set it.
- */
- rt->dst.flags |= DST_NOCACHE;
+ dst_release(&rt->dst);
}
}
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index a356e1f..9be30b0 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5604,8 +5604,7 @@ void tcp_finish_connect(struct sock *sk, struct sk_buff *skb)
tcp_set_state(sk, TCP_ESTABLISHED);
if (skb != NULL) {
- sk->sk_rx_dst = dst_clone(skb_dst(skb));
- inet_sk(sk)->rx_dst_ifindex = skb->skb_iif;
+ inet_sk_rx_dst_set(sk, skb);
security_inet_conn_established(sk, skb);
}
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 2fbd992..7f91e5a 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1617,19 +1617,19 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
#endif
if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
+ struct dst_entry *dst = sk->sk_rx_dst;
+
sock_rps_save_rxhash(sk, skb);
- if (sk->sk_rx_dst) {
- struct dst_entry *dst = sk->sk_rx_dst;
+ if (dst) {
if (inet_sk(sk)->rx_dst_ifindex != skb->skb_iif ||
dst->ops->check(dst, 0) == NULL) {
dst_release(dst);
sk->sk_rx_dst = NULL;
}
}
- if (unlikely(sk->sk_rx_dst == NULL)) {
- sk->sk_rx_dst = dst_clone(skb_dst(skb));
- inet_sk(sk)->rx_dst_ifindex = skb->skb_iif;
- }
+ if (unlikely(sk->sk_rx_dst == NULL))
+ inet_sk_rx_dst_set(sk, skb);
+
if (tcp_rcv_established(sk, skb, tcp_hdr(skb), skb->len)) {
rsk = sk;
goto reset;
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 3f1cc20..232a90c 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -387,8 +387,7 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
struct tcp_sock *oldtp = tcp_sk(sk);
struct tcp_cookie_values *oldcvp = oldtp->cookie_values;
- newsk->sk_rx_dst = dst_clone(skb_dst(skb));
- inet_sk(newsk)->rx_dst_ifindex = skb->skb_iif;
+ inet_sk_rx_dst_set(newsk, skb);
/* TCP Cookie Transactions require space for the cookie pair,
* as it differs for each connection. There is no need to
^ permalink raw reply related
* [PATCH] net: TCP early demux cleanup
From: Eric Dumazet @ 2012-07-30 7:06 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
early_demux() handlers should be called in RCU context, and as we
use skb_dst_set_noref(skb, dst), caller must not exit from RCU context
before dst use (skb_dst(skb)) or release (skb_drop(dst))
Therefore, rcu_read_lock()/rcu_read_unlock() pairs around
->early_demux() are confusing and not needed :
Protocol handlers are already in an RCU read lock section.
(__netif_receive_skb() does the rcu_read_lock() )
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/ip_input.c | 2 --
net/ipv6/ip6_input.c | 2 --
2 files changed, 4 deletions(-)
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 981ff1e..f1395a6 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -325,14 +325,12 @@ static int ip_rcv_finish(struct sk_buff *skb)
const struct net_protocol *ipprot;
int protocol = iph->protocol;
- rcu_read_lock();
ipprot = rcu_dereference(inet_protos[protocol]);
if (ipprot && ipprot->early_demux) {
ipprot->early_demux(skb);
/* must reload iph, skb->head might have changed */
iph = ip_hdr(skb);
}
- rcu_read_unlock();
}
/*
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index 47975e3..a52d864 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -52,11 +52,9 @@ int ip6_rcv_finish(struct sk_buff *skb)
if (sysctl_ip_early_demux && !skb_dst(skb)) {
const struct inet6_protocol *ipprot;
- rcu_read_lock();
ipprot = rcu_dereference(inet6_protos[ipv6_hdr(skb)->nexthdr]);
if (ipprot && ipprot->early_demux)
ipprot->early_demux(skb);
- rcu_read_unlock();
}
if (!skb_dst(skb))
ip6_route_input(skb);
^ permalink raw reply related
* [PATCH] codel: refine one condition to avoid a nul rec_inv_sqrt
From: Eric Dumazet @ 2012-07-30 6:52 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Anton Mich
From: Eric Dumazet <edumazet@google.com>
One condition before codel_Newton_step() was not good if
we never left the dropping state for a flow. As a result
rec_inv_sqrt was 0, instead of the ~0 initial value.
codel control law was then set to a very aggressive mode, dropping
many packets before reaching 'target' and recovering from this problem.
To keep codel_vars_init() as efficient as possible, refine
the condition to make sure rec_inv_sqrt initial value is correct
Many thanks to Anton Mich for discovering the issue and suggesting
a fix.
Reported-by: Anton Mich <lp2s1h@gmail.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/codel.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/net/codel.h b/include/net/codel.h
index 550debf..389cf62 100644
--- a/include/net/codel.h
+++ b/include/net/codel.h
@@ -305,6 +305,8 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch,
}
}
} else if (drop) {
+ u32 delta;
+
if (params->ecn && INET_ECN_set_ce(skb)) {
stats->ecn_mark++;
} else {
@@ -320,9 +322,11 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch,
* assume that the drop rate that controlled the queue on the
* last cycle is a good starting point to control it now.
*/
- if (codel_time_before(now - vars->drop_next,
+ delta = vars->count - vars->lastcount;
+ if (delta > 1 &&
+ codel_time_before(now - vars->drop_next,
16 * params->interval)) {
- vars->count = (vars->count - vars->lastcount) | 1;
+ vars->count = delta;
/* we dont care if rec_inv_sqrt approximation
* is not very precise :
* Next Newton steps will correct it quadratically.
^ permalink raw reply related
* Surprise! A new friend is searching 4 you;)
From: Roseanna Vida @ 2012-07-30 6:50 UTC (permalink / raw)
To: netdev@vger.kernel.org
Hello gorgeous!
I really want to make u my friend!
My name is Roseanna.
Waiting for your answer! (:
^ permalink raw reply
* Re: [PATCH v2] net/tun: fix ioctl() based info leaks
From: David Miller @ 2012-07-30 6:34 UTC (permalink / raw)
To: eric.dumazet; +Cc: minipli, richard.weinberger, netdev
In-Reply-To: <1343629340.2626.13268.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 30 Jul 2012 08:22:20 +0200
> On Mon, 2012-07-30 at 07:45 +0200, Mathias Krause wrote:
>> The tun module leaks up to 36 bytes of memory by not fully initializing
>> a structure located on the stack that gets copied to user memory by the
>> TUNGETIFF and SIOCGIFHWADDR ioctl()s.
>>
>> Signed-off-by: Mathias Krause <minipli@googlemail.com>
>> ---
>> v2:
>> - removed braces around else branch
>> - minor adjustment of the commit message
>>
>> drivers/net/tun.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index 987aeef..01255ff 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -1252,9 +1252,11 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
>> int vnet_hdr_sz;
>> int ret;
>>
>> - if (cmd == TUNSETIFF || _IOC_TYPE(cmd) == 0x89)
>> + if (cmd == TUNSETIFF || _IOC_TYPE(cmd) == 0x89) {
>> if (copy_from_user(&ifr, argp, ifreq_len))
>> return -EFAULT;
>> + } else
>> + memset(&ifr, 0, sizeof(ifr));
>>
>> if (cmd == TUNGETFEATURES) {
>> /* Currently this just means: "what IFF flags are valid?".
>
>
> Actually braces were better
>
> vi +169 Documentation/CodingStyle
>
> This does not apply if only one branch of a conditional statement is a
> single
> statement; in the latter case use braces in both branches:
>
> if (condition) {
> do_this();
> do_that();
> } else {
> otherwise();
> }
Ok I'll fix this up myself.
^ permalink raw reply
* Re: [net-next RFC V5 3/5] virtio: intorduce an API to set affinity for a virtqueue
From: Paolo Bonzini @ 2012-07-30 6:27 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: krkumar2, habanero, kvm, netdev, mashirle, linux-kernel,
virtualization, edumazet, tahm, jwhan, davem, sri
In-Reply-To: <20120729204030.GB13802@redhat.com>
Il 29/07/2012 22:40, Michael S. Tsirkin ha scritto:
>> > Did you set the affinity manually in your experiments, or perhaps there
>> > is a difference between scsi and networking... (interrupt mitigation?)
>
> You need to run irqbalancer in guest to make it actually work. Do you?
Yes, of course, now on to debugging that one. I just wanted to ask
before the weekend if I was missing something as obvious as that.
Paolo
^ permalink raw reply
* Re: [PATCH v2] net/tun: fix ioctl() based info leaks
From: Eric Dumazet @ 2012-07-30 6:22 UTC (permalink / raw)
To: Mathias Krause; +Cc: David S. Miller, richard -rw- weinberger, netdev
In-Reply-To: <1343627114-17899-1-git-send-email-minipli@googlemail.com>
On Mon, 2012-07-30 at 07:45 +0200, Mathias Krause wrote:
> The tun module leaks up to 36 bytes of memory by not fully initializing
> a structure located on the stack that gets copied to user memory by the
> TUNGETIFF and SIOCGIFHWADDR ioctl()s.
>
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
> ---
> v2:
> - removed braces around else branch
> - minor adjustment of the commit message
>
> drivers/net/tun.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 987aeef..01255ff 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1252,9 +1252,11 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
> int vnet_hdr_sz;
> int ret;
>
> - if (cmd == TUNSETIFF || _IOC_TYPE(cmd) == 0x89)
> + if (cmd == TUNSETIFF || _IOC_TYPE(cmd) == 0x89) {
> if (copy_from_user(&ifr, argp, ifreq_len))
> return -EFAULT;
> + } else
> + memset(&ifr, 0, sizeof(ifr));
>
> if (cmd == TUNGETFEATURES) {
> /* Currently this just means: "what IFF flags are valid?".
Actually braces were better
vi +169 Documentation/CodingStyle
This does not apply if only one branch of a conditional statement is a
single
statement; in the latter case use braces in both branches:
if (condition) {
do_this();
do_that();
} else {
otherwise();
}
^ permalink raw reply
* Re: [PATCH v2] net/tun: fix ioctl() based info leaks
From: David Miller @ 2012-07-30 6:20 UTC (permalink / raw)
To: minipli; +Cc: richard.weinberger, netdev
In-Reply-To: <1343627114-17899-1-git-send-email-minipli@googlemail.com>
From: Mathias Krause <minipli@googlemail.com>
Date: Mon, 30 Jul 2012 07:45:14 +0200
> The tun module leaks up to 36 bytes of memory by not fully initializing
> a structure located on the stack that gets copied to user memory by the
> TUNGETIFF and SIOCGIFHWADDR ioctl()s.
>
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net 1/5] tg3: Request APE_LOCK_PHY before PHY access
From: David Miller @ 2012-07-30 6:20 UTC (permalink / raw)
To: mchan; +Cc: netdev
In-Reply-To: <1343625345-5155-1-git-send-email-mchan@broadcom.com>
All 5 patches applied, thanks Michael.
^ permalink raw reply
* Re: [PATCH v4] ipv6: fix incorrect route 'expires' value passed to userspace
From: David Miller @ 2012-07-30 6:20 UTC (permalink / raw)
To: lw; +Cc: eric.dumazet, netdev
In-Reply-To: <1343613690-3236-1-git-send-email-lw@cn.fujitsu.com>
From: Li Wei <lw@cn.fujitsu.com>
Date: Mon, 30 Jul 2012 10:01:30 +0800
> When userspace use RTM_GETROUTE to dump route table, with an already
> expired route entry, we always got an 'expires' value(2147157)
> calculated base on INT_MAX.
>
> The reason of this problem is in the following satement:
> rt->dst.expires - jiffies < INT_MAX
> gcc promoted the type of both sides of '<' to unsigned long, thus
> a small negative value would be considered greater than INT_MAX.
>
> With the help of Eric Dumazet, do the out of bound checks in
> rtnl_put_cacheinfo(), _after_ conversion to clock_t.
>
> Signed-off-by: Li Wei <lw@cn.fujitsu.com>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1] mISDN: Bugfix only few bytes are transfered on a connection
From: David Miller @ 2012-07-30 6:19 UTC (permalink / raw)
To: keil; +Cc: netdev, tobias.powalowski
In-Reply-To: <1343582113-14752-2-git-send-email-keil@b1-systems.de>
From: Karsten Keil <keil@b1-systems.de>
Date: Sun, 29 Jul 2012 19:15:13 +0200
> The test for the fillempty condition was wrong in one place.
> Changed the variable to the right boolean type.
>
> Signed-off-by: Karsten Keil <keil@b1-systems.de>
Applied.
^ permalink raw reply
* Re: [PATCH] seeq: use PTR_RET at init_module of driver
From: David Miller @ 2012-07-30 6:19 UTC (permalink / raw)
To: develkernel412222; +Cc: jeffrey.t.kirsher, dhowells, jpirko, netdev, netdev
In-Reply-To: <1343568527-12729-1-git-send-email-develkernel412222@gmail.com>
From: Devendra Naga <develkernel412222@gmail.com>
Date: Sun, 29 Jul 2012 19:13:47 +0545
> the driver sees wether the dev_seeq pointer is having a error that can be
> read by using the PTR_ERR, and returns it at error case, other wise 0 at
> success case.
>
> the PTR_RET does the same thing, and use PTR_RET instead of redoing the
> code of PTR_RET
>
> Signed-off-by: Devendra Naga <develkernel412222@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] bnx2x: remove cast around the kmalloc in bnx2x_prev_mark_path
From: David Miller @ 2012-07-30 6:19 UTC (permalink / raw)
To: develkernel412222; +Cc: eilong, netdev
In-Reply-To: <1343567963-12389-1-git-send-email-develkernel412222@gmail.com>
From: Devendra Naga <develkernel412222@gmail.com>
Date: Sun, 29 Jul 2012 19:04:23 +0545
> casting the void pointer is redundant (Documentation/CodingStyle)
>
> Signed-off-by: Devendra Naga <develkernel412222@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] ipv4: clean up put_child
From: David Miller @ 2012-07-30 6:19 UTC (permalink / raw)
To: mlin; +Cc: netdev
In-Reply-To: <1343563203.16365.1.camel@monkey32>
From: Lin Ming <mlin@ss.pku.edu.cn>
Date: Sun, 29 Jul 2012 20:00:03 +0800
> The first parameter struct trie *t is not used anymore.
> Remove it.
>
> Signed-off-by: Lin Ming <mlin@ss.pku.edu.cn>
Applied.
^ permalink raw reply
* Re: [PATCH] ipv4: fix debug info in tnode_new
From: David Miller @ 2012-07-30 6:19 UTC (permalink / raw)
To: mlin; +Cc: netdev
In-Reply-To: <1343560795.5975.1.camel@monkey32>
From: Lin Ming <mlin@ss.pku.edu.cn>
Date: Sun, 29 Jul 2012 19:19:55 +0800
> It should print size of struct rt_trie_node * allocated instead of size
> of struct rt_trie_node.
>
> Signed-off-by: Lin Ming <mlin@ss.pku.edu.cn>
Applied.
^ permalink raw reply
* Re: [PATCH] qlge: Add offload features to vlan interfaces
From: David Miller @ 2012-07-30 6:18 UTC (permalink / raw)
To: brenohl; +Cc: jitendra.kalsaria, ron.mercer, netdev
In-Reply-To: <1343415292-49323-1-git-send-email-brenohl@br.ibm.com>
From: brenohl@br.ibm.com
Date: Fri, 27 Jul 2012 13:54:52 -0500
> This patch fills the net_device vlan_features with the proper hardware features,
> thus, improving the vlan interface performance.
>
> With the patch applied, I can see around 148% improvement on a TCP_STREAM test,
> from 3.5 Gb/s to 8.7 Gb/s. On TCP_RR, I see a 11% improvement, from 18k
> to 20. The CPU utilization is almost the same on both cases, from the comparison
> above.
>
> Signed-off-by: Breno Leitao <brenohl@br.ibm.com>
Applied.
^ permalink raw reply
* Re: [PATCH] [XFRM] Fix unexpected SA hard expiration after changing date
From: David Miller @ 2012-07-30 6:15 UTC (permalink / raw)
To: fdu; +Cc: herbert, netdev
In-Reply-To: <1343356759-24767-2-git-send-email-fdu@windriver.com>
From: Fan Du <fdu@windriver.com>
Date: Fri, 27 Jul 2012 10:39:19 +0800
> + x->curlft.add_time = now - x->saved_tmo - 1;
> + tmo = x->lft.hard_add_expires_seconds - x->saved_tmo;
> +
^^^^^^^^^^^^^^^^
> + } else
Unnecessary empty line, please remove it.
^ permalink raw reply
* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: Xufeng Zhang @ 2012-07-30 5:47 UTC (permalink / raw)
To: Vlad Yasevich
Cc: Neil Horman, xufeng zhang, sri, davem, linux-sctp, netdev,
linux-kernel
In-Reply-To: <CA+=dFzh4dFatOy42FydbVvoXB8=5CvLu5AwPvUA16h+kymq0Lw@mail.gmail.com>
On 7/30/12, Xufeng Zhang <xufengzhang.main@gmail.com> wrote:
> On 7/28/12, Vlad Yasevich <vyasevich@gmail.com> wrote:
>> here is an untested prototype of what I was talking about. This should
>> handle multiple data chunks.
>
> Yes, it works if only the end of the DATA chunk in a packet has
> invalid stream identifier
> and I have verified this patch by my test case, but what happens if
> there are multiple
> DATA chunks which have invalid stream identifier in a packet?
>
> Consider the below example:
> A packet has several chunks bundling together: "COOKIE_ECHO DATA DATA",
> both
> of the two DATA chunks have invalid stream identifier, then the
> response will be
> "COOKIE_ACK ERROR SACK ERROR", right?
I just wrote a test case for my above assumption and have verified
that SACK always
bundled before the end of the ERROR chunk if multiple error DATA
chunks happened.
So this patch didn't handle all the situations and this is really what
I suspected before.
Thanks,
Xufeng Zhang
>
>
>
> Thanks,
> Xufeng Zhang
>
>>
>> -vlad
>>
>> ---
>> include/net/sctp/command.h | 1 +
>> net/sctp/sm_sideeffect.c | 22 ++++++++++++++++++++++
>> net/sctp/sm_statefuns.c | 18 ++++++++++--------
>> 3 files changed, 33 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
>> index 712b3be..4043445 100644
>> --- a/include/net/sctp/command.h
>> +++ b/include/net/sctp/command.h
>> @@ -110,6 +110,7 @@ typedef enum {
>> SCTP_CMD_SEND_NEXT_ASCONF, /* Send the next ASCONF after ACK */
>> SCTP_CMD_PURGE_ASCONF_QUEUE, /* Purge all asconf queues.*/
>> SCTP_CMD_SET_ASOC, /* Restore association context */
>> + SCTP_CMD_GEN_BAD_STREAM, /* Issue an Invalid Stream error */
>> SCTP_CMD_LAST
>> } sctp_verb_t;
>>
>> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
>> index 1ff51c9..c5a1322 100644
>> --- a/net/sctp/sm_sideeffect.c
>> +++ b/net/sctp/sm_sideeffect.c
>> @@ -1036,6 +1036,22 @@ static void sctp_cmd_send_asconf(struct
>> sctp_association *asoc)
>> }
>> }
>>
>> +static void sctp_cmd_make_inv_stream_err(sctp_cmd_seq_t *commands,
>> + struct sctp_association *asoc,
>> + struct sctp_chunk *chunk,
>> + struct sctp_datahdr *data_hdr)
>> +{
>> + struct sctp_chunk *err;
>> +
>> + err = sctp_make_op_error(asoc, chunk, SCTP_ERROR_INV_STRM,
>> + &data_hdr->stream,
>> + sizeof(data_hdr->stream),
>> + sizeof(u16));
>> + if (err)
>> + sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
>> + SCTP_CHUNK(err));
>> +}
>> +
>>
>> /* These three macros allow us to pull the debugging code out of the
>> * main flow of sctp_do_sm() to keep attention focused on the real
>> @@ -1700,6 +1716,12 @@ static int sctp_cmd_interpreter(sctp_event_t
>> event_type,
>> asoc = cmd->obj.asoc;
>> break;
>>
>> + case SCTP_CMD_GEN_BAD_STREAM:
>> + sctp_cmd_make_inv_stream_err(commands,
>> + asoc, chunk,
>> + (struct sctp_datahdr *)cmd->obj.ptr);
>> + break;
>> +
>> default:
>> pr_warn("Impossible command: %u, %p\n",
>> cmd->verb, cmd->obj.ptr);
>> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
>> index 891f5db..57532e3 100644
>> --- a/net/sctp/sm_statefuns.c
>> +++ b/net/sctp/sm_statefuns.c
>> @@ -2972,6 +2972,12 @@ discard_noforce:
>> if (chunk->end_of_packet)
>> sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, force);
>>
>> + /* Queue the INVALID STREAM error after the SACK if one is needed. */
>> + if (SCTP_IERROR_BAD_STREAM == error) {
>> + sctp_add_cmd_sf(commands, SCTP_CMD_GEN_BAD_STREAM,
>> + SCTP_PTR(chunk->subh.data_hdr));
>> + }
>> +
>> return SCTP_DISPOSITION_DISCARD;
>> consume:
>> return SCTP_DISPOSITION_CONSUME;
>> @@ -3044,6 +3050,10 @@ sctp_disposition_t
>> sctp_sf_eat_data_fast_4_4(const struct sctp_endpoint *ep,
>> */
>> sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SHUTDOWN, SCTP_NULL());
>> sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, SCTP_FORCE());
>> + if (SCTP_IERROR_BAD_STREAM == error) {
>> + sctp_add_cmd_sf(commands, SCTP_CMD_GEN_BAD_STREAM,
>> + SCTP_PTR(chunk->subh.data_hdr));
>> + }
>> sctp_add_cmd_sf(commands, SCTP_CMD_TIMER_RESTART,
>> SCTP_TO(SCTP_EVENT_TIMEOUT_T2_SHUTDOWN));
>> }
>> @@ -6140,14 +6150,6 @@ static int sctp_eat_data(const struct
>> sctp_association *asoc,
>> if (sid >= asoc->c.sinit_max_instreams) {
>> /* Mark tsn as received even though we drop it */
>> sctp_add_cmd_sf(commands, SCTP_CMD_REPORT_TSN, SCTP_U32(tsn));
>> -
>> - err = sctp_make_op_error(asoc, chunk, SCTP_ERROR_INV_STRM,
>> - &data_hdr->stream,
>> - sizeof(data_hdr->stream),
>> - sizeof(u16));
>> - if (err)
>> - sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
>> - SCTP_CHUNK(err));
>> return SCTP_IERROR_BAD_STREAM;
>> }
>>
>> -- 1.7.7.6
>>
>>
>>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox