Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH V4] Export ACPI _DSM provided firmware instance number and string name to sysfs
From: Greg KH @ 2011-03-07 19:56 UTC (permalink / raw)
  To: Narendra_K
  Cc: a.beregalov, linux-next, jbarnes, linux-pci, linux-hotplug,
	netdev, mjg, Matt_Domsch, Charles_Rose, Jordan_Hargrave,
	Shyam_Iyer, sfr
In-Reply-To: <20110307200512.GA22450@fedora14-r610.oslab.blr.amer.dell.com>

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.

thanks,

greg k-h

^ permalink raw reply

* [PATCH net-next-2.6 V2] net: enhance the documentation for rx_handler.
From: Nicolas de Pesloüan @ 2011-03-07 20:01 UTC (permalink / raw)
  To: netdev
  Cc: davem, shemminger, eric.dumazet, kaber, fubar, andy,
	Nicolas de Pesloüan
In-Reply-To: <20110307083607.3299ed0c@nehalam>

Signed-off-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
Reviewed-by: Jiri Pirko <jpirko@redhat.com>
---
This apply on top of Jiri's last patch serie, including the last one that
commented the RX_HANDLER_* values.

 include/linux/netdevice.h |   53 ++++++++++++++++++++++++++++++++++++++------
 net/core/dev.c            |    2 +
 2 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 26e03f9..0c9dc93 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -390,15 +390,52 @@ enum gro_result {
 };
 typedef enum gro_result gro_result_t;
 
+/**
+ * enum rx_handler_result - Possible return values for rx_handlers.
+ * @RX_HANDLER_CONSUMED: skb was consumed by rx_handler, do not process it
+ * further.
+ * @RX_HANDLER_ANOTHER: Do another round in receive path. This is indicated in
+ * case skb->dev was changed by rx_handler.
+ * @RX_HANDLER_EXACT: Force exact delivery, no wildcard.
+ * @RX_HANDLER_PASS: Do nothing, process the skb as if no rx_handler was called.
+ *
+ * rx_handlers are functions called from inside __netif_receive_skb(), to do
+ * special processing of the skb, prior to delivery to protocol handlers.
+ *
+ * Currently, a net_device can only have a single rx_handler registered. Trying
+ * to register a second rx_handler will return -EBUSY.
+ *
+ * To register a rx_handler on a net_device, use netdev_rx_handler_register().
+ * To unregister a rx_handler on a net_device, use
+ * netdev_rx_handler_unregister().
+ *
+ * Upon return, rx_handler is expected to tell __netif_receive_skb() what to
+ * do with the skb.
+ *
+ * If the rx_handler consumed to skb in some way, it should return
+ * RX_HANDLER_CONSUMED. This is appropriate when the rx_handler arranged for
+ * the skb to be delivered in some other ways.
+ *
+ * If the rx_handler changed skb->dev, to divert the skb to another
+ * net_device, it should return RX_HANDLER_ANOTHER. The rx_handler for the
+ * new device will be called if it exists.
+ *
+ * If the rx_handler consider the skb should be ignored, it should return
+ * RX_HANDLER_EXACT. The skb will only be delivered to protocol handlers that
+ * are registred on exact device (ptype->dev == skb->dev).
+ *
+ * If the rx_handler didn't changed skb->dev, but want the skb to be normally 
+ * delivered, it should return RX_HANDLER_PASS.
+ * 
+ * A device without a registered rx_handler will behave as if rx_handler
+ * returned RX_HANDLER_PASS.
+ */
+
 enum rx_handler_result {
-	RX_HANDLER_CONSUMED,	/* skb was consumed by rx_handler,
-				   do not process it further. */
-	RX_HANDLER_ANOTHER,	/* Do another round in receive path.
-				   This is indicated in case skb->dev
-				   was changed by rx_handler */
-	RX_HANDLER_EXACT,	/* Force exact delivery, no wildcard */
-	RX_HANDLER_PASS,	/* Do nothing, pass the skb as if
-				   no rx_handler was called */
+	RX_HANDLER_CONSUMED,
+	RX_HANDLER_ANOTHER,	
+	RX_HANDLER_EXACT,
+	RX_HANDLER_PASS,
 };
 typedef enum rx_handler_result rx_handler_result_t;
 typedef rx_handler_result_t rx_handler_func_t(struct sk_buff **pskb);
diff --git a/net/core/dev.c b/net/core/dev.c
index a368223..3630722 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3060,6 +3060,8 @@ out:
  *	on a failure.
  *
  *	The caller must hold the rtnl_mutex.
+ *
+ *	For a general description of rx_handler, see enum rx_handler_result.
  */
 int netdev_rx_handler_register(struct net_device *dev,
 			       rx_handler_func_t *rx_handler,
-- 
1.7.2.3


^ permalink raw reply related

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

Le 07/03/2011 15:32, Andy Gospodarek a écrit :
> 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>

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

> Thanks, Jiri.  I will test and make sure it works correctly now.  I know
> several users who would be quite disappointed if this feature was
> removed.

Andy, while you are testing it, can you ensure it also works for the following setup?

eth0 -> bond0 -> br0 -> br0.100

I think it should, as long as the slave is the first device in the path.

Can you imagine a bonding setup where the slaves are not first in the path?

eth0 -> br0 -> bond0?
eth0 -> eth1(macvlan) -> bond0?
eth0 -> eth0.100 -> bond0?

	Nicolas.

^ permalink raw reply

* Re: [PATCH 1/8] Phonet: fix NULL-deref in previous patch series
From: David Miller @ 2011-03-07 20:23 UTC (permalink / raw)
  To: remi.denis-courmont; +Cc: netdev
In-Reply-To: <1299232831-3132-1-git-send-email-remi.denis-courmont@nokia.com>

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"?

Nobody looking at this patch in isoluation will understand
what you're talking about.

Reference other changes like a man, by mentioned the SHA1 ID and
giving the commit message header line inside of ("") right
afterwards. :-)



^ permalink raw reply

* Re: [PATCH 3/8] Phonet: fix backlog callback return value
From: David Miller @ 2011-03-07 20:23 UTC (permalink / raw)
  To: remi.denis-courmont; +Cc: netdev
In-Reply-To: <1299232831-3132-3-git-send-email-remi.denis-courmont@nokia.com>


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?

If all I get is single header line as a commit message, you're not
doing due dilligence and you're making your work harder to
review and understand for other people.

^ permalink raw reply

* Re: [PATCH] rds: prevent BUG_ON triggering on congestion map updates
From: David Miller @ 2011-03-07 20:27 UTC (permalink / raw)
  To: nhorman; +Cc: netdev, rds-devel, andy.grover
In-Reply-To: <1299083302-5168-1-git-send-email-nhorman@tuxdriver.com>

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.

^ permalink raw reply

* Re: [PATCH 3/8] Phonet: fix backlog callback return value
From: Rémi Denis-Courmont @ 2011-03-07 20:30 UTC (permalink / raw)
  To: netdev
In-Reply-To: <20110307.122329.39193888.davem@davemloft.net>

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.
I can "fix" it but it seemed obvious.

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

^ permalink raw reply

* Re: [Bugme-new] [Bug 30092] New: smsc911x.c drops long packets with VLAN tags
From: David Miller @ 2011-03-07 20:31 UTC (permalink / raw)
  To: weinholt; +Cc: akpm, netdev, bugzilla-daemon, bugme-daemon, steve.glendinning
In-Reply-To: <lnsjv58vl2.fsf@desktop3.csbnet.se>

From: weinholt@csbnet.se (Göran Weinholt)
Date: Wed, 02 Mar 2011 15:07:21 +0100

> Subject: [PATCH] net/smsc911x.c: Set the VLAN1 register to fix VLAN MTU problem
> From: Göran Weinholt <weinholt@csbnet.se>
> 
> The smsc911x driver would drop frames longer than 1518 bytes, which is a
> problem for networks with VLAN tagging. The VLAN1 tag register is used
> to increase the legal frame size to 1522 when a VLAN tag is identified.
> 
> Signed-off-by: Göran Weinholt <weinholt@csbnet.se>

Applied, thanks.

^ 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

* 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 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: [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] 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: [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 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: [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 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: [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: 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: 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: [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: 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: 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 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: [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


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