Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text
From: Jason Cooper @ 2014-02-18 12:52 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: David S. Miller, netdev, Gregory Clement, Andrew Lunn,
	Sebastian Hesselbarth, Ezequiel Garcia, linux-arm-kernel
In-Reply-To: <20140218134532.7607a616@skate>

On Tue, Feb 18, 2014 at 01:45:32PM +0100, Thomas Petazzoni wrote:
> Dear Jason Cooper,
> 
> On Tue, 18 Feb 2014 07:29:52 -0500, Jason Cooper wrote:
> 
> > >  config MVNETA
> > > -	tristate "Marvell Armada 370/XP network interface support"
> > > -	depends on MACH_ARMADA_370_XP
> > 
> > Have you build-tested this on the usual fail scenarios?  eg x86_64,
> > powerpc, s390, allno, allyes, allmod, etc?
> > 
> > I think you might be opening up pandora's box here without needing to.
> > :)
> 
> No, I haven't tested all the build scenarios of course. If you feel
> that this is too dangerous, I can resend a patch that replaces 'depends
> on ARMADA_370_XP' by 'depends on PLAT_ORION'. Would this be OK for you?

I'm fine either way.  I'm definitely a "What's in the box?" kind of guy.
But I do like being prepared.

The real question is: Do you think this IP block will ever be found on
anything other than Marvell Armada boards?  If so, stick with this
version and build test the crap out of it.  Otherwise, let's save
opening the box for another day.

thx,

Jason.

^ permalink raw reply

* Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text
From: Thomas Petazzoni @ 2014-02-18 12:45 UTC (permalink / raw)
  To: Jason Cooper
  Cc: David S. Miller, netdev, Gregory Clement, Andrew Lunn,
	Sebastian Hesselbarth, Ezequiel Garcia, linux-arm-kernel
In-Reply-To: <20140218122952.GM7862@titan.lakedaemon.net>

Dear Jason Cooper,

On Tue, 18 Feb 2014 07:29:52 -0500, Jason Cooper wrote:

> >  config MVNETA
> > -	tristate "Marvell Armada 370/XP network interface support"
> > -	depends on MACH_ARMADA_370_XP
> 
> Have you build-tested this on the usual fail scenarios?  eg x86_64,
> powerpc, s390, allno, allyes, allmod, etc?
> 
> I think you might be opening up pandora's box here without needing to.
> :)

No, I haven't tested all the build scenarios of course. If you feel
that this is too dangerous, I can resend a patch that replaces 'depends
on ARMADA_370_XP' by 'depends on PLAT_ORION'. Would this be OK for you?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text
From: Jason Cooper @ 2014-02-18 12:29 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: David S. Miller, netdev, Gregory Clement, Andrew Lunn,
	Sebastian Hesselbarth, Ezequiel Garcia, linux-arm-kernel
In-Reply-To: <1392719391-8851-1-git-send-email-thomas.petazzoni@free-electrons.com>

On Tue, Feb 18, 2014 at 11:29:51AM +0100, Thomas Petazzoni wrote:
> With the introduction of the support for Armada 375 and Armada 38x,
> the hidden Kconfig option MACH_ARMADA_370_XP is being renamed to
> MACH_MVEBU_V7. Therefore, the dependency that was used for the mvneta
> driver can no longer work.
> 
> However, such a dependency is not really necessary: there is no point
> in preventing this driver from being built in other situations, just
> like the mv643xx_eth driver. As a consequence, this commit removes the
> unnecessary Kconfig dependency.
> 
> In addition to this, it takes this opportunity to adjust the
> description and help text to indicate that the driver can is also used
> for Armada 38x. Note that Armada 375 cannot use this driver as it has
> a completely different networking unit, which will require a separate
> driver.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  drivers/net/ethernet/marvell/Kconfig | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
> index 6300fd2..97b91f7 100644
> --- a/drivers/net/ethernet/marvell/Kconfig
> +++ b/drivers/net/ethernet/marvell/Kconfig
> @@ -43,12 +43,11 @@ config MVMDIO
>  	  This driver is used by the MV643XX_ETH and MVNETA drivers.
>  
>  config MVNETA
> -	tristate "Marvell Armada 370/XP network interface support"
> -	depends on MACH_ARMADA_370_XP

Have you build-tested this on the usual fail scenarios?  eg x86_64,
powerpc, s390, allno, allyes, allmod, etc?

I think you might be opening up pandora's box here without needing to.
:)

thx,

Jason.

^ permalink raw reply

* Re: [PATCH net-next 1/2] pcnet32: fix reallocation error
From: Tetsuo Handa @ 2014-02-18 12:29 UTC (permalink / raw)
  To: pcnet32; +Cc: davem, netdev
In-Reply-To: <1392699466.4020.27.camel@Linux>

Don Fry wrote:
> pcnet32_realloc_rx_ring() only worked on the first log2 number of
> entries in the receive ring instead of the all the entries.
> Replaced "1 << size" with more descriptive variable.

>  	/* first copy the current receive buffers */
> -	overlap = min(size, lp->rx_ring_size);
> +	overlap = min(entries, lp->rx_ring_size);

>  	/* now allocate any new buffers needed */
> -	for (; new < size; new++) {
> +	for (; new < entries; new++) {

Until this fix, lp->rx_skbuff[size...(1<<size)-1] were not allocated by
netdev_alloc_skb() and lp->rx_ring_dma_addr[size...(1<<size)-1] were not mapped
by pci_map_single(), right?

Since lp->rx_ring_size was set to (1<<size), wasn't there possibility that
something bad happens by accessing lp->rx_skbuff[] and lp->rx_ring_dma_addr[]
up to (1<<size)-1 ? (In other words, don't we want to backport this fix?)

^ permalink raw reply

* Re: [PATCH RFC] sctp: Update HEARTBEAT timer immediately after user changed HB.interval
From: Neil Horman @ 2014-02-18 12:23 UTC (permalink / raw)
  To: Xufeng Zhang; +Cc: vyasevich, davem, linux-sctp, netdev, linux-kernel
In-Reply-To: <1392703010-6128-1-git-send-email-xufeng.zhang@windriver.com>

On Tue, Feb 18, 2014 at 01:56:50PM +0800, Xufeng Zhang wrote:
> For an established association, if user has updated the HB.interval
> parameter by setsockopt(), this new heartbeat interval will not
> take effect until:
>   - the expiry of the heartbeat timer and new hearbeat is sent.
>   - DATA chunk has been sent and the transport resets the timer.
> This could not meet the requirement of the user who need to
> get HEARTBEAT sent at the specified time.
> 
> Thus, we need to update the heartbeat timer immediately after
> user has changed HB.interval.
> 
> Signed-off-by: Xufeng Zhang <xufeng.zhang@windriver.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>

> ---
>  net/sctp/socket.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 9e91d6e..699ae1e 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -2344,6 +2344,11 @@ static int sctp_apply_peer_addr_params(struct sctp_paddrparams *params,
>  			if (trans) {
>  				trans->hbinterval =
>  				    msecs_to_jiffies(params->spp_hbinterval);
> +
> +				/* Update the heartbeat timer immediately. */
> +				if (!mod_timer(&trans->hb_timer,
> +					    sctp_transport_timeout(trans)))
> +					sctp_transport_hold(trans);
>  			} else if (asoc) {
>  				asoc->hbinterval =
>  				    msecs_to_jiffies(params->spp_hbinterval);
> -- 
> 1.7.0.2
> 
> 

^ permalink raw reply

* [patch] af_packet: remove a stray tab in packet_set_ring()
From: Dan Carpenter @ 2014-02-18 12:20 UTC (permalink / raw)
  To: David S. Miller
  Cc: Daniel Borkmann, Willem de Bruijn, Hannes Frederic Sowa,
	Eric Dumazet, Phil Sutter, Atzm Watanabe, Richard Cochran, netdev,
	kernel-janitors

At first glance it looks like there is a missing curly brace but
actually the code works the same either way.  I have adjusted the
indenting but left the code the same.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 6a2bb37506c5..7fd37de503d1 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -3786,7 +3786,7 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u,
 		 */
 			if (!tx_ring)
 				init_prb_bdqc(po, rb, pg_vec, req_u, tx_ring);
-				break;
+			break;
 		default:
 			break;
 		}

^ permalink raw reply related

* [PATCH net] tun: remove bogus hardware vlan acceleration flags from vlan_features
From: Toshiaki Makita @ 2014-02-18 12:20 UTC (permalink / raw)
  To: David S . Miller
  Cc: Fernando Luis Vazquez Cao, Maxim Krasnyansky, netdev,
	Toshiaki Makita
In-Reply-To: <1392726009-8083-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>

From: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>

Even though only the outer vlan tag can be HW accelerated in the transmission
path, in the TUN/TAP driver vlan_features mirrors hw_features, which happens
to have the NETIF_F_HW_VLAN_?TAG_TX flags set. Because of this, during packet
tranmisssion through a stacked vlan device dev_hard_start_xmit, (incorrectly)
assuming that the vlan device supports hardware vlan acceleration, does not
add the vlan header to the skb payload and the inner vlan tags are lost
(vlan_tci contains the outer vlan tag when userspace reads the packet from
the tap device).

Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---

diff -urNp linux-3.13.3-orig/drivers/net/tun.c linux-3.13.3/drivers/net/tun.c
--- linux-3.13.3-orig/drivers/net/tun.c	2014-01-20 11:40:07.000000000 +0900
+++ linux-3.13.3/drivers/net/tun.c	2014-02-18 16:23:04.461078629 +0900
@@ -1651,7 +1651,9 @@ static int tun_set_iff(struct net *net,
 				   TUN_USER_FEATURES | NETIF_F_HW_VLAN_CTAG_TX |
 				   NETIF_F_HW_VLAN_STAG_TX;
 		dev->features = dev->hw_features;
-		dev->vlan_features = dev->features;
+		dev->vlan_features = dev->features &
+				     ~(NETIF_F_HW_VLAN_CTAG_TX |
+				       NETIF_F_HW_VLAN_STAG_TX);
 
 		INIT_LIST_HEAD(&tun->disabled);
 		err = tun_attach(tun, file, false);

^ permalink raw reply

* [PATCH net] veth: Fix vlan_features so as to be able to use stacked vlan interfaces
From: Toshiaki Makita @ 2014-02-18 12:20 UTC (permalink / raw)
  To: David S . Miller; +Cc: Toshiaki Makita, Flavio Leitner, netdev

Even if we create a stacked vlan interface such as veth0.10.20, it sends
single tagged frames (tagged with only vid 10).
Because vlan_features of a veth interface has the
NETIF_F_HW_VLAN_[CTAG/STAG]_TX bits, veth0.10 also has that feature, so
dev_hard_start_xmit(veth0.10) doesn't call __vlan_put_tag() and
vlan_dev_hard_start_xmit(veth0.10) overwrites vlan_tci.
This prevents us from using a combination of 802.1ad and 802.1Q
in containers, etc.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 drivers/net/veth.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 2ec2041..5b37437 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -285,7 +285,8 @@ static void veth_setup(struct net_device *dev)
 	dev->ethtool_ops = &veth_ethtool_ops;
 	dev->features |= NETIF_F_LLTX;
 	dev->features |= VETH_FEATURES;
-	dev->vlan_features = dev->features;
+	dev->vlan_features = dev->features &
+			     ~(NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX);
 	dev->destructor = veth_dev_free;
 
 	dev->hw_features = VETH_FEATURES;
-- 
1.8.1.2

^ permalink raw reply related

* Re: [PATCH net-next] bonding: Fix RTNL: assertion failed at net/core/rtnetlink.c for 802.3ad mode
From: Ding Tianhong @ 2014-02-18 12:16 UTC (permalink / raw)
  To: Thomas Glanzmann
  Cc: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico, Cong Wang,
	Jiri Pirko, David S. Miller, Eric Dumazet, Scott Feldman, Netdev
In-Reply-To: <20140218121417.GB30299@glanzmann.de>

On 2014/2/18 20:14, Thomas Glanzmann wrote:
> Hello Ding,
> 
> * Ding Tianhong <dingtianhong@huawei.com> [2014-02-18 12:25]:
>> The problem was introduced by the commit 1d3ee88ae0d
>> (bonding: add netlink attributes to slave link dev).
>> The bond_set_active_slave() and bond_set_backup_slave()
>> will use rtmsg_ifinfo to send slave's states, so these
>> two functions should be called in RTNL.
> 
>> In 802.3ad mode, acquiring RTNL for the __enable_port and
>> __disable_port cases is difficult, as those calls generally
>> already hold the state machine lock, and cannot unconditionally
>> call rtnl_lock because either they already hold RTNL (for calls
>> via bond_3ad_unbind_slave) or due to the potential for deadlock
>> with bond_3ad_adapter_speed_changed, bond_3ad_adapter_duplex_changed,
>> bond_3ad_link_change, or bond_3ad_update_lacp_rate.  All four of
>> those are called with RTNL held, and acquire the state machine lock
>> second.  The calling contexts for __enable_port and __disable_port
>> already hold the state machine lock, and may or may not need RTNL.
> 
>> According to the Jay's opinion, I don't think it is a problem that
>> the slave don't send notify message synchronously when the status
>> changed, normally the state machine is running every 100 ms, send
>> the notify message at the end of the state machine if the slave's
>> state changed should be better.
> 
>> I fix the problem through these steps:
> 
>> 1). add a new function bond_set_slave_state() which could change
>>     the slave's state and call rtmsg_ifinfo() according to the input
>>     parameters called notify.
> 
>> 2). Add a new slave parameter which called should_notify, if the slave's state
>>     changed and don't notify yet, the parameter will be set to 1, and then if
>>     the slave's state changed again, the param will be set to 0, it indicate that
>>     the slave's state has been restored, no need to notify any one.
> 
>> 3). the __enable_port and __disable_port should not call rtmsg_ifinfo
>>     in the state machine lock, any change in the state of slave could
>>     set a flag in the slave, it will indicated that an rtmsg_ifinfo
>>     should be called at the end of the state machine.
> 
> I applied the same on top of Linus Tip and tested:
> 
> (node-62) [~] dmesg | pbot
> http://pbot.rmdir.de/tlM017PXoi9PsV3j32z4gA
> (node-62) [~] pbot /proc/net/bonding/bond0
> http://pbot.rmdir.de/zNSSqmjSI0o1Qvt6DrSEQw
> (node-62) [~] pbot /proc/net/bonding/bond1
> http://pbot.rmdir.de/CoI00Rguie2P-kOQytOM9w
> 
> Looks good to me.
> 
> Tested-by: Thomas Glanzmann <thomas@glanzmann.de>
> 
> Cheers,
>         Thomas
> 
> .

Thanks for testing this patch again.

Regards
Ding
> 

^ permalink raw reply

* [PATCH v2] of_mdio: fix phy interrupt passing
From: Ben Dooks @ 2014-02-18 12:16 UTC (permalink / raw)
  To: grant.likely
  Cc: linux-kernel, devicetree, linux-kernel, netdev, linux-sh,
	sergei.shtylyov, Ben Dooks

The of_mdiobus_register_phy() is not setting phy->irq thus causing
some drivers to incorrectly assume that the PHY does not have an
IRQ associated with it. Not only do some drivers report no IRQ
they do not install an interrupt handler for the PHY.

Simplify the code setting irq and set the phy->irq at the same
time so that we cover the following issues, which should cover
all the cases the code will find:

- Set phy->irq if node has irq property and mdio->irq is NULL
- Set phy->irq if node has no irq and mdio->irq is not NULL
- Leave phy->irq as PHY_POLL default if none of the above

This fixes the issue:
 net eth0: attached PHY 1 (IRQ -1) to driver Micrel KSZ8041RNLI

to the correct:
 net eth0: attached PHY 1 (IRQ 416) to driver Micrel KSZ8041RNLI

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>

---
Since v1:
	- Updated phy->irq setting code
	- Deal with issue if mdio->irq array NULL

Notes:

It was discussed if this should be two patches, but we end up
making enough changes in #2 the same area as #1 means that we
basically rewrite #1 in #2. So keep it as 1 patch.
---
 drivers/of/of_mdio.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 875b7b6..46d95fc 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -44,7 +44,7 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi
 {
 	struct phy_device *phy;
 	bool is_c45;
-	int rc, prev_irq;
+	int rc;
 	u32 max_speed = 0;
 
 	is_c45 = of_device_is_compatible(child,
@@ -54,12 +54,14 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi
 	if (!phy || IS_ERR(phy))
 		return 1;
 
-	if (mdio->irq) {
-		prev_irq = mdio->irq[addr];
-		mdio->irq[addr] =
-			irq_of_parse_and_map(child, 0);
-		if (!mdio->irq[addr])
-			mdio->irq[addr] = prev_irq;
+	rc = irq_of_parse_and_map(child, 0);
+	if (rc > 0) {
+		phy->irq = rc;
+		if (mdio->irq)
+			mdio->irq[addr] = rc;
+	} else {
+		if (mdio->irq)
+			phy->irq = mdio->irq[addr];
 	}
 
 	/* Associate the OF node with the device structure so it
-- 
1.8.5.3


^ permalink raw reply related

* Re: ixgbevf: Use pci_enable_msix_range() instead of pci_enable_msix()
From: Amir Vadai @ 2014-02-18 12:14 UTC (permalink / raw)
  To: Alexander Gordeev
  Cc: linux-kernel, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
	e1000-devel, netdev, linux-pci
In-Reply-To: <20140218120136.GA8770@mtl-eit-vdi-22.mtl.labs.mlnx>

On 18/02/14 14:01 +0200, Amir Vadai wrote:
> On 18/02/14 11:11 +0100, Alexander Gordeev wrote:
> > As result of deprecation of MSI-X/MSI enablement functions
> > pci_enable_msix() and pci_enable_msi_block() all drivers
> > using these two interfaces need to be updated to use the
> > new pci_enable_msi_range() and pci_enable_msix_range()
> > interfaces.
> > 
> > Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> > Cc: Bruce Allan <bruce.w.allan@intel.com>
> > Cc: e1000-devel@lists.sourceforge.net
> > Cc: netdev@vger.kernel.org
> > Cc: linux-pci@vger.kernel.org
> > ---
> 
> Acked-By: Amir Vadai <amirv@mellanox.com>
Oh, I'm still working for Mellanox - just replied to the wrong mail :)

^ permalink raw reply

* Re: [PATCH net-next] bonding: Fix RTNL: assertion failed at net/core/rtnetlink.c for 802.3ad mode
From: Thomas Glanzmann @ 2014-02-18 12:14 UTC (permalink / raw)
  To: Ding Tianhong
  Cc: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico, Cong Wang,
	Jiri Pirko, David S. Miller, Eric Dumazet, Scott Feldman, Netdev
In-Reply-To: <53034312.1060203@huawei.com>

Hello Ding,

* Ding Tianhong <dingtianhong@huawei.com> [2014-02-18 12:25]:
> The problem was introduced by the commit 1d3ee88ae0d
> (bonding: add netlink attributes to slave link dev).
> The bond_set_active_slave() and bond_set_backup_slave()
> will use rtmsg_ifinfo to send slave's states, so these
> two functions should be called in RTNL.

> In 802.3ad mode, acquiring RTNL for the __enable_port and
> __disable_port cases is difficult, as those calls generally
> already hold the state machine lock, and cannot unconditionally
> call rtnl_lock because either they already hold RTNL (for calls
> via bond_3ad_unbind_slave) or due to the potential for deadlock
> with bond_3ad_adapter_speed_changed, bond_3ad_adapter_duplex_changed,
> bond_3ad_link_change, or bond_3ad_update_lacp_rate.  All four of
> those are called with RTNL held, and acquire the state machine lock
> second.  The calling contexts for __enable_port and __disable_port
> already hold the state machine lock, and may or may not need RTNL.

> According to the Jay's opinion, I don't think it is a problem that
> the slave don't send notify message synchronously when the status
> changed, normally the state machine is running every 100 ms, send
> the notify message at the end of the state machine if the slave's
> state changed should be better.

> I fix the problem through these steps:

> 1). add a new function bond_set_slave_state() which could change
>     the slave's state and call rtmsg_ifinfo() according to the input
>     parameters called notify.

> 2). Add a new slave parameter which called should_notify, if the slave's state
>     changed and don't notify yet, the parameter will be set to 1, and then if
>     the slave's state changed again, the param will be set to 0, it indicate that
>     the slave's state has been restored, no need to notify any one.

> 3). the __enable_port and __disable_port should not call rtmsg_ifinfo
>     in the state machine lock, any change in the state of slave could
>     set a flag in the slave, it will indicated that an rtmsg_ifinfo
>     should be called at the end of the state machine.

I applied the same on top of Linus Tip and tested:

(node-62) [~] dmesg | pbot
http://pbot.rmdir.de/tlM017PXoi9PsV3j32z4gA
(node-62) [~] pbot /proc/net/bonding/bond0
http://pbot.rmdir.de/zNSSqmjSI0o1Qvt6DrSEQw
(node-62) [~] pbot /proc/net/bonding/bond1
http://pbot.rmdir.de/CoI00Rguie2P-kOQytOM9w

Looks good to me.

Tested-by: Thomas Glanzmann <thomas@glanzmann.de>

Cheers,
        Thomas

^ permalink raw reply

* Re: ixgbevf: Use pci_enable_msix_range() instead of pci_enable_msix()
From: Amir Vadai @ 2014-02-18 12:01 UTC (permalink / raw)
  To: Alexander Gordeev
  Cc: linux-kernel, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
	e1000-devel, netdev, linux-pci
In-Reply-To: <9ee830cfd4b417f230f77dd37256ca8e4396ecbe.1392717503.git.agordeev@redhat.com>

On 18/02/14 11:11 +0100, Alexander Gordeev wrote:
> As result of deprecation of MSI-X/MSI enablement functions
> pci_enable_msix() and pci_enable_msi_block() all drivers
> using these two interfaces need to be updated to use the
> new pci_enable_msi_range() and pci_enable_msix_range()
> interfaces.
> 
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Bruce Allan <bruce.w.allan@intel.com>
> Cc: e1000-devel@lists.sourceforge.net
> Cc: netdev@vger.kernel.org
> Cc: linux-pci@vger.kernel.org
> ---

Acked-By: Amir Vadai <amirv@mellanox.com>

^ permalink raw reply

* Re: [PATCH] net: add init-regs for of_phy support
From: Mark Rutland @ 2014-02-18 11:54 UTC (permalink / raw)
  To: Ben Dooks
  Cc: Florian Fainelli, Sergei Shtylyov, netdev,
	devicetree@vger.kernel.org, Linux-sh list, David Miller
In-Reply-To: <530316EE.5010104@codethink.co.uk>

On Tue, Feb 18, 2014 at 08:16:46AM +0000, Ben Dooks wrote:
> On 17/02/14 20:48, Florian Fainelli wrote:
> 
> [snip]
> 
> >>> - fixing up some design mistake?
> >>> - accounting for a specific board design?
> >>
> >>
> >>     Kind of both. This was invented to defy the necessity of having platform
> >> fixup in the DT case (where there should be no board file to place it into).
> >> I have already described that platform fixup necessary on the Renesas
> >> Lager/Koelsch boards where the LED0 signat is connected to ETH_LINK signal
> >> on the SoC and the PHY reset sets the LED control bits to default 0 which
> >> means that LED0 will be LINK/ACTIVITY signal and thus blink on activity and
> >> cause ETH_LINK to bounce off/on after each packet.
> >>
> >>
> >>> In any case a PHY fixup would do the job for you.
> >>
> >>
> >>     Not in any case. In case of DT we have no place for it, so should invent
> >> something involving DT.
> >
> > How is DT different than any machine probing mechanism here? The way
> > to involve DT is to do the following:
> >
> > if (of_machine_is_compatible("renesas,foo-board-with-broken-micrel-phy"))
> >             phy_register_fixup(&foo_board_with_broken_micrel_phy);
> 
> Oh yes, but now I have to do that for Linux, for $BSD, and for
> anything else I want to run on the device. I thought dt was meant
> to allow us to describe the hardware.

It does allow you to describe the hardware. Arbitrary register writes
aren't a description of the hardware, they're a sequence of instructions
that tells the OS nothing about the hardware and limit the ability of an
OS to do something different that might be better.

It's already the case that the OS has to have some knowledge of the
hardware that's implicit in a binding. We don't expect to have to
include bytecode to tell the OS how to poke a particular UART when it
can figure that out from a compatible string.

> If this is the case, let's just call this linuxtree and let everyone
> else get on with their own thing again.

This doesn't follow at all. Any OS needs to have some understanding of
the hardware it will try to poke. Describing a specific sequence of
writes in a DT is no more operating system independent than identifying
the hardware and expecting the OS to have a driver for it. The
requirements aren't any more suited to an individual OS in either case.

> 
> See also comment below.
> 
> > If your machine compatible string does not allow you to uniquely
> > identify your machine, this is a DT problem, as this should really be
> > the case. If you do not want to add this code to wherever this is
> > relevant in arch/arm/mach-shmobile/board-*.c, neither is
> > drivers/net/phy/phy_device.c this the place to add it.
> 
> So where should it be added? If we keep piling stuff into board files
> in arch/arm.... then we're just back to the pre-dt case and going to
> keep getting shouted at.

The general trend has been to allocate new compatible strings for
components and let individual drivers handle this.

As far as I can see your case doesn't involve any components external to
the PHY, so should probably live in a PHY driver. The PHY can have a
specific compatible string with the generic string as a fallback (if it
works to some degree without special poking).

I don't see that we need anything board-specific.

> 
> > Dealing with quirks applying to industry standard blocks is to update
> > the relevant driver, based on information provided by the specifically
> > affected systems. Failure to identify either of those correctly is a
> > problem that must not lead to a generic "let's override PHY registers
> > from DT" type of solution.
> >
> > As usual, mechanism vs policy applies even more when DT is involved.
> 
> There's an industry standard for the access method, but every PHY seems
> to have different extra setup registers for their own cases.

So? Have a driver for each PHY, or fixups for each PHY in a shared
driver.

Cheers,
Mark.

^ permalink raw reply

* Re: [PATCH v5 net-next 01/12] bonding: remove bond->lock from bond_arp_rcv
From: Ding Tianhong @ 2014-02-18 11:54 UTC (permalink / raw)
  To: Veaceslav Falico, netdev; +Cc: Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1392706127-28390-2-git-send-email-vfalico@redhat.com>

On 2014/2/18 14:48, Veaceslav Falico wrote:
> We're always called with rcu_read_lock() held (bond_arp_rcv() is only
> called from bond_handle_frame(), which is rx_handler and always called
> under rcu from __netif_receive_skb_core() ).
> 
> The slave active/passive and/or bonding params can change in-flight, however
> we don't really care about that - we only modify the last time packet was
> received, which is harmless.
> 
> CC: Jay Vosburgh <fubar@us.ibm.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
> ---
>  drivers/net/bonding/bond_main.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 3bce855..3c50bec 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2260,8 +2260,6 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
>  	if (skb->protocol != __cpu_to_be16(ETH_P_ARP))
>  		return RX_HANDLER_ANOTHER;
>  
> -	read_lock(&bond->lock);
> -
>  	if (!slave_do_arp_validate(bond, slave))
>  		goto out_unlock;
>  
> @@ -2318,7 +2316,6 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
>  		bond_validate_arp(bond, slave, tip, sip);
>  
>  out_unlock:
> -	read_unlock(&bond->lock);
>  	if (arp != (struct arphdr *)skb->data)
>  		kfree(arp);
>  	return RX_HANDLER_ANOTHER;
> 
Acked-by: Ding Tianhong <dingtianhong@huawei.com>

^ permalink raw reply

* Re: [PATCH net-next] bonding: Fix RTNL: assertion failed at net/core/rtnetlink.c for 802.3ad mode
From: Nikolay Aleksandrov @ 2014-02-18 11:53 UTC (permalink / raw)
  To: Ding Tianhong, Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
	Cong Wang, Thomas Glanzmann, Jiri Pirko, David S. Miller,
	Eric Dumazet, Scott Feldman, Netdev
In-Reply-To: <530348C4.4050408@redhat.com>

On 02/18/2014 12:49 PM, Nikolay Aleksandrov wrote:
> On 02/18/2014 12:25 PM, Ding Tianhong wrote:
>> The problem was introduced by the commit 1d3ee88ae0d
>> (bonding: add netlink attributes to slave link dev).
>> The bond_set_active_slave() and bond_set_backup_slave()
>> will use rtmsg_ifinfo to send slave's states, so these
>> two functions should be called in RTNL.
>>
>> In 802.3ad mode, acquiring RTNL for the __enable_port and
>> __disable_port cases is difficult, as those calls generally
>> already hold the state machine lock, and cannot unconditionally
>> call rtnl_lock because either they already hold RTNL (for calls
>> via bond_3ad_unbind_slave) or due to the potential for deadlock
>> with bond_3ad_adapter_speed_changed, bond_3ad_adapter_duplex_changed,
>> bond_3ad_link_change, or bond_3ad_update_lacp_rate.  All four of
>> those are called with RTNL held, and acquire the state machine lock
>> second.  The calling contexts for __enable_port and __disable_port
>> already hold the state machine lock, and may or may not need RTNL.
>>
>> According to the Jay's opinion, I don't think it is a problem that
>> the slave don't send notify message synchronously when the status
>> changed, normally the state machine is running every 100 ms, send
>> the notify message at the end of the state machine if the slave's
>> state changed should be better.
>>
>> I fix the problem through these steps:
>>
>> 1). add a new function bond_set_slave_state() which could change
>>     the slave's state and call rtmsg_ifinfo() according to the input
>>     parameters called notify.
>>
>> 2). Add a new slave parameter which called should_notify, if the slave's state
>>     changed and don't notify yet, the parameter will be set to 1, and then if
>>     the slave's state changed again, the param will be set to 0, it indicate that
>>     the slave's state has been restored, no need to notify any one.
>>
>> 3). the __enable_port and __disable_port should not call rtmsg_ifinfo
>>     in the state machine lock, any change in the state of slave could
>>     set a flag in the slave, it will indicated that an rtmsg_ifinfo
>>     should be called at the end of the state machine.
>>
>> Cc: Jay Vosburgh <fubar@us.ibm.com>
>> Cc: Veaceslav Falico <vfalico@redhat.com>
>> Cc: Andy Gospodarek <andy@greyhouse.net>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
> Hi Ding,
> I think there's a possible race condition which could lead to inconsistent
> state because you set slave->should_notify to 0 under RTNL but
> __disable_port can update it without RTNL e.g. can be called via
> bond_3ad_state_machine_handler -> ad_agg_selection_logic so in theory (I
> haven't tested it) they can execute concurrently. This is not a big deal
> though, but it would make this kind of message unreliable.
> 
> Nik
> 
Ah, missed where it gets updated, never mind this comment, it's fine.

^ permalink raw reply

* Re: [PATCH net-next] bonding: Fix RTNL: assertion failed at net/core/rtnetlink.c for 802.3ad mode
From: Nikolay Aleksandrov @ 2014-02-18 11:49 UTC (permalink / raw)
  To: Ding Tianhong, Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
	Cong Wang, Thomas Glanzmann, Jiri Pirko, David S. Miller,
	Eric Dumazet, Scott Feldman, Netdev
In-Reply-To: <53034312.1060203@huawei.com>

On 02/18/2014 12:25 PM, Ding Tianhong wrote:
> The problem was introduced by the commit 1d3ee88ae0d
> (bonding: add netlink attributes to slave link dev).
> The bond_set_active_slave() and bond_set_backup_slave()
> will use rtmsg_ifinfo to send slave's states, so these
> two functions should be called in RTNL.
> 
> In 802.3ad mode, acquiring RTNL for the __enable_port and
> __disable_port cases is difficult, as those calls generally
> already hold the state machine lock, and cannot unconditionally
> call rtnl_lock because either they already hold RTNL (for calls
> via bond_3ad_unbind_slave) or due to the potential for deadlock
> with bond_3ad_adapter_speed_changed, bond_3ad_adapter_duplex_changed,
> bond_3ad_link_change, or bond_3ad_update_lacp_rate.  All four of
> those are called with RTNL held, and acquire the state machine lock
> second.  The calling contexts for __enable_port and __disable_port
> already hold the state machine lock, and may or may not need RTNL.
> 
> According to the Jay's opinion, I don't think it is a problem that
> the slave don't send notify message synchronously when the status
> changed, normally the state machine is running every 100 ms, send
> the notify message at the end of the state machine if the slave's
> state changed should be better.
> 
> I fix the problem through these steps:
> 
> 1). add a new function bond_set_slave_state() which could change
>     the slave's state and call rtmsg_ifinfo() according to the input
>     parameters called notify.
> 
> 2). Add a new slave parameter which called should_notify, if the slave's state
>     changed and don't notify yet, the parameter will be set to 1, and then if
>     the slave's state changed again, the param will be set to 0, it indicate that
>     the slave's state has been restored, no need to notify any one.
> 
> 3). the __enable_port and __disable_port should not call rtmsg_ifinfo
>     in the state machine lock, any change in the state of slave could
>     set a flag in the slave, it will indicated that an rtmsg_ifinfo
>     should be called at the end of the state machine.
> 
> Cc: Jay Vosburgh <fubar@us.ibm.com>
> Cc: Veaceslav Falico <vfalico@redhat.com>
> Cc: Andy Gospodarek <andy@greyhouse.net>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> ---
Hi Ding,
I think there's a possible race condition which could lead to inconsistent
state because you set slave->should_notify to 0 under RTNL but
__disable_port can update it without RTNL e.g. can be called via
bond_3ad_state_machine_handler -> ad_agg_selection_logic so in theory (I
haven't tested it) they can execute concurrently. This is not a big deal
though, but it would make this kind of message unreliable.

Nik

^ permalink raw reply

* Dear
From: Dr. Pius Okagba @ 2014-02-18 11:37 UTC (permalink / raw)


Attention:
Regarding the transaction made so far, I waited for your message as you told 
me
with none received. Remember, I supposed to have traveled last night 
but the
weather is too bad. I will be leaving to Paraguay tomorrow for estate project.
Meanwhile, contact the Bank manager with below address, i have kept the cheque
worth of USD2.5M. They will either mail it to you or remit it for transfer
depending on how you want it; Here is Bank contact information:
 
Name Of Manager: Mr. Franklin Barnard
MAIL: (franklinb1@barid.com,
okagbapius1@live.com
Thank you
UN Office.

^ permalink raw reply

* [PATCH 6/6] drivers:net:wireless:airo.c:checkpatch.pl cleanup
From: avi.kp.137-Re5JQEeQqe8AvxtiuMwx3w @ 2014-02-18 11:34 UTC (permalink / raw)
  To: linville-2XuSBdqkA4R54TAoqtyWWQ
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Avinash kumar

From: Avinash kumar <avi.kp.137-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

removed following warnings-
drivers/net/wireless/airo.c:39: WARNING: Use #include <linux/io.h> instead of <asm/io.h>
drivers/net/wireless/airo.c:48: WARNING: Use #include <linux/uaccess.h> instead of <asm/uaccess.h>

Signed-off-by: Avinash Kumar <avi.kp.137-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/net/wireless/airo.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
index edf4b57..79c4a76 100644
--- a/drivers/net/wireless/airo.c
+++ b/drivers/net/wireless/airo.c
@@ -36,7 +36,7 @@
 #include <linux/bitops.h>
 #include <linux/scatterlist.h>
 #include <linux/crypto.h>
-#include <asm/io.h>
+#include <linux/io.h>
 #include <asm/unaligned.h>
 
 #include <linux/netdevice.h>
@@ -45,7 +45,7 @@
 #include <linux/if_arp.h>
 #include <linux/ioport.h>
 #include <linux/pci.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 #include <linux/kthread.h>
 #include <linux/freezer.h>
 
-- 
1.7.9.5

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

^ permalink raw reply related

* [PATCH net-next] bonding: Fix RTNL: assertion failed at net/core/rtnetlink.c for 802.3ad mode
From: Ding Tianhong @ 2014-02-18 11:25 UTC (permalink / raw)
  To: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico, Cong Wang,
	Thomas Glanzmann, Jiri Pirko, David S. Miller, Eric Dumazet,
	Scott Feldman, Netdev

The problem was introduced by the commit 1d3ee88ae0d
(bonding: add netlink attributes to slave link dev).
The bond_set_active_slave() and bond_set_backup_slave()
will use rtmsg_ifinfo to send slave's states, so these
two functions should be called in RTNL.

In 802.3ad mode, acquiring RTNL for the __enable_port and
__disable_port cases is difficult, as those calls generally
already hold the state machine lock, and cannot unconditionally
call rtnl_lock because either they already hold RTNL (for calls
via bond_3ad_unbind_slave) or due to the potential for deadlock
with bond_3ad_adapter_speed_changed, bond_3ad_adapter_duplex_changed,
bond_3ad_link_change, or bond_3ad_update_lacp_rate.  All four of
those are called with RTNL held, and acquire the state machine lock
second.  The calling contexts for __enable_port and __disable_port
already hold the state machine lock, and may or may not need RTNL.

According to the Jay's opinion, I don't think it is a problem that
the slave don't send notify message synchronously when the status
changed, normally the state machine is running every 100 ms, send
the notify message at the end of the state machine if the slave's
state changed should be better.

I fix the problem through these steps:

1). add a new function bond_set_slave_state() which could change
    the slave's state and call rtmsg_ifinfo() according to the input
    parameters called notify.

2). Add a new slave parameter which called should_notify, if the slave's state
    changed and don't notify yet, the parameter will be set to 1, and then if
    the slave's state changed again, the param will be set to 0, it indicate that
    the slave's state has been restored, no need to notify any one.

3). the __enable_port and __disable_port should not call rtmsg_ifinfo
    in the state machine lock, any change in the state of slave could
    set a flag in the slave, it will indicated that an rtmsg_ifinfo
    should be called at the end of the state machine.

Cc: Jay Vosburgh <fubar@us.ibm.com>
Cc: Veaceslav Falico <vfalico@redhat.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 drivers/net/bonding/bond_3ad.c  | 22 ++++++++++++++++++++--
 drivers/net/bonding/bond_main.c | 30 +++++++++++++++---------------
 drivers/net/bonding/bonding.h   | 31 ++++++++++++++++++++++++++-----
 3 files changed, 61 insertions(+), 22 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index e9edd84..c450d04 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -181,7 +181,7 @@ static inline int __agg_has_partner(struct aggregator *agg)
  */
 static inline void __disable_port(struct port *port)
 {
-	bond_set_slave_inactive_flags(port->slave);
+	bond_set_slave_inactive_flags(port->slave, false);
 }
 
 /**
@@ -193,7 +193,7 @@ static inline void __enable_port(struct port *port)
 	struct slave *slave = port->slave;
 
 	if ((slave->link == BOND_LINK_UP) && IS_UP(slave->dev))
-		bond_set_slave_active_flags(slave);
+		bond_set_slave_active_flags(slave, false);
 }
 
 /**
@@ -2065,6 +2065,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 	struct list_head *iter;
 	struct slave *slave;
 	struct port *port;
+	int slave_should_notify = 0;
 
 	read_lock(&bond->lock);
 	rcu_read_lock();
@@ -2122,8 +2123,25 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 	}
 
 re_arm:
+	bond_for_each_slave_rcu(bond, slave, iter) {
+		if (slave->should_notify) {
+			slave_should_notify = 1;
+			break;
+		}
+	}
 	rcu_read_unlock();
 	read_unlock(&bond->lock);
+
+	if (slave_should_notify && rtnl_trylock()) {
+		bond_for_each_slave(bond, slave, iter) {
+			if (slave->should_notify) {
+				rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0,
+					     GFP_KERNEL);
+				slave->should_notify = 0;
+			}
+		}
+		rtnl_unlock();
+	}
 	queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
 }
 
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 3bce855..1c14e64 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -829,21 +829,21 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
 	if (bond_is_lb(bond)) {
 		bond_alb_handle_active_change(bond, new_active);
 		if (old_active)
-			bond_set_slave_inactive_flags(old_active);
+			bond_set_slave_inactive_flags(old_active, true);
 		if (new_active)
-			bond_set_slave_active_flags(new_active);
+			bond_set_slave_active_flags(new_active, true);
 	} else {
 		rcu_assign_pointer(bond->curr_active_slave, new_active);
 	}
 
 	if (bond->params.mode == BOND_MODE_ACTIVEBACKUP) {
 		if (old_active)
-			bond_set_slave_inactive_flags(old_active);
+			bond_set_slave_inactive_flags(old_active, true);
 
 		if (new_active) {
 			bool should_notify_peers = false;
 
-			bond_set_slave_active_flags(new_active);
+			bond_set_slave_active_flags(new_active, true);
 
 			if (bond->params.fail_over_mac)
 				bond_do_fail_over_mac(bond, new_active,
@@ -1462,14 +1462,14 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 
 	switch (bond->params.mode) {
 	case BOND_MODE_ACTIVEBACKUP:
-		bond_set_slave_inactive_flags(new_slave);
+		bond_set_slave_inactive_flags(new_slave, true);
 		break;
 	case BOND_MODE_8023AD:
 		/* in 802.3ad mode, the internal mechanism
 		 * will activate the slaves in the selected
 		 * aggregator
 		 */
-		bond_set_slave_inactive_flags(new_slave);
+		bond_set_slave_inactive_flags(new_slave, true);
 		/* if this is the first slave */
 		if (!prev_slave) {
 			SLAVE_AD_INFO(new_slave).id = 1;
@@ -1487,7 +1487,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 	case BOND_MODE_TLB:
 	case BOND_MODE_ALB:
 		bond_set_active_slave(new_slave);
-		bond_set_slave_inactive_flags(new_slave);
+		bond_set_slave_inactive_flags(new_slave, true);
 		break;
 	default:
 		pr_debug("This slave is always active in trunk mode\n");
@@ -2009,7 +2009,7 @@ static void bond_miimon_commit(struct bonding *bond)
 
 			if (bond->params.mode == BOND_MODE_ACTIVEBACKUP ||
 			    bond->params.mode == BOND_MODE_8023AD)
-				bond_set_slave_inactive_flags(slave);
+				bond_set_slave_inactive_flags(slave, true);
 
 			pr_info("%s: link status definitely down for interface %s, disabling it\n",
 				bond->dev->name, slave->dev->name);
@@ -2555,7 +2555,7 @@ static void bond_ab_arp_commit(struct bonding *bond)
 				slave->link = BOND_LINK_UP;
 				if (bond->current_arp_slave) {
 					bond_set_slave_inactive_flags(
-						bond->current_arp_slave);
+						bond->current_arp_slave, true);
 					bond->current_arp_slave = NULL;
 				}
 
@@ -2575,7 +2575,7 @@ static void bond_ab_arp_commit(struct bonding *bond)
 				slave->link_failure_count++;
 
 			slave->link = BOND_LINK_DOWN;
-			bond_set_slave_inactive_flags(slave);
+			bond_set_slave_inactive_flags(slave, true);
 
 			pr_info("%s: link status definitely down for interface %s, disabling it\n",
 				bond->dev->name, slave->dev->name);
@@ -2650,7 +2650,7 @@ static bool bond_ab_arp_probe(struct bonding *bond)
 		}
 	}
 
-	bond_set_slave_inactive_flags(curr_arp_slave);
+	bond_set_slave_inactive_flags(curr_arp_slave, true);
 
 	bond_for_each_slave(bond, slave, iter) {
 		if (!found && !before && IS_UP(slave->dev))
@@ -2670,7 +2670,7 @@ static bool bond_ab_arp_probe(struct bonding *bond)
 			if (slave->link_failure_count < UINT_MAX)
 				slave->link_failure_count++;
 
-			bond_set_slave_inactive_flags(slave);
+			bond_set_slave_inactive_flags(slave, true);
 
 			pr_info("%s: backup interface %s is now down\n",
 				bond->dev->name, slave->dev->name);
@@ -2688,7 +2688,7 @@ static bool bond_ab_arp_probe(struct bonding *bond)
 	}
 
 	new_slave->link = BOND_LINK_BACK;
-	bond_set_slave_active_flags(new_slave);
+	bond_set_slave_active_flags(new_slave, true);
 	bond_arp_send_all(bond, new_slave);
 	new_slave->jiffies = jiffies;
 	rcu_assign_pointer(bond->current_arp_slave, new_slave);
@@ -3035,9 +3035,9 @@ static int bond_open(struct net_device *bond_dev)
 		bond_for_each_slave(bond, slave, iter) {
 			if ((bond->params.mode == BOND_MODE_ACTIVEBACKUP)
 				&& (slave != bond->curr_active_slave)) {
-				bond_set_slave_inactive_flags(slave);
+				bond_set_slave_inactive_flags(slave, true);
 			} else {
-				bond_set_slave_active_flags(slave);
+				bond_set_slave_active_flags(slave, true);
 			}
 		}
 		read_unlock(&bond->curr_slave_lock);
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 86ccfb9..1f51a5f 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -195,7 +195,8 @@ struct slave {
 	s8     new_link;
 	u8     backup:1,   /* indicates backup slave. Value corresponds with
 			      BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
-	       inactive:1; /* indicates inactive slave */
+	       inactive:1, /* indicates inactive slave */
+	       should_notify:1; /* indicateds whether the state changed */
 	u8     duplex;
 	u32    original_mtu;
 	u32    link_failure_count;
@@ -303,6 +304,24 @@ static inline void bond_set_backup_slave(struct slave *slave)
 	}
 }
 
+static inline void bond_set_slave_state(struct slave *slave,
+					int slave_state, bool notify)
+{
+	if (slave->backup == slave_state)
+		return;
+
+	slave->backup = slave_state;
+	if (notify) {
+		rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_KERNEL);
+		slave->should_notify = 0;
+	} else {
+		if (slave->should_notify)
+			slave->should_notify = 0;
+		else
+			slave->should_notify = 1;
+	}
+}
+
 static inline void bond_slave_state_change(struct bonding *bond)
 {
 	struct list_head *iter;
@@ -394,17 +413,19 @@ static inline void bond_netpoll_send_skb(const struct slave *slave,
 }
 #endif
 
-static inline void bond_set_slave_inactive_flags(struct slave *slave)
+static inline void bond_set_slave_inactive_flags(struct slave *slave,
+						 bool notify)
 {
 	if (!bond_is_lb(slave->bond))
-		bond_set_backup_slave(slave);
+		bond_set_slave_state(slave, BOND_STATE_BACKUP, notify);
 	if (!slave->bond->params.all_slaves_active)
 		slave->inactive = 1;
 }
 
-static inline void bond_set_slave_active_flags(struct slave *slave)
+static inline void bond_set_slave_active_flags(struct slave *slave,
+					       bool notify)
 {
-	bond_set_active_slave(slave);
+	bond_set_slave_state(slave, BOND_STATE_ACTIVE, notify);
 	slave->inactive = 0;
 }
 
-- 
1.8.0

^ permalink raw reply related

* Re: [Xen-devel] [RFC v2 3/4] xen-netback: use a random MAC address
From: Ian Campbell @ 2014-02-18 11:22 UTC (permalink / raw)
  To: David Vrabel
  Cc: Luis R. Rodriguez, netdev, Wei Liu, kvm, Luis R. Rodriguez,
	linux-kernel, Paul Durrant, xen-devel
In-Reply-To: <5301E496.40802@citrix.com>

On Mon, 2014-02-17 at 10:29 +0000, David Vrabel wrote:
> On 15/02/14 02:59, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> > 
> > The purpose of using a static MAC address of FE:FF:FF:FF:FF:FF
> > was to prevent our backend interfaces from being used by the
> > bridge and nominating our interface as a root bridge. This was
> > possible given that the bridge code will use the lowest MAC
> > address for a port once a new interface gets added to the bridge.
> > The bridge code has a generic feature now to allow interfaces
> > to opt out from root bridge nominations, use that instead.
> [...]
> > --- a/drivers/net/xen-netback/interface.c
> > +++ b/drivers/net/xen-netback/interface.c
> > @@ -42,6 +42,8 @@
> >  #define XENVIF_QUEUE_LENGTH 32
> >  #define XENVIF_NAPI_WEIGHT  64
> >  
> > +static const u8 xen_oui[3] = { 0x00, 0x16, 0x3e };
> 
> You shouldn't use a vendor prefix with a random MAC address.  You should
> set the locally administered bit and clear the multicast/unicast bit and
> randomize the remaining 46 bits.

I'd have thought that eth_hw_addr_random would get this right, *checks*
yes it does. And then this patch tramples overt the top three bytes.

Might there be any requirement to have a specific MAC on the vif device?
IOW do we need to figure out a way to plumb this through the Xen tools
(perhaps having the vif script sort it out).

Speaking of which -- do the Xen tools not overwrite this random mac from
xen-network-common.sh:_setup_bridge_port. What is the plan to change
that (in a forwards/backwards compatible manner).

Ian.

^ permalink raw reply

* Re: [PATCH v5 net-next 02/12] bonding: permit using arp_validate with non-ab modes
From: Nikolay Aleksandrov @ 2014-02-18 11:21 UTC (permalink / raw)
  To: Veaceslav Falico, netdev; +Cc: Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1392706127-28390-3-git-send-email-vfalico@redhat.com>

On 02/18/2014 07:48 AM, Veaceslav Falico wrote:
> Currently it's disabled because it's sometimes hard, in typical configs, to
> make it work - because of the nature how the loadbalance modes work - as
> it's hard to deliver valid arp replies to correct slaves by the switch.
> 
> However we still can use arp_validation in loadbalance with several other
> configs, per example with arp_validate == 2 for backup with one broadcast
> domain, without the switch(es) doing any balancing - this way we'd be (a
> bit more) sure that the slave is up.
> 
> So, enable it to let users decide which one works/suits them best. Also
> correct the mode limitation from BOND_OPT_ARP_VALIDATE.
> 
> CC: Nikolay Aleksandrov <nikolay@redhat.com>
> CC: Jay Vosburgh <fubar@us.ibm.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
> ---

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

^ permalink raw reply

* (unknown)
From: Rocky View Schools Community Learning @ 2014-02-17 19:41 UTC (permalink / raw)


Your Email has won (£1,373,420 pounds) in our
British Promotion.contact us for details via:
lottery_b1@yahoo.com


Sir Edmond Newton


_____________________________________________________________________________________
This communication is intended for the use of the recipient to which it
is addressed, and may contain confidential, personal, and or privileged
information. Please contact us immediately if you are not the intended
recipient of this communication, and do not copy, distribute, or take
action relying on it. Any communication received in error, or subsequent
reply, should be deleted or destroyed.

^ permalink raw reply

* Re: [PATCH v2] ethtool: check the ethtool_ops is NULL in dev_ethtool
From: Wang Weidong @ 2014-02-18 10:41 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Daniel Borkmann
In-Reply-To: <5302BAA0.9060905@huawei.com>

Just ignore it.

Regards
Wang

On 2014/2/18 9:42, Wang Weidong wrote:
> some drivers maybe not implement the ethtool_ops with only
> set NULL to ethtool_ops. So when call the ethtool devx will
> lead to a 'NULL pointer dereference'.
> 
> So add a check in dev_ethtool
> 
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> ---
> Change note:
> 
> v2: fix a trailing whitespace/tab pointed out by Daniel
> 
> ---
>  net/core/ethtool.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 30071de..c8cfd8f 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -1500,6 +1500,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
>  			return -EPERM;
>  	}
>  
> +	if (!dev->ethtool_ops)
> +		return -EOPNOTSUPP;
> +
>  	if (dev->ethtool_ops->begin) {
>  		rc = dev->ethtool_ops->begin(dev);
>  		if (rc  < 0)
> 

^ permalink raw reply

* Re: [PATCH] ethtool: check the ethtool_ops is NULL in dev_ethtool
From: Wang Weidong @ 2014-02-18 10:40 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: David Miller, netdev
In-Reply-To: <53032639.3060701@redhat.com>

On 2014/2/18 17:22, Daniel Borkmann wrote:
> On 02/18/2014 02:20 AM, Wang Weidong wrote:
>> On 2014/2/18 1:09, Daniel Borkmann wrote:
>>> On 02/17/2014 12:31 PM, Wang Weidong wrote:
>>>> some drivers maybe not implement the ethtool_ops with only
>>>> set NULL. So when call the ethtool cmds will lead to a
>>>> 'NULL pointer dereference'.
>>>>
>>>> So add a checking in dev_ethtool.
>>>>
>>>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
>>>> ---
>>>>    net/core/ethtool.c | 3 +++
>>>>    1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>>>> index 30071de..f418dcb 100644
>>>> --- a/net/core/ethtool.c
>>>> +++ b/net/core/ethtool.c
>>>> @@ -1499,6 +1499,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
>>>>            if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
>>>>                return -EPERM;
>>>>        }
>>>> +
>>>
>>> You have a trailing whitespace/tab in the line above. Please
>>> use checkpatch for detecting such things.
>>>
>> Sorry for that. I will fix it soon.
>>
>>> Can you be more specific with "some drivers"? Any driver that
>>> is in the mainline tree?
>>>
>> No. My team implements a driver without considering the ethtool_ops.
>> So I got the problem.
> 
> If it's code that is out of the mainline tree, then why should the
> kernel support that? Submit your driver to the tree first, and then
> such a change could be considered. Or, even better, let them implement
> ethtool ops/stubs.
> 
Thanks for your reply. I will suggest them to implement ethtool ops.

Regards
Wang

>>>> +    if (!dev->ethtool_ops)
>>>> +        return -EOPNOTSUPP;
>>>>
>>>>        if (dev->ethtool_ops->begin) {
>>>>            rc = dev->ethtool_ops->begin(dev);
>>>>
>>>
>>>
>>
>>
> 
> 

^ 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