Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] drivers/net: fix build warnings with CONFIG_PM_SLEEP disabled
From: Stephen Hemminger @ 2011-03-07 23:07 UTC (permalink / raw)
  To: Michel Lespinasse
  Cc: David S. Miller, netdev, Andrew Morton, Linus Torvalds,
	linux-kernel
In-Reply-To: <20110307021450.GC31188@google.com>

On Sun, 6 Mar 2011 18:14:50 -0800
Michel Lespinasse <walken@google.com> wrote:

> This fixes a couple of build warnings when CONFIG_PM is enabled but
> CONFIG_PM_SLEEP is disabled. Applies on top of v2.6.38-rc7 - I know it's
> late, but it would be great if v2.6.38 could compile without warnings!
> 
> Signed-off-by: Michel Lespinasse <walken@google.com>

This is standard throughout all the network drivers.
Why just fix 2?


-- 

^ permalink raw reply

* Re: [patch net-next-2.6] net: reinject arps into bonding slave instead of master
From: Andy Gospodarek @ 2011-03-07 22:43 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Nicolas de Pesloüan, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet, andy
In-Reply-To: <20110307125059.GA6053@psychotron.brq.redhat.com>

On Mon, Mar 07, 2011 at 01:51:00PM +0100, Jiri Pirko wrote:
> Recent patch "bonding: move processing of recv handlers into
> handle_frame()" caused a regression on following net scheme:
> 
> eth0 - bond0 - bond0.5
> 
> where arp monitoring is happening over vlan. This patch fixes it by
> reinjecting the arp packet into bonding slave device so the bonding
> rx_handler can pickup and process it.
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> ---
>  net/core/dev.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index c71bd18..3d88458 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3094,12 +3094,12 @@ void netdev_rx_handler_unregister(struct net_device *dev)
>  }
>  EXPORT_SYMBOL_GPL(netdev_rx_handler_unregister);
>  
> -static void vlan_on_bond_hook(struct sk_buff *skb)
> +static void vlan_on_bond_hook(struct sk_buff *skb, struct net_device *orig_dev)
>  {
>  	/*
>  	 * Make sure ARP frames received on VLAN interfaces stacked on
>  	 * bonding interfaces still make their way to any base bonding
> -	 * device that may have registered for a specific ptype.
> +	 * device by reinjecting the frame into bonding slave (orig_dev)
>  	 */
>  	if (skb->dev->priv_flags & IFF_802_1Q_VLAN &&
>  	    vlan_dev_real_dev(skb->dev)->priv_flags & IFF_BONDING &&
> @@ -3108,7 +3108,7 @@ static void vlan_on_bond_hook(struct sk_buff *skb)
>  
>  		if (!skb2)
>  			return;
> -		skb2->dev = vlan_dev_real_dev(skb->dev);
> +		skb2->dev = orig_dev;
>  		netif_rx(skb2);
>  	}
>  }
> @@ -3202,7 +3202,7 @@ ncls:
>  			goto out;
>  	}
>  
> -	vlan_on_bond_hook(skb);
> +	vlan_on_bond_hook(skb, orig_dev);
>  
>  	/* deliver only exact match when indicated */
>  	null_or_dev = deliver_exact ? skb->dev : NULL;

This patch doesn't work.

My setup has bond0.100 -> bond0 -> eth2 and eth3.  ARP monitoring is
enabled as is arp_valiate.

The initial problem was just that just before vlan_on_bond_hook is
called, skb->dev = bond0.100 and orig_dev = eth2.   (This is after
running goto another_route and having been called back through
__netif_receive_skb since vlan_hwaccel_do_receive it true.)

Now vlan_on_bond_hook is called and we have 2 skbs.

The original skb still have skb->dev = bond0.100 and orig_dev = eth2.
Since bond_arp_rcv is registered for traffic only to bond0, the handler
is not hit and the frame is dropped (or processed by another handler).

The cloned skb has skb->dev = bond0 and is put back on the receive queue
and comes back through __netif_receive_skb.  This frame will match the
ptype entry for bond_arp_rcv, but since orig_dev = bond0 in this case,
the code in bond_arp_rcv will not handle the frame.  

If we truly want to track the original interface that received the
frame, the following is a better option.  With the recursive nature of
__netif_receive_skb at this point, we should really consider setting
orig_dev from skb_iif rather than just from skb->dev.

diff --git a/net/core/dev.c b/net/core/dev.c
index 30440e7..500fdbc 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3135,7 +3135,6 @@ static int __netif_receive_skb(struct sk_buff *skb)
 
 	if (!skb->skb_iif)
 		skb->skb_iif = skb->dev->ifindex;
-	orig_dev = skb->dev;
 
 	skb_reset_network_header(skb);
 	skb_reset_transport_header(skb);
@@ -3145,6 +3144,7 @@ static int __netif_receive_skb(struct sk_buff *skb)
 
 	rcu_read_lock();
 
+	orig_dev = dev_get_by_index_rcu(dev_net(skb->dev),skb->skb_iif);
 another_round:
 
 	__this_cpu_inc(softnet_data.processed);


^ permalink raw reply related

* Re: [PATCH 13/17] mlx4: Add blue flame support for kernel consumers
From: David Miller @ 2011-03-07 22:09 UTC (permalink / raw)
  To: eli; +Cc: roland, netdev
In-Reply-To: <20110307215803.GB19540@mtldesk30>

From: Eli Cohen <eli@dev.mellanox.co.il>
Date: Mon, 7 Mar 2011 23:58:03 +0200

> On Mon, Mar 07, 2011 at 01:49:31PM -0800, David Miller wrote:
>> 
>> It's a performance optimization, if you don't get write combining you'll
>> get more strict ordering, rather than less.
>> 
>> It cannot cause problem.
> 
> I agree, but the function could still fail and the caller's logic
> could attempt to call ioreamp or take other action. For example, in
> the case of blue flame, it is better performance-wise to avoid using
> this feature if write combining is not available.

It could, but the less complicated the interfaces the better.

^ permalink raw reply

* Re: [PATCH 13/17] mlx4: Add blue flame support for kernel consumers
From: Eli Cohen @ 2011-03-07 21:58 UTC (permalink / raw)
  To: David Miller; +Cc: roland, netdev
In-Reply-To: <20110307.134931.232736253.davem@davemloft.net>

On Mon, Mar 07, 2011 at 01:49:31PM -0800, David Miller wrote:
> 
> It's a performance optimization, if you don't get write combining you'll
> get more strict ordering, rather than less.
> 
> It cannot cause problem.

I agree, but the function could still fail and the caller's logic
could attempt to call ioreamp or take other action. For example, in
the case of blue flame, it is better performance-wise to avoid using
this feature if write combining is not available.

^ permalink raw reply

* Re: [PATCH] bridge:  control carrier based on ports online
From: Nicolas de Pesloüan @ 2011-03-07 21:51 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: David S. Miller, Adam Majer, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, bridge, netdev, Andy Gospodarek, Jay Vosburgh
In-Reply-To: <20110307134450.652aea32@nehalam>

Le 07/03/2011 22:44, Stephen Hemminger a écrit :
> On Mon, 07 Mar 2011 21:48:16 +0100
> Nicolas de Pesloüan<nicolas.2p.debian@gmail.com>  wrote:
>
>> Le 07/03/2011 19:34, Stephen Hemminger a écrit :
[snip]
>>>    	list_for_each_entry(p,&br->port_list, list) {
>>> -		if (p->state != BR_STATE_DISABLED) {
>>> -			if (p->port_no == br->root_port) {
>>> -				p->config_pending = 0;
>>> -				p->topology_change_ack = 0;
>>> -				br_make_forwarding(p);
>>> -			} else if (br_is_designated_port(p)) {
>>> -				del_timer(&p->message_age_timer);
>>> -				br_make_forwarding(p);
>>> -			} else {
>>> -				p->config_pending = 0;
>>> -				p->topology_change_ack = 0;
>>> -				br_make_blocking(p);
>>> -			}
>>> +		if (p->state == BR_STATE_DISABLED)
>>> +			continue;
>>> +
>>> +		if (p->port_no == br->root_port) {
>>> +			p->config_pending = 0;
>>> +			p->topology_change_ack = 0;
>>> +			br_make_forwarding(p);
>>> +		} else if (br_is_designated_port(p)) {
>>> +			del_timer(&p->message_age_timer);
>>> +			br_make_forwarding(p);
>>> +		} else {
>>> +			p->config_pending = 0;
>>> +			p->topology_change_ack = 0;
>>> +			br_make_blocking(p);
>>
>> Is the above part really related to the purpose of this patch? It looks like (good) cleanup, but
>> should be in a different patch.
>>
>> Except from this comment,
>>
>> Reviewed-by: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
>
> The loop is going over the state of ports.
> Since the new code at the end of loop has to check for STATE_FORWARDING
> it is clearer with continue statement.  When adding code it is always
> better to clarify the logic in the process rather than making it
> more complex.

Sound's good to me. Thanks for clarifying.

	Nicolas.

^ permalink raw reply

* Re: [PATCH 13/17] mlx4: Add blue flame support for kernel consumers
From: David Miller @ 2011-03-07 21:50 UTC (permalink / raw)
  To: eli; +Cc: roland, netdev
In-Reply-To: <20110307.134931.232736253.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Mon, 07 Mar 2011 13:49:31 -0800 (PST)

> From: Eli Cohen <eli@dev.mellanox.co.il>
> Date: Mon, 7 Mar 2011 23:48:12 +0200
> 
>> On Mon, Mar 07, 2011 at 01:40:01PM -0800, David Miller wrote:
>>> From: Eli Cohen <eli@dev.mellanox.co.il>
>>> Date: Mon, 7 Mar 2011 23:36:48 +0200
>>> 
>>> > Specifically, some archs may not support write combining.
>>> 
>>> They should just create a non-write-combining mapping if they
>>> don't support it.
>>> 
>> 
>> I wouldn't expect that since the caller function could be mislead to
>> believe it has a write combining capable area.
> 
> It's a performance optimization, if you don't get write combining you'll
> get more strict ordering, rather than less.
> 
> It cannot cause problem.

BTW, if we did as you suggest, fail if we don't support write combining,
then half the drivers in the tree would fail to probe on sparc64.

Every other driver expects it to succeed, with either write-combining
or more strict ordering semantics.  Never to fail simply because
write-combining isn't supported.

It's a request, not a requirement.


^ permalink raw reply

* Re: [PATCH 13/17] mlx4: Add blue flame support for kernel consumers
From: David Miller @ 2011-03-07 21:49 UTC (permalink / raw)
  To: eli; +Cc: roland, netdev
In-Reply-To: <20110307214812.GA19540@mtldesk30>

From: Eli Cohen <eli@dev.mellanox.co.il>
Date: Mon, 7 Mar 2011 23:48:12 +0200

> On Mon, Mar 07, 2011 at 01:40:01PM -0800, David Miller wrote:
>> From: Eli Cohen <eli@dev.mellanox.co.il>
>> Date: Mon, 7 Mar 2011 23:36:48 +0200
>> 
>> > Specifically, some archs may not support write combining.
>> 
>> They should just create a non-write-combining mapping if they
>> don't support it.
>> 
> 
> I wouldn't expect that since the caller function could be mislead to
> believe it has a write combining capable area.

It's a performance optimization, if you don't get write combining you'll
get more strict ordering, rather than less.

It cannot cause problem.

^ permalink raw reply

* Re: [PATCH 13/17] mlx4: Add blue flame support for kernel consumers
From: Eli Cohen @ 2011-03-07 21:48 UTC (permalink / raw)
  To: David Miller; +Cc: roland, netdev
In-Reply-To: <20110307.134001.245386278.davem@davemloft.net>

On Mon, Mar 07, 2011 at 01:40:01PM -0800, David Miller wrote:
> From: Eli Cohen <eli@dev.mellanox.co.il>
> Date: Mon, 7 Mar 2011 23:36:48 +0200
> 
> > Specifically, some archs may not support write combining.
> 
> They should just create a non-write-combining mapping if they
> don't support it.
> 

I wouldn't expect that since the caller function could be mislead to
believe it has a write combining capable area.

^ permalink raw reply

* Re: [PATCH] bridge:  control carrier based on ports online
From: Stephen Hemminger @ 2011-03-07 21:44 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: David S. Miller, Adam Majer, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, bridge, netdev, Andy Gospodarek, Jay Vosburgh
In-Reply-To: <4D754490.4000105@gmail.com>

On Mon, 07 Mar 2011 21:48:16 +0100
Nicolas de Pesloüan <nicolas.2p.debian@gmail.com> wrote:

> Le 07/03/2011 19:34, Stephen Hemminger a écrit :
> > This makes the bridge device behave like a physical device.
> > In earlier releases the bridge always asserted carrier. This
> > changes the behavior so that bridge device carrier is on only
> > if one or more ports are in the forwarding state. This
> > should help IPv6 autoconfiguration, DHCP, and routing daemons.
> >
> > I did brief testing with Network and Virt manager and they
> > seem fine, but since this changes behavior of bridge, it should
> > wait until net-next (2.6.39).
> >
> > Signed-off-by: Stephen Hemminger<shemminger@vyatta.com>
> >
> > ---
> >   net/bridge/br_device.c    |    4 ++++
> >   net/bridge/br_stp.c       |   35 ++++++++++++++++++++++-------------
> >   net/bridge/br_stp_timer.c |    1 +
> >   3 files changed, 27 insertions(+), 13 deletions(-)
> >
> > --- a/net/bridge/br_device.c	2011-03-07 08:40:08.913599513 -0800
> > +++ b/net/bridge/br_device.c	2011-03-07 08:40:48.382377389 -0800
> > @@ -78,6 +78,8 @@ static int br_dev_open(struct net_device
> >   {
> >   	struct net_bridge *br = netdev_priv(dev);
> >
> > +	netif_carrier_off(dev);
> > +
> >   	br_features_recompute(br);
> >   	netif_start_queue(dev);
> >   	br_stp_enable_bridge(br);
> > @@ -94,6 +96,8 @@ static int br_dev_stop(struct net_device
> >   {
> >   	struct net_bridge *br = netdev_priv(dev);
> >
> > +	netif_carrier_off(dev);
> > +
> >   	br_stp_disable_bridge(br);
> >   	br_multicast_stop(br);
> >
> > --- a/net/bridge/br_stp.c	2011-03-07 08:41:58.619783678 -0800
> > +++ b/net/bridge/br_stp.c	2011-03-07 08:53:58.953558810 -0800
> > @@ -397,28 +397,37 @@ static void br_make_forwarding(struct ne
> >   void br_port_state_selection(struct net_bridge *br)
> >   {
> >   	struct net_bridge_port *p;
> > +	unsigned int liveports = 0;
> >
> >   	/* Don't change port states if userspace is handling STP */
> >   	if (br->stp_enabled == BR_USER_STP)
> >   		return;
> >
> >   	list_for_each_entry(p,&br->port_list, list) {
> > -		if (p->state != BR_STATE_DISABLED) {
> > -			if (p->port_no == br->root_port) {
> > -				p->config_pending = 0;
> > -				p->topology_change_ack = 0;
> > -				br_make_forwarding(p);
> > -			} else if (br_is_designated_port(p)) {
> > -				del_timer(&p->message_age_timer);
> > -				br_make_forwarding(p);
> > -			} else {
> > -				p->config_pending = 0;
> > -				p->topology_change_ack = 0;
> > -				br_make_blocking(p);
> > -			}
> > +		if (p->state == BR_STATE_DISABLED)
> > +			continue;
> > +
> > +		if (p->port_no == br->root_port) {
> > +			p->config_pending = 0;
> > +			p->topology_change_ack = 0;
> > +			br_make_forwarding(p);
> > +		} else if (br_is_designated_port(p)) {
> > +			del_timer(&p->message_age_timer);
> > +			br_make_forwarding(p);
> > +		} else {
> > +			p->config_pending = 0;
> > +			p->topology_change_ack = 0;
> > +			br_make_blocking(p);
> 
> Is the above part really related to the purpose of this patch? It looks like (good) cleanup, but 
> should be in a different patch.
> 
> Except from this comment,
> 
> Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

The loop is going over the state of ports.
Since the new code at the end of loop has to check for STATE_FORWARDING
it is clearer with continue statement.  When adding code it is always
better to clarify the logic in the process rather than making it
more complex.


-- 

^ permalink raw reply

* Re: [PATCH 13/17] mlx4: Add blue flame support for kernel consumers
From: David Miller @ 2011-03-07 21:40 UTC (permalink / raw)
  To: eli; +Cc: roland, netdev
In-Reply-To: <20110307213648.GA18682@mtldesk30>

From: Eli Cohen <eli@dev.mellanox.co.il>
Date: Mon, 7 Mar 2011 23:36:48 +0200

> Specifically, some archs may not support write combining.

They should just create a non-write-combining mapping if they
don't support it.

It could still fail due to resource constraints, but not because
of the reason you're stating.

^ permalink raw reply

* Re: FW: [PATCH 13/17] mlx4: Add blue flame support for kernel consumers
From: Eli Cohen @ 2011-03-07 21:36 UTC (permalink / raw)
  To: roland, davem; +Cc: davem, netdev
In-Reply-To: <CFE9BFE80FFE4D4892AA5D31387E310F02F3ED@mtldag01.mtl.com>

> 
> On Mon, Mar 7, 2011 at 2:17 AM, Yevgeny Petrilin
> <yevgenyp@mellanox.co.il> wrote:
> > +       if (map_bf_area(dev))
> > +               mlx4_dbg(dev, "Kernel support for blue flame is not available for kernels < 2.6.28\n");
> 
> This seems like a really bad error message.  Can map_bf_area() actually fail?

I agree that this message is inappropriate here; it is originiated
from the OFED patches which support older kernels too.
As for the question if a message is justified here at all, I think the
answer is yes becuase of this:

+static int map_bf_area(struct mlx4_dev *dev)
+{
+       struct mlx4_priv *priv = mlx4_priv(dev);
+       resource_size_t bf_start;
+       resource_size_t bf_len;
+       int err = 0;
+
+       bf_start = pci_resource_start(dev->pdev, 2) +
(dev->caps.num_uars << PAGE_SHIFT);
+       bf_len = pci_resource_len(dev->pdev, 2) - (dev->caps.num_uars
<< PAGE_SHIFT);
+       priv->bf_mapping = io_mapping_create_wc(bf_start, bf_len);
+       if (!priv->bf_mapping)
+               err = -ENOMEM;

Specifically, some archs may not support write combining.
 

^ permalink raw reply

* Re: [patch net-next-2.6] net: reinject arps into bonding slave instead of master
From: Nicolas de Pesloüan @ 2011-03-07 21:30 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Andy Gospodarek, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet
In-Reply-To: <20110307211957.GB3266@psychotron.redhat.com>

Le 07/03/2011 22:19, Jiri Pirko a écrit :
>>
>> Andy, while you are testing it, can you ensure it also works for the following setup?
>>
>> eth0 ->  bond0 ->  br0 ->  br0.100
>>
>
> I do not think this will work (it never did).

Yes, but it should...

> Bridge reinjects skb, does not do another loop.

Yes, but it should...

> Therefore when skb->dev == br0 and vlan_on_bond hook is called, orig_dev == br0 as well. + No
> IFF_BONDING is set.

If only bridge returned RX_HANDLER_ANOTHER with skb->dev changed to br0... instead of re-injecting 
the skb... One more reason to avoid re-injecting.

> I have some kind nice solution in mind and I'm going to submit that as a patch later (too many
> patches are in the wind atm).

Agreed. It's time for others to review, test and ack :-D

	Nicolas.

^ permalink raw reply

* Re: [PATCH 13/17] mlx4: Add blue flame support for kernel consumers
From: David Miller @ 2011-03-07 21:27 UTC (permalink / raw)
  To: roland; +Cc: yevgenyp, netdev, eli
In-Reply-To: <AANLkTi=uRL2GAb7CBqJiVUuXTgpX_w7=3PZ3JdmL_+jj@mail.gmail.com>

From: Roland Dreier <roland@purestorage.com>
Date: Mon, 7 Mar 2011 13:07:48 -0800

> On Mon, Mar 7, 2011 at 2:17 AM, Yevgeny Petrilin
> <yevgenyp@mellanox.co.il> wrote:
>> +       if (map_bf_area(dev))
>> +               mlx4_dbg(dev, "Kernel support for blue flame is not available for kernels < 2.6.28\n");
> 
> This seems like a really bad error message.  Can map_bf_area() actually fail?

Indeed, referencing specific old kernel versions in the upstream
driver makes no sense at all.

^ permalink raw reply

* Re: [patch net-next-2.6] net: reinject arps into bonding slave instead of master
From: Jiri Pirko @ 2011-03-07 21:19 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: Andy Gospodarek, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet
In-Reply-To: <4D753C40.2030502@gmail.com>

>
>Andy, while you are testing it, can you ensure it also works for the following setup?
>
>eth0 -> bond0 -> br0 -> br0.100
>

I do not think this will work (it never did). Bridge reinjects skb, does
not do another loop. Therefore when skb->dev == br0 and vlan_on_bond
hook is called, orig_dev == br0 as well. + No IFF_BONDING is set.

I have some kind nice solution in mind and I'm going to submit that as
a patch later (too many patches are in the wind atm).

Jirka


^ permalink raw reply

* Re: bonding can't change to another slave if you ifdown the active slave
From: Nicolas de Pesloüan @ 2011-03-07 21:15 UTC (permalink / raw)
  To: Weiping Pan; +Cc: Andy Gospodarek, netdev, bonding-devel, Linda Wang
In-Reply-To: <4D744D57.9050908@gmail.com>

Le 07/03/2011 04:13, Weiping Pan a écrit :
> On 03/05/2011 09:49 PM, Nicolas de Pesloüan wrote:
>> Le 05/03/2011 03:53, Andy Gospodarek a écrit :
>>> On Fri, Mar 04, 2011 at 10:15:17AM +0800, Weiping Pan wrote:
>>>> Hi,
>>>>
>>>> I'm doing some Linux bonding driver test, and I find a problem in
>>>> balance-rr mode.
>>>> That's it can't change to another slave if you ifdown the active slave.
>>>> Any comments are warmly welcomed!
>>>>
>>>> regards
>>>> Weiping Pan
>>>>
>>>> My host is Fedora 14, and I install VirtualBox (4.0.2), and enable 4
>>>> nics for the guest system.
>>>
>>> Does this mean you are passing 4 NICs from your host to your guest
>>> (maybe via direct pci-device assignment to the guest) or are you
>>> creating 4 virtual devices on the host that are in a bridge group on the
>>> host?
>>
>> VirtualBox does not allow assignment of pci-device to the guest. The
>> network interfaces on the guest are pure virtual one, with several
>> modes available. In order to help you trouble shooting this problem,
>> we need to know the mode form each of the virtual interfaces. Possible
>> modes are NAT, bridged, internal-network, and host-only-network.
>>
>> Please provide the output of the following command:
>>
>> VBoxManage showvminfo <your-vm-uuid> | grep ^NIC
>>
>> To display your vm uuid, use the following command:
>>
>> VBoxManage list vms
> [root@localhost ~]# VBoxManage showvminfo
> 67b83c47-0ee2-46bc-b0ff-e0eb43edc1c2 |grep ^NIC
> NIC 1: MAC: 0800270481A8, Attachment: Bridged Interface 'eth0', Cable
> connected: on, Trace: off (file: none), Type: 82540EM, Reported speed: 0
> Mbps, Boot priority: 0
> NIC 2: MAC: 08002778F641, Attachment: Bridged Interface 'eth0', Cable
> connected: on, Trace: off (file: none), Type: 82540EM, Reported speed: 0
> Mbps, Boot priority: 0
> NIC 3: MAC: 080027C408BA, Attachment: Bridged Interface 'eth0', Cable
> connected: on, Trace: off (file: none), Type: 82540EM, Reported speed: 0
> Mbps, Boot priority: 0
> NIC 4: MAC: 080027DB339A, Attachment: Bridged Interface 'eth0', Cable
> connected: on, Trace: off (file: none), Type: 82540EM, Reported speed: 0
> Mbps, Boot priority: 0
> NIC 5: disabled
> NIC 6: disabled
> NIC 7: disabled
> NIC 8: disabled
>
> And when guest starts, i find that:
> NIC 1: eth7
> NIC 2: eth6
> NIC 3: eth9
> NIC 4: eth8

Would you mind testing with "Host-only Interface 'vboxnet0'", instead of "Bridged Interface 'eth0'"?

All the bonding tests I do use this setup and the link failure detection work well.

	Nicolas.

^ permalink raw reply

* Re: [PATCH net-next-2.6] net: harmonize the call to ptype_all and ptype_base handlers.
From: Jiri Pirko @ 2011-03-07 21:12 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: Nicolas de Pesloüan, netdev, davem, shemminger, eric.dumazet,
	kaber, fubar, andy
In-Reply-To: <4D7542EF.5000201@gmail.com>

Mon, Mar 07, 2011 at 09:41:19PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 07/03/2011 11:03, Jiri Pirko a écrit :
>>Sun, Mar 06, 2011 at 02:25:16PM CET, nicolas.2p.debian@free.fr wrote:
>>>Until now, ptype_all and ptype_base delivery in __netif_receive_skb() is
>>>inconsistent.
>>>
>>>- For ptype_all, we deliver to every device crossed while walking the
>>>rx_handler path (inside the another_round loop), and there is no way to stop
>>>wildcard delivery (no exact match logic).
>>>- For ptype_base, we deliver to the lowest device (orig_dev) and to the highest
>>>(skb->dev) and we can ask for exact match delivery.
>>>
>>>This patch try and fix this, by:
>>>
>>>1/ Doing exact match delivery for both ptype_all and ptype_base, while walking
>>>   the rx_handler path.
>>>2/ Doing wildcard match delivery at the end of __netif_receive_skb(), if not
>>>   asked to do exact match delivery only.
>>>
>>>Signed-off-by: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
>>>---
>>>
>>>This apply on top of the last batch of patch from Jiri Pirko.
>>>---
>>>net/core/dev.c |   32 ++++++++++++++++++++++++--------
>>>1 files changed, 24 insertions(+), 8 deletions(-)
>>>
>>
>>I tend to like this patch. However I'm not sure if extra 2 loops don't
>>introduce noticable overhead :/
>
>I think ptype_all and ptype_base lists should only contain entries having ptype->dev == NULL.
>
>The entries having ptype->dev != NULL should be on per net_device
>lists. The head of those lists could/should be in a ptype_all and a
>ptype_base property in net_device.
>
>This would speed up the exact-match loops, because they would scan small (or empty) lists.
>
>I need to double check the possible impact of this proposal.

On the first glance, this makes sense to me.

>
>	Nicolas.

^ permalink raw reply

* Re: [PATCH 13/17] mlx4: Add blue flame support for kernel consumers
From: Roland Dreier @ 2011-03-07 21:07 UTC (permalink / raw)
  To: Yevgeny Petrilin; +Cc: davem, netdev, eli
In-Reply-To: <4D74B0D2.1000907@mellanox.co.il>

On Mon, Mar 7, 2011 at 2:17 AM, Yevgeny Petrilin
<yevgenyp@mellanox.co.il> wrote:
> +       if (map_bf_area(dev))
> +               mlx4_dbg(dev, "Kernel support for blue flame is not available for kernels < 2.6.28\n");

This seems like a really bad error message.  Can map_bf_area() actually fail?

 - R.

^ permalink raw reply

* Re: [rds-devel] [PATCH] rds: prevent BUG_ON triggering on congestion map updates
From: Neil Horman @ 2011-03-07 20:56 UTC (permalink / raw)
  To: Chris Mason; +Cc: David Miller, netdev, rds-devel, Venkat Venkatsubra
In-Reply-To: <1299530363-sup-8922@think>

On Mon, Mar 07, 2011 at 03:41:04PM -0500, Chris Mason wrote:
> Excerpts from David Miller's message of 2011-03-07 15:27:53 -0500:
> > From: Neil Horman <nhorman@tuxdriver.com>
> > Date: Wed,  2 Mar 2011 11:28:22 -0500
> > 
> > > Recently had this bug halt reported to me:
> > 
> > Well, does anyone on the RDS team care about this bug fix at all?
> > 
> > Stuff like this should not sit for nearly a week without any reply
> > whatsoever.
> > 
> 
> The patch looks good to me, but I'm surprised we haven't seen it here.
> Venkat please take a look (link to the patch below)  Has it only been seen on ppc?
> 
Yes, its only been observed on ppc64.  Its does seem like it should be
observable on other arches.  I presumed it had something to do with ppc64
alignment and how it filled in the sg array, that led to a leftover bit of space
in the scattergather array.  Honestly though I wasnt too worried about ferreting
out the source, since it seemed apparent to me that buffers with
RDS_FLAG_CONG_BITMAP set were just causing rds_[loop|ib]_xmit to return a dummy
positive value (I say dummy because theres no actual data transmitted).  So I
figured if the value is just there to satisfy the return code, it might as well
also satisfy what is otherwise a valid BUG_ON check too.

Best
Neil

> http://permalink.gmane.org/gmane.linux.network/187933
> 
> -chris
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Re: [PATCH V4] Export ACPI _DSM provided firmware instance number and string name to sysfs
From: Narendra_K @ 2011-03-07 20:55 UTC (permalink / raw)
  To: greg
  Cc: a.beregalov, linux-next, jbarnes, linux-pci, linux-hotplug,
	netdev, mjg, Matt_Domsch, Charles_Rose, Jordan_Hargrave,
	Shyam_Iyer, sfr
In-Reply-To: <20110307195616.GA14888@kroah.com>

On Tue, Mar 08, 2011 at 01:26:16AM +0530, Greg KH wrote:
> On Mon, Mar 07, 2011 at 11:44:52AM -0800, Narendra_K@Dell.com wrote:
> > --- a/drivers/pci/pci-label.c
> > +++ b/drivers/pci/pci-label.c
> > @@ -29,7 +29,9 @@
> >  #include <linux/nls.h>
> >  #include <linux/acpi.h>
> >  #include <linux/pci-acpi.h>
> > +#ifdef CONFIG_ACPI
> >  #include <acpi/acpi_drivers.h>
> > +#endif
> 
> You should never need a #ifdef in a .c file for an include file.  If so,
> something is really wrong.

I agree. Also, i realized that the include was not required to address the
reported error. Please find the revised patch here.

From: Narendra K <narendra_k@dell.com>
Subject: [PATCH] Fix compilation error when CONFIG_ACPI is unset

This patch fixes compilation error descibed below introduced by
the commit 6058989bad05b82e78baacce69ec14f27a11b5fd

drivers/pci/pci-label.c: In function ‘pci_create_firmware_label_files’:
drivers/pci/pci-label.c:366:2: error: implicit declaration of function ‘device_has_dsm’

Signed-off-by: Narendra K <narendra_k@dell.com>
---
 drivers/pci/pci-label.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
index 824e247..8c80138 100644
--- a/drivers/pci/pci-label.c
+++ b/drivers/pci/pci-label.c
@@ -174,6 +174,12 @@ pci_remove_acpi_index_label_files(struct pci_dev *pdev)
 	return -1;
 }
 
+static inline bool
+device_has_dsm(struct device *dev)
+{
+	return false;
+}
+
 #else
 
 static const char device_label_dsm_uuid[] = {
-- 
1.7.3.1

With regards,
Narendra K

^ permalink raw reply related

* Re: [PATCH] net: Enter net/ipv6/ even if CONFIG_IPV6=n
From: David Miller @ 2011-03-07 20:51 UTC (permalink / raw)
  To: randy.dunlap; +Cc: kaber, sfr, netdev, linux-next
In-Reply-To: <4D711B02.2060200@oracle.com>

From: Randy Dunlap <randy.dunlap@oracle.com>
Date: Fri, 04 Mar 2011 09:01:54 -0800

> On 03/04/11 01:35, Thomas Graf wrote:
>> exthdrs_core.c and addrconf_core.c in net/ipv6/ contain bits which
>> must be made available even if IPv6 is disabled.
>> 
>> net/ipv6/Makefile already correctly includes them if CONFIG_IPV6=n
>> but net/Makefile prevents entering the subdirectory.
>> 
>> Signed-off-by: Thomas Graf <tgraf@infradead.org>
> 
> Acked-by: Randy Dunlap <randy.dunlap@oracle.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] bridge:  control carrier based on ports online
From: Nicolas de Pesloüan @ 2011-03-07 20:48 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: David S. Miller, Adam Majer, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, bridge, netdev, Andy Gospodarek, Jay Vosburgh
In-Reply-To: <20110307103406.27330529@nehalam>

Le 07/03/2011 19:34, Stephen Hemminger a écrit :
> This makes the bridge device behave like a physical device.
> In earlier releases the bridge always asserted carrier. This
> changes the behavior so that bridge device carrier is on only
> if one or more ports are in the forwarding state. This
> should help IPv6 autoconfiguration, DHCP, and routing daemons.
>
> I did brief testing with Network and Virt manager and they
> seem fine, but since this changes behavior of bridge, it should
> wait until net-next (2.6.39).
>
> Signed-off-by: Stephen Hemminger<shemminger@vyatta.com>
>
> ---
>   net/bridge/br_device.c    |    4 ++++
>   net/bridge/br_stp.c       |   35 ++++++++++++++++++++++-------------
>   net/bridge/br_stp_timer.c |    1 +
>   3 files changed, 27 insertions(+), 13 deletions(-)
>
> --- a/net/bridge/br_device.c	2011-03-07 08:40:08.913599513 -0800
> +++ b/net/bridge/br_device.c	2011-03-07 08:40:48.382377389 -0800
> @@ -78,6 +78,8 @@ static int br_dev_open(struct net_device
>   {
>   	struct net_bridge *br = netdev_priv(dev);
>
> +	netif_carrier_off(dev);
> +
>   	br_features_recompute(br);
>   	netif_start_queue(dev);
>   	br_stp_enable_bridge(br);
> @@ -94,6 +96,8 @@ static int br_dev_stop(struct net_device
>   {
>   	struct net_bridge *br = netdev_priv(dev);
>
> +	netif_carrier_off(dev);
> +
>   	br_stp_disable_bridge(br);
>   	br_multicast_stop(br);
>
> --- a/net/bridge/br_stp.c	2011-03-07 08:41:58.619783678 -0800
> +++ b/net/bridge/br_stp.c	2011-03-07 08:53:58.953558810 -0800
> @@ -397,28 +397,37 @@ static void br_make_forwarding(struct ne
>   void br_port_state_selection(struct net_bridge *br)
>   {
>   	struct net_bridge_port *p;
> +	unsigned int liveports = 0;
>
>   	/* Don't change port states if userspace is handling STP */
>   	if (br->stp_enabled == BR_USER_STP)
>   		return;
>
>   	list_for_each_entry(p,&br->port_list, list) {
> -		if (p->state != BR_STATE_DISABLED) {
> -			if (p->port_no == br->root_port) {
> -				p->config_pending = 0;
> -				p->topology_change_ack = 0;
> -				br_make_forwarding(p);
> -			} else if (br_is_designated_port(p)) {
> -				del_timer(&p->message_age_timer);
> -				br_make_forwarding(p);
> -			} else {
> -				p->config_pending = 0;
> -				p->topology_change_ack = 0;
> -				br_make_blocking(p);
> -			}
> +		if (p->state == BR_STATE_DISABLED)
> +			continue;
> +
> +		if (p->port_no == br->root_port) {
> +			p->config_pending = 0;
> +			p->topology_change_ack = 0;
> +			br_make_forwarding(p);
> +		} else if (br_is_designated_port(p)) {
> +			del_timer(&p->message_age_timer);
> +			br_make_forwarding(p);
> +		} else {
> +			p->config_pending = 0;
> +			p->topology_change_ack = 0;
> +			br_make_blocking(p);

Is the above part really related to the purpose of this patch? It looks like (good) cleanup, but 
should be in a different patch.

Except from this comment,

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

>   		}
>
> +		if (p->state == BR_STATE_FORWARDING)
> +			++liveports;
>   	}
> +
> +	if (liveports == 0)
> +		netif_carrier_off(br->dev);
> +	else
> +		netif_carrier_on(br->dev);
>   }
>
>   /* called under bridge lock */
> --- a/net/bridge/br_stp_timer.c	2011-03-07 08:53:25.728770710 -0800
> +++ b/net/bridge/br_stp_timer.c	2011-03-07 08:53:40.273116636 -0800
> @@ -94,6 +94,7 @@ static void br_forward_delay_timer_expir
>   		p->state = BR_STATE_FORWARDING;
>   		if (br_is_designated_for_some_port(br))
>   			br_topology_change_detection(br);
> +		netif_carrier_on(br->dev);
>   	}
>   	br_log_state(p);
>   	spin_unlock(&br->lock);
>


^ permalink raw reply

* Re: [rds-devel] [PATCH] rds: prevent BUG_ON triggering on congestion map updates
From: Chris Mason @ 2011-03-07 20:41 UTC (permalink / raw)
  To: David Miller; +Cc: nhorman, netdev, rds-devel, Venkat Venkatsubra
In-Reply-To: <20110307.122753.59686845.davem@davemloft.net>

Excerpts from David Miller's message of 2011-03-07 15:27:53 -0500:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Wed,  2 Mar 2011 11:28:22 -0500
> 
> > Recently had this bug halt reported to me:
> 
> Well, does anyone on the RDS team care about this bug fix at all?
> 
> Stuff like this should not sit for nearly a week without any reply
> whatsoever.
> 

The patch looks good to me, but I'm surprised we haven't seen it here.
Venkat please take a look (link to the patch below)  Has it only been seen on ppc?

http://permalink.gmane.org/gmane.linux.network/187933

-chris

^ permalink raw reply

* Re: [PATCH net-next-2.6] net: harmonize the call to ptype_all and ptype_base handlers.
From: Nicolas de Pesloüan @ 2011-03-07 20:41 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Nicolas de Pesloüan, netdev, davem, shemminger, eric.dumazet,
	kaber, fubar, andy
In-Reply-To: <20110307100318.GD2832@psychotron.brq.redhat.com>

Le 07/03/2011 11:03, Jiri Pirko a écrit :
> Sun, Mar 06, 2011 at 02:25:16PM CET, nicolas.2p.debian@free.fr wrote:
>> Until now, ptype_all and ptype_base delivery in __netif_receive_skb() is
>> inconsistent.
>>
>> - For ptype_all, we deliver to every device crossed while walking the
>> rx_handler path (inside the another_round loop), and there is no way to stop
>> wildcard delivery (no exact match logic).
>> - For ptype_base, we deliver to the lowest device (orig_dev) and to the highest
>> (skb->dev) and we can ask for exact match delivery.
>>
>> This patch try and fix this, by:
>>
>> 1/ Doing exact match delivery for both ptype_all and ptype_base, while walking
>>    the rx_handler path.
>> 2/ Doing wildcard match delivery at the end of __netif_receive_skb(), if not
>>    asked to do exact match delivery only.
>>
>> Signed-off-by: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
>> ---
>>
>> This apply on top of the last batch of patch from Jiri Pirko.
>> ---
>> net/core/dev.c |   32 ++++++++++++++++++++++++--------
>> 1 files changed, 24 insertions(+), 8 deletions(-)
>>
>
> I tend to like this patch. However I'm not sure if extra 2 loops don't
> introduce noticable overhead :/

I think ptype_all and ptype_base lists should only contain entries having ptype->dev == NULL.

The entries having ptype->dev != NULL should be on per net_device lists. The head of those lists 
could/should be in a ptype_all and a ptype_base property in net_device.

This would speed up the exact-match loops, because they would scan small (or empty) lists.

I need to double check the possible impact of this proposal.

	Nicolas.

^ permalink raw reply

* Re: [PATCH 1/8] Phonet: fix NULL-deref in previous patch series
From: Rémi Denis-Courmont @ 2011-03-07 20:33 UTC (permalink / raw)
  To: netdev
In-Reply-To: <20110307.122327.183060932.davem@davemloft.net>

Le lundi 7 mars 2011 22:23:27 David Miller, vous avez écrit :
> From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> Date: Fri,  4 Mar 2011 12:00:24 +0200
> 
> > Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> 
> What "previous patch series"?

The one you complained about applying patches manually, so nothing "stable". 
Though that's not an excuse for OOPSing nor being terse.

-- 
Rémi Denis-Courmont
http://www.remlab.info/
http://fi.linkedin.com/in/remidenis

^ permalink raw reply

* Re: [PATCH 3/8] Phonet: fix backlog callback return value
From: David Miller @ 2011-03-07 20:31 UTC (permalink / raw)
  To: remi; +Cc: netdev
In-Reply-To: <201103072230.14080.remi@remlab.net>

From: "Rémi Denis-Courmont" <remi@remlab.net>
Date: Mon, 7 Mar 2011 22:30:13 +0200

> Le lundi 7 mars 2011 22:23:29 David Miller, vous avez écrit :
>> More verbose commit message please.
>> 
>> What was wrong with the old error values and what exactly do
>> you claim to be doing to correct them?
> 
> They were returning -Exxx errno's where the caller expected NET_RX_* values.

Then say that in the commit message.


^ 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