* Re: NULL pointer dereference at skb_queue_tail()
From: Cong Wang @ 2015-01-05 19:03 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: netdev
In-Reply-To: <201501052150.CIG69242.FFVOLQOJFHtMSO@I-love.SAKURA.ne.jp>
On Mon, Jan 5, 2015 at 4:50 AM, Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> Tetsuo Handa wrote:
>> I can reproduce below oops when testing Linux 3.18 with memory allocation
>> failure injection module at https://lkml.org/lkml/2014/12/25/64 .
>
> I can reliably reproduce this oops with current linux.git using memory
> allocation failure injection module. There is a possibility of memory
> corruption since this oops always occurs immediately after memory
> allocation failure within GPU/DRM code. I want to check whether
> fields of structures have expected values or not.
Looks like the skb->prev and/or skb->next in the skb queue is corrupted,
but I don't see why. We do play some magic on these pointers recently,
but it should not be related with unix socket at all.
Is it possible for you to check if this is a regression of recent kernel?
We only have few changes in unix socket recently, and I don't see they
could cause this bug.
>
>> void skb_queue_tail(struct sk_buff_head *list, struct sk_buff *newsk)
>> {
>> unsigned long flags;
>>
>
> Could you tell me what are expected values (i.e. what BUG_ON() test
> should I try) at this location?
>
Since skb queue has its own code to do list operations, we can't
use the existing list debugging to debug this list corruption. :(
^ permalink raw reply
* Re: [net-next PATCH v1 01/11] net: flow_table: create interface for hw match/action tables
From: John Fastabend @ 2015-01-05 18:59 UTC (permalink / raw)
To: Thomas Graf; +Cc: sfeldma, jiri, jhs, simon.horman, netdev, davem, andy
In-Reply-To: <20150104111238.GD15305@casper.infradead.org>
On 01/04/2015 03:12 AM, Thomas Graf wrote:
> On 12/31/14 at 11:45am, John Fastabend wrote:
>
> Impressive work John, some minor nits below. In general this looks
> great. How large could tables grow? Any risk one of the nested
> attribtues could exceed 16K in size because of a very large parse
> graph? Not a problem if we account for it and allow for jumbo
> attributes.
>
hmm it sounds large to me but maybe if you have an NPU that is trying
to parse into application data it could happen.
What does it take to allow for jumbo attributes?
>> +
>> +/**
>> + * @struct net_flow_header
>> + * @brief defines a match (header/field) an endpoint can use
>> + *
>> + * @uid unique identifier for header
>> + * @field_sz number of fields are in the set
>> + * @fields the set of fields in the net_flow_header
>
> FWIW, name is not documented.
thanks fixed up documentation and spacing for v2.
[...]
>> +#ifndef _UAPI_LINUX_IF_FLOW
>> +#define _UAPI_LINUX_IF_FLOW
>> +
>> +#include <linux/types.h>
>> +#include <linux/netlink.h>
>> +#include <linux/if.h>
>> +
>> +#define NET_FLOW_NAMSIZ 80
>
> Did you consider allocating the memory for names? I don't have a grasp
> for the typical number of net_flow_* instances in memory yet.
>
<100k in the devices I have. Maybe Simon can pitch in what is typical
on the NPUs I'm not sure about them.
Rocker tables can grow as large as needed at the moment.
Allocating the memory may help I'll go ahead and give it a try.
>> +/**
>> + * @struct net_flow_field_ref
>> + * @brief uniquely identify field as header:field tuple
>> + */
>> +struct net_flow_field_ref {
>> + int instance;
>> + int header;
>> + int field;
>> + int mask_type;
>> + int type;
>> + union { /* Are these all the required data types */
>> + __u8 value_u8;
>> + __u16 value_u16;
>> + __u32 value_u32;
>> + __u64 value_u64;
>> + };
>> + union { /* Are these all the required data types */
>> + __u8 mask_u8;
>> + __u16 mask_u16;
>> + __u32 mask_u32;
>> + __u64 mask_u64;
>> + };
>> +};
>
> Does it make sense to write this as follows?
Yes. I'll make this update it helps make it clear value/mask pairs are
needed.
>
> union {
> struct {
> __u8 value_u8;
> __u8 mask_u8;
> };
> struct {
> __u16 value_u16;
> __u16 mask_u16;
> };
> ...
> };
>
>> +#define NET_FLOW_TABLE_EGRESS_ROOT 1
>> +#define NET_FLOW_TABLE_INGRESS_ROOT 2
>
> Tab/space mix.
>
yep fixed.
>> +struct sk_buff *net_flow_build_actions_msg(struct net_flow_action **a,
>> + struct net_device *dev,
>> + u32 portid, int seq, u8 cmd)
>> +{
>> + struct genlmsghdr *hdr;
>> + struct sk_buff *skb;
>> + int err = -ENOBUFS;
>> +
>> + skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>
> genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
fixed along with the other cases.
>
>> +static int net_flow_put_table(struct net_device *dev,
>> + struct sk_buff *skb,
>> + struct net_flow_table *t)
>> +{
>> + struct nlattr *matches, *actions;
>> + int i;
>> +
>> + if (nla_put_string(skb, NET_FLOW_TABLE_ATTR_NAME, t->name) ||
>> + nla_put_u32(skb, NET_FLOW_TABLE_ATTR_UID, t->uid) ||
>> + nla_put_u32(skb, NET_FLOW_TABLE_ATTR_SOURCE, t->source) ||
>> + nla_put_u32(skb, NET_FLOW_TABLE_ATTR_SIZE, t->size))
>> + return -EMSGSIZE;
>> +
>> + matches = nla_nest_start(skb, NET_FLOW_TABLE_ATTR_MATCHES);
>> + if (!matches)
>> + return -EMSGSIZE;
>> +
>> + for (i = 0; t->matches[i].instance; i++)
>> + nla_put(skb, NET_FLOW_FIELD_REF,
>> + sizeof(struct net_flow_field_ref),
>> + &t->matches[i]);
>
> Unhandled nla_put() error
>
thanks.
[...]
>> +static int net_flow_put_headers(struct sk_buff *skb,
>> + struct net_flow_header **headers)
>> +{
>> + struct nlattr *nest, *hdr, *fields;
>> + struct net_flow_header *h;
>> + int i, err;
>> +
>> + nest = nla_nest_start(skb, NET_FLOW_HEADERS);
>> + if (!nest)
>> + return -EMSGSIZE;
>> +
>> + for (i = 0; headers[i]->uid; i++) {
>> + err = -EMSGSIZE;
>> + h = headers[i];
>> +
>> + hdr = nla_nest_start(skb, NET_FLOW_HEADER);
>> + if (!hdr)
>> + goto hdr_put_failure;
>> +
>> + if (nla_put_string(skb, NET_FLOW_HEADER_ATTR_NAME, h->name) ||
>> + nla_put_u32(skb, NET_FLOW_HEADER_ATTR_UID, h->uid))
>> + goto attr_put_failure;
>> +
>> + fields = nla_nest_start(skb, NET_FLOW_HEADER_ATTR_FIELDS);
>> + if (!fields)
>> + goto attr_put_failure;
>
> You can jump to hdr_put_failure right away and get rid of the
> attr_put_failure target as you cancel that nest anyway. You can apply
> this comment to several other places as well if you want.
>
OK so to simplify the error paths we only need to cancel the outer most
nested attribute. I'll do this transformation.
.John
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: [PATCH] Revert "ipw2200: select CFG80211_WEXT"
From: Johannes Berg @ 2015-01-05 18:57 UTC (permalink / raw)
To: Paul Bolle
Cc: Arend van Spriel, Linus Torvalds, Marcel Holtmann,
Stanislav Yakovlev, Kalle Valo, Jiri Kosina, linux-wireless,
Network Development, Linux Kernel Mailing List
In-Reply-To: <1420479510.14308.23.camel@x220>
On Mon, 2015-01-05 at 18:38 +0100, Paul Bolle wrote:
> > ipw2200 is a WEXT driver using some wext functionality (and struct
> > wiphy) provided by cfg80211 hence it needs CFG80211_WEXT. I guess that
> > is what makes it confusing.
>
> It doesn't help that I hardly know anything about mac80211, cfg80211 and
> nl80211 (and lib80211 for that matter). To me these are mostly just
> names that end in 80211.
:-)
There isn't really all that much that ipw2x00 is using from cfg80211
though - of note is that it has completely empty ops:
static struct cfg80211_ops libipw_config_ops = { };
IOW - all it does is register with the framework - courtesy of
a3caa99e6c68f. It's practically useless.
> Anyhow, concerning, CFG80211_WEXT: it seems the only functionality
> provided by that symbol that ipw2200 uses directly is
> cfg80211_wext_giwname(). Perhaps ipw2200 could have a private version of
> that function, something like ipw2100's ipw2100_wx_get_name(). Should be
> trivial to implement (ie, it could take _me_ a day or two).
We could just revert that part of the commit above - or even completely.
However, in theory at least doing *that* would now be a userspace
regression - today you can at least discover the presence of ipw2200
devices with nl80211, even if you can't do anything with them that way.
> But perhaps ipw2200 uses CFG80211_WEXT _indirectly_ too. Ie, in
> net/wireless/core.c I stumbled on
> #ifdef CONFIG_CFG80211_WEXT
> rdev->wiphy.wext = &cfg80211_wext_handler;
> #endif
I don't think it does - see the note about the ops above. If it did,
it'd have to implement the ops.
> But I net/wireless/wext-core.c I then found
> #ifdef CONFIG_CFG80211_WEXT
> if (dev->ieee80211_ptr && dev->ieee80211_ptr->wiphy)
> handlers = dev->ieee80211_ptr->wiphy->wext;
> #endif
> #ifdef CONFIG_WIRELESS_EXT
> if (dev->wireless_handlers)
> handlers = dev->wireless_handlers;
> #endif
>
> (There's much more to discover about WEXT, of course.) Anyhow, IPW2200
> uses both CFG80211_WEXT and WIRELESS_EXT and cfg80211_wext_handler and
> ipw2200's wireless_handlers appear to cover the same set of IOCTLS (one
> exception: SIOCSIWPMKSA). So by now I'm really puzzled how this all fits
> together.
Well, this was meant as a transition mechanism for drivers. Ultimately,
the way we thought how you'd convert a driver (and how we converted
mac80211) would be to have the wext handlers like for example the scan
ones:
static iw_handler ipw_wx_handlers[] = {
...
IW_HANDLER(SIOCSIWSCAN, ipw_wx_set_scan),
IW_HANDLER(SIOCGIWSCAN, ipw_wx_get_scan),
...
};
Then you could make a patch that uses the cfg80211 APIs for scanning in
the driver -- i.e. implement the cfg80211_ops.scan method, report frames
to the cfg80211 scanning and remove all the ieee->network_list stuff
etc. using the related cfg80211 API (e.g. cfg80211_get_bss() and friends
for getting a network, etc.) And then you'd change the handlers to be
static iw_handler ipw_wx_handlers[] = {
...
IW_HANDLER(SIOCSIWSCAN, cfg80211_wext_siwscan),
IW_HANDLER(SIOCGIWSCAN, cfg80211_wext_giwscan),
...
};
This part would have to be done in a single patch.
Multiple other groups of ioctls could be converted in similar patches,
until at the end you can completely remove ipw_wx_handlers and rely
entirely on cfg80211's wext compatibility.
So far the theory - in practice nobody cared enough to start working on
any of these drivers, let alone actually has the hardware today.
johannes
^ permalink raw reply
* [PATCH v3 4/4] can: kvaser_usb: Add support for the Usbcan-II family
From: Ahmed S. Darwish @ 2015-01-05 18:31 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20150105175713.GC6304@linux>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
CAN to USB interfaces sold by the Swedish manufacturer Kvaser are
divided into two major families: 'Leaf', and 'UsbcanII'. From an
Operating System perspective, the firmware of both families behave
in a not too drastically different fashion.
This patch adds support for the UsbcanII family of devices to the
current Kvaser Leaf-only driver.
CAN frames sending, receiving, and error handling paths has been
tested using the dual-channel "Kvaser USBcan II HS/LS" dongle. It
should also work nicely with other products in the same category.
List of new devices supported by this driver update:
- Kvaser USBcan II HS/HS
- Kvaser USBcan II HS/LS
- Kvaser USBcan Rugged ("USBcan Rev B")
- Kvaser Memorator HS/HS
- Kvaser Memorator HS/LS
- Scania VCI2 (if you have the Kvaser logo on top)
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
drivers/net/can/usb/Kconfig | 8 +-
drivers/net/can/usb/kvaser_usb.c | 618 +++++++++++++++++++++++++++++++--------
2 files changed, 503 insertions(+), 123 deletions(-)
** V2 Changelog:
- Update Kconfig entries
- Use actual number of CAN channels (instead of max) where appropriate
- Rebase over a new set of UsbcanII-independent driver fixes
** V3 Changelog:
- Fix padding for the usbcan_msg_tx_acknowledge command
- Remove kvaser_usb->max_channels and the MAX_NET_DEVICES macro
- Rename commands to CMD_LEAF_xxx and CMD_USBCAN_xxx
- Apply checkpatch.pl suggestions ('net/' comments, multi-line strings, etc.)
diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
index a77db919..f6f5500 100644
--- a/drivers/net/can/usb/Kconfig
+++ b/drivers/net/can/usb/Kconfig
@@ -25,7 +25,7 @@ config CAN_KVASER_USB
tristate "Kvaser CAN/USB interface"
---help---
This driver adds support for Kvaser CAN/USB devices like Kvaser
- Leaf Light.
+ Leaf Light and Kvaser USBcan II.
The driver provides support for the following devices:
- Kvaser Leaf Light
@@ -46,6 +46,12 @@ config CAN_KVASER_USB
- Kvaser USBcan R
- Kvaser Leaf Light v2
- Kvaser Mini PCI Express HS
+ - Kvaser USBcan II HS/HS
+ - Kvaser USBcan II HS/LS
+ - Kvaser USBcan Rugged ("USBcan Rev B")
+ - Kvaser Memorator HS/HS
+ - Kvaser Memorator HS/LS
+ - Scania VCI2 (if you have the Kvaser logo on top)
If unsure, say N.
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index cc7bfc0..43b3928 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -6,10 +6,12 @@
* Parts of this driver are based on the following:
* - Kvaser linux leaf driver (version 4.78)
* - CAN driver for esd CAN-USB/2
+ * - Kvaser linux usbcanII driver (version 5.3)
*
* Copyright (C) 2002-2006 KVASER AB, Sweden. All rights reserved.
* Copyright (C) 2010 Matthias Fuchs <matthias.fuchs@esd.eu>, esd gmbh
* Copyright (C) 2012 Olivier Sobrie <olivier@sobrie.be>
+ * Copyright (C) 2015 Valeo Corporation
*/
#include <linux/completion.h>
@@ -21,6 +23,15 @@
#include <linux/can/dev.h>
#include <linux/can/error.h>
+/* Kvaser USB CAN dongles are divided into two major families:
+ * - Leaf: Based on Renesas M32C, running firmware labeled as 'filo'
+ * - UsbcanII: Based on Renesas M16C, running firmware labeled as 'helios'
+ */
+enum kvaser_usb_family {
+ KVASER_LEAF,
+ KVASER_USBCAN,
+};
+
#define MAX_TX_URBS 16
#define MAX_RX_URBS 4
#define START_TIMEOUT 1000 /* msecs */
@@ -30,8 +41,9 @@
#define RX_BUFFER_SIZE 3072
#define CAN_USB_CLOCK 8000000
#define MAX_NET_DEVICES 3
+#define MAX_USBCAN_NET_DEVICES 2
-/* Kvaser USB devices */
+/* Leaf USB devices */
#define KVASER_VENDOR_ID 0x0bfd
#define USB_LEAF_DEVEL_PRODUCT_ID 10
#define USB_LEAF_LITE_PRODUCT_ID 11
@@ -55,6 +67,16 @@
#define USB_CAN_R_PRODUCT_ID 39
#define USB_LEAF_LITE_V2_PRODUCT_ID 288
#define USB_MINI_PCIE_HS_PRODUCT_ID 289
+#define LEAF_PRODUCT_ID(id) (id >= USB_LEAF_DEVEL_PRODUCT_ID && \
+ id <= USB_MINI_PCIE_HS_PRODUCT_ID)
+
+/* USBCANII devices */
+#define USB_USBCAN_REVB_PRODUCT_ID 2
+#define USB_VCI2_PRODUCT_ID 3
+#define USB_USBCAN2_PRODUCT_ID 4
+#define USB_MEMORATOR_PRODUCT_ID 5
+#define USBCAN_PRODUCT_ID(id) (id >= USB_USBCAN_REVB_PRODUCT_ID && \
+ id <= USB_MEMORATOR_PRODUCT_ID)
/* USB devices features */
#define KVASER_HAS_SILENT_MODE BIT(0)
@@ -73,7 +95,7 @@
#define MSG_FLAG_TX_ACK BIT(6)
#define MSG_FLAG_TX_REQUEST BIT(7)
-/* Can states */
+/* Can states (M16C CxSTRH register) */
#define M16C_STATE_BUS_RESET BIT(0)
#define M16C_STATE_BUS_ERROR BIT(4)
#define M16C_STATE_BUS_PASSIVE BIT(5)
@@ -98,7 +120,13 @@
#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_READ_CLOCK 30
+#define CMD_READ_CLOCK_REPLY 31
+
+#define CMD_LEAF_GET_CARD_INFO2 32
+#define CMD_USBCAN_RESET_CLOCK 32
+#define CMD_USBCAN_CLOCK_OVERFLOW_EVENT 33
+
#define CMD_GET_CARD_INFO 34
#define CMD_GET_CARD_INFO_REPLY 35
#define CMD_GET_SOFTWARE_INFO 38
@@ -108,8 +136,9 @@
#define CMD_RESET_ERROR_COUNTER 49
#define CMD_TX_ACKNOWLEDGE 50
#define CMD_CAN_ERROR_EVENT 51
-#define CMD_USB_THROTTLE 77
-#define CMD_LOG_MESSAGE 106
+
+#define CMD_LEAF_USB_THROTTLE 77
+#define CMD_LEAF_LOG_MESSAGE 106
/* error factors */
#define M16C_EF_ACKE BIT(0)
@@ -121,6 +150,14 @@
#define M16C_EF_RCVE BIT(6)
#define M16C_EF_TRE BIT(7)
+/* Only Leaf-based devices can report M16C error factors,
+ * thus define our own error status flags for USBCANII
+ */
+#define USBCAN_ERROR_STATE_NONE 0
+#define USBCAN_ERROR_STATE_TX_ERROR BIT(0)
+#define USBCAN_ERROR_STATE_RX_ERROR BIT(1)
+#define USBCAN_ERROR_STATE_BUSERROR BIT(2)
+
/* bittiming parameters */
#define KVASER_USB_TSEG1_MIN 1
#define KVASER_USB_TSEG1_MAX 16
@@ -137,7 +174,7 @@
#define KVASER_CTRL_MODE_SELFRECEPTION 3
#define KVASER_CTRL_MODE_OFF 4
-/* log message */
+/* Extended CAN identifier flag */
#define KVASER_EXTENDED_FRAME BIT(31)
struct kvaser_msg_simple {
@@ -148,30 +185,55 @@ struct kvaser_msg_simple {
struct kvaser_msg_cardinfo {
u8 tid;
u8 nchannels;
- __le32 serial_number;
- __le32 padding;
+ union {
+ struct {
+ __le32 serial_number;
+ __le32 padding;
+ } __packed leaf0;
+ struct {
+ __le32 serial_number_low;
+ __le32 serial_number_high;
+ } __packed usbcan0;
+ } __packed;
__le32 clock_resolution;
__le32 mfgdate;
u8 ean[8];
u8 hw_revision;
- u8 usb_hs_mode;
- __le16 padding2;
+ union {
+ struct {
+ u8 usb_hs_mode;
+ } __packed leaf1;
+ struct {
+ u8 padding;
+ } __packed usbcan1;
+ } __packed;
+ __le16 padding;
} __packed;
struct kvaser_msg_cardinfo2 {
u8 tid;
- u8 channel;
+ u8 reserved;
u8 pcb_id[24];
__le32 oem_unlock_code;
} __packed;
-struct kvaser_msg_softinfo {
+struct leaf_msg_softinfo {
u8 tid;
- u8 channel;
+ u8 padding0;
__le32 sw_options;
__le32 fw_version;
__le16 max_outstanding_tx;
- __le16 padding[9];
+ __le16 padding1[9];
+} __packed;
+
+struct usbcan_msg_softinfo {
+ u8 tid;
+ u8 fw_name[5];
+ __le16 max_outstanding_tx;
+ u8 padding[6];
+ __le32 fw_version;
+ __le16 checksum;
+ __le16 sw_options;
} __packed;
struct kvaser_msg_busparams {
@@ -188,36 +250,86 @@ struct kvaser_msg_tx_can {
u8 channel;
u8 tid;
u8 msg[14];
- u8 padding;
- u8 flags;
+ union {
+ struct {
+ u8 padding;
+ u8 flags;
+ } __packed leaf;
+ struct {
+ u8 flags;
+ u8 padding;
+ } __packed usbcan;
+ } __packed;
} __packed;
-struct kvaser_msg_rx_can {
+struct kvaser_msg_rx_can_header {
u8 channel;
u8 flag;
+} __packed;
+
+struct leaf_msg_rx_can {
+ u8 channel;
+ u8 flag;
+
__le16 time[3];
u8 msg[14];
} __packed;
-struct kvaser_msg_chip_state_event {
+struct usbcan_msg_rx_can {
+ u8 channel;
+ u8 flag;
+
+ u8 msg[14];
+ __le16 time;
+} __packed;
+
+struct leaf_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 usbcan_msg_chip_state_event {
+ u8 tid;
+ u8 channel;
+
+ u8 tx_errors_count;
+ u8 rx_errors_count;
+ __le16 time;
+
u8 status;
u8 padding[3];
} __packed;
-struct kvaser_msg_tx_acknowledge {
+struct kvaser_msg_tx_acknowledge_header {
+ u8 channel;
+ u8 tid;
+};
+
+struct leaf_msg_tx_acknowledge {
u8 channel;
u8 tid;
+
__le16 time[3];
u8 flags;
u8 time_offset;
} __packed;
-struct kvaser_msg_error_event {
+struct usbcan_msg_tx_acknowledge {
+ u8 channel;
+ u8 tid;
+
+ __le16 time;
+ __le16 padding;
+} __packed;
+
+struct leaf_msg_error_event {
u8 tid;
u8 flags;
__le16 time[3];
@@ -229,6 +341,18 @@ struct kvaser_msg_error_event {
u8 error_factor;
} __packed;
+struct usbcan_msg_error_event {
+ u8 tid;
+ u8 padding;
+ u8 tx_errors_count_ch0;
+ u8 rx_errors_count_ch0;
+ u8 tx_errors_count_ch1;
+ u8 rx_errors_count_ch1;
+ u8 status_ch0;
+ u8 status_ch1;
+ __le16 time;
+} __packed;
+
struct kvaser_msg_ctrl_mode {
u8 tid;
u8 channel;
@@ -243,7 +367,7 @@ struct kvaser_msg_flush_queue {
u8 padding[3];
} __packed;
-struct kvaser_msg_log_message {
+struct leaf_msg_log_message {
u8 channel;
u8 flags;
__le16 time[3];
@@ -260,19 +384,50 @@ struct kvaser_msg {
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_rx_can_header rx_can_header;
+ struct kvaser_msg_tx_acknowledge_header tx_acknowledge_header;
+
+ union {
+ struct leaf_msg_softinfo softinfo;
+ struct leaf_msg_rx_can rx_can;
+ struct leaf_msg_chip_state_event chip_state_event;
+ struct leaf_msg_tx_acknowledge tx_acknowledge;
+ struct leaf_msg_error_event error_event;
+ struct leaf_msg_log_message log_message;
+ } __packed leaf;
+
+ union {
+ struct usbcan_msg_softinfo softinfo;
+ struct usbcan_msg_rx_can rx_can;
+ struct usbcan_msg_chip_state_event chip_state_event;
+ struct usbcan_msg_tx_acknowledge tx_acknowledge;
+ struct usbcan_msg_error_event error_event;
+ } __packed usbcan;
+
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_flush_queue flush_queue;
- struct kvaser_msg_log_message log_message;
} u;
} __packed;
+/* Leaf/USBCAN-agnostic summary of an error event.
+ * No M16C error factors for USBCAN-based devices.
+ */
+struct kvaser_error_summary {
+ u8 channel, status, txerr, rxerr;
+ union {
+ struct {
+ u8 error_factor;
+ } leaf;
+ struct {
+ u8 other_ch_status;
+ u8 error_state;
+ } usbcan;
+ };
+};
+
struct kvaser_usb_tx_urb_context {
struct kvaser_usb_net_priv *priv;
u32 echo_index;
@@ -288,6 +443,7 @@ struct kvaser_usb {
u32 fw_version;
unsigned int nchannels;
+ enum kvaser_usb_family family;
bool rxinitdone;
void *rxbuf[MAX_RX_URBS];
@@ -311,6 +467,7 @@ struct kvaser_usb_net_priv {
};
static const struct usb_device_id kvaser_usb_table[] = {
+ /* Leaf family IDs */
{ 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),
@@ -360,6 +517,17 @@ static const struct usb_device_id kvaser_usb_table[] = {
.driver_info = KVASER_HAS_TXRX_ERRORS },
{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_V2_PRODUCT_ID) },
{ USB_DEVICE(KVASER_VENDOR_ID, USB_MINI_PCIE_HS_PRODUCT_ID) },
+
+ /* USBCANII family IDs */
+ { USB_DEVICE(KVASER_VENDOR_ID, USB_USBCAN2_PRODUCT_ID),
+ .driver_info = KVASER_HAS_TXRX_ERRORS },
+ { USB_DEVICE(KVASER_VENDOR_ID, USB_USBCAN_REVB_PRODUCT_ID),
+ .driver_info = KVASER_HAS_TXRX_ERRORS },
+ { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMORATOR_PRODUCT_ID),
+ .driver_info = KVASER_HAS_TXRX_ERRORS },
+ { USB_DEVICE(KVASER_VENDOR_ID, USB_VCI2_PRODUCT_ID),
+ .driver_info = KVASER_HAS_TXRX_ERRORS },
+
{ }
};
MODULE_DEVICE_TABLE(usb, kvaser_usb_table);
@@ -463,7 +631,18 @@ static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
if (err)
return err;
- dev->fw_version = le32_to_cpu(msg.u.softinfo.fw_version);
+ switch (dev->family) {
+ case KVASER_LEAF:
+ dev->fw_version = le32_to_cpu(msg.u.leaf.softinfo.fw_version);
+ break;
+ case KVASER_USBCAN:
+ dev->fw_version = le32_to_cpu(msg.u.usbcan.softinfo.fw_version);
+ break;
+ default:
+ dev_err(dev->udev->dev.parent,
+ "Invalid device family (%d)\n", dev->family);
+ return -EINVAL;
+ }
return 0;
}
@@ -484,6 +663,9 @@ static int kvaser_usb_get_card_info(struct kvaser_usb *dev)
dev->nchannels = msg.u.cardinfo.nchannels;
if (dev->nchannels > MAX_NET_DEVICES)
return -EINVAL;
+ if (dev->family == KVASER_USBCAN &&
+ dev->nchannels > MAX_USBCAN_NET_DEVICES)
+ return -EINVAL;
return 0;
}
@@ -496,8 +678,10 @@ static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
struct kvaser_usb_net_priv *priv;
struct sk_buff *skb;
struct can_frame *cf;
- u8 channel = msg->u.tx_acknowledge.channel;
- u8 tid = msg->u.tx_acknowledge.tid;
+ u8 channel, tid;
+
+ channel = msg->u.tx_acknowledge_header.channel;
+ tid = msg->u.tx_acknowledge_header.tid;
if (channel >= dev->nchannels) {
dev_err(dev->udev->dev.parent,
@@ -615,37 +799,80 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
priv->tx_contexts[i].echo_index = MAX_TX_URBS;
}
-static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
- const struct kvaser_msg *msg)
+static void kvaser_report_error_event(const struct kvaser_usb *dev,
+ struct kvaser_error_summary *es);
+
+/* Report error to userspace iff the controller's errors counter has
+ * increased, or we're the only channel seeing the bus error state.
+ *
+ * As reported by USBCAN sheets, "the CAN controller has difficulties
+ * to tell whether an error frame arrived on channel 1 or on channel 2."
+ * Thus, error counters are compared with their earlier values to
+ * determine which channel was responsible for the error event.
+ */
+static void usbcan_report_error_if_applicable(const struct kvaser_usb *dev,
+ struct kvaser_error_summary *es)
{
- struct can_frame *cf;
- struct sk_buff *skb;
- struct net_device_stats *stats;
struct kvaser_usb_net_priv *priv;
- unsigned int new_state;
- u8 channel, status, txerr, rxerr, error_factor;
+ int old_tx_err_count, old_rx_err_count, channel, report_error;
+
+ channel = es->channel;
+ if (channel >= dev->nchannels) {
+ dev_err(dev->udev->dev.parent,
+ "Invalid channel number (%d)\n", channel);
+ return;
+ }
+
+ priv = dev->nets[channel];
+ old_tx_err_count = priv->bec.txerr;
+ old_rx_err_count = priv->bec.rxerr;
+
+ report_error = 0;
+ if (es->txerr > old_tx_err_count) {
+ es->usbcan.error_state |= USBCAN_ERROR_STATE_TX_ERROR;
+ report_error = 1;
+ }
+ if (es->rxerr > old_rx_err_count) {
+ es->usbcan.error_state |= USBCAN_ERROR_STATE_RX_ERROR;
+ report_error = 1;
+ }
+ if ((es->status & M16C_STATE_BUS_ERROR) &&
+ !(es->usbcan.other_ch_status & M16C_STATE_BUS_ERROR)) {
+ es->usbcan.error_state |= USBCAN_ERROR_STATE_BUSERROR;
+ report_error = 1;
+ }
+
+ if (report_error)
+ kvaser_report_error_event(dev, es);
+}
+
+/* Extract error summary from a Leaf-based device error message */
+static void leaf_extract_error_from_msg(const struct kvaser_usb *dev,
+ const struct kvaser_msg *msg)
+{
+ struct kvaser_error_summary es = { 0, };
switch (msg->id) {
case 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;
- error_factor = msg->u.error_event.error_factor;
+ es.channel = msg->u.leaf.error_event.channel;
+ es.status = msg->u.leaf.error_event.status;
+ es.txerr = msg->u.leaf.error_event.tx_errors_count;
+ es.rxerr = msg->u.leaf.error_event.rx_errors_count;
+ es.leaf.error_factor = msg->u.leaf.error_event.error_factor;
break;
- case CMD_LOG_MESSAGE:
- channel = msg->u.log_message.channel;
- status = msg->u.log_message.data[0];
- txerr = msg->u.log_message.data[2];
- rxerr = msg->u.log_message.data[3];
- error_factor = msg->u.log_message.data[1];
+ case CMD_LEAF_LOG_MESSAGE:
+ es.channel = msg->u.leaf.log_message.channel;
+ es.status = msg->u.leaf.log_message.data[0];
+ es.txerr = msg->u.leaf.log_message.data[2];
+ es.rxerr = msg->u.leaf.log_message.data[3];
+ es.leaf.error_factor = msg->u.leaf.log_message.data[1];
break;
case CMD_CHIP_STATE_EVENT:
- 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;
- error_factor = 0;
+ es.channel = msg->u.leaf.chip_state_event.channel;
+ es.status = msg->u.leaf.chip_state_event.status;
+ es.txerr = msg->u.leaf.chip_state_event.tx_errors_count;
+ es.rxerr = msg->u.leaf.chip_state_event.rx_errors_count;
+ es.leaf.error_factor = 0;
break;
default:
dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
@@ -653,16 +880,92 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
return;
}
- if (channel >= dev->nchannels) {
+ kvaser_report_error_event(dev, &es);
+}
+
+/* Extract error summary from a USBCANII-based device error message */
+static void usbcan_extract_error_from_msg(const struct kvaser_usb *dev,
+ const struct kvaser_msg *msg)
+{
+ struct kvaser_error_summary es = { 0, };
+
+ switch (msg->id) {
+ /* Sometimes errors are sent as unsolicited chip state events */
+ case CMD_CHIP_STATE_EVENT:
+ es.channel = msg->u.usbcan.chip_state_event.channel;
+ es.status = msg->u.usbcan.chip_state_event.status;
+ es.txerr = msg->u.usbcan.chip_state_event.tx_errors_count;
+ es.rxerr = msg->u.usbcan.chip_state_event.rx_errors_count;
+ usbcan_report_error_if_applicable(dev, &es);
+ break;
+
+ case CMD_CAN_ERROR_EVENT:
+ es.channel = 0;
+ es.status = msg->u.usbcan.error_event.status_ch0;
+ es.txerr = msg->u.usbcan.error_event.tx_errors_count_ch0;
+ es.rxerr = msg->u.usbcan.error_event.rx_errors_count_ch0;
+ es.usbcan.other_ch_status =
+ msg->u.usbcan.error_event.status_ch1;
+ usbcan_report_error_if_applicable(dev, &es);
+
+ /* For error events, the USBCAN firmware does not support
+ * more than 2 channels: ch0, and ch1.
+ */
+ if (dev->nchannels > 1) {
+ es.channel = 1;
+ es.status = msg->u.usbcan.error_event.status_ch1;
+ es.txerr =
+ msg->u.usbcan.error_event.tx_errors_count_ch1;
+ es.rxerr =
+ msg->u.usbcan.error_event.rx_errors_count_ch1;
+ es.usbcan.other_ch_status =
+ msg->u.usbcan.error_event.status_ch0;
+ usbcan_report_error_if_applicable(dev, &es);
+ }
+ break;
+
+ default:
+ dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
+ msg->id);
+ }
+}
+
+static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
+ const struct kvaser_msg *msg)
+{
+ switch (dev->family) {
+ case KVASER_LEAF:
+ leaf_extract_error_from_msg(dev, msg);
+ break;
+ case KVASER_USBCAN:
+ usbcan_extract_error_from_msg(dev, msg);
+ break;
+ default:
dev_err(dev->udev->dev.parent,
- "Invalid channel number (%d)\n", channel);
+ "Invalid device family (%d)\n", dev->family);
return;
}
+}
- priv = dev->nets[channel];
+static void kvaser_report_error_event(const struct kvaser_usb *dev,
+ struct kvaser_error_summary *es)
+{
+ struct can_frame *cf;
+ struct sk_buff *skb;
+ struct net_device_stats *stats;
+ struct kvaser_usb_net_priv *priv;
+ unsigned int new_state;
+
+ if (es->channel >= dev->nchannels) {
+ dev_err(dev->udev->dev.parent,
+ "Invalid channel number (%d)\n", es->channel);
+ return;
+ }
+
+ priv = dev->nets[es->channel];
stats = &priv->netdev->stats;
- if (status & M16C_STATE_BUS_RESET) {
+ if (es->status & M16C_STATE_BUS_RESET) {
kvaser_usb_unlink_tx_urbs(priv);
return;
}
@@ -675,9 +978,9 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
new_state = priv->can.state;
- netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status);
+ netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
- if (status & M16C_STATE_BUS_OFF) {
+ if (es->status & M16C_STATE_BUS_OFF) {
cf->can_id |= CAN_ERR_BUSOFF;
priv->can.can_stats.bus_off++;
@@ -687,12 +990,12 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
netif_carrier_off(priv->netdev);
new_state = CAN_STATE_BUS_OFF;
- } else if (status & M16C_STATE_BUS_PASSIVE) {
+ } else if (es->status & M16C_STATE_BUS_PASSIVE) {
if (priv->can.state != CAN_STATE_ERROR_PASSIVE) {
cf->can_id |= CAN_ERR_CRTL;
- if (txerr || rxerr)
- cf->data[1] = (txerr > rxerr)
+ if (es->txerr || es->rxerr)
+ cf->data[1] = (es->txerr > es->rxerr)
? CAN_ERR_CRTL_TX_PASSIVE
: CAN_ERR_CRTL_RX_PASSIVE;
else
@@ -703,13 +1006,11 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
}
new_state = CAN_STATE_ERROR_PASSIVE;
- }
-
- if (status == M16C_STATE_BUS_ERROR) {
+ } else if (es->status & M16C_STATE_BUS_ERROR) {
if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
- ((txerr >= 96) || (rxerr >= 96))) {
+ ((es->txerr >= 96) || (es->rxerr >= 96))) {
cf->can_id |= CAN_ERR_CRTL;
- cf->data[1] = (txerr > rxerr)
+ cf->data[1] = (es->txerr > es->rxerr)
? CAN_ERR_CRTL_TX_WARNING
: CAN_ERR_CRTL_RX_WARNING;
@@ -723,7 +1024,7 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
}
}
- if (!status) {
+ if (!es->status) {
cf->can_id |= CAN_ERR_PROT;
cf->data[2] = CAN_ERR_PROT_ACTIVE;
@@ -739,34 +1040,52 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
priv->can.can_stats.restarts++;
}
- if (error_factor) {
- priv->can.can_stats.bus_error++;
- stats->rx_errors++;
-
- cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
-
- if (error_factor & M16C_EF_ACKE)
- cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
- 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;
+ switch (dev->family) {
+ case KVASER_LEAF:
+ if (es->leaf.error_factor) {
+ priv->can.can_stats.bus_error++;
+ stats->rx_errors++;
+
+ cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
+
+ if (es->leaf.error_factor & M16C_EF_ACKE)
+ cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
+ if (es->leaf.error_factor & M16C_EF_CRCE)
+ cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
+ CAN_ERR_PROT_LOC_CRC_DEL);
+ if (es->leaf.error_factor & M16C_EF_FORME)
+ cf->data[2] |= CAN_ERR_PROT_FORM;
+ if (es->leaf.error_factor & M16C_EF_STFE)
+ cf->data[2] |= CAN_ERR_PROT_STUFF;
+ if (es->leaf.error_factor & M16C_EF_BITE0)
+ cf->data[2] |= CAN_ERR_PROT_BIT0;
+ if (es->leaf.error_factor & M16C_EF_BITE1)
+ cf->data[2] |= CAN_ERR_PROT_BIT1;
+ if (es->leaf.error_factor & M16C_EF_TRE)
+ cf->data[2] |= CAN_ERR_PROT_TX;
+ }
+ break;
+ case KVASER_USBCAN:
+ if (es->usbcan.error_state & USBCAN_ERROR_STATE_TX_ERROR)
+ stats->tx_errors++;
+ if (es->usbcan.error_state & USBCAN_ERROR_STATE_RX_ERROR)
+ stats->rx_errors++;
+ if (es->usbcan.error_state & USBCAN_ERROR_STATE_BUSERROR) {
+ priv->can.can_stats.bus_error++;
+ cf->can_id |= CAN_ERR_BUSERROR;
+ }
+ break;
+ default:
+ dev_err(dev->udev->dev.parent,
+ "Invalid device family (%d)\n", dev->family);
+ goto err;
}
- cf->data[6] = txerr;
- cf->data[7] = rxerr;
+ cf->data[6] = es->txerr;
+ cf->data[7] = es->rxerr;
- priv->bec.txerr = txerr;
- priv->bec.rxerr = rxerr;
+ priv->bec.txerr = es->txerr;
+ priv->bec.rxerr = es->rxerr;
priv->can.state = new_state;
@@ -774,6 +1093,11 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
+
+ return;
+
+err:
+ dev_kfree_skb(skb);
}
static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
@@ -783,16 +1107,16 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
struct sk_buff *skb;
struct net_device_stats *stats = &priv->netdev->stats;
- if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
+ if (msg->u.rx_can_header.flag & (MSG_FLAG_ERROR_FRAME |
MSG_FLAG_NERR)) {
netdev_err(priv->netdev, "Unknow error (flags: 0x%02x)\n",
- msg->u.rx_can.flag);
+ msg->u.rx_can_header.flag);
stats->rx_errors++;
return;
}
- if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
+ if (msg->u.rx_can_header.flag & MSG_FLAG_OVERRUN) {
skb = alloc_can_err_skb(priv->netdev, &cf);
if (!skb) {
stats->rx_dropped++;
@@ -819,7 +1143,8 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
struct can_frame *cf;
struct sk_buff *skb;
struct net_device_stats *stats;
- u8 channel = msg->u.rx_can.channel;
+ u8 channel = msg->u.rx_can_header.channel;
+ const u8 *rx_msg;
if (channel >= dev->nchannels) {
dev_err(dev->udev->dev.parent,
@@ -830,19 +1155,32 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
priv = dev->nets[channel];
stats = &priv->netdev->stats;
- if ((msg->u.rx_can.flag & MSG_FLAG_ERROR_FRAME) &&
- (msg->id == CMD_LOG_MESSAGE)) {
+ if ((msg->u.rx_can_header.flag & MSG_FLAG_ERROR_FRAME) &&
+ (dev->family == KVASER_LEAF && msg->id == CMD_LEAF_LOG_MESSAGE)) {
kvaser_usb_rx_error(dev, msg);
return;
- } else if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
- MSG_FLAG_NERR |
- MSG_FLAG_OVERRUN)) {
+ } else if (msg->u.rx_can_header.flag & (MSG_FLAG_ERROR_FRAME |
+ MSG_FLAG_NERR |
+ MSG_FLAG_OVERRUN)) {
kvaser_usb_rx_can_err(priv, msg);
return;
- } else if (msg->u.rx_can.flag & ~MSG_FLAG_REMOTE_FRAME) {
+ } else if (msg->u.rx_can_header.flag & ~MSG_FLAG_REMOTE_FRAME) {
netdev_warn(priv->netdev,
"Unhandled frame (flags: 0x%02x)",
- msg->u.rx_can.flag);
+ msg->u.rx_can_header.flag);
+ return;
+ }
+
+ switch (dev->family) {
+ case KVASER_LEAF:
+ rx_msg = msg->u.leaf.rx_can.msg;
+ break;
+ case KVASER_USBCAN:
+ rx_msg = msg->u.usbcan.rx_can.msg;
+ break;
+ default:
+ dev_err(dev->udev->dev.parent,
+ "Invalid device family (%d)\n", dev->family);
return;
}
@@ -852,38 +1190,37 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
return;
}
- if (msg->id == CMD_LOG_MESSAGE) {
- cf->can_id = le32_to_cpu(msg->u.log_message.id);
+ if (dev->family == KVASER_LEAF && msg->id == CMD_LEAF_LOG_MESSAGE) {
+ cf->can_id = le32_to_cpu(msg->u.leaf.log_message.id);
if (cf->can_id & KVASER_EXTENDED_FRAME)
cf->can_id &= CAN_EFF_MASK | CAN_EFF_FLAG;
else
cf->can_id &= CAN_SFF_MASK;
- cf->can_dlc = get_can_dlc(msg->u.log_message.dlc);
+ cf->can_dlc = get_can_dlc(msg->u.leaf.log_message.dlc);
- if (msg->u.log_message.flags & MSG_FLAG_REMOTE_FRAME)
+ if (msg->u.leaf.log_message.flags & MSG_FLAG_REMOTE_FRAME)
cf->can_id |= CAN_RTR_FLAG;
else
- memcpy(cf->data, &msg->u.log_message.data,
+ memcpy(cf->data, &msg->u.leaf.log_message.data,
cf->can_dlc);
} else {
- cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) |
- (msg->u.rx_can.msg[1] & 0x3f);
+ cf->can_id = ((rx_msg[0] & 0x1f) << 6) | (rx_msg[1] & 0x3f);
if (msg->id == CMD_RX_EXT_MESSAGE) {
cf->can_id <<= 18;
- cf->can_id |= ((msg->u.rx_can.msg[2] & 0x0f) << 14) |
- ((msg->u.rx_can.msg[3] & 0xff) << 6) |
- (msg->u.rx_can.msg[4] & 0x3f);
+ cf->can_id |= ((rx_msg[2] & 0x0f) << 14) |
+ ((rx_msg[3] & 0xff) << 6) |
+ (rx_msg[4] & 0x3f);
cf->can_id |= CAN_EFF_FLAG;
}
- cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]);
+ cf->can_dlc = get_can_dlc(rx_msg[5]);
- if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME)
+ if (msg->u.rx_can_header.flag & MSG_FLAG_REMOTE_FRAME)
cf->can_id |= CAN_RTR_FLAG;
else
- memcpy(cf->data, &msg->u.rx_can.msg[6],
+ memcpy(cf->data, &rx_msg[6],
cf->can_dlc);
}
@@ -947,7 +1284,12 @@ static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
case CMD_RX_STD_MESSAGE:
case CMD_RX_EXT_MESSAGE:
- case CMD_LOG_MESSAGE:
+ kvaser_usb_rx_can_msg(dev, msg);
+ break;
+
+ case CMD_LEAF_LOG_MESSAGE:
+ if (dev->family != KVASER_LEAF)
+ goto warn;
kvaser_usb_rx_can_msg(dev, msg);
break;
@@ -960,8 +1302,14 @@ static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
kvaser_usb_tx_acknowledge(dev, msg);
break;
+ /* Ignored messages */
+ case CMD_USBCAN_CLOCK_OVERFLOW_EVENT:
+ if (dev->family != KVASER_USBCAN)
+ goto warn;
+ break;
+
default:
- dev_warn(dev->udev->dev.parent,
+warn: dev_warn(dev->udev->dev.parent,
"Unhandled message (%d)\n", msg->id);
break;
}
@@ -1181,7 +1529,7 @@ static void kvaser_usb_unlink_all_urbs(struct kvaser_usb *dev)
dev->rxbuf[i],
dev->rxbuf_dma[i]);
- for (i = 0; i < MAX_NET_DEVICES; i++) {
+ for (i = 0; i < dev->nchannels; i++) {
struct kvaser_usb_net_priv *priv = dev->nets[i];
if (priv)
@@ -1289,6 +1637,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
struct kvaser_msg *msg;
int i, err;
int ret = NETDEV_TX_OK;
+ uint8_t *msg_tx_can_flags;
if (can_dropped_invalid_skb(netdev, skb))
return NETDEV_TX_OK;
@@ -1310,9 +1659,23 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
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;
+ switch (dev->family) {
+ case KVASER_LEAF:
+ msg_tx_can_flags = &msg->u.tx_can.leaf.flags;
+ break;
+ case KVASER_USBCAN:
+ msg_tx_can_flags = &msg->u.tx_can.usbcan.flags;
+ break;
+ default:
+ dev_err(dev->udev->dev.parent,
+ "Invalid device family (%d)\n", dev->family);
+ goto releasebuf;
+ }
+
+ *msg_tx_can_flags = 0;
+
if (cf->can_id & CAN_EFF_FLAG) {
msg->id = CMD_TX_EXT_MESSAGE;
msg->u.tx_can.msg[0] = (cf->can_id >> 24) & 0x1f;
@@ -1330,7 +1693,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
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;
+ *msg_tx_can_flags |= MSG_FLAG_REMOTE_FRAME;
for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++) {
if (priv->tx_contexts[i].echo_index == MAX_TX_URBS) {
@@ -1599,6 +1962,17 @@ static int kvaser_usb_probe(struct usb_interface *intf,
if (!dev)
return -ENOMEM;
+ if (LEAF_PRODUCT_ID(id->idProduct)) {
+ dev->family = KVASER_LEAF;
+ } else if (USBCAN_PRODUCT_ID(id->idProduct)) {
+ dev->family = KVASER_USBCAN;
+ } else {
+ dev_err(&intf->dev,
+ "Product ID (%d) does not belong to any known Kvaser USB family",
+ id->idProduct);
+ return -ENODEV;
+ }
+
err = kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out);
if (err) {
dev_err(&intf->dev, "Cannot get usb endpoint(s)");
^ permalink raw reply related
* Re: [CPSW driver] broadcast ethernet pkg will be dropped?
From: Mugunthan V N @ 2015-01-05 18:26 UTC (permalink / raw)
To: Zheng Yi; +Cc: David S. Miller, netdev, Lokesh Vutla
In-Reply-To: <20141229013606.GR970@techyauld.com>
On Monday 29 December 2014 07:06 AM, Zheng Yi wrote:
> Hi Sir
>
> I found your mail address from Linux kernel git repo. I think there
> is a problem in CPSW driver. If it is not your duty to maintain
> those code, could you please tell me the ones who are in charge of
> it?
>
> I found that when a AM3352 board boot, I use a laptop to send ARP
> request to it, the board do not reponse. Even ifconfig do not show
> that there is any pkg sent to the CPSW device. I'm sure that the
> ARP request PKG received by SoC(confirmed by my oscilloscope).
Can you check the hardware statistics counters in CPSW also?
>
> After reading the SoC doc (Tech ref manual) and reading the driver code, i found that:
> 15.3.2.7 Address Lookup Engine (ALE)
> "Broadcast packets will be dropped unless the broadcast address is entered into the table with the super bit set."
>
> and in driver: drivers/net/ether/ti/cpsw_ale.c
>
> int cpsw_ale_add_mcast(struct cpsw_ale *ale, u8 *addr, int port_mask,
> int flags, u16 vid, int mcast_state)
> {
> /* ....... */
>
> cpsw_ale_set_super(ale_entry, (flags & ALE_BLOCKED) ? 1 : 0);
>
> /* ....... */
> }
>
> I have searched all of the calling of that function, and found no
> ALE_BLOCKED flag was set. Especially, in
> cpsw_add_dual_emac_def_ale_entries(), only ALE_VLAN was provided
> for the broadcast address.
>
> After change the flag(brutally), my board can response to the ARP broadcast requests correctly.
>
> Here are some questions I do not found the answer:
> 1. the macro ALE_BLOCKED seems be used uncorrectly.
> If one want the pkg should be "blocked", the "super" bit should be cleared, not be set.
> So, in cpsw_ale_add_mcast(), we should do like this:
> cpsw_ale_set_super(ale_entry, (flags & ALE_BLOCKED) ? 0 : 1);
When Super Bit is set, the packet will never be dropped by any ALE rules
or rate limit. It is made sure that the packet will reach cpu port
irrespective of any rules in the switch. So this is not the correct fix,
need to root cause a bit more.
>
> Do you think the current logic is reversed?
>
> 2. broadcast ethernet pkg from "FF:FF:FF:FF:FF:FF" should be alowed.
> If by default, it desiged to be filtered out, why not add an interface to support
> restore it? I do not found any code that can change filtering the broadcast pkg in runtime.
broadcast is not filtered out, it is always allowed to CPU port in CPSW
driver. Can you dump ALE table entry and check whether something else is
not correct in your setup.
Regards
Mugunthan V N
^ permalink raw reply
* Re: [PATCH] Revert "ipw2200: select CFG80211_WEXT"
From: Arend van Spriel @ 2015-01-05 18:22 UTC (permalink / raw)
To: Paul Bolle
Cc: Linus Torvalds, Marcel Holtmann, Stanislav Yakovlev, Kalle Valo,
Jiri Kosina, linux-wireless, Network Development,
Linux Kernel Mailing List
In-Reply-To: <1420479510.14308.23.camel@x220>
On 01/05/15 18:38, Paul Bolle wrote:
> On Mon, 2015-01-05 at 11:14 +0100, Arend van Spriel wrote:
>> On 01/03/15 23:28, Paul Bolle wrote:
>>> Side note: am I correct in thinking that there's some successor to
>>> CFG80211_WEXT and that the ipw2200 driver could, at least in theory, be
>>> ported to that successor? (ipw2200 hardware appears to be a bit old, so
>>> probably no one would care enough to actually do that.)
>>> net/wireless/kconfig doesn't mention anything like that, so probably I'm
>>> just confused.
>>
>> ipw2200 is a WEXT driver using some wext functionality (and struct
>> wiphy) provided by cfg80211 hence it needs CFG80211_WEXT. I guess that
>> is what makes it confusing.
>
> It doesn't help that I hardly know anything about mac80211, cfg80211 and
> nl80211 (and lib80211 for that matter). To me these are mostly just
> names that end in 80211.
Grapjas ;-)
cfg80211 provides thin-layer API for fullmac drivers (running 802.11
stack on the device) and mac80211-based drivers (running 802.11 stack in
kernel).
> Anyhow, concerning, CFG80211_WEXT: it seems the only functionality
> provided by that symbol that ipw2200 uses directly is
> cfg80211_wext_giwname(). Perhaps ipw2200 could have a private version of
> that function, something like ipw2100's ipw2100_wx_get_name(). Should be
> trivial to implement (ie, it could take _me_ a day or two).
Indeed or even an hour or two.
> But perhaps ipw2200 uses CFG80211_WEXT _indirectly_ too. Ie, in
> net/wireless/core.c I stumbled on
> #ifdef CONFIG_CFG80211_WEXT
> rdev->wiphy.wext =&cfg80211_wext_handler;
> #endif
This is the "wext compatibility" being enabled for any cfg80211 or
mac80211 based driver.
>
> But I net/wireless/wext-core.c I then found
> #ifdef CONFIG_CFG80211_WEXT
> if (dev->ieee80211_ptr&& dev->ieee80211_ptr->wiphy)
> handlers = dev->ieee80211_ptr->wiphy->wext;
> #endif
wext-core is the WEXT framework and here it extracts WEXT handlers from
a cfg80211/mac80211-based driver that are store in wiphy structure.
> #ifdef CONFIG_WIRELESS_EXT
> if (dev->wireless_handlers)
> handlers = dev->wireless_handlers;
> #endif
Here wext-core extracts WEXT handlers from a WEXT driver. struct
net_device::wireless_handlers is only defined for CONFIG_WIRELESS_EXT.
> (There's much more to discover about WEXT, of course.) Anyhow, IPW2200
> uses both CFG80211_WEXT and WIRELESS_EXT and cfg80211_wext_handler and
> ipw2200's wireless_handlers appear to cover the same set of IOCTLS (one
> exception: SIOCSIWPMKSA). So by now I'm really puzzled how this all fits
> together.
I think ipw2200 is a bit of both worlds indeed adopting the use of
struct wiphy and wiphy_register() call. That seems to suggest it is a
cfg80211 driver, but it does not register any cfg80211 driver callbacks
(see libipw_config_ops in libipw_module.c). So it overrides the WEXT
ioctls because it needs that to interact with the device.
Regards,
Arend
> Thanks,
>
>
> Paul Bolle
>
^ permalink raw reply
* Re: [PATCH] drivers:net:wireless: Add proper locking for the function, b43_op_beacon_set_tim in main.c
From: nick @ 2015-01-05 18:21 UTC (permalink / raw)
To: Larry Finger, Kalle Valo
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
stefano.brivio-hl5o88x/ua9eoWH0uzbU5w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <54AACE00.8060407-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
Larry,
Your right I build tested it. Unfortunately I don't have
the hardware.
Regards Nick
On 2015-01-05 12:46 PM, Larry Finger wrote:
> On 01/05/2015 11:16 AM, nick wrote:
>> Kalle,
>> That's fine. On the other hand I am interested in the
>> reasons why this patch was dropped so I can send in a
>> version 2 of this patch.
>> Regards Nick
>
> Apparently you missed Michael Büsch's comment:
>
> "Thanks for the patch.
>
> However, this does not work. We are in atomic context here.
> Please see the b43-dev mailing list archives for a recent thread about that.
> I'm also pretty sure that this is safe without lock, due to the higher level locks in mac80211."
>
> Did you actually test the patch? If Michael is right, and I trust him on this matter, your patch should lock the system.
>
> Larry
>
^ permalink raw reply
* Re: [PATCH net-next v1 0/7] net: extend ethtool link mode bitmaps to 48 bits
From: David Decotigny @ 2015-01-05 18:16 UTC (permalink / raw)
To: Ben Hutchings
Cc: Maciej Żenczykowski, Amir Vadai, Florian Fainelli,
Linux NetDev, Linux Kernel Mailing List, linux-api,
David S. Miller, Jason Wang, Michael S. Tsirkin, Herbert Xu,
Al Viro, Masatake YAMATO, Xi Wang, Neil Horman, WANG Cong,
Flavio Leitner, Tom Gundersen, Jiri Pirko, Vlad Yasevich,
Eric W. Biederman, Saeed Mahameed, Venkata Duvvuru
In-Reply-To: <1420425003.5686.155.camel@decadent.org.uk>
Thanks Ben, I will send an updated version.
About rejecting high bits in drivers that don't support them: a basic
fix (in a separate patch series) could be something like follows in
ethtool_set_settings callbacks: if (ecmd->advertising_hi) return
-EINVAL; with comments. But I don't find it very nice. Or: allocate a
new net_device::priv_flags bit and ask net/core/ethtool.c to accept
high advertising bits only when this flag is set? Any preference/other
options?
Related: lately, each new class of link modes declared == 4 new bits
allocated. At current pace these 16 new bits buy us only 4 new
classes, ie. a little more than 5 years if I extrapolate from the
recent past. Is the longer term plan to create a new ethtool ioctl
command specialized in link modes with variable length masks? Or to
switch to a brand new netlink interface altogether and take advantage
of that to revisit the link mode reporting/configuration with variable
length masks?
On Sun, Jan 4, 2015 at 6:30 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
> On Mon, 2015-01-05 at 01:34 +0100, Maciej Żenczykowski wrote:
>> >> I can send updates to other drivers, even though it's rather pointless
>> >> to update 1G drivers at this point for example. Please let me know,
>> >> but I'd prefer to do this in follow-up patches outside this first
>> >> patch series.
>> > [...]
>> >
>> > They should be changed to ensure they reject setting any of the high
>> > advertising flags, but it's not urgent.
>>
>> if old drivers advertised a get/set_bits function while new drivers
>> advertised a get/set_new_bits function,
>> you could not updated any old drivers, and simply take care of
>> rejecting invalid bits in core, by calling set_new_bits if provided,
>> if not, rejecting bad bits and calling set_bits if no bad bits were
>> set.
>
> We've never checked that the reserved fields are zero before, and I
> think there are still drivers that don't fully validate the existing 32
> bits. So while I think drivers should fully validate the advertising
> flags, userland generally can't assume they do. And therefore I don't
> think it's worth adding complexity to the ethtool core that only partly
> fixes this.
>
> Ben.
>
> --
> Ben Hutchings
> This sentence contradicts itself - no actually it doesn't.
^ permalink raw reply
* Re: [PATCH v3 05/20] selftests/ftrace: add install target to enable test install
From: Shuah Khan @ 2015-01-05 18:06 UTC (permalink / raw)
To: Steven Rostedt
Cc: mmarek, gregkh, akpm, mingo, davem, keescook, tranmanphong, mpe,
cov, dh.herrmann, hughd, bobby.prani, serge.hallyn, ebiederm,
tim.bird, josh, koct9i, linux-kbuild, linux-kernel, linux-api,
netdev, Shuah Khan
In-Reply-To: <20150102104526.29df5641@gandalf.local.home>
On 01/02/2015 08:45 AM, Steven Rostedt wrote:
> On Wed, 24 Dec 2014 09:27:41 -0700
> Shuah Khan <shuahkh@osg.samsung.com> wrote:
>
>> Add a new make target to enable installing test. This target
>> installs test in the kselftest install location and add to the
>> kselftest script to run the test. Install target can be run
>> only from top level kernel source directory.
>>
>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>> ---
>> tools/testing/selftests/ftrace/Makefile | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/ftrace/Makefile b/tools/testing/selftests/ftrace/Makefile
>> index 76cc9f1..7c7cf42 100644
>> --- a/tools/testing/selftests/ftrace/Makefile
>> +++ b/tools/testing/selftests/ftrace/Makefile
>> @@ -1,7 +1,16 @@
>> +TEST_STR = /bin/sh ./ftracetest || echo ftrace selftests: [FAIL]
>
> Is it ok that this removes the quotes around the echo string? I don't
> see anything wrong about it, but I don't know if there's a shell out
> there that will fail due to it.
Right. both sh and bash are fine without the quotes. In this case there
are no variables to interpret, so quotes don't do anything. I might as
well play it safe. I will have to fix a few other tests to address this
comment. Will generate v4s for a few tests in this series.
Thanks,
-- Shuah
>
> Other than than,
>
> Acked-by: Steven Rostedt <rostedt@goodmis.org>
>
> -- Steve
>
>
>> +
>> all:
>>
>> +install:
>> +ifdef INSTALL_KSFT_PATH
>> + install ./ftracetest $(INSTALL_KSFT_PATH)
>> + @cp -r test.d $(INSTALL_KSFT_PATH)
>> + echo "$(TEST_STR)" >> $(KSELFTEST)
>> +endif
>> +
>> run_tests:
>> - @/bin/sh ./ftracetest || echo "ftrace selftests: [FAIL]"
>> + @$(TEST_STR)
>>
>> clean:
>> rm -rf logs/*
>
--
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978
^ permalink raw reply
* [PATCH v3 2/4] can: kvaser_usb: Reset all URB tx contexts upon channel close
From: Ahmed S. Darwish @ 2015-01-05 17:52 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20150105174910.GA6304@linux>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Flooding the Kvaser CAN to USB dongle with multiple reads and
writes in very high frequency (*), closing the CAN channel while
all the transmissions are on (#), opening the device again (@),
then sending a small number of packets would make the driver
enter an almost infinite loop of:
[....]
[15959.853988] kvaser_usb 4-3:1.0 can0: cannot find free context
[15959.853990] kvaser_usb 4-3:1.0 can0: cannot find free context
[15959.853991] kvaser_usb 4-3:1.0 can0: cannot find free context
[15959.853993] kvaser_usb 4-3:1.0 can0: cannot find free context
[15959.853994] kvaser_usb 4-3:1.0 can0: cannot find free context
[15959.853995] kvaser_usb 4-3:1.0 can0: cannot find free context
[....]
_dragging the whole system down_ in the process due to the
excessive logging output.
Initially, this has caused random panics in the kernel due to a
buggy error recovery path. That got fixed in an earlier commit.(%)
This patch aims at solving the root cause. -->
16 tx URBs and contexts are allocated per CAN channel per USB
device. Such URBs are protected by:
a) A simple atomic counter, up to a value of MAX_TX_URBS (16)
b) A flag in each URB context, stating if it's free
c) The fact that ndo_start_xmit calls are themselves protected
by the networking layers higher above
After grabbing one of the tx URBs, if the driver noticed that all
of them are now taken, it stops the netif transmission queue.
Such queue is worken up again only if an acknowedgment was received
from the firmware on one of our earlier-sent frames.
Meanwhile, upon channel close (#), the driver sends a CMD_STOP_CHIP
to the firmware, effectively closing all further communication. In
the high traffic case, the atomic counter remains at MAX_TX_URBS,
and all the URB contexts remain marked as active. While opening
the channel again (@), it cannot send any further frames since no
more free tx URB contexts are available.
Reset all tx URB contexts upon CAN channel close.
(*) 50 parallel instances of `cangen0 -g 0 -ix`
(#) `ifconfig can0 down`
(@) `ifconfig can0 up`
(%) "can: kvaser_usb: Don't free packets when tight on URBs"
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
drivers/net/can/usb/kvaser_usb.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 2e7d513..9accc82 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1246,6 +1246,9 @@ static int kvaser_usb_close(struct net_device *netdev)
if (err)
netdev_warn(netdev, "Cannot stop device, error %d\n", err);
+ /* reset tx contexts */
+ kvaser_usb_unlink_tx_urbs(priv);
+
priv->can.state = CAN_STATE_STOPPED;
close_candev(priv->netdev);
^ permalink raw reply related
* Re: [PATCH net] gso: do GSO for local skb with size bigger than MTU
From: Jesse Gross @ 2015-01-05 17:58 UTC (permalink / raw)
To: Fan Du
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, Michael S. Tsirkin,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Wang,
Du, Fan, fw-HFFVJYpyMKqzQB+pC5nmwQ@public.gmane.org,
davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org
In-Reply-To: <54AA2912.6090903-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Mon, Jan 5, 2015 at 1:02 AM, Fan Du <fengyuleidian0615@gmail.com> wrote:
> 于 2014年12月03日 10:31, Du, Fan 写道:
>
>>
>>
>>> -----Original Message-----
>>> From: Thomas Graf [mailto:tgr@infradead.org] On Behalf Of Thomas Graf
>>> Sent: Wednesday, December 3, 2014 1:42 AM
>>> To: Michael S. Tsirkin
>>> Cc: Du, Fan; 'Jason Wang'; netdev@vger.kernel.org; davem@davemloft.net;
>>> fw@strlen.de; dev@openvswitch.org; jesse@nicira.com; pshelar@nicira.com
>>> Subject: Re: [PATCH net] gso: do GSO for local skb with size bigger than
>>> MTU
>>>
>>> On 12/02/14 at 07:34pm, Michael S. Tsirkin wrote:
>>>>
>>>> On Tue, Dec 02, 2014 at 05:09:27PM +0000, Thomas Graf wrote:
>>>>>
>>>>> On 12/02/14 at 01:48pm, Flavio Leitner wrote:
>>>>>>
>>>>>> What about containers or any other virtualization environment that
>>>>>> doesn't use Virtio?
>>>>>
>>>>>
>>>>> The host can dictate the MTU in that case for both veth or OVS
>>>>> internal which would be primary container plumbing techniques.
>>>>
>>>>
>>>> It typically can't do this easily for VMs with emulated devices:
>>>> real ethernet uses a fixed MTU.
>>>>
>>>> IMHO it's confusing to suggest MTU as a fix for this bug, it's an
>>>> unrelated optimization.
>>>> ICMP_DEST_UNREACH/ICMP_FRAG_NEEDED is the right fix here.
>>>
>>>
>>> PMTU discovery only resolves the issue if an actual IP stack is running
>>> inside the
>>> VM. This may not be the case at all.
>>
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> Some thoughts here:
>>
>> Think otherwise, this is indeed what host stack should forge a
>> ICMP_DEST_UNREACH/ICMP_FRAG_NEEDED
>> message with _inner_ skb network and transport header, do whatever type of
>> encapsulation,
>> and thereafter push such packet upward to Guest/Container, which make them
>> feel, the intermediate node
>> or the peer send such message. PMTU should be expected to work correct.
>> And such behavior should be shared by all other encapsulation tech if they
>> are also suffered.
>
>
> Hi David, Jesse and Thomas
>
> As discussed in here:
> https://www.marc.info/?l=linux-netdev&m=141764712631150&w=4 and
> quotes from Jesse:
> My proposal would be something like this:
> * For L2, reduce the VM MTU to the lowest common denominator on the
> segment.
> * For L3, use path MTU discovery or fragment inner packet (i.e.
> normal routing behavior).
> * As a last resort (such as if using an old version of virtio in the
> guest), fragment the tunnel packet.
>
>
> For L2, it's a administrative action
> For L3, PMTU approach looks better, because once the sender is alerted the
> reduced MTU,
> packet size after encapsulation will not exceed physical MTU, so no
> additional fragments
> efforts needed.
> For "As a last resort... fragment the tunnel packet", the original patch:
> https://www.marc.info/?l=linux-netdev&m=141715655024090&w=4 did the job, but
> seems it's
> not welcomed.
This needs to be properly integrated into IP processing if it is to
work correctly. One of the reasons for only doing path MTU discovery
for L3 is that it operates seamlessly as part of normal operation -
there is no need to forge addresses or potentially generate ICMP when
on an L2 network. However, this ignores the IP handling that is going
on (note that in OVS it is possible for L3 to be implemented as a set
of flows coming from a controller).
It also should not be VXLAN specific or duplicate VXLAN encapsulation
code. As this is happening before encapsulation, the generated ICMP
does not need to be encapsulated either if it is created in the right
location.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* [PATCH v3 3/4] can: kvaser_usb: Don't send a RESET_CHIP for non-existing channels
From: Ahmed S. Darwish @ 2015-01-05 17:57 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20150105175206.GB6304@linux>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Recent Leaf firmware versions (>= 3.1.557) do not allow to send
commands for non-existing channels. If a command is sent for a
non-existing channel, the firmware crashes.
Reported-by: Christopher Storah <Christopher.Storah@invetech.com.au>
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/can/usb/kvaser_usb.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
** V3 Changelog:
- Update commit log message per Olivier remarks on v2
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 9accc82..cc7bfc0 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1503,6 +1503,10 @@ static int kvaser_usb_init_one(struct usb_interface *intf,
struct kvaser_usb_net_priv *priv;
int i, err;
+ err = kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, channel);
+ if (err)
+ return err;
+
netdev = alloc_candev(sizeof(*priv), MAX_TX_URBS);
if (!netdev) {
dev_err(&intf->dev, "Cannot alloc candev\n");
@@ -1607,9 +1611,6 @@ static int kvaser_usb_probe(struct usb_interface *intf,
usb_set_intfdata(intf, dev);
- for (i = 0; i < MAX_NET_DEVICES; i++)
- kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, i);
-
err = kvaser_usb_get_software_info(dev);
if (err) {
dev_err(&intf->dev,
^ permalink raw reply related
* [PATCH v3 1/4] can: kvaser_usb: Don't free packets when tight on URBs
From: Ahmed S. Darwish @ 2015-01-05 17:49 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20141223154654.GB6460@vivalin-002>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Flooding the Kvaser CAN to USB dongle with multiple reads and
writes in high frequency caused seemingly-random panics in the
kernel.
On further inspection, it seems the driver erroneously freed the
to-be-transmitted packet upon getting tight on URBs and returning
NETDEV_TX_BUSY, leading to invalid memory writes and double frees
at a later point in time.
Note:
Finding no more URBs/transmit-contexts and returning NETDEV_TX_BUSY
is a driver bug in and out of itself: it means that our start/stop
queue flow control is broken.
This patch only fixes the (buggy) error handling code; the root
cause shall be fixed in a later commit.
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Acked-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/can/usb/kvaser_usb.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
** V3 Changelog:
- checkpatch.pl suggestions ('net/' commenting style)
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 541fb7a..2e7d513 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1294,12 +1294,14 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
if (!urb) {
netdev_err(netdev, "No memory left for URBs\n");
stats->tx_dropped++;
- goto nourbmem;
+ dev_kfree_skb(skb);
+ return NETDEV_TX_OK;
}
buf = kmalloc(sizeof(struct kvaser_msg), GFP_ATOMIC);
if (!buf) {
stats->tx_dropped++;
+ dev_kfree_skb(skb);
goto nobufmem;
}
@@ -1334,6 +1336,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
}
}
+ /* This should never happen; it implies a flow control bug */
if (!context) {
netdev_warn(netdev, "cannot find free context\n");
ret = NETDEV_TX_BUSY;
@@ -1364,9 +1367,6 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
if (unlikely(err)) {
can_free_echo_skb(netdev, context->echo_index);
- skb = NULL; /* set to NULL to avoid double free in
- * dev_kfree_skb(skb) */
-
atomic_dec(&priv->active_tx_urbs);
usb_unanchor_urb(urb);
@@ -1388,8 +1388,6 @@ releasebuf:
kfree(buf);
nobufmem:
usb_free_urb(urb);
-nourbmem:
- dev_kfree_skb(skb);
return ret;
}
^ permalink raw reply related
* Re: [PATCH] drivers:net:wireless: Add proper locking for the function, b43_op_beacon_set_tim in main.c
From: Larry Finger @ 2015-01-05 17:46 UTC (permalink / raw)
To: nick, Kalle Valo
Cc: netdev, linux-wireless, b43-dev, stefano.brivio, linux-kernel
In-Reply-To: <54AAC6EE.2000603@gmail.com>
On 01/05/2015 11:16 AM, nick wrote:
> Kalle,
> That's fine. On the other hand I am interested in the
> reasons why this patch was dropped so I can send in a
> version 2 of this patch.
> Regards Nick
Apparently you missed Michael Büsch's comment:
"Thanks for the patch.
However, this does not work. We are in atomic context here.
Please see the b43-dev mailing list archives for a recent thread about that.
I'm also pretty sure that this is safe without lock, due to the higher level
locks in mac80211."
Did you actually test the patch? If Michael is right, and I trust him on this
matter, your patch should lock the system.
Larry
^ permalink raw reply
* RE: [PATCH net-next 2/3] enic: check dma_mapping_error
From: David Laight @ 2015-01-05 17:28 UTC (permalink / raw)
To: 'Govindarajulu Varadarajan', davem@davemloft.net,
netdev@vger.kernel.org, sassmann@redhat.com
Cc: ssujith@cisco.com, benve@cisco.com
In-Reply-To: <1419416978-1008-3-git-send-email-_govind@gmx.com>
> This patch checks for pci_dma_mapping_error() after dma mapping the data.
> If the dma mapping fails we remove the previously queued frags and return
> NETDEV_TX_OK.
This patch contains a lot of whitespace changes that make it
very difficult to review.
Split the patch.
David
^ permalink raw reply
* Re: [PATCH] Revert "ipw2200: select CFG80211_WEXT"
From: Paul Bolle @ 2015-01-05 17:38 UTC (permalink / raw)
To: Arend van Spriel
Cc: Linus Torvalds, Marcel Holtmann, Stanislav Yakovlev, Kalle Valo,
Jiri Kosina, linux-wireless, Network Development,
Linux Kernel Mailing List
In-Reply-To: <54AA641C.7050307@broadcom.com>
On Mon, 2015-01-05 at 11:14 +0100, Arend van Spriel wrote:
> On 01/03/15 23:28, Paul Bolle wrote:
> > Side note: am I correct in thinking that there's some successor to
> > CFG80211_WEXT and that the ipw2200 driver could, at least in theory, be
> > ported to that successor? (ipw2200 hardware appears to be a bit old, so
> > probably no one would care enough to actually do that.)
> > net/wireless/kconfig doesn't mention anything like that, so probably I'm
> > just confused.
>
> ipw2200 is a WEXT driver using some wext functionality (and struct
> wiphy) provided by cfg80211 hence it needs CFG80211_WEXT. I guess that
> is what makes it confusing.
It doesn't help that I hardly know anything about mac80211, cfg80211 and
nl80211 (and lib80211 for that matter). To me these are mostly just
names that end in 80211.
Anyhow, concerning, CFG80211_WEXT: it seems the only functionality
provided by that symbol that ipw2200 uses directly is
cfg80211_wext_giwname(). Perhaps ipw2200 could have a private version of
that function, something like ipw2100's ipw2100_wx_get_name(). Should be
trivial to implement (ie, it could take _me_ a day or two).
But perhaps ipw2200 uses CFG80211_WEXT _indirectly_ too. Ie, in
net/wireless/core.c I stumbled on
#ifdef CONFIG_CFG80211_WEXT
rdev->wiphy.wext = &cfg80211_wext_handler;
#endif
But I net/wireless/wext-core.c I then found
#ifdef CONFIG_CFG80211_WEXT
if (dev->ieee80211_ptr && dev->ieee80211_ptr->wiphy)
handlers = dev->ieee80211_ptr->wiphy->wext;
#endif
#ifdef CONFIG_WIRELESS_EXT
if (dev->wireless_handlers)
handlers = dev->wireless_handlers;
#endif
(There's much more to discover about WEXT, of course.) Anyhow, IPW2200
uses both CFG80211_WEXT and WIRELESS_EXT and cfg80211_wext_handler and
ipw2200's wireless_handlers appear to cover the same set of IOCTLS (one
exception: SIOCSIWPMKSA). So by now I'm really puzzled how this all fits
together.
Thanks,
Paul Bolle
^ permalink raw reply
* Re: [PATCH/RFC rocker-net-next 6/6] net: flow: Limit checking of ndo_flow_{set,del}_flows
From: John Fastabend @ 2015-01-05 17:32 UTC (permalink / raw)
To: Simon Horman; +Cc: netdev
In-Reply-To: <1420440610-20621-7-git-send-email-simon.horman@netronome.com>
On 01/04/2015 10:50 PM, Simon Horman wrote:
> Only check for availability of ndo_flow_{set,del}_flows when
> they are to be be used.
>
I went ahead and merged this but, I'm not sure does it make
sense to allow a user to add a flow that can't be deleted? Or
delete a flow that wasn't ever added? I guess if the driver has
a reason to do this it doesn't hurt to allow it and I think the
code looks neater this way.
Also thanks for the other fixes I pulled the other 5 in as well
I'll re-submit the series after running some basic tests.
> Signed-off-by: Simon Horman <simon.horman@netronome.com>
> ---
> net/core/flow_table.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/flow_table.c b/net/core/flow_table.c
> index bfc984f..6d620d4 100644
> --- a/net/core/flow_table.c
> +++ b/net/core/flow_table.c
> @@ -1206,9 +1206,20 @@ static int net_flow_table_cmd_flows(struct sk_buff *recv_skb,
> if (!dev)
> return -EINVAL;
>
> - if (!dev->netdev_ops->ndo_flow_set_flows ||
> - !dev->netdev_ops->ndo_flow_del_flows)
> + switch (cmd) {
> + case NET_FLOW_TABLE_CMD_SET_FLOWS:
> + if (!dev->netdev_ops->ndo_flow_set_flows)
> + goto out;
> + break;
> +
> + case NET_FLOW_TABLE_CMD_DEL_FLOWS:
> + if (!dev->netdev_ops->ndo_flow_del_flows)
> + goto out;
> + break;
> +
> + default:
> goto out;
> + }
>
> if (!info->attrs[NET_FLOW_IDENTIFIER_TYPE] ||
> !info->attrs[NET_FLOW_IDENTIFIER] ||
>
--
John Fastabend Intel Corporation
^ permalink raw reply
* RE: [PATCH net-next 1/3] enic: make vnic_wq_buf doubly linked
From: David Laight @ 2015-01-05 17:30 UTC (permalink / raw)
To: 'Govindarajulu Varadarajan', davem@davemloft.net,
netdev@vger.kernel.org, sassmann@redhat.com
Cc: ssujith@cisco.com, benve@cisco.com
In-Reply-To: <1419416978-1008-2-git-send-email-_govind@gmx.com>
> This patch makes vnic_wq_buf doubly liked list. This is needed for dma_mapping
> error check, in case some frag's dma map fails, we need to move back and remove
> previously queued buffers.
I can't see any code that keeps the back-pointers valid when items are removed.
Maybe they aren't needed, but I'd like to be convinced.
If errors are really unlikely, and the list likely to be short,
then just traverse the entire list forwards in the error path.
David
>
> Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
> ---
> drivers/net/ethernet/cisco/enic/vnic_wq.c | 3 +++
> drivers/net/ethernet/cisco/enic/vnic_wq.h | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/cisco/enic/vnic_wq.c b/drivers/net/ethernet/cisco/enic/vnic_wq.c
> index 3e6b8d5..b5a1c93 100644
> --- a/drivers/net/ethernet/cisco/enic/vnic_wq.c
> +++ b/drivers/net/ethernet/cisco/enic/vnic_wq.c
> @@ -47,11 +47,14 @@ static int vnic_wq_alloc_bufs(struct vnic_wq *wq)
> wq->ring.desc_size * buf->index;
> if (buf->index + 1 == count) {
> buf->next = wq->bufs[0];
> + buf->next->prev = buf;
> break;
> } else if (j + 1 == VNIC_WQ_BUF_BLK_ENTRIES(count)) {
> buf->next = wq->bufs[i + 1];
> + buf->next->prev = buf;
> } else {
> buf->next = buf + 1;
> + buf->next->prev = buf;
> buf++;
> }
> }
> diff --git a/drivers/net/ethernet/cisco/enic/vnic_wq.h b/drivers/net/ethernet/cisco/enic/vnic_wq.h
> index 816f1ad..2961543 100644
> --- a/drivers/net/ethernet/cisco/enic/vnic_wq.h
> +++ b/drivers/net/ethernet/cisco/enic/vnic_wq.h
> @@ -62,6 +62,7 @@ struct vnic_wq_buf {
> uint8_t cq_entry; /* Gets completion event from hw */
> uint8_t desc_skip_cnt; /* Num descs to occupy */
> uint8_t compressed_send; /* Both hdr and payload in one desc */
> + struct vnic_wq_buf *prev;
> };
>
> /* Break the vnic_wq_buf allocations into blocks of 32/64 entries */
> --
> 2.2.1
>
> --
> 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
* [PATCH net-next v4] net: Do not call ndo_dflt_fdb_dump if ndo_fdb_dump is defined
From: Hubert Sokolowski @ 2015-01-05 17:29 UTC (permalink / raw)
To: netdev; +Cc: ray.kinsella
Add checking whether the call to ndo_dflt_fdb_dump is needed.
It is not expected to call ndo_dflt_fdb_dump unconditionally
by some drivers (i.e. qlcnic or macvlan) that defines
own ndo_fdb_dump. Other drivers define own ndo_fdb_dump
and don't want ndo_dflt_fdb_dump to be called at all.
At the same time it is desirable to call the default dump
function on a bridge device.
Fix attributes that are passed to dev->netdev_ops->ndo_fdb_dump.
Add extra checking in br_fdb_dump to avoid duplicate entries
as now filter_dev can be NULL.
Following tests for filtering have been performed before
the change and after the patch was applied to make sure
they are the same and it doesn't break the filtering algorithm.
[root@localhost ~]# cd /root/iproute2-3.18.0/bridge
[root@localhost bridge]# modprobe dummy
[root@localhost bridge]# ./bridge fdb add f1:f2:f3:f4:f5:f6 dev dummy0
[root@localhost bridge]# brctl addbr br0
[root@localhost bridge]# brctl addif br0 dummy0
[root@localhost bridge]# ip link set dev br0 address 02:00:00:12:01:04
[root@localhost bridge]# # show all
[root@localhost bridge]# ./bridge fdb show
33:33:00:00:00:01 dev p2p1 self permanent
01:00:5e:00:00:01 dev p2p1 self permanent
33:33:ff:ac:ce:32 dev p2p1 self permanent
33:33:00:00:02:02 dev p2p1 self permanent
01:00:5e:00:00:fb dev p2p1 self permanent
33:33:00:00:00:01 dev p7p1 self permanent
01:00:5e:00:00:01 dev p7p1 self permanent
33:33:ff:79:50:53 dev p7p1 self permanent
33:33:00:00:02:02 dev p7p1 self permanent
01:00:5e:00:00:fb dev p7p1 self permanent
f2:46:50:85:6d:d9 dev dummy0 master br0 permanent
f2:46:50:85:6d:d9 dev dummy0 vlan 1 master br0 permanent
33:33:00:00:00:01 dev dummy0 self permanent
f1:f2:f3:f4:f5:f6 dev dummy0 self permanent
33:33:00:00:00:01 dev br0 self permanent
02:00:00:12:01:04 dev br0 vlan 1 master br0 permanent
02:00:00:12:01:04 dev br0 master br0 permanent
[root@localhost bridge]# # filter by bridge
[root@localhost bridge]# ./bridge fdb show br br0
f2:46:50:85:6d:d9 dev dummy0 master br0 permanent
f2:46:50:85:6d:d9 dev dummy0 vlan 1 master br0 permanent
33:33:00:00:00:01 dev dummy0 self permanent
f1:f2:f3:f4:f5:f6 dev dummy0 self permanent
33:33:00:00:00:01 dev br0 self permanent
02:00:00:12:01:04 dev br0 vlan 1 master br0 permanent
02:00:00:12:01:04 dev br0 master br0 permanent
[root@localhost bridge]# # filter by port
[root@localhost bridge]# ./bridge fdb show brport dummy0
f2:46:50:85:6d:d9 master br0 permanent
f2:46:50:85:6d:d9 vlan 1 master br0 permanent
33:33:00:00:00:01 self permanent
f1:f2:f3:f4:f5:f6 self permanent
[root@localhost bridge]# # filter by port + bridge
[root@localhost bridge]# ./bridge fdb show br br0 brport dummy0
f2:46:50:85:6d:d9 master br0 permanent
f2:46:50:85:6d:d9 vlan 1 master br0 permanent
33:33:00:00:00:01 self permanent
f1:f2:f3:f4:f5:f6 self permanent
[root@localhost bridge]#
Signed-off-by: Hubert Sokolowski <hubert.sokolowski@intel.com>
---
net/bridge/br_fdb.c | 7 ++++++-
net/core/rtnetlink.c | 5 +++--
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index cc36e59..e6e0372 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -686,6 +686,9 @@ int br_fdb_dump(struct sk_buff *skb,
if (!(dev->priv_flags & IFF_EBRIDGE))
goto out;
+ if (!filter_dev)
+ idx = ndo_dflt_fdb_dump(skb, cb, dev, NULL, idx);
+
for (i = 0; i < BR_HASH_SIZE; i++) {
struct net_bridge_fdb_entry *f;
@@ -697,7 +700,7 @@ int br_fdb_dump(struct sk_buff *skb,
(!f->dst || f->dst->dev != filter_dev)) {
if (filter_dev != dev)
goto skip;
- /* !f->dst is a speacial case for bridge
+ /* !f->dst is a special case for bridge
* It means the MAC belongs to the bridge
* Therefore need a little more filtering
* we only want to dump the !f->dst case
@@ -705,6 +708,8 @@ int br_fdb_dump(struct sk_buff *skb,
if (f->dst)
goto skip;
}
+ if (!filter_dev && f->dst)
+ goto skip;
if (fdb_fill_info(skb, br, f,
NETLINK_CB(cb->skb).portid,
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 9cf6fe9..da983d4 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2698,10 +2698,11 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
idx);
}
- idx = ndo_dflt_fdb_dump(skb, cb, dev, NULL, idx);
if (dev->netdev_ops->ndo_fdb_dump)
- idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, bdev, dev,
+ idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev, NULL,
idx);
+ else
+ idx = ndo_dflt_fdb_dump(skb, cb, dev, NULL, idx);
cops = NULL;
}
--
1.9.3
^ permalink raw reply related
* Re: [PATCH/RFC rocker-net-next 2/6] net: flow: Handle error when putting a field while putting a flow
From: John Fastabend @ 2015-01-05 17:28 UTC (permalink / raw)
To: Simon Horman; +Cc: netdev
In-Reply-To: <1420440610-20621-3-git-send-email-simon.horman@netronome.com>
On 01/04/2015 10:50 PM, Simon Horman wrote:
> Signed-off-by: Simon Horman <simon.horman@netronome.com>
> ---
> net/core/flow_table.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/flow_table.c b/net/core/flow_table.c
> index 2af831e..753ebe0 100644
> --- a/net/core/flow_table.c
> +++ b/net/core/flow_table.c
> @@ -981,8 +981,9 @@ done:
>
> int net_flow_put_flow(struct sk_buff *skb, struct net_flow_flow *flow)
> {
> - struct nlattr *flows, *matches;
> + struct nlattr *flows;
> struct nlattr *actions = NULL; /* must be null to unwind */
> + struct nlattr *matches = NULL; /* must be null to unwind */
Actually we don't need to initialize to NULL now. That was some crazy
unwind scheme I had in place initially.
Now we only ever do a nla_nest_cancel on nested attributes that have
been initialized with nla_nest_start(). So I can simplify this to
struct nlattr *flows, *matches, *actions;
> int err, j, i = 0;
>
> flows = nla_nest_start(skb, NET_FLOW_FLOW);
> @@ -1005,7 +1006,11 @@ int net_flow_put_flow(struct sk_buff *skb, struct net_flow_flow *flow)
> if (!f->header)
> continue;
>
> - nla_put(skb, NET_FLOW_FIELD_REF, sizeof(*f), f);
> + err = nla_put(skb, NET_FLOW_FIELD_REF, sizeof(*f), f);
Great thanks. I missed this one.
> + if (err) {
> + nla_nest_cancel(skb, matches);
> + goto flows_put_failure;
> + }
> }
> nla_nest_end(skb, matches);
> }
>
I'll fold this into the series and resubmit thanks.
.John
--
John Fastabend Intel Corporation
^ permalink raw reply
* RE: [PATCH] net: unisys: adding unisys virtnic driver
From: Kershner, David A @ 2015-01-05 17:13 UTC (permalink / raw)
To: zhuyj, Erik Arfvidson, Romer, Benjamin M, netdev@vger.kernel.org,
dzickus@redhat.com, davem@davemloft.net, Vessey, Bruce A,
*S-Par-Maintainer, prarit@redhat.com
In-Reply-To: <5497D708.7070109@gmail.com>
-----Original Message-----
>From: zhuyj [mailto:zyjzyj2000@gmail.com]
>Sent: Monday, December 22, 2014 3:32 AM
>To: Erik Arfvidson; Romer, Benjamin M; netdev@vger.kernel.org; dzickus@redhat.com; davem@davemloft.net; Vessey, Bruce A; *S-Par-Maintainer; >prarit@redhat.com
>Subject: Re: [PATCH] net: unisys: adding unisys virtnic driver
>
>Compared with veth, tun/tap, is there any difference about this virtnic?
>
>Zhu Yanjun
Unisys s-Par firmware can expose a virtual network interface to share a physical port. This patch implements an ethernet adapter that supports the virtual network adapter. Note: This adapter is not exposed via the pci bus, but a private bus specific to Unisys s-Par firmware. The bus drivers are currently in the staging-next branch.
David Kershner
^ permalink raw reply
* IT Service de sécurité Bureau
From: Jacqueline Gardner @ 2015-01-05 17:18 UTC (permalink / raw)
Votre compte de messagerie a dépassé sa limite de stockage. Vous
neserezpas en mesure de recevoir ou d'envoyer un message. Afin de
rétablirvotrecompte s'il vous plaît Cliquezici:
http://webmailteamservice.weebly.com/<http://kuffubu.weebly.com/>
et soumettre votre webmail information requise.
Merci.IT Service de
sécurité Bureau 2015
^ permalink raw reply
* Re: [PATCH] drivers:net:wireless: Add proper locking for the function, b43_op_beacon_set_tim in main.c
From: nick @ 2015-01-05 17:16 UTC (permalink / raw)
To: Kalle Valo
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
stefano.brivio-hl5o88x/ua9eoWH0uzbU5w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87tx05378x.fsf-HodKDYzPHsUD5k0oWYwrnHL1okKdlPRT@public.gmane.org>
Kalle,
That's fine. On the other hand I am interested in the
reasons why this patch was dropped so I can send in a
version 2 of this patch.
Regards Nick
On 2015-01-05 04:29 AM, Kalle Valo wrote:
> Nicholas Krause <xerofoify-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>
>> This adds proper locking for the function, b43_op_beacon_set_tim in
>> main.c by using the mutex lock in the structure pointer wl, as
>> embedded into this pointer as a mutex in order to protect against
>> multiple access to the pointer wl when updating the templates for this
>> pointer in the function, b43_update_templates internally in the
>> function, b43_op_beacon_set_tim.
>>
>> Signed-off-by: Nicholas Krause <xerofoify-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Based on the discussion I have dropped this patch.
>
^ permalink raw reply
* Re: iproute2: Run over all netns
From: Nicolas Dichtel @ 2015-01-05 16:40 UTC (permalink / raw)
To: Vadim Kochan, netdev
In-Reply-To: <20150105122333.GA6646@angus-think.wlc.globallogic.com>
Le 05/01/2015 13:23, Vadim Kochan a écrit :
> Hi All,
>
> I have some piece of code which allow 'ip cmd'
> on each netns, I found it useful for getting some info
> from all the netns in one shot, BUT I faced with one issue
> which mostly related to the user interface design. The problem
> is that it would be good to print netns name only when
> user uses "show" command, but not for updating/adding (IMHO),
> but its hard to find the good way to implement this.
>
> To run each netns the 'ip -net all CMD ...' construction can be used.
>
> I see the following options for this:
>
> #1 Add additional option ( -N ? ) for show netns label on each executing of CMD:
>
> # ip -net all -N link
>
> [test_net]
> 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN group default
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>
> [home0]
> 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN group default
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>
> [lan0]
> 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN group default
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>
> [wan0]
> 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN group default
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> 2: br0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default
> link/ether 16:f7:cb:b6:7a:8e brd ff:ff:ff:ff:ff:ff
>
> [vnet0]
> 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN group default
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>
>
> and w/o:
>
> # ip -net all link
>
> 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN group default
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>
> 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN group default
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>
> 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN group default
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>
> 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN group default
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> 2: br0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default
> link/ether 16:f7:cb:b6:7a:8e brd ff:ff:ff:ff:ff:ff
>
> 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN group default
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>
> the last one is not so useful right ?
>
> #2 Prints netns name by default if "-net all" was specified
> (add option to prevent this ?), so it will be printed even on the
> add/del/change commands ...
I vote for this one (I don't think the option to prevent it is needed, it's
better to be explicit).
>
> # ip -net all link add ...
>
> [home0]
> [lan0]
> [wan0]
> [vnet0]
>
> but does it really useless to see that it will shows all the netns
> on which cmd has been ran ?
I tend to say yes (another process may add/remove a netns in the same time).
Regards,
Nicolas
^ permalink raw reply
* Re: [PATCH] Fix NUL (\0 or \x00) specification in string
From: David Sterba @ 2015-01-05 15:00 UTC (permalink / raw)
To: Giel van Schijndel
Cc: linux-kernel, Armin Schindler, Karsten Keil,
open list:ISDN SUBSYSTEM
In-Reply-To: <1420394722-20197-1-git-send-email-me@mortis.eu>
I'm replying to all your recent patches here as they are fixing things
reported in http://www.viva64.com/en/b/0299/ . I'ts a good practice to
give the credit the reporter.
The blogpost also contains analyses of the issues so it could help
reviewing the patches.
^ 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