* 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
* 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 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: 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] 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: 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 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 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] 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: Nicolas de Pesloüan @ 2011-03-07 23:09 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Jiri Pirko, netdev, davem, shemminger, kaber, fubar, eric.dumazet
In-Reply-To: <20110307224338.GU11864@gospo.rdu.redhat.com>
Le 07/03/2011 23:43, 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>
>> ---
>> 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).
After Jiri's last patch series, bond_arp_rcv() is not registered anymore as a protocol handler on
bond0, but directly called from inside bond_handle_frame(), through bond->recv_probe.
Because bond_handler_frame() is a rx_handler for the slave interfaces, bond_arp_rcv() is now called
at the slave level and not a the master level anymore.
Hence this patch and the reason I thought it should work.
Did you tested this patch with Jiri's previous patches applied before?
> 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.
I definitely hate all those unnecessary reinjects from rx_handler. The another_round loop is
designed to allow for stacking inside __netif_receive_skb().
Jiri apparently has another (better) solution in mind. I hope to see it, but Jiri arguably want some
of the patchs in the queue to flow before adding more.
Does someone had a look at my proposal of a late_delivery property for packet_type, previously in
this thread, to handle the situation where a given protocol handler registered on a particular
device would like to receive the final skb instead of the one at the time it crossed that particular
device?
>
> 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.
Or we can remove all this orig_dev stuff...
Nicolas.
> 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
* [PATCH 0/2] fib semantic matching
From: David Miller @ 2011-03-07 23:13 UTC (permalink / raw)
To: netdev
fib_semantic_match() creates unnecessary overhead on route lookups
for two reasons:
1) It does type validation at run time instead of when we insert
the route.
2) Argument passing is expensive because it does it's work as
a seperate function.
So these two patches move type validation into fib_create_info(),
and then we inline fib_semantic_match() completely into check_leaf().
This is good for 50+ cycle gain on output route lookups on Niagara2.
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* [PATCH 1/2] ipv4: Validate route entry type at insert instead of every lookup.
From: David Miller @ 2011-03-07 23:13 UTC (permalink / raw)
To: netdev
fib_semantic_match() requires that if the type doesn't signal an
automatic error, it must be of type RTN_UNICAST, RTN_LOCAL,
RTN_BROADCAST, RTN_ANYCAST, or RTN_MULTICAST.
Checking this every route lookup is pointless work.
Instead validate it during route insertion, via fib_create_info().
Also, there was nothing making sure the type value was less than
RTN_MAX, so add that missing check while we're here.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/ipv4/fib_semantics.c | 54 +++++++++++++++++++++++----------------------
1 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 562f34c..c29291b 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -707,6 +707,9 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
int nhs = 1;
struct net *net = cfg->fc_nlinfo.nl_net;
+ if (cfg->fc_type > RTN_MAX)
+ goto err_inval;
+
/* Fast check to catch the most weird cases */
if (fib_props[cfg->fc_type].scope > cfg->fc_scope)
goto err_inval;
@@ -812,6 +815,17 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
if (cfg->fc_gw || cfg->fc_oif || cfg->fc_mp)
goto err_inval;
goto link_it;
+ } else {
+ switch (cfg->fc_type) {
+ case RTN_UNICAST:
+ case RTN_LOCAL:
+ case RTN_BROADCAST:
+ case RTN_ANYCAST:
+ case RTN_MULTICAST:
+ break;
+ default:
+ goto err_inval;
+ }
}
if (cfg->fc_scope > RT_SCOPE_HOST)
@@ -915,35 +929,23 @@ int fib_semantic_match(struct fib_table *tb, struct list_head *head,
if (fi->fib_flags & RTNH_F_DEAD)
continue;
- switch (fa->fa_type) {
- case RTN_UNICAST:
- case RTN_LOCAL:
- case RTN_BROADCAST:
- case RTN_ANYCAST:
- case RTN_MULTICAST:
- for_nexthops(fi) {
- if (nh->nh_flags & RTNH_F_DEAD)
- continue;
- if (!flp->oif || flp->oif == nh->nh_oif)
- break;
- }
+ for_nexthops(fi) {
+ if (nh->nh_flags & RTNH_F_DEAD)
+ continue;
+ if (!flp->oif || flp->oif == nh->nh_oif)
+ break;
+ }
#ifdef CONFIG_IP_ROUTE_MULTIPATH
- if (nhsel < fi->fib_nhs) {
- nh_sel = nhsel;
- goto out_fill_res;
- }
+ if (nhsel < fi->fib_nhs) {
+ nh_sel = nhsel;
+ goto out_fill_res;
+ }
#else
- if (nhsel < 1)
- goto out_fill_res;
+ if (nhsel < 1)
+ goto out_fill_res;
#endif
- endfor_nexthops(fi);
- continue;
-
- default:
- pr_warning("fib_semantic_match bad type %#x\n",
- fa->fa_type);
- return -EINVAL;
- }
+ endfor_nexthops(fi);
+ continue;
}
return err;
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH 2/2] ipv4: Inline fib_semantic_match into check_leaf
From: David Miller @ 2011-03-07 23:13 UTC (permalink / raw)
To: netdev
This elimiates a lot of pure overhead due to parameter
passing.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/ipv4/fib_lookup.h | 7 +++++
net/ipv4/fib_semantics.c | 68 +---------------------------------------------
net/ipv4/fib_trie.c | 51 +++++++++++++++++++++++++++++-----
3 files changed, 51 insertions(+), 75 deletions(-)
diff --git a/net/ipv4/fib_lookup.h b/net/ipv4/fib_lookup.h
index d5c40d8..84db2da 100644
--- a/net/ipv4/fib_lookup.h
+++ b/net/ipv4/fib_lookup.h
@@ -51,4 +51,11 @@ static inline void fib_result_assign(struct fib_result *res,
res->fi = fi;
}
+struct fib_prop {
+ int error;
+ u8 scope;
+};
+
+extern const struct fib_prop fib_props[RTN_MAX + 1];
+
#endif /* _FIB_LOOKUP_H */
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index c29291b..6349a21 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -90,11 +90,7 @@ static DEFINE_SPINLOCK(fib_multipath_lock);
#define endfor_nexthops(fi) }
-static const struct
-{
- int error;
- u8 scope;
-} fib_props[RTN_MAX + 1] = {
+const struct fib_prop fib_props[RTN_MAX + 1] = {
[RTN_UNSPEC] = {
.error = 0,
.scope = RT_SCOPE_NOWHERE,
@@ -902,68 +898,6 @@ failure:
return ERR_PTR(err);
}
-/* Note! fib_semantic_match intentionally uses RCU list functions. */
-int fib_semantic_match(struct fib_table *tb, struct list_head *head,
- const struct flowi *flp, struct fib_result *res,
- int prefixlen, int fib_flags)
-{
- struct fib_alias *fa;
- int nh_sel = 0;
-
- list_for_each_entry_rcu(fa, head, fa_list) {
- int err;
-
- if (fa->fa_tos &&
- fa->fa_tos != flp->fl4_tos)
- continue;
-
- if (fa->fa_scope < flp->fl4_scope)
- continue;
-
- fib_alias_accessed(fa);
-
- err = fib_props[fa->fa_type].error;
- if (err == 0) {
- struct fib_info *fi = fa->fa_info;
-
- if (fi->fib_flags & RTNH_F_DEAD)
- continue;
-
- for_nexthops(fi) {
- if (nh->nh_flags & RTNH_F_DEAD)
- continue;
- if (!flp->oif || flp->oif == nh->nh_oif)
- break;
- }
-#ifdef CONFIG_IP_ROUTE_MULTIPATH
- if (nhsel < fi->fib_nhs) {
- nh_sel = nhsel;
- goto out_fill_res;
- }
-#else
- if (nhsel < 1)
- goto out_fill_res;
-#endif
- endfor_nexthops(fi);
- continue;
- }
- return err;
- }
- return 1;
-
-out_fill_res:
- res->prefixlen = prefixlen;
- res->nh_sel = nh_sel;
- res->type = fa->fa_type;
- res->scope = fa->fa_scope;
- res->fi = fa->fa_info;
- res->table = tb;
- res->fa_head = head;
- if (!(fib_flags & FIB_LOOKUP_NOREF))
- atomic_inc(&res->fi->fib_clntref);
- return 0;
-}
-
/* Find appropriate source address to this destination */
__be32 __fib_res_prefsrc(struct fib_result *res)
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index edf3b09..057062b 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1349,23 +1349,58 @@ static int check_leaf(struct fib_table *tb, struct trie *t, struct leaf *l,
struct hlist_node *node;
hlist_for_each_entry_rcu(li, node, hhead, hlist) {
- int err;
+ struct fib_alias *fa;
int plen = li->plen;
__be32 mask = inet_make_mask(plen);
if (l->key != (key & ntohl(mask)))
continue;
- err = fib_semantic_match(tb, &li->falh, flp, res, plen, fib_flags);
+ list_for_each_entry_rcu(fa, &li->falh, fa_list) {
+ struct fib_info *fi = fa->fa_info;
+ int nhsel, err;
+
+ if (fa->fa_tos && fa->fa_tos != flp->fl4_tos)
+ continue;
+ if (fa->fa_scope < flp->fl4_scope)
+ continue;
+ fib_alias_accessed(fa);
+ err = fib_props[fa->fa_type].error;
+ if (err) {
+ifdef CONFIG_IP_FIB_TRIE_STATS
+ t->stats.semantic_match_miss++;
+#endif
+ return 1;
+ }
+ if (fi->fib_flags & RTNH_F_DEAD)
+ continue;
+ for (nhsel = 0; nhsel < fi->fib_nhs; nhsel++) {
+ const struct fib_nh *nh = &fi->fib_nh[nhsel];
+
+ if (nh->nh_flags & RTNH_F_DEAD)
+ continue;
+ if (flp->oif && flp->oif != nh->nh_oif)
+ continue;
#ifdef CONFIG_IP_FIB_TRIE_STATS
- if (err <= 0)
- t->stats.semantic_match_passed++;
- else
- t->stats.semantic_match_miss++;
+ t->stats.semantic_match_passed++;
+#endif
+ res->prefixlen = plen;
+ res->nh_sel = nhsel;
+ res->type = fa->fa_type;
+ res->scope = fa->fa_scope;
+ res->fi = fi;
+ res->table = tb;
+ res->fa_head = &li->falh;
+ if (!(fib_flags & FIB_LOOKUP_NOREF))
+ atomic_inc(&res->fi->fib_clntref);
+ return 0;
+ }
+ }
+
+#ifdef CONFIG_IP_FIB_TRIE_STATS
+ t->stats.semantic_match_miss++;
#endif
- if (err <= 0)
- return err;
}
return 1;
--
1.7.4.1
^ permalink raw reply related
* pull request: sfc-next-2.6 2011-03-07
From: Ben Hutchings @ 2011-03-07 23:23 UTC (permalink / raw)
To: David Miller; +Cc: sf-linux-drivers, netdev
The following changes since commit 07df5294a753dfac2cc9f75e6159fc25fdc22149:
inet: Replace left-over references to inet->cork (2011-03-01 23:00:58 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next-2.6.git for-davem
Just one more performance optimisation.
Ben.
Ben Hutchings (1):
sfc: Use write-combining to reduce TX latency
drivers/net/sfc/efx.c | 4 ++--
drivers/net/sfc/io.h | 13 +++++++++----
drivers/net/sfc/mcdi.c | 9 +++++----
3 files changed, 16 insertions(+), 10 deletions(-)
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH net-next-2.6] sfc: Use write-combining to reduce TX latency
From: Ben Hutchings @ 2011-03-07 23:25 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1299540194.2522.46.camel@bwh-desktop>
Based on work by Neil Turton <nturton@solarflare.com> and
Kieran Mansley <kmansley@solarflare.com>.
The BIU has now been verified to handle 3- and 4-dword writes within a
single 128-bit register correctly. This means we can enable write-
combining and only insert write barriers between writes to distinct
registers.
This has been observed to save about 0.5 us when pushing a TX
descriptor to an empty TX queue.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/sfc/efx.c | 4 ++--
drivers/net/sfc/io.h | 13 +++++++++----
drivers/net/sfc/mcdi.c | 9 +++++----
3 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index d563049..b8bd936 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -1104,8 +1104,8 @@ static int efx_init_io(struct efx_nic *efx)
rc = -EIO;
goto fail3;
}
- efx->membase = ioremap_nocache(efx->membase_phys,
- efx->type->mem_map_size);
+ efx->membase = ioremap_wc(efx->membase_phys,
+ efx->type->mem_map_size);
if (!efx->membase) {
netif_err(efx, probe, efx->net_dev,
"could not map memory BAR at %llx+%x\n",
diff --git a/drivers/net/sfc/io.h b/drivers/net/sfc/io.h
index dc45110..d9d8c2e 100644
--- a/drivers/net/sfc/io.h
+++ b/drivers/net/sfc/io.h
@@ -48,9 +48,9 @@
* replacing the low 96 bits with zero does not affect functionality.
* - If the host writes to the last dword address of such a register
* (i.e. the high 32 bits) the underlying register will always be
- * written. If the collector does not hold values for the low 96
- * bits of the register, they will be written as zero. Writing to
- * the last qword does not have this effect and must not be done.
+ * written. If the collector and the current write together do not
+ * provide values for all 128 bits of the register, the low 96 bits
+ * will be written as zero.
* - If the host writes to the address of any other part of such a
* register while the collector already holds values for some other
* register, the write is discarded and the collector maintains its
@@ -103,6 +103,7 @@ static inline void efx_writeo(struct efx_nic *efx, efx_oword_t *value,
_efx_writed(efx, value->u32[2], reg + 8);
_efx_writed(efx, value->u32[3], reg + 12);
#endif
+ wmb();
mmiowb();
spin_unlock_irqrestore(&efx->biu_lock, flags);
}
@@ -125,6 +126,7 @@ static inline void efx_sram_writeq(struct efx_nic *efx, void __iomem *membase,
__raw_writel((__force u32)value->u32[0], membase + addr);
__raw_writel((__force u32)value->u32[1], membase + addr + 4);
#endif
+ wmb();
mmiowb();
spin_unlock_irqrestore(&efx->biu_lock, flags);
}
@@ -139,6 +141,7 @@ static inline void efx_writed(struct efx_nic *efx, efx_dword_t *value,
/* No lock required */
_efx_writed(efx, value->u32[0], reg);
+ wmb();
}
/* Read a 128-bit CSR, locking as appropriate. */
@@ -237,12 +240,14 @@ static inline void _efx_writeo_page(struct efx_nic *efx, efx_oword_t *value,
#ifdef EFX_USE_QWORD_IO
_efx_writeq(efx, value->u64[0], reg + 0);
+ _efx_writeq(efx, value->u64[1], reg + 8);
#else
_efx_writed(efx, value->u32[0], reg + 0);
_efx_writed(efx, value->u32[1], reg + 4);
-#endif
_efx_writed(efx, value->u32[2], reg + 8);
_efx_writed(efx, value->u32[3], reg + 12);
+#endif
+ wmb();
}
#define efx_writeo_page(efx, value, reg, page) \
_efx_writeo_page(efx, value, \
diff --git a/drivers/net/sfc/mcdi.c b/drivers/net/sfc/mcdi.c
index 8bba895..5e118f0 100644
--- a/drivers/net/sfc/mcdi.c
+++ b/drivers/net/sfc/mcdi.c
@@ -94,14 +94,15 @@ static void efx_mcdi_copyin(struct efx_nic *efx, unsigned cmd,
efx_writed(efx, &hdr, pdu);
- for (i = 0; i < inlen; i += 4)
+ for (i = 0; i < inlen; i += 4) {
_efx_writed(efx, *((__le32 *)(inbuf + i)), pdu + 4 + i);
-
- /* Ensure the payload is written out before the header */
- wmb();
+ /* use wmb() within loop to inhibit write combining */
+ wmb();
+ }
/* ring the doorbell with a distinctive value */
_efx_writed(efx, (__force __le32) 0x45789abc, doorbell);
+ wmb();
}
static void efx_mcdi_copyout(struct efx_nic *efx, u8 *outbuf, size_t outlen)
--
1.7.4
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* Re: [PATCH net-next-2.6 0/7] be2net: Patch series for supporting Lancer
From: David Miller @ 2011-03-07 23:27 UTC (permalink / raw)
To: padmanabh.ratnakar; +Cc: netdev, sathya.perla, subbu.seetharaman
In-Reply-To: <20cba747-f885-47d5-b27d-788efc426bec@exht1.ad.emulex.com>
From: Padmanabh Ratnakar <padmanabh.ratnakar@emulex.com>
Date: Mon, 7 Mar 2011 18:35:20 +0530
> Hi David,
> Following are patches to support Lancer family of CNA.
> Please apply.
All applied to net-next-2.6, thanks.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox