* Re: [PATCH 1/2] IPv6: keep route for tentative address
From: Greg KH @ 2010-05-24 18:47 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Emil S Tantilov, David S. Miller, NetDev, Tantilov, Emil S,
stable
In-Reply-To: <20100524113118.47cc9852@nehalam>
On Mon, May 24, 2010 at 11:31:18AM -0700, Stephen Hemminger wrote:
> Recent changes preserve IPv6 address when link goes down (good).
> But would cause address to point to dead dst entry (bad).
> The simplest fix is to just not delete route if address is
> being held for later use.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (cherry picked from commit 93fa159abe50d3c55c7f83622d3f5c09b6e06f4b)
> ---
> Patch is for 2.6.34-stable (problem doesn't exist in earlier kernel)
Normally I wait for David to send networking patches for the stable
trees, so I would like to get David's ack on these two before I am
willing to apply them.
thanks,
greg k-h
^ permalink raw reply
* 【免費的IPHONE等你來拿】
From: ella92260565 @ 2010-05-24 18:37 UTC (permalink / raw)
To: artinodigesu, 3567520, ruby168can168, iablue88, jay32111,
rj_jmyang, chuchi.com, apememessox
凡推薦成功立即免費擁有IPHONE手機一隻
只要邀友填寫免費體驗(http://fly2.ws/w6fEZsT )
就可輕鬆擁有唷！！
活動時間：2010/5/17-2010/6/30 限量10台
獲得iphone二步驟：
1.邀友填寫線上免費體驗單 (http://fly2.ws/w6fEZsT )
2.推薦成功立即擁有IPHONE一支。
您的身邊有過敏問題的朋友嗎？您是否對家中的空氣品質
感到不滿意呢？全程免費的體驗，又有機會拿IPHONE
這麼好康的活動，快到http://fly2.ws/w6fEZsT 填寫免費體驗單！
--------------------------------------------------------------
Ovi Store: New apps daily
http://store.ovi.com/?cid=ovistore-fw-bac-na-acq-na-ovimail-g0-na-3
^ permalink raw reply
* [PATCH 1/2] IPv6: keep route for tentative address
From: Stephen Hemminger @ 2010-05-24 18:31 UTC (permalink / raw)
To: Emil S Tantilov
Cc: David S. Miller, NetDev, Tantilov, Emil S, Greg KH, stable
In-Reply-To: <AANLkTin3CfATS0-ePL5WUSiuq0seRCQgM6xc-YwxpPEy@mail.gmail.com>
Recent changes preserve IPv6 address when link goes down (good).
But would cause address to point to dead dst entry (bad).
The simplest fix is to just not delete route if address is
being held for later use.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 93fa159abe50d3c55c7f83622d3f5c09b6e06f4b)
---
Patch is for 2.6.34-stable (problem doesn't exist in earlier kernel)
net/ipv6/addrconf.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 413054f..12e9558 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4047,7 +4047,8 @@ static void __ipv6_ifa_notify(int event, struct inet6_ifaddr *ifp)
addrconf_leave_anycast(ifp);
addrconf_leave_solict(ifp->idev, &ifp->addr);
dst_hold(&ifp->rt->u.dst);
- if (ip6_del_rt(ifp->rt))
+
+ if (ifp->dead && ip6_del_rt(ifp->rt))
dst_free(&ifp->rt->u.dst);
break;
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/2] IPv6: only notify protocols if address is completely gone
From: Stephen Hemminger @ 2010-05-24 18:33 UTC (permalink / raw)
To: Emil S Tantilov, David S. Miller, Greg KH
Cc: NetDev, Tantilov, Emil S, stable
In-Reply-To: <20100524113118.47cc9852@nehalam>
The notifier for address down should only be called if address is completely
gone, not just being marked as tentative on link transition. The code
in net-next would case bonding/sctp/s390 to see address disappear on link
down, but they would never see it reappear on link up.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 8595805aafc8b077e01804c9a3668e9aa3510e89)
---
Patch for 2.6.34-stable, not needed on earlier or later kernels
net/ipv6/addrconf.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 12e9558..844ffc5 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2729,7 +2729,9 @@ static int addrconf_ifdown(struct net_device *dev, int how)
write_unlock_bh(&idev->lock);
__ipv6_ifa_notify(RTM_DELADDR, ifa);
- atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifa);
+ if (ifa->dead)
+ atomic_notifier_call_chain(&inet6addr_chain,
+ NETDEV_DOWN, ifa);
in6_ifa_put(ifa);
write_lock_bh(&idev->lock);
--
1.7.0.4
^ permalink raw reply related
* RE: ixgbe and SRIOV failure in driver?
From: Tantilov, Emil S @ 2010-05-24 17:57 UTC (permalink / raw)
To: Fischer, Anna, Rose, Gregory V
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org
In-Reply-To: <0199E0D51A61344794750DC57738F58E70B14DC9A7@GVW1118EXC.americas.hpqcorp.net>
Fischer, Anna wrote:
>> Subject: RE: ixgbe and SRIOV failure in driver?
>>
>>> -----Original Message-----
>>> From: netdev-owner@vger.kernel.org
>>> [mailto:netdev-owner@vger.kernel.org] On Behalf Of Fischer, Anna
>>> Sent: Friday, May 21, 2010 9:33 AM
>>> To: e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org
>>> Subject: ixgbe and SRIOV failure in driver?
>>>
>>> I am running a system with 3 Intel NICs. Two of them are 82598
>>> devices, and one is a SRIOV capable 82599.
>>>
>>> All devices use the ixgbe driver. What happens (I believe) now is
>>> that when the driver loads at first, it sees the 82598 first
>>> (because of its position in the PCI tree) and then it says "Device
>>> not IOV capable - switching off IOV."
>>>
>>> So then it switches into non-IOV mode, and I can never enable SRIOV
>>> on my 82599, because the driver does not enable it any more for
>>> further devices.
>>>
>>> So to get around this issue, I tried to use pciback.hide to hide the
>>> 82598 devices from the OS. That way I was hoping that the driver
>>> would switch on SRIOV on my 82599. However, then I got a kernel
>>> panic on boot (see below).
>>>
>>> I am running Xen 4 and the Dom0 kernel is a 2.6.31 kernel.
>>
>> The ixgbe driver included with the 2.6.31 kernel does not support
>> SR-IOV. Where did you get the driver that does? Please run ethtool
>> -i <ethx> and post the results.
>
> I have installed the driver separately on my pv-ops kernel.
>
> [root@211052610-f-cpu ~]# ethtool -i eth
> driver: ixgbe
> version: 2.0.75.7-NAPI
> firmware-version: 0.9-3
> bus-info: 0000:05:00.0
>
> I have attached a log on when the drivers loads. There is a bit more
> debugging information enabled. You can see that the driver loads at
> first on devices 0000:02:00.0 and 0000:02:00.1 which are 82598
> devices and don't support SRIOV. So the driver switches off SRIOV.
> When it loads it on the 82599 which sits on 0000:05:00.0, it does not
> even have the SRIOV parameter enabled anymore, even though I load the
> driver with 'modprobe ixgbe max_vfs=4'.
Load the driver by specifying max_vfs for each adapter. For example if your adapters enumerate 82598, 82598, 82599:
modprobe ixgbe max_vfs=0,0,4
Thanks,
Emil
^ permalink raw reply
* Re: ixgbe: macvlan on PF/VF when SRIOV is enabled
From: Rose, Gregory V @ 2010-05-24 17:54 UTC (permalink / raw)
To: Shirley Ma
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Kirsher, Jeffrey T, kvm@vger.kernel.org, davem@davemloft.net
In-Reply-To: <1274720919.18023.19.camel@localhost.localdomain>
>-----Original Message-----
>From: Shirley Ma [mailto:mashirle@us.ibm.com]
>Sent: Monday, May 24, 2010 10:09 AM
>To: Rose, Gregory V
>Cc: Kirsher, Jeffrey T; davem@davemloft.net; kvm@vger.kernel.org;
>netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net
>Subject: RE: ixgbe: macvlan on PF/VF when SRIOV is enabled
>
>Hello Greg,
>
>Thanks for your prompt response.
>
>On Sat, 2010-05-22 at 10:53 -0700, Rose, Gregory V wrote:
>> As of 2.6.34 the ixgbe driver does not support multiple queues for
>> macvlan.
>> Support for multiple queues for macvlan will come in a subsequent
>> release.
>
>When it might happen?
Support for multiple queues in the PF driver is in the planning stage right now. Hopefully we'll get it in sooner rather than later but I can't give any solid dates for it.
I will double check my test to see whether macvlan
>was multiple queue or not.
Ah, so you just want to set multiple macvlan filters for the PF driver but aren't concerned about directing the traffic to different queues? We haven't tested that in SR-IOV modes of operation but we can have a look at it.
Then submitting my experimental patch for
>review.
We look forward to it and will be happy to provide feedback.
>
>> The VF driver does not support macvlan. Future releases may but there
>> are no immediate plans to support it.
>
>When it might be support in future. For performance reason, we are
>interested in macvlan + VF for multiples VMs.
There is a resource contention issue in this case. There are 128 MAC filters available. When VFs are allocated each will use a MAC filter entry. In the case where the maximum number of VFs are allocated (63 in this case) there aren't a whole lot of MAC filters left over to spread across that many VFs. Our view has been that it wouldn't be that much added value to support macvlan in the VFs but if there is a good case for it we'll consider it.
One thing you can do is allocate VFs and then load the VF driver in your host domain and then assign each of them a macvlan filter. You'd get a similar effect.
>
>One more question here: Does VF support promiscuous mode? I don't see
>the flag in ixgbevf driver.
No, there is no per-VF support for promiscuous mode in the HW as such, however you could set the unicast hash table array to all ones and then set the VFs you want to be in promiscuous mode to accept untagged packets via the VM filtering and offload register at offset 0F000h. However, re-provisioning the UTA for this purpose would preclude using it for the designed purpose.
I suggest that you pick up a copy of the developer's manual for the 82599 at the Intel developer's site.
http://developer.intel.com/products/ethernet/index.htm?iid=nc+ethernet#s1=all&s2=82576EB&s3=all
- Greg
------------------------------------------------------------------------------
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH][VHOST] fix race with guest on multi-buffer used buffer updates
From: Michael S. Tsirkin @ 2010-05-24 17:52 UTC (permalink / raw)
To: David Stevens; +Cc: kvm, netdev, netdev-owner
In-Reply-To: <OF4F730E37.19043E17-ON8825772D.00616972-8825772D.0062068B@us.ibm.com>
On Mon, May 24, 2010 at 10:50:44AM -0700, David Stevens wrote:
> Michael S. Tsirkin <mst@redhat.com> wrote on 05/24/2010 09:42:05 AM:
> >
> > I see. The logging is still bugg though I think.
>
> Possibly; migration isn't working for me under load even
> without mergeable buffers (investigating), so I haven't
> yet been able to test wrap w/ logging, but did you see
> something specific that's wrong?
>
> +-DLS
Yes, code only logs a single entry as dirty even if
multiple entries have been written.
--
MST
^ permalink raw reply
* Re: [PATCH][VHOST] fix race with guest on multi-buffer used buffer updates
From: David Stevens @ 2010-05-24 17:50 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: kvm, netdev, netdev-owner
In-Reply-To: <20100524164205.GA6664@redhat.com>
Michael S. Tsirkin <mst@redhat.com> wrote on 05/24/2010 09:42:05 AM:
>
> I see. The logging is still bugg though I think.
Possibly; migration isn't working for me under load even
without mergeable buffers (investigating), so I haven't
yet been able to test wrap w/ logging, but did you see
something specific that's wrong?
+-DLS
^ permalink raw reply
* Re: [PATCH 2.6.34-rc6] net: Improve ks8851 snl transmit performance
From: David Miller @ 2010-05-24 17:44 UTC (permalink / raw)
To: Tristram.Ha; +Cc: ben, netdev, linux-kernel, x0066660, s-jan
In-Reply-To: <14385191E87B904DBD836449AA30269D6DF528@MORGANITE.micrel.com>
From: "Ha, Tristram" <Tristram.Ha@Micrel.Com>
Date: Mon, 24 May 2010 10:28:40 -0700
> As the socket buffer is accepted by the lowest level network driver and
> reaches the end of its life, I think using one of its variables
> temporarily does not cause any harm.
The packet can be referenced by packet sniffers and other entities in
the kernel.
^ permalink raw reply
* Re: [PATCH][VHOST] fix race with guest on multi-buffer used buffer updates
From: Michael S. Tsirkin @ 2010-05-24 16:42 UTC (permalink / raw)
To: David Stevens; +Cc: kvm, netdev, netdev-owner
In-Reply-To: <OF1C9B2FC2.B6CFE18F-ON8825772D.0059C934-8825772D.005A61BA@us.ibm.com>
On Mon, May 24, 2010 at 09:27:15AM -0700, David Stevens wrote:
> netdev-owner@vger.kernel.org wrote on 05/24/2010 09:13:51 AM:
>
> > On Mon, May 24, 2010 at 08:52:40AM -0700, David Stevens wrote:
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote on 05/24/2010 03:17:10 AM:
> > >
> > > > On Thu, May 20, 2010 at 09:58:06AM -0700, David L Stevens wrote:
> > > > > [for Michael Tsirkin's vhost development git tree]
> > > > >
> > > > > This patch fixes a race between guest and host when
> > > > > adding used buffers wraps the ring. Without it, guests
> > > > > can see partial packets before num_buffers is set in
> > > > > the vnet header.
> > > > >
> > > > > Signed-off-by: David L Stevens <dlstevens@us.ibm.com>
> > > >
> > > > Could you please explain what the race is?
> > >
> > > Sure. The pre-patch code in the ring-wrap case
> > > does this:
> > >
> > > add part1 bufs
> > > update used index
> > > add part2 bufs
> > > update used index
> > >
> > > After we update the used index for part1, the part1
> > > buffers are available to the guest. If the guest is
> > > consuming at that point, it can process the partial
> > > packet before the rest of the packet is there. In that
> > > case, num_buffers will be greater than the number of
> > > buffers available to the guest and it'll drop the
> > > packet with a framing error. I was seeing 2 or 3 framing
> > > errors every 100 million packets or so pre-patch, none
> > > post-patch.
> > > Actually, the second sentence is incorrect in the
> > > original description-- num_buffers is up to date when
> > > the guest sees it, but the used index is not.
> > >
> > > +-DLS
> >
> > so this happens always - what does wrap-around refer to?
>
> The 2-part update only happens when a packet spans
> the end/beginning of the vring (the wrap). The framing error only
> happens if the guest sees the vring-wrapping packets
> before the second used-index write (the race).
> So, the framing error doesn't happen always--it's
> pretty rare. But with the patch, it never happens.
>
> +-DLS
I see. The logging is still bugg though I think.
^ permalink raw reply
* RE: [PATCH 2.6.34-rc6] net: Improve ks8851 snl transmit performance
From: Ha, Tristram @ 2010-05-24 17:28 UTC (permalink / raw)
To: David Miller; +Cc: ben, netdev, linux-kernel, x0066660, s-jan
In-Reply-To: <20100516.002607.84408834.davem@davemloft.net>
David Miller wrote:
> From: "Ha, Tristram" <Tristram.Ha@Micrel.Com>
> Date: Thu, 6 May 2010 15:50:27 -0700
>
>> From: Tristram Ha <Tristram.Ha@micrel.com>
>>
>> Under heavy transmission the driver will put 4 1514-byte packets in
>> queue and stop the device transmit queue. Only the last packet
>> triggers the transmit done interrupt and wakes up the device transmit
>> queue. That means a bit of time is wasted when the CPU cannot send
>> any more packet.
>>
>> The new implementation triggers the transmit interrupt when the
>> transmit buffer left is less than 3 packets. The maximum transmit
>> buffer size is 6144 bytes. This allows the device transmit queue to
>> be restarted sooner so that CPU can send more packets.
>>
>> For TCP receiving it also has the benefit of not triggering any
transmit interrupt at all.
>>
>> There is a driver option no_tx_opt so that the driver can revert to
>> original implementation. This allows user to verify if the transmit
>> performance actually improves.
>>
>> Signed-off-by: Tristram Ha <Tristram.Ha@micrel.com>
>
> First, if you want to post patches you have to format them properly as
ascii text with no longer
> than 80 column lines in your commit message.
> I really don't want to hear about your email client being a reason you
can't do this properly :-)
>
> Second, I don't think you can use the skb->ip_summed for this hacked
state tracking you are
> using. The packet might be shared with other entities, and therefore
if you change the field it
> won't be correct for them any more.
Sorry about the patch description. I must have forgotten the rules.
As the socket buffer is accepted by the lowest level network driver and
reaches the end of its life, I think using one of its variables
temporarily does not cause any harm.
^ permalink raw reply
* Re: [PATCH net-next-2.6 2/2] bonding: allow user-controlled output slave selection
From: Neil Horman @ 2010-05-24 17:08 UTC (permalink / raw)
To: Andy Gospodarek; +Cc: Jay Vosburgh, netdev
In-Reply-To: <20100524153108.GJ7497@gospo.rdu.redhat.com>
On Mon, May 24, 2010 at 11:31:08AM -0400, Andy Gospodarek wrote:
> On Mon, May 17, 2010 at 06:21:54PM -0700, Jay Vosburgh wrote:
> > Jay Vosburgh <fubar@us.ibm.com> wrote:
> > [...]
> > > For your patch, I'm exploring the idea of not setting
> > >IFF_SLAVE_INACTIVE on "inactive" slaves for an "all_slaves_active"
> > >option (I think that's a more descriptive name than "keep_all") instead
> > >of adding a new KEEP_ALL flag bit to priv_flags. Did you consider this
> > >methodology and exclude it for some reason?
> >
> > Following up to myself, I coded up approximately what I was
> > talking about. This doesn't require the extra priv_flag, and the sysfs
> > _store is a little more complicated, but this appears to work (testing
> > with ping -f after clearing the switch's MAC table to induce traffic
> > flooding). I didn't change the option name from "keep_all" here, but as
> > far as the functionality goes, this seems to do what I think you want it
> > to.
> >
> > -J
>
> This looks good to me, Jay. I tested the patch here along with the
> patch that started this thread and it works as expected.
>
> Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
>
> It seems like you were willing to take the patch that started this
> thread rather than a change that adds a new mode. If we agree that this
> is the correct direction, can you take a look at the patch and decide if
> you are willing to ACK it as-is or if more changes are needed?
>
> Thanks,
>
> -andy
>
Awesome, thanks Andy!
Neil
> --
> 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: ixgbe: macvlan on PF/VF when SRIOV is enabled
From: Shirley Ma @ 2010-05-24 17:08 UTC (permalink / raw)
To: Rose, Gregory V
Cc: Kirsher, Jeffrey T, davem@davemloft.net, kvm@vger.kernel.org,
netdev@vger.kernel.org, e1000-devel@lists.sourceforge.net
In-Reply-To: <43F901BD926A4E43B106BF17856F0755A79C099C@orsmsx508.amr.corp.intel.com>
Hello Greg,
Thanks for your prompt response.
On Sat, 2010-05-22 at 10:53 -0700, Rose, Gregory V wrote:
> As of 2.6.34 the ixgbe driver does not support multiple queues for
> macvlan.
> Support for multiple queues for macvlan will come in a subsequent
> release.
When it might happen? I will double check my test to see whether macvlan
was multiple queue or not. Then submitting my experimental patch for
review.
> The VF driver does not support macvlan. Future releases may but there
> are no immediate plans to support it.
When it might be support in future. For performance reason, we are
interested in macvlan + VF for multiples VMs.
One more question here: Does VF support promiscuous mode? I don't see
the flag in ixgbevf driver.
Thanks
Shirley
^ permalink raw reply
* Re: [PATCH net-2.6] macvlan: do a proper cleanup in macvlan_common_newlink()
From: Jiri Pirko @ 2010-05-24 17:02 UTC (permalink / raw)
To: netdev; +Cc: davem, kaber
In-Reply-To: <20100524155857.GO2810@psychotron.lab.eng.brq.redhat.com>
Mon, May 24, 2010 at 05:58:58PM CEST, jpirko@redhat.com wrote:
>Fixes possible memory leak.
>
>Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>---
> drivers/net/macvlan.c | 8 +++++++-
> 1 files changed, 7 insertions(+), 1 deletions(-)
>
>diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>index 4e238af..5f4694b 100644
>--- a/drivers/net/macvlan.c
>+++ b/drivers/net/macvlan.c
>@@ -634,11 +634,17 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
>
> err = register_netdevice(dev);
> if (err < 0)
>- return err;
>+ goto destroy_port;
>
> list_add_tail(&vlan->list, &port->vlans);
> netif_stacked_transfer_operstate(lowerdev, dev);
>+
> return 0;
>+
>+destroy_port:
>+ macvlan_port_destroy(lowerdev);
>+
>+ return err;
> }
> EXPORT_SYMBOL_GPL(macvlan_common_newlink);
>
>--
>1.6.6.1
Actually it should be rather like this:
Subject: [PATCH net-2.6] macvlan: do proper cleanup in macvlan_common_newlink() V2
Fixes possible memory leak.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/macvlan.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 4e238af..87e8d4c 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -634,11 +634,18 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
err = register_netdevice(dev);
if (err < 0)
- return err;
+ goto destroy_port;
list_add_tail(&vlan->list, &port->vlans);
netif_stacked_transfer_operstate(lowerdev, dev);
+
return 0;
+
+destroy_port:
+ if (list_empty(&port->vlans))
+ macvlan_port_destroy(lowerdev);
+
+ return err;
}
EXPORT_SYMBOL_GPL(macvlan_common_newlink);
--
1.6.6.1
^ permalink raw reply related
* [patch v2] caif: cleanup: remove duplicate checks
From: Dan Carpenter @ 2010-05-24 16:29 UTC (permalink / raw)
To: walter harms
Cc: netdev, Sjur Braendeland, Stephen Rothwell, kernel-janitors,
David S. Miller
In-Reply-To: <4BF8E6FE.9000303@bfs.de>
"phyinfo" can never be null here because we assigned it an address, so I
removed both the assert and the second check inside the if statement. I
removed the "phyinfo->phy_layer != NULL" check as well because that was
asserted earlier.
Walter Harms suggested I move the "phyinfo->phy_ref_count++;" outside
the if condition for readability, so I have done that.
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
The original code increments phyinfo->phy_ref_count++ even if
phyinfo->phy_layer->modemcmd is NULL. I kept it the same in my code,
but maybe I should change that?
v2: Removed an assert. Moved the increment outside the if condition.
diff --git a/net/caif/cfcnfg.c b/net/caif/cfcnfg.c
index df43f26..7c81974 100644
--- a/net/caif/cfcnfg.c
+++ b/net/caif/cfcnfg.c
@@ -308,19 +308,15 @@ cfcnfg_linkup_rsp(struct cflayer *layer, u8 channel_id, enum cfctrl_srv serv,
caif_assert(cnfg != NULL);
caif_assert(phyid != 0);
phyinfo = &cnfg->phy_layers[phyid];
- caif_assert(phyinfo != NULL);
caif_assert(phyinfo->id == phyid);
caif_assert(phyinfo->phy_layer != NULL);
caif_assert(phyinfo->phy_layer->id == phyid);
- if (phyinfo != NULL &&
- phyinfo->phy_ref_count++ == 0 &&
- phyinfo->phy_layer != NULL &&
+ phyinfo->phy_ref_count++;
+ if (phyinfo->phy_ref_count == 1 &&
phyinfo->phy_layer->modemcmd != NULL) {
- caif_assert(phyinfo->phy_layer->id == phyid);
phyinfo->phy_layer->modemcmd(phyinfo->phy_layer,
_CAIF_MODEMCMD_PHYIF_USEFULL);
-
}
adapt_layer->id = channel_id;
^ permalink raw reply related
* Re: [PATCH][VHOST] fix race with guest on multi-buffer used buffer updates
From: David Stevens @ 2010-05-24 16:27 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: kvm, netdev, netdev-owner
In-Reply-To: <20100524161351.GA6580@redhat.com>
netdev-owner@vger.kernel.org wrote on 05/24/2010 09:13:51 AM:
> On Mon, May 24, 2010 at 08:52:40AM -0700, David Stevens wrote:
> > "Michael S. Tsirkin" <mst@redhat.com> wrote on 05/24/2010 03:17:10 AM:
> >
> > > On Thu, May 20, 2010 at 09:58:06AM -0700, David L Stevens wrote:
> > > > [for Michael Tsirkin's vhost development git tree]
> > > >
> > > > This patch fixes a race between guest and host when
> > > > adding used buffers wraps the ring. Without it, guests
> > > > can see partial packets before num_buffers is set in
> > > > the vnet header.
> > > >
> > > > Signed-off-by: David L Stevens <dlstevens@us.ibm.com>
> > >
> > > Could you please explain what the race is?
> >
> > Sure. The pre-patch code in the ring-wrap case
> > does this:
> >
> > add part1 bufs
> > update used index
> > add part2 bufs
> > update used index
> >
> > After we update the used index for part1, the part1
> > buffers are available to the guest. If the guest is
> > consuming at that point, it can process the partial
> > packet before the rest of the packet is there. In that
> > case, num_buffers will be greater than the number of
> > buffers available to the guest and it'll drop the
> > packet with a framing error. I was seeing 2 or 3 framing
> > errors every 100 million packets or so pre-patch, none
> > post-patch.
> > Actually, the second sentence is incorrect in the
> > original description-- num_buffers is up to date when
> > the guest sees it, but the used index is not.
> >
> > +-DLS
>
> so this happens always - what does wrap-around refer to?
The 2-part update only happens when a packet spans
the end/beginning of the vring (the wrap). The framing error only
happens if the guest sees the vring-wrapping packets
before the second used-index write (the race).
So, the framing error doesn't happen always--it's
pretty rare. But with the patch, it never happens.
+-DLS
^ permalink raw reply
* Re: [PATCH][VHOST] fix race with guest on multi-buffer used buffer updates
From: Michael S. Tsirkin @ 2010-05-24 16:13 UTC (permalink / raw)
To: David Stevens; +Cc: kvm, netdev
In-Reply-To: <OF343AB88F.017B31FE-ON8825772D.00564524-8825772D.00573773@us.ibm.com>
On Mon, May 24, 2010 at 08:52:40AM -0700, David Stevens wrote:
> "Michael S. Tsirkin" <mst@redhat.com> wrote on 05/24/2010 03:17:10 AM:
>
> > On Thu, May 20, 2010 at 09:58:06AM -0700, David L Stevens wrote:
> > > [for Michael Tsirkin's vhost development git tree]
> > >
> > > This patch fixes a race between guest and host when
> > > adding used buffers wraps the ring. Without it, guests
> > > can see partial packets before num_buffers is set in
> > > the vnet header.
> > >
> > > Signed-off-by: David L Stevens <dlstevens@us.ibm.com>
> >
> > Could you please explain what the race is?
>
> Sure. The pre-patch code in the ring-wrap case
> does this:
>
> add part1 bufs
> update used index
> add part2 bufs
> update used index
>
> After we update the used index for part1, the part1
> buffers are available to the guest. If the guest is
> consuming at that point, it can process the partial
> packet before the rest of the packet is there. In that
> case, num_buffers will be greater than the number of
> buffers available to the guest and it'll drop the
> packet with a framing error. I was seeing 2 or 3 framing
> errors every 100 million packets or so pre-patch, none
> post-patch.
> Actually, the second sentence is incorrect in the
> original description-- num_buffers is up to date when
> the guest sees it, but the used index is not.
>
> +-DLS
so this happens always - what does wrap-around refer to?
>
> >
> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > index 7f2568d..74790ab 100644
> > > --- a/drivers/vhost/vhost.c
> > > +++ b/drivers/vhost/vhost.c
> > > @@ -1065,14 +1065,6 @@ static int __vhost_add_used_n(struct
> vhost_virtqueue *vq,
> > > vq_err(vq, "Failed to write used");
> > > return -EFAULT;
> > > }
> > > - /* Make sure buffer is written before we update index. */
> > > - smp_wmb();
> > > - if (put_user(vq->last_used_idx + count, &vq->used->idx)) {
> > > - vq_err(vq, "Failed to increment used idx");
> > > - return -EFAULT;
> > > - }
> > > - if (unlikely(vq->log_used))
> > > - vhost_log_used(vq, used);
> > > vq->last_used_idx += count;
> > > return 0;
> > > }
> > > @@ -1093,7 +1085,17 @@ int vhost_add_used_n(struct vhost_virtqueue
> *vq,
> > struct vring_used_elem *heads,
> > > heads += n;
> > > count -= n;
> > > }
> > > - return __vhost_add_used_n(vq, heads, count);
> > > + r = __vhost_add_used_n(vq, heads, count);
> > > +
> > > + /* Make sure buffer is written before we update index. */
> > > + smp_wmb();
> > > + if (put_user(vq->last_used_idx, &vq->used->idx)) {
> > > + vq_err(vq, "Failed to increment used idx");
> > > + return -EFAULT;
> > > + }
> > > + if (unlikely(vq->log_used))
> > > + vhost_log_used(vq, vq->used->ring + start);
> > > + return r;
> > > }
I think a single vhost_log_used will not DTRT here:
it only updates log for a single entry.
So we'll need to split this to functions that
1. log used entries writes: called from __vhost_add_used_n
2. log used index write: called from vhost_add_used_n
> > >
> > > /* This actually signals the guest, using eventfd. */
> > >
^ permalink raw reply
* [PATCH net-2.6] macvlan: do a proper cleanup in macvlan_common_newlink()
From: Jiri Pirko @ 2010-05-24 15:58 UTC (permalink / raw)
To: netdev; +Cc: davem, kaber
Fixes possible memory leak.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/macvlan.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 4e238af..5f4694b 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -634,11 +634,17 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
err = register_netdevice(dev);
if (err < 0)
- return err;
+ goto destroy_port;
list_add_tail(&vlan->list, &port->vlans);
netif_stacked_transfer_operstate(lowerdev, dev);
+
return 0;
+
+destroy_port:
+ macvlan_port_destroy(lowerdev);
+
+ return err;
}
EXPORT_SYMBOL_GPL(macvlan_common_newlink);
--
1.6.6.1
^ permalink raw reply related
* Re: [PATCH][VHOST] fix race with guest on multi-buffer used buffer updates
From: David Stevens @ 2010-05-24 15:52 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: kvm, netdev
In-Reply-To: <20100524101709.GA28349@redhat.com>
"Michael S. Tsirkin" <mst@redhat.com> wrote on 05/24/2010 03:17:10 AM:
> On Thu, May 20, 2010 at 09:58:06AM -0700, David L Stevens wrote:
> > [for Michael Tsirkin's vhost development git tree]
> >
> > This patch fixes a race between guest and host when
> > adding used buffers wraps the ring. Without it, guests
> > can see partial packets before num_buffers is set in
> > the vnet header.
> >
> > Signed-off-by: David L Stevens <dlstevens@us.ibm.com>
>
> Could you please explain what the race is?
Sure. The pre-patch code in the ring-wrap case
does this:
add part1 bufs
update used index
add part2 bufs
update used index
After we update the used index for part1, the part1
buffers are available to the guest. If the guest is
consuming at that point, it can process the partial
packet before the rest of the packet is there. In that
case, num_buffers will be greater than the number of
buffers available to the guest and it'll drop the
packet with a framing error. I was seeing 2 or 3 framing
errors every 100 million packets or so pre-patch, none
post-patch.
Actually, the second sentence is incorrect in the
original description-- num_buffers is up to date when
the guest sees it, but the used index is not.
+-DLS
>
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index 7f2568d..74790ab 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -1065,14 +1065,6 @@ static int __vhost_add_used_n(struct
vhost_virtqueue *vq,
> > vq_err(vq, "Failed to write used");
> > return -EFAULT;
> > }
> > - /* Make sure buffer is written before we update index. */
> > - smp_wmb();
> > - if (put_user(vq->last_used_idx + count, &vq->used->idx)) {
> > - vq_err(vq, "Failed to increment used idx");
> > - return -EFAULT;
> > - }
> > - if (unlikely(vq->log_used))
> > - vhost_log_used(vq, used);
> > vq->last_used_idx += count;
> > return 0;
> > }
> > @@ -1093,7 +1085,17 @@ int vhost_add_used_n(struct vhost_virtqueue
*vq,
> struct vring_used_elem *heads,
> > heads += n;
> > count -= n;
> > }
> > - return __vhost_add_used_n(vq, heads, count);
> > + r = __vhost_add_used_n(vq, heads, count);
> > +
> > + /* Make sure buffer is written before we update index. */
> > + smp_wmb();
> > + if (put_user(vq->last_used_idx, &vq->used->idx)) {
> > + vq_err(vq, "Failed to increment used idx");
> > + return -EFAULT;
> > + }
> > + if (unlikely(vq->log_used))
> > + vhost_log_used(vq, vq->used->ring + start);
> > + return r;
> > }
> >
> > /* This actually signals the guest, using eventfd. */
> >
^ permalink raw reply
* Re: [PATCH net-next-2.6 2/2] bonding: allow user-controlled output slave selection
From: Andy Gospodarek @ 2010-05-24 15:31 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: Neil Horman, Andy Gospodarek, netdev
In-Reply-To: <12958.1274145714@death.nxdomain.ibm.com>
On Mon, May 17, 2010 at 06:21:54PM -0700, Jay Vosburgh wrote:
> Jay Vosburgh <fubar@us.ibm.com> wrote:
> [...]
> > For your patch, I'm exploring the idea of not setting
> >IFF_SLAVE_INACTIVE on "inactive" slaves for an "all_slaves_active"
> >option (I think that's a more descriptive name than "keep_all") instead
> >of adding a new KEEP_ALL flag bit to priv_flags. Did you consider this
> >methodology and exclude it for some reason?
>
> Following up to myself, I coded up approximately what I was
> talking about. This doesn't require the extra priv_flag, and the sysfs
> _store is a little more complicated, but this appears to work (testing
> with ping -f after clearing the switch's MAC table to induce traffic
> flooding). I didn't change the option name from "keep_all" here, but as
> far as the functionality goes, this seems to do what I think you want it
> to.
>
> -J
This looks good to me, Jay. I tested the patch here along with the
patch that started this thread and it works as expected.
Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
It seems like you were willing to take the patch that started this
thread rather than a change that adds a new mode. If we agree that this
is the correct direction, can you take a look at the patch and decide if
you are willing to ACK it as-is or if more changes are needed?
Thanks,
-andy
^ permalink raw reply
* [PATCH net-2.6] be2net: Bug fix in init code in probe
From: Sarveshwar Bandi @ 2010-05-24 11:20 UTC (permalink / raw)
To: netdev; +Cc: davem
PCI function reset needs to invoked after fw init ioctl is issued.
Signed-off-by: Sarveshwar Bandi <sarveshwarb@serverengines.com>
---
drivers/net/benet/be_main.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index 058d7f9..1c79c20 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -2487,10 +2487,6 @@ static int __devinit be_probe(struct pci
status = be_cmd_POST(adapter);
if (status)
goto ctrl_clean;
-
- status = be_cmd_reset_function(adapter);
- if (status)
- goto ctrl_clean;
}
/* tell fw we're ready to fire cmds */
@@ -2498,6 +2494,12 @@ static int __devinit be_probe(struct pci
if (status)
goto ctrl_clean;
+ if (be_physfn(adapter)) {
+ status = be_cmd_reset_function(adapter);
+ if (status)
+ goto ctrl_clean;
+ }
+
status = be_stats_init(adapter);
if (status)
goto ctrl_clean;
--
1.4.0
^ permalink raw reply related
* Re: cls_cgroup: Store classid in struct sock
From: Herbert Xu @ 2010-05-24 11:24 UTC (permalink / raw)
To: Neil Horman; +Cc: David Miller, eric.dumazet, bmb, tgraf, nhorman, netdev
In-Reply-To: <20100524110950.GA22216@hmsreliant.think-freely.org>
On Mon, May 24, 2010 at 07:09:50AM -0400, Neil Horman wrote:
>
> Excuse all my noise from before, Herberts patch works fine. Apparently the
> problem was mine. QEMU creates several threads during startup, but adding the
> processid to a cgroup doesn't cause its child threads to get added
> automatically. Doing this by hand causes this to work.
Thanks a lot for testing Neil!
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Question about an assignment in handle_ing()
From: Jiri Pirko @ 2010-05-24 11:22 UTC (permalink / raw)
To: hadi; +Cc: netdev, davem, herbert, kaber
Hi Jamal.
I want to ask you about the following chunk of code of net/core/dev.c in function handle_ing():
2699 if (*pt_prev) {
2700 *ret = deliver_skb(skb, *pt_prev, orig_dev);
2701 *pt_prev = NULL;
2702 } else {
2703 /* Huh? Why does turning on AF_PACKET affect this? */
2704 skb->tc_verd = SET_TC_OK2MUNGE(skb->tc_verd);
2705 }
The assignment (in "else" statement) was added by you here:
http://linux.bkbits.net:8080/linux-2.6.12-stable/?PAGE=cset&REV=40cfc085xjyQLyB1UiTbgaRZSlCN9w
The comment was added after move of this code by Herbert:
http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=commitdiff;h=f697c3e8b35c18b2698d64137c0fa84b0cdb3d10
Question is if this code is correct here. Maybe I'm missing something but
why is this dependent on a ptype was found previously?
Thanks,
Jirka
^ permalink raw reply
* Re: cls_cgroup: Store classid in struct sock
From: Neil Horman @ 2010-05-24 11:09 UTC (permalink / raw)
To: David Miller; +Cc: herbert, eric.dumazet, bmb, tgraf, nhorman, netdev
In-Reply-To: <20100524.001429.259990428.davem@davemloft.net>
On Mon, May 24, 2010 at 12:14:29AM -0700, David Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Mon, 24 May 2010 17:07:43 +1000
>
> > On Sun, May 23, 2010 at 11:55:57PM -0700, David Miller wrote:
> >>
> >> Probably you only tested the build with cgroups enabled?
> >
> > Indeed, that does seem to be the problem here. It turns out
> > that their struct is only declared when CONFIG_CGROUPS is defined,
> > how annoying.
> >
> > Oh well I guess I'll follow their example :)
> >
> > cls_cgroup: Store classid in struct sock
>
> Thanks for fixing this up, applied.
>
Excuse all my noise from before, Herberts patch works fine. Apparently the
problem was mine. QEMU creates several threads during startup, but adding the
processid to a cgroup doesn't cause its child threads to get added
automatically. Doing this by hand causes this to work.
Thanks
Neil
^ permalink raw reply
* Re: [PATCH][VHOST] fix race with guest on multi-buffer used buffer updates
From: Michael S. Tsirkin @ 2010-05-24 10:17 UTC (permalink / raw)
To: David L Stevens; +Cc: netdev, kvm
In-Reply-To: <1274374686.8492.12.camel@w-dls.beaverton.ibm.com>
On Thu, May 20, 2010 at 09:58:06AM -0700, David L Stevens wrote:
> [for Michael Tsirkin's vhost development git tree]
>
> This patch fixes a race between guest and host when
> adding used buffers wraps the ring. Without it, guests
> can see partial packets before num_buffers is set in
> the vnet header.
>
> Signed-off-by: David L Stevens <dlstevens@us.ibm.com>
Could you please explain what the race is?
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 7f2568d..74790ab 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1065,14 +1065,6 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
> vq_err(vq, "Failed to write used");
> return -EFAULT;
> }
> - /* Make sure buffer is written before we update index. */
> - smp_wmb();
> - if (put_user(vq->last_used_idx + count, &vq->used->idx)) {
> - vq_err(vq, "Failed to increment used idx");
> - return -EFAULT;
> - }
> - if (unlikely(vq->log_used))
> - vhost_log_used(vq, used);
> vq->last_used_idx += count;
> return 0;
> }
> @@ -1093,7 +1085,17 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
> heads += n;
> count -= n;
> }
> - return __vhost_add_used_n(vq, heads, count);
> + r = __vhost_add_used_n(vq, heads, count);
> +
> + /* Make sure buffer is written before we update index. */
> + smp_wmb();
> + if (put_user(vq->last_used_idx, &vq->used->idx)) {
> + vq_err(vq, "Failed to increment used idx");
> + return -EFAULT;
> + }
> + if (unlikely(vq->log_used))
> + vhost_log_used(vq, vq->used->ring + start);
> + return r;
> }
>
> /* This actually signals the guest, using eventfd. */
>
^ 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