* [PATCH RFC v1 3/7] net: phy: resume/suspend PHYs on attach/detach
From: Sebastian Hesselbarth @ 2013-11-20 20:21 UTC (permalink / raw)
To: Sebastian Hesselbarth
Cc: Sebastian Hesselbarth, David S. Miller, netdev, linux-arm-kernel,
linux-kernel
In-Reply-To: <1384978913-8052-1-git-send-email-sebastian.hesselbarth@gmail.com>
This ensures PHYs are resumed on attach and suspended on detach.
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
drivers/net/phy/phy_device.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 2442895..a421fc6 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -570,6 +570,8 @@ static int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
if (err)
phy_detach(phydev);
+ phy_resume(phydev);
+
return err;
}
@@ -615,6 +617,7 @@ void phy_detach(struct phy_device *phydev)
{
phydev->attached_dev->phydev = NULL;
phydev->attached_dev = NULL;
+ phy_suspend(phydev);
/* If the device had no specific driver before (i.e. - it
* was using the generic driver), we unbind the device
--
1.7.2.5
^ permalink raw reply related
* [PATCH RFC v1 5/7] net: mv643xx_eth: resume/suspend PHY on port start/stop
From: Sebastian Hesselbarth @ 2013-11-20 20:21 UTC (permalink / raw)
To: Sebastian Hesselbarth
Cc: Sebastian Hesselbarth, David S. Miller, netdev, linux-arm-kernel,
linux-kernel
In-Reply-To: <1384978913-8052-1-git-send-email-sebastian.hesselbarth@gmail.com>
Network PHYs consume a noticable amount of power. This adds phy_resume
on port start and phy_suspend on port stop to save this power if the
port is down anyway. While at it, also properly start/stop the phy.
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
drivers/net/ethernet/marvell/mv643xx_eth.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 00cd36e..55805b2 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -2097,6 +2097,8 @@ static void port_start(struct mv643xx_eth_private *mp)
mv643xx_eth_get_settings(mp->dev, &cmd);
phy_reset(mp);
+ phy_resume(mp->phy);
+ phy_start(mp->phy);
mv643xx_eth_set_settings(mp->dev, &cmd);
}
@@ -2306,6 +2308,11 @@ static int mv643xx_eth_stop(struct net_device *dev)
for (i = 0; i < mp->txq_count; i++)
txq_deinit(mp->txq + i);
+ if (mp->phy) {
+ phy_stop(mp->phy);
+ phy_suspend(mp->phy);
+ }
+
return 0;
}
--
1.7.2.5
^ permalink raw reply related
* [PATCH RFC v1 6/7] net: mvneta: resume/suspend PHY on port start/stop
From: Sebastian Hesselbarth @ 2013-11-20 20:21 UTC (permalink / raw)
To: Sebastian Hesselbarth
Cc: Sebastian Hesselbarth, David S. Miller, netdev, linux-arm-kernel,
linux-kernel
In-Reply-To: <1384978913-8052-1-git-send-email-sebastian.hesselbarth@gmail.com>
Network PHYs consume a noticable amount of power. This adds phy_resume
on start_dev and phy_suspend on stop_dev to save this power if the
port is down anyway.
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
drivers/net/ethernet/marvell/mvneta.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index b8e232b..8688e91 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -2194,6 +2194,7 @@ static void mvneta_start_dev(struct mvneta_port *pp)
mvreg_write(pp, MVNETA_INTR_NEW_MASK,
MVNETA_RX_INTR_MASK(rxq_number));
+ phy_resume(pp->phy_dev);
phy_start(pp->phy_dev);
netif_tx_start_all_queues(pp->dev);
}
@@ -2201,6 +2202,7 @@ static void mvneta_start_dev(struct mvneta_port *pp)
static void mvneta_stop_dev(struct mvneta_port *pp)
{
phy_stop(pp->phy_dev);
+ phy_suspend(pp->phy_dev);
napi_disable(&pp->napi);
--
1.7.2.5
^ permalink raw reply related
* [PATCH RFC v1 7/7] net: cpsw: resume/suspend PHY on port start/stop
From: Sebastian Hesselbarth @ 2013-11-20 20:21 UTC (permalink / raw)
To: Sebastian Hesselbarth
Cc: Sebastian Hesselbarth, David S. Miller, netdev, linux-arm-kernel,
linux-kernel
In-Reply-To: <1384978913-8052-1-git-send-email-sebastian.hesselbarth@gmail.com>
Network PHYs consume a noticable amount of power. This adds phy_resume
on slave_open and phy_suspend on slave_stop to save this power if the
port is down anyway.
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
drivers/net/ethernet/ti/cpsw.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 90d41d2..f1dc54f 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1013,6 +1013,7 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
} else {
dev_info(priv->dev, "phy found : id is : 0x%x\n",
slave->phy->phy_id);
+ phy_resume(slave->phy);
phy_start(slave->phy);
/* Configure GMII_SEL register */
@@ -1081,6 +1082,7 @@ static void cpsw_slave_stop(struct cpsw_slave *slave, struct cpsw_priv *priv)
if (!slave->phy)
return;
phy_stop(slave->phy);
+ phy_suspend(slave->phy);
phy_disconnect(slave->phy);
slave->phy = NULL;
}
--
1.7.2.5
^ permalink raw reply related
* [PATCH RFC v1 4/7] net: phy: suspend unused PHYs on mdio_bus in late_initcall
From: Sebastian Hesselbarth @ 2013-11-20 20:21 UTC (permalink / raw)
To: Sebastian Hesselbarth
Cc: Sebastian Hesselbarth, David S. Miller, netdev, linux-arm-kernel,
linux-kernel
In-Reply-To: <1384978913-8052-1-git-send-email-sebastian.hesselbarth@gmail.com>
Since phy_attach ensures PHYs are resumed, we can now suspend all
PHYs that have no attached netdev after initcalls.
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
drivers/net/phy/mdio_bus.c | 27 +++++++++++++++++++++++++++
1 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 5617876..10eba58 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -320,6 +320,33 @@ static int mdio_bus_match(struct device *dev, struct device_driver *drv)
(phydev->phy_id & phydrv->phy_id_mask));
}
+static int mdio_bus_suspend_unused(struct device *busdev, void *data)
+{
+ struct mii_bus *bus = to_mii_bus(busdev);
+ struct phy_device *phydev;
+ struct phy_driver *phydrv;
+ int i;
+
+ for (i = 0; i < PHY_MAX_ADDR; i++) {
+ if (!bus->phy_map[i])
+ continue;
+
+ phydev = to_phy_device(&bus->phy_map[i]->dev);
+ phydrv = to_phy_driver(phydev->dev.driver);
+ if (!phydev->attached_dev && phydrv && phydrv->suspend)
+ phy_suspend(phydev);
+ }
+
+ return 0;
+}
+
+static int mdio_bus_class_suspend_unused(void)
+{
+ return class_for_each_device(&mdio_bus_class, NULL, NULL,
+ mdio_bus_suspend_unused);
+}
+late_initcall_sync(mdio_bus_class_suspend_unused);
+
#ifdef CONFIG_PM
static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
--
1.7.2.5
^ permalink raw reply related
* Fw: [Bug 65271] New: Acceptable RST packets (with seq number in window) are ignored in LAST_ACK state (and previously sent data pending acknowledgement)
From: Stephen Hemminger @ 2013-11-20 20:22 UTC (permalink / raw)
To: netdev
Begin forwarded message:
Date: Wed, 20 Nov 2013 07:03:59 -0800
From: "bugzilla-daemon@bugzilla.kernel.org" <bugzilla-daemon@bugzilla.kernel.org>
To: "stephen@networkplumber.org" <stephen@networkplumber.org>
Subject: [Bug 65271] New: Acceptable RST packets (with seq number in window) are ignored in LAST_ACK state (and previously sent data pending acknowledgement)
https://bugzilla.kernel.org/show_bug.cgi?id=65271
Bug ID: 65271
Summary: Acceptable RST packets (with seq number in window) are
ignored in LAST_ACK state (and previously sent data
pending acknowledgement)
Product: Networking
Version: 2.5
Kernel Version: 2.6.32.59-0.7-default
Hardware: All
OS: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: IPV4
Assignee: shemminger@linux-foundation.org
Reporter: julien.delacroix@gmail.com
Regression: No
Created attachment 115271
--> https://bugzilla.kernel.org/attachment.cgi?id=115271&action=edit
TCP dump showing described scenario
PCAP file of traffic capture is attached.
Note that capture was taken directly on server host, so RST packets definitely
reached the server host.
Here is the flow:
1: Client ---- SYN ----> Server
2: Client <- SYN, ACK -- Server
3: Client ---- ACK ----> Server
4: Client ---- FIN ----> Server ( server in CLOSE_WAIT state)
5: Client <- PSH, ACK -- Server
6: Client <- FIN, ACK -- Server ( server in LAST_ACK state)
7: Client ---- RST ----> Server
Then repeat N times:
8: Client <- PSH, ACK -- Server
9: Client ---RST, ACK -> Server
Notes:
5: packet contains 3 bytes of data
7: no ACK flag, and sequence number is correct, but RST packet is ignored
8: retransmission of step 5
9: the sequence number is correct as well as the acknowledgment number
(acknowledging data in 5, but not yet FIN in 6), but this RST is again ignored.
>From this point client keeps retransmitting 5, while server keeps answering
with acceptable RST packets.
You can observe this scenario (with 5 retransmissions and 5 RST) in the
attached PCAP file.
--
You are receiving this mail because:
You are the assignee for the bug.
^ permalink raw reply
* Fw: [Bug 65261] New: Packet loss or excessive packet delay repeatedly for some seconds
From: Stephen Hemminger @ 2013-11-20 20:23 UTC (permalink / raw)
To: netdev
Begin forwarded message:
Date: Wed, 20 Nov 2013 05:57:59 -0800
From: "bugzilla-daemon@bugzilla.kernel.org" <bugzilla-daemon@bugzilla.kernel.org>
To: "stephen@networkplumber.org" <stephen@networkplumber.org>
Subject: [Bug 65261] New: Packet loss or excessive packet delay repeatedly for some seconds
https://bugzilla.kernel.org/show_bug.cgi?id=65261
Bug ID: 65261
Summary: Packet loss or excessive packet delay repeatedly for
some seconds
Product: Networking
Version: 2.5
Kernel Version: 3.10.19
Hardware: x86-64
OS: Linux
Tree: Mainline
Status: NEW
Severity: high
Priority: P1
Component: IPV4
Assignee: shemminger@linux-foundation.org
Reporter: arno@wagner.name
Regression: No
Created attachment 115261
--> https://bugzilla.kernel.org/attachment.cgi?id=115261&action=edit
Kernel config 3.10.19
I recently upgraded my development-server/firewall/NAT-box from 3.10.17 to
3.10.19. Since then I noticed increased DNS lookup failures on a connected
Windows box and occasional slow updates on putty-SSH logins when scrolling in
an editor (joe, takes something like an estimated 100-300ms for screen
updates). These update delays are repeatable for something like 10-20 seconds
or more, e.g. inserting a line and then deleting again, then vanish. The delays
make remote editing hard to do when they happen. The DNS lookup failures are
really annoying.
I have not found a way to reliably trigger the problem.
Going back to 3.10.17 fixed the issue as far as I can tell. (Several hours
editing source code without it showing up.) The only change between the two
configurations was that I added the scsi CDROM driver (which should not be able
to cause this?).
The network connection has an iptables "all pass" on the server side for the
affected connection. The client side is a Win7 machine. The network link is GbE
with 2 Switches in there. No packet loss on ping/ping -f. Network hardware is
Intel 82574L Gigabit card on both sides.
--
You are receiving this mail because:
You are the assignee for the bug.
^ permalink raw reply
* Re: [RFC net-next PATCH] macvtap: Add packet capture support
From: Michael S. Tsirkin @ 2013-11-20 20:30 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <528D1935.9020403@redhat.com>
On Wed, Nov 20, 2013 at 03:19:01PM -0500, Vlad Yasevich wrote:
> On 11/20/2013 03:06 PM, Michael S. Tsirkin wrote:
> > On Wed, Nov 20, 2013 at 02:36:40PM -0500, Vlad Yasevich wrote:
> >> On 11/20/2013 01:19 PM, Michael S. Tsirkin wrote:
> >>> On Wed, Nov 20, 2013 at 01:04:09PM -0500, Vlad Yasevich wrote:
> >>>> Currently it is impossible to capture traffic when using a macvtap
> >>>> device. The reason is that all capture handling is done either in
> >>>> dev_hard_start_xmit() or in __netif_receive_skb_core(). Macvtap
> >>>> currenlty doesn't use dev_hard_start_xmit(), and at the time the
> >>>> packet traverses __netif_receive_skb_core, the skb->dev is set to
> >>>> the lower-level device that doesn't end up matching macvtap.
> >>>>
> >>>> To solve the issue, use dev_hard_start_xmit() on the output path.
> >>>> On the input path, it is toughter to solve since macvtap ends up
> >>>> consuming the skb so there is nothing more left for
> >>>> __netif_receive_skb_core() to do.
> >
> > Actually I thought I understand what you are saying here, but now I
> > don't. bridge installs rx handler exactly in the same way.
> > packet handlers seem to be called before the rx handlers so
> > everything should just work.
> >
> > Is this about the bridge mode again?
>
> No. It has to do with bridge submitting the frame back to the
> network stack to forward it to the ports, but macvtap ends up
> stealing it.
Confused.
rx_handler = rcu_dereference(skb->dev->rx_handler);
if (rx_handler) {
if (pt_prev) {
ret = deliver_skb(skb, pt_prev, orig_dev);
pt_prev = NULL;
}
switch (rx_handler(&skb)) {
....
so packet handlers (including packet socket)
seem to be invoked before rx handlers (including bridge and macvtap).
What's the issue then?
I guess I'm missing something obvious.
> Also, bridge, if running in promisc mode will hand the frame up
> (as if it received it). See the IFF_PROMISC code in
> br_handle_frame_finish().
> So, if someone is capturing on the bridge itself, this lets it
> see the packets even though they are not destined for the bridge
> device.
Yes, that's quite expected: same thing will happen with
real hardware: set it to promisc, see lots of packets
destined at others.
> >
> >> A simple solution is to
> >>>> pull the code that delivers things to the taps/captures into
> >>>> its own function and allow macvtap to call it from its receive
> >>>> routine.
> >>>>
> >>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> >>>> ---
> >>>> This is only an RFC. I'd like to solicit comments on this simple
> >>>> approach.
> >>>
> >>> I'm kind of worried about this. What worries me is that normally
> >>> if you have a packet socket bound to all interfaces, what it shows is
> >>> traffic to/from the box.
> >>>
> >>> This might be a bug for someone, but I suspect at this point this
> >>> is part of the ABI.
> >>>
> >>> But macvtap bypasses most of the host networking stack,
> >>> So I worry that suddenly showing these packets would be confusing.
> >>
> >> Is it really different from using bridge and tap? If someone
> >> does 'tcpdump -i any', they will see packets sent by the guest with
> >> bridge+tap.
> >> It makes sense to do that same thing for macvtap, no?
> >
> > I was going by your comments not the code.
> > Assuming we never showed macvtap traffic this might
> > be part of ABI.
>
> BTW, if we end up doing it with a new rx_handler, it will end up
> showing exactly the same traffic is this patch does because the
> common path in __netif_receive_skb_core() will run and deliver
> to all registered eligible entries in ptype_all.
>
> -vlad
Dave here thinks it's not a problem, I trust him ...
> >
> >>>
> >>> Assuming we want to show this traffic, I think it's preferable to
> >>> - if (!ptype->dev || ptype->dev == skb->dev) {
> >>> + if (ptype->dev == skb->dev) {
> >>> so you have to bind to macvtap explicitly to see the traffic.
> >>>
> >>>
> >>> Of course when you start binding to specific macvtaps
> >>> it doesn't scale well because we'll have a long list
> >>> on ptype_all suddenly.
> >>
> >> How likely is that really? ptype_all doesn't scale as it.
> >> Any time you don't set the protocol field, the entry goes
> >> into ptype_all. Macvtap doesn't change that. You can
> >> create a long list in ptype all if you have lots of guests and
> >> you want to snoop on each guest separately thus binding to tap
> >> device.
> >>
> >> -vlad
> >>
> >>> This might mean we need to keep this list per-device ...
> >>>
> >>>> A more complicated solution would have been to give
> >>>> macvtap its own rx_handler and return RX_HANDLER_ANOTHER during
> >>>> receive operation to make the packet go through another round of
> >>>> capturing and hit the macvtap rx_handler. I thought this would
> >>>> hurt performance too much for no real gain.
> >>>>
> >>>> Thanks
> >>>> -vlad
> >>>>
> >>>> drivers/net/macvtap.c | 4 +++-
> >>>> include/linux/netdevice.h | 2 ++
> >>>> net/core/dev.c | 33 ++++++++++++++++++++++++++-------
> >>>> 3 files changed, 31 insertions(+), 8 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> >>>> index dc76670..0ed8fae 100644
> >>>> --- a/drivers/net/macvtap.c
> >>>> +++ b/drivers/net/macvtap.c
> >>>> @@ -334,6 +334,7 @@ drop:
> >>>> */
> >>>> static int macvtap_receive(struct sk_buff *skb)
> >>>> {
> >>>> + netif_receive_skb_taps(skb, true);
> >>>> skb_push(skb, ETH_HLEN);
> >>>> return macvtap_forward(skb->dev, skb);
> >>>> }
> >>>> @@ -727,8 +728,9 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
> >>>> skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
> >>>> }
> >>>> if (vlan) {
> >>>> + skb->dev = vlan->dev;
> >>>> local_bh_disable();
> >>>> - macvlan_start_xmit(skb, vlan->dev);
> >>>> + dev_hard_start_xmit(skb, vlan->dev, NULL, NULL);
> >>>> local_bh_enable();
> >>>> } else {
> >>>> kfree_skb(skb);
> >>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> >>>> index 8b3de7c..84880eb 100644
> >>>> --- a/include/linux/netdevice.h
> >>>> +++ b/include/linux/netdevice.h
> >>>> @@ -2383,6 +2383,8 @@ void dev_kfree_skb_any(struct sk_buff *skb);
> >>>> int netif_rx(struct sk_buff *skb);
> >>>> int netif_rx_ni(struct sk_buff *skb);
> >>>> int netif_receive_skb(struct sk_buff *skb);
> >>>> +struct packet_type *netif_receive_skb_taps(struct sk_buff *skb,
> >>>> + bool last_deliver);
> >>>> gro_result_t napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb);
> >>>> void napi_gro_flush(struct napi_struct *napi, bool flush_old);
> >>>> struct sk_buff *napi_get_frags(struct napi_struct *napi);
> >>>> diff --git a/net/core/dev.c b/net/core/dev.c
> >>>> index da9c5e1..50f0ac4 100644
> >>>> --- a/net/core/dev.c
> >>>> +++ b/net/core/dev.c
> >>>> @@ -3484,6 +3484,31 @@ static bool skb_pfmemalloc_protocol(struct sk_buff *skb)
> >>>> }
> >>>> }
> >>>>
> >>>> +struct packet_type *netif_receive_skb_taps(struct sk_buff *skb,
> >>>> + bool last_deliver)
> >>>> +{
> >>>> + struct packet_type *ptype, *pt_prev = NULL;
> >>>> + struct net_device *orig_dev;
> >>>> +
> >>>> + if (list_empty(&ptype_all))
> >>>> + return NULL;
> >>>> +
> >>>> + orig_dev = skb->dev;
> >>>> + list_for_each_entry_rcu(ptype, &ptype_all, list) {
> >>>> + if (!ptype->dev || ptype->dev == skb->dev) {
> >>>> + if (pt_prev)
> >>>> + deliver_skb(skb, pt_prev, orig_dev);
> >>>> + pt_prev = ptype;
> >>>> + }
> >>>> + }
> >>>> +
> >>>> + if (last_deliver && pt_prev)
> >>>> + deliver_skb(skb, pt_prev, orig_dev);
> >>>> +
> >>>> + return pt_prev;
> >>>> +}
> >>>> +EXPORT_SYMBOL_GPL(netif_receive_skb_taps);
> >>>> +
> >>>> static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
> >>>> {
> >>>> struct packet_type *ptype, *pt_prev;
> >>>> @@ -3535,13 +3560,7 @@ another_round:
> >>>> if (pfmemalloc)
> >>>> goto skip_taps;
> >>>>
> >>>> - list_for_each_entry_rcu(ptype, &ptype_all, list) {
> >>>> - if (!ptype->dev || ptype->dev == skb->dev) {
> >>>> - if (pt_prev)
> >>>> - ret = deliver_skb(skb, pt_prev, orig_dev);
> >>>> - pt_prev = ptype;
> >>>> - }
> >>>> - }
> >>>> + pt_prev = netif_receive_skb_taps(skb, false);
> >>>>
> >>>> skip_taps:
> >>>> #ifdef CONFIG_NET_CLS_ACT
> >>>> --
> >>>> 1.8.4.2
^ permalink raw reply
* [PATCH 3/5] tun: tun.c calls skb_get_sw_rxhash
From: Tom Herbert @ 2013-11-20 20:27 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, hkchu
In tun.c, skb_get_rxhash is called on the transmit path with the
expectation that this matches the hash value returned by the same flow
in the receive path. If the hardware is providing the rxhash values,
the computation on the transmit path will not match that of the
received path for the flow, so the flow lookups will always fail
on the receive side and the queue selection optimization is unused.
The patch changes the calls to skb_get_sw_rxhash to ensure that the
stack's rxhash calculation is always used for flow matching. This
should always generate the same hash for a flow in both transmit and
receive paths.
Signed-off-by: Tom Herbert <therbert@google.com>
---
drivers/net/tun.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 7cb105c..c3e20c4 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -358,7 +358,7 @@ static u16 tun_select_queue(struct net_device *dev, struct sk_buff *skb)
rcu_read_lock();
numqueues = ACCESS_ONCE(tun->numqueues);
- txq = skb_get_rxhash(skb);
+ txq = skb_get_sw_rxhash(skb);
if (txq) {
e = tun_flow_find(&tun->flows[tun_hashfn(txq)], txq);
if (e)
@@ -1138,7 +1138,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
skb_reset_network_header(skb);
skb_probe_transport_header(skb, 0);
- rxhash = skb_get_rxhash(skb);
+ rxhash = skb_get_sw_rxhash(skb);
netif_rx_ni(skb);
tun->dev->stats.rx_packets++;
--
1.8.4.1
^ permalink raw reply related
* [PATCH 4/5] net: Allow setting sock flow hash without a sock
From: Tom Herbert @ 2013-11-20 20:27 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, hkchu
This patch adds sock_rps_record_flow_hash and sock_rps_reset_flow_hash
which take a hash value as an argument and sets the sock_flow_table
accordingly. This allows the table to be populated in cases where flow
is being tracked outside of a sock structure.
sock_rps_record_flow and sock_rps_reset_flow call this function
where the hash is taken from sk_rxhash.
Signed-off-by: Tom Herbert <therbert@google.com>
---
include/net/sock.h | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index e3a18ff..fe2410b 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -820,30 +820,40 @@ static inline int sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
return sk->sk_backlog_rcv(sk, skb);
}
-static inline void sock_rps_record_flow(const struct sock *sk)
+static inline void sock_rps_record_flow_hash(__u32 hash)
{
#ifdef CONFIG_RPS
struct rps_sock_flow_table *sock_flow_table;
rcu_read_lock();
sock_flow_table = rcu_dereference(rps_sock_flow_table);
- rps_record_sock_flow(sock_flow_table, sk->sk_rxhash);
+ rps_record_sock_flow(sock_flow_table, hash);
rcu_read_unlock();
#endif
}
-static inline void sock_rps_reset_flow(const struct sock *sk)
+static inline void sock_rps_reset_flow_hash(__u32 hash)
{
#ifdef CONFIG_RPS
struct rps_sock_flow_table *sock_flow_table;
rcu_read_lock();
sock_flow_table = rcu_dereference(rps_sock_flow_table);
- rps_reset_sock_flow(sock_flow_table, sk->sk_rxhash);
+ rps_reset_sock_flow(sock_flow_table, hash);
rcu_read_unlock();
#endif
}
+static inline void sock_rps_record_flow(const struct sock *sk)
+{
+ sock_rps_record_flow_hash(sk->sk_rxhash);
+}
+
+static inline void sock_rps_reset_flow(const struct sock *sk)
+{
+ sock_rps_reset_flow_hash(sk->sk_rxhash);
+}
+
static inline void sock_rps_save_rxhash(struct sock *sk,
const struct sk_buff *skb)
{
--
1.8.4.1
^ permalink raw reply related
* [PATCH 5/5] tun: Added support for RFS on tun flows
From: Tom Herbert @ 2013-11-20 20:27 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, hkchu
This patch adds support so that the rps_flow_tables (RFS) can be
programmed using the tun flows which are already set up to track flows
for the purposes of queue selection.
On the receive path (corresponding to select_queue and tun_net_xmit) the
rxhash is saved in the flow_entry. The original code only does flow
lookup in select_queue, so this patch adds a flow lookup in tun_net_xmit
if num_queues == 1 (select_queue is not called from
dev_queue_xmit->netdev_pick_tx in that case).
The flow is recorded (processing CPU) in tun_flow_update (TX path), and
reset when flow is deleted.
Signed-off-by: Tom Herbert <therbert@google.com>
---
drivers/net/tun.c | 47 ++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 44 insertions(+), 3 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index c3e20c4..81ba181 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -152,6 +152,7 @@ struct tun_flow_entry {
struct tun_struct *tun;
u32 rxhash;
+ u32 rps_rxhash;
int queue_index;
unsigned long updated;
};
@@ -220,6 +221,7 @@ static struct tun_flow_entry *tun_flow_create(struct tun_struct *tun,
rxhash, queue_index);
e->updated = jiffies;
e->rxhash = rxhash;
+ e->rps_rxhash = 0;
e->queue_index = queue_index;
e->tun = tun;
hlist_add_head_rcu(&e->hash_link, head);
@@ -232,6 +234,7 @@ static void tun_flow_delete(struct tun_struct *tun, struct tun_flow_entry *e)
{
tun_debug(KERN_INFO, tun, "delete flow: hash %u index %u\n",
e->rxhash, e->queue_index);
+ sock_rps_reset_flow_hash(e->rps_rxhash);
hlist_del_rcu(&e->hash_link);
kfree_rcu(e, rcu);
--tun->flow_count;
@@ -325,6 +328,7 @@ static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
/* TODO: keep queueing to old queue until it's empty? */
e->queue_index = queue_index;
e->updated = jiffies;
+ sock_rps_record_flow_hash(e->rps_rxhash);
} else {
spin_lock_bh(&tun->lock);
if (!tun_flow_find(head, rxhash) &&
@@ -341,6 +345,18 @@ unlock:
rcu_read_unlock();
}
+/*
+ * Save the hash received in the stack receive path and update the
+ * flow_hash table accordingly.
+ */
+static inline void tun_flow_save_rps_rxhash(struct tun_flow_entry *e, u32 hash)
+{
+ if (unlikely(e->rps_rxhash != hash)) {
+ sock_rps_reset_flow_hash(e->rps_rxhash);
+ e->rps_rxhash = hash;
+ }
+}
+
/* We try to identify a flow through its rxhash first. The reason that
* we do not check rxq no. is becuase some cards(e.g 82599), chooses
* the rxq based on the txq where the last packet of the flow comes. As
@@ -352,18 +368,26 @@ static u16 tun_select_queue(struct net_device *dev, struct sk_buff *skb)
{
struct tun_struct *tun = netdev_priv(dev);
struct tun_flow_entry *e;
- u32 txq = 0;
+ u32 txq = 0, rps_rxhash;
u32 numqueues = 0;
rcu_read_lock();
numqueues = ACCESS_ONCE(tun->numqueues);
+ /*
+ * Save the original rxhash from the skb. This may be a hash provided
+ * by the hardware. This should be the one used to program the rps_flow
+ * table.
+ */
+ rps_rxhash = skb_get_rxhash(skb);
+
txq = skb_get_sw_rxhash(skb);
if (txq) {
e = tun_flow_find(&tun->flows[tun_hashfn(txq)], txq);
- if (e)
+ if (e) {
txq = e->queue_index;
- else
+ tun_flow_save_rps_rxhash(e, rps_rxhash);
+ } else
/* use multiply and shift instead of expensive divide */
txq = ((u64)txq * numqueues) >> 32;
} else if (likely(skb_rx_queue_recorded(skb))) {
@@ -728,6 +752,23 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
if (txq >= tun->numqueues)
goto drop;
+ if (tun->numqueues == 1) {
+ /*
+ * Select queue was not called for the skbuff, so we extract the
+ * RPS hash and save it into the flow_table here.
+ */
+ __u32 rps_rxhash, rxhash;
+
+ rps_rxhash = skb_get_rxhash(skb);
+ rxhash = skb_get_sw_rxhash(skb);
+ if (rxhash) {
+ struct tun_flow_entry *e;
+ e = tun_flow_find(&tun->flows[tun_hashfn(rxhash)], rxhash);
+ if (e)
+ tun_flow_save_rps_rxhash(e, rps_rxhash);
+ }
+ }
+
tun_debug(KERN_INFO, tun, "tun_net_xmit %d\n", skb->len);
BUG_ON(!tfile);
--
1.8.4.1
^ permalink raw reply related
* Re: [PATCH net] ipv4: fix race in concurrent ip_route_input_slow()
From: David Miller @ 2013-11-20 20:29 UTC (permalink / raw)
To: ast; +Cc: edumazet, netdev
In-Reply-To: <1384917154-11049-1-git-send-email-ast@plumgrid.com>
From: Alexei Starovoitov <ast@plumgrid.com>
Date: Tue, 19 Nov 2013 19:12:34 -0800
> CPUs can ask for local route via ip_route_input_noref() concurrently.
> if nh_rth_input is not cached yet, CPUs will proceed to allocate
> equivalent DSTs on 'lo' and then will try to cache them in nh_rth_input
> via rt_cache_route()
> Most of the time they succeed, but on occasion the following two lines:
> orig = *p;
> prev = cmpxchg(p, orig, rt);
> in rt_cache_route() do race and one of the cpus fails to complete cmpxchg.
> But ip_route_input_slow() doesn't check the return code of rt_cache_route(),
> so dst is leaking. dst_destroy() is never called and 'lo' device
> refcnt doesn't go to zero, which can be seen in the logs as:
> unregister_netdevice: waiting for lo to become free. Usage count = 1
> Adding mdelay() between above two lines makes it easily reproducible.
> Fix it similar to nh_pcpu_rth_output case.
>
> Fixes: d2d68ba9fe8b ("ipv4: Cache input routes in fib_info nexthops.")
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
> ---
>
> David,
>
> looks like caacf05e5ad1 ("ipv4: Properly purge netdev references on uncached routes.")
> fixed the race for nexthop/rth_output, but missed it for rth_input.
> I'm not sure what was the assumption why it's not needed there.
> We're definitely seeing it every 12-24hr during nightly tests.
> There are several bugs on ubuntu and debian forums with similar description.
> Some were closed, since folks struggled to reproduce it.
> It took us more than a month to debug it.
> Please queue for stable.
Your analysis is accurate and your fix is absolutely correct, applied
and queued up for -stable, thanks!
^ permalink raw reply
* Re: [PATCHv3] net: core: Always propagate flag changes to interfaces
From: David Miller @ 2013-11-20 20:30 UTC (permalink / raw)
To: vyasevic; +Cc: netdev, s.priebe, vfalico
In-Reply-To: <1384912035-17800-1-git-send-email-vyasevic@redhat.com>
From: Vlad Yasevich <vyasevic@redhat.com>
Date: Tue, 19 Nov 2013 20:47:15 -0500
> The following commit:
> b6c40d68ff6498b7f63ddf97cf0aa818d748dee7
> net: only invoke dev->change_rx_flags when device is UP
>
> tried to fix a problem with VLAN devices and promiscuouse flag setting.
> The issue was that VLAN device was setting a flag on an interface that
> was down, thus resulting in bad promiscuity count.
> This commit blocked flag propagation to any device that is currently
> down.
>
> A later commit:
> deede2fabe24e00bd7e246eb81cd5767dc6fcfc7
> vlan: Don't propagate flag changes on down interfaces
>
> fixed VLAN code to only propagate flags when the VLAN interface is up,
> thus fixing the same issue as above, only localized to VLAN.
>
> The problem we have now is that if we have create a complex stack
> involving multiple software devices like bridges, bonds, and vlans,
> then it is possible that the flags would not propagate properly to
> the physical devices. A simple examle of the scenario is the
> following:
>
> eth0----> bond0 ----> bridge0 ---> vlan50
>
> If bond0 or eth0 happen to be down at the time bond0 is added to
> the bridge, then eth0 will never have promisc mode set which is
> currently required for operation as part of the bridge. As a
> result, packets with vlan50 will be dropped by the interface.
>
> The only 2 devices that implement the special flag handling are
> VLAN and DSA and they both have required code to prevent incorrect
> flag propagation. As a result we can remove the generic solution
> introduced in b6c40d68ff6498b7f63ddf97cf0aa818d748dee7 and leave
> it to the individual devices to decide whether they will block
> flag propagation or not.
>
> Reported-by: Stefan Priebe <s.priebe@profihost.ag>
> Suggested-by: Veaceslav Falico <vfalico@redhat.com>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
Applied and queued up for -stable, thanks Vlad.
^ permalink raw reply
* [PATCH 1/5] net: Add utility function to clear rxhash
From: Tom Herbert @ 2013-11-20 20:26 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, hkchu
In several places 'skb->rxhash = 0' is being done to clear the
rxhash value in an skb. This does not clear l4_rxhash which could
still be set so that the rxhash wouldn't be recalculated on subsequent
call to skb_get_rxhash. This patch adds an explict function to clear
all the rxhash related information in the skb properly.
Signed-off-by: Tom Herbert <therbert@google.com>
---
include/linux/skbuff.h | 6 ++++++
net/ipv4/ip_fragment.c | 2 +-
net/ipv4/ip_tunnel_core.c | 4 ++--
net/openvswitch/actions.c | 10 +++++-----
4 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 215b5ea..76d3aa9 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -712,6 +712,12 @@ static inline __u32 skb_get_rxhash(struct sk_buff *skb)
return skb->rxhash;
}
+static inline void skb_clear_rxhash(struct sk_buff *skb)
+{
+ skb->rxhash = 0;
+ skb->l4_rxhash = 0;
+}
+
#ifdef NET_SKBUFF_DATA_USES_OFFSET
static inline unsigned char *skb_end_pointer(const struct sk_buff *skb)
{
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 2481993..3bc1d0d 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -704,7 +704,7 @@ struct sk_buff *ip_check_defrag(struct sk_buff *skb, u32 user)
memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));
if (ip_defrag(skb, user))
return NULL;
- skb->rxhash = 0;
+ skb_clear_rxhash(skb);
}
}
return skb;
diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index 42ffbc8..2148c1d 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -56,7 +56,7 @@ int iptunnel_xmit(struct rtable *rt, struct sk_buff *skb,
skb_scrub_packet(skb, xnet);
- skb->rxhash = 0;
+ skb_clear_rxhash(skb);
skb_dst_set(skb, &rt->dst);
memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
@@ -108,7 +108,7 @@ int iptunnel_pull_header(struct sk_buff *skb, int hdr_len, __be16 inner_proto)
nf_reset(skb);
secpath_reset(skb);
if (!skb->l4_rxhash)
- skb->rxhash = 0;
+ skb_clear_rxhash(skb);
skb_dst_drop(skb);
skb->vlan_tci = 0;
skb_set_queue_mapping(skb, 0);
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 65cfaa8..ab0d925 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -165,7 +165,7 @@ static void set_ip_addr(struct sk_buff *skb, struct iphdr *nh,
}
csum_replace4(&nh->check, *addr, new_addr);
- skb->rxhash = 0;
+ skb_clear_rxhash(skb);
*addr = new_addr;
}
@@ -199,7 +199,7 @@ static void set_ipv6_addr(struct sk_buff *skb, u8 l4_proto,
if (recalculate_csum)
update_ipv6_checksum(skb, l4_proto, addr, new_addr);
- skb->rxhash = 0;
+ skb_clear_rxhash(skb);
memcpy(addr, new_addr, sizeof(__be32[4]));
}
@@ -296,7 +296,7 @@ static void set_tp_port(struct sk_buff *skb, __be16 *port,
{
inet_proto_csum_replace2(check, skb, *port, new_port, 0);
*port = new_port;
- skb->rxhash = 0;
+ skb_clear_rxhash(skb);
}
static void set_udp_port(struct sk_buff *skb, __be16 *port, __be16 new_port)
@@ -310,7 +310,7 @@ static void set_udp_port(struct sk_buff *skb, __be16 *port, __be16 new_port)
uh->check = CSUM_MANGLED_0;
} else {
*port = new_port;
- skb->rxhash = 0;
+ skb_clear_rxhash(skb);
}
}
@@ -381,7 +381,7 @@ static int set_sctp(struct sk_buff *skb,
/* Carry any checksum errors through. */
sh->checksum = old_csum ^ old_correct_csum ^ new_csum;
- skb->rxhash = 0;
+ skb_clear_rxhash(skb);
}
return 0;
--
1.8.4.1
^ permalink raw reply related
* [PATCH 2/5] net: Add flag for SW computed rxhash
From: Tom Herbert @ 2013-11-20 20:26 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, hkchu
This patch adds sw_rxhash flag bit to the skbuff. It is set in
flow_dissecor.c to indicate that the rxhash was computed in the stack
as opposed to being provided by hardware. This flag is checked in
skb_get_rxhash so don't repeatedly do full flow dissection every
time for a packet for which we can't extract an L4 hash. The
skb_get_sw_rxhash function was also added to explictly force that
the rxhash is computed by the stack.
Signed-off-by: Tom Herbert <therbert@google.com>
---
include/linux/skbuff.h | 14 ++++++++++++--
net/core/flow_dissector.c | 1 +
net/core/skbuff.c | 1 +
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 76d3aa9..3c8ea3c 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -379,6 +379,7 @@ typedef unsigned char *sk_buff_data_t;
* @ooo_okay: allow the mapping of a socket to a queue to be changed
* @l4_rxhash: indicate rxhash is a canonical 4-tuple hash over transport
* ports.
+ * @sw_rxhash: indicate rxhash was computed in software (flow_dissector).
* @wifi_acked_valid: wifi_acked was set
* @wifi_acked: whether frame was acked on wifi or not
* @no_fcs: Request NIC to treat last 4 bytes as Ethernet FCS
@@ -483,6 +484,7 @@ struct sk_buff {
__u8 pfmemalloc:1;
__u8 ooo_okay:1;
__u8 l4_rxhash:1;
+ __u8 sw_rxhash:1;
__u8 wifi_acked_valid:1;
__u8 wifi_acked:1;
__u8 no_fcs:1;
@@ -493,7 +495,7 @@ struct sk_buff {
* headers if needed
*/
__u8 encapsulation:1;
- /* 6/8 bit hole (depending on ndisc_nodetype presence) */
+ /* 5/7 bit hole (depending on ndisc_nodetype presence) */
kmemcheck_bitfield_end(flags2);
#if defined CONFIG_NET_DMA || defined CONFIG_NET_RX_BUSY_POLL
@@ -706,7 +708,15 @@ unsigned int skb_find_text(struct sk_buff *skb, unsigned int from,
void __skb_get_rxhash(struct sk_buff *skb);
static inline __u32 skb_get_rxhash(struct sk_buff *skb)
{
- if (!skb->l4_rxhash)
+ if (!skb->l4_rxhash && !skb->sw_rxhash)
+ __skb_get_rxhash(skb);
+
+ return skb->rxhash;
+}
+
+static inline __u32 skb_get_sw_rxhash(struct sk_buff *skb)
+{
+ if (!skb->sw_rxhash)
__skb_get_rxhash(skb);
return skb->rxhash;
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index d6ef173..6a4bee4 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -233,6 +233,7 @@ void __skb_get_rxhash(struct sk_buff *skb)
hash = 1;
skb->rxhash = hash;
+ skb->sw_rxhash = 1;
}
EXPORT_SYMBOL(__skb_get_rxhash);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 8cec1e6..a16f486 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -715,6 +715,7 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
new->rxhash = old->rxhash;
new->ooo_okay = old->ooo_okay;
new->l4_rxhash = old->l4_rxhash;
+ new->sw_rxhash = old->sw_rxhash;
new->no_fcs = old->no_fcs;
new->encapsulation = old->encapsulation;
#ifdef CONFIG_XFRM
--
1.8.4.1
^ permalink raw reply related
* Re: [PATCH net v2] bridge: flush br's address entry in fdb when remove the bridge dev
From: David Miller @ 2013-11-20 20:32 UTC (permalink / raw)
To: dingtianhong; +Cc: stephen, netdev, toshiaki.makita1, bridge, vyasevich
In-Reply-To: <528C172A.3090004@huawei.com>
From: Ding Tianhong <dingtianhong@huawei.com>
Date: Wed, 20 Nov 2013 09:58:02 +0800
> From 241ddeaad73c7ed9e376a4b0926e79bdc3df9f58 Mon Sep 17 00:00:00 2001
> From: Ding Tianhong <dingtianhong@huawei.com>
> Date: Sat, 7 Dec 2013 22:12:05 +0800
> Subject: [PATCH] bridge: flush br's address entry in fdb when remove the
> bridge dev
>
> When the following commands are executed:
>
> brctl addbr br0
> ifconfig br0 hw ether <addr>
> rmmod bridge
>
> The calltrace will occur:
...
> The reason is that when the bridge dev's address is changed, the
> br_fdb_change_mac_address() will add new address in fdb, but when
> the bridge was removed, the address entry in the fdb did not free,
> the bridge_fdb_cache still has objects when destroy the cache, Fix
> this by flushing the bridge address entry when removing the bridge.
>
> v2: according to the Toshiaki Makita and Vlad's suggestion, I only
> delete the vlan0 entry, it still have a leak here if the vlan id
> is other number, so I need to call fdb_delete_by_port(br, NULL, 1)
> to flush all entries whose dst is NULL for the bridge.
>
> Suggested-by: Toshiaki Makita <toshiaki.makita1@gmail.com>
> Suggested-by: Vlad Yasevich <vyasevich@gmail.com>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Get rxhash fixes and RFS support in tun
From: Tom Herbert @ 2013-11-20 20:25 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, hkchu
This patch series fixes some subtle bugs in tun use of skb->rxhash, all
rxhash hash not be cleared appropraitely, and adds support for tun flows
to work with RFS.
Testing, in particular with tun, hasn't been completed yet.
^ permalink raw reply
* Re: [PATCH RFC v1 0/7] net: phy: Ethernet PHY powerdown optimization
From: David Miller @ 2013-11-20 20:36 UTC (permalink / raw)
To: sebastian.hesselbarth; +Cc: netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <1384978913-8052-1-git-send-email-sebastian.hesselbarth@gmail.com>
From: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Date: Wed, 20 Nov 2013 21:21:46 +0100
> Ethernet PHYs consume a significant amount of power when link is detected.
> Especially, for embedded systems it can be easily 20-40% of total system
> power. Now, currently most likely all ethernet drivers leave PHYs powered
> on, even if the device is taken down. Also, some stupid boot loaders power
> on all PHYs available.
>
> This RFC deals with saving power consumed by ethernet PHYs, that have no
> corresponding ethernet device driver or are attached to ethernet devices
> which are taken down by user request, i.e. ifconfig ethN down. Ports with
> no link, i.e. cable removed, are already quite good at power saving due to
> PHY internal link detection.
The idea is sound and the goal is of course valuable, but it brings up
a chronically reoccurring issue as of late.
You cannot reset the PHY or take it down without somehow retaining the
settings the PHY had when you bring it back up.
If I ifdown/ifup a device, my ethtool link configuration better be
retained.
This means the PHY layer must have a way to reprogram the device when
it is brought back up, with whatever settings the software state
things are there.
^ permalink raw reply
* Re: [PATCH RFC v1 2/7] net: phy: provide phy_resume/phy_suspend helpers
From: Florian Fainelli @ 2013-11-20 20:36 UTC (permalink / raw)
To: Sebastian Hesselbarth
Cc: David S. Miller, netdev, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1384978913-8052-3-git-send-email-sebastian.hesselbarth@gmail.com>
2013/11/20 Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>:
> This adds helper functions to resume and suspend a given phy_device
> by calling the corresponding driver callbacks if available.
>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Looks good, thanks Sebastian:
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> Cc: David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/net/phy/phy_device.c | 19 +++++++++++++++++++
> include/linux/phy.h | 2 ++
> 2 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 74630e9..2442895 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -625,6 +625,25 @@ void phy_detach(struct phy_device *phydev)
> }
> EXPORT_SYMBOL(phy_detach);
>
> +int phy_suspend(struct phy_device *phydev)
> +{
> + struct phy_driver *phydrv = to_phy_driver(phydev->dev.driver);
> +
> + if (phydrv->suspend)
> + return phydrv->suspend(phydev);
> + return 0;
> +}
> +EXPORT_SYMBOL(phy_suspend);
> +
> +int phy_resume(struct phy_device *phydev)
> +{
> + struct phy_driver *phydrv = to_phy_driver(phydev->dev.driver);
> +
> + if (phydrv->resume)
> + return phydrv->resume(phydev);
> + return 0;
> +}
> +EXPORT_SYMBOL(phy_resume);
>
> /* Generic PHY support and helper functions */
>
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 64ab823..ed0d6d8 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -552,6 +552,8 @@ void phy_detach(struct phy_device *phydev);
> void phy_start(struct phy_device *phydev);
> void phy_stop(struct phy_device *phydev);
> int phy_start_aneg(struct phy_device *phydev);
> +int phy_suspend(struct phy_device *phydev);
> +int phy_resume(struct phy_device *phydev);
>
> int phy_stop_interrupts(struct phy_device *phydev);
>
> --
> 1.7.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Florian
^ permalink raw reply
* Re: Fw: [Bug 65271] New: Acceptable RST packets (with seq number in window) are ignored in LAST_ACK state (and previously sent data pending acknowledgement)
From: Eric Dumazet @ 2013-11-20 20:41 UTC (permalink / raw)
To: Stephen Hemminger, julien.delacroix; +Cc: netdev
In-Reply-To: <20131120122211.386c9680@nehalam.linuxnetplumber.net>
On Wed, 2013-11-20 at 12:22 -0800, Stephen Hemminger wrote:
>
> Begin forwarded message:
>
> Date: Wed, 20 Nov 2013 07:03:59 -0800
> From: "bugzilla-daemon@bugzilla.kernel.org" <bugzilla-daemon@bugzilla.kernel.org>
> To: "stephen@networkplumber.org" <stephen@networkplumber.org>
> Subject: [Bug 65271] New: Acceptable RST packets (with seq number in window) are ignored in LAST_ACK state (and previously sent data pending acknowledgement)
>
>
> https://bugzilla.kernel.org/show_bug.cgi?id=65271
>
> Bug ID: 65271
> Summary: Acceptable RST packets (with seq number in window) are
> ignored in LAST_ACK state (and previously sent data
> pending acknowledgement)
> Product: Networking
> Version: 2.5
> Kernel Version: 2.6.32.59-0.7-default
> Hardware: All
> OS: Linux
> Tree: Mainline
> Status: NEW
> Severity: normal
> Priority: P1
> Component: IPV4
> Assignee: shemminger@linux-foundation.org
> Reporter: julien.delacroix@gmail.com
> Regression: No
>
> Created attachment 115271
> --> https://bugzilla.kernel.org/attachment.cgi?id=115271&action=edit
> TCP dump showing described scenario
>
> PCAP file of traffic capture is attached.
> Note that capture was taken directly on server host, so RST packets definitely
> reached the server host.
>
>
> Here is the flow:
>
> 1: Client ---- SYN ----> Server
> 2: Client <- SYN, ACK -- Server
> 3: Client ---- ACK ----> Server
> 4: Client ---- FIN ----> Server ( server in CLOSE_WAIT state)
> 5: Client <- PSH, ACK -- Server
> 6: Client <- FIN, ACK -- Server ( server in LAST_ACK state)
>
> 7: Client ---- RST ----> Server
>
> Then repeat N times:
> 8: Client <- PSH, ACK -- Server
> 9: Client ---RST, ACK -> Server
>
>
> Notes:
> 5: packet contains 3 bytes of data
> 7: no ACK flag, and sequence number is correct, but RST packet is ignored
> 8: retransmission of step 5
> 9: the sequence number is correct as well as the acknowledgment number
> (acknowledging data in 5, but not yet FIN in 6), but this RST is again ignored.
>
> From this point client keeps retransmitting 5, while server keeps answering
> with acceptable RST packets.
>
> You can observe this scenario (with 5 retransmissions and 5 RST) in the
> attached PCAP file.
>
Thanks for the report, you'll take a look.
^ permalink raw reply
* Re: ipv6 fragmentation-related panic in netfilter
From: David Miller @ 2013-11-20 20:43 UTC (permalink / raw)
To: linux; +Cc: hannes, kaber, steffen.klassert, tmshlvck, netdev,
netfilter-devel
In-Reply-To: <3998760.5g9L5dp08X@h2o.as.studentenwerk.mhn.de>
From: Wolfgang Walter <linux@stwm.de>
Date: Tue, 19 Nov 2013 23:27:40 +0100
> Am Dienstag, 19. November 2013, 13:40:32 schrieb Hannes Frederic Sowa:
>> Yes, see here:
>>
>> http://patchwork.ozlabs.org/patch/288967/
>> http://patchwork.ozlabs.org/patch/288970/
>>
>
> This fixed it. The second patch did not cleanly apply to 3.12 due to
> formatting changes or because some functions in 3.12 get
>
> unsigned int hooknum
>
> instead of
>
> const struct nf_hook_ops *ops
>
> I hopefully got it right:at least it works and no crashes any more :-).
>
> I hope both patches go into stable soon.
They are both queued up.
^ permalink raw reply
* Re: [PATCH RFC v1 7/7] net: cpsw: resume/suspend PHY on port start/stop
From: Florian Fainelli @ 2013-11-20 20:48 UTC (permalink / raw)
To: Sebastian Hesselbarth
Cc: David S. Miller, netdev, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1384978913-8052-8-git-send-email-sebastian.hesselbarth@gmail.com>
2013/11/20 Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>:
> Network PHYs consume a noticable amount of power. This adds phy_resume
> on slave_open and phy_suspend on slave_stop to save this power if the
> port is down anyway.
>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> ---
> Cc: David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/net/ethernet/ti/cpsw.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 90d41d2..f1dc54f 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -1013,6 +1013,7 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
> } else {
> dev_info(priv->dev, "phy found : id is : 0x%x\n",
> slave->phy->phy_id);
> + phy_resume(slave->phy);
> phy_start(slave->phy);
Cannot phy_start() figure this out for us based on the internal PHY
device state machine?
I would imagine that you would want to call phy_suspend/resume from an
Ethernet driver's PM suspend/resume callbacks to make sure that the
PHY also enters a low power mode, but I do not want to have to
remember that I need to call phy_resume before phy_start for instance.
As of today we have PHY_HALTED/PHY_RESUMING, and I think we certainly
need at least a PHY_SUSPENDED state to help us with that.
>
> /* Configure GMII_SEL register */
> @@ -1081,6 +1082,7 @@ static void cpsw_slave_stop(struct cpsw_slave *slave, struct cpsw_priv *priv)
> if (!slave->phy)
> return;
> phy_stop(slave->phy);
> + phy_suspend(slave->phy);
Same here, why is not that hidden in phy_stop? If there is any fear
that this breaks setups where WoL is used or such, we could add a new
argument to phy_connect() and friends which says whether it is okay to
auto-suspend the PHY upon PHY_stop
> phy_disconnect(slave->phy);
> slave->phy = NULL;
> }
> --
> 1.7.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Florian
^ permalink raw reply
* Re: [PATCH RFC v1 0/7] net: phy: Ethernet PHY powerdown optimization
From: Sebastian Hesselbarth @ 2013-11-20 20:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <20131120.153642.2045230991917379052.davem@davemloft.net>
On 11/20/2013 09:36 PM, David Miller wrote:
> From: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Date: Wed, 20 Nov 2013 21:21:46 +0100
>
>> Ethernet PHYs consume a significant amount of power when link is detected.
>> Especially, for embedded systems it can be easily 20-40% of total system
>> power. Now, currently most likely all ethernet drivers leave PHYs powered
>> on, even if the device is taken down. Also, some stupid boot loaders power
>> on all PHYs available.
>>
>> This RFC deals with saving power consumed by ethernet PHYs, that have no
>> corresponding ethernet device driver or are attached to ethernet devices
>> which are taken down by user request, i.e. ifconfig ethN down. Ports with
>> no link, i.e. cable removed, are already quite good at power saving due to
>> PHY internal link detection.
>
> The idea is sound and the goal is of course valuable, but it brings up
> a chronically reoccurring issue as of late.
>
> You cannot reset the PHY or take it down without somehow retaining the
> settings the PHY had when you bring it back up.
Right, as far as I understand BMCR powerdown, i.e. what is called in
genphy_suspend/resume, powers down the PHY but _does_ retain PHY config.
It is not resetting the device.
I haven't checked a lot of datasheets but [1] notes that "registers will
preserve their configuration". Even if we have PHYs that do not preserve
it, they should have a device specific callback for suspend/resume that
takes care of preserving it.
[1] http://www.ti.com/lit/an/snoa463a/snoa463a.pdf
> If I ifdown/ifup a device, my ethtool link configuration better be
> retained.
>
> This means the PHY layer must have a way to reprogram the device when
> it is brought back up, with whatever settings the software state
> things are there.
>
^ permalink raw reply
* Re: [PATCH RFC v1 7/7] net: cpsw: resume/suspend PHY on port start/stop
From: Sebastian Hesselbarth @ 2013-11-20 20:57 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CAGVrzcZ+5oaucp+OSeGGe=6MBjEwi4K=E_NbCm3PjR_Owyxstw@mail.gmail.com>
On 11/20/2013 09:48 PM, Florian Fainelli wrote:
> 2013/11/20 Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>:
>> Network PHYs consume a noticable amount of power. This adds phy_resume
>> on slave_open and phy_suspend on slave_stop to save this power if the
>> port is down anyway.
>>
>> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
>> ---
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: netdev@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> ---
>> drivers/net/ethernet/ti/cpsw.c | 2 ++
>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
>> index 90d41d2..f1dc54f 100644
>> --- a/drivers/net/ethernet/ti/cpsw.c
>> +++ b/drivers/net/ethernet/ti/cpsw.c
>> @@ -1013,6 +1013,7 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
>> } else {
>> dev_info(priv->dev, "phy found : id is : 0x%x\n",
>> slave->phy->phy_id);
>> + phy_resume(slave->phy);
>> phy_start(slave->phy);
>
> Cannot phy_start() figure this out for us based on the internal PHY
> device state machine?
Yeah, as I said in the cover letter, I started with mv643xx_eth which
is not using phy_start/stop as it probably should be. As soon as I
looked at mvneta/cpsw, I also came to the conclusion that
phy_suspend/resume should be hidden in phy_start/stop.
> I would imagine that you would want to call phy_suspend/resume from an
> Ethernet driver's PM suspend/resume callbacks to make sure that the
> PHY also enters a low power mode, but I do not want to have to
> remember that I need to call phy_resume before phy_start for instance.
> As of today we have PHY_HALTED/PHY_RESUMING, and I think we certainly
> need at least a PHY_SUSPENDED state to help us with that.
>
>>
>> /* Configure GMII_SEL register */
>> @@ -1081,6 +1082,7 @@ static void cpsw_slave_stop(struct cpsw_slave *slave, struct cpsw_priv *priv)
>> if (!slave->phy)
>> return;
>> phy_stop(slave->phy);
>> + phy_suspend(slave->phy);
>
> Same here, why is not that hidden in phy_stop? If there is any fear
> that this breaks setups where WoL is used or such, we could add a new
> argument to phy_connect() and friends which says whether it is okay to
> auto-suspend the PHY upon PHY_stop
>
>> phy_disconnect(slave->phy);
>> slave->phy = NULL;
>> }
>> --
>> 1.7.2.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>
>
>
^ permalink raw reply
* Re: [PATCH RFC v1 4/7] net: phy: suspend unused PHYs on mdio_bus in late_initcall
From: Florian Fainelli @ 2013-11-20 20:58 UTC (permalink / raw)
To: Sebastian Hesselbarth
Cc: netdev, linux-kernel@vger.kernel.org, David S. Miller,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1384978913-8052-5-git-send-email-sebastian.hesselbarth@gmail.com>
2013/11/20 Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>:
> Since phy_attach ensures PHYs are resumed, we can now suspend all
> PHYs that have no attached netdev after initcalls.
I do like the idea, but I think you might want to make sure that the
MDIO bus suspend policy was set to "auto" (which is the default afair)
not to expose unexpected behavior.
>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> ---
> Cc: David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/net/phy/mdio_bus.c | 27 +++++++++++++++++++++++++++
> 1 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 5617876..10eba58 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -320,6 +320,33 @@ static int mdio_bus_match(struct device *dev, struct device_driver *drv)
> (phydev->phy_id & phydrv->phy_id_mask));
> }
>
> +static int mdio_bus_suspend_unused(struct device *busdev, void *data)
> +{
> + struct mii_bus *bus = to_mii_bus(busdev);
> + struct phy_device *phydev;
> + struct phy_driver *phydrv;
> + int i;
> +
> + for (i = 0; i < PHY_MAX_ADDR; i++) {
> + if (!bus->phy_map[i])
> + continue;
> +
> + phydev = to_phy_device(&bus->phy_map[i]->dev);
> + phydrv = to_phy_driver(phydev->dev.driver);
> + if (!phydev->attached_dev && phydrv && phydrv->suspend)
> + phy_suspend(phydev);
> + }
> +
> + return 0;
You might want to reuse mdio_bus_phy_may_suspend() here to have a
central place checking for phydev->attached_dev and phydrv->suspend
just in case we need to add more callbacks in the future or implicit
PHY state machine hooks. That might also take care of my concern
expressed above.
> +}
> +
> +static int mdio_bus_class_suspend_unused(void)
> +{
> + return class_for_each_device(&mdio_bus_class, NULL, NULL,
> + mdio_bus_suspend_unused);
> +}
> +late_initcall_sync(mdio_bus_class_suspend_unused);
> +
> #ifdef CONFIG_PM
>
> static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
> --
> 1.7.2.5
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Florian
^ 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