Netdev List
 help / color / mirror / Atom feed
* [PATCH iproute2] ss: add more tcp socket diagnostics
From: Eric Dumazet @ 2014-10-01 11:17 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

Display 4 additional tcp socket info fields :

backoff : exponential backoff
lastsnd : time in milli second since last send
lastrcv : time in milli second since last receive
lastack : time in milli second since last acknowledgement

$ ss -ti dst :22
State       Recv-Q Send-Q                  Local Address:Port
Peer Address:Port   
ESTAB       0      0                        172.16.5.1:58470
172.17.131.143:ssh     
	 cubic wscale:7,7 rto:228 rtt:30/20 ato:40 mss:1256 cwnd:6 ssthresh:4
send 2.0Mbps lastsnd:3480 lastrcv:3464 lastack:3464 rcv_rtt:81.5
rcv_space:87812

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 misc/ss.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/misc/ss.c b/misc/ss.c
index c847954..2420b51 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -1554,6 +1554,8 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 			       info->tcpi_rcv_wscale);
 		if (info->tcpi_rto && info->tcpi_rto != 3000000)
 			printf(" rto:%g", (double)info->tcpi_rto/1000);
+		if (info->tcpi_backoff)
+			printf(" backoff:%u", info->tcpi_backoff);
 		if (info->tcpi_rtt)
 			printf(" rtt:%g/%g", (double)info->tcpi_rtt/1000,
 			       (double)info->tcpi_rttvar/1000);
@@ -1583,6 +1585,15 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 					 / rtt));
 		}
 
+		if (info->tcpi_last_data_sent)
+			printf(" lastsnd:%u", info->tcpi_last_data_sent);
+
+		if (info->tcpi_last_data_recv)
+			printf(" lastrcv:%u", info->tcpi_last_data_recv);
+
+		if (info->tcpi_last_ack_recv)
+			printf(" lastack:%u", info->tcpi_last_ack_recv);
+
 		if (info->tcpi_pacing_rate &&
 		    info->tcpi_pacing_rate != ~0ULL) {
 			printf(" pacing_rate %sbps",

^ permalink raw reply related

* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Marc Kleine-Budde @ 2014-10-01 11:11 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Roger Quadros, wg, tony, tglx, mugunthanvnm, george.cherian,
	balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can,
	netdev
In-Reply-To: <20141001104317.GB1261@katana>

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

On 10/01/2014 12:43 PM, Wolfram Sang wrote:
> 	compatible = "commodore,c64ultra", "bosch,d_can";

\o/

-- 
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: 181 bytes --]

^ permalink raw reply

* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Wolfram Sang @ 2014-10-01 11:10 UTC (permalink / raw)
  To: Roger Quadros
  Cc: Marc Kleine-Budde, wg, tony, tglx, mugunthanvnm, george.cherian,
	balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can,
	netdev
In-Reply-To: <542BDE32.6030309@ti.com>

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


> Is it OK to create a new platform_data structure for CAN and put the type and raminit start/stop
> bits there?

I'd say yes, don't see a reason not to.


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

^ permalink raw reply

* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Marc Kleine-Budde @ 2014-10-01 11:06 UTC (permalink / raw)
  To: Roger Quadros, Wolfram Sang
  Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <542BDE32.6030309@ti.com>

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

On 10/01/2014 12:57 PM, Roger Quadros wrote:
>>> ...if we just have the instance parameter in the syscon phandle, we have
>>> to put the mapping into the driver, which makes IMHO no sense, because
>>> you have to touch the driver, if there is another SoC with the DCAN core.
> 
> My guess is that TI won't come up with a 3rd variant so we won't have
> to touch the driver, but you never know for sure.

When I comes to 99% compatible hardware.... I've seen some.

>> ... which would be my preferred solution. I think new SoCs should have
>> some kind of:
>>
>> 	compatible = "commodore,c64ultra", "bosch,d_can";
>>
>> in the DT anyhow to allow for SoC specific quirks/adjustments. And
>> custom raminit belongs to that IMO (see the ti routine getting more and
>> more specific).
>>
> 
> Right. For now we need 2 start/stop definations for the existing TI Socs.
> 
> but where to store the raminit start/stop bits? The driver_data currently seems to 
> contain the CAN type C_CAN vs D_CAN without containing it in a platform_data structure.
> 
> Is it OK to create a new platform_data structure for CAN and put the type and raminit start/stop
> bits there?

Yes, have a look how it's handled in the flexcan driver.

regards,
Marc

-- 
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: 181 bytes --]

^ permalink raw reply

* RE: [PATCH v2 3/4] net: stmmac: add support for Intel Quark X1000
From: Kweh, Hock Leong @ 2014-10-01 11:06 UTC (permalink / raw)
  To: 'David Miller'
  Cc: 'peppe.cavallaro@st.com',
	'rayagond@vayavyalabs.com',
	'vbridgers2013@gmail.com',
	'srinivas.kandagatla@st.com', 'wens@csie.org',
	'netdev@vger.kernel.org',
	'linux-kernel@vger.kernel.org', Ong, Boon Leong

> -----Original Message-----
> From: Kweh, Hock Leong
> Sent: Thursday, September 18, 2014 11:18 AM
> > From: David Miller [mailto:davem@davemloft.net]
> > Sent: Wednesday, September 17, 2014 12:56 PM
> > Are you kidding me?  It's a perfect way to identify this device, it
> > properly uses PCI_CLASS_NETWORK_ETHERNET (0x0200) in both cases and
> > this will not match any other function on this PCI device at all.
> >
> > Please do as I suggested and use the PCI class for the differentiation
> > and matching during probing.
> 
> Hi David,
> 
> Thanks for your feedback so far. Appreciate it.
> 
> Sorry to my poorly written description that may have caused some confusion.
> My sincere apology here.
> Unfortunately, I don't really grasp your idea clearly based on your responses
> which I appreciate them a lot.
> Sorry for the long description below but I hope to clearly pen down my
> thinking process so that you can follow my thinking incrementally without
> being confused.
> 
> So, let's roll back a bit so that with my following description, you can help
> correct me if my understanding of using PCI function ID to differentiate PHY
> port that is associated with each Ethernet controller is wrong:
> 
> The high-level idea about the change that I made for STMMAC IP inside
> Quark is as follow:
> 
> (1) Based on Quark-specific PCI ID declared inside stmmac_id_table[],  the
> probe() function is
>        called to continue setting-up STMMAC for Quark.
> 
> @@ -228,11 +303,13 @@ static int stmmac_pci_resume(struct pci_dev *pdev)
> 
>  #define STMMAC_VENDOR_ID 0x700
>  #define STMMAC_DEVICE_ID 0x1108
> +#define STMMAC_QUARK_X1000_ID 0x0937
> 
>  static const struct pci_device_id stmmac_id_table[] = {
>  	{PCI_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID),
> PCI_ANY_ID,
>  			PCI_ANY_ID, CHIP_STMICRO},
>  	{PCI_VDEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_MAC),
> CHIP_STMICRO},
> +	{PCI_VDEVICE(INTEL, STMMAC_QUARK_X1000_ID),
> CHIP_QUARK_X1000},
>  	{}
>  };
> 
> (2) Back-ground on STMMAC hardware configuration on Intel Galileo Gen 1 &
> Gen 2 platforms:
> Intel Quark SoC has 2 MAC controller as described by lspci output below:
> 
> 00:14.6 Class 0200: 8086:0937    ====> 1st MAC controller
> 00:14.7 Class 0200: 8086:0937    ====> 2nd MAC controller
> 
> These Galileo boards use the same Intel Quark SoC and there is only one PHY
> connect to the 1st MAC [00:14.6 Class 0200: 8086:0937] The 2nd MAC [00:14.7
> Class 0200: 8086:0937] is NOT connected to any PHY at all.
> 
> So, it appears to me that the only way that I can differentiate between 1st &
> 2nd MAC are based on PCI function ID, i.e. 14.6 & 14.7. Therefore, within the
> probe() function, for Intel Quark SoC only, the function performs next-level
> discovery of 1st or 2nd MAC controller through quark_run_time_config()
> function.
> For other PCI ID (currently STMICRO_MAC) there is NO next-level discovery
> involved as rt_config is NULL.
> Changes shown below:
> 
> static struct platform_data platform_info[] = { @@ -59,15 +65,76 @@ static
> struct platform_data platform_info[] = {
>  		.phy_reset = NULL,
>  		.phy_mask = 0,
>  		.pbl = 32,
> +		.fixed_burst = 0,
>  		.burst_len = DMA_AXI_BLEN_256,
> +		.rt_config = NULL,  ===================> no 2nd-level
> discovery for other PCI ID
> +	},
> +	[CHIP_QUARK_X1000] = {
> +		.phy_addr = 1,
> +		.interface = PHY_INTERFACE_MODE_RMII,
> +		.clk_csr = 2,
> +		.has_gmac = 1,
> +		.force_sf_dma_mode = 1,
> +		.multicast_filter_bins = HASH_TABLE_SIZE,
> +		.unicast_filter_entries = 1,
> +		.phy_reset = NULL,
> +		.phy_mask = 0,
> +		.pbl = 16,
> +		.fixed_burst = 1,
> +		.burst_len = DMA_AXI_BLEN_256,
> +		.rt_config = &quark_run_time_config,       ==========>
> Quark specific 2nd-level discovery
> +	},
> +};
> 
> (3) Within quark_run_time_config(), due to the only way to differentiate 1st
> or 2nd MAC controller according to difference in function ID explained above,
> the following changes are made:
> 
> +static void quark_run_time_config(int chip_id, struct pci_dev *pdev) {
> +	const char *board_name =
> dmi_get_system_info(DMI_BOARD_NAME);
> +	int i;
> +	int func_num = PCI_FUNC(pdev->devfn);
> +
> +	if (!board_name)
> +		return;
> +
> +	for (i = 0; i < ARRAY_SIZE(quark_x1000_phy_info); i++) {
> +		if ((!strcmp(quark_x1000_phy_info[i].board_name,
> board_name)) &&
> +		    quark_x1000_phy_info[i].pci_func_num == func_num)
> +			platform_info[chip_id].phy_addr =
> +				quark_x1000_phy_info[i].phy_address;
> +	}
> +}
> 
> The reasons for the above proposed condition checks, i.e. "board name" &
> "pci function name" are below:
>   a) As described above, the only difference in both instance of STMMAC IP
> inside Intel Quark SoC is the function ID,
>        so I have proposed to use function ID to be the decision point here to
> differentiate 1st MAC from 2nd MAC.
>   b) Allow future expansion of any other Intel Quark platforms with specific
> need to fix PHY address
>   c) A PHY address set as "-1"  is to mark that the PHY (associated with
> function ID) is not connected to MAC, which
>       is being used here for the 2 Galileo boards -> 2nd MAC port not connected
> with PHY.
> 
> 
> Finally, based on the above description, it appears to me that using PCI
> function ID to decode seems viable for Intel Quark specific hardware
> configuration.
> 
> Appreciate your time and any feedback is very much appreciated.
> 
> Thanks.
> 
> 
> Regards,
> Wilson


Hi Guys,

Just gently ping for the discussion to carry on before forgetting the context.
Anyone have any better idea or comments or concern to this topic?
Hope the above explanation clear out your doubt.


Regards,
Wilson

^ permalink raw reply

* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Roger Quadros @ 2014-10-01 10:57 UTC (permalink / raw)
  To: Wolfram Sang, Marc Kleine-Budde
  Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <20141001104317.GB1261@katana>

On 10/01/2014 01:43 PM, Wolfram Sang wrote:
> 
>>> Unfortunately it is 5 ;)
>>> We have display IP related bit in between 3 and 5 :P
>>
>> What on earth were the HW engineers thinking????????????
> 
> "Let's test my RNG on the bit-placement of this register" :)

:D

> 
>> ...if we just have the instance parameter in the syscon phandle, we have
>> to put the mapping into the driver, which makes IMHO no sense, because
>> you have to touch the driver, if there is another SoC with the DCAN core.

My guess is that TI won't come up with a 3rd variant so we won't have to
touch the driver, but you never know for sure.

> 
> ... which would be my preferred solution. I think new SoCs should have
> some kind of:
> 
> 	compatible = "commodore,c64ultra", "bosch,d_can";
> 
> in the DT anyhow to allow for SoC specific quirks/adjustments. And
> custom raminit belongs to that IMO (see the ti routine getting more and
> more specific).
> 

Right. For now we need 2 start/stop definations for the existing TI Socs.

but where to store the raminit start/stop bits? The driver_data currently seems to 
contain the CAN type C_CAN vs D_CAN without containing it in a platform_data structure.

Is it OK to create a new platform_data structure for CAN and put the type and raminit start/stop
bits there?

cheers,
-roger

^ permalink raw reply

* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Wolfram Sang @ 2014-10-01 10:43 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Roger Quadros, wg, tony, tglx, mugunthanvnm, george.cherian,
	balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can,
	netdev
In-Reply-To: <542BD6F1.2020900@pengutronix.de>

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


> > Unfortunately it is 5 ;)
> > We have display IP related bit in between 3 and 5 :P
> 
> What on earth were the HW engineers thinking????????????

"Let's test my RNG on the bit-placement of this register" :)

> ...if we just have the instance parameter in the syscon phandle, we have
> to put the mapping into the driver, which makes IMHO no sense, because
> you have to touch the driver, if there is another SoC with the DCAN core.

... which would be my preferred solution. I think new SoCs should have
some kind of:

	compatible = "commodore,c64ultra", "bosch,d_can";

in the DT anyhow to allow for SoC specific quirks/adjustments. And
custom raminit belongs to that IMO (see the ti routine getting more and
more specific).


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

^ permalink raw reply

* Re: [PATCH 32/34] netfilter: bridge: move br_netfilter out of the core
From: Pablo Neira Ayuso @ 2014-10-01 10:33 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netfilter-devel, davem, netdev, fw
In-Reply-To: <20140930085802.15460008@urahara>

On Tue, Sep 30, 2014 at 08:58:02AM -0700, Stephen Hemminger wrote:
> On Mon, 29 Sep 2014 14:39:21 +0200
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> 
> > Jesper reported that br_netfilter always registers the hooks since
> > this is part of the bridge core. This harms performance for people that
> > don't need this.
> > 
> > This patch modularizes br_netfilter so it can be rmmod'ed, thus,
> > the hooks can be unregistered. I think the bridge netfilter should have
> > been a separated module since the beginning, Patrick agreed on that.
> > 
> > Note that this is breaking compatibility for users that expect that
> > bridge netfilter is going to be available after explicitly 'modprobe
> > bridge' or via automatic load through brctl.
> > 
> > However, the damage can be easily undone by modprobing br_netfilter.
> > The bridge core also spots a message to provide a clue to people that
> > didn't notice that this has been deprecated.
> > 
> > On top of that, the plan is that nftables will not rely on this software
> > layer, but integrate the connection tracking into the bridge layer to
> > enable stateful filtering and NAT, which is was bridge netfilter users
> > seem to require.
> > 
> > This patch still keeps the fake_dst_ops in the bridge core, since this
> > is required by when the bridge port is initialized. So we can safely
> > modprobe/rmmod br_netfilter anytime.
> > 
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > Acked-by: Florian Westphal <fw@strlen.de>
> 
> I think this is a good idea but you can't break users.
> 
> We need to figure out a way to autoload br_netfilter module on first use.

Indeed, we've been discussing several possibilities depending, short
summary:

1) Autoload br_netfilter from the configuration path, as it happens
   with many other netfilter modules. However, {ip,ip6,arp}tables are
   decoupled from br_netfilter, so we don't have an obvious way to know if
   you're using them from the bridge.

2) Florian proposed to detect this from the packet path. The idea is
   to move the bridge-nf-call sysctl entries to the bridge core, then if
   you see the first bridged packet and bridge-nf-call is set to 1
   (from sysctl or sysfs), the br_netfilter is requested via workqueue
   and the packets are dropped until the br_netfilter module is in place.
   Then, wait for some time to disable bridge-nf-call by default.

My consideration to not follow 2) was that users only need to
'modprobe br_netfilter' if they really need this. This breakage is not
forcing users to upgrade userspace binaries. Instead, it is asking for
some explicit consent from the user.

Sorry, I should have Cc you since the beginning in this discussion. It
was too late when I realized. I can live with going 2) if you think
it's the way to go.

Let us know if you have a better idea. Thanks!

^ permalink raw reply

* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Marc Kleine-Budde @ 2014-10-01 10:26 UTC (permalink / raw)
  To: Roger Quadros, Wolfram Sang
  Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <542BD3A7.5060200@ti.com>

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

On 10/01/2014 12:12 PM, Roger Quadros wrote:
> On 10/01/2014 01:01 PM, Marc Kleine-Budde wrote:
>> On 10/01/2014 11:06 AM, Roger Quadros wrote:
>>> On 10/01/2014 11:47 AM, Marc Kleine-Budde wrote:
>>>> On 10/01/2014 10:45 AM, Roger Quadros wrote:
>>>>> On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote:
>>>>>> On 09/30/2014 05:25 PM, Wolfram Sang wrote:
>>>>>>>
>>>>>>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for
>>>>>>>> additional parameters. Have a look at:
>>>>>>>>
>>>>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
>>>>>>>>
>>>>>>>> First get the regmap, then the 1st argument is the offset in the regmap,
>>>>>>>> the 2nd and 3rd could be the bits.
>>>>>>>
>>>>>>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit>
>>>>>>> For another driver (the stmmac example): <reg_offset> <reg_shift>
>>>>>>
>>>>>> The DCAN's "reg" is a "reg_offset" as in the stmmc.
>>>>>>
>>>>>> Roger, can we derive both start and done bit from a common reg_shift?
>>>>>
>>>>> I'm sorry I didn't understand what you meant.
>>>>>
>>>>> <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us.
>>>>> Even though reg offset is the same for both the DCAN instances.
>>>>
>>>> What's start bit and stop bit for instance 0 and 1 on that SoC that has
>>>> two instances?
>>>>
>>>
>>> we have 3 SoCs at the moment, all have 2 DCAN instances.
>>>
>>> AM33xx & AM43xx
>>> instance	start	stop
>>> 1		0	8
>>> 2		1	9	
>>
>> If we use a 0-based numbering for the instances:
>> instance	start		stop
>> 0		(0 << instance)	(8 << instance)
>> 1		(0 << instance)	(8 << instance)
>>
> 
> How does the instance number get set? What happens on boards where
> the first instance is unused while the second one is in use?

Via a single "instance" parameter of the syscon phandle....

> 
>>> DRA7xx
>>> instance	start	stop
>>> 1		3	1
>>> 2		5	2
>>                 ^
>> 5 or 4?
> 
> Unfortunately it is 5 ;)
> We have display IP related bit in between 3 and 5 :P

What on earth were the HW engineers thinking????????????

...if we just have the instance parameter in the syscon phandle, we have
to put the mapping into the driver, which makes IMHO no sense, because
you have to touch the driver, if there is another SoC with the DCAN core.

AFAICS we have these options:
1) syscon phandle with three parameters:
   reg offset, start bit shift, stop bit shift
   (the name of the syscon phandle is a different topic)
2) a single ti,start-stop-bit option with two parameter
3) two ti,* entries with a single parameter each
   (as in Roger's initial patch)

Wolfram, which solution do you prefer? I'm in favour of 3 (+ plus a
phandle with a reg offset parameter).

puzzled,
Marc

-- 
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: 181 bytes --]

^ permalink raw reply

* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Roger Quadros @ 2014-10-01 10:12 UTC (permalink / raw)
  To: Marc Kleine-Budde, Wolfram Sang
  Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <542BD116.4090809@pengutronix.de>

On 10/01/2014 01:01 PM, Marc Kleine-Budde wrote:
> On 10/01/2014 11:06 AM, Roger Quadros wrote:
>> On 10/01/2014 11:47 AM, Marc Kleine-Budde wrote:
>>> On 10/01/2014 10:45 AM, Roger Quadros wrote:
>>>> On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote:
>>>>> On 09/30/2014 05:25 PM, Wolfram Sang wrote:
>>>>>>
>>>>>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for
>>>>>>> additional parameters. Have a look at:
>>>>>>>
>>>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
>>>>>>>
>>>>>>> First get the regmap, then the 1st argument is the offset in the regmap,
>>>>>>> the 2nd and 3rd could be the bits.
>>>>>>
>>>>>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit>
>>>>>> For another driver (the stmmac example): <reg_offset> <reg_shift>
>>>>>
>>>>> The DCAN's "reg" is a "reg_offset" as in the stmmc.
>>>>>
>>>>> Roger, can we derive both start and done bit from a common reg_shift?
>>>>
>>>> I'm sorry I didn't understand what you meant.
>>>>
>>>> <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us.
>>>> Even though reg offset is the same for both the DCAN instances.
>>>
>>> What's start bit and stop bit for instance 0 and 1 on that SoC that has
>>> two instances?
>>>
>>
>> we have 3 SoCs at the moment, all have 2 DCAN instances.
>>
>> AM33xx & AM43xx
>> instance	start	stop
>> 1		0	8
>> 2		1	9	
> 
> If we use a 0-based numbering for the instances:
> instance	start		stop
> 0		(0 << instance)	(8 << instance)
> 1		(0 << instance)	(8 << instance)
> 

How does the instance number get set? What happens on boards where the first instance is unused
while the second one is in use?

>> DRA7xx
>> instance	start	stop
>> 1		3	1
>> 2		5	2
>                 ^
> 5 or 4?

Unfortunately it is 5 ;)
We have display IP related bit in between 3 and 5 :P

cheers,
-roger


^ permalink raw reply

* Re: [PATCH net-next v6 2/2] bonding: Simplify the xmit function for modes that use xmit_hash
From: Nikolay Aleksandrov @ 2014-10-01 10:08 UTC (permalink / raw)
  To: Mahesh Bandewar, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
	David Miller
  Cc: netdev, Eric Dumazet, Maciej Zenczykowski
In-Reply-To: <1412152711-12646-1-git-send-email-maheshb@google.com>

On 01/10/14 10:38, Mahesh Bandewar wrote:
> Earlier change to use usable slave array for TLB mode had an additional
> performance advantage. So extending the same logic to all other modes
> that use xmit-hash for slave selection (viz 802.3AD, and XOR modes).
> Also consolidating this with the earlier TLB change.
>
> The main idea is to build the usable slaves array in the control path
> and use that array for slave selection during xmit operation.
>
> Measured performance in a setup with a bond of 4x1G NICs with 200
> instances of netperf for the modes involved (3ad, xor, tlb)
> cmd: netperf -t TCP_RR -H <TargetHost> -l 60 -s 5
>
> Mode        TPS-Before   TPS-After
>
> 802.3ad   : 468,694      493,101
> TLB (lb=0): 392,583      392,965
> XOR       : 475,696      484,517
>
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> ---
> v1:
>    (a) If bond_update_slave_arr() fails to allocate memory, it will overwrite
>        the slave that need to be removed.
>    (b) Freeing of array will assign NULL (to handle bond->down to bond->up
>        transition gracefully.
>    (c) Change from pr_debug() to pr_err() if bond_update_slave_arr() returns
>        failure.
>    (d) XOR: bond_update_slave_arr() will consider mii-mon, arp-mon cases and
>        will populate the array even if these parameters are not used.
>    (e) 3AD: Should handle the ad_agg_selection_logic correctly.
> v2:
>    (a) Removed rcu_read_{un}lock() calls from array manipulation code.
>    (b) Slave link-events now refresh array for all these modes.
>    (c) Moved free-array call from bond_close() to bond_uninit().
> v3:
>    (a) Fixed null pointer dereference.
>    (b) Removed bond->lock lockdep dependency.
> v4:
>    (a) Made to changes to comply with Nikolay's locking changes
>    (b) Added a work-queue to refresh slave-array when RTNL is not held
>    (c) Array refresh happens ONLY with RTNL now.
>    (d) alloc changed from GFP_ATOMIC to GFP_KERNEL
> v5:
>    (a) Consolidated all delayed slave-array updates at one place in
>        3ad_state_machine_handler()
> v6:
>    (a) Free slave array when there is no active aggregator
>
>   drivers/net/bonding/bond_3ad.c  | 140 +++++++++++------------------
>   drivers/net/bonding/bond_alb.c  |  51 ++---------
>   drivers/net/bonding/bond_alb.h  |   8 --
>   drivers/net/bonding/bond_main.c | 192 +++++++++++++++++++++++++++++++++++++---
>   drivers/net/bonding/bonding.h   |  10 +++
>   5 files changed, 249 insertions(+), 152 deletions(-)
>
<<<snip>>>
> @@ -3869,6 +4029,12 @@ static void bond_uninit(struct net_device *bond_dev)
>   		__bond_release_one(bond_dev, slave->dev, true);
>   	netdev_info(bond_dev, "Released all slaves\n");
>
> +	arr = rtnl_dereference(bond->slave_arr);
> +	if (arr) {
> +		kfree_rcu(arr, rcu);
> +		RCU_INIT_POINTER(bond->slave_arr, NULL);
> +	}
> +
>   	list_del(&bond->bond_list);
>
>   	bond_debug_unregister(bond);
<<<snip>>>
I'm fine with this version, just one last question about something I just 
noticed in the hunk above:
You first call kfree_rcu() and then RCU_INIT_POINTER(). This feels wrong as the 
currently used slave_arr can get freed before it's set to NULL if we get 
preempted after the kfree_rcu(). Now, I know it's not really a problem because 
at this point the bond device has been closed and shouldn't operate, but just in 
case I think it'd be nice to first NULL it and call kfree_rcu() after that.

Thanks for all your hard work on this.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>

^ permalink raw reply

* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Marc Kleine-Budde @ 2014-10-01 10:01 UTC (permalink / raw)
  To: Roger Quadros, Wolfram Sang
  Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <542BC42F.2010406@ti.com>

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

On 10/01/2014 11:06 AM, Roger Quadros wrote:
> On 10/01/2014 11:47 AM, Marc Kleine-Budde wrote:
>> On 10/01/2014 10:45 AM, Roger Quadros wrote:
>>> On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote:
>>>> On 09/30/2014 05:25 PM, Wolfram Sang wrote:
>>>>>
>>>>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for
>>>>>> additional parameters. Have a look at:
>>>>>>
>>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
>>>>>>
>>>>>> First get the regmap, then the 1st argument is the offset in the regmap,
>>>>>> the 2nd and 3rd could be the bits.
>>>>>
>>>>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit>
>>>>> For another driver (the stmmac example): <reg_offset> <reg_shift>
>>>>
>>>> The DCAN's "reg" is a "reg_offset" as in the stmmc.
>>>>
>>>> Roger, can we derive both start and done bit from a common reg_shift?
>>>
>>> I'm sorry I didn't understand what you meant.
>>>
>>> <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us.
>>> Even though reg offset is the same for both the DCAN instances.
>>
>> What's start bit and stop bit for instance 0 and 1 on that SoC that has
>> two instances?
>>
> 
> we have 3 SoCs at the moment, all have 2 DCAN instances.
> 
> AM33xx & AM43xx
> instance	start	stop
> 1		0	8
> 2		1	9	

If we use a 0-based numbering for the instances:
instance	start		stop
0		(0 << instance)	(8 << instance)
1		(0 << instance)	(8 << instance)

> DRA7xx
> instance	start	stop
> 1		3	1
> 2		5	2
                ^
5 or 4?

Marc

-- 
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: 181 bytes --]

^ permalink raw reply

* Re: [PATCH RFC ipsec-next] xfrm: Add sysctl option to enforce inbound policies for transport mode
From: Steffen Klassert @ 2014-10-01  9:58 UTC (permalink / raw)
  To: Tobias Brunner; +Cc: davem, netdev, Herbert Xu
In-Reply-To: <54296113.3080106@strongswan.org>

On Mon, Sep 29, 2014 at 03:39:31PM +0200, Tobias Brunner wrote:
> > 
> > commit 8fe7ee2ba983fd89b2555dce5930ffd0f7f6c361
> > Author: Herbert Xu <herbert@gondor.apana.org.au>
> > Date:   Thu Oct 23 14:57:11 2003 -0700
> > 
> >     [IPSEC]: Strengthen policy checks.
> > 
> > Maybe Herbert remembers why this was done only for tunnel mode.
> 
> Would be great to hear from Herbert about this.
> 
> > If I read section 5.2.1 of RFC 2401 correct, the inbound policy
> > must be enforced regardless of the mode.
> 
> It looks like the wording changed with RFC 4301.  

RFC 4301 did not yet exist when this code change was made,
so I tried to find the reason for that in RFC 2401.

> The SPD and its
> policies are not mentioned explicitly anymore in section 5.2 (like
> they were in step 3 in section 5.2.1 of RFC 2401).  Instead, packets
> must be matched against the "selectors identified by the SAD entry".
> It's not entirely clear to me whether these selectors are part of the
> SPD or properties of the SAD entries themselves, like the single
> selector that can currently be configured on SAs in the kernel.

It reads like the packets must match against the selectors of the
used SA. That's what we do at the beginning of __xfrm_policy_check().

This was already required in RFC 2401. But there was a matching policy
required too, seems this is not necessary anymore with RFC 4301.

Instead they recommend: "Every SPD SHOULD have a nominal, final entry
that catches anything that is otherwise unmatched, and discards it."

> Also, section 4.4.2 explicitly states that manually keyed SAD entries
> do not necessarily need to have a corresponding SPD entry.  Which might
> make sense for simple host-host (transport mode) SAs, but this wouldn't
> be possible anymore when enforcing inbound policies for transport mode.

Well, this states just that such SAD entries can exist. But does not
say that packets transformed by such a SA are allowed to pass without
matching policy.

> 
> Subject: [PATCH] xfrm: Enforce inbound transport mode policies like those in other modes
> 
> Currently inbound policies for transport mode SAs are not enforced.
> If no policy is found or if the templates don't match this is not
> considered an error for transport mode SAs.

If we find a policy, the templates should match. We need to fix this.
But it seems our policy enforcement for tunnel mode is too strict if
no policy is found. So I think we should leave transport mode as
it is here.

^ permalink raw reply

* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Roger Quadros @ 2014-10-01  9:06 UTC (permalink / raw)
  To: Marc Kleine-Budde, Wolfram Sang
  Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <542BBFBE.90406@pengutronix.de>

On 10/01/2014 11:47 AM, Marc Kleine-Budde wrote:
> On 10/01/2014 10:45 AM, Roger Quadros wrote:
>> On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote:
>>> On 09/30/2014 05:25 PM, Wolfram Sang wrote:
>>>>
>>>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for
>>>>> additional parameters. Have a look at:
>>>>>
>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
>>>>>
>>>>> First get the regmap, then the 1st argument is the offset in the regmap,
>>>>> the 2nd and 3rd could be the bits.
>>>>
>>>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit>
>>>> For another driver (the stmmac example): <reg_offset> <reg_shift>
>>>
>>> The DCAN's "reg" is a "reg_offset" as in the stmmc.
>>>
>>> Roger, can we derive both start and done bit from a common reg_shift?
>>
>> I'm sorry I didn't understand what you meant.
>>
>> <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us.
>> Even though reg offset is the same for both the DCAN instances.
> 
> What's start bit and stop bit for instance 0 and 1 on that SoC that has
> two instances??
> 

we have 3 SoCs at the moment, all have 2 DCAN instances.

AM33xx & AM43xx
instance	start	stop
1		0	8
2		1	9	

DRA7xx
instance	start	stop
1		3	1
2		5	2

cheers,
-roger

^ permalink raw reply

* [PATCH v2] net: ll_temac: Remove unnecessary ether_setup after alloc_etherdev
From: Michal Simek @ 2014-10-01  9:01 UTC (permalink / raw)
  To: linux-kernel, monstr
  Cc: Subbaraya Sundeep Bhatta, David S. Miller, Manuel Schölling,
	Paul Gortmaker, Julia Lawall, Joe Perches, Ricardo Ribalda,
	Eric Dumazet, netdev, linux-arm-kernel

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

From: Subbaraya Sundeep Bhatta <subbaraya.sundeep.bhatta@xilinx.com>

Calling ether_setup is redundant since alloc_etherdev calls it.

Signed-off-by: Subbaraya Sundeep Bhatta <sbhatta@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
- Remove axienet because it is already applied in separate patch
  https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=c706471b2601d1c9058e7b866db77f6eb7dd37af

 drivers/net/ethernet/xilinx/ll_temac_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c
index fda5891835d4..629077050fce 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_main.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
@@ -1012,7 +1012,6 @@ static int temac_of_probe(struct platform_device *op)
 	if (!ndev)
 		return -ENOMEM;

-	ether_setup(ndev);
 	platform_set_drvdata(op, ndev);
 	SET_NETDEV_DEV(ndev, &op->dev);
 	ndev->flags &= ~IFF_MULTICAST;  /* clear multicast */
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply related

* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Marc Kleine-Budde @ 2014-10-01  8:47 UTC (permalink / raw)
  To: Roger Quadros, Wolfram Sang
  Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <542BBF3F.2040803@ti.com>

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

On 10/01/2014 10:45 AM, Roger Quadros wrote:
> On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote:
>> On 09/30/2014 05:25 PM, Wolfram Sang wrote:
>>>
>>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for
>>>> additional parameters. Have a look at:
>>>>
>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
>>>>
>>>> First get the regmap, then the 1st argument is the offset in the regmap,
>>>> the 2nd and 3rd could be the bits.
>>>
>>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit>
>>> For another driver (the stmmac example): <reg_offset> <reg_shift>
>>
>> The DCAN's "reg" is a "reg_offset" as in the stmmc.
>>
>> Roger, can we derive both start and done bit from a common reg_shift?
> 
> I'm sorry I didn't understand what you meant.
> 
> <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us.
> Even though reg offset is the same for both the DCAN instances.

What's start bit and stop bit for instance 0 and 1 on that SoC that has
two instances??

Marc

-- 
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: 181 bytes --]

^ permalink raw reply

* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Roger Quadros @ 2014-10-01  8:45 UTC (permalink / raw)
  To: Marc Kleine-Budde, Wolfram Sang
  Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <542AD483.2020808@pengutronix.de>

On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote:
> On 09/30/2014 05:25 PM, Wolfram Sang wrote:
>>
>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for
>>> additional parameters. Have a look at:
>>>
>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
>>>
>>> First get the regmap, then the 1st argument is the offset in the regmap,
>>> the 2nd and 3rd could be the bits.
>>
>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit>
>> For another driver (the stmmac example): <reg_offset> <reg_shift>
> 
> The DCAN's "reg" is a "reg_offset" as in the stmmc.
> 
> Roger, can we derive both start and done bit from a common reg_shift?

I'm sorry I didn't understand what you meant.

<&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us.
Even though reg offset is the same for both the DCAN instances.

> 
>> Phew... Then we should really have a "syscon-raminit" property probably,
>> so that at least plain "syscon" has a consistent syntax?
> 
> I think^whope we can have the same syntax as the stmmc :D

I agree too.

> 
>>>> So, I'd rather drop additional arguments.
>>>>
>>>> Why would you like to have it encoded in DT?
>>>
>>> Where put the information then? Into the driver, but where do you get
>>> the reference which instance of the DCAN you are, so that you can look
>>> up the correct bits?
>>
>> Agreed. I thought we had this information in the driver already, but we
>> haven't...
>>
> 
> The current driver relies on the of_alias_get_id(), which isn't
> considered best practice, is it? So I want to avoid this when switching
> to syscon.

cheers,
-roger

^ permalink raw reply

* [PATCH net-next v6 2/2] bonding: Simplify the xmit function for modes that use xmit_hash
From: Mahesh Bandewar @ 2014-10-01  8:38 UTC (permalink / raw)
  To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David Miller
  Cc: netdev, Mahesh Bandewar, Eric Dumazet, Maciej Zenczykowski

Earlier change to use usable slave array for TLB mode had an additional
performance advantage. So extending the same logic to all other modes
that use xmit-hash for slave selection (viz 802.3AD, and XOR modes).
Also consolidating this with the earlier TLB change.

The main idea is to build the usable slaves array in the control path
and use that array for slave selection during xmit operation.

Measured performance in a setup with a bond of 4x1G NICs with 200
instances of netperf for the modes involved (3ad, xor, tlb)
cmd: netperf -t TCP_RR -H <TargetHost> -l 60 -s 5

Mode        TPS-Before   TPS-After

802.3ad   : 468,694      493,101
TLB (lb=0): 392,583      392,965
XOR       : 475,696      484,517

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
v1:
  (a) If bond_update_slave_arr() fails to allocate memory, it will overwrite
      the slave that need to be removed.
  (b) Freeing of array will assign NULL (to handle bond->down to bond->up
      transition gracefully.
  (c) Change from pr_debug() to pr_err() if bond_update_slave_arr() returns
      failure.
  (d) XOR: bond_update_slave_arr() will consider mii-mon, arp-mon cases and
      will populate the array even if these parameters are not used.
  (e) 3AD: Should handle the ad_agg_selection_logic correctly.
v2:
  (a) Removed rcu_read_{un}lock() calls from array manipulation code.
  (b) Slave link-events now refresh array for all these modes.
  (c) Moved free-array call from bond_close() to bond_uninit().
v3:
  (a) Fixed null pointer dereference.
  (b) Removed bond->lock lockdep dependency.
v4:
  (a) Made to changes to comply with Nikolay's locking changes
  (b) Added a work-queue to refresh slave-array when RTNL is not held
  (c) Array refresh happens ONLY with RTNL now.
  (d) alloc changed from GFP_ATOMIC to GFP_KERNEL
v5:
  (a) Consolidated all delayed slave-array updates at one place in
      3ad_state_machine_handler()
v6:
  (a) Free slave array when there is no active aggregator

 drivers/net/bonding/bond_3ad.c  | 140 +++++++++++------------------
 drivers/net/bonding/bond_alb.c  |  51 ++---------
 drivers/net/bonding/bond_alb.h  |   8 --
 drivers/net/bonding/bond_main.c | 192 +++++++++++++++++++++++++++++++++++++---
 drivers/net/bonding/bonding.h   |  10 +++
 5 files changed, 249 insertions(+), 152 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 7e9e522fd476..2110215f3528 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -102,17 +102,20 @@ static const u8 lacpdu_mcast_addr[ETH_ALEN] = MULTICAST_LACPDU_ADDR;
 /* ================= main 802.3ad protocol functions ================== */
 static int ad_lacpdu_send(struct port *port);
 static int ad_marker_send(struct port *port, struct bond_marker *marker);
-static void ad_mux_machine(struct port *port);
+static void ad_mux_machine(struct port *port, bool *update_slave_arr);
 static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port);
 static void ad_tx_machine(struct port *port);
 static void ad_periodic_machine(struct port *port);
-static void ad_port_selection_logic(struct port *port);
-static void ad_agg_selection_logic(struct aggregator *aggregator);
+static void ad_port_selection_logic(struct port *port, bool *update_slave_arr);
+static void ad_agg_selection_logic(struct aggregator *aggregator,
+				   bool *update_slave_arr);
 static void ad_clear_agg(struct aggregator *aggregator);
 static void ad_initialize_agg(struct aggregator *aggregator);
 static void ad_initialize_port(struct port *port, int lacp_fast);
-static void ad_enable_collecting_distributing(struct port *port);
-static void ad_disable_collecting_distributing(struct port *port);
+static void ad_enable_collecting_distributing(struct port *port,
+					      bool *update_slave_arr);
+static void ad_disable_collecting_distributing(struct port *port,
+					       bool *update_slave_arr);
 static void ad_marker_info_received(struct bond_marker *marker_info,
 				    struct port *port);
 static void ad_marker_response_received(struct bond_marker *marker,
@@ -796,8 +799,9 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
 /**
  * ad_mux_machine - handle a port's mux state machine
  * @port: the port we're looking at
+ * @update_slave_arr: Does slave array need update?
  */
-static void ad_mux_machine(struct port *port)
+static void ad_mux_machine(struct port *port, bool *update_slave_arr)
 {
 	mux_states_t last_state;
 
@@ -901,7 +905,8 @@ static void ad_mux_machine(struct port *port)
 		switch (port->sm_mux_state) {
 		case AD_MUX_DETACHED:
 			port->actor_oper_port_state &= ~AD_STATE_SYNCHRONIZATION;
-			ad_disable_collecting_distributing(port);
+			ad_disable_collecting_distributing(port,
+							   update_slave_arr);
 			port->actor_oper_port_state &= ~AD_STATE_COLLECTING;
 			port->actor_oper_port_state &= ~AD_STATE_DISTRIBUTING;
 			port->ntt = true;
@@ -913,13 +918,15 @@ static void ad_mux_machine(struct port *port)
 			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);
+			ad_disable_collecting_distributing(port,
+							   update_slave_arr);
 			port->ntt = true;
 			break;
 		case AD_MUX_COLLECTING_DISTRIBUTING:
 			port->actor_oper_port_state |= AD_STATE_COLLECTING;
 			port->actor_oper_port_state |= AD_STATE_DISTRIBUTING;
-			ad_enable_collecting_distributing(port);
+			ad_enable_collecting_distributing(port,
+							  update_slave_arr);
 			port->ntt = true;
 			break;
 		default:
@@ -1187,12 +1194,13 @@ static void ad_periodic_machine(struct port *port)
 /**
  * ad_port_selection_logic - select aggregation groups
  * @port: the port we're looking at
+ * @update_slave_arr: Does slave array need update?
  *
  * Select aggregation groups, and assign each port for it's aggregetor. The
  * selection logic is called in the inititalization (after all the handshkes),
  * and after every lacpdu receive (if selected is off).
  */
-static void ad_port_selection_logic(struct port *port)
+static void ad_port_selection_logic(struct port *port, bool *update_slave_arr)
 {
 	struct aggregator *aggregator, *free_aggregator = NULL, *temp_aggregator;
 	struct port *last_port = NULL, *curr_port;
@@ -1347,7 +1355,7 @@ static void ad_port_selection_logic(struct port *port)
 			      __agg_ports_are_ready(port->aggregator));
 
 	aggregator = __get_first_agg(port);
-	ad_agg_selection_logic(aggregator);
+	ad_agg_selection_logic(aggregator, update_slave_arr);
 }
 
 /* Decide if "agg" is a better choice for the new active aggregator that
@@ -1435,6 +1443,7 @@ static int agg_device_up(const struct aggregator *agg)
 /**
  * ad_agg_selection_logic - select an aggregation group for a team
  * @aggregator: the aggregator we're looking at
+ * @update_slave_arr: Does slave array need update?
  *
  * It is assumed that only one aggregator may be selected for a team.
  *
@@ -1457,7 +1466,8 @@ static int agg_device_up(const struct aggregator *agg)
  * __get_active_agg() won't work correctly. This function should be better
  * called with the bond itself, and retrieve the first agg from it.
  */
-static void ad_agg_selection_logic(struct aggregator *agg)
+static void ad_agg_selection_logic(struct aggregator *agg,
+				   bool *update_slave_arr)
 {
 	struct aggregator *best, *active, *origin;
 	struct bonding *bond = agg->slave->bond;
@@ -1550,6 +1560,8 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 				__disable_port(port);
 			}
 		}
+		/* Slave array needs update. */
+		*update_slave_arr = true;
 	}
 
 	/* if the selected aggregator is of join individuals
@@ -1678,24 +1690,30 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
 /**
  * ad_enable_collecting_distributing - enable a port's transmit/receive
  * @port: the port we're looking at
+ * @update_slave_arr: Does slave array need update?
  *
  * Enable @port if it's in an active aggregator
  */
-static void ad_enable_collecting_distributing(struct port *port)
+static void ad_enable_collecting_distributing(struct port *port,
+					      bool *update_slave_arr)
 {
 	if (port->aggregator->is_active) {
 		pr_debug("Enabling port %d(LAG %d)\n",
 			 port->actor_port_number,
 			 port->aggregator->aggregator_identifier);
 		__enable_port(port);
+		/* Slave array needs update */
+		*update_slave_arr = true;
 	}
 }
 
 /**
  * ad_disable_collecting_distributing - disable a port's transmit/receive
  * @port: the port we're looking at
+ * @update_slave_arr: Does slave array need update?
  */
-static void ad_disable_collecting_distributing(struct port *port)
+static void ad_disable_collecting_distributing(struct port *port,
+					       bool *update_slave_arr)
 {
 	if (port->aggregator &&
 	    !MAC_ADDRESS_EQUAL(&(port->aggregator->partner_system),
@@ -1704,6 +1722,8 @@ static void ad_disable_collecting_distributing(struct port *port)
 			 port->actor_port_number,
 			 port->aggregator->aggregator_identifier);
 		__disable_port(port);
+		/* Slave array needs an update */
+		*update_slave_arr = true;
 	}
 }
 
@@ -1868,6 +1888,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 	struct bonding *bond = slave->bond;
 	struct slave *slave_iter;
 	struct list_head *iter;
+	bool dummy_slave_update; /* Ignore this value as caller updates array */
 
 	/* Sync against bond_3ad_state_machine_handler() */
 	spin_lock_bh(&bond->mode_lock);
@@ -1951,7 +1972,8 @@ void bond_3ad_unbind_slave(struct slave *slave)
 				ad_clear_agg(aggregator);
 
 				if (select_new_active_agg)
-					ad_agg_selection_logic(__get_first_agg(port));
+					ad_agg_selection_logic(__get_first_agg(port),
+							       &dummy_slave_update);
 			} else {
 				netdev_warn(bond->dev, "unbinding aggregator, and could not find a new aggregator for its ports\n");
 			}
@@ -1966,7 +1988,8 @@ void bond_3ad_unbind_slave(struct slave *slave)
 				/* select new active aggregator */
 				temp_aggregator = __get_first_agg(port);
 				if (temp_aggregator)
-					ad_agg_selection_logic(temp_aggregator);
+					ad_agg_selection_logic(temp_aggregator,
+							       &dummy_slave_update);
 			}
 		}
 	}
@@ -1996,7 +2019,8 @@ void bond_3ad_unbind_slave(struct slave *slave)
 					if (select_new_active_agg) {
 						netdev_info(bond->dev, "Removing an active aggregator\n");
 						/* select new active aggregator */
-						ad_agg_selection_logic(__get_first_agg(port));
+						ad_agg_selection_logic(__get_first_agg(port),
+							               &dummy_slave_update);
 					}
 				}
 				break;
@@ -2031,6 +2055,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 	struct slave *slave;
 	struct port *port;
 	bool should_notify_rtnl = BOND_SLAVE_NOTIFY_LATER;
+	bool update_slave_arr = false;
 
 	/* Lock to protect data accessed by all (e.g., port->sm_vars) and
 	 * against running with bond_3ad_unbind_slave. ad_rx_machine may run
@@ -2058,7 +2083,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 			}
 
 			aggregator = __get_first_agg(port);
-			ad_agg_selection_logic(aggregator);
+			ad_agg_selection_logic(aggregator, &update_slave_arr);
 		}
 		bond_3ad_set_carrier(bond);
 	}
@@ -2074,8 +2099,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 
 		ad_rx_machine(NULL, port);
 		ad_periodic_machine(port);
-		ad_port_selection_logic(port);
-		ad_mux_machine(port);
+		ad_port_selection_logic(port, &update_slave_arr);
+		ad_mux_machine(port, &update_slave_arr);
 		ad_tx_machine(port);
 
 		/* turn off the BEGIN bit, since we already handled it */
@@ -2093,6 +2118,9 @@ re_arm:
 	rcu_read_unlock();
 	spin_unlock_bh(&bond->mode_lock);
 
+	if (update_slave_arr)
+		bond_slave_arr_work_rearm(bond, 0);
+
 	if (should_notify_rtnl && rtnl_trylock()) {
 		bond_slave_state_notify(bond);
 		rtnl_unlock();
@@ -2283,6 +2311,11 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
 	port->sm_vars |= AD_PORT_BEGIN;
 
 	spin_unlock_bh(&slave->bond->mode_lock);
+
+	/* RTNL is held and mode_lock is released so it's safe
+	 * to update slave_array here.
+	 */
+	bond_update_slave_arr(slave->bond, NULL);
 }
 
 /**
@@ -2377,73 +2410,6 @@ int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
 	return ret;
 }
 
-int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
-{
-	struct bonding *bond = netdev_priv(dev);
-	struct slave *slave, *first_ok_slave;
-	struct aggregator *agg;
-	struct ad_info ad_info;
-	struct list_head *iter;
-	int slaves_in_agg;
-	int slave_agg_no;
-	int agg_id;
-
-	if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
-		netdev_dbg(dev, "__bond_3ad_get_active_agg_info failed\n");
-		goto err_free;
-	}
-
-	slaves_in_agg = ad_info.ports;
-	agg_id = ad_info.aggregator_id;
-
-	if (slaves_in_agg == 0) {
-		netdev_dbg(dev, "active aggregator is empty\n");
-		goto err_free;
-	}
-
-	slave_agg_no = bond_xmit_hash(bond, skb) % slaves_in_agg;
-	first_ok_slave = NULL;
-
-	bond_for_each_slave_rcu(bond, slave, iter) {
-		agg = SLAVE_AD_INFO(slave)->port.aggregator;
-		if (!agg || agg->aggregator_identifier != agg_id)
-			continue;
-
-		if (slave_agg_no >= 0) {
-			if (!first_ok_slave && bond_slave_can_tx(slave))
-				first_ok_slave = slave;
-			slave_agg_no--;
-			continue;
-		}
-
-		if (bond_slave_can_tx(slave)) {
-			bond_dev_queue_xmit(bond, skb, slave->dev);
-			goto out;
-		}
-	}
-
-	if (slave_agg_no >= 0) {
-		netdev_err(dev, "Couldn't find a slave to tx on for aggregator ID %d\n",
-			   agg_id);
-		goto err_free;
-	}
-
-	/* we couldn't find any suitable slave after the agg_no, so use the
-	 * first suitable found, if found.
-	 */
-	if (first_ok_slave)
-		bond_dev_queue_xmit(bond, skb, first_ok_slave->dev);
-	else
-		goto err_free;
-
-out:
-	return NETDEV_TX_OK;
-err_free:
-	/* no suitable interface, frame not sent */
-	dev_kfree_skb_any(skb);
-	goto out;
-}
-
 int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
 			 struct slave *slave)
 {
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 615f3bebd019..d2eadab787c5 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -177,7 +177,6 @@ static int tlb_initialize(struct bonding *bond)
 static void tlb_deinitialize(struct bonding *bond)
 {
 	struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
-	struct tlb_up_slave *arr;
 
 	spin_lock_bh(&bond->mode_lock);
 
@@ -185,10 +184,6 @@ static void tlb_deinitialize(struct bonding *bond)
 	bond_info->tx_hashtbl = NULL;
 
 	spin_unlock_bh(&bond->mode_lock);
-
-	arr = rtnl_dereference(bond_info->slave_arr);
-	if (arr)
-		kfree_rcu(arr, rcu);
 }
 
 static long long compute_gap(struct slave *slave)
@@ -1336,39 +1331,9 @@ out:
 	return NETDEV_TX_OK;
 }
 
-static int bond_tlb_update_slave_arr(struct bonding *bond,
-				     struct slave *skipslave)
-{
-	struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
-	struct slave *tx_slave;
-	struct list_head *iter;
-	struct tlb_up_slave *new_arr, *old_arr;
-
-	new_arr = kzalloc(offsetof(struct tlb_up_slave, arr[bond->slave_cnt]),
-			  GFP_ATOMIC);
-	if (!new_arr)
-		return -ENOMEM;
-
-	bond_for_each_slave(bond, tx_slave, iter) {
-		if (!bond_slave_can_tx(tx_slave))
-			continue;
-		if (skipslave == tx_slave)
-			continue;
-		new_arr->arr[new_arr->count++] = tx_slave;
-	}
-
-	old_arr = rtnl_dereference(bond_info->slave_arr);
-	rcu_assign_pointer(bond_info->slave_arr, new_arr);
-	if (old_arr)
-		kfree_rcu(old_arr, rcu);
-
-	return 0;
-}
-
 int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
-	struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
 	struct ethhdr *eth_data;
 	struct slave *tx_slave = NULL;
 	u32 hash_index;
@@ -1389,12 +1354,14 @@ int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
 							      hash_index & 0xFF,
 							      skb->len);
 			} else {
-				struct tlb_up_slave *slaves;
+				struct bond_up_slave *slaves;
+				unsigned int count;
 
-				slaves = rcu_dereference(bond_info->slave_arr);
-				if (slaves && slaves->count)
+				slaves = rcu_dereference(bond->slave_arr);
+				count = slaves ? ACCESS_ONCE(slaves->count) : 0;
+				if (likely(count))
 					tx_slave = slaves->arr[hash_index %
-							       slaves->count];
+							       count];
 			}
 			break;
 		}
@@ -1641,10 +1608,6 @@ void bond_alb_deinit_slave(struct bonding *bond, struct slave *slave)
 		rlb_clear_slave(bond, slave);
 	}
 
-	if (bond_is_nondyn_tlb(bond))
-		if (bond_tlb_update_slave_arr(bond, slave))
-			pr_err("Failed to build slave-array for TLB mode.\n");
-
 }
 
 void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char link)
@@ -1669,7 +1632,7 @@ void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char
 	}
 
 	if (bond_is_nondyn_tlb(bond)) {
-		if (bond_tlb_update_slave_arr(bond, NULL))
+		if (bond_update_slave_arr(bond, NULL))
 			pr_err("Failed to build slave-array for TLB mode.\n");
 	}
 }
diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
index 3c6a7ff974d7..1ad473b4ade5 100644
--- a/drivers/net/bonding/bond_alb.h
+++ b/drivers/net/bonding/bond_alb.h
@@ -139,19 +139,11 @@ struct tlb_slave_info {
 			 */
 };
 
-struct tlb_up_slave {
-	unsigned int	count;
-	struct rcu_head rcu;
-	struct slave	*arr[0];
-};
-
 struct alb_bond_info {
 	struct tlb_client_info	*tx_hashtbl; /* Dynamically allocated */
 	u32			unbalanced_load;
 	int			tx_rebalance_counter;
 	int			lp_counter;
-	/* -------- non-dynamic tlb mode only ---------*/
-	struct tlb_up_slave __rcu *slave_arr;	  /* Up slaves */
 	/* -------- rlb parameters -------- */
 	int rlb_enabled;
 	struct rlb_client_info	*rx_hashtbl;	/* Receive hash table */
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c2adc2755ff6..6f79f495b01a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -210,6 +210,7 @@ static int bond_init(struct net_device *bond_dev);
 static void bond_uninit(struct net_device *bond_dev);
 static struct rtnl_link_stats64 *bond_get_stats(struct net_device *bond_dev,
 						struct rtnl_link_stats64 *stats);
+static void bond_slave_arr_handler(struct work_struct *work);
 
 /*---------------------------- General routines -----------------------------*/
 
@@ -1551,6 +1552,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 		unblock_netpoll_tx();
 	}
 
+	if (bond_mode_uses_xmit_hash(bond))
+		bond_update_slave_arr(bond, NULL);
+
 	netdev_info(bond_dev, "Enslaving %s as %s interface with %s link\n",
 		    slave_dev->name,
 		    bond_is_active_slave(new_slave) ? "an active" : "a backup",
@@ -1668,6 +1672,9 @@ static int __bond_release_one(struct net_device *bond_dev,
 	if (BOND_MODE(bond) == BOND_MODE_8023AD)
 		bond_3ad_unbind_slave(slave);
 
+	if (bond_mode_uses_xmit_hash(bond))
+		bond_update_slave_arr(bond, slave);
+
 	netdev_info(bond_dev, "Releasing %s interface %s\n",
 		    bond_is_active_slave(slave) ? "active" : "backup",
 		    slave_dev->name);
@@ -1970,6 +1977,9 @@ static void bond_miimon_commit(struct bonding *bond)
 				bond_alb_handle_link_change(bond, slave,
 							    BOND_LINK_UP);
 
+			if (BOND_MODE(bond) == BOND_MODE_XOR)
+				bond_update_slave_arr(bond, NULL);
+
 			if (!bond->curr_active_slave || slave == primary)
 				goto do_failover;
 
@@ -1997,6 +2007,9 @@ static void bond_miimon_commit(struct bonding *bond)
 				bond_alb_handle_link_change(bond, slave,
 							    BOND_LINK_DOWN);
 
+			if (BOND_MODE(bond) == BOND_MODE_XOR)
+				bond_update_slave_arr(bond, NULL);
+
 			if (slave == rcu_access_pointer(bond->curr_active_slave))
 				goto do_failover;
 
@@ -2453,6 +2466,8 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
 
 		if (slave_state_changed) {
 			bond_slave_state_change(bond);
+			if (BOND_MODE(bond) == BOND_MODE_XOR)
+				bond_update_slave_arr(bond, NULL);
 		} else if (do_failover) {
 			block_netpoll_tx();
 			bond_select_active_slave(bond);
@@ -2829,8 +2844,20 @@ static int bond_slave_netdev_event(unsigned long event,
 			if (old_duplex != slave->duplex)
 				bond_3ad_adapter_duplex_changed(slave);
 		}
+		/* Refresh slave-array if applicable!
+		 * If the setup does not use miimon or arpmon (mode-specific!),
+		 * then these events will not cause the slave-array to be
+		 * refreshed. This will cause xmit to use a slave that is not
+		 * usable. Avoid such situation by refeshing the array at these
+		 * events. If these (miimon/arpmon) parameters are configured
+		 * then array gets refreshed twice and that should be fine!
+		 */
+		if (bond_mode_uses_xmit_hash(bond))
+			bond_update_slave_arr(bond, NULL);
 		break;
 	case NETDEV_DOWN:
+		if (bond_mode_uses_xmit_hash(bond))
+			bond_update_slave_arr(bond, NULL);
 		break;
 	case NETDEV_CHANGEMTU:
 		/* TODO: Should slaves be allowed to
@@ -3010,6 +3037,7 @@ static void bond_work_init_all(struct bonding *bond)
 	else
 		INIT_DELAYED_WORK(&bond->arp_work, bond_loadbalance_arp_mon);
 	INIT_DELAYED_WORK(&bond->ad_work, bond_3ad_state_machine_handler);
+	INIT_DELAYED_WORK(&bond->slave_arr_work, bond_slave_arr_handler);
 }
 
 static void bond_work_cancel_all(struct bonding *bond)
@@ -3019,6 +3047,7 @@ static void bond_work_cancel_all(struct bonding *bond)
 	cancel_delayed_work_sync(&bond->alb_work);
 	cancel_delayed_work_sync(&bond->ad_work);
 	cancel_delayed_work_sync(&bond->mcast_work);
+	cancel_delayed_work_sync(&bond->slave_arr_work);
 }
 
 static int bond_open(struct net_device *bond_dev)
@@ -3068,6 +3097,9 @@ static int bond_open(struct net_device *bond_dev)
 		bond_3ad_initiate_agg_selection(bond, 1);
 	}
 
+	if (bond_mode_uses_xmit_hash(bond))
+		bond_update_slave_arr(bond, NULL);
+
 	return 0;
 }
 
@@ -3573,20 +3605,148 @@ static int bond_xmit_activebackup(struct sk_buff *skb, struct net_device *bond_d
 	return NETDEV_TX_OK;
 }
 
-/* In bond_xmit_xor() , we determine the output device by using a pre-
- * determined xmit_hash_policy(), If the selected device is not enabled,
- * find the next active slave.
+/* Use this to update slave_array when (a) it's not appropriate to update
+ * slave_array right away (note that update_slave_array() may sleep)
+ * and / or (b) RTNL is not held.
  */
-static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
+void bond_slave_arr_work_rearm(struct bonding *bond, unsigned long delay)
 {
-	struct bonding *bond = netdev_priv(bond_dev);
-	int slave_cnt = ACCESS_ONCE(bond->slave_cnt);
+	queue_delayed_work(bond->wq, &bond->slave_arr_work, delay);
+}
 
-	if (likely(slave_cnt))
-		bond_xmit_slave_id(bond, skb,
-				   bond_xmit_hash(bond, skb) % slave_cnt);
-	else
+/* Slave array work handler. Holds only RTNL */
+static void bond_slave_arr_handler(struct work_struct *work)
+{
+	struct bonding *bond = container_of(work, struct bonding,
+					    slave_arr_work.work);
+	int ret;
+
+	if (!rtnl_trylock())
+		goto err;
+
+	ret = bond_update_slave_arr(bond, NULL);
+	rtnl_unlock();
+	if (ret) {
+		pr_warn_ratelimited("Failed to update slave array from WT\n");
+		goto err;
+	}
+	return;
+
+err:
+	bond_slave_arr_work_rearm(bond, 1);
+}
+
+/* Build the usable slaves array in control path for modes that use xmit-hash
+ * to determine the slave interface -
+ * (a) BOND_MODE_8023AD
+ * (b) BOND_MODE_XOR
+ * (c) BOND_MODE_TLB && tlb_dynamic_lb == 0
+ *
+ * The caller is expected to hold RTNL only and NO other lock!
+ */
+int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
+{
+	struct slave *slave;
+	struct list_head *iter;
+	struct bond_up_slave *new_arr, *old_arr;
+	int slaves_in_agg;
+	int agg_id = 0;
+	int ret = 0;
+
+#ifdef CONFIG_LOCKDEP
+	WARN_ON(lockdep_is_held(&bond->mode_lock));
+#endif
+
+	new_arr = kzalloc(offsetof(struct bond_up_slave, arr[bond->slave_cnt]),
+			  GFP_KERNEL);
+	if (!new_arr) {
+		ret = -ENOMEM;
+		pr_err("Failed to build slave-array.\n");
+		goto out;
+	}
+	if (BOND_MODE(bond) == BOND_MODE_8023AD) {
+		struct ad_info ad_info;
+
+		if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
+			pr_debug("bond_3ad_get_active_agg_info failed\n");
+			kfree_rcu(new_arr, rcu);
+			/* No active aggragator means its not safe to use
+			 * the previous array.
+			 */
+			old_arr = rtnl_dereference(bond->slave_arr);
+			if (old_arr) {
+				RCU_INIT_POINTER(bond->slave_arr, NULL);
+				kfree_rcu(old_arr, rcu);
+			}
+			goto out;
+		}
+		slaves_in_agg = ad_info.ports;
+		agg_id = ad_info.aggregator_id;
+	}
+	bond_for_each_slave(bond, slave, iter) {
+		if (BOND_MODE(bond) == BOND_MODE_8023AD) {
+			struct aggregator *agg;
+
+			agg = SLAVE_AD_INFO(slave)->port.aggregator;
+			if (!agg || agg->aggregator_identifier != agg_id)
+				continue;
+		}
+		if (!bond_slave_can_tx(slave))
+			continue;
+		if (skipslave == slave)
+			continue;
+		new_arr->arr[new_arr->count++] = slave;
+	}
+
+	old_arr = rtnl_dereference(bond->slave_arr);
+	rcu_assign_pointer(bond->slave_arr, new_arr);
+	if (old_arr)
+		kfree_rcu(old_arr, rcu);
+out:
+	if (ret != 0 && skipslave) {
+		int idx;
+
+		/* Rare situation where caller has asked to skip a specific
+		 * slave but allocation failed (most likely!). BTW this is
+		 * only possible when the call is initiated from
+		 * __bond_release_one(). In this situation; overwrite the
+		 * skipslave entry in the array with the last entry from the
+		 * array to avoid a situation where the xmit path may choose
+		 * this to-be-skipped slave to send a packet out.
+		 */
+		old_arr = rtnl_dereference(bond->slave_arr);
+		for (idx = 0; idx < old_arr->count; idx++) {
+			if (skipslave == old_arr->arr[idx]) {
+				old_arr->arr[idx] =
+				    old_arr->arr[old_arr->count-1];
+				old_arr->count--;
+				break;
+			}
+		}
+	}
+	return ret;
+}
+
+/* Use this Xmit function for 3AD as well as XOR modes. The current
+ * usable slave array is formed in the control path. The xmit function
+ * just calculates hash and sends the packet out.
+ */
+int bond_3ad_xor_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct bonding *bond = netdev_priv(dev);
+	struct slave *slave;
+	struct bond_up_slave *slaves;
+	unsigned int count;
+
+	slaves = rcu_dereference(bond->slave_arr);
+	count = slaves ? ACCESS_ONCE(slaves->count) : 0;
+	if (likely(count)) {
+		slave = slaves->arr[bond_xmit_hash(bond, skb) % count];
+		bond_dev_queue_xmit(bond, skb, slave->dev);
+	} else {
 		dev_kfree_skb_any(skb);
+		atomic_long_inc(&dev->tx_dropped);
+	}
 
 	return NETDEV_TX_OK;
 }
@@ -3682,12 +3842,11 @@ static netdev_tx_t __bond_start_xmit(struct sk_buff *skb, struct net_device *dev
 		return bond_xmit_roundrobin(skb, dev);
 	case BOND_MODE_ACTIVEBACKUP:
 		return bond_xmit_activebackup(skb, dev);
+	case BOND_MODE_8023AD:
 	case BOND_MODE_XOR:
-		return bond_xmit_xor(skb, dev);
+		return bond_3ad_xor_xmit(skb, dev);
 	case BOND_MODE_BROADCAST:
 		return bond_xmit_broadcast(skb, dev);
-	case BOND_MODE_8023AD:
-		return bond_3ad_xmit_xor(skb, dev);
 	case BOND_MODE_ALB:
 		return bond_alb_xmit(skb, dev);
 	case BOND_MODE_TLB:
@@ -3861,6 +4020,7 @@ static void bond_uninit(struct net_device *bond_dev)
 	struct bonding *bond = netdev_priv(bond_dev);
 	struct list_head *iter;
 	struct slave *slave;
+	struct bond_up_slave *arr;
 
 	bond_netpoll_cleanup(bond_dev);
 
@@ -3869,6 +4029,12 @@ static void bond_uninit(struct net_device *bond_dev)
 		__bond_release_one(bond_dev, slave->dev, true);
 	netdev_info(bond_dev, "Released all slaves\n");
 
+	arr = rtnl_dereference(bond->slave_arr);
+	if (arr) {
+		kfree_rcu(arr, rcu);
+		RCU_INIT_POINTER(bond->slave_arr, NULL);
+	}
+
 	list_del(&bond->bond_list);
 
 	bond_debug_unregister(bond);
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 5b022da9cad2..10920f0686e2 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -179,6 +179,12 @@ struct slave {
 	struct rtnl_link_stats64 slave_stats;
 };
 
+struct bond_up_slave {
+	unsigned int	count;
+	struct rcu_head rcu;
+	struct slave	*arr[0];
+};
+
 /*
  * Link pseudo-state only used internally by monitors
  */
@@ -193,6 +199,7 @@ struct bonding {
 	struct   slave __rcu *curr_active_slave;
 	struct   slave __rcu *current_arp_slave;
 	struct   slave __rcu *primary_slave;
+	struct   bond_up_slave __rcu *slave_arr; /* Array of usable slaves */
 	bool     force_primary;
 	s32      slave_cnt; /* never change this value outside the attach/detach wrappers */
 	int     (*recv_probe)(const struct sk_buff *, struct bonding *,
@@ -222,6 +229,7 @@ struct bonding {
 	struct   delayed_work alb_work;
 	struct   delayed_work ad_work;
 	struct   delayed_work mcast_work;
+	struct   delayed_work slave_arr_work;
 #ifdef CONFIG_DEBUG_FS
 	/* debugging support via debugfs */
 	struct	 dentry *debug_dir;
@@ -534,6 +542,8 @@ const char *bond_slave_link_status(s8 link);
 struct bond_vlan_tag *bond_verify_device_path(struct net_device *start_dev,
 					      struct net_device *end_dev,
 					      int level);
+int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave);
+void bond_slave_arr_work_rearm(struct bonding *bond, unsigned long delay);
 
 #ifdef CONFIG_PROC_FS
 void bond_create_proc_entry(struct bonding *bond);
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related

* [PATCH net-next v6 1/2] bonding: display xmit_hash_policy for non-dynamic-tlb mode
From: Mahesh Bandewar @ 2014-10-01  8:38 UTC (permalink / raw)
  To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David Miller
  Cc: netdev, Mahesh Bandewar, Eric Dumazet, Maciej Zenczykowski

It's a trivial fix to display xmit_hash_policy for this new TLB mode
since it uses transmit-hash-poilicy as part of bonding-master info
(/proc/net/bonding/<bonding-interface).

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
v1
 Rebase
v2
 Added bond_mode_uses_xmit_hash() inline function
v3-v6
 Rebase

 drivers/net/bonding/bond_procfs.c | 3 +--
 drivers/net/bonding/bonding.h     | 7 +++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index bb09d0442aa8..a3948f8d1e53 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -73,8 +73,7 @@ static void bond_info_show_master(struct seq_file *seq)
 
 	seq_printf(seq, "\n");
 
-	if (BOND_MODE(bond) == BOND_MODE_XOR ||
-		BOND_MODE(bond) == BOND_MODE_8023AD) {
+	if (bond_mode_uses_xmit_hash(bond)) {
 		optval = bond_opt_get_val(BOND_OPT_XMIT_HASH,
 					  bond->params.xmit_policy);
 		seq_printf(seq, "Transmit Hash Policy: %s (%d)\n",
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 57917e63b4e6..5b022da9cad2 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -274,6 +274,13 @@ static inline bool bond_is_nondyn_tlb(const struct bonding *bond)
 	       (bond->params.tlb_dynamic_lb == 0);
 }
 
+static inline bool bond_mode_uses_xmit_hash(const struct bonding *bond)
+{
+	return (BOND_MODE(bond) == BOND_MODE_8023AD ||
+		BOND_MODE(bond) == BOND_MODE_XOR ||
+		bond_is_nondyn_tlb(bond));
+}
+
 static inline bool bond_mode_uses_arp(int mode)
 {
 	return mode != BOND_MODE_8023AD && mode != BOND_MODE_TLB &&
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related

* Re: [net-next v3 20/29] fm10k: Add support for netdev offloads
From: Or Gerlitz @ 2014-10-01  8:31 UTC (permalink / raw)
  To: Jeff Kirsher, Alexander Duyck
  Cc: David Miller, Linux Netdev List, nhorman, sassmann, jogreene,
	Tom Herbert
In-Reply-To: <1411471017-28213-21-git-send-email-jeffrey.t.kirsher@intel.com>

On Tue, Sep 23, 2014 at 2:16 PM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> This patch adds support for basic offloads including TSO, Tx checksum, Rx
> checksum, Rx hash, and the same features applied to VXLAN/NVGRE tunnels.

> --- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
[...]
> +#define VXLAN_HLEN (sizeof(struct udphdr) + 8)
> +static struct ethhdr *fm10k_port_is_vxlan(struct sk_buff *skb)
> +{
> +       struct fm10k_intfc *interface = netdev_priv(skb->dev);
> +       struct fm10k_vxlan_port *vxlan_port;
> +
> +       /* we can only offload a vxlan if we recognize it as such */
> +       vxlan_port = list_first_entry_or_null(&interface->vxlan_port,
> +                                             struct fm10k_vxlan_port, list);
> +
> +       if (!vxlan_port)
> +               return NULL;
> +       if (vxlan_port->port != udp_hdr(skb)->dest)
> +               return NULL;
> +
> +       /* return offset of udp_hdr plus 8 bytes for VXLAN header */
> +       return (struct ethhdr *)(skb_transport_header(skb) + VXLAN_HLEN);
> +}
> +
> +#define FM10K_NVGRE_RESERVED0_FLAGS htons(0x9FFF)
> +#define NVGRE_TNI htons(0x2000)
> +struct fm10k_nvgre_hdr {
> +       __be16 flags;
> +       __be16 proto;
> +       __be32 tni;
> +};
> +
> +static struct ethhdr *fm10k_gre_is_nvgre(struct sk_buff *skb)
> +{
> +       struct fm10k_nvgre_hdr *nvgre_hdr;
> +       int hlen = ip_hdrlen(skb);
> +
> +       /* currently only IPv4 is supported due to hlen above */
> +       if (vlan_get_protocol(skb) != htons(ETH_P_IP))
> +               return NULL;
> +
> +       /* our transport header should be NVGRE */
> +       nvgre_hdr = (struct fm10k_nvgre_hdr *)(skb_network_header(skb) + hlen);
> +
> +       /* verify all reserved flags are 0 */
> +       if (nvgre_hdr->flags & FM10K_NVGRE_RESERVED0_FLAGS)
> +               return NULL;
> +
> +       /* verify protocol is transparent Ethernet bridging */
> +       if (nvgre_hdr->proto != htons(ETH_P_TEB))
> +               return NULL;
> +
> +       /* report start of ethernet header */
> +       if (nvgre_hdr->flags & NVGRE_TNI)
> +               return (struct ethhdr *)(nvgre_hdr + 1);
> +
> +       return (struct ethhdr *)(&nvgre_hdr->tni);
> +}

this helper is fully generic, should reside elsewhere in the stack


> +
> +static __be16 fm10k_tx_encap_offload(struct sk_buff *skb)
> +{
> +       struct ethhdr *eth_hdr;
> +       u8 l4_hdr = 0;
> +
> +       switch (vlan_get_protocol(skb)) {
> +       case htons(ETH_P_IP):
> +               l4_hdr = ip_hdr(skb)->protocol;
> +               break;
> +       case htons(ETH_P_IPV6):
> +               l4_hdr = ipv6_hdr(skb)->nexthdr;
> +               break;
> +       default:
> +               return 0;
> +       }
> +
> +       switch (l4_hdr) {
> +       case IPPROTO_UDP:
> +               eth_hdr = fm10k_port_is_vxlan(skb);
> +               break;
> +       case IPPROTO_GRE:
> +               eth_hdr = fm10k_gre_is_nvgre(skb);
> +               break;
> +       default:
> +               return 0;
> +       }
> +
> +       if (!eth_hdr)
> +               return 0;
> +
> +       switch (eth_hdr->h_proto) {
> +       case htons(ETH_P_IP):
> +       case htons(ETH_P_IPV6):
> +               break;
> +       default:
> +               return 0;
> +       }
> +
> +       return eth_hdr->h_proto;
> +}
> +
> +static int fm10k_tso(struct fm10k_ring *tx_ring,
> +                    struct fm10k_tx_buffer *first)
> +{
> +       struct sk_buff *skb = first->skb;
> +       struct fm10k_tx_desc *tx_desc;
> +       unsigned char *th;
> +       u8 hdrlen;
> +
> +       if (skb->ip_summed != CHECKSUM_PARTIAL)
> +               return 0;
> +
> +       if (!skb_is_gso(skb))
> +               return 0;
> +
> +       /* compute header lengths */
> +       if (skb->encapsulation) {
> +               if (!fm10k_tx_encap_offload(skb))
> +                       goto err_vxlan;
> +               th = skb_inner_transport_header(skb);
> +       } else {
> +               th = skb_transport_header(skb);
> +       }
> +
> +       /* compute offset from SOF to transport header and add header len */
> +       hdrlen = (th - skb->data) + (((struct tcphdr *)th)->doff << 2);
> +
> +       first->tx_flags |= FM10K_TX_FLAGS_CSUM;
> +
> +       /* update gso size and bytecount with header size */
> +       first->gso_segs = skb_shinfo(skb)->gso_segs;
> +       first->bytecount += (first->gso_segs - 1) * hdrlen;
> +
> +       /* populate Tx descriptor header size and mss */
> +       tx_desc = FM10K_TX_DESC(tx_ring, tx_ring->next_to_use);
> +       tx_desc->hdrlen = hdrlen;
> +       tx_desc->mss = cpu_to_le16(skb_shinfo(skb)->gso_size);
> +
> +       return 1;
> +err_vxlan:
> +       tx_ring->netdev->features &= ~NETIF_F_GSO_UDP_TUNNEL;
> +       if (!net_ratelimit())
> +               netdev_err(tx_ring->netdev,
> +                          "TSO requested for unsupported tunnel, disabling offload\n");
> +       return -1;
> +}

why? if TSO was requested for some packet the driver can't do, you disable GSO
for udp tunnels for any future packets too? maybe just disable it
permanently  till you
feel safer to run under the current stack?


[...]

> @@ -732,6 +1024,7 @@ netdev_tx_t fm10k_xmit_frame_ring(struct sk_buff *skb,
>                                   struct fm10k_ring *tx_ring)
>  {
>         struct fm10k_tx_buffer *first;
> +       int tso;
>         u32 tx_flags = 0;
>  #if PAGE_SIZE > FM10K_MAX_DATA_PER_TXD
>         unsigned short f;
> @@ -763,11 +1056,23 @@ netdev_tx_t fm10k_xmit_frame_ring(struct sk_buff *skb,
>         /* record initial flags and protocol */
>         first->tx_flags = tx_flags;
>
> +       tso = fm10k_tso(tx_ring, first);
> +       if (tso < 0)
> +               goto out_drop;
> +       else if (!tso)
> +               fm10k_tx_csum(tx_ring, first);
> +
>         fm10k_tx_map(tx_ring, first);
>
>         fm10k_maybe_stop_tx(tx_ring, DESC_NEEDED);
>
>         return NETDEV_TX_OK;
> +
> +out_drop:
> +       dev_kfree_skb_any(first->skb);
> +       first->skb = NULL;
> +
> +       return NETDEV_TX_OK;
>  }

^ permalink raw reply

* Re: [PATCH] net: xilinx: Remove unnecessary ether_setup after alloc_etherdev
From: Michal Simek @ 2014-10-01  8:25 UTC (permalink / raw)
  To: Michal Simek, linux-kernel
  Cc: Subbaraya Sundeep Bhatta, Anirudha Sarangi, John Linn,
	David S. Miller, Manuel Schölling, Paul Gortmaker,
	Julia Lawall, Joe Perches, Ricardo Ribalda, Eric Dumazet, netdev,
	linux-arm-kernel
In-Reply-To: <d43b465eca3e2a53c930f6e02dc4ac8b3c084693.1412143216.git.michal.simek@xilinx.com>

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

On 10/01/2014 08:00 AM, Michal Simek wrote:
> From: Subbaraya Sundeep Bhatta <subbaraya.sundeep.bhatta@xilinx.com>
> 
> Calling ether_setup is redundant since alloc_etherdev calls it.
> 
> Signed-off-by: Subbaraya Sundeep Bhatta <sbhatta@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
>  drivers/net/ethernet/xilinx/ll_temac_main.c       | 1 -
>  drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c
> index fda5891835d4..629077050fce 100644
> --- a/drivers/net/ethernet/xilinx/ll_temac_main.c
> +++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
> @@ -1012,7 +1012,6 @@ static int temac_of_probe(struct platform_device *op)
>  	if (!ndev)
>  		return -ENOMEM;
> 
> -	ether_setup(ndev);
>  	platform_set_drvdata(op, ndev);
>  	SET_NETDEV_DEV(ndev, &op->dev);
>  	ndev->flags &= ~IFF_MULTICAST;  /* clear multicast */
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index c8fd94133ecd..4ea2d4e6f1d1 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -1485,7 +1485,6 @@ static int axienet_of_probe(struct platform_device *op)
>  	if (!ndev)
>  		return -ENOMEM;
> 
> -	ether_setup(ndev);
>  	platform_set_drvdata(op, ndev);
> 
>  	SET_NETDEV_DEV(ndev, &op->dev);
> --
> 1.8.2.3
> 

Please ignore this one it is already applied for axienet.
Will send v2 just for ll_temac.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



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

^ permalink raw reply

* Re: [PATCH v2 net-next] net: pktgen: packet bursting via skb->xmit_more
From: Jesper Dangaard Brouer @ 2014-10-01  8:16 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: David S. Miller, Eric Dumazet, netdev, brouer
In-Reply-To: <1412124801-32096-1-git-send-email-ast@plumgrid.com>

On Tue, 30 Sep 2014 17:53:21 -0700
Alexei Starovoitov <ast@plumgrid.com> wrote:

> This patch demonstrates the effect of delaying update of HW tailptr.
> (based on earlier patch by Jesper)
> 
> burst=1 is the default. It sends one packet with xmit_more=false
> burst=2 sends one packet with xmit_more=true and
>         2nd copy of the same packet with xmit_more=false
> burst=3 sends two copies of the same packet with xmit_more=true and
>         3rd copy with xmit_more=false
> 
> Performance with ixgbe (usec 30):
> burst=1  tx:9.2 Mpps
> burst=2  tx:13.5 Mpps
> burst=3  tx:14.5 Mpps full 10G line rate
> 
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
> ---

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>

Thanks Alexei

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [datapath minor fixes 0/3] datapath minor fixes
From: Andy Zhou @ 2014-10-01  7:59 UTC (permalink / raw)
  To: David Miller; +Cc: netdev@vger.kernel.org, Andy Zhou
In-Reply-To: <1412150148-9991-1-git-send-email-azhou@nicira.com>

Please ignore this series. I meant to send them to dev@openvswitch.com.

On Wed, Oct 1, 2014 at 12:55 AM, Andy Zhou <azhou@nicira.com> wrote:
> A few small issues found in preparing for GENEVE support upstreaming
>
> *** BLURB HERE ***
>
> Andy Zhou (3):
>   datapath: avoid hard code OVS_VPORT_TYPE_GENEVE
>   datapath: avoid double free routing entry in vxlan_port xmit
>   datapath: serializing GENEVE options based on flow key's tun_flags
>
>  datapath/flow_netlink.c                           | 2 +-
>  datapath/linux/compat/include/linux/openvswitch.h | 2 +-
>  datapath/vport-vxlan.c                            | 2 --
>  3 files changed, 2 insertions(+), 4 deletions(-)
>
> --
> 1.9.1
>

^ permalink raw reply

* [datapath minor fixes 3/3] datapath: serializing GENEVE options based on flow key's tun_flags
From: Andy Zhou @ 2014-10-01  7:55 UTC (permalink / raw)
  To: davem; +Cc: netdev, Andy Zhou
In-Reply-To: <1412150148-9991-1-git-send-email-azhou@nicira.com>

TUNNEL_OPTIONS_PRESENT should always be checked against the tun_flgs
bits in a flow key, for both flow and mask serialization.

Signed-off-by: Andy Zhou <azhou@nicira.com>
---
 datapath/flow_netlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index a3f34f1..35feedc 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -1122,7 +1122,7 @@ int ovs_nla_put_flow(const struct sw_flow_key *swkey,
 	if ((swkey->tun_key.ipv4_dst || is_mask)) {
 		const struct geneve_opt *opts = NULL;
 
-		if (output->tun_key.tun_flags & TUNNEL_OPTIONS_PRESENT)
+		if (swkey->tun_key.tun_flags & TUNNEL_OPTIONS_PRESENT)
 			opts = GENEVE_OPTS(output, swkey->tun_opts_len);
 
 		if (ipv4_tun_to_nlattr(skb, &output->tun_key, opts,
-- 
1.9.1

^ permalink raw reply related

* [datapath minor fixes 2/3] datapath: avoid double free routing entry in vxlan_port xmit
From: Andy Zhou @ 2014-10-01  7:55 UTC (permalink / raw)
  To: davem; +Cc: netdev, Andy Zhou
In-Reply-To: <1412150148-9991-1-git-send-email-azhou@nicira.com>

Route entry will be free on error by vport when freeing skb.
additional error check and free after xmit() will cause double free.

Signed-off-by: Andy Zhou <azhou@nicira.com>
---
 datapath/vport-vxlan.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/datapath/vport-vxlan.c b/datapath/vport-vxlan.c
index 8689853..44968d0 100644
--- a/datapath/vport-vxlan.c
+++ b/datapath/vport-vxlan.c
@@ -180,8 +180,6 @@ static int vxlan_tnl_send(struct vport *vport, struct sk_buff *skb)
 			     tun_key->ipv4_ttl, df,
 			     src_port, dst_port,
 			     htonl(be64_to_cpu(tun_key->tun_id) << 8));
-	if (err < 0)
-		ip_rt_put(rt);
 error:
 	return err;
 }
-- 
1.9.1

^ 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