Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v3 1/2] iproute2: bridge: support vlan range adds
From: roopa @ 2015-01-26  2:26 UTC (permalink / raw)
  To: netdev, shemminger, vyasevic; +Cc: wkok, sfeldma

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This patch adds vlan range support to bridge add command
using the newly added vinfo flags BRIDGE_VLAN_INFO_RANGE_BEGIN and
BRIDGE_VLAN_INFO_RANGE_END.

$bridge vlan show
port    vlan ids
br0      1 PVID Egress Untagged

dummy0   1 PVID Egress Untagged

$bridge vlan add vid 10-15 dev dummy0
port    vlan ids
br0      1 PVID Egress Untagged

dummy0   1 PVID Egress Untagged
         10
         11
         12
         13
         14
         15

$bridge vlan del vid 14 dev dummy0

$bridge vlan show
port    vlan ids
br0      1 PVID Egress Untagged

dummy0   1 PVID Egress Untagged
         10
         11
         12
         13
         15

$bridge vlan del vid 10-15 dev dummy0

$bridge vlan show
port    vlan ids
br0      1 PVID Egress Untagged

dummy0   1 PVID Egress Untagged

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Signed-off-by: Wilson Kok <wkok@cumulusnetworks.com>
---
 bridge/vlan.c             |   44 ++++++++++++++++++++++++++++++++++++++++----
 include/linux/if_bridge.h |    2 ++
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/bridge/vlan.c b/bridge/vlan.c
index 3bd7b0d..88992e6 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -32,6 +32,7 @@ static int vlan_modify(int cmd, int argc, char **argv)
 	} req;
 	char *d = NULL;
 	short vid = -1;
+	short vid_end = -1;
 	struct rtattr *afspec;
 	struct bridge_vlan_info vinfo;
 	unsigned short flags = 0;
@@ -49,8 +50,18 @@ static int vlan_modify(int cmd, int argc, char **argv)
 			NEXT_ARG();
 			d = *argv;
 		} else if (strcmp(*argv, "vid") == 0) {
+			char *p;
 			NEXT_ARG();
-			vid = atoi(*argv);
+			p = strchr(*argv, '-');
+			if (p) {
+				*p = '\0';
+				p++;
+				vid = atoi(*argv);
+				vid_end = atoi(p);
+				vinfo.flags |= BRIDGE_VLAN_INFO_RANGE_BEGIN;
+			} else {
+				vid = atoi(*argv);
+			}
 		} else if (strcmp(*argv, "self") == 0) {
 			flags |= BRIDGE_FLAGS_SELF;
 		} else if (strcmp(*argv, "master") == 0) {
@@ -83,15 +94,40 @@ static int vlan_modify(int cmd, int argc, char **argv)
 		return -1;
 	}
 
-	vinfo.vid = vid;
+	if (vinfo.flags & BRIDGE_VLAN_INFO_RANGE_BEGIN) {
+		if (vid_end == -1 || vid_end >= 4096 || vid >= vid_end) {
+			fprintf(stderr, "Invalid VLAN range \"%hu-%hu\"\n",
+				vid, vid_end);
+			return -1;
+		}
+		if (vinfo.flags & BRIDGE_VLAN_INFO_PVID) {
+			fprintf(stderr,
+				"pvid cannot be configured for a vlan range\n");
+			return -1;
+		}
+	}
 
 	afspec = addattr_nest(&req.n, sizeof(req), IFLA_AF_SPEC);
 
 	if (flags)
 		addattr16(&req.n, sizeof(req), IFLA_BRIDGE_FLAGS, flags);
 
-	addattr_l(&req.n, sizeof(req), IFLA_BRIDGE_VLAN_INFO, &vinfo,
-		  sizeof(vinfo));
+	vinfo.vid = vid;
+	if (vid_end != -1) {
+		/* send vlan range start */
+		addattr_l(&req.n, sizeof(req), IFLA_BRIDGE_VLAN_INFO, &vinfo,
+			  sizeof(vinfo));
+		vinfo.flags &= ~BRIDGE_VLAN_INFO_RANGE_BEGIN;
+
+		/* Now send the vlan range end */
+		vinfo.flags |= BRIDGE_VLAN_INFO_RANGE_END;
+		vinfo.vid = vid_end;
+		addattr_l(&req.n, sizeof(req), IFLA_BRIDGE_VLAN_INFO, &vinfo,
+			  sizeof(vinfo));
+	} else {
+		addattr_l(&req.n, sizeof(req), IFLA_BRIDGE_VLAN_INFO, &vinfo,
+			  sizeof(vinfo));
+	}
 
 	addattr_nest_end(&req.n, afspec);
 
diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 19ff22a..efa10b8 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -125,6 +125,8 @@ enum {
 #define BRIDGE_VLAN_INFO_MASTER	(1<<0)	/* Operate on Bridge device as well */
 #define BRIDGE_VLAN_INFO_PVID	(1<<1)	/* VLAN is PVID, ingress untagged */
 #define BRIDGE_VLAN_INFO_UNTAGGED	(1<<2)	/* VLAN egresses untagged */
+#define BRIDGE_VLAN_INFO_RANGE_BEGIN	(1<<3) /* VLAN is start of vlan range */
+#define BRIDGE_VLAN_INFO_RANGE_END	(1<<4)	/* VLAN is end of vlan range */
 
 struct bridge_vlan_info {
 	__u16 flags;
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next v3 2/2] iproute2: bridge vlan show new option to print ranges
From: roopa @ 2015-01-26  2:26 UTC (permalink / raw)
  To: netdev, shemminger, vyasevic; +Cc: wkok, sfeldma

From: Roopa Prabhu <roopa@cumulusnetworks.com>

Introduce new option -c[ompressvlans] to request
vlan ranges from kernel

(pls suggest better option names if this does not look ok)

$bridge vlan show
port	vlan ids
dummy0	 1 PVID Egress Untagged

dummy1	 1 PVID Egress Untagged
	 2
	 3
	 4
	 5
	 6
	 7
	 9
	 10
	 12

br0	 1 PVID Egress Untagged

$bridge help
Usage: bridge [ OPTIONS ] OBJECT { COMMAND | help }
where  OBJECT := { link | fdb | mdb | vlan | monitor }
       OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] |
                    -o[neline] | -t[imestamp] | -n[etns] name |
                    -c[ompressvlans] }
$bridge -c vlan show
port	vlan ids
dummy0	 1 PVID Egress Untagged

dummy1	 1 PVID Egress Untagged
	 2-7
	 9-10
	 12

br0	 1 PVID Egress Untagged

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 bridge/br_common.h        |    1 +
 bridge/bridge.c           |    6 +++++-
 bridge/vlan.c             |   11 +++++++++--
 include/linux/rtnetlink.h |    1 +
 4 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/bridge/br_common.h b/bridge/br_common.h
index 12fce3e..169a162 100644
--- a/bridge/br_common.h
+++ b/bridge/br_common.h
@@ -16,4 +16,5 @@ extern int preferred_family;
 extern int show_stats;
 extern int show_details;
 extern int timestamp;
+extern int compress_vlans;
 extern struct rtnl_handle rth;
diff --git a/bridge/bridge.c b/bridge/bridge.c
index 5fcc552..88469ca 100644
--- a/bridge/bridge.c
+++ b/bridge/bridge.c
@@ -21,6 +21,7 @@ int resolve_hosts;
 int oneline = 0;
 int show_stats;
 int show_details;
+int compress_vlans;
 int timestamp;
 char * _SL_ = NULL;
 
@@ -32,7 +33,8 @@ static void usage(void)
 "Usage: bridge [ OPTIONS ] OBJECT { COMMAND | help }\n"
 "where  OBJECT := { link | fdb | mdb | vlan | monitor }\n"
 "       OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] |\n"
-"                    -o[neline] | -t[imestamp] | -n[etns] name }\n");
+"                    -o[neline] | -t[imestamp] | -n[etns] name |\n"
+"                    -c[ompressvlans] }\n");
 	exit(-1);
 }
 
@@ -117,6 +119,8 @@ main(int argc, char **argv)
 			NEXT_ARG();
 			if (netns_switch(argv[1]))
 				exit(-1);
+		} else if (matches(opt, "-compressvlans") == 0) {
+			++compress_vlans;
 		} else {
 			fprintf(stderr, "Option \"%s\" is unknown, try \"bridge help\".\n", opt);
 			exit(-1);
diff --git a/bridge/vlan.c b/bridge/vlan.c
index 88992e6..9f6c84e 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -182,7 +182,12 @@ static int print_vlan(const struct sockaddr_nl *who,
 				continue;
 
 			vinfo = RTA_DATA(i);
-			fprintf(fp, "\t %hu", vinfo->vid);
+			if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_END)
+				fprintf(fp, "-%hu", vinfo->vid);
+			else
+				fprintf(fp, "\t %hu", vinfo->vid);
+			if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_BEGIN)
+				continue;
 			if (vinfo->flags & BRIDGE_VLAN_INFO_PVID)
 				fprintf(fp, " PVID");
 			if (vinfo->flags & BRIDGE_VLAN_INFO_UNTAGGED)
@@ -218,7 +223,9 @@ static int vlan_show(int argc, char **argv)
 	}
 
 	if (rtnl_wilddump_req_filter(&rth, PF_BRIDGE, RTM_GETLINK,
-				     RTEXT_FILTER_BRVLAN) < 0) {
+				    (compress_vlans ?
+				    RTEXT_FILTER_BRVLAN_COMPRESSED :
+				    RTEXT_FILTER_BRVLAN)) < 0) {
 		perror("Cannont send dump request");
 		exit(1);
 	}
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 9aa5c2f..19eadb1 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -632,6 +632,7 @@ struct tcamsg {
 /* New extended info filters for IFLA_EXT_MASK */
 #define RTEXT_FILTER_VF		(1 << 0)
 #define RTEXT_FILTER_BRVLAN	(1 << 1)
+#define RTEXT_FILTER_BRVLAN_COMPRESSED  (1 << 2)
 
 /* End of information exported to user level */
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH v1 0/2] ARM EXYNOS5 & net: add fixed phy support
From: Ming Lei @ 2015-01-26  2:37 UTC (permalink / raw)
  To: linux-arm-kernel, David S. Miller
  Cc: linux-samsung-soc, Kukjin Kim, netdev, Byungho An,
	Florian Fainelli

Hi,

These two patches adds fixed phy support using the fixed-link DT binding
for stmmac.

v1:
	- bypassing check on phy id for fixed phy (patch 2/2)

Thanks,
Ming Lei

^ permalink raw reply

* [PATCH v1 1/2] ARM: exynos5440-sd5v1: switch to fixed-link DT binding
From: Ming Lei @ 2015-01-26  2:37 UTC (permalink / raw)
  To: linux-arm-kernel, David S. Miller
  Cc: linux-samsung-soc, Kukjin Kim, netdev, Byungho An,
	Florian Fainelli, Ming Lei
In-Reply-To: <1422239858-3452-1-git-send-email-ming.lei@canonical.com>

The previous dts property isn't standard way for fixed phy
support, and switch to the fixed-link DT binding.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 arch/arm/boot/dts/exynos5440-sd5v1.dts |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5440-sd5v1.dts b/arch/arm/boot/dts/exynos5440-sd5v1.dts
index 268609a..ee65a5c 100644
--- a/arch/arm/boot/dts/exynos5440-sd5v1.dts
+++ b/arch/arm/boot/dts/exynos5440-sd5v1.dts
@@ -28,8 +28,10 @@
 	};
 
 	gmac: ethernet@00230000 {
-		fixed_phy;
-		phy_addr = <1>;
+		fixed-link {
+			speed = <1000>;
+			full-duplex;
+		};
 	};
 
 	spi {
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v1 2/2] net: stmmac: add fixed_phy support via fixed-link DT binding
From: Ming Lei @ 2015-01-26  2:37 UTC (permalink / raw)
  To: linux-arm-kernel, David S. Miller
  Cc: linux-samsung-soc, Kukjin Kim, netdev, Byungho An,
	Florian Fainelli, Ming Lei
In-Reply-To: <1422239858-3452-1-git-send-email-ming.lei@canonical.com>

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |    5 ++++-
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |   10 ++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 8c6b7c16..ddb4351 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -847,8 +847,11 @@ static int stmmac_init_phy(struct net_device *dev)
 	 * 0 rather than 0xffff. Catch this here and treat 0 as a non-existent
 	 * device as well.
 	 * Note: phydev->phy_id is the result of reading the UID PHY registers.
+	 * But phy_id returned from fixed phy is always zero, so bypass the
+	 * check for fixed phy.
 	 */
-	if (phydev->phy_id == 0) {
+	if (phydev->phy_id == 0 && (!priv->plat->phy_bus_name ||
+			strcmp(priv->plat->phy_bus_name,"fixed"))) {
 		phy_disconnect(phydev);
 		return -ENODEV;
 	}
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 3039de2..73a3ced 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -27,6 +27,7 @@
 #include <linux/of.h>
 #include <linux/of_net.h>
 #include <linux/of_device.h>
+#include <linux/of_mdio.h>
 
 #include "stmmac.h"
 #include "stmmac_platform.h"
@@ -216,6 +217,15 @@ static int stmmac_probe_config_dt(struct platform_device *pdev,
 		plat->pmt = 1;
 	}
 
+	if (of_phy_is_fixed_link(np)) {
+		int ret = of_phy_register_fixed_link(np);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to register fixed PHY\n");
+			return ret;
+		}
+		plat->phy_bus_name = "fixed";
+	}
+
 	if (of_device_is_compatible(np, "snps,dwmac-3.610") ||
 		of_device_is_compatible(np, "snps,dwmac-3.710")) {
 		plat->enh_desc = 1;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH] brcmfmac: avoid duplicated suspend/resume operation
From: Fu, Zhonghui @ 2015-01-26  2:46 UTC (permalink / raw)
  To: brudley-dY08KVG/lbpWk0Htik3J/w, Arend van Spriel, Franky Lin,
	meuleman-dY08KVG/lbpWk0Htik3J/w, Kalle Valo,
	linville-2XuSBdqkA4R54TAoqtyWWQ, pieterpg-dY08KVG/lbpWk0Htik3J/w,
	hdegoede-H+wXaHxf7aLQT0dZR+AlfA, wens-jdAy2FN1RRM,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	brcm80211-dev-list-dY08KVG/lbpWk0Htik3J/w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

>From ff39ed4af9f1c50358fe92ec4c8eaac9db183e00 Mon Sep 17 00:00:00 2001
From: Zhonghui Fu <zhonghui.fu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Date: Mon, 26 Jan 2015 10:13:21 +0800
Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

WiFi chip has 2 SDIO functions, and PM core will trigger
twice suspend/resume operations for one WiFi chip to do
the same things. This patch avoid this case.

Acked-by: Arend van Spriel<arend-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Zhonghui Fu <zhonghui.fu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |   17 +++++++++++++++--
 1 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index 9880dae..618b545 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -1139,11 +1139,17 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled)
 static int brcmf_ops_sdio_suspend(struct device *dev)
 {
 	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+	struct brcmf_sdio_dev *sdiodev;
 	mmc_pm_flag_t sdio_flags;
+	struct sdio_func *func = dev_to_sdio_func(dev);
 
 	brcmf_dbg(SDIO, "Enter\n");
 
+	if (func->num == 2)
+		return 0;
+
+	sdiodev = bus_if->bus_priv.sdio;
+
 	atomic_set(&sdiodev->suspend, true);
 
 	if (sdiodev->wowl_enabled) {
@@ -1164,9 +1170,16 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
 static int brcmf_ops_sdio_resume(struct device *dev)
 {
 	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+	struct brcmf_sdio_dev *sdiodev;
+	struct sdio_func *func = dev_to_sdio_func(dev);
 
 	brcmf_dbg(SDIO, "Enter\n");
+
+	if (func->num == 2)
+		return 0;
+
+	sdiodev = bus_if->bus_priv.sdio;
+
 	if (sdiodev->pdata && sdiodev->pdata->oob_irq_supported)
 		disable_irq_wake(sdiodev->pdata->oob_irq_nr);
 	brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS);
-- 1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH v1 2/2] net: stmmac: add fixed_phy support via fixed-link DT binding
From: Andreas Färber @ 2015-01-26  3:01 UTC (permalink / raw)
  To: Ming Lei, linux-arm-kernel, David S. Miller
  Cc: linux-samsung-soc, Kukjin Kim, netdev, Byungho An,
	Florian Fainelli
In-Reply-To: <1422239858-3452-3-git-send-email-ming.lei@canonical.com>

[-- Attachment #1: Type: text/plain, Size: 1152 bytes --]

Am 26.01.2015 um 03:37 schrieb Ming Lei:
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 8c6b7c16..ddb4351 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -847,8 +847,11 @@ static int stmmac_init_phy(struct net_device *dev)
>  	 * 0 rather than 0xffff. Catch this here and treat 0 as a non-existent
>  	 * device as well.
>  	 * Note: phydev->phy_id is the result of reading the UID PHY registers.
> +	 * But phy_id returned from fixed phy is always zero, so bypass the
> +	 * check for fixed phy.
>  	 */
> -	if (phydev->phy_id == 0) {
> +	if (phydev->phy_id == 0 && (!priv->plat->phy_bus_name ||
> +			strcmp(priv->plat->phy_bus_name,"fixed"))) {

Small nit here: There's a space missing between the arguments.

Cheers,
Andreas

>  		phy_disconnect(phydev);
>  		return -ENODEV;
>  	}
[snip]

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)


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

^ permalink raw reply

* netlink: Kill redundant net argument in netlink_insert
From: Herbert Xu @ 2015-01-26  3:02 UTC (permalink / raw)
  To: netdev

The socket already carries the net namespace with it so there is
no need to be passing another net around.
    
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 7a94185..d77b346 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -994,11 +994,10 @@ static struct sock *__netlink_lookup(struct netlink_table *table, u32 portid,
 					 &netlink_compare, &arg);
 }
 
-static bool __netlink_insert(struct netlink_table *table, struct sock *sk,
-			     struct net *net)
+static bool __netlink_insert(struct netlink_table *table, struct sock *sk)
 {
 	struct netlink_compare_arg arg = {
-		.net = net,
+		.net = sock_net(sk),
 		.portid = nlk_sk(sk)->portid,
 	};
 
@@ -1047,7 +1046,7 @@ netlink_update_listeners(struct sock *sk)
 	 * makes sure updates are visible before bind or setsockopt return. */
 }
 
-static int netlink_insert(struct sock *sk, struct net *net, u32 portid)
+static int netlink_insert(struct sock *sk, u32 portid)
 {
 	struct netlink_table *table = &nl_table[sk->sk_protocol];
 	int err;
@@ -1067,7 +1066,7 @@ static int netlink_insert(struct sock *sk, struct net *net, u32 portid)
 	sock_hold(sk);
 
 	err = 0;
-	if (!__netlink_insert(table, sk, net)) {
+	if (!__netlink_insert(table, sk)) {
 		err = -EADDRINUSE;
 		sock_put(sk);
 	}
@@ -1289,7 +1288,7 @@ retry:
 	}
 	rcu_read_unlock();
 
-	err = netlink_insert(sk, net, portid);
+	err = netlink_insert(sk, portid);
 	if (err == -EADDRINUSE)
 		goto retry;
 
@@ -1477,7 +1476,7 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr,
 
 	if (!nlk->portid) {
 		err = nladdr->nl_pid ?
-			netlink_insert(sk, net, nladdr->nl_pid) :
+			netlink_insert(sk, nladdr->nl_pid) :
 			netlink_autobind(sock);
 		if (err) {
 			netlink_undo_bind(nlk->ngroups, groups, sk);
@@ -2483,7 +2482,7 @@ __netlink_kernel_create(struct net *net, int unit, struct module *module,
 	if (cfg && cfg->input)
 		nlk_sk(sk)->netlink_rcv = cfg->input;
 
-	if (netlink_insert(sk, net, 0))
+	if (netlink_insert(sk, 0))
 		goto out_sock_release;
 
 	nlk = nlk_sk(sk);
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply related

* Re: [PATCH net-next v3 2/2] iproute2: bridge vlan show new option to print ranges
From: Scott Feldman @ 2015-01-26  4:35 UTC (permalink / raw)
  To: Roopa Prabhu; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson Kok
In-Reply-To: <1422239185-14054-3-git-send-email-roopa@cumulusnetworks.com>

> diff --git a/bridge/bridge.c b/bridge/bridge.c
> index 5fcc552..88469ca 100644
> --- a/bridge/bridge.c
> +++ b/bridge/bridge.c
> @@ -21,6 +21,7 @@ int resolve_hosts;
>  int oneline = 0;
>  int show_stats;
>  int show_details;
> +int compress_vlans;
>  int timestamp;
>  char * _SL_ = NULL;
>
> @@ -32,7 +33,8 @@ static void usage(void)
>  "Usage: bridge [ OPTIONS ] OBJECT { COMMAND | help }\n"
>  "where  OBJECT := { link | fdb | mdb | vlan | monitor }\n"
>  "       OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] |\n"
> -"                    -o[neline] | -t[imestamp] | -n[etns] name }\n");
> +"                    -o[neline] | -t[imestamp] | -n[etns] name |\n"
> +"                    -c[ompressvlans] }\n");

This option is awkward at this level since it's specific to bridge vlan cmd.

Is it even necessary?  The real user of compressed vlans isn't the
"bridge show" cmd; just keep "bridge show" simple, listing each vlan
per line.

^ permalink raw reply

* [PATCH v6 0/7] can: kvaser_usb: Leaf bugfixes and USBCan-II support
From: Ahmed S. Darwish @ 2015-01-26  5:17 UTC (permalink / raw)
  To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
	Marc Kleine-Budde, Andri Yngvason
  Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20141223154654.GB6460@vivalin-002>

Hi!

This is an updated patch series for the Kvaser CAN/USB devices:

1- Extra patches are now added to the series. Most importantly
patch #1 which fixes a critical `sleep in atomic context' bug
in the current upstream driver. Patch #2 fixes a corruption in
the kernel logs, also affecting current upstream driver. Patch
#4 was originally USBCan-II only, but since it's a generic fix,
it's now retrofitted to the Leaf-only upstream driver first.

2- The series has been re-organized so that patches #1 -> #4
inclusive can go to linux-can/origin, while the rest can move
to -next.

3- There are some important updates regarding the USBCan-II
error counters, and how really erratic their heaviour is. All
the new details are covered at the bottom of this URL:

	http://article.gmane.org/gmane.linux.can/7481

4- Attached below is the new candump traces. Now
`back-to-error-active' states appear _if_ the hardware was
kind enough and decreased the error counters appropriately.
The earlier code did not recognize the error counters going
down, thus the `back-to-error-active' transitions did not
appear.

###########################################################

* Bus-off scenario (with transitions active -> passive -> back-to-active):

 (000.000000)  can0  20000080   [8]  00 00 00 00 00 00 30 00   ERRORFRAME
	bus-error
	error-counter-tx-rx{{48}{0}}
 (000.000011)  can0  20000084   [8]  00 20 00 00 00 00 88 00   ERRORFRAME
	controller-problem{tx-error-passive}
	bus-error
	error-counter-tx-rx{{136}{0}}
 (000.000001)  can0  20000080   [8]  00 00 00 00 00 00 88 00   ERRORFRAME
	bus-error
	error-counter-tx-rx{{136}{0}}
 (000.000987)  can0  20000080   [8]  00 00 00 00 00 00 98 00   ERRORFRAME
	bus-error
	error-counter-tx-rx{{152}{0}}
 (000.002011)  can0  20000084   [8]  00 40 00 00 00 00 30 00   ERRORFRAME
	controller-problem{back-to-error-active}
	bus-error
	error-counter-tx-rx{{48}{0}}
 (000.000004)  can0  20000084   [8]  00 20 00 00 00 00 78 00   ERRORFRAME
	controller-problem{tx-error-passive}
	bus-error
	error-counter-tx-rx{{120}{0}}
 (000.000002)  can0  20000080   [8]  00 00 00 00 00 00 88 00   ERRORFRAME
	bus-error
	error-counter-tx-rx{{136}{0}}
 (000.000005)  can0  20000080   [8]  00 00 00 00 00 00 90 00   ERRORFRAME
	bus-error
	error-counter-tx-rx{{144}{0}}
 (000.000002)  can0  20000080   [8]  00 00 00 00 00 00 98 00   ERRORFRAME
	bus-error
	error-counter-tx-rx{{152}{0}}
 (000.000001)  can0  20000080   [8]  00 00 00 00 00 00 A0 00   ERRORFRAME
	bus-error
	error-counter-tx-rx{{160}{0}}
 (000.000966)  can0  20000080   [8]  00 00 00 00 00 00 A8 00   ERRORFRAME
	bus-error
	error-counter-tx-rx{{168}{0}}
 (000.002998)  can0  20000084   [8]  00 40 00 00 00 00 30 00   ERRORFRAME
	controller-problem{back-to-error-active}
	bus-error
	error-counter-tx-rx{{48}{0}}
 (000.000004)  can0  20000084   [8]  00 20 00 00 00 00 80 00   ERRORFRAME
	controller-problem{tx-error-passive}
	bus-error
	error-counter-tx-rx{{128}{0}}
 (000.000002)  can0  20000080   [8]  00 00 00 00 00 00 88 00   ERRORFRAME
	bus-error
	error-counter-tx-rx{{136}{0}}
 (001.031035)  can0  200000C0   [8]  00 00 00 00 00 00 00 29   ERRORFRAME
	bus-off
	bus-error
	error-counter-tx-rx{{0}{41}}

###########################################################

Regular sending, unpluggng CAN connector, then plugging again:

(with transitions active -> warning -> passive)

[ As stated earlier, the counters don't get decreased upon CAN
replug, even if they were constantly polled. ]

 (000.011001)  can0  2A1   [1]  E5
 (000.010001)  can0  50E   [8]  E6 05 00 00 00 00 00 00
 (000.009999)  can0  009   [1]  E7
 (000.011000)  can0  6E2   [8]  E8 05 00 00 00 00 00 00
 (000.009999)  can0  314   [2]  E9 05
 (000.010001)  can0  708   [6]  EA 05 00 00 00 00
 (000.010991)  can0  20000080   [8]  00 00 00 00 00 00 00 09   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{9}}
 (000.000002)  can0  20000080   [8]  00 00 00 00 00 00 00 0A   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{10}}
 (000.000007)  can0  20000080   [8]  00 00 00 00 00 00 00 2E   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{46}}
 (000.000003)  can0  20000080   [8]  00 00 00 00 00 00 00 37   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{55}}
 (000.000002)  can0  20000080   [8]  00 00 00 00 00 00 00 40   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{64}}
 (000.000993)  can0  20000080   [8]  00 00 00 00 00 00 00 49   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{73}}
 (000.000002)  can0  20000080   [8]  00 00 00 00 00 00 00 52   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{82}}
 (000.000001)  can0  20000080   [8]  00 00 00 00 00 00 00 5B   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{91}}
 (000.000028)  can0  20000084   [8]  00 04 00 00 00 00 00 6C   ERRORFRAME
	controller-problem{rx-error-warning}
	bus-error
	error-counter-tx-rx{{0}{108}}
 (000.000955)  can0  20000080   [8]  00 00 00 00 00 00 00 7F   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{127}}
 (000.000008)  can0  20000084   [8]  00 10 00 00 00 00 00 87   ERRORFRAME
	controller-problem{rx-error-passive}
	bus-error
	error-counter-tx-rx{{0}{135}}
 (000.000001)  can0  20000080   [8]  00 00 00 00 00 00 00 87   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{135}}
 (000.000004)  can0  20000080   [8]  00 00 00 00 00 00 00 87   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{135}}

((( Then a continous flood, exactly similar to the above packet, appears )))

 (000.500004)  can0  2ED   [4]  EB 05 00 00
 (000.000006)  can0  0DD   [5]  EC 05 00 00 00
 (000.000002)  can0  1D3   [1]  ED
 (000.000988)  can0  20D   [8]  EE 05 00 00 00 00 00 00
 (000.000006)  can0  04B   [8]  EF 05 00 00 00 00 00 00
 (000.000002)  can0  320   [8]  F0 05 00 00 00 00 00 00
 (000.000002)  can0  023   [8]  F1 05 00 00 00 00 00 00
 (000.000989)  can0  21D   [8]  F2 05 00 00 00 00 00 00
 (000.000005)  can0  17D   [8]  F3 05 00 00 00 00 00 00
 (000.000002)  can0  6DC   [8]  F4 05 00 00 00 00 00 00
 (000.000993)  can0  62D   [8]  F5 05 00 00 00 00 00 00
 (000.000006)  can0  18B   [6]  F6 05 00 00 00 00
 (000.000001)  can0  7EB   [8]  F7 05 00 00 00 00 00 00
 (000.000001)  can0  014   [8]  F8 05 00 00 00 00 00 00
 (000.000994)  can0  52F   [8]  F9 05 00 00 00 00 00 00

--
Regards,
Darwish

^ permalink raw reply

* Re: [PATCH net-next v3 2/2] iproute2: bridge vlan show new option to print ranges
From: roopa @ 2015-01-26  5:18 UTC (permalink / raw)
  To: Scott Feldman; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson Kok
In-Reply-To: <CAE4R7bCR6YSK9J+OV+04w_UXWoSu56xSvHXkYkDzqP4Qudn76Q@mail.gmail.com>

On 1/25/15, 8:35 PM, Scott Feldman wrote:
>> diff --git a/bridge/bridge.c b/bridge/bridge.c
>> index 5fcc552..88469ca 100644
>> --- a/bridge/bridge.c
>> +++ b/bridge/bridge.c
>> @@ -21,6 +21,7 @@ int resolve_hosts;
>>   int oneline = 0;
>>   int show_stats;
>>   int show_details;
>> +int compress_vlans;
>>   int timestamp;
>>   char * _SL_ = NULL;
>>
>> @@ -32,7 +33,8 @@ static void usage(void)
>>   "Usage: bridge [ OPTIONS ] OBJECT { COMMAND | help }\n"
>>   "where  OBJECT := { link | fdb | mdb | vlan | monitor }\n"
>>   "       OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] |\n"
>> -"                    -o[neline] | -t[imestamp] | -n[etns] name }\n");
>> +"                    -o[neline] | -t[imestamp] | -n[etns] name |\n"
>> +"                    -c[ompressvlans] }\n");
> This option is awkward at this level since it's specific to bridge vlan cmd.
>
> Is it even necessary?  The real user of compressed vlans isn't the
> "bridge show" cmd; just keep "bridge show" simple, listing each vlan
> per line.
without ranges it does get a bit difficult (or rather long) to read if 
there are many vlans (it was also a comment I received on v2).

All the available options whether they apply to the immediate 
sub-command or not are at the toplevel command today.
Hence the position (Not a big fan of the current name. suggestions welcome).

I would have loved to have the default output print ranges, If that is 
acceptable. But, that can break new iproute2 on older kernels (If that 
is something we should care about).

other option is keyword argument like maybe 'bridge vlan show 
compressed' or 'bridge vlan show ranges'

Thanks,
Roopa

^ permalink raw reply

* [PATCH v6 1/7] can: kvaser_usb: Do not sleep in atomic context
From: Ahmed S. Darwish @ 2015-01-26  5:20 UTC (permalink / raw)
  To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
	Marc Kleine-Budde, Andri Yngvason
  Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150126051755.GA3014@Darwish.PC>

From: Ahmed S. Darwish <ahmed.darwish@valeo.com>

Upon receiving a hardware event with the BUS_RESET flag set,
the driver kills all of its anchored URBs and resets all of
its transmit URB contexts.

Unfortunately it does so under the context of URB completion
handler `kvaser_usb_read_bulk_callback()', which is often
called in an atomic context.

While the device is flooded with many received error packets,
usb_kill_urb() typically sleeps/reschedules till the transfer
request of each killed URB in question completes, leading to
the sleep in atomic bug. [3]

In v2 submission of the original driver patch [1], it was
stated that the URBs kill and tx contexts reset was needed
since we don't receive any tx acknowledgments later and thus
such resources will be locked down forever. Fortunately this
is no longer needed since an earlier bugfix in this patch
series is now applied: all tx URB contexts are reset upon CAN
channel close. [2]

Moreover, a BUS_RESET is now treated _exactly_ like a BUS_OFF
event, which is the recommended handling method advised by
the device manufacturer.

[1] http://article.gmane.org/gmane.linux.network/239442
    http://www.webcitation.org/6Vr2yagAQ

[2] can: kvaser_usb: Reset all URB tx contexts upon channel close
    889b77f7fd2bcc922493d73a4c51d8a851505815

[3] Stacktrace:

 <IRQ>  [<ffffffff8158de87>] dump_stack+0x45/0x57
 [<ffffffff8158b60c>] __schedule_bug+0x41/0x4f
 [<ffffffff815904b1>] __schedule+0x5f1/0x700
 [<ffffffff8159360a>] ? _raw_spin_unlock_irqrestore+0xa/0x10
 [<ffffffff81590684>] schedule+0x24/0x70
 [<ffffffff8147d0a5>] usb_kill_urb+0x65/0xa0
 [<ffffffff81077970>] ? prepare_to_wait_event+0x110/0x110
 [<ffffffff8147d7d8>] usb_kill_anchored_urbs+0x48/0x80
 [<ffffffffa01f4028>] kvaser_usb_unlink_tx_urbs+0x18/0x50 [kvaser_usb]
 [<ffffffffa01f45d0>] kvaser_usb_rx_error+0xc0/0x400 [kvaser_usb]
 [<ffffffff8108b14a>] ? vprintk_default+0x1a/0x20
 [<ffffffffa01f5241>] kvaser_usb_read_bulk_callback+0x4c1/0x5f0 [kvaser_usb]
 [<ffffffff8147a73e>] __usb_hcd_giveback_urb+0x5e/0xc0
 [<ffffffff8147a8a1>] usb_hcd_giveback_urb+0x41/0x110
 [<ffffffffa0008748>] finish_urb+0x98/0x180 [ohci_hcd]
 [<ffffffff810cd1a7>] ? acct_account_cputime+0x17/0x20
 [<ffffffff81069f65>] ? local_clock+0x15/0x30
 [<ffffffffa000a36b>] ohci_work+0x1fb/0x5a0 [ohci_hcd]
 [<ffffffff814fbb31>] ? process_backlog+0xb1/0x130
 [<ffffffffa000cd5b>] ohci_irq+0xeb/0x270 [ohci_hcd]
 [<ffffffff81479fc1>] usb_hcd_irq+0x21/0x30
 [<ffffffff8108bfd3>] handle_irq_event_percpu+0x43/0x120
 [<ffffffff8108c0ed>] handle_irq_event+0x3d/0x60
 [<ffffffff8108ec84>] handle_fasteoi_irq+0x74/0x110
 [<ffffffff81004dfd>] handle_irq+0x1d/0x30
 [<ffffffff81004727>] do_IRQ+0x57/0x100
 [<ffffffff8159482a>] common_interrupt+0x6a/0x6a

Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
 drivers/net/can/usb/kvaser_usb.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index c32cd61..978a25e 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -662,11 +662,6 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
 	priv = dev->nets[channel];
 	stats = &priv->netdev->stats;
 
-	if (status & M16C_STATE_BUS_RESET) {
-		kvaser_usb_unlink_tx_urbs(priv);
-		return;
-	}
-
 	skb = alloc_can_err_skb(priv->netdev, &cf);
 	if (!skb) {
 		stats->rx_dropped++;
@@ -677,7 +672,7 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
 
 	netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status);
 
-	if (status & M16C_STATE_BUS_OFF) {
+	if (status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
 		cf->can_id |= CAN_ERR_BUSOFF;
 
 		priv->can.can_stats.bus_off++;
-- 
1.7.7.6



^ permalink raw reply related

* [PATCH v6 2/7] can: kvaser_usb: Send correct context to URB completion
From: Ahmed S. Darwish @ 2015-01-26  5:22 UTC (permalink / raw)
  To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
	Marc Kleine-Budde, Andri Yngvason
  Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150126052039.GB3014@Darwish.PC>

From: Ahmed S. Darwish <ahmed.darwish@valeo.com>

Send expected argument to the URB completion hander: a CAN
netdevice instead of the network interface private context
`kvaser_usb_net_priv'.

This was discovered by having some garbage in the kernel
log in place of the netdevice names: can0 and can1.

Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
 drivers/net/can/usb/kvaser_usb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 978a25e..f0c6207 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -587,7 +587,7 @@ static int kvaser_usb_simple_msg_async(struct kvaser_usb_net_priv *priv,
 			  usb_sndbulkpipe(dev->udev,
 					  dev->bulk_out->bEndpointAddress),
 			  buf, msg->len,
-			  kvaser_usb_simple_msg_callback, priv);
+			  kvaser_usb_simple_msg_callback, netdev);
 	usb_anchor_urb(urb, &priv->tx_submitted);
 
 	err = usb_submit_urb(urb, GFP_ATOMIC);
-- 
1.7.7.6


^ permalink raw reply related

* [PATCH v6 3/7] can: kvaser_usb: Retry the first bulk transfer on -ETIMEDOUT
From: Ahmed S. Darwish @ 2015-01-26  5:24 UTC (permalink / raw)
  To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
	Marc Kleine-Budde, Andri Yngvason
  Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150126052254.GC3014@Darwish.PC>

From: Ahmed S. Darwish <ahmed.darwish@valeo.com>

On some x86 laptops, plugging a Kvaser device again after an
unplug makes the firmware always ignore the very first command.
For such a case, provide some room for retries instead of
completely exiting the driver init code.

Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
 drivers/net/can/usb/kvaser_usb.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index f0c6207..55407b9 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1585,7 +1585,7 @@ static int kvaser_usb_probe(struct usb_interface *intf,
 {
 	struct kvaser_usb *dev;
 	int err = -ENOMEM;
-	int i;
+	int i, retry = 3;
 
 	dev = devm_kzalloc(&intf->dev, sizeof(*dev), GFP_KERNEL);
 	if (!dev)
@@ -1603,7 +1603,15 @@ static int kvaser_usb_probe(struct usb_interface *intf,
 
 	usb_set_intfdata(intf, dev);
 
-	err = kvaser_usb_get_software_info(dev);
+	/* On some x86 laptops, plugging a Kvaser device again after
+	 * an unplug makes the firmware always ignore the very first
+	 * command. For such a case, provide some room for retries
+	 * instead of completely exiting the driver.
+	 */
+	do {
+		err = kvaser_usb_get_software_info(dev);
+	} while (--retry && err == -ETIMEDOUT);
+
 	if (err) {
 		dev_err(&intf->dev,
 			"Cannot get software infos, error %d\n", err);
-- 
1.7.7.6

^ permalink raw reply related

* [PATCH v6 4/7] can: kvaser_usb: Fix state handling upon BUS_ERROR events
From: Ahmed S. Darwish @ 2015-01-26  5:25 UTC (permalink / raw)
  To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
	Marc Kleine-Budde, Andri Yngvason
  Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150126052406.GD3014@Darwish.PC>

From: Ahmed S. Darwish <ahmed.darwish@valeo.com>

While being in an ERROR_WARNING state, and receiving further
bus error events with error counters still in the ERROR_WARNING
range of 97-127 inclusive, the state handling code erroneously
reverts back to ERROR_ACTIVE.

Per the CAN standard, only revert to ERROR_ACTIVE when the
error counters are less than 96.

Moreover, in certain Kvaser models, the BUS_ERROR flag is
always set along with undefined bits in the M16C status
register. Thus use bitwise operators instead of full equality
for checking that register against bus errors.

Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
 drivers/net/can/usb/kvaser_usb.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 55407b9..7af379c 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -698,9 +698,7 @@ 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 (status & M16C_STATE_BUS_ERROR) {
 		if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
 		    ((txerr >= 96) || (rxerr >= 96))) {
 			cf->can_id |= CAN_ERR_CRTL;
@@ -710,7 +708,8 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
 
 			priv->can.can_stats.error_warning++;
 			new_state = CAN_STATE_ERROR_WARNING;
-		} else if (priv->can.state > CAN_STATE_ERROR_ACTIVE) {
+		} else if ((priv->can.state > CAN_STATE_ERROR_ACTIVE) &&
+			   ((txerr < 96) && (rxerr < 96))) {
 			cf->can_id |= CAN_ERR_PROT;
 			cf->data[2] = CAN_ERR_PROT_ACTIVE;
 
-- 
1.7.7.6

^ permalink raw reply related

* [PATCH v6 5/7] can: kvaser_usb: Update interface state before exiting on OOM
From: Ahmed S. Darwish @ 2015-01-26  5:27 UTC (permalink / raw)
  To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
	Marc Kleine-Budde, Andri Yngvason
  Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150126052543.GE3014@Darwish.PC>

From: Ahmed S. Darwish <ahmed.darwish@valeo.com>

Update all of the can interface's state and error counters before
trying any skb allocation that can actually fail with -ENOMEM.

Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
 drivers/net/can/usb/kvaser_usb.c |  181 ++++++++++++++++++++++----------------
 1 files changed, 105 insertions(+), 76 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 7af379c..f57ce55 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -273,6 +273,10 @@ struct kvaser_msg {
 	} u;
 } __packed;
 
+struct kvaser_usb_error_summary {
+	u8 channel, status, txerr, rxerr, error_factor;
+};
+
 struct kvaser_usb_tx_urb_context {
 	struct kvaser_usb_net_priv *priv;
 	u32 echo_index;
@@ -615,6 +619,54 @@ 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_update_can_state(struct kvaser_usb_net_priv *priv,
+						 const struct kvaser_usb_error_summary *es)
+{
+	struct net_device_stats *stats;
+	enum can_state new_state;
+
+	stats = &priv->netdev->stats;
+	new_state = priv->can.state;
+
+	netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
+
+	if (es->status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
+		priv->can.can_stats.bus_off++;
+		new_state = CAN_STATE_BUS_OFF;
+	} else if (es->status & M16C_STATE_BUS_PASSIVE) {
+		if (priv->can.state != CAN_STATE_ERROR_PASSIVE)
+			priv->can.can_stats.error_passive++;
+		new_state = CAN_STATE_ERROR_PASSIVE;
+	} else if (es->status & M16C_STATE_BUS_ERROR) {
+		if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
+		    ((es->txerr >= 96) || (es->rxerr >= 96))) {
+			priv->can.can_stats.error_warning++;
+			new_state = CAN_STATE_ERROR_WARNING;
+		} else if ((priv->can.state > CAN_STATE_ERROR_ACTIVE) &&
+			   ((es->txerr < 96) && (es->rxerr < 96))) {
+			new_state = CAN_STATE_ERROR_ACTIVE;
+		}
+	}
+
+	if (!es->status)
+		new_state = CAN_STATE_ERROR_ACTIVE;
+
+	if (priv->can.restart_ms &&
+	    (priv->can.state >= CAN_STATE_BUS_OFF) &&
+	    (new_state < CAN_STATE_BUS_OFF)) {
+		priv->can.can_stats.restarts++;
+	}
+
+	if (es->error_factor) {
+		priv->can.can_stats.bus_error++;
+		stats->rx_errors++;
+	}
+
+	priv->bec.txerr = es->txerr;
+	priv->bec.rxerr = es->rxerr;
+	priv->can.state = new_state;
+}
+
 static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
 				const struct kvaser_msg *msg)
 {
@@ -622,30 +674,30 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
 	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;
+	struct kvaser_usb_error_summary es = { };
+	enum can_state old_state;
 
 	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.error_event.channel;
+		es.status =  msg->u.error_event.status;
+		es.txerr = msg->u.error_event.tx_errors_count;
+		es.rxerr = msg->u.error_event.rx_errors_count;
+		es.error_factor = msg->u.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];
+		es.channel = msg->u.log_message.channel;
+		es.status = msg->u.log_message.data[0];
+		es.txerr = msg->u.log_message.data[2];
+		es.rxerr = msg->u.log_message.data[3];
+		es.error_factor = msg->u.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.chip_state_event.channel;
+		es.status =  msg->u.chip_state_event.status;
+		es.txerr = msg->u.chip_state_event.tx_errors_count;
+		es.rxerr = msg->u.chip_state_event.rx_errors_count;
+		es.error_factor = 0;
 		break;
 	default:
 		dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
@@ -653,116 +705,93 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
 		return;
 	}
 
-	if (channel >= dev->nchannels) {
+	if (es.channel >= dev->nchannels) {
 		dev_err(dev->udev->dev.parent,
-			"Invalid channel number (%d)\n", channel);
+			"Invalid channel number (%d)\n", es.channel);
 		return;
 	}
 
-	priv = dev->nets[channel];
+	priv = dev->nets[es.channel];
 	stats = &priv->netdev->stats;
 
+	/* Update all of the can interface's state and error counters before
+	 * trying any skb allocation that can actually fail with -ENOMEM.
+	 */
+	old_state = priv->can.state;
+	kvaser_usb_rx_error_update_can_state(priv, &es);
+
 	skb = alloc_can_err_skb(priv->netdev, &cf);
 	if (!skb) {
 		stats->rx_dropped++;
 		return;
 	}
 
-	new_state = priv->can.state;
-
-	netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status);
-
-	if (status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
+	if (es.status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
 		cf->can_id |= CAN_ERR_BUSOFF;
 
-		priv->can.can_stats.bus_off++;
 		if (!priv->can.restart_ms)
 			kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
-
 		netif_carrier_off(priv->netdev);
-
-		new_state = CAN_STATE_BUS_OFF;
-	} else if (status & M16C_STATE_BUS_PASSIVE) {
-		if (priv->can.state != CAN_STATE_ERROR_PASSIVE) {
+	} else if (es.status & M16C_STATE_BUS_PASSIVE) {
+		if (old_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
 				cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
 					      CAN_ERR_CRTL_RX_PASSIVE;
-
-			priv->can.can_stats.error_passive++;
 		}
-
-		new_state = CAN_STATE_ERROR_PASSIVE;
-	} else if (status & M16C_STATE_BUS_ERROR) {
-		if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
-		    ((txerr >= 96) || (rxerr >= 96))) {
+	} else if (es.status & M16C_STATE_BUS_ERROR) {
+		if ((old_state < CAN_STATE_ERROR_WARNING) &&
+		    ((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;
-
-			priv->can.can_stats.error_warning++;
-			new_state = CAN_STATE_ERROR_WARNING;
-		} else if ((priv->can.state > CAN_STATE_ERROR_ACTIVE) &&
-			   ((txerr < 96) && (rxerr < 96))) {
+		} else if ((old_state > CAN_STATE_ERROR_ACTIVE) &&
+			   ((es.txerr < 96) && (es.rxerr < 96))) {
 			cf->can_id |= CAN_ERR_PROT;
 			cf->data[2] = CAN_ERR_PROT_ACTIVE;
-
-			new_state = CAN_STATE_ERROR_ACTIVE;
 		}
 	}
 
-	if (!status) {
+	if (!es.status) {
 		cf->can_id |= CAN_ERR_PROT;
 		cf->data[2] = CAN_ERR_PROT_ACTIVE;
-
-		new_state = CAN_STATE_ERROR_ACTIVE;
 	}
 
 	if (priv->can.restart_ms &&
-	    (priv->can.state >= CAN_STATE_BUS_OFF) &&
-	    (new_state < CAN_STATE_BUS_OFF)) {
+	    (old_state >= CAN_STATE_BUS_OFF) &&
+	    (priv->can.state < CAN_STATE_BUS_OFF)) {
 		cf->can_id |= CAN_ERR_RESTARTED;
 		netif_carrier_on(priv->netdev);
-
-		priv->can.can_stats.restarts++;
 	}
 
-	if (error_factor) {
-		priv->can.can_stats.bus_error++;
-		stats->rx_errors++;
-
+	if (es.error_factor) {
 		cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
 
-		if (error_factor & M16C_EF_ACKE)
+		if (es.error_factor & M16C_EF_ACKE)
 			cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
-		if (error_factor & M16C_EF_CRCE)
+		if (es.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)
+		if (es.error_factor & M16C_EF_FORME)
 			cf->data[2] |= CAN_ERR_PROT_FORM;
-		if (error_factor & M16C_EF_STFE)
+		if (es.error_factor & M16C_EF_STFE)
 			cf->data[2] |= CAN_ERR_PROT_STUFF;
-		if (error_factor & M16C_EF_BITE0)
+		if (es.error_factor & M16C_EF_BITE0)
 			cf->data[2] |= CAN_ERR_PROT_BIT0;
-		if (error_factor & M16C_EF_BITE1)
+		if (es.error_factor & M16C_EF_BITE1)
 			cf->data[2] |= CAN_ERR_PROT_BIT1;
-		if (error_factor & M16C_EF_TRE)
+		if (es.error_factor & M16C_EF_TRE)
 			cf->data[2] |= CAN_ERR_PROT_TX;
 	}
 
-	cf->data[6] = txerr;
-	cf->data[7] = rxerr;
-
-	priv->bec.txerr = txerr;
-	priv->bec.rxerr = rxerr;
-
-	priv->can.state = new_state;
+	cf->data[6] = es.txerr;
+	cf->data[7] = es.rxerr;
 
 	stats->rx_packets++;
 	stats->rx_bytes += cf->can_dlc;
@@ -786,6 +815,9 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
 	}
 
 	if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
+		stats->rx_over_errors++;
+		stats->rx_errors++;
+
 		skb = alloc_can_err_skb(priv->netdev, &cf);
 		if (!skb) {
 			stats->rx_dropped++;
@@ -795,9 +827,6 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
 		cf->can_id |= CAN_ERR_CRTL;
 		cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
 
-		stats->rx_over_errors++;
-		stats->rx_errors++;
-
 		stats->rx_packets++;
 		stats->rx_bytes += cf->can_dlc;
 		netif_rx(skb);
-- 
1.7.7.6

^ permalink raw reply related

* [PATCH v6 6/7] can: kvaser_usb: Consolidate and unify state change handling
From: Ahmed S. Darwish @ 2015-01-26  5:29 UTC (permalink / raw)
  To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
	Marc Kleine-Budde, Andri Yngvason
  Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150126052719.GF3014@Darwish.PC>

From: Ahmed S. Darwish <ahmed.darwish@valeo.com>

Replace most of the can interface's state and error counters
handling with the new can-dev can_change_state() mechanism.

Suggested-by: Andri Yngvason <andri.yngvason@marel.com>
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
 drivers/net/can/usb/kvaser_usb.c |  113 ++++++++++++++++---------------------
 1 files changed, 49 insertions(+), 64 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index f57ce55..ddc2954 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -620,39 +620,44 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
 }
 
 static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
-						 const struct kvaser_usb_error_summary *es)
+						 const struct kvaser_usb_error_summary *es,
+						 struct can_frame *cf)
 {
 	struct net_device_stats *stats;
-	enum can_state new_state;
-
-	stats = &priv->netdev->stats;
-	new_state = priv->can.state;
+	enum can_state cur_state, new_state, tx_state, rx_state;
 
 	netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
 
-	if (es->status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
-		priv->can.can_stats.bus_off++;
+	stats = &priv->netdev->stats;
+	new_state = cur_state = priv->can.state;
+
+	if (es->status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET))
 		new_state = CAN_STATE_BUS_OFF;
-	} else if (es->status & M16C_STATE_BUS_PASSIVE) {
-		if (priv->can.state != CAN_STATE_ERROR_PASSIVE)
-			priv->can.can_stats.error_passive++;
+	else if (es->status & M16C_STATE_BUS_PASSIVE)
 		new_state = CAN_STATE_ERROR_PASSIVE;
-	} else if (es->status & M16C_STATE_BUS_ERROR) {
-		if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
-		    ((es->txerr >= 96) || (es->rxerr >= 96))) {
-			priv->can.can_stats.error_warning++;
+	else if (es->status & M16C_STATE_BUS_ERROR) {
+		if ((es->txerr >= 256) || (es->rxerr >= 256))
+			new_state = CAN_STATE_BUS_OFF;
+		else if ((es->txerr >= 128) || (es->rxerr >= 128))
+			new_state = CAN_STATE_ERROR_PASSIVE;
+		else if ((es->txerr >= 96) || (es->rxerr >= 96))
 			new_state = CAN_STATE_ERROR_WARNING;
-		} else if ((priv->can.state > CAN_STATE_ERROR_ACTIVE) &&
-			   ((es->txerr < 96) && (es->rxerr < 96))) {
+		else if (cur_state > CAN_STATE_ERROR_ACTIVE)
 			new_state = CAN_STATE_ERROR_ACTIVE;
-		}
 	}
 
 	if (!es->status)
 		new_state = CAN_STATE_ERROR_ACTIVE;
 
+	if (new_state != cur_state) {
+		tx_state = (es->txerr >= es->rxerr) ? new_state : 0;
+		rx_state = (es->txerr <= es->rxerr) ? new_state : 0;
+
+		can_change_state(priv->netdev, cf, tx_state, rx_state);
+	}
+
 	if (priv->can.restart_ms &&
-	    (priv->can.state >= CAN_STATE_BUS_OFF) &&
+	    (cur_state >= CAN_STATE_BUS_OFF) &&
 	    (new_state < CAN_STATE_BUS_OFF)) {
 		priv->can.can_stats.restarts++;
 	}
@@ -664,18 +669,17 @@ static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *pri
 
 	priv->bec.txerr = es->txerr;
 	priv->bec.rxerr = es->rxerr;
-	priv->can.state = new_state;
 }
 
 static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
 				const struct kvaser_msg *msg)
 {
-	struct can_frame *cf;
+	struct can_frame *cf, tmp_cf = { .can_id = CAN_ERR_FLAG, .can_dlc = CAN_ERR_DLC };
 	struct sk_buff *skb;
 	struct net_device_stats *stats;
 	struct kvaser_usb_net_priv *priv;
 	struct kvaser_usb_error_summary es = { };
-	enum can_state old_state;
+	enum can_state old_state, new_state;
 
 	switch (msg->id) {
 	case CMD_CAN_ERROR_EVENT:
@@ -715,59 +719,40 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
 	stats = &priv->netdev->stats;
 
 	/* Update all of the can interface's state and error counters before
-	 * trying any skb allocation that can actually fail with -ENOMEM.
+	 * trying any memory allocation that can actually fail with -ENOMEM.
+	 *
+	 * We send a temporary stack-allocated error can frame to
+	 * can_change_state() for the very same reason.
+	 *
+	 * TODO: Split can_change_state() responsibility between updating the
+	 * can interface's state and counters, and the setting up of can error
+	 * frame ID and data to userspace. Remove stack allocation afterwards.
 	 */
 	old_state = priv->can.state;
-	kvaser_usb_rx_error_update_can_state(priv, &es);
+	kvaser_usb_rx_error_update_can_state(priv, &es, &tmp_cf);
+	new_state = priv->can.state;
 
 	skb = alloc_can_err_skb(priv->netdev, &cf);
 	if (!skb) {
 		stats->rx_dropped++;
 		return;
 	}
-
-	if (es.status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
-		cf->can_id |= CAN_ERR_BUSOFF;
-
-		if (!priv->can.restart_ms)
-			kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
-		netif_carrier_off(priv->netdev);
-	} else if (es.status & M16C_STATE_BUS_PASSIVE) {
-		if (old_state != CAN_STATE_ERROR_PASSIVE) {
-			cf->can_id |= CAN_ERR_CRTL;
-
-			if (es.txerr || es.rxerr)
-				cf->data[1] = (es.txerr > es.rxerr)
-						? CAN_ERR_CRTL_TX_PASSIVE
-						: CAN_ERR_CRTL_RX_PASSIVE;
-			else
-				cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
-					      CAN_ERR_CRTL_RX_PASSIVE;
-		}
-	} else if (es.status & M16C_STATE_BUS_ERROR) {
-		if ((old_state < CAN_STATE_ERROR_WARNING) &&
-		    ((es.txerr >= 96) || (es.rxerr >= 96))) {
-			cf->can_id |= CAN_ERR_CRTL;
-			cf->data[1] = (es.txerr > es.rxerr)
-					? CAN_ERR_CRTL_TX_WARNING
-					: CAN_ERR_CRTL_RX_WARNING;
-		} else if ((old_state > CAN_STATE_ERROR_ACTIVE) &&
-			   ((es.txerr < 96) && (es.rxerr < 96))) {
-			cf->can_id |= CAN_ERR_PROT;
-			cf->data[2] = CAN_ERR_PROT_ACTIVE;
+	memcpy(cf, &tmp_cf, sizeof(*cf));
+
+	if (new_state != old_state) {
+		if (es.status &
+		    (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
+			if (!priv->can.restart_ms)
+				kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
+			netif_carrier_off(priv->netdev);
 		}
-	}
-
-	if (!es.status) {
-		cf->can_id |= CAN_ERR_PROT;
-		cf->data[2] = CAN_ERR_PROT_ACTIVE;
-	}
 
-	if (priv->can.restart_ms &&
-	    (old_state >= CAN_STATE_BUS_OFF) &&
-	    (priv->can.state < CAN_STATE_BUS_OFF)) {
-		cf->can_id |= CAN_ERR_RESTARTED;
-		netif_carrier_on(priv->netdev);
+		if (priv->can.restart_ms &&
+		    (old_state >= CAN_STATE_BUS_OFF) &&
+		    (new_state < CAN_STATE_BUS_OFF)) {
+			cf->can_id |= CAN_ERR_RESTARTED;
+			netif_carrier_on(priv->netdev);
+		}
 	}
 
 	if (es.error_factor) {
-- 
1.7.7.6

^ permalink raw reply related

* [PATCH v6 7/7] can: kvaser_usb: Add support for the USBcan-II family
From: Ahmed S. Darwish @ 2015-01-26  5:33 UTC (permalink / raw)
  To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
	Marc Kleine-Budde, Andri Yngvason
  Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150126052915.GG3014@Darwish.PC>

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 |  590 ++++++++++++++++++++++++++++++--------
 2 files changed, 474 insertions(+), 124 deletions(-)

** V6 Changelog:
- Revert to the error-active state if the error counters were
  decreased by hardware
- Rebase over a new set of upstream Leaf-driver bugfixes

** V5 Changelog:
- Rebase on the new CAN error state changes added for the Leaf driver
- Add minor changes (remove unused commands, constify poniters, etc.)

** V4 Changelog:
- Use type-safe C methods instead of cpp macros
- Remove defensive checks against non-existing families
- Re-order methods to remove forward declarations
- Smaller stuff spotted by earlier review (function prefexes, etc.)

** 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.)

** 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

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 ddc2954..17d28d7 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 A.S.
  */
 
 #include <linux/completion.h>
@@ -30,8 +32,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 */
+/* Kvaser Leaf USB devices */
 #define KVASER_VENDOR_ID		0x0bfd
 #define USB_LEAF_DEVEL_PRODUCT_ID	10
 #define USB_LEAF_LITE_PRODUCT_ID	11
@@ -56,6 +59,24 @@
 #define USB_LEAF_LITE_V2_PRODUCT_ID	288
 #define USB_MINI_PCIE_HS_PRODUCT_ID	289
 
+static inline bool kvaser_is_leaf(const struct usb_device_id *id)
+{
+	return id->idProduct >= USB_LEAF_DEVEL_PRODUCT_ID &&
+	       id->idProduct <= USB_MINI_PCIE_HS_PRODUCT_ID;
+}
+
+/* Kvaser USBCan-II 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
+
+static inline bool kvaser_is_usbcan(const struct usb_device_id *id)
+{
+	return id->idProduct >= USB_USBCAN_REVB_PRODUCT_ID &&
+	       id->idProduct <= USB_MEMORATOR_PRODUCT_ID;
+}
+
 /* USB devices features */
 #define KVASER_HAS_SILENT_MODE		BIT(0)
 #define KVASER_HAS_TXRX_ERRORS		BIT(1)
@@ -73,7 +94,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 +119,11 @@
 #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_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 +133,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 +147,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,9 +171,18 @@
 #define KVASER_CTRL_MODE_SELFRECEPTION	3
 #define KVASER_CTRL_MODE_OFF		4
 
-/* log message */
+/* Extended CAN identifier flag */
 #define KVASER_EXTENDED_FRAME		BIT(31)
 
+/* 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,
+};
+
 struct kvaser_msg_simple {
 	u8 tid;
 	u8 channel;
@@ -148,30 +191,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 +256,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_header {
+	u8 channel;
+	u8 flag;
 } __packed;
 
-struct kvaser_msg_rx_can {
+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;
+} __packed;
+
+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 +347,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 +373,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,21 +390,55 @@ 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;
 
+/* Summary of a kvaser error event, for a unified Leaf/Usbcan error
+ * handling. Some discrepancies between the two families exist:
+ *
+ * - USBCAN firmware does not report M16C "error factors"
+ * - USBCAN controllers has difficulties reporting if the raised error
+ *   event is for ch0 or ch1. They leave such arbitration to the OS
+ *   driver by letting it compare error counters with previous values
+ *   and decide the error event's channel. Thus for USBCAN, the channel
+ *   field is only advisory.
+ */
 struct kvaser_usb_error_summary {
-	u8 channel, status, txerr, rxerr, error_factor;
+	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 {
@@ -292,6 +456,7 @@ struct kvaser_usb {
 
 	u32 fw_version;
 	unsigned int nchannels;
+	enum kvaser_usb_family family;
 
 	bool rxinitdone;
 	void *rxbuf[MAX_RX_URBS];
@@ -315,6 +480,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),
@@ -364,6 +530,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);
@@ -467,7 +644,14 @@ 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;
+	}
 
 	return 0;
 }
@@ -486,7 +670,9 @@ static int kvaser_usb_get_card_info(struct kvaser_usb *dev)
 		return err;
 
 	dev->nchannels = msg.u.cardinfo.nchannels;
-	if (dev->nchannels > MAX_NET_DEVICES)
+	if ((dev->nchannels > MAX_NET_DEVICES) ||
+	    (dev->family == KVASER_USBCAN &&
+	     dev->nchannels > MAX_USBCAN_NET_DEVICES))
 		return -EINVAL;
 
 	return 0;
@@ -500,8 +686,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,
@@ -623,12 +811,12 @@ static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *pri
 						 const struct kvaser_usb_error_summary *es,
 						 struct can_frame *cf)
 {
-	struct net_device_stats *stats;
+	struct kvaser_usb *dev = priv->dev;
+	struct net_device_stats *stats = &priv->netdev->stats;
 	enum can_state cur_state, new_state, tx_state, rx_state;
 
 	netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
 
-	stats = &priv->netdev->stats;
 	new_state = cur_state = priv->can.state;
 
 	if (es->status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET))
@@ -662,9 +850,22 @@ static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *pri
 		priv->can.can_stats.restarts++;
 	}
 
-	if (es->error_factor) {
-		priv->can.can_stats.bus_error++;
-		stats->rx_errors++;
+	switch (dev->family) {
+	case KVASER_LEAF:
+		if (es->leaf.error_factor) {
+			priv->can.can_stats.bus_error++;
+			stats->rx_errors++;
+		}
+		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++;
+		}
+		break;
 	}
 
 	priv->bec.txerr = es->txerr;
@@ -672,50 +873,21 @@ static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *pri
 }
 
 static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
-				const struct kvaser_msg *msg)
+				const struct kvaser_usb_error_summary *es)
 {
 	struct can_frame *cf, tmp_cf = { .can_id = CAN_ERR_FLAG, .can_dlc = CAN_ERR_DLC };
 	struct sk_buff *skb;
 	struct net_device_stats *stats;
 	struct kvaser_usb_net_priv *priv;
-	struct kvaser_usb_error_summary es = { };
 	enum can_state old_state, new_state;
 
-	switch (msg->id) {
-	case CMD_CAN_ERROR_EVENT:
-		es.channel = msg->u.error_event.channel;
-		es.status =  msg->u.error_event.status;
-		es.txerr = msg->u.error_event.tx_errors_count;
-		es.rxerr = msg->u.error_event.rx_errors_count;
-		es.error_factor = msg->u.error_event.error_factor;
-		break;
-	case CMD_LOG_MESSAGE:
-		es.channel = msg->u.log_message.channel;
-		es.status = msg->u.log_message.data[0];
-		es.txerr = msg->u.log_message.data[2];
-		es.rxerr = msg->u.log_message.data[3];
-		es.error_factor = msg->u.log_message.data[1];
-		break;
-	case CMD_CHIP_STATE_EVENT:
-		es.channel = msg->u.chip_state_event.channel;
-		es.status =  msg->u.chip_state_event.status;
-		es.txerr = msg->u.chip_state_event.tx_errors_count;
-		es.rxerr = msg->u.chip_state_event.rx_errors_count;
-		es.error_factor = 0;
-		break;
-	default:
-		dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
-			msg->id);
-		return;
-	}
-
-	if (es.channel >= dev->nchannels) {
+	if (es->channel >= dev->nchannels) {
 		dev_err(dev->udev->dev.parent,
-			"Invalid channel number (%d)\n", es.channel);
+			"Invalid channel number (%d)\n", es->channel);
 		return;
 	}
 
-	priv = dev->nets[es.channel];
+	priv = dev->nets[es->channel];
 	stats = &priv->netdev->stats;
 
 	/* Update all of the can interface's state and error counters before
@@ -729,7 +901,7 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
 	 * frame ID and data to userspace. Remove stack allocation afterwards.
 	 */
 	old_state = priv->can.state;
-	kvaser_usb_rx_error_update_can_state(priv, &es, &tmp_cf);
+	kvaser_usb_rx_error_update_can_state(priv, es, &tmp_cf);
 	new_state = priv->can.state;
 
 	skb = alloc_can_err_skb(priv->netdev, &cf);
@@ -740,7 +912,7 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
 	memcpy(cf, &tmp_cf, sizeof(*cf));
 
 	if (new_state != old_state) {
-		if (es.status &
+		if (es->status &
 		    (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
 			if (!priv->can.restart_ms)
 				kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
@@ -755,34 +927,161 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
 		}
 	}
 
-	if (es.error_factor) {
-		cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
-
-		if (es.error_factor & M16C_EF_ACKE)
-			cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
-		if (es.error_factor & M16C_EF_CRCE)
-			cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
-					CAN_ERR_PROT_LOC_CRC_DEL);
-		if (es.error_factor & M16C_EF_FORME)
-			cf->data[2] |= CAN_ERR_PROT_FORM;
-		if (es.error_factor & M16C_EF_STFE)
-			cf->data[2] |= CAN_ERR_PROT_STUFF;
-		if (es.error_factor & M16C_EF_BITE0)
-			cf->data[2] |= CAN_ERR_PROT_BIT0;
-		if (es.error_factor & M16C_EF_BITE1)
-			cf->data[2] |= CAN_ERR_PROT_BIT1;
-		if (es.error_factor & M16C_EF_TRE)
-			cf->data[2] |= CAN_ERR_PROT_TX;
+	switch (dev->family) {
+	case KVASER_LEAF:
+		if (es->leaf.error_factor) {
+			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_BUSERROR) {
+			cf->can_id |= CAN_ERR_BUSERROR;
+		}
+		break;
 	}
 
-	cf->data[6] = es.txerr;
-	cf->data[7] = es.rxerr;
+	cf->data[6] = es->txerr;
+	cf->data[7] = es->rxerr;
 
 	stats->rx_packets++;
 	stats->rx_bytes += cf->can_dlc;
 	netif_rx(skb);
 }
 
+/* For USBCAN, report error to userspace iff the channels's errors counter
+ * has changed, or we're the only channel seeing a bus error state.
+ */
+static void kvaser_usbcan_conditionally_rx_error(const struct kvaser_usb *dev,
+						 struct kvaser_usb_error_summary *es)
+{
+	struct kvaser_usb_net_priv *priv;
+	int channel;
+	bool 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];
+	report_error = false;
+
+	if (es->txerr != priv->bec.txerr) {
+		es->usbcan.error_state |= USBCAN_ERROR_STATE_TX_ERROR;
+		report_error = true;
+	}
+	if (es->rxerr != priv->bec.rxerr) {
+		es->usbcan.error_state |= USBCAN_ERROR_STATE_RX_ERROR;
+		report_error = true;
+	}
+	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 = true;
+	}
+
+	if (report_error)
+		kvaser_usb_rx_error(dev, es);
+}
+
+static void kvaser_usbcan_rx_error(const struct kvaser_usb *dev,
+				   const struct kvaser_msg *msg)
+{
+	struct kvaser_usb_error_summary es = { };
+
+	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;
+		kvaser_usbcan_conditionally_rx_error(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;
+		kvaser_usbcan_conditionally_rx_error(dev, &es);
+
+		/* The USBCAN firmware supports up to 2 channels.
+		 * Now that ch0 was checked, check if ch1 has any errors.
+		 */
+		if (dev->nchannels == MAX_USBCAN_NET_DEVICES) {
+			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;
+			kvaser_usbcan_conditionally_rx_error(dev, &es);
+		}
+		break;
+
+	default:
+		dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
+			msg->id);
+	}
+}
+
+static void kvaser_leaf_rx_error(const struct kvaser_usb *dev,
+				 const struct kvaser_msg *msg)
+{
+	struct kvaser_usb_error_summary es = { };
+
+	switch (msg->id) {
+	case CMD_CAN_ERROR_EVENT:
+		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_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:
+		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",
+			msg->id);
+		return;
+	}
+
+	kvaser_usb_rx_error(dev, &es);
+}
+
 static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
 				  const struct kvaser_msg *msg)
 {
@@ -790,16 +1089,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) {
 		stats->rx_over_errors++;
 		stats->rx_errors++;
 
@@ -825,7 +1124,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 = NULL;	/* GCC */
 
 	if (channel >= dev->nchannels) {
 		dev_err(dev->udev->dev.parent,
@@ -836,60 +1136,68 @@ 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)) {
-		kvaser_usb_rx_error(dev, msg);
+	if ((msg->u.rx_can_header.flag & MSG_FLAG_ERROR_FRAME) &&
+	    (dev->family == KVASER_LEAF && msg->id == CMD_LEAF_LOG_MESSAGE)) {
+		kvaser_leaf_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;
+	}
+
 	skb = alloc_can_skb(priv->netdev, &cf);
 	if (!skb) {
 		stats->tx_dropped++;
 		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);
 	}
 
@@ -952,21 +1260,35 @@ 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;
 
 	case CMD_CHIP_STATE_EVENT:
 	case CMD_CAN_ERROR_EVENT:
-		kvaser_usb_rx_error(dev, msg);
+		if (dev->family == KVASER_LEAF)
+			kvaser_leaf_rx_error(dev, msg);
+		else
+			kvaser_usbcan_rx_error(dev, msg);
 		break;
 
 	case CMD_TX_ACKNOWLEDGE:
 		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;
 	}
@@ -1186,7 +1508,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)
@@ -1294,6 +1616,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;
+	u8 *msg_tx_can_flags = NULL;		/* GCC */
 
 	if (can_dropped_invalid_skb(netdev, skb))
 		return NETDEV_TX_OK;
@@ -1315,9 +1638,19 @@ 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;
+	}
+
+	*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;
@@ -1335,7 +1668,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) {
@@ -1604,6 +1937,17 @@ static int kvaser_usb_probe(struct usb_interface *intf,
 	if (!dev)
 		return -ENOMEM;
 
+	if (kvaser_is_leaf(id)) {
+		dev->family = KVASER_LEAF;
+	} else if (kvaser_is_usbcan(id)) {
+		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)");
-- 
1.7.7.6

^ permalink raw reply related

* Re: [PATCH net-next v3 2/2] iproute2: bridge vlan show new option to print ranges
From: Scott Feldman @ 2015-01-26  5:57 UTC (permalink / raw)
  To: roopa; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson Kok
In-Reply-To: <54C5CE0A.2050209@cumulusnetworks.com>

On Sun, Jan 25, 2015 at 9:18 PM, roopa <roopa@cumulusnetworks.com> wrote:
> On 1/25/15, 8:35 PM, Scott Feldman wrote:
>>>
>>> diff --git a/bridge/bridge.c b/bridge/bridge.c
>>> index 5fcc552..88469ca 100644
>>> --- a/bridge/bridge.c
>>> +++ b/bridge/bridge.c
>>> @@ -21,6 +21,7 @@ int resolve_hosts;
>>>   int oneline = 0;
>>>   int show_stats;
>>>   int show_details;
>>> +int compress_vlans;
>>>   int timestamp;
>>>   char * _SL_ = NULL;
>>>
>>> @@ -32,7 +33,8 @@ static void usage(void)
>>>   "Usage: bridge [ OPTIONS ] OBJECT { COMMAND | help }\n"
>>>   "where  OBJECT := { link | fdb | mdb | vlan | monitor }\n"
>>>   "       OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] |\n"
>>> -"                    -o[neline] | -t[imestamp] | -n[etns] name }\n");
>>> +"                    -o[neline] | -t[imestamp] | -n[etns] name |\n"
>>> +"                    -c[ompressvlans] }\n");
>>
>> This option is awkward at this level since it's specific to bridge vlan
>> cmd.
>>
>> Is it even necessary?  The real user of compressed vlans isn't the
>> "bridge show" cmd; just keep "bridge show" simple, listing each vlan
>> per line.
>
> without ranges it does get a bit difficult (or rather long) to read if there
> are many vlans (it was also a comment I received on v2).
>
> All the available options whether they apply to the immediate sub-command or
> not are at the toplevel command today.
> Hence the position (Not a big fan of the current name. suggestions welcome).
>
> I would have loved to have the default output print ranges, If that is
> acceptable. But, that can break new iproute2 on older kernels (If that is
> something we should care about).
>
> other option is keyword argument like maybe 'bridge vlan show compressed' or
> 'bridge vlan show ranges'

How about try RTEXT_FILTER_BRVLAN_COMPRESSED first, and if that fails,
try RTEXT_FILTER_BRVLAN, and don't add -c option?

-scott

^ permalink raw reply

* Re: [PATCH net-next v3 2/2] iproute2: bridge vlan show new option to print ranges
From: roopa @ 2015-01-26  6:09 UTC (permalink / raw)
  To: Scott Feldman; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson Kok
In-Reply-To: <CAE4R7bAOwQnoC4sbNzW=Tadrp239Lq1f4LwhLR-xeiZg3JdofQ@mail.gmail.com>

On 1/25/15, 9:57 PM, Scott Feldman wrote:
> On Sun, Jan 25, 2015 at 9:18 PM, roopa <roopa@cumulusnetworks.com> wrote:
>> On 1/25/15, 8:35 PM, Scott Feldman wrote:
>>>> diff --git a/bridge/bridge.c b/bridge/bridge.c
>>>> index 5fcc552..88469ca 100644
>>>> --- a/bridge/bridge.c
>>>> +++ b/bridge/bridge.c
>>>> @@ -21,6 +21,7 @@ int resolve_hosts;
>>>>    int oneline = 0;
>>>>    int show_stats;
>>>>    int show_details;
>>>> +int compress_vlans;
>>>>    int timestamp;
>>>>    char * _SL_ = NULL;
>>>>
>>>> @@ -32,7 +33,8 @@ static void usage(void)
>>>>    "Usage: bridge [ OPTIONS ] OBJECT { COMMAND | help }\n"
>>>>    "where  OBJECT := { link | fdb | mdb | vlan | monitor }\n"
>>>>    "       OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] |\n"
>>>> -"                    -o[neline] | -t[imestamp] | -n[etns] name }\n");
>>>> +"                    -o[neline] | -t[imestamp] | -n[etns] name |\n"
>>>> +"                    -c[ompressvlans] }\n");
>>> This option is awkward at this level since it's specific to bridge vlan
>>> cmd.
>>>
>>> Is it even necessary?  The real user of compressed vlans isn't the
>>> "bridge show" cmd; just keep "bridge show" simple, listing each vlan
>>> per line.
>> without ranges it does get a bit difficult (or rather long) to read if there
>> are many vlans (it was also a comment I received on v2).
>>
>> All the available options whether they apply to the immediate sub-command or
>> not are at the toplevel command today.
>> Hence the position (Not a big fan of the current name. suggestions welcome).
>>
>> I would have loved to have the default output print ranges, If that is
>> acceptable. But, that can break new iproute2 on older kernels (If that is
>> something we should care about).
>>
>> other option is keyword argument like maybe 'bridge vlan show compressed' or
>> 'bridge vlan show ranges'
> How about try RTEXT_FILTER_BRVLAN_COMPRESSED first, and if that fails,
> try RTEXT_FILTER_BRVLAN, and don't add -c option?
>
>
That would have been nice, except that  it will not fail. It will pass 
but not return any vlans.

Better would be to send both flags. I think that will work. New kernel 
looks at RTEXT_FILTER_BRVLAN_COMPRESSED first.
v4  soon ....after some testing ...  ;)

^ permalink raw reply

* Re: [PATCH net] ipv6: Fix __ip6_route_redirect
From: David Miller @ 2015-01-26  6:10 UTC (permalink / raw)
  To: kafai; +Cc: netdev, hannes, kernel-team
In-Reply-To: <1421810162-2910767-1-git-send-email-kafai@fb.com>

From: Martin KaFai Lau <kafai@fb.com>
Date: Tue, 20 Jan 2015 19:16:02 -0800

> In my last commit (a3c00e4: ipv6: Remove BACKTRACK macro), the changes in
> __ip6_route_redirect is incorrect.  The following case is missed:
> 1. The for loop tries to find a valid gateway rt. If it fails to find
>    one, rt will be NULL.
> 2. When rt is NULL, it is set to the ip6_null_entry.
> 3. The newly added 'else if', from a3c00e4, will stop the backtrack from
>    happening.
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>

Applied and queued up for -stable, thanks!

^ permalink raw reply

* [PATCH net-next v2 0/5] bonding: various 802.3ad fixes
From: Jonathan Toppins @ 2015-01-26  6:16 UTC (permalink / raw)
  To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek; +Cc: netdev

This patch series is a forward porting of patches we (Cumulus) are shipping
in our 3.2 series kernels. These fixes attempt to make 802.3ad bonding mode
more predictable in certian state machine transtions in addition to fixing
802.3ad bond carrier determination when bonding min_links option is changed.
Specific notes are contained within each patch.

For this patch series there are no userspace facing changes, a diff between
the modinfo output showed no difference. However, there are behavioral
facing changes, primarily in the bond carrier state. Please make sure to
review carefully.

v2:
 * fixed some style issues
 * dropped a portion of patch 1 in favor of more testing on my side

Jonathan Toppins (2):
  bonding: update bond carrier state when min_links option changes
  bonding: cleanup and remove dead code

Satish Ashok (1):
  bonding: fix LACP PDU not sent on slave port sometimes

Wilson Kok (2):
  bonding: fix bond_open() don't always set slave active flag
  bonding: fix incorrect lacp mux state when agg not active

 drivers/net/bonding/bond_3ad.c     |   55 +++++++++++++++++++++++++++---------
 drivers/net/bonding/bond_main.c    |    6 ++--
 drivers/net/bonding/bond_options.c |    1 +
 include/net/bond_3ad.h             |    1 -
 include/net/bonding.h              |    1 +
 5 files changed, 47 insertions(+), 17 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* [PATCH net-next v2 1/5] bonding: update bond carrier state when min_links option changes
From: Jonathan Toppins @ 2015-01-26  6:16 UTC (permalink / raw)
  To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek; +Cc: netdev, Andy Gospodarek
In-Reply-To: <1422253021-3798-1-git-send-email-jtoppins@cumulusnetworks.com>

Cc: Andy Gospodarek <gospo@cumulusnetworks.com>
Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>
---
 drivers/net/bonding/bond_main.c    |    2 +-
 drivers/net/bonding/bond_options.c |    1 +
 include/net/bonding.h              |    1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 0dceba1..02ffedb 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -334,7 +334,7 @@ static int bond_vlan_rx_kill_vid(struct net_device *bond_dev,
  *
  * Returns zero if carrier state does not change, nonzero if it does.
  */
-static int bond_set_carrier(struct bonding *bond)
+int bond_set_carrier(struct bonding *bond)
 {
 	struct list_head *iter;
 	struct slave *slave;
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 9bd538d4..4df2894 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -1181,6 +1181,7 @@ static int bond_option_min_links_set(struct bonding *bond,
 	netdev_info(bond->dev, "Setting min links value to %llu\n",
 		    newval->value);
 	bond->params.min_links = newval->value;
+	bond_set_carrier(bond);
 
 	return 0;
 }
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 983a94b..29f53ea 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -525,6 +525,7 @@ void bond_sysfs_slave_del(struct slave *slave);
 int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev);
 int bond_release(struct net_device *bond_dev, struct net_device *slave_dev);
 u32 bond_xmit_hash(struct bonding *bond, struct sk_buff *skb);
+int bond_set_carrier(struct bonding *bond);
 void bond_select_active_slave(struct bonding *bond);
 void bond_change_active_slave(struct bonding *bond, struct slave *new_active);
 void bond_create_debugfs(void);
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next v2 2/5] bonding: fix bond_open() don't always set slave active flag
From: Jonathan Toppins @ 2015-01-26  6:16 UTC (permalink / raw)
  To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek
  Cc: netdev, Wilson Kok, Andy Gospodarek
In-Reply-To: <1422253021-3798-1-git-send-email-jtoppins@cumulusnetworks.com>

From: Wilson Kok <wkok@cumulusnetworks.com>

Mode 802.3ad, fix incorrect bond slave active state when slave is not in
active aggregator. During bond_open(), the bonding driver always sets
the slave active flag to true if the bond is not in active-backup, alb,
or tlb modes. Bonding should let the aggregator selection logic set the
active flag when in 802.3ad mode.

Cc: Andy Gospodarek <gospo@cumulusnetworks.com>
Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: Wilson Kok <wkok@cumulusnetworks.com>
Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>
---
 drivers/net/bonding/bond_main.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 02ffedb..c475d90 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3066,7 +3066,7 @@ static int bond_open(struct net_device *bond_dev)
 			    slave != rcu_access_pointer(bond->curr_active_slave)) {
 				bond_set_slave_inactive_flags(slave,
 							      BOND_SLAVE_NOTIFY_NOW);
-			} else {
+			} else if (BOND_MODE(bond) != BOND_MODE_8023AD) {
 				bond_set_slave_active_flags(slave,
 							    BOND_SLAVE_NOTIFY_NOW);
 			}
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next v2 3/5] bonding: fix incorrect lacp mux state when agg not active
From: Jonathan Toppins @ 2015-01-26  6:16 UTC (permalink / raw)
  To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek
  Cc: netdev, Wilson Kok, Andy Gospodarek
In-Reply-To: <1422253021-3798-1-git-send-email-jtoppins@cumulusnetworks.com>

From: Wilson Kok <wkok@cumulusnetworks.com>

This patch attempts to fix the following problems when an actor or
partner's aggregator is not active:
    1. a slave's lacp port state is marked as AD_STATE_SYNCHRONIZATION
       even if it is attached to an inactive aggregator. LACP advertises
       this state to the partner, making the partner think he can move
       into COLLECTING_DISTRIBUTING state even though this link will not
       pass traffic on the local side

    2. a slave goes into COLLECTING_DISTRIBUTING state without checking
       if the aggregator is actually active

    3. when in COLLECTING_DISTRIBUTING state, the partner parameters may
       change, e.g. the partner_oper_port_state.SYNCHRONIZATION. The
       local mux machine is not reacting to the change and continue to
       keep the slave and bond up

    4. When bond slave leaves an inactive aggregator and joins an active
       aggregator, the actor oper port state need to update to SYNC state.

v2:
 * fix style issues in bond_3ad.c

Cc: Andy Gospodarek <gospo@cumulusnetworks.com>
Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: Wilson Kok <wkok@cumulusnetworks.com>
Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>
---
 drivers/net/bonding/bond_3ad.c |   44 +++++++++++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 8baa87d..e3c96b2 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -467,11 +467,14 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port)
 		/* set the partner sync. to on if the partner is sync,
 		 * and the port is matched
 		 */
-		if ((port->sm_vars & AD_PORT_MATCHED)
-		    && (lacpdu->actor_state & AD_STATE_SYNCHRONIZATION))
+		if ((port->sm_vars & AD_PORT_MATCHED) &&
+		    (lacpdu->actor_state & AD_STATE_SYNCHRONIZATION)) {
 			partner->port_state |= AD_STATE_SYNCHRONIZATION;
-		else
+			pr_debug("%s partner sync=1\n", port->slave->dev->name);
+		} else {
 			partner->port_state &= ~AD_STATE_SYNCHRONIZATION;
+			pr_debug("%s partner sync=0\n", port->slave->dev->name);
+		}
 	}
 }
 
@@ -726,6 +729,8 @@ static inline void __update_lacpdu_from_port(struct port *port)
 	lacpdu->actor_port_priority = htons(port->actor_port_priority);
 	lacpdu->actor_port = htons(port->actor_port_number);
 	lacpdu->actor_state = port->actor_oper_port_state;
+	pr_debug("update lacpdu: %s, actor port state %x\n",
+		 port->slave->dev->name, port->actor_oper_port_state);
 
 	/* lacpdu->reserved_3_1              initialized
 	 * lacpdu->tlv_type_partner_info     initialized
@@ -898,7 +903,9 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
 			if ((port->sm_vars & AD_PORT_SELECTED) &&
 			    (port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION) &&
 			    !__check_agg_selection_timer(port)) {
-				port->sm_mux_state = AD_MUX_COLLECTING_DISTRIBUTING;
+				if (port->aggregator->is_active)
+					port->sm_mux_state =
+					    AD_MUX_COLLECTING_DISTRIBUTING;
 			} else if (!(port->sm_vars & AD_PORT_SELECTED) ||
 				   (port->sm_vars & AD_PORT_STANDBY)) {
 				/* if UNSELECTED or STANDBY */
@@ -910,12 +917,16 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
 				 */
 				__set_agg_ports_ready(port->aggregator, __agg_ports_are_ready(port->aggregator));
 				port->sm_mux_state = AD_MUX_DETACHED;
+			} else if (port->aggregator->is_active) {
+				port->actor_oper_port_state |=
+				    AD_STATE_SYNCHRONIZATION;
 			}
 			break;
 		case AD_MUX_COLLECTING_DISTRIBUTING:
 			if (!(port->sm_vars & AD_PORT_SELECTED) ||
 			    (port->sm_vars & AD_PORT_STANDBY) ||
-			    !(port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION)) {
+			    !(port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION) ||
+			    !(port->actor_oper_port_state & AD_STATE_SYNCHRONIZATION)) {
 				port->sm_mux_state = AD_MUX_ATTACHED;
 			} else {
 				/* if port state hasn't changed make
@@ -937,8 +948,10 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
 
 	/* check if the state machine was changed */
 	if (port->sm_mux_state != last_state) {
-		pr_debug("Mux Machine: Port=%d, Last State=%d, Curr State=%d\n",
-			 port->actor_port_number, last_state,
+		pr_debug("Mux Machine: Port=%d (%s), Last State=%d, Curr State=%d\n",
+			 port->actor_port_number,
+			 port->slave->dev->name,
+			 last_state,
 			 port->sm_mux_state);
 		switch (port->sm_mux_state) {
 		case AD_MUX_DETACHED:
@@ -953,7 +966,12 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
 			port->sm_mux_timer_counter = __ad_timer_to_ticks(AD_WAIT_WHILE_TIMER, 0);
 			break;
 		case AD_MUX_ATTACHED:
-			port->actor_oper_port_state |= AD_STATE_SYNCHRONIZATION;
+			if (port->aggregator->is_active)
+				port->actor_oper_port_state |=
+				    AD_STATE_SYNCHRONIZATION;
+			else
+				port->actor_oper_port_state &=
+				    ~AD_STATE_SYNCHRONIZATION;
 			port->actor_oper_port_state &= ~AD_STATE_COLLECTING;
 			port->actor_oper_port_state &= ~AD_STATE_DISTRIBUTING;
 			ad_disable_collecting_distributing(port,
@@ -963,6 +981,7 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
 		case AD_MUX_COLLECTING_DISTRIBUTING:
 			port->actor_oper_port_state |= AD_STATE_COLLECTING;
 			port->actor_oper_port_state |= AD_STATE_DISTRIBUTING;
+			port->actor_oper_port_state |= AD_STATE_SYNCHRONIZATION;
 			ad_enable_collecting_distributing(port,
 							  update_slave_arr);
 			port->ntt = true;
@@ -1044,8 +1063,10 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 
 	/* check if the State machine was changed or new lacpdu arrived */
 	if ((port->sm_rx_state != last_state) || (lacpdu)) {
-		pr_debug("Rx Machine: Port=%d, Last State=%d, Curr State=%d\n",
-			 port->actor_port_number, last_state,
+		pr_debug("Rx Machine: Port=%d (%s), Last State=%d, Curr State=%d\n",
+			 port->actor_port_number,
+			 port->slave->dev->name,
+			 last_state,
 			 port->sm_rx_state);
 		switch (port->sm_rx_state) {
 		case AD_RX_INITIALIZE:
@@ -1394,6 +1415,9 @@ static void ad_port_selection_logic(struct port *port, bool *update_slave_arr)
 
 	aggregator = __get_first_agg(port);
 	ad_agg_selection_logic(aggregator, update_slave_arr);
+
+	if (!port->aggregator->is_active)
+		port->actor_oper_port_state &= ~AD_STATE_SYNCHRONIZATION;
 }
 
 /* Decide if "agg" is a better choice for the new active aggregator that
-- 
1.7.10.4

^ permalink raw reply related


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