* [PATCH 0/2] net: qcom/emac: fixes for pause frame floods
@ 2017-08-01 21:37 Timur Tabi
2017-08-01 21:37 ` [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default Timur Tabi
2017-08-01 21:37 ` [PATCH 2/2] net: qcom/emac: add software control for pause frame mode Timur Tabi
0 siblings, 2 replies; 26+ messages in thread
From: Timur Tabi @ 2017-08-01 21:37 UTC (permalink / raw)
To: David S. Miller, netdev; +Cc: timur
The first patch is for 4.13. It's changes the default behavior of the
EMAC driver so that it doesn't send pause frames unless the user
enables them.
The second patch is for 4.14, but it can be applied to 4.13 if you
want. It adds the ability for the user to enable a special "single
pause frame" mode that could be useful in some situations.
Timur Tabi (2):
[for 4.13] net: qcom/emac: disable flow control autonegotiation by
default
net: qcom/emac: add software control for pause frame mode
drivers/net/ethernet/qualcomm/emac/emac-ethtool.c | 30 +++++++++++++++++++++++
drivers/net/ethernet/qualcomm/emac/emac-mac.c | 22 +++++++++++++++++
drivers/net/ethernet/qualcomm/emac/emac.c | 12 +++++++--
drivers/net/ethernet/qualcomm/emac/emac.h | 3 +++
4 files changed, 65 insertions(+), 2 deletions(-)
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-01 21:37 [PATCH 0/2] net: qcom/emac: fixes for pause frame floods Timur Tabi @ 2017-08-01 21:37 ` Timur Tabi 2017-08-01 21:55 ` Florian Fainelli ` (3 more replies) 2017-08-01 21:37 ` [PATCH 2/2] net: qcom/emac: add software control for pause frame mode Timur Tabi 1 sibling, 4 replies; 26+ messages in thread From: Timur Tabi @ 2017-08-01 21:37 UTC (permalink / raw) To: David S. Miller, netdev; +Cc: timur The EMAC has a curious qwirk when RX flow control is enabled and the kernel hangs. With the kernel hung, the EMAC's RX queue soon fills. If RX flow control is enabled, the EMAC will then send a non-stop stream of pause frames until the system is reset. The EMAC does not have a built-in watchdog. In various tests, the pause frame stream sometimes overloads nearby switches, effectively disabling the network. Since the RX queue is large and the host processor is more than capable of handling incoming packets quickly, the only time the EMAC will send any pause frames is when the kernel is hung and unrecoverable. To avoid all these problems, we disable flow control autonegotiation by default, and only enable receiving pause frames. Cc: stable@vger.kernel.org # 4.11.x Signed-off-by: Timur Tabi <timur@codeaurora.org> --- drivers/net/ethernet/qualcomm/emac/emac.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c index 60850bfa3d32..475c0ea29235 100644 --- a/drivers/net/ethernet/qualcomm/emac/emac.c +++ b/drivers/net/ethernet/qualcomm/emac/emac.c @@ -441,8 +441,13 @@ static void emac_init_adapter(struct emac_adapter *adpt) /* others */ adpt->preamble = EMAC_PREAMBLE_DEF; - /* default to automatic flow control */ - adpt->automatic = true; + /* Disable transmission of pause frames by default, to avoid the + * risk of a pause frame flood that can occur if the kernel hangs. + * We still want to be able to respond to them, however. + */ + adpt->automatic = false; + adpt->tx_flow_control = false; + adpt->rx_flow_control = true; } /* Get the clock */ -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-01 21:37 ` [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default Timur Tabi @ 2017-08-01 21:55 ` Florian Fainelli 2017-08-01 22:02 ` Timur Tabi 2017-08-01 23:15 ` Andrew Lunn ` (2 subsequent siblings) 3 siblings, 1 reply; 26+ messages in thread From: Florian Fainelli @ 2017-08-01 21:55 UTC (permalink / raw) To: Timur Tabi, David S. Miller, netdev On 08/01/2017 02:37 PM, Timur Tabi wrote: > The EMAC has a curious qwirk when RX flow control is enabled and the > kernel hangs. With the kernel hung, the EMAC's RX queue soon fills. > If RX flow control is enabled, the EMAC will then send a non-stop > stream of pause frames until the system is reset. The EMAC does not > have a built-in watchdog. > > In various tests, the pause frame stream sometimes overloads nearby > switches, effectively disabling the network. Since the RX queue is > large and the host processor is more than capable of handling incoming > packets quickly, the only time the EMAC will send any pause frames is > when the kernel is hung and unrecoverable. This is not specific to your EMAC, a lot of adapters have this problem actually. I wonder if it would make sense to reach for a broader solution where we could have a networking stack panic/oops notifier which will actively clean up the active network devices' RX queue(s) and if tx_pause was enabled, disable it. We could have drivers announce themselves as needing this either via NETIF_F_* feature bit or some other private flag. > > To avoid all these problems, we disable flow control autonegotiation > by default, and only enable receiving pause frames. > > Cc: stable@vger.kernel.org # 4.11.x > Signed-off-by: Timur Tabi <timur@codeaurora.org> > --- > drivers/net/ethernet/qualcomm/emac/emac.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c > index 60850bfa3d32..475c0ea29235 100644 > --- a/drivers/net/ethernet/qualcomm/emac/emac.c > +++ b/drivers/net/ethernet/qualcomm/emac/emac.c > @@ -441,8 +441,13 @@ static void emac_init_adapter(struct emac_adapter *adpt) > /* others */ > adpt->preamble = EMAC_PREAMBLE_DEF; > > - /* default to automatic flow control */ > - adpt->automatic = true; > + /* Disable transmission of pause frames by default, to avoid the > + * risk of a pause frame flood that can occur if the kernel hangs. > + * We still want to be able to respond to them, however. > + */ > + adpt->automatic = false; > + adpt->tx_flow_control = false; > + adpt->rx_flow_control = true; > } > > /* Get the clock */ > -- Florian ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-01 21:55 ` Florian Fainelli @ 2017-08-01 22:02 ` Timur Tabi 2017-08-01 22:08 ` Florian Fainelli 0 siblings, 1 reply; 26+ messages in thread From: Timur Tabi @ 2017-08-01 22:02 UTC (permalink / raw) To: Florian Fainelli, David S. Miller, netdev On 08/01/2017 04:55 PM, Florian Fainelli wrote: > This is not specific to your EMAC, a lot of adapters have this problem > actually. > > I wonder if it would make sense to reach for a broader solution where we > could have a networking stack panic/oops notifier which will actively > clean up the active network devices' RX queue(s) and if tx_pause was > enabled, disable it. We could have drivers announce themselves as > needing this either via NETIF_F_* feature bit or some other private flag. Unfortunately, the problem occurs only when Linux hangs, to the point where the driver's interrupt handlers are blocked. The RX queue is 256 entries, and the processor has 48 cores, so the EMAC is never going to send pause frames in any real-world situation. The only time I've seen pause frames sent out is in the lab when I halt the cores with a hardware debugger, and only if I have enough network traffic that the EMAC picks up. -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-01 22:02 ` Timur Tabi @ 2017-08-01 22:08 ` Florian Fainelli 0 siblings, 0 replies; 26+ messages in thread From: Florian Fainelli @ 2017-08-01 22:08 UTC (permalink / raw) To: Timur Tabi, David S. Miller, netdev On 08/01/2017 03:02 PM, Timur Tabi wrote: > On 08/01/2017 04:55 PM, Florian Fainelli wrote: >> This is not specific to your EMAC, a lot of adapters have this problem >> actually. >> >> I wonder if it would make sense to reach for a broader solution where we >> could have a networking stack panic/oops notifier which will actively >> clean up the active network devices' RX queue(s) and if tx_pause was >> enabled, disable it. We could have drivers announce themselves as >> needing this either via NETIF_F_* feature bit or some other private flag. > > Unfortunately, the problem occurs only when Linux hangs, to the point > where the driver's interrupt handlers are blocked. The RX queue is 256 > entries, and the processor has 48 cores, so the EMAC is never going to > send pause frames in any real-world situation. > > The only time I've seen pause frames sent out is in the lab when I halt > the cores with a hardware debugger, and only if I have enough network > traffic that the EMAC picks up. The size and scale of your system makes it so but imagine e.g: a single core ~ 1Ghz @ 1Gbits/sec system having the same problems, here you are quite likely to see the system under panic flooding the network. Then again your patch is fine and can be revised at any time a broader facility is offered, I just felt like we actually have a good way with reasonably driver-agnostic code to possibly deal with that problem. Implementing such a solution would not be a -stable backport candidate though.... -- Florian ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-01 21:37 ` [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default Timur Tabi 2017-08-01 21:55 ` Florian Fainelli @ 2017-08-01 23:15 ` Andrew Lunn 2017-08-02 0:56 ` Timur Tabi 2017-08-02 13:48 ` David Laight 2017-08-02 17:54 ` David Miller 3 siblings, 1 reply; 26+ messages in thread From: Andrew Lunn @ 2017-08-01 23:15 UTC (permalink / raw) To: Timur Tabi; +Cc: David S. Miller, netdev On Tue, Aug 01, 2017 at 04:37:39PM -0500, Timur Tabi wrote: > The EMAC has a curious qwirk when RX flow control is enabled and the > kernel hangs. With the kernel hung, the EMAC's RX queue soon fills. > If RX flow control is enabled, the EMAC will then send a non-stop > stream of pause frames until the system is reset. The EMAC does not > have a built-in watchdog. > > In various tests, the pause frame stream sometimes overloads nearby > switches, effectively disabling the network. Since the RX queue is > large and the host processor is more than capable of handling incoming > packets quickly, the only time the EMAC will send any pause frames is > when the kernel is hung and unrecoverable. > > To avoid all these problems, we disable flow control autonegotiation > by default, and only enable receiving pause frames. > > Cc: stable@vger.kernel.org # 4.11.x > Signed-off-by: Timur Tabi <timur@codeaurora.org> > --- > drivers/net/ethernet/qualcomm/emac/emac.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c > index 60850bfa3d32..475c0ea29235 100644 > --- a/drivers/net/ethernet/qualcomm/emac/emac.c > +++ b/drivers/net/ethernet/qualcomm/emac/emac.c > @@ -441,8 +441,13 @@ static void emac_init_adapter(struct emac_adapter *adpt) > /* others */ > adpt->preamble = EMAC_PREAMBLE_DEF; > > - /* default to automatic flow control */ > - adpt->automatic = true; > + /* Disable transmission of pause frames by default, to avoid the > + * risk of a pause frame flood that can occur if the kernel hangs. > + * We still want to be able to respond to them, however. > + */ > + adpt->automatic = false; > + adpt->tx_flow_control = false; > + adpt->rx_flow_control = true; Hi Timur Pause frames are something you can auto-negotiate at the PHY level. Should you also be clearing some bits in the phydev, so the peer knows pause frames are not supported? Andrew ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-01 23:15 ` Andrew Lunn @ 2017-08-02 0:56 ` Timur Tabi 2017-08-02 2:58 ` Andrew Lunn 0 siblings, 1 reply; 26+ messages in thread From: Timur Tabi @ 2017-08-02 0:56 UTC (permalink / raw) To: Andrew Lunn; +Cc: David S. Miller, netdev On 8/1/17 6:15 PM, Andrew Lunn wrote: > Pause frames are something you can auto-negotiate at the PHY > level. Should you also be clearing some bits in the phydev, so the > peer knows pause frames are not supported? When pause frame autonegotiation is enabled in the driver, that only means that the driver looks at what the PHY has autonegotiated, and then configures the MAC to match that. The driver doesn't touch the PHY at all. It leaves all that to phylib. Now if autonegotiation is disabled in the driver, then it just hard-codes those TX/RX settings in the driver. Are you saying I should program the PHY at the point to disable autonegotiation on the PHY level? If so, then I don't know how to do that. I just assumed that the MAC never tells the PHY what to do. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-02 0:56 ` Timur Tabi @ 2017-08-02 2:58 ` Andrew Lunn 2017-08-02 3:22 ` Timur Tabi 0 siblings, 1 reply; 26+ messages in thread From: Andrew Lunn @ 2017-08-02 2:58 UTC (permalink / raw) To: Timur Tabi; +Cc: David S. Miller, netdev On Tue, Aug 01, 2017 at 07:56:31PM -0500, Timur Tabi wrote: > On 8/1/17 6:15 PM, Andrew Lunn wrote: > >Pause frames are something you can auto-negotiate at the PHY > >level. Should you also be clearing some bits in the phydev, so the > >peer knows pause frames are not supported? > > When pause frame autonegotiation is enabled in the driver, that only means > that the driver looks at what the PHY has autonegotiated, and then > configures the MAC to match that. > > The driver doesn't touch the PHY at all. It leaves all that to phylib. > > Now if autonegotiation is disabled in the driver, then it just hard-codes > those TX/RX settings in the driver. Are you saying I should program the PHY > at the point to disable autonegotiation on the PHY level? If so, then I > don't know how to do that. I just assumed that the MAC never tells the PHY > what to do. Documentation/networking/phy.txt says: Pause frames / flow control The PHY does not participate directly in flow control/pause frames except by making sure that the SUPPORTED_Pause and SUPPORTED_AsymPause bits are set in MII_ADVERTISE to indicate towards the link partner that the Ethernet MAC controller supports such a thing. Since flow control/pause frames generation involves the Ethernet MAC driver, it is recommended that this driver takes care of properly indicating advertisement and support for such features by setting the SUPPORTED_Pause and SUPPORTED_AsymPause bits accordingly. This can be done either before or after phy_connect() and/or as a result of implementing the ethtool::set_pauseparam feature. So just check if the MAC driver is setting SUPPORTED_Pause and SUPPORTED_AsymPause. Andrew ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-02 2:58 ` Andrew Lunn @ 2017-08-02 3:22 ` Timur Tabi 0 siblings, 0 replies; 26+ messages in thread From: Timur Tabi @ 2017-08-02 3:22 UTC (permalink / raw) To: Andrew Lunn; +Cc: David S. Miller, netdev On 8/1/17 9:58 PM, Andrew Lunn wrote: > The PHY does not participate directly in flow control/pause frames except by > making sure that the SUPPORTED_Pause and SUPPORTED_AsymPause bits are set in > MII_ADVERTISE to indicate towards the link partner that the Ethernet MAC > controller supports such a thing. Since flow control/pause frames generation > involves the Ethernet MAC driver, it is recommended that this driver takes care > of properly indicating advertisement and support for such features by setting > the SUPPORTED_Pause and SUPPORTED_AsymPause bits accordingly. This can be done > either before or after phy_connect() and/or as a result of implementing the > ethtool::set_pauseparam feature. > > So just check if the MAC driver is setting SUPPORTED_Pause and > SUPPORTED_AsymPause. I think this was covered in this thread: http://www.spinics.net/lists/netdev/msg408978.html -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation. ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-01 21:37 ` [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default Timur Tabi 2017-08-01 21:55 ` Florian Fainelli 2017-08-01 23:15 ` Andrew Lunn @ 2017-08-02 13:48 ` David Laight 2017-08-02 14:21 ` Timur Tabi 2017-08-02 17:54 ` David Miller 3 siblings, 1 reply; 26+ messages in thread From: David Laight @ 2017-08-02 13:48 UTC (permalink / raw) To: 'Timur Tabi', David S. Miller, netdev From: Timur Tabi > Sent: 01 August 2017 22:38 > The EMAC has a curious qwirk when RX flow control is enabled and the > kernel hangs. With the kernel hung, the EMAC's RX queue soon fills. > If RX flow control is enabled, the EMAC will then send a non-stop > stream of pause frames until the system is reset. The EMAC does not > have a built-in watchdog. > > In various tests, the pause frame stream sometimes overloads nearby > switches, effectively disabling the network. ... If the nearby switches cannot handle pause frames, then the MAC shouldn't be sending them at all. They I suspect that they should only ever be sent if the phy autonegotiation indicates that they are supported. You might want to avoid sending them even if allowed, or advertise non-support on hardware that could support them, but sending them anyway is likely to cause grief. This is similar to the problems that arise when the 'speed' is forced instead of limiting the advertised speed to one value. David ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-02 13:48 ` David Laight @ 2017-08-02 14:21 ` Timur Tabi 2017-08-02 14:51 ` David Laight 0 siblings, 1 reply; 26+ messages in thread From: Timur Tabi @ 2017-08-02 14:21 UTC (permalink / raw) To: David Laight, David S. Miller, netdev On 8/2/17 8:48 AM, David Laight wrote: > If the nearby switches cannot handle pause frames, then the MAC shouldn't > be sending them at all. There's no way for me to know whether the switches can handle the pause frames or not. You would think that sending one multicast pause frame ever 33ms would not overload a switch, but in our lab it does. This is why I changed the default to disable sending pause frames. > They > > I suspect that they should only ever be sent if the phy autonegotiation > indicates that they are supported. Or if you manually enable them. That's why I changed to default: the MAC is now configured by default to accept pause frames but not send them. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation. ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-02 14:21 ` Timur Tabi @ 2017-08-02 14:51 ` David Laight 2017-08-02 15:08 ` Timur Tabi 0 siblings, 1 reply; 26+ messages in thread From: David Laight @ 2017-08-02 14:51 UTC (permalink / raw) To: 'Timur Tabi', David S. Miller, netdev From: Timur Tabi > Sent: 02 August 2017 15:22 > On 8/2/17 8:48 AM, David Laight wrote: > > If the nearby switches cannot handle pause frames, then the MAC shouldn't > > be sending them at all. > > There's no way for me to know whether the switches can handle the pause > frames or not. You would think that sending one multicast pause frame > ever 33ms would not overload a switch, but in our lab it does. > > This is why I changed the default to disable sending pause frames. ... Thinks ... Sending pause frames just tells the adjacent switch not to send you packets (because you'll discard them). Since the idea is to avoid the discards, the switch will buffer the packets it would have sent. The buffers in the switch then fill up with packets it isn't sending you. The switch then runs out of buffers, it has 2 choices: 1) Throw the packets away. 2) Send 'pause' frames to the sources. If it sends 'pause' frames the entire network will very quickly lock up. If it discards the packets they might as well have been discarded by the receiving MAC. Doesn't this mean that pause frames are 99.999% useless?? David ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-02 14:51 ` David Laight @ 2017-08-02 15:08 ` Timur Tabi 2017-08-02 15:38 ` David Laight 0 siblings, 1 reply; 26+ messages in thread From: Timur Tabi @ 2017-08-02 15:08 UTC (permalink / raw) To: David Laight, David S. Miller, netdev On 08/02/2017 09:51 AM, David Laight wrote: > Sending pause frames just tells the adjacent switch not to send you packets > (because you'll discard them). > Since the idea is to avoid the discards, the switch will buffer the > packets it would have sent. > The buffers in the switch then fill up with packets it isn't sending you. I was under the impression that the switch forwards the pause frames to other devices, so that the transmitting NIC can stop sending the data, but your explanation makes a lot more sense. If the EMAC never stops sending pause frames, then the switch's buffers will fill up, disabling all other devices. If the switch does not have per-port buffers, then it makes sense when the buffer is full, it blocks all ports. > The switch then runs out of buffers, it has 2 choices: > 1) Throw the packets away. > 2) Send 'pause' frames to the sources. > If it sends 'pause' frames the entire network will very quickly lock up. > If it discards the packets they might as well have been discarded by the > receiving MAC. > > Doesn't this mean that pause frames are 99.999% useless?? Pause frames are intended for situations where the receiving CPU is temporarily overwhelmed and just needs a second or two to resume processing incoming packets. That makes sense on a dinky single-core 32-bit CPU. -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-02 15:08 ` Timur Tabi @ 2017-08-02 15:38 ` David Laight 0 siblings, 0 replies; 26+ messages in thread From: David Laight @ 2017-08-02 15:38 UTC (permalink / raw) To: 'Timur Tabi', David S. Miller, netdev From: Timur Tabi > Sent: 02 August 2017 16:09 > On 08/02/2017 09:51 AM, David Laight wrote: > > Sending pause frames just tells the adjacent switch not to send you packets > > (because you'll discard them). > > Since the idea is to avoid the discards, the switch will buffer the > > packets it would have sent. > > The buffers in the switch then fill up with packets it isn't sending you. > > I was under the impression that the switch forwards the pause frames to > other devices, so that the transmitting NIC can stop sending the data, Most of my ethernet knowledge predates FDX :-) It wouldn't make any sense to try to use the source address of a pause frame to suppress traffic to a specific MAC - that would have to go way down into IP on all the receiving systems. Also ISTR that pause frames are very short and don't even contain MAC addresses. > but your explanation makes a lot more sense. If the EMAC never stops > sending pause frames, then the switch's buffers will fill up, disabling > all other devices. If the switch does not have per-port buffers, then > it makes sense when the buffer is full, it blocks all ports. A switch will (probably) either have buffers for each input port, or for each output port (or maybe both). Output port buffers are less likely to cause grief when an output port is running at a slower speed than the input port. But is a switch is going to send pause frames it doesn't really matter how the buffers are arranged. The cascade will still happen. It would take very careful heuristics in a switch to manage pause frames properly. Of course, once the kernel has crashed, even multicast packets will eventually run the MAC out of buffers. (Unless you run on private network and manage to reinstall and reboot while the MAC is still active and then eventually die when a receive frame overwrites what used to be a receive buffer.) David ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-01 21:37 ` [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default Timur Tabi ` (2 preceding siblings ...) 2017-08-02 13:48 ` David Laight @ 2017-08-02 17:54 ` David Miller 2017-08-02 18:23 ` Timur Tabi 3 siblings, 1 reply; 26+ messages in thread From: David Miller @ 2017-08-02 17:54 UTC (permalink / raw) To: timur; +Cc: netdev From: Timur Tabi <timur@codeaurora.org> Date: Tue, 1 Aug 2017 16:37:39 -0500 > The EMAC has a curious qwirk when RX flow control is enabled and the > kernel hangs. With the kernel hung, the EMAC's RX queue soon fills. > If RX flow control is enabled, the EMAC will then send a non-stop > stream of pause frames until the system is reset. The EMAC does not > have a built-in watchdog. > > In various tests, the pause frame stream sometimes overloads nearby > switches, effectively disabling the network. Since the RX queue is > large and the host processor is more than capable of handling incoming > packets quickly, the only time the EMAC will send any pause frames is > when the kernel is hung and unrecoverable. > > To avoid all these problems, we disable flow control autonegotiation > by default, and only enable receiving pause frames. > > Cc: stable@vger.kernel.org # 4.11.x > Signed-off-by: Timur Tabi <timur@codeaurora.org> I've thought about this a lot and I don't like it for many reasons. First of all, this hung kernel scenerio is completely bogus. The ethernet chip may not have a proper watchdog for the pause frame generation logic, but the whole kernel sure as hell does. And if this kind of thing matters to the user, they will have a software or hardware watchdog driver enabled to break out of this situation. Turning off flow control by default has so many negative ramifications and don't try to convince me that users will be "aware" of this and turn it back on. They largely won't. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-02 17:54 ` David Miller @ 2017-08-02 18:23 ` Timur Tabi 2017-08-02 18:35 ` David Miller 2017-08-02 18:36 ` David Miller 0 siblings, 2 replies; 26+ messages in thread From: Timur Tabi @ 2017-08-02 18:23 UTC (permalink / raw) To: David Miller; +Cc: netdev On 08/02/2017 12:54 PM, David Miller wrote: > And if this kind of thing matters to the user, they will have a > software or hardware watchdog driver enabled to break out of this > situation. The problem is that the user is not going to expect that the EMAC can disable the nearby switch(es) when the kernel is hung and not rebooted quickly enough. Internally, this bug/feature has caused quite a bit of mayhem, so the problem is real. No cares about enabling flow control -- it just happens to be enabled on some systems where the switch agrees to it. So random individuals can't debug the hardware because suddenly the EMAC has gone haywire and disabled the local network. > Turning off flow control by default has so many negative ramifications > and don't try to convince me that users will be "aware" of this and > turn it back on. What are the negative ramifications? It's practically impossible to overload the chip such that it can't process the incoming packets fast enough. I don't know of any real-world situation where the EMAC needs to transmit pause frames. -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-02 18:23 ` Timur Tabi @ 2017-08-02 18:35 ` David Miller 2017-08-02 18:39 ` Timur Tabi 2017-08-02 18:36 ` David Miller 1 sibling, 1 reply; 26+ messages in thread From: David Miller @ 2017-08-02 18:35 UTC (permalink / raw) To: timur; +Cc: netdev From: Timur Tabi <timur@codeaurora.org> Date: Wed, 2 Aug 2017 13:23:18 -0500 > On 08/02/2017 12:54 PM, David Miller wrote: >> And if this kind of thing matters to the user, they will have a >> software or hardware watchdog driver enabled to break out of this >> situation. > > The problem is that the user is not going to expect that the EMAC can > disable the nearby switch(es) when the kernel is hung and not rebooted > quickly enough. Internally, this bug/feature has caused quite a bit > of mayhem, so the problem is real. No cares about enabling flow > control -- it just happens to be enabled on some systems where the > switch agrees to it. So random individuals can't debug the hardware > because suddenly the EMAC has gone haywire and disabled the local > network. Again, any serious installation will have a system watchdog enabled which will break the pause frame bomb. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-02 18:35 ` David Miller @ 2017-08-02 18:39 ` Timur Tabi 2017-08-02 23:15 ` David Miller 0 siblings, 1 reply; 26+ messages in thread From: Timur Tabi @ 2017-08-02 18:39 UTC (permalink / raw) To: David Miller; +Cc: netdev On 08/02/2017 01:35 PM, David Miller wrote: > Again, any serious installation will have a system watchdog enabled > which will break the pause frame bomb. Oh well. I guess I'll have to carry this patch internally. What about patch #2? -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-02 18:39 ` Timur Tabi @ 2017-08-02 23:15 ` David Miller 2017-08-03 1:00 ` Timur Tabi 0 siblings, 1 reply; 26+ messages in thread From: David Miller @ 2017-08-02 23:15 UTC (permalink / raw) To: timur; +Cc: netdev From: Timur Tabi <timur@codeaurora.org> Date: Wed, 2 Aug 2017 13:39:34 -0500 > On 08/02/2017 01:35 PM, David Miller wrote: >> Again, any serious installation will have a system watchdog enabled >> which will break the pause frame bomb. > > Oh well. I guess I'll have to carry this patch internally. So you don't want to run a proper watchdog on your systems? You want them to just hang there and wait for someone to notice instead? > What about patch #2? No real objection. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-02 23:15 ` David Miller @ 2017-08-03 1:00 ` Timur Tabi 0 siblings, 0 replies; 26+ messages in thread From: Timur Tabi @ 2017-08-03 1:00 UTC (permalink / raw) To: David Miller; +Cc: netdev On 8/2/17 6:15 PM, David Miller wrote: > So you don't want to run a proper watchdog on your systems? > You want them to just hang there and wait for someone to > notice instead? This is for internal development. We noticed the problem first during debugging, when we would halt a core for more than a couple minutes. Then the entire lab network would go down. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default 2017-08-02 18:23 ` Timur Tabi 2017-08-02 18:35 ` David Miller @ 2017-08-02 18:36 ` David Miller 1 sibling, 0 replies; 26+ messages in thread From: David Miller @ 2017-08-02 18:36 UTC (permalink / raw) To: timur; +Cc: netdev From: Timur Tabi <timur@codeaurora.org> Date: Wed, 2 Aug 2017 13:23:18 -0500 > It's practically impossible to overload the chip such that it can't > process the incoming packets fast enough. I don't know of any > real-world situation where the EMAC needs to transmit pause frames. Slow cpus or very expensive stack operations can cause this, it's not a property of the EMAC chip at all. ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 2/2] net: qcom/emac: add software control for pause frame mode 2017-08-01 21:37 [PATCH 0/2] net: qcom/emac: fixes for pause frame floods Timur Tabi 2017-08-01 21:37 ` [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default Timur Tabi @ 2017-08-01 21:37 ` Timur Tabi 2017-08-01 21:51 ` Florian Fainelli 2017-09-12 22:07 ` Timur Tabi 1 sibling, 2 replies; 26+ messages in thread From: Timur Tabi @ 2017-08-01 21:37 UTC (permalink / raw) To: David S. Miller, netdev; +Cc: timur The EMAC has the option of sending only a single pause frame when flow control is enabled and the RX queue is full. Although sending only one pause frame has little value, this would allow admins to enable automatic flow control without having to worry about the EMAC flooding nearby switches with pause frames if the kernel hangs. The option is enabled by using the single-pause-mode private flag. Signed-off-by: Timur Tabi <timur@codeaurora.org> --- drivers/net/ethernet/qualcomm/emac/emac-ethtool.c | 30 +++++++++++++++++++++++ drivers/net/ethernet/qualcomm/emac/emac-mac.c | 22 +++++++++++++++++ drivers/net/ethernet/qualcomm/emac/emac.c | 3 +++ drivers/net/ethernet/qualcomm/emac/emac.h | 3 +++ 4 files changed, 58 insertions(+) diff --git a/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c b/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c index bbe24639aa5a..c8c6231b87f3 100644 --- a/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c +++ b/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c @@ -88,6 +88,8 @@ static void emac_set_msglevel(struct net_device *netdev, u32 data) static int emac_get_sset_count(struct net_device *netdev, int sset) { switch (sset) { + case ETH_SS_PRIV_FLAGS: + return 1; case ETH_SS_STATS: return EMAC_STATS_LEN; default: @@ -100,6 +102,10 @@ static void emac_get_strings(struct net_device *netdev, u32 stringset, u8 *data) unsigned int i; switch (stringset) { + case ETH_SS_PRIV_FLAGS: + strcpy(data, "single-pause-mode"); + break; + case ETH_SS_STATS: for (i = 0; i < EMAC_STATS_LEN; i++) { strlcpy(data, emac_ethtool_stat_strings[i], @@ -230,6 +236,27 @@ static int emac_get_regs_len(struct net_device *netdev) return EMAC_MAX_REG_SIZE * sizeof(u32); } +#define EMAC_PRIV_ENABLE_SINGLE_PAUSE BIT(0) + +static int emac_set_priv_flags(struct net_device *netdev, u32 flags) +{ + struct emac_adapter *adpt = netdev_priv(netdev); + + adpt->single_pause_mode = !!(flags & EMAC_PRIV_ENABLE_SINGLE_PAUSE); + + if (netif_running(netdev)) + return emac_reinit_locked(adpt); + + return 0; +} + +static u32 emac_get_priv_flags(struct net_device *netdev) +{ + struct emac_adapter *adpt = netdev_priv(netdev); + + return adpt->single_pause_mode ? EMAC_PRIV_ENABLE_SINGLE_PAUSE : 0; +} + static const struct ethtool_ops emac_ethtool_ops = { .get_link_ksettings = phy_ethtool_get_link_ksettings, .set_link_ksettings = phy_ethtool_set_link_ksettings, @@ -253,6 +280,9 @@ static int emac_get_regs_len(struct net_device *netdev) .get_regs_len = emac_get_regs_len, .get_regs = emac_get_regs, + + .set_priv_flags = emac_set_priv_flags, + .get_priv_flags = emac_get_priv_flags, }; void emac_set_ethtool_ops(struct net_device *netdev) diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.c b/drivers/net/ethernet/qualcomm/emac/emac-mac.c index bcd4708b3745..0ea3ca09c689 100644 --- a/drivers/net/ethernet/qualcomm/emac/emac-mac.c +++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.c @@ -551,6 +551,28 @@ static void emac_mac_start(struct emac_adapter *adpt) mac &= ~(HUGEN | VLAN_STRIP | TPAUSE | SIMR | HUGE | MULTI_ALL | DEBUG_MODE | SINGLE_PAUSE_MODE); + /* Enable single-pause-frame mode if requested. + * + * If enabled, the EMAC will send a single pause frame when the RX + * queue is full. This normally leads to packet loss because + * the pause frame disables the remote MAC only for 33ms (the quanta), + * and then the remote MAC continues sending packets even though + * the RX queue is still full. + * + * If disabled, the EMAC sends a pause frame every 31ms until the RX + * queue is no longer full. Normally, this is the preferred + * method of operation. However, when the system is hung (e.g. + * cores are halted), the EMAC interrupt handler is never called + * and so the RX queue fills up quickly and stays full. The resuling + * non-stop "flood" of pause frames sometimes has the effect of + * disabling nearby switches. In some cases, other nearby switches + * are also affected, shutting down the entire network. + * + * The user can enable or disable single-pause-frame mode + * via ethtool. + */ + mac |= adpt->single_pause_mode ? SINGLE_PAUSE_MODE : 0; + writel_relaxed(csr1, adpt->csr + EMAC_EMAC_WRAPPER_CSR1); writel_relaxed(mac, adpt->base + EMAC_MAC_CTRL); diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c index 475c0ea29235..6f5e858ffbf3 100644 --- a/drivers/net/ethernet/qualcomm/emac/emac.c +++ b/drivers/net/ethernet/qualcomm/emac/emac.c @@ -448,6 +448,9 @@ static void emac_init_adapter(struct emac_adapter *adpt) adpt->automatic = false; adpt->tx_flow_control = false; adpt->rx_flow_control = true; + + /* Disable single-pause-frame mode by default */ + adpt->single_pause_mode = false; } /* Get the clock */ diff --git a/drivers/net/ethernet/qualcomm/emac/emac.h b/drivers/net/ethernet/qualcomm/emac/emac.h index 8ee4ec6aef2e..d7c9f44209d4 100644 --- a/drivers/net/ethernet/qualcomm/emac/emac.h +++ b/drivers/net/ethernet/qualcomm/emac/emac.h @@ -363,6 +363,9 @@ struct emac_adapter { bool tx_flow_control; bool rx_flow_control; + /* True == use single-pause-frame mode. */ + bool single_pause_mode; + /* Ring parameter */ u8 tpd_burst; u8 rfd_burst; -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] net: qcom/emac: add software control for pause frame mode 2017-08-01 21:37 ` [PATCH 2/2] net: qcom/emac: add software control for pause frame mode Timur Tabi @ 2017-08-01 21:51 ` Florian Fainelli 2017-08-01 22:00 ` Timur Tabi 2017-09-12 22:07 ` Timur Tabi 1 sibling, 1 reply; 26+ messages in thread From: Florian Fainelli @ 2017-08-01 21:51 UTC (permalink / raw) To: Timur Tabi, David S. Miller, netdev On 08/01/2017 02:37 PM, Timur Tabi wrote: > The EMAC has the option of sending only a single pause frame when > flow control is enabled and the RX queue is full. Although sending > only one pause frame has little value, this would allow admins to > enable automatic flow control without having to worry about the EMAC > flooding nearby switches with pause frames if the kernel hangs. > > The option is enabled by using the single-pause-mode private flag. A few adapters (bcmgenet, bcmsysport) support configuring the pause quanta so it would not be inconceivable to try to update ethtool_pauseparam to include additional fields such as: - number of pause frames to send where we define an arbitrary high default value (e.g: 0xffff), N < 0xffff is something drivers can test for whether they support it, and 0 is only valid if pause is already disabled - pause quanta (16-bits) Private flags are not usually that great and there could be more adapters capable of doing the same pause frame number configuration, but since there is no available knob it's hard to know. > > Signed-off-by: Timur Tabi <timur@codeaurora.org> > --- > drivers/net/ethernet/qualcomm/emac/emac-ethtool.c | 30 +++++++++++++++++++++++ > drivers/net/ethernet/qualcomm/emac/emac-mac.c | 22 +++++++++++++++++ > drivers/net/ethernet/qualcomm/emac/emac.c | 3 +++ > drivers/net/ethernet/qualcomm/emac/emac.h | 3 +++ > 4 files changed, 58 insertions(+) > > diff --git a/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c b/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c > index bbe24639aa5a..c8c6231b87f3 100644 > --- a/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c > +++ b/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c > @@ -88,6 +88,8 @@ static void emac_set_msglevel(struct net_device *netdev, u32 data) > static int emac_get_sset_count(struct net_device *netdev, int sset) > { > switch (sset) { > + case ETH_SS_PRIV_FLAGS: > + return 1; > case ETH_SS_STATS: > return EMAC_STATS_LEN; > default: > @@ -100,6 +102,10 @@ static void emac_get_strings(struct net_device *netdev, u32 stringset, u8 *data) > unsigned int i; > > switch (stringset) { > + case ETH_SS_PRIV_FLAGS: > + strcpy(data, "single-pause-mode"); > + break; > + > case ETH_SS_STATS: > for (i = 0; i < EMAC_STATS_LEN; i++) { > strlcpy(data, emac_ethtool_stat_strings[i], > @@ -230,6 +236,27 @@ static int emac_get_regs_len(struct net_device *netdev) > return EMAC_MAX_REG_SIZE * sizeof(u32); > } > > +#define EMAC_PRIV_ENABLE_SINGLE_PAUSE BIT(0) > + > +static int emac_set_priv_flags(struct net_device *netdev, u32 flags) > +{ > + struct emac_adapter *adpt = netdev_priv(netdev); > + > + adpt->single_pause_mode = !!(flags & EMAC_PRIV_ENABLE_SINGLE_PAUSE); > + > + if (netif_running(netdev)) > + return emac_reinit_locked(adpt); > + > + return 0; > +} > + > +static u32 emac_get_priv_flags(struct net_device *netdev) > +{ > + struct emac_adapter *adpt = netdev_priv(netdev); > + > + return adpt->single_pause_mode ? EMAC_PRIV_ENABLE_SINGLE_PAUSE : 0; > +} > + > static const struct ethtool_ops emac_ethtool_ops = { > .get_link_ksettings = phy_ethtool_get_link_ksettings, > .set_link_ksettings = phy_ethtool_set_link_ksettings, > @@ -253,6 +280,9 @@ static int emac_get_regs_len(struct net_device *netdev) > > .get_regs_len = emac_get_regs_len, > .get_regs = emac_get_regs, > + > + .set_priv_flags = emac_set_priv_flags, > + .get_priv_flags = emac_get_priv_flags, > }; > > void emac_set_ethtool_ops(struct net_device *netdev) > diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.c b/drivers/net/ethernet/qualcomm/emac/emac-mac.c > index bcd4708b3745..0ea3ca09c689 100644 > --- a/drivers/net/ethernet/qualcomm/emac/emac-mac.c > +++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.c > @@ -551,6 +551,28 @@ static void emac_mac_start(struct emac_adapter *adpt) > mac &= ~(HUGEN | VLAN_STRIP | TPAUSE | SIMR | HUGE | MULTI_ALL | > DEBUG_MODE | SINGLE_PAUSE_MODE); > > + /* Enable single-pause-frame mode if requested. > + * > + * If enabled, the EMAC will send a single pause frame when the RX > + * queue is full. This normally leads to packet loss because > + * the pause frame disables the remote MAC only for 33ms (the quanta), > + * and then the remote MAC continues sending packets even though > + * the RX queue is still full. > + * > + * If disabled, the EMAC sends a pause frame every 31ms until the RX > + * queue is no longer full. Normally, this is the preferred > + * method of operation. However, when the system is hung (e.g. > + * cores are halted), the EMAC interrupt handler is never called > + * and so the RX queue fills up quickly and stays full. The resuling > + * non-stop "flood" of pause frames sometimes has the effect of > + * disabling nearby switches. In some cases, other nearby switches > + * are also affected, shutting down the entire network. This explanation would have more value in your previous commit message, especially since you are targeting it as a bugfix. > + * > + * The user can enable or disable single-pause-frame mode > + * via ethtool. > + */ > + mac |= adpt->single_pause_mode ? SINGLE_PAUSE_MODE : 0; > + > writel_relaxed(csr1, adpt->csr + EMAC_EMAC_WRAPPER_CSR1); > > writel_relaxed(mac, adpt->base + EMAC_MAC_CTRL); > diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c > index 475c0ea29235..6f5e858ffbf3 100644 > --- a/drivers/net/ethernet/qualcomm/emac/emac.c > +++ b/drivers/net/ethernet/qualcomm/emac/emac.c > @@ -448,6 +448,9 @@ static void emac_init_adapter(struct emac_adapter *adpt) > adpt->automatic = false; > adpt->tx_flow_control = false; > adpt->rx_flow_control = true; > + > + /* Disable single-pause-frame mode by default */ > + adpt->single_pause_mode = false; > } > > /* Get the clock */ > diff --git a/drivers/net/ethernet/qualcomm/emac/emac.h b/drivers/net/ethernet/qualcomm/emac/emac.h > index 8ee4ec6aef2e..d7c9f44209d4 100644 > --- a/drivers/net/ethernet/qualcomm/emac/emac.h > +++ b/drivers/net/ethernet/qualcomm/emac/emac.h > @@ -363,6 +363,9 @@ struct emac_adapter { > bool tx_flow_control; > bool rx_flow_control; > > + /* True == use single-pause-frame mode. */ > + bool single_pause_mode; > + > /* Ring parameter */ > u8 tpd_burst; > u8 rfd_burst; > -- Florian ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] net: qcom/emac: add software control for pause frame mode 2017-08-01 21:51 ` Florian Fainelli @ 2017-08-01 22:00 ` Timur Tabi 2017-08-01 22:06 ` Florian Fainelli 0 siblings, 1 reply; 26+ messages in thread From: Timur Tabi @ 2017-08-01 22:00 UTC (permalink / raw) To: Florian Fainelli, David S. Miller, netdev On 08/01/2017 04:51 PM, Florian Fainelli wrote: > A few adapters (bcmgenet, bcmsysport) support configuring the pause > quanta so it would not be inconceivable to try to update > ethtool_pauseparam to include additional fields such as: Wouldn't this require a change to the user space tool? > - number of pause frames to send where we define an arbitrary high > default value (e.g: 0xffff), N < 0xffff is something drivers can test > for whether they support it, and 0 is only valid if pause is already > disabled > > - pause quanta (16-bits) > > Private flags are not usually that great and there could be more > adapters capable of doing the same pause frame number configuration, but > since there is no available knob it's hard to know. Well, for the EMAC, the quanta in this case would be either 1 or infinite. For other devices, it could be any combination of values. In a future revision of the hardware, we might support a variable quanta. And I suspect that some devices measure the quanta in time, not count. How would the user know what the acceptable values are? If I set the quanta to 10 via user space, and my driver truncates that to 1, I don't think that would be acceptable. -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] net: qcom/emac: add software control for pause frame mode 2017-08-01 22:00 ` Timur Tabi @ 2017-08-01 22:06 ` Florian Fainelli 0 siblings, 0 replies; 26+ messages in thread From: Florian Fainelli @ 2017-08-01 22:06 UTC (permalink / raw) To: Timur Tabi, David S. Miller, netdev On 08/01/2017 03:00 PM, Timur Tabi wrote: > On 08/01/2017 04:51 PM, Florian Fainelli wrote: > >> A few adapters (bcmgenet, bcmsysport) support configuring the pause >> quanta so it would not be inconceivable to try to update >> ethtool_pauseparam to include additional fields such as: > > Wouldn't this require a change to the user space tool? It would yes. > >> - number of pause frames to send where we define an arbitrary high >> default value (e.g: 0xffff), N < 0xffff is something drivers can test >> for whether they support it, and 0 is only valid if pause is already >> disabled >> >> - pause quanta (16-bits) >> >> Private flags are not usually that great and there could be more >> adapters capable of doing the same pause frame number configuration, but >> since there is no available knob it's hard to know. > > Well, for the EMAC, the quanta in this case would be either 1 or > infinite. For other devices, it could be any combination of values. In > a future revision of the hardware, we might support a variable quanta. > And I suspect that some devices measure the quanta in time, not count. OK, either way is fine and there should be ways to convert back and forth without too much trouble. > > How would the user know what the acceptable values are? If I set the > quanta to 10 via user space, and my driver truncates that to 1, I don't > think that would be acceptable. There can be several ways to deal with that, we can either ask for a strict rejection before committing the changes to the HW, or we can have the HW round up/down to the nearest supported values and an user would call get_pauseparam() to see which value ended-up being used. Anyway food for thought, David decides what gets merged ;) -- Florian ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] net: qcom/emac: add software control for pause frame mode 2017-08-01 21:37 ` [PATCH 2/2] net: qcom/emac: add software control for pause frame mode Timur Tabi 2017-08-01 21:51 ` Florian Fainelli @ 2017-09-12 22:07 ` Timur Tabi 1 sibling, 0 replies; 26+ messages in thread From: Timur Tabi @ 2017-09-12 22:07 UTC (permalink / raw) To: David S. Miller, netdev On 08/01/2017 04:37 PM, Timur Tabi wrote: > The EMAC has the option of sending only a single pause frame when > flow control is enabled and the RX queue is full. Although sending > only one pause frame has little value, this would allow admins to > enable automatic flow control without having to worry about the EMAC > flooding nearby switches with pause frames if the kernel hangs. > > The option is enabled by using the single-pause-mode private flag. > > Signed-off-by: Timur Tabi<timur@codeaurora.org> Dave, I don't see this patch in net-next. Can you pick it up for 4.14? -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2017-09-12 22:07 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-01 21:37 [PATCH 0/2] net: qcom/emac: fixes for pause frame floods Timur Tabi 2017-08-01 21:37 ` [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default Timur Tabi 2017-08-01 21:55 ` Florian Fainelli 2017-08-01 22:02 ` Timur Tabi 2017-08-01 22:08 ` Florian Fainelli 2017-08-01 23:15 ` Andrew Lunn 2017-08-02 0:56 ` Timur Tabi 2017-08-02 2:58 ` Andrew Lunn 2017-08-02 3:22 ` Timur Tabi 2017-08-02 13:48 ` David Laight 2017-08-02 14:21 ` Timur Tabi 2017-08-02 14:51 ` David Laight 2017-08-02 15:08 ` Timur Tabi 2017-08-02 15:38 ` David Laight 2017-08-02 17:54 ` David Miller 2017-08-02 18:23 ` Timur Tabi 2017-08-02 18:35 ` David Miller 2017-08-02 18:39 ` Timur Tabi 2017-08-02 23:15 ` David Miller 2017-08-03 1:00 ` Timur Tabi 2017-08-02 18:36 ` David Miller 2017-08-01 21:37 ` [PATCH 2/2] net: qcom/emac: add software control for pause frame mode Timur Tabi 2017-08-01 21:51 ` Florian Fainelli 2017-08-01 22:00 ` Timur Tabi 2017-08-01 22:06 ` Florian Fainelli 2017-09-12 22:07 ` Timur Tabi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.