Netdev List
 help / color / mirror / Atom feed
* [PATCH iproute2 v2] ss: avoid passing negative numbers to malloc
From: Andreas Henriksson @ 2013-11-13  8:46 UTC (permalink / raw)
  To: stephen, Eric Dumazet; +Cc: netdev
In-Reply-To: <1384298327.28458.37.camel@edumazet-glaptop2.roam.corp.google.com>

Example:

$ ss state established \( sport = :4060  or sport = :4061 or sport = :4062  or sport = :4063 or sport = :4064  or sport = :4065 or sport = :4066  or sport = :4067 \)  > /dev/null
Aborted

In the example above ssfilter_bytecompile(...) will return (int)136.
char l1 = 136; means -120 which will result in a negative number
being passed to malloc at misc/ss.c:913.

Simply declare l1 and l2 as integers to avoid the char overflow.

This is one of the issues originally reported in http://bugs.debian.org/511720

Fix the same problem in other code paths as well (thanks to Eric Dumazet).

Reported-by: Andreas Schuldei <andreas@debian.org>
Signed-off-by: Andreas Henriksson <andreas@fatal.se>
---
 misc/ss.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

v2 fixes the same problem in other code paths (AND, NOT).
Pointed out by Eric Dumazet.


diff --git a/misc/ss.c b/misc/ss.c
index c0369f1..6f38ae7 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -894,7 +894,8 @@ static int ssfilter_bytecompile(struct ssfilter *f, char **bytecode)
 
 		case SSF_AND:
 	{
-		char *a1, *a2, *a, l1, l2;
+		char *a1, *a2, *a;
+		int l1, l2;
 		l1 = ssfilter_bytecompile(f->pred, &a1);
 		l2 = ssfilter_bytecompile(f->post, &a2);
 		if (!(a = malloc(l1+l2))) abort();
@@ -907,7 +908,8 @@ static int ssfilter_bytecompile(struct ssfilter *f, char **bytecode)
 	}
 		case SSF_OR:
 	{
-		char *a1, *a2, *a, l1, l2;
+		char *a1, *a2, *a;
+		int l1, l2;
 		l1 = ssfilter_bytecompile(f->pred, &a1);
 		l2 = ssfilter_bytecompile(f->post, &a2);
 		if (!(a = malloc(l1+l2+4))) abort();
@@ -920,7 +922,8 @@ static int ssfilter_bytecompile(struct ssfilter *f, char **bytecode)
 	}
 		case SSF_NOT:
 	{
-		char *a1, *a, l1;
+		char *a1, *a;
+		int l1;
 		l1 = ssfilter_bytecompile(f->pred, &a1);
 		if (!(a = malloc(l1+4))) abort();
 		memcpy(a, a1, l1);
-- 
1.8.4.3

^ permalink raw reply related

* Re: [PATCH iproute2] htb: support 64bit rates
From: Yang Yingliang @ 2013-11-13  9:11 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Stephen Hemminger, netdev
In-Reply-To: <1384295647.28458.31.camel@edumazet-glaptop2.roam.corp.google.com>

On 2013/11/13 6:34, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> Starting from linux-3.13, we can break the 32bit limitation of
> rates on HTB qdisc/classes.
> 
> Prior limit was 34.359.738.360 bits per second.
> 
> lpq83:~# tc -s qdisc show dev lo ; tc -s class show dev lo
> qdisc htb 1: root refcnt 2 r2q 2000 default 1 direct_packets_stat 0 direct_qlen 6000
>  Sent 6591936144493 bytes 149549182 pkt (dropped 0, overlimits 213757419 requeues 0) 
>  rate 39464Mbit 114938pps backlog 0b 15p requeues 0 
> class htb 1:1 root prio 0 rate 50000Mbit ceil 50000Mbit burst 200000b cburst 0b 
>  Sent 6591942184547 bytes 149549310 pkt (dropped 0, overlimits 0 requeues 0) 
>  rate 39464Mbit 114938pps backlog 0b 15p requeues 0 
>  lended: 149549310 borrowed: 0 giants: 0
>  tokens: 336 ctokens: -164
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  tc/q_htb.c   |   56 ++++++++++++++++++++++++++++++++++++-------------
>  tc/tc_core.c |    6 ++---
>  tc/tc_core.h |    4 +--
>  tc/tc_util.c |   27 ++++++++++++++++++++++-
>  tc/tc_util.h |    1 
>  5 files changed, 74 insertions(+), 20 deletions(-)
> 
> diff --git a/tc/q_htb.c b/tc/q_htb.c
> index e108857..1d8c56f 100644
> --- a/tc/q_htb.c
> +++ b/tc/q_htb.c
> @@ -113,6 +113,7 @@ static int htb_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
>  	unsigned int direct_qlen = ~0U;
>  	unsigned int linklayer  = LINKLAYER_ETHERNET; /* Assume ethernet */
>  	struct rtattr *tail;
> +	__u64 ceil64 = 0, rate64 = 0;
>  
>  	memset(&opt, 0, sizeof(opt)); mtu = 1600; /* eth packet len */
>  
> @@ -173,22 +174,22 @@ static int htb_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
>  			ok++;
>  		} else if (strcmp(*argv, "ceil") == 0) {
>  			NEXT_ARG();
> -			if (opt.ceil.rate) {
> +			if (ceil64) {
>  				fprintf(stderr, "Double \"ceil\" spec\n");
>  				return -1;
>  			}
> -			if (get_rate(&opt.ceil.rate, *argv)) {
> +			if (get_rate64(&ceil64, *argv)) {
>  				explain1("ceil");
>  				return -1;
>  			}
>  			ok++;
>  		} else if (strcmp(*argv, "rate") == 0) {
>  			NEXT_ARG();
> -			if (opt.rate.rate) {
> +			if (rate64) {
>  				fprintf(stderr, "Double \"rate\" spec\n");
>  				return -1;
>  			}
> -			if (get_rate(&opt.rate.rate, *argv)) {
> +			if (get_rate64(&rate64, *argv)) {
>  				explain1("rate");
>  				return -1;
>  			}
> @@ -207,17 +208,23 @@ static int htb_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
>  	/*	if (!ok)
>  		return 0;*/
>  
> -	if (opt.rate.rate == 0) {
> +	if (!rate64) {
>  		fprintf(stderr, "\"rate\" is required.\n");
>  		return -1;
>  	}
>  	/* if ceil params are missing, use the same as rate */
> -	if (!opt.ceil.rate) opt.ceil = opt.rate;
> +	if (!ceil64)
> +		ceil64 = rate64;
> +
> +	opt.rate.rate = (rate64 >= (1ULL << 32)) ? ~0U : rate64;
> +	opt.ceil.rate = (ceil64 >= (1ULL << 32)) ? ~0U : ceil64;
>  
>  	/* compute minimal allowed burst from rate; mtu is added here to make
>  	   sute that buffer is larger than mtu and to have some safeguard space */
> -	if (!buffer) buffer = opt.rate.rate / get_hz() + mtu;
> -	if (!cbuffer) cbuffer = opt.ceil.rate / get_hz() + mtu;
> +	if (!buffer)
> +		buffer = rate64 / get_hz() + mtu;
> +	if (!cbuffer)
> +		cbuffer = ceil64 / get_hz() + mtu;

Hi,
It may overflow here if rate64 and mtu are big enough.

Regards,
Yang

>  
>  	opt.ceil.overhead = overhead;
>  	opt.rate.overhead = overhead;
> @@ -229,19 +236,26 @@ static int htb_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
>  		fprintf(stderr, "htb: failed to calculate rate table.\n");
>  		return -1;
>  	}
> -	opt.buffer = tc_calc_xmittime(opt.rate.rate, buffer);
> +	opt.buffer = tc_calc_xmittime(rate64, buffer);
>  
>  	if (tc_calc_rtable(&opt.ceil, ctab, ccell_log, mtu, linklayer) < 0) {
>  		fprintf(stderr, "htb: failed to calculate ceil rate table.\n");
>  		return -1;
>  	}
> -	opt.cbuffer = tc_calc_xmittime(opt.ceil.rate, cbuffer);
> +	opt.cbuffer = tc_calc_xmittime(ceil64, cbuffer);
>  
>  	tail = NLMSG_TAIL(n);
>  	if (direct_qlen != ~0U)
>  		addattr_l(n, 1024, TCA_HTB_DIRECT_QLEN,
>  			  &direct_qlen, sizeof(direct_qlen));
>  	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
> +
> +	if (rate64 >= (1ULL << 32))
> +		addattr_l(n, 1124, TCA_HTB_RATE64, &rate64, sizeof(rate64));
> +
> +	if (ceil64 >= (1ULL << 32))
> +		addattr_l(n, 1224, TCA_HTB_CEIL64, &ceil64, sizeof(ceil64));
> +
>  	addattr_l(n, 2024, TCA_HTB_PARMS, &opt, sizeof(opt));
>  	addattr_l(n, 3024, TCA_HTB_RTAB, rtab, 1024);
>  	addattr_l(n, 4024, TCA_HTB_CTAB, ctab, 1024);
> @@ -256,6 +270,7 @@ static int htb_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
>  	struct tc_htb_glob *gopt;
>  	double buffer,cbuffer;
>  	unsigned int linklayer;
> +	__u64 rate64, ceil64;
>  	SPRINT_BUF(b1);
>  	SPRINT_BUF(b2);
>  	SPRINT_BUF(b3);
> @@ -275,12 +290,25 @@ static int htb_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
>  			if (show_details)
>  				fprintf(f, "quantum %d ", (int)hopt->quantum);
>  		}
> -		fprintf(f, "rate %s ", sprint_rate(hopt->rate.rate, b1));
> +
> +		rate64 = hopt->rate.rate;
> +		if (tb[TCA_HTB_RATE64] &&
> +		    RTA_PAYLOAD(tb[TCA_HTB_RATE64]) >= sizeof(rate64)) {
> +			rate64 = rta_getattr_u64(tb[TCA_HTB_RATE64]);
> +		}
> +
> +		ceil64 = hopt->ceil.rate;
> +		if (tb[TCA_HTB_CEIL64] &&
> +		    RTA_PAYLOAD(tb[TCA_HTB_CEIL64]) >= sizeof(ceil64))
> +			ceil64 = rta_getattr_u64(tb[TCA_HTB_CEIL64]);
> +
> +		fprintf(f, "rate %s ", sprint_rate(rate64, b1));
>  		if (hopt->rate.overhead)
>  			fprintf(f, "overhead %u ", hopt->rate.overhead);
> -		buffer = tc_calc_xmitsize(hopt->rate.rate, hopt->buffer);
> -		fprintf(f, "ceil %s ", sprint_rate(hopt->ceil.rate, b1));
> -		cbuffer = tc_calc_xmitsize(hopt->ceil.rate, hopt->cbuffer);
> +		buffer = tc_calc_xmitsize(rate64, hopt->buffer);
> +
> +		fprintf(f, "ceil %s ", sprint_rate(ceil64, b1));
> +		cbuffer = tc_calc_xmitsize(ceil64, hopt->cbuffer);
>  		linklayer = (hopt->rate.linklayer & TC_LINKLAYER_MASK);
>  		if (linklayer > TC_LINKLAYER_ETHERNET || show_details)
>  			fprintf(f, "linklayer %s ", sprint_linklayer(linklayer, b4));
> diff --git a/tc/tc_core.c b/tc/tc_core.c
> index a524337..46eaefb 100644
> --- a/tc/tc_core.c
> +++ b/tc/tc_core.c
> @@ -56,12 +56,12 @@ unsigned tc_core_ktime2time(unsigned ktime)
>  	return ktime / clock_factor;
>  }
>  
> -unsigned tc_calc_xmittime(unsigned rate, unsigned size)
> +unsigned tc_calc_xmittime(__u64 rate, unsigned size)
>  {
> -	return tc_core_time2tick(TIME_UNITS_PER_SEC*((double)size/rate));
> +	return tc_core_time2tick(TIME_UNITS_PER_SEC*((double)size/(double)rate));
>  }
>  
> -unsigned tc_calc_xmitsize(unsigned rate, unsigned ticks)
> +unsigned tc_calc_xmitsize(__u64 rate, unsigned ticks)
>  {
>  	return ((double)rate*tc_core_tick2time(ticks))/TIME_UNITS_PER_SEC;
>  }
> diff --git a/tc/tc_core.h b/tc/tc_core.h
> index 5a693ba..8a63b79 100644
> --- a/tc/tc_core.h
> +++ b/tc/tc_core.h
> @@ -18,8 +18,8 @@ unsigned tc_core_time2tick(unsigned time);
>  unsigned tc_core_tick2time(unsigned tick);
>  unsigned tc_core_time2ktime(unsigned time);
>  unsigned tc_core_ktime2time(unsigned ktime);
> -unsigned tc_calc_xmittime(unsigned rate, unsigned size);
> -unsigned tc_calc_xmitsize(unsigned rate, unsigned ticks);
> +unsigned tc_calc_xmittime(__u64 rate, unsigned size);
> +unsigned tc_calc_xmitsize(__u64 rate, unsigned ticks);
>  int tc_calc_rtable(struct tc_ratespec *r, __u32 *rtab,
>  		   int cell_log, unsigned mtu, enum link_layer link_layer);
>  int tc_calc_size_table(struct tc_sizespec *s, __u16 **stab);
> diff --git a/tc/tc_util.c b/tc/tc_util.c
> index be3ed07..808c768 100644
> --- a/tc/tc_util.c
> +++ b/tc/tc_util.c
> @@ -171,6 +171,31 @@ int get_rate(unsigned *rate, const char *str)
>  	return 0;
>  }
>  
> +int get_rate64(__u64 *rate, const char *str)
> +{
> +	char *p;
> +	double bps = strtod(str, &p);
> +	const struct rate_suffix *s;
> +
> +	if (p == str)
> +		return -1;
> +
> +	for (s = suffixes; s->name; ++s) {
> +		if (strcasecmp(s->name, p) == 0) {
> +			bps *= s->scale;
> +			p += strlen(p);
> +			break;
> +		}
> +	}
> +
> +	if (*p)
> +		return -1; /* unknown suffix */
> +
> +	bps /= 8; /* -> bytes per second */
> +	*rate = bps;
> +	return 0;
> +}
> +
>  void print_rate(char *buf, int len, __u64 rate)
>  {
>  	double tmp = (double)rate*8;
> diff --git a/tc/tc_util.h b/tc/tc_util.h
> index 7c3709f..d418367 100644
> --- a/tc/tc_util.h
> +++ b/tc/tc_util.h
> @@ -58,6 +58,7 @@ extern struct filter_util *get_filter_kind(const char *str);
>  
>  extern int get_qdisc_handle(__u32 *h, const char *str);
>  extern int get_rate(unsigned *rate, const char *str);
> +extern int get_rate64(__u64 *rate, const char *str);
>  extern int get_size(unsigned *size, const char *str);
>  extern int get_size_and_cell(unsigned *size, int *cell_log, char *str);
>  extern int get_time(unsigned *time, const char *str);
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> .
> 

^ permalink raw reply

* Re: [PATCH] 6lowpan: Uncompression of traffic class field was incorrect
From: Alexander Aring @ 2013-11-13  9:14 UTC (permalink / raw)
  To: Jukka Rissanen; +Cc: netdev
In-Reply-To: <1384333419-4087-1-git-send-email-jukka.rissanen@linux.intel.com>

Hi Jukka,

I know there are some issues with traffic class 
compression/uncompression, but I didn't have time to dig into it.
That's all what I can say now about traffic class issue. Maybe there are
more than one issues or not, I need to dig into the code. :-)


Another note:
Since we known you don't need the fragmentation api accroding rfc4944
for blte 6lowpan I will send the udp fixes in the next days, so then you
can work with it and maybe we can share some code.

- Alex

^ permalink raw reply

* Re: [PATCH] usbnet: fix status interrupt urb handling
From: Oliver Neukum @ 2013-11-13  9:15 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: Felix Fietkau, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, dcbw-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <87r4akoefv.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>

On Wed, 2013-11-13 at 10:09 +0100, Bjørn Mork wrote:
> Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org> writes:
> 
> > On Tue, 2013-11-12 at 16:34 +0100, Felix Fietkau wrote:
> >> Since commit 7b0c5f21f348a66de495868b8df0284e8dfd6bbf
> >> "sierra_net: keep status interrupt URB active", sierra_net triggers
> >> status interrupt polling before the net_device is opened (in order to
> >> properly receive the sync message response).
> >> 
> >> To be able to receive further interrupts, the interrupt urb needs to be
> >> re-submitted, so this patch removes the bogus check for netif_running().
> >
> > And what about the other drivers for whom this patch means added
> > traffic? This fix is done with a sledge hammer and cares only about
> > some drivers.
> 
> usbnet_stop calls usbnet_status_stop which kills the status URB, unless
> the driver explicitly asked for it not to be killed.  And the callback
> properly returns on status == -ENOENT, before the lines in question.
> 
> So I think Felix is right.   None of the other drivers will ever hit the
> code he deletes. It only affects sierra_net, and that is unwanted.

True. I take back the objection.

	Regards
		Oliver


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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

* Re: [PATCH 3/3] can: mcp251x: Add device tree support
From: Marc Kleine-Budde @ 2013-11-13  9:16 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: linux-can, netdev, devicetree, Wolfgang Grandegger, Rob Herring,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell
In-Reply-To: <1384332341-15363-1-git-send-email-shc_work@mail.ru>

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

On 11/13/2013 09:45 AM, Alexander Shiyan wrote:
> This patch adds Device Tree support to the Microchip MCP251X driver.
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  .../bindings/net/can/microchip,mcp251x.txt         | 25 ++++++++++
>  drivers/net/can/mcp251x.c                          | 57 ++++++++++++++++------
>  2 files changed, 66 insertions(+), 16 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/can/microchip,mcp251x.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/can/microchip,mcp251x.txt b/Documentation/devicetree/bindings/net/can/microchip,mcp251x.txt
> new file mode 100644
> index 0000000..06d95ee
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/microchip,mcp251x.txt
> @@ -0,0 +1,25 @@
> +* Microchip MCP251X device tree bindings
> +
> +Registers a Microchip MCP251X stand-alone CAN controller connected to SPI bus.
> +
> +Required properties:
> + - compatible: Should be one of the following:
> +   - "microchip,mcp2510" for MCP2510.
> +   - "microchip,mcp2515" for MCP2515.
> + - reg: SPI chip select.
> + - clocks: The clock feeding the CAN controller.
> + - interrupt-parent: The parent interrupt controller.
> + - interrupts: Should contain IRQ line for the CAN controller.
> +
> +Optional properties:
> + - vdd-supply: Regulator that powers the CAN controller.
> + - xceiver-supply: Regulator that powers the CAN transceiver.
> +
> +Example:
> +	can0: can@1 {
> +		compatible = "microchip,mcp2515";
> +		reg = <1>;
> +		clocks = <&clk24m>;
> +		interrupt-parent = <&gpio4>;
> +		interrupts = <13 0x2>;
> +	};
> diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
> index 8e133aa..9aa2822 100644
> --- a/drivers/net/can/mcp251x.c
> +++ b/drivers/net/can/mcp251x.c
> @@ -60,6 +60,8 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/netdevice.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/spi/spi.h>
> @@ -987,10 +989,38 @@ static const struct net_device_ops mcp251x_netdev_ops = {
>  	.ndo_start_xmit = mcp251x_hard_start_xmit,
>  };
>  
> +static struct of_device_id mcp251x_of_match[] = {
> +	{
> +		.compatible	= "microchip,mcp2510",
> +		.data		= (void *)CAN_MCP251X_MCP2510,
> +	},
> +	{
> +		.compatible	= "microchip,mcp2515",
> +		.data		= (void *)CAN_MCP251X_MCP2515,
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, mcp251x_of_match);
> +
> +static const struct spi_device_id mcp251x_id_table[] = {
> +	{
> +		.name		= "mcp2510",
> +		.driver_data	= (kernel_ulong_t)CAN_MCP251X_MCP2510,
> +	},
> +	{
> +		.name		= "mcp2515",
> +		.driver_data	= (kernel_ulong_t)CAN_MCP251X_MCP2515,
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(spi, mcp251x_id_table);
> +
>  static int mcp251x_can_probe(struct spi_device *spi)
>  {
>  	struct net_device *net;
>  	struct mcp251x_priv *priv;
> +	const struct of_device_id *of_id = of_match_device(mcp251x_of_match,
> +							   &spi->dev);
>  	int ret;
>  
>  	/* Allocate can/net device */
> @@ -1017,7 +1047,10 @@ static int mcp251x_can_probe(struct spi_device *spi)
>  	priv->can.clock.freq = clk_get_rate(priv->clk) / 2;
>  	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES |
>  		CAN_CTRLMODE_LOOPBACK | CAN_CTRLMODE_LISTENONLY;
> -	priv->model = spi_get_device_id(spi)->driver_data;
> +	if (of_id)
> +		priv->model = (enum mcp251x_model)of_id->data;
> +	else
> +		priv->model = spi_get_device_id(spi)->driver_data;
>  	priv->net = net;
>  
>  	priv->power = devm_regulator_get(&spi->dev, "vdd");
> @@ -1198,24 +1231,16 @@ static int mcp251x_can_resume(struct device *dev)
>  static SIMPLE_DEV_PM_OPS(mcp251x_can_pm_ops, mcp251x_can_suspend,
>  	mcp251x_can_resume);
>  
> -static const struct spi_device_id mcp251x_id_table[] = {
> -	{ "mcp2510",	CAN_MCP251X_MCP2510 },
> -	{ "mcp2515",	CAN_MCP251X_MCP2515 },
> -	{ },
> -};
> -
> -MODULE_DEVICE_TABLE(spi, mcp251x_id_table);
> -
>  static struct spi_driver mcp251x_can_driver = {
>  	.driver = {
> -		.name = DEVICE_NAME,
> -		.owner = THIS_MODULE,
> -		.pm = &mcp251x_can_pm_ops,
> +		.name		= DEVICE_NAME,
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= of_match_ptr(mcp251x_of_match),
> +		.pm		= &mcp251x_can_pm_ops,
>  	},
> -
> -	.id_table = mcp251x_id_table,
> -	.probe = mcp251x_can_probe,
> -	.remove = mcp251x_can_remove,
> +	.id_table	= mcp251x_id_table,
> +	.probe		= mcp251x_can_probe,
> +	.remove		= mcp251x_can_remove,

Please don't reformat this. I personally don't like alignment here, as
it tends to be to short if another member is added to the struct.

Looks good otherwise, please add my:

Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>

Marc

>  };
>  module_spi_driver(mcp251x_can_driver);
>  
> 


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


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

^ permalink raw reply

* Re: [PATCH net-next v2 0/10] bonding: rebuild the lock use for bond monitor
From: Veaceslav Falico @ 2013-11-13  9:26 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: David Miller, dingtianhong, fubar, andy, nikolay, netdev
In-Reply-To: <1384298787.3802.59.camel@bwh-desktop.uk.level5networks.com>

On Tue, Nov 12, 2013 at 11:26:27PM +0000, Ben Hutchings wrote:
>On Fri, 2013-11-08 at 01:45 -0500, David Miller wrote:
>> Such patches should not be submitted at this time, the merge window
>> has openned up and therefore the net-next tree is closed.
>>
>> Please wait for the merge window to close and the net-next tree
>> to open back up before submitting these changes.
>
>These look like bug fixes that should have been submitted for the net
>tree.

I am not really sure that it's suitable for net tree, cause these aren't
really bugfixes - but rather locking removal. It might fix/improve some
situations - but the changes touch really sensitive parts and I think they
should go through net-next, to brew a little.

>
>Ben.
>
>-- 
>Ben Hutchings, Staff Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>

^ permalink raw reply

* RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.
From: David Laight @ 2013-11-13  9:28 UTC (permalink / raw)
  To: Alan Stern; +Cc: Sarah Sharp, netdev, linux-usb
In-Reply-To: <Pine.LNX.4.44L0.1311121343060.1200-100000@iolanthe.rowland.org>

> > Since you don't really want to do all the work twice, the sensible
> > way is to add each input fragment to the ring one a time.
> > If you need to cross a link TRB and the last MBP boundary is within
> > the previous data TRB then you can split the previous data TRB at the
> > MBP boundary and continue.
> > If the previous TRB is short then you'd need to go back through the
> > earlier TRB until you found the one that contains a TRB boundary,
> > split it, and write a link TRB in the following slot.
> 
> Right.  You could avoid doing a lot of work twice by using two passes.
> In the first pass, keep track of the number of bytes allotted to each
> TRB and the MBP boundaries, without doing anything else.  That way,
> you'll know where to put a Link TRB without any need for backtracking.
> Then in the second pass, fill in the actual contents of the TRBs.

No - you really don't want to process everything twice.
You could remember the TRB and offset of the last MBP boundary.
 
> > If you are within the first MBP then you'd need to replace the first
> > TRB of the message with a link TRB.
> 
> Unless the first TRB of the message is the first TRB of the ring
> segment.  Then you're in trouble...  Hopefully, real URBs will never be
> that badly fragmented.

I suspect that ones from the network stack might be badly fragmented.
The code needs to at least detect and error them.
I don't think linux skb can be as fragmented as I've seen network
buffers on other systems - 1 byte per buffer chained together to
a maximal sized ethernet packet.
 
> > > > For bulk data the link TRB can be forced at a packet boundary
> > > > by splitting the TD up - the receiving end won't know the difference.
> > >
> > > That won't work.  What happens if you split a TD up into two pieces and
> > > the first piece receives a short packet?  The host controller will
> > > automatically move to the start of the second piece.  That's not what
> > > we want.
> >
> > You can split a bulk TD on a 1k boundary and the target won't know the
> > difference.
> 
> The target won't know the difference, but the host will.  Here's an
> example: Suppose the driver submits two URBs, each for a data-in
> transfer of 32 KB.  You split each URB up into two 16-KB TDs; let's
> call them A, B, C, and D (where A and B make up the first URB, and C
> and D make up the second URB).

I was thinking about OUT transfers, IN ones are unlikely to be badly
fragmented.

> > > > There is no necessity for taking an interrupt from every link segment.
> > >
> > > Yes, there is.  The HCD needs to know when the dequeue pointer has
> > > moved beyond the end of the ring segment, so that it can start reusing
> > > the TRB slots in that segment.
> >
> > You already know that because of the interrupts for the data packets
> > themselves.
> 
> I don't know what you mean by this.  The host controller does not
> generate an interrupt for each data packet.  In generates interrupts
> only for TRBs with the IOC bit set (which is generally the last TRB in
> each TD).
> 
> Suppose you have only two ring segments, and a driver submits an URB
> which is so fragmented that it requires more TRBs than you have room
> for in those two segments.  When do you want the interrupts to arrive?
> Answer: At each segment crossing.

You bounce the original request and fix the driver to submit URB
with fewer fragments.

> > > > I would change the code to use a single segment (for coding simplicity)
> > > > and queue bulk URB when there isn't enough ring space.
> > > > URB with too many fragments could either be rejected, sent in sections,
> > > > or partially linearised (and probably still sent in sections).
> > >
> > > Rejecting an URB is not feasible.  Splitting it up into multiple TDs is
> > > not acceptable, as explained above.  Sending it in sections (i.e.,
> > > queueing only some of the TRBs at any time) would work, provided you
> > > got at least two interrupts every time the queue wrapped around (which
> > > suggests you might want at least two ring segments).
> >
> > Rejecting badly fragmented URB is almost certainly ok. You really don't
> > want the hardware overhead of processing a TRB every few bytes.
> > This would be especially bad on iommu systems.
> 
> I disagree.  Sure, it would be bad.  But if you can handle it, you
> should.

You have to draw the line somewhere.

	David

^ permalink raw reply

* RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.
From: David Laight @ 2013-11-13  9:45 UTC (permalink / raw)
  To: Alan Stern, Sarah Sharp; +Cc: netdev, linux-usb
In-Reply-To: <Pine.LNX.4.44L0.1311121401510.1200-100000@iolanthe.rowland.org>

> > that doesn't matter; we don't get an interrupt when a ring segment is
> > crossed.  Instead we set the interrupt-on-completion flag on the last
> > TRB of the TD, not on any earlier fragment or link TRB.
> 
> That's because you don't worry about handling URBs which require too
> many TRBs (i.e., more than are available).  You just add more ring
> segments.  Instead, you could re-use segments on the fly.
> 
> For example, suppose you have only two ring segments and you get an URB
> which requires enough TRBs to fill up four segments.  You could fill in
> the first two segments worth, and get an interrupt when the controller
> traverses the Link TRB between them.  At that point you store the third
> set of TRBs in the first segment, which is now vacant.  Similarly, when
> the second Link TRB is traversed, you fill in the fourth set of TRBs.

That isn't going to work.
It might work for very long TD, but for very fragmented ones the interrupt
rate would be stupid. In any case you'd definitely need enough ring
segments for the TRB that describe a single 'max packet size' block.

> > Finally, it's interesting to note that the USB mass storage driver is
> > using scatter gather lists just fine without the driver following the TD
> > fragment rules.  Or at least no one has reported any issues.  I wonder
> > why it works?
> 
> I'd guess this is because the hardware is actually a lot more flexible
> than the "No Link TRBs in the middle of a TD fragment" rule.

With the hardware I have (Intel i7 Sandy bridge) Link TRB cannot
be placed at arbitrary boundaries.
I don't know whether the actual restriction is only to packet boundaries.
I don't have a USB3 monitor and it would also require a more contrived
test than I've been doing.

> The whole idea of TD fragments makes no sense to begin with.  What
> point is there in grouping packets into MaxBurst-sized collections?

It probably saved a few logic gates somewhere, either that or it is
a bug in some hardware implementation that got documented in the spec
instead of being fixed :-)

	David

^ permalink raw reply

* Re: [ovs-dev] [PATCH] linux: Signal datapath that unaligned Netlink message can be received
From: Thomas Graf @ 2013-11-13  9:46 UTC (permalink / raw)
  To: Jesse Gross
  Cc: Ben Pfaff, dev@openvswitch.org, Ben Hutchings, fleitner,
	dborkmann, netdev
In-Reply-To: <CAEP_g=_KWgmfVkNLPGYr=WjNR=UKSt3PhkqU8PE6Ug3ju2R3ig@mail.gmail.com>

On 11/13/2013 07:11 AM, Jesse Gross wrote:
> On Mon, Nov 11, 2013 at 11:53 PM, Thomas Graf <tgraf@redhat.com> wrote:
>> On 11/11/2013 04:50 PM, Ben Pfaff wrote:
>>>
>>> On Mon, Nov 11, 2013 at 04:36:24PM +0100, Thomas Graf wrote:
>>>>
>>>> Following commit (''netlink: Do not enforce alignment of last Netlink
>>>> attribute''), signal the ability to receive unaligned Netlink messages
>>>> to the datapath to enable utilization of zerocopy optimizations.
>>>>
>>>> Signed-off-by: Thomas Graf <tgraf@redhat.com>
>>>
>>>
>>> Seems OK from a userspace point of view.  I am a little concerned that
>>> downgrading userspace without deleting and re-creating the datapath
>>> (e.g. via "force-reload-kmod") will result in a totally broken setup
>>> since userspace will then drop every packet from the kernel.
>>
>>
>> Is that something that occurs occasionally in installations? Utilizing
>> the version field in the genl header could be used to track this and
>> clear user_features.
>
> It's probably a good idea. I could see us having more of these
> features flags in the future (although obviously we should try to
> avoid them if possible) and, as Ben said, it would potentially lead to
> a bad state otherwise.
>
> I'm not sure exactly what you have in mind though, can you elaborate a little?

My initial thought was to use a version field to notice the replacement
of user space. On second thought that is not required, modifying user
space to provide the user features in OVS_DP_CMD_GET as well will
overwrite the features. Resetting user_features to 0 if not features are
provided will provide backwards compatibility to versions not aware of
user features yet. Thoughts?

> (By the way, it might be a good idea to keep the same CC list on all
> of the patches. Otherwise, some people might miss parts of the
> discussion.)

I did not CC netdev because this is a pure user space patch and the
respective kernel bits are included in v5 of the kernel series.

^ permalink raw reply

* [PATCH] mac80211: minstrels: spare numerous useless calls to get_random_bytes
From: Karl Beldan @ 2013-11-13  9:54 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Felix Fietkau, linux-wireless, Hannes Frederic Sowa, netdev,
	Karl Beldan

From: Karl Beldan <karl.beldan@rivierawaves.com>

ATM, only the first array value returned by get_random_bytes is used.
This change moves the call to get_random_bytes from the nested loop it
is in to its parent.
While at it, replace get_random_bytes with prandom_bytes since PRNs are
way enough for the selection process.
After this, minstrel_ht reclaims 80 PR-bytes instead of 640 R-bytes.

minstrels use sample tables to probe different rates in a randomized
manner.
minstrel_ht inits one single sample table upon registration (during
subsys_initcalls) and minstrel uses one per STA addition in minstrel.

Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
---
 net/mac80211/rc80211_minstrel.c    | 3 +--
 net/mac80211/rc80211_minstrel_ht.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
index 7fa1b36..d2f19f7 100644
--- a/net/mac80211/rc80211_minstrel.c
+++ b/net/mac80211/rc80211_minstrel.c
@@ -422,10 +422,9 @@ init_sample_table(struct minstrel_sta_info *mi)
 	memset(mi->sample_table, 0xff, SAMPLE_COLUMNS * mi->n_rates);
 
 	for (col = 0; col < SAMPLE_COLUMNS; col++) {
+		prandom_bytes(rnd, sizeof(rnd));
 		for (i = 0; i < mi->n_rates; i++) {
-			get_random_bytes(rnd, sizeof(rnd));
 			new_idx = (i + rnd[i & 7]) % mi->n_rates;
-
 			while (SAMPLE_TBL(mi, new_idx, col) != 0xff)
 				new_idx = (new_idx + 1) % mi->n_rates;
 
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index 5d60779..91912f8 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -1052,10 +1052,9 @@ init_sample_table(void)
 
 	memset(sample_table, 0xff, sizeof(sample_table));
 	for (col = 0; col < SAMPLE_COLUMNS; col++) {
+		prandom_bytes(rnd, sizeof(rnd));
 		for (i = 0; i < MCS_GROUP_RATES; i++) {
-			get_random_bytes(rnd, sizeof(rnd));
 			new_idx = (i + rnd[i]) % MCS_GROUP_RATES;
-
 			while (sample_table[col][new_idx] != 0xff)
 				new_idx = (new_idx + 1) % MCS_GROUP_RATES;
 
-- 
1.8.2

^ permalink raw reply related

* Xmas Loan ...Apply Now
From: ''Fredrick Loan Investment'' @ 2013-11-13  9:46 UTC (permalink / raw)
  To: Recipients

Do you need a loan ? if yes contact fredrickplc@qq.com for more.

^ permalink raw reply

* Re: [PATCH net RESEND] bonding: add ip checks when store ip target
From: Veaceslav Falico @ 2013-11-13 10:00 UTC (permalink / raw)
  To: Ding Tianhong
  Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller,
	Nikolay Aleksandrov, Netdev
In-Reply-To: <5282E196.8030401@huawei.com>

On Wed, Nov 13, 2013 at 10:19:02AM +0800, Ding Tianhong wrote:
>I met a Bug when I add ip target with the wrong ip address:
>
>echo +500.500.500.500 > /sys/class/net/bond0/bonding/arp_ip_target
>
>the wrong ip address will transfor to 245.245.245.244 and add
>to the ip target success, it is uncorrect, so I add checks to avoid
>adding wrong address.
>
>The in4_pton() will set wrong ip address to 0.0.0.0, it will return by
>the next check and will not add to ip target.
>
>Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>---
> drivers/net/bonding/bond_sysfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index ec9b646..e0c97fb 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -653,7 +653,7 @@ static ssize_t bonding_store_arp_targets(struct device *d,
> 	int ind, i, j, ret = -EINVAL;
>
> 	targets = bond->params.arp_targets;
>-	newtarget = in_aton(buf + 1);
>+	in4_pton(buf + 1, strlen(buf) - 1, (u8 *)&newtarget, -1, NULL);

No need for strlen(buf)-1, if you specify -1 it will compute it by itself.

Also, you might simplify the code a bit in the function. Otherwise - looks
good, thank you.

Untested patch:

diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index c29b836..e3fff6e 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -661,16 +661,16 @@ static ssize_t bonding_store_arp_targets(struct device *d,
  	unsigned long *targets_rx;
  	int ind, i, j, ret = -EINVAL;
  
+	if (!in4_pton(buf + 1, -1, (u8 *)&newtarget, -1, NULL)) {
+		pr_err("%s: invalid ARP target specified, use +<addr> or -<addr>\n",
+		       bond->dev->name);
+		goto out;
+	}
+
  	targets = bond->params.arp_targets;
-	newtarget = in_aton(buf + 1);
+
  	/* look for adds */
  	if (buf[0] == '+') {
-		if ((newtarget == 0) || (newtarget == htonl(INADDR_BROADCAST))) {
-			pr_err("%s: invalid ARP target %pI4 specified for addition\n",
-			       bond->dev->name, &newtarget);
-			goto out;
-		}
-
  		if (bond_get_targets_ip(targets, newtarget) != -1) { /* dup */
  			pr_err("%s: ARP target %pI4 is already present\n",
  			       bond->dev->name, &newtarget);
@@ -693,12 +693,6 @@ static ssize_t bonding_store_arp_targets(struct device *d,
  		targets[ind] = newtarget;
  		write_unlock_bh(&bond->lock);
  	} else if (buf[0] == '-')	{
-		if ((newtarget == 0) || (newtarget == htonl(INADDR_BROADCAST))) {
-			pr_err("%s: invalid ARP target %pI4 specified for removal\n",
-			       bond->dev->name, &newtarget);
-			goto out;
-		}
-
  		ind = bond_get_targets_ip(targets, newtarget);
  		if (ind == -1) {
  			pr_err("%s: unable to remove nonexistent ARP target %pI4.\n",

^ permalink raw reply related

* RE: [Fwd: Re: [PATCH v2 2/2] x86: add prefetching to do_csum]
From: David Laight @ 2013-11-13 10:09 UTC (permalink / raw)
  To: Neil Horman, Joe Perches
  Cc: netdev, Dave Jones, linux-kernel, sebastien.dugue,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Eric Dumazet
In-Reply-To: <20131112195005.GD19780@hmsreliant.think-freely.org>

> Sure, I modified the code so that we only prefetched 2 cache lines ahead, but
> only if the overall length of the input buffer is more than 2 cache lines.
> Below are the results (all counts are the average of 1000000 iterations of the
> csum operation, as previous tests were, I just omitted that column).

Hmmm.... averaging over 100000 iterations means that all the code
is in the i-cache and the branch predictor will be correctly primed.

For short checksum requests I'd guess that the relevant data
has just been written and is already in the cpu cache (unless
there has been a process and cpu switch).
So prefetch is likely to be unnecessary.

If you assume that the checksum code isn't in the i-cache then
small requests are likely to be dominated by the code size.

	David

^ permalink raw reply

* Re: [PATCH 3/3] can: mcp251x: Add device tree support
From: Kumar Gala @ 2013-11-13 10:53 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: linux-can, netdev, devicetree, Wolfgang Grandegger,
	Marc Kleine-Budde, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell
In-Reply-To: <1384332341-15363-1-git-send-email-shc_work@mail.ru>


On Nov 13, 2013, at 2:45 AM, Alexander Shiyan <shc_work@mail.ru> wrote:

> This patch adds Device Tree support to the Microchip MCP251X driver.
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
> .../bindings/net/can/microchip,mcp251x.txt         | 25 ++++++++++
> drivers/net/can/mcp251x.c                          | 57 ++++++++++++++++------
> 2 files changed, 66 insertions(+), 16 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/can/microchip,mcp251x.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/can/microchip,mcp251x.txt b/Documentation/devicetree/bindings/net/can/microchip,mcp251x.txt
> new file mode 100644
> index 0000000..06d95ee
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/microchip,mcp251x.txt
> @@ -0,0 +1,25 @@
> +* Microchip MCP251X device tree bindings
> +
> +Registers a Microchip MCP251X stand-alone CAN controller connected to SPI bus.

“Register a” doesn’t seem like it should be in a binding spec.

> +
> +Required properties:
> + - compatible: Should be one of the following:
> +   - "microchip,mcp2510" for MCP2510.
> +   - "microchip,mcp2515" for MCP2515.
> + - reg: SPI chip select.
> + - clocks: The clock feeding the CAN controller.
> + - interrupt-parent: The parent interrupt controller.
> + - interrupts: Should contain IRQ line for the CAN controller.
> +
> +Optional properties:
> + - vdd-supply: Regulator that powers the CAN controller.
> + - xceiver-supply: Regulator that powers the CAN transceiver.
> +
> +Example:
> +	can0: can@1 {
> +		compatible = "microchip,mcp2515";
> +		reg = <1>;
> +		clocks = <&clk24m>;
> +		interrupt-parent = <&gpio4>;
> +		interrupts = <13 0x2>;

Usually good to have example show all properties used (even optional ones)

> +	};

Other than the nits, binding looks good.

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply

* Re: [PATCH net] netlink: fix netlink_ack with large messages
From: Jiri Benc @ 2013-11-13 11:25 UTC (permalink / raw)
  To: David Miller; +Cc: pablo, jhs, tgraf, netdev
In-Reply-To: <20131112.143515.1092879587357986857.davem@davemloft.net>

On Tue, 12 Nov 2013 14:35:15 -0500 (EST), David Miller wrote:
> > Do you think capping at NLMSG_GOODSIZE would be too low? The allocation
> > won't fit into one page with NLMSG_GOODSIZE but I doubt we can go lower
> > than that. Alternatively, we can do some math to fully use the two
> > pages, like
> > 	NLMSG_GOODSIZE + min(PAGE_SIZE, 8192UL) - NLMSG_HDRLEN - NLMSG_ALIGN(sizeof(struct nlmsgerr))
> > (which I'm not sure is worth it).
> 
> I don't think this is the way to go.
> 
> I think since existing apps expect the whole message, we have to
> provide it.
> 
> We should add a new socket option so that applications can ask that
> messages not be quoted in ACKs, as I've stated a few times already in
> this thread.

I completely agree with this, sorry for not being clear. I just
understood from the thread that the way to go is to do both, in order
to not generate too large ACKs for the _new_ code (i.e. for the
messages that were not plausible before "netlink: allow large data
transfers from user-space"). I don't know what the "too large" should
be, though, hence the question.

But then, if we don't do any capping, the only outcome of a failed
allocation is the ACK won't be sent and it's clearly stated that
netlink does not provide reliability. Works for me.

 Jiri

-- 
Jiri Benc

^ permalink raw reply

* Re: [Fwd: Re: [PATCH v2 2/2] x86: add prefetching to do_csum]
From: Neil Horman @ 2013-11-13 12:30 UTC (permalink / raw)
  To: David Laight
  Cc: Joe Perches, netdev, Dave Jones, linux-kernel, sebastien.dugue,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Eric Dumazet
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B740E@saturn3.aculab.com>

On Wed, Nov 13, 2013 at 10:09:51AM -0000, David Laight wrote:
> > Sure, I modified the code so that we only prefetched 2 cache lines ahead, but
> > only if the overall length of the input buffer is more than 2 cache lines.
> > Below are the results (all counts are the average of 1000000 iterations of the
> > csum operation, as previous tests were, I just omitted that column).
> 
> Hmmm.... averaging over 100000 iterations means that all the code
> is in the i-cache and the branch predictor will be correctly primed.
> 
> For short checksum requests I'd guess that the relevant data
> has just been written and is already in the cpu cache (unless
> there has been a process and cpu switch).
> So prefetch is likely to be unnecessary.
> 
> If you assume that the checksum code isn't in the i-cache then
> small requests are likely to be dominated by the code size.
> 
I'm not sure, whats the typical capacity for the branch predictors ability to
remember code paths?  I ask because the most likely use of do_csum will be in
the receive path of the networking stack (specifically in the softirq handler).
So if we run do_csum once, we're likely to run it many more times, as we clean
out an adapters receive queue.

Neil

> 	David
> 
> 
> 
> 

^ permalink raw reply

* [PATCH] net: ethernet: ti/cpsw: do not crash on single-MAC machines during resume
From: Daniel Mack @ 2013-11-13 12:53 UTC (permalink / raw)
  To: netdev; +Cc: mugunthanvnm, davem, Daniel Mack

To prevent Oopses, cpsw_suspend() should only soft-reset sliver 1
on dual MAC enabled machines.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/net/ethernet/ti/cpsw.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index edd3e79..702b063 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2173,8 +2173,11 @@ static int cpsw_suspend(struct device *dev)
 
 	if (netif_running(ndev))
 		cpsw_ndo_stop(ndev);
+
 	soft_reset("sliver 0", &priv->slaves[0].sliver->soft_reset);
-	soft_reset("sliver 1", &priv->slaves[1].sliver->soft_reset);
+	if (priv->data.dual_emac)
+		soft_reset("sliver 1", &priv->slaves[1].sliver->soft_reset);
+
 	pm_runtime_put_sync(&pdev->dev);
 
 	/* Select sleep pin state */
-- 
1.8.4.2

^ permalink raw reply related

* network namespaces usage in routing appplications with VRF-like features
From: Jacob Avraham @ 2013-11-13 12:56 UTC (permalink / raw)
  To: netdev@vger.kernel.org

Hi,
I'm trying to evaluate the use of network namespaces in some routing applications, which are currently based on the VFR patchset.
I was thinking about distributing several interfaces among a few namespaces, whereby each namespace has its own routing table(s), ala VRF.
The questions that I have are:
1. Can I open a listening socket that accepts traffic from all interfaces in all namespaces? That's the current behavior of my routing protocol daemons.
2.   Does RTNETLINK API support adding routes to a particular namespace routing table?

Thanks,
Jacob

^ permalink raw reply

* Re: [Fwd: Re: [PATCH v2 2/2] x86: add prefetching to do_csum]
From: Ingo Molnar @ 2013-11-13 13:08 UTC (permalink / raw)
  To: Neil Horman
  Cc: David Laight, Joe Perches, netdev, Dave Jones, linux-kernel,
	sebastien.dugue, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	x86, Eric Dumazet, Peter Zijlstra
In-Reply-To: <20131113123010.GB2993@hmsreliant.think-freely.org>


* Neil Horman <nhorman@tuxdriver.com> wrote:

> On Wed, Nov 13, 2013 at 10:09:51AM -0000, David Laight wrote:
> > > Sure, I modified the code so that we only prefetched 2 cache lines ahead, but
> > > only if the overall length of the input buffer is more than 2 cache lines.
> > > Below are the results (all counts are the average of 1000000 iterations of the
> > > csum operation, as previous tests were, I just omitted that column).
> > 
> > Hmmm.... averaging over 100000 iterations means that all the code
> > is in the i-cache and the branch predictor will be correctly primed.
> > 
> > For short checksum requests I'd guess that the relevant data
> > has just been written and is already in the cpu cache (unless
> > there has been a process and cpu switch).
> > So prefetch is likely to be unnecessary.
> > 
> > If you assume that the checksum code isn't in the i-cache then
> > small requests are likely to be dominated by the code size.
> 
> I'm not sure, whats the typical capacity for the branch predictors 
> ability to remember code paths?  I ask because the most likely use of 
> do_csum will be in the receive path of the networking stack 
> (specifically in the softirq handler). So if we run do_csum once, we're 
> likely to run it many more times, as we clean out an adapters receive 
> queue.

For such simple single-target branches it goes near or over a thousand for 
recent Intel and AMD microarchitectures. Thousands for really recent CPUs.

Note that branch prediction caches are hierarchical and are typically 
attached to cache hierarchies (where the uops are fetched from), so the 
first level BTB is typically shared between SMT CPUs that share an icache 
and L2 BTBs (which is larger and more associative) are shared by all cores 
in a package.

So it's possible for some other task on another (sibling) CPU to keep 
pressure on your BTB, but I'd say it's relatively rare, it's hard to do it 
at a really high rate that blows away all the cache all the time. (PeterZ 
has written some artificial pseudorandom branching monster just to be able 
to generate cache misses and validate perf's branch stats - but even if 
deliberately want to it's pretty hard to beat that cache.)

I'd definitely not worry about the prediction accuracy of repetitive loops 
like csum routines, they'll be cached well.

Thanks,

	Ingo

^ permalink raw reply

* RE: [Fwd: Re: [PATCH v2 2/2] x86: add prefetching to do_csum]
From: David Laight @ 2013-11-13 13:32 UTC (permalink / raw)
  To: Ingo Molnar, Neil Horman
  Cc: Joe Perches, netdev, Dave Jones, linux-kernel, sebastien.dugue,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Eric Dumazet,
	Peter Zijlstra
In-Reply-To: <20131113130832.GB22140@gmail.com>

> > I'm not sure, whats the typical capacity for the branch predictors
> > ability to remember code paths?
...
> 
> For such simple single-target branches it goes near or over a thousand for
> recent Intel and AMD microarchitectures. Thousands for really recent CPUs.

IIRC the x86 can also correctly predict simple sequences - like a branch
in a loop that is taken every other iteration, or only after a previous
branch is taken.

Much simpler cpus may use a much simpler strategy.
I think one I've used (a fpga soft-core cpu) just uses the low
bits of the instruction address to index a single bit table.
This means that branches alias each other.
In order to get the consistent cycle counts in order to minimise
the worst case code path we had to disable the dynamic prediction.

For the checksum code the loop branch isn't a problem.
Tests on entry to the function might get mispredicted.

So if you have conditional prefetch when the buffer is long
then time a short buffer after a 100 long ones you'll almost
certainly see the mispredition penalty.

FWIW I remember speeding up a copy (I think) loop on a strongarm by
adding an extra instruction to fetch a word from later in the buffer
into a register I never otherwise used.
(That was an unpaged system so I knew it couldn't fault.)

	David

^ permalink raw reply

* Re: [Fwd: Re: [PATCH v2 2/2] x86: add prefetching to do_csum]
From: Ingo Molnar @ 2013-11-13 13:53 UTC (permalink / raw)
  To: David Laight
  Cc: Neil Horman, Joe Perches, netdev, Dave Jones, linux-kernel,
	sebastien.dugue, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	x86, Eric Dumazet, Peter Zijlstra
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7412@saturn3.aculab.com>


* David Laight <David.Laight@ACULAB.COM> wrote:

> > > I'm not sure, whats the typical capacity for the branch predictors 
> > > ability to remember code paths?
> ...
> > 
> > For such simple single-target branches it goes near or over a thousand 
> > for recent Intel and AMD microarchitectures. Thousands for really 
> > recent CPUs.
> 
> IIRC the x86 can also correctly predict simple sequences - like a branch 
> in a loop that is taken every other iteration, or only after a previous 
> branch is taken.

They tend to be rather capable but not very well documented :) With a 
large out of order execution design and 20+ pipeline stages x86 branch 
prediction accuracy is perhaps the most important design aspect to good 
CPU performance.

> Much simpler cpus may use a much simpler strategy.

Yeah. The patches in this thread are about the x86 assembly implementation 
of the csum routines, and for 'typical' x86 CPUs the branch prediction 
units and caches are certainly sophisticated enough.

Also note that here, for real usecases, the csum routines are (or should 
be) memory bandwidth limited, missing the data cache most of the time, 
with a partially idling pipeline, while branch prediction accuracy matters 
most when the pipeline is well fed and there are a lot of instructions in 
flight.

Thanks,

	Ingo

^ permalink raw reply

* how to mix bridges and bonding inc. vlans correctly on Kernel > 3.10
From: Stefan Priebe - Profihost AG @ 2013-11-13 13:58 UTC (permalink / raw)
  To: Linux Netdev List

Hello,

while my vlans, bridging and bonding stuff was working until 3.9 i never
thought about how it is right. So maybe i was always wrong.

I've this:

eth2
     \
      -- bond1 -- vmbr1
     /
eth3

This works fine and as expected now i want to have a vlan using the
bonding and using a bridge.

I the past i had this:
eth2
     \
      -- bond1 -- vmbr1
     /              \
eth3                 \ vmbr1.3000
                           \ ---- tap114i1

This was working fine until 3.9.X since 3.10. Right now using 3.10 i
need to put eth2 and eth3 into promisc mode to get it working ;-( this
is bad!

I also tried this one without success:
eth2
     \
      -- bond1 -- vmbr1
     /     \
eth3        ----- bond1.3000 --- vmbr1v3000
                                     \ ---- tap114i1



Greets,
Stefan

^ permalink raw reply

* Re: [PATCH] tcp: tsq: restore minimal amount of queueing
From: Eric Dumazet @ 2013-11-13 14:06 UTC (permalink / raw)
  To: Arnaud Ebalard
  Cc: David Miller, Sujith Manoharan, Cong Wang, netdev, Felix Fietkau
In-Reply-To: <1384267141.28458.24.camel@edumazet-glaptop2.roam.corp.google.com>

On Tue, 2013-11-12 at 06:39 -0800, Eric Dumazet wrote:

> -		limit = max(skb->truesize, sk->sk_pacing_rate >> 10);
> +		limit = max(sysctl_tcp_limit_output_bytes,
> +			    sk->sk_pacing_rate >> 10);
>  


I'll send a v2, a max_t(unsigned int, ..., ...) is needed here,
as reported by kbuild bot.

net/ipv4/tcp_output.c:1882:177: warning: comparison of distinct pointer
types lacks a cast [enabled by default]

^ permalink raw reply

* Re: how to mix bridges and bonding inc. vlans correctly on Kernel > 3.10
From: Veaceslav Falico @ 2013-11-13 14:12 UTC (permalink / raw)
  To: Stefan Priebe - Profihost AG; +Cc: Linux Netdev List
In-Reply-To: <52838590.5070806@profihost.ag>

On Wed, Nov 13, 2013 at 02:58:40PM +0100, Stefan Priebe - Profihost AG wrote:
>Hello,
>
>while my vlans, bridging and bonding stuff was working until 3.9 i never
>thought about how it is right. So maybe i was always wrong.
>
>I've this:
>
>eth2
>     \
>      -- bond1 -- vmbr1
>     /
>eth3
>
>This works fine and as expected now i want to have a vlan using the
>bonding and using a bridge.
>
>I the past i had this:
>eth2
>     \
>      -- bond1 -- vmbr1
>     /              \
>eth3                 \ vmbr1.3000
>                           \ ---- tap114i1
>
>This was working fine until 3.9.X since 3.10. Right now using 3.10 i
>need to put eth2 and eth3 into promisc mode to get it working ;-( this
>is bad!

As a guess - do you use arp monitoring for bonding? Try using miimon -
there were some issues with it in 3.10, which were fixed by some huge
patchsets that will never hit 3.10 stable.

Also, the bonding configuration would be welcome.

>
>I also tried this one without success:
>eth2
>     \
>      -- bond1 -- vmbr1
>     /     \
>eth3        ----- bond1.3000 --- vmbr1v3000
>                                     \ ---- tap114i1
>
>
>
>Greets,
>Stefan
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: shutdown(3) and bluetooth.
From: John W. Linville @ 2013-11-13 14:02 UTC (permalink / raw)
  To: David Miller; +Cc: davej, netdev, linux-bluetooth, linux-wireless
In-Reply-To: <20131112.161350.946584501122269943.davem@davemloft.net>

On Tue, Nov 12, 2013 at 04:13:50PM -0500, David Miller wrote:
> From: Dave Jones <davej@redhat.com>
> Date: Tue, 12 Nov 2013 16:11:25 -0500
> 
> > Is shutdown() allowed to block indefinitely ? The man page doesn't say either way,
> > and I've noticed that my fuzz tester occasionally hangs for days spinning in bt_sock_wait_state()
> > 
> > Is there something I should be doing to guarantee that this operation
> > will either time out, or return instantly ?
> > 
> > In this specific case, I doubt anything is on the "sender" end of the socket, so
> > it's going to be waiting forever for a state change that won't arrive.
> 
> Adding bluetooth and wireless lists.  Dave, please consult MAINTAINERS when
> asking questions like this, thanks!

I don't have an authoritative answer.  I do, however, seem to recall
that trying to shutdown a SunOS box with a hung NFS mount would seem
to hang forever.  I don't think that is a great metric for how we
should behave, of course...

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply


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