* Re: [PATCH net-next v2 0/6] cxgb4: collect hardware logs via ethtool
From: David Miller @ 2017-12-08 19:34 UTC (permalink / raw)
To: rahul.lakkireddy; +Cc: netdev, ganeshgr, nirranjan, indranil
In-Reply-To: <cover.1512705841.git.rahul.lakkireddy@chelsio.com>
From: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
Date: Fri, 8 Dec 2017 09:48:35 +0530
> Collect more hardware logs via ethtool --get-dump facility.
>
> Patch 1 collects on-chip memory layout information.
>
> Patch 2 collects on-chip MC memory dumps.
>
> Patch 3 collects HMA memory dump.
>
> Patch 4 evaluates and skips TX and RX payload regions in memory dumps.
>
> Patch 5 collects egress and ingress SGE queue contexts.
>
> Patch 6 collects PCIe configuration logs
...
> ---
> v2:
> - Fix uninitialized variable "size" build warning in Patch 1.
That looks better, series applied, thanks.
^ permalink raw reply
* Re: [PATCH v6 net-next,mips 6/7] netdev: octeon-ethernet: Add Cavium Octeon III support.
From: David Miller @ 2017-12-08 19:29 UTC (permalink / raw)
To: david.daney-YGCgFSpz5w/QT0dZR+AlfA
Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA, ralf-6z/3iImG2C8G8FEW9MqTrA,
james.hogan-8NJIiSa5LzA, netdev-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
steven.hill-YGCgFSpz5w/QT0dZR+AlfA,
devicetree-u79uwXL29TY76Z2rM5mHXA, andrew-g2DYL2Zd6BY,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, pombredanne-od1rfyK75/E,
cmunoz-YGCgFSpz5w/QT0dZR+AlfA
In-Reply-To: <20171208000934.6554-7-david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
Date: Thu, 7 Dec 2017 16:09:33 -0800
> +static void bgx_port_check_state(struct work_struct *work)
> +{
...
> + mutex_lock(&priv->lock);
> + if (priv->work_queued)
> + queue_delayed_work(check_state_wq, &priv->dwork, HZ);
> + mutex_unlock(&priv->lock);
> +}
...
> +int bgx_port_disable(struct net_device *netdev)
> +{
...
> + mutex_lock(&priv->lock);
> + if (priv->work_queued) {
> + cancel_delayed_work_sync(&priv->dwork);
> + priv->work_queued = false;
This can deadlock.
When you do a sync work cancel, it waits until all running work
instances finish. You have the priv->lock, so if
bgx_port_check_status() need to still take priv->lock to complete
then no further progress will be made.
I think it is pointless to use this weird work_queued boolean state.
Just unconditionally, and without locking, cancel the delayed work,
ragardless of whether it was actually used ever or not.
Thank you.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/3] Bluetooth: hci_ll: add support for setting public address
From: Marcel Holtmann @ 2017-12-08 19:24 UTC (permalink / raw)
To: David Lechner
Cc: devicetree, open list:BLUETOOTH DRIVERS, Rob Herring,
Mark Rutland, Gustavo F. Padovan, Johan Hedberg,
Network Development, linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1a106ed6-1408-4d0b-9c9d-5a3058d6df2d-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
Hi David,
>>> This adds support for setting the public address on Texas Instruments
>>> Bluetooth chips using a vendor-specific command.
>>>
>>> This has been tested on a CC2560A. The TI wiki also indicates that this
>>> command should work on TI WL17xx/WL18xx Bluetooth chips.
>>>
>>> Signed-off-by: David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
>>> ---
>>>
>>> v2 changes:
>>> * This is a new patch in v2
>>>
>>> drivers/bluetooth/hci_ll.c | 17 +++++++++++++++++
>>> 1 file changed, 17 insertions(+)
>>>
>>> diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
>>> index 974a788..b732004 100644
>>> --- a/drivers/bluetooth/hci_ll.c
>>> +++ b/drivers/bluetooth/hci_ll.c
>>> @@ -57,6 +57,7 @@
>>> #include "hci_uart.h"
>>>
>>> /* Vendor-specific HCI commands */
>>> +#define HCI_VS_WRITE_BD_ADDR 0xfc06
>>> #define HCI_VS_UPDATE_UART_HCI_BAUDRATE 0xff36
>>>
>>> /* HCILL commands */
>>> @@ -662,6 +663,20 @@ static int download_firmware(struct ll_device *lldev)
>>> return err;
>>> }
>>>
>>> +static int ll_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
>>> +{
>>> + bdaddr_t bdaddr_swapped;
>>> + struct sk_buff *skb;
>>> +
>>> + baswap(&bdaddr_swapped, bdaddr);
>>> + skb = __hci_cmd_sync(hdev, HCI_VS_WRITE_BD_ADDR, sizeof(bdaddr_t),
>>> + &bdaddr_swapped, HCI_INIT_TIMEOUT);
>>> + if (!IS_ERR(skb))
>>> + kfree_skb(skb);
>>> +
>> You have a trailing whitespace here.
>> Does the HCI command really expect the BD_ADDR in the swapped order. The caller of hdev->set_bdaddr while provide it in the same order as the HCI Read BD Address command and everything in HCI. So it seems odd that you have to swap it for the vendor command.
>> So have you actually tested this with btmgmt public-add <xx:xx..> and checked that the address comes out correctly. I think ll_set_bdaddr should function correctly for the mgmt interface. And if needed any other caller outside of mgmt has to do the swapping.
>
> I did test using btmgmt public-address 00:11:22:33:44:55, which is how I found out that the order needed to be swapped. Like you, I was surprised. I couldn't find any documentation from TI saying what the order is supposed to be, so I can only assume that because this works, it is indeed correct as-is.
then please add a comment for that and I would appreciate to have the parts from btmon showing the public-addr command and the following HCI Read BD Address command as part of the commit message. Just for being able to dig this out at some later point if needed.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH net-next v3 0/2] veth and GSO maximums
From: David Miller @ 2017-12-08 19:23 UTC (permalink / raw)
To: stephen; +Cc: netdev, sthemmin
In-Reply-To: <20171207234020.18783-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Thu, 7 Dec 2017 15:40:18 -0800
> This is the more general way to solving the issue of GSO limits
> not being set correctly for containers on Azure. If a GSO packet
> is sent to host that exceeds the limit (reported by NDIS), then
> the host is forced to do segmentation in software which has noticeable
> performance impact.
>
> The core rtnetlink infrastructure already has the messages and
> infrastructure to allow changing gso limits. With an updated iproute2
> the following already works:
> # ip li set dev dummy0 gso_max_size 30000
>
> These patches are about making it easier with veth.
Ok, this is definitely a step forward.
Series applied, thanks Stephen.
^ permalink raw reply
* Re: [PATCH v2 1/3] Bluetooth: hci_ll: add support for setting public address
From: David Lechner @ 2017-12-08 19:21 UTC (permalink / raw)
To: Marcel Holtmann
Cc: devicetree, open list:BLUETOOTH DRIVERS, Rob Herring,
Mark Rutland, Gustavo F. Padovan, Johan Hedberg,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <8B6C3CFA-35C6-4A9B-90AB-22834B9AE2C4-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
On 12/08/2017 02:07 AM, Marcel Holtmann wrote:
> Hi David,
>
>> This adds support for setting the public address on Texas Instruments
>> Bluetooth chips using a vendor-specific command.
>>
>> This has been tested on a CC2560A. The TI wiki also indicates that this
>> command should work on TI WL17xx/WL18xx Bluetooth chips.
>>
>> Signed-off-by: David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
>> ---
>>
>> v2 changes:
>> * This is a new patch in v2
>>
>> drivers/bluetooth/hci_ll.c | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
>> index 974a788..b732004 100644
>> --- a/drivers/bluetooth/hci_ll.c
>> +++ b/drivers/bluetooth/hci_ll.c
>> @@ -57,6 +57,7 @@
>> #include "hci_uart.h"
>>
>> /* Vendor-specific HCI commands */
>> +#define HCI_VS_WRITE_BD_ADDR 0xfc06
>> #define HCI_VS_UPDATE_UART_HCI_BAUDRATE 0xff36
>>
>> /* HCILL commands */
>> @@ -662,6 +663,20 @@ static int download_firmware(struct ll_device *lldev)
>> return err;
>> }
>>
>> +static int ll_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
>> +{
>> + bdaddr_t bdaddr_swapped;
>> + struct sk_buff *skb;
>> +
>> + baswap(&bdaddr_swapped, bdaddr);
>> + skb = __hci_cmd_sync(hdev, HCI_VS_WRITE_BD_ADDR, sizeof(bdaddr_t),
>> + &bdaddr_swapped, HCI_INIT_TIMEOUT);
>> + if (!IS_ERR(skb))
>> + kfree_skb(skb);
>> +
>
> You have a trailing whitespace here.
>
> Does the HCI command really expect the BD_ADDR in the swapped order. The caller of hdev->set_bdaddr while provide it in the same order as the HCI Read BD Address command and everything in HCI. So it seems odd that you have to swap it for the vendor command.
>
> So have you actually tested this with btmgmt public-add <xx:xx..> and checked that the address comes out correctly. I think ll_set_bdaddr should function correctly for the mgmt interface. And if needed any other caller outside of mgmt has to do the swapping.
>
I did test using btmgmt public-address 00:11:22:33:44:55, which is how I
found out that the order needed to be swapped. Like you, I was
surprised. I couldn't find any documentation from TI saying what the
order is supposed to be, so I can only assume that because this works,
it is indeed correct as-is.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next] net: stmmac: fix broken dma_interrupt handling for multi-queues
From: David Miller @ 2017-12-08 19:19 UTC (permalink / raw)
To: niklas.cassel
Cc: Joao.Pinto, peppe.cavallaro, alexandre.torgue, niklass, netdev,
linux-kernel
In-Reply-To: <20171207225610.15572-1-niklas.cassel@axis.com>
From: Niklas Cassel <niklas.cassel@axis.com>
Date: Thu, 7 Dec 2017 23:56:10 +0100
> There is nothing that says that number of TX queues == number of RX
> queues. E.g. the ARTPEC-6 SoC has 2 TX queues and 1 RX queue.
>
> This code is obviously wrong:
> for (chan = 0; chan < tx_channel_count; chan++) {
> struct stmmac_rx_queue *rx_q = &priv->rx_queue[chan];
>
> priv->rx_queue has size MTL_MAX_RX_QUEUES, so this will send an
> uninitialized napi_struct to __napi_schedule(), causing us to
> crash in net_rx_action(), because napi_struct->poll is zero.
...
> Since each DMA channel can be used for rx and tx simultaneously,
> the current code should probably be rewritten so that napi_struct is
> embedded in a new struct stmmac_channel.
> That way, stmmac_poll() can call stmmac_tx_clean() on just the tx queue
> where we got the IRQ, instead of looping through all tx queues.
> This is also how the xgbe driver does it (another driver for this IP).
>
> Fixes: c22a3f48ef99 ("net: stmmac: adding multiple napi mechanism")
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
Applied, but indeed a lot more fixes are needed in this area.
^ permalink raw reply
* Re: [PATH net 0/4] tcp: RACK loss recovery bug fixes
From: David Miller @ 2017-12-08 19:14 UTC (permalink / raw)
To: ycheng; +Cc: netdev, edumazet, ncardwell, priyarjha
In-Reply-To: <20171207193333.59039-1-ycheng@google.com>
From: Yuchung Cheng <ycheng@google.com>
Date: Thu, 7 Dec 2017 11:33:29 -0800
> This patch set has four minor bug fixes in TCP RACK loss recovery.
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next] net: dsa: lan9303: Protect ALR operations with mutex
From: David Miller @ 2017-12-08 19:12 UTC (permalink / raw)
To: privat; +Cc: andrew, vivien.didelot, f.fainelli, netdev, linux-kernel
In-Reply-To: <20171207185604.30745-1-privat@egil-hjelmeland.no>
From: Egil Hjelmeland <privat@egil-hjelmeland.no>
Date: Thu, 7 Dec 2017 19:56:04 +0100
> ALR table operations are a sequence of related register operations which
> should be protected from concurrent access. The alr_cache should also be
> protected. Add alr_mutex doing that.
>
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
Applied.
^ permalink raw reply
* Re: [patch net-next] net: sched: fix use-after-free in tcf_block_put_ext
From: David Miller @ 2017-12-08 19:10 UTC (permalink / raw)
To: jiri; +Cc: netdev, jhs, xiyou.wangcong, mlxsw, code
In-Reply-To: <20171208182727.4100-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 8 Dec 2017 19:27:27 +0100
> From: Jiri Pirko <jiri@mellanox.com>
>
> Since the block is freed with last chain being put, once we reach the
> end of iteration of list_for_each_entry_safe, the block may be
> already freed. I'm hitting this only by creating and deleting clsact:
...
> Fix this by holding the block also by chain 0 and put chain 0
> explicitly, out of the list_for_each_entry_safe loop at the very
> end of tcf_block_put_ext.
>
> Fixes: efbf78973978 ("net_sched: get rid of rcu_barrier() in tcf_block_put_ext()")
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Applied, thanks Jiri.
^ permalink raw reply
* Re: [PATCH] bnxt_en: Fix sources of spurious netpoll warnings
From: David Miller @ 2017-12-08 19:08 UTC (permalink / raw)
To: michael.chan; +Cc: calvinowens, netdev, linux-kernel, kernel-team
In-Reply-To: <CACKFLinnDFOMCE5cx7hXs6DjjKmG7Bb65Nc41oMkpLdKCqHrYA@mail.gmail.com>
From: Michael Chan <michael.chan@broadcom.com>
Date: Fri, 8 Dec 2017 09:35:42 -0800
> On Fri, Dec 8, 2017 at 9:05 AM, Calvin Owens <calvinowens@fb.com> wrote:
>> After applying 2270bc5da3497945 ("bnxt_en: Fix netpoll handling") and
>> 903649e718f80da2 ("bnxt_en: Improve -ENOMEM logic in NAPI poll loop."),
>> we still see the following WARN fire:
...
>> This happens because we increment rx_pkts on -ENOMEM and -EIO, resulting
>> in rx_pkts > 0. Fix this by only bumping rx_pkts if we were actually
>> given a non-zero budget.
>>
>> Signed-off-by: Calvin Owens <calvinowens@fb.com>
>
> Thanks.
> Acked-by: Michael Chan <michael.chan@broadcom.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net-next 5/6] net: qualcomm: rmnet: Allow to configure flags for new devices
From: Subash Abhinov Kasiviswanathan @ 2017-12-08 18:59 UTC (permalink / raw)
To: Dan Williams; +Cc: davem, netdev
In-Reply-To: <1512750387.4390.3.camel@redhat.com>
On 2017-12-08 09:26, Dan Williams wrote:
> On Sun, 2017-12-03 at 23:37 -0700, Subash Abhinov Kasiviswanathan
> wrote:
>> Add an option to configure the rmnet aggregation and command features
>> on device creation. This is achieved by using the vlan flags option.
>
> Does this overload the VLAN flags item with different meanings than
> VLAN_FLAG_* that are specific to rmnet?
>
> Dan
Hi Dan
Yes, that's right. I am using it to configure rmnet specific features.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [net-next PATCH 00/14] lockless qdisc series
From: David Miller @ 2017-12-08 18:50 UTC (permalink / raw)
To: john.fastabend
Cc: willemdebruijn.kernel, daniel, eric.dumazet, netdev, jiri,
xiyou.wangcong
In-Reply-To: <20171207173500.5771.41198.stgit@john-Precision-Tower-5810>
From: John Fastabend <john.fastabend@gmail.com>
Date: Thu, 07 Dec 2017 09:53:46 -0800
> This series adds support for building lockless qdiscs.
Series applied, nice work.
I particularly like how you solved the GSO and bad_tx requeueing.
Thanks!
^ permalink raw reply
* Re: [iproute2] ss: print tcpi_rcv_ssthresh
From: Stephen Hemminger @ 2017-12-08 18:30 UTC (permalink / raw)
To: Wei Wang; +Cc: netdev, Eric Dumazet, Wei Wang
In-Reply-To: <20171208001200.40875-1-tracywwnj@gmail.com>
On Thu, 7 Dec 2017 16:12:00 -0800
Wei Wang <tracywwnj@gmail.com> wrote:
> From: Wei Wang <weiwan@google.com>
>
> tcpi_rcv_ssthresh is an important stats when debugging receive side
> behavior.
> Add it to the ss output.
>
> Signed-off-by: Wei Wang <weiwan@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied
^ permalink raw reply
* Re: [PATCH iproute2 net-next 0/4] Abstract columns, properly space and wrap fields
From: Stephen Hemminger @ 2017-12-08 18:29 UTC (permalink / raw)
To: Stefano Brivio; +Cc: netdev, Sabrina Dubroca
In-Reply-To: <cover.1512750298.git.sbrivio@redhat.com>
On Fri, 8 Dec 2017 18:07:19 +0100
Stefano Brivio <sbrivio@redhat.com> wrote:
> Currently, 'ss' simply subdivides the whole available screen width
> between available columns, starting from a set of hardcoded amount
> of spacing and growing column widths.
>
> This makes the output unreadable in several cases, as it doesn't take
> into account the actual content width.
>
> Fix this by introducing a simple abstraction for columns, buffering
> the output, measuring the width of the fields, grouping fields into
> lines as they fit, equally distributing any remaining whitespace, and
> finally rendering the result. Some examples are reported below [1].
>
> This implementation doesn't seem to cause any significant performance
> issues, as reported in 3/4.
>
> Patch 1/4 replaces all relevant printf() calls by the out() helper,
> which simply consists of the usual printf() implementation.
>
> Patch 2/4 implements column abstraction, with configurable column
> width and delimiters, and 3/4 splits buffering and rendering phases,
> employing a simple buffering mechanism with chunked allocation and
> introducing a rendering function.
>
> Up to this point, the output is still unchanged.
>
> Finally, 4/4 introduces field width calculation based on content
> length measured while buffering, in order to split fields onto
> multiple lines and equally space them within the single lines.
>
> Now that column behaviour is well-defined and more easily
> configurable, it should be easier to further improve the output by
> splitting logically separable information (e.g. TCP details) into
> additional columns. However, this patchset keeps the full "extended"
> information into a single column, for the moment being.
>
>
> [1]
>
> - 80 columns terminal, ss -Z -f netlink
> * before:
> Recv-Q Send-Q Local Address:Port Peer Address:Port
>
> 0 0 rtnl:evolution-calen/2075 * pr
> oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0 0 rtnl:abrt-applet/32700 * pr
> oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0 0 rtnl:firefox/21619 * pr
> oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0 0 rtnl:evolution-calen/32639 * p
> roc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> [...]
>
> * after:
> Recv-Q Send-Q Local Address:Port Peer Address:Port
> 0 0 rtnl:evolution-calen/2075 *
> proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0 0 rtnl:abrt-applet/32700 *
> proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0 0 rtnl:firefox/21619 *
> proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> 0 0 rtnl:evolution-calen/32639 *
> proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> [...]
>
> - 80 colums terminal, ss -tunpl
> * before:
> Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
> udp UNCONN 0 0 *:37732 *:*
> udp UNCONN 0 0 *:5353 *:*
> udp UNCONN 0 0 192.168.122.1:53 *:*
> udp UNCONN 0 0 *%virbr0:67 *:*
> [...]
>
> * after:
> Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
> udp UNCONN 0 0 *:37732 *:*
> udp UNCONN 0 0 *:5353 *:*
> udp UNCONN 0 0 192.168.122.1:53 *:*
> udp UNCONN 0 0 *%virbr0:67 *:*
> [...]
>
> - 66 columns terminal, ss -tunpl
> * before:
> Netid State Recv-Q Send-Q Local Address:Port P
> eer Address:Port
> udp UNCONN 0 0 *:37732 *:*
>
> udp UNCONN 0 0 *:5353 *:*
>
> udp UNCONN 0 0 192.168.122.1:53
> *:*
> udp UNCONN 0 0 *%virbr0:67 *:*
> [...]
>
> * after:
> Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
> udp UNCONN 0 0 *:37732 *:*
> udp UNCONN 0 0 *:5353 *:*
> udp UNCONN 0 0 192.168.122.1:53 *:*
> udp UNCONN 0 0 *%virbr0:67 *:*
> [...]
>
>
> Stefano Brivio (4):
> ss: Replace printf() calls for "main" output by calls to helper
> ss: Introduce columns lightweight abstraction
> ss: Buffer raw fields first, then render them as a table
> ss: Implement automatic column width calculation
>
> misc/ss.c | 893 +++++++++++++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 621 insertions(+), 272 deletions(-)
>
This looks good, would like some acknowledgment from heavy users such as Google
that this works for them.
^ permalink raw reply
* Re: [PATCH net 0/3] TCP BBR sampling fixes for loss recovery undo
From: David Miller @ 2017-12-08 18:28 UTC (permalink / raw)
To: ncardwell; +Cc: netdev
In-Reply-To: <20171207174332.17689-1-ncardwell@google.com>
From: Neal Cardwell <ncardwell@google.com>
Date: Thu, 7 Dec 2017 12:43:29 -0500
> This patch series has a few minor bug fixes for cases where spurious
> loss recoveries can trick BBR estimators into estimating that the
> available bandwidth is much lower than the true available bandwidth.
> In both cases the fix here is to just reset the estimator upon loss
> recovery undo.
Series applied and queued up for -stable, thanks Neal.
^ permalink raw reply
* [patch net-next] net: sched: fix use-after-free in tcf_block_put_ext
From: Jiri Pirko @ 2017-12-08 18:27 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, mlxsw, code
From: Jiri Pirko <jiri@mellanox.com>
Since the block is freed with last chain being put, once we reach the
end of iteration of list_for_each_entry_safe, the block may be
already freed. I'm hitting this only by creating and deleting clsact:
[ 202.171952] ==================================================================
[ 202.180182] BUG: KASAN: use-after-free in tcf_block_put_ext+0x240/0x390
[ 202.187590] Read of size 8 at addr ffff880225539a80 by task tc/796
[ 202.194508]
[ 202.196185] CPU: 0 PID: 796 Comm: tc Not tainted 4.15.0-rc2jiri+ #5
[ 202.203200] Hardware name: Mellanox Technologies Ltd. "MSN2100-CB2F"/"SA001017", BIOS 5.6.5 06/07/2016
[ 202.213613] Call Trace:
[ 202.216369] dump_stack+0xda/0x169
[ 202.220192] ? dma_virt_map_sg+0x147/0x147
[ 202.224790] ? show_regs_print_info+0x54/0x54
[ 202.229691] ? tcf_chain_destroy+0x1dc/0x250
[ 202.234494] print_address_description+0x83/0x3d0
[ 202.239781] ? tcf_block_put_ext+0x240/0x390
[ 202.244575] kasan_report+0x1ba/0x460
[ 202.248707] ? tcf_block_put_ext+0x240/0x390
[ 202.253518] tcf_block_put_ext+0x240/0x390
[ 202.258117] ? tcf_chain_flush+0x290/0x290
[ 202.262708] ? qdisc_hash_del+0x82/0x1a0
[ 202.267111] ? qdisc_hash_add+0x50/0x50
[ 202.271411] ? __lock_is_held+0x5f/0x1a0
[ 202.275843] clsact_destroy+0x3d/0x80 [sch_ingress]
[ 202.281323] qdisc_destroy+0xcb/0x240
[ 202.285445] qdisc_graft+0x216/0x7b0
[ 202.289497] tc_get_qdisc+0x260/0x560
Fix this by holding the block also by chain 0 and put chain 0
explicitly, out of the list_for_each_entry_safe loop at the very
end of tcf_block_put_ext.
Fixes: efbf78973978 ("net_sched: get rid of rcu_barrier() in tcf_block_put_ext()")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
net/sched/cls_api.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index d51051d..5b9b8a6 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -342,23 +342,24 @@ void tcf_block_put_ext(struct tcf_block *block, struct Qdisc *q,
{
struct tcf_chain *chain, *tmp;
- /* Hold a refcnt for all chains, except 0, so that they don't disappear
+ /* Hold a refcnt for all chains, so that they don't disappear
* while we are iterating.
*/
list_for_each_entry(chain, &block->chain_list, list)
- if (chain->index)
- tcf_chain_hold(chain);
+ tcf_chain_hold(chain);
list_for_each_entry(chain, &block->chain_list, list)
tcf_chain_flush(chain);
tcf_block_offload_unbind(block, q, ei);
- /* At this point, all the chains should have refcnt >= 1. Block will be
- * freed after all chains are gone.
- */
+ /* At this point, all the chains should have refcnt >= 1. */
list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
tcf_chain_put(chain);
+
+ /* Finally, put chain 0 and allow block to be freed. */
+ chain = list_first_entry(&block->chain_list, struct tcf_chain, list);
+ tcf_chain_put(chain);
}
EXPORT_SYMBOL(tcf_block_put_ext);
--
2.9.5
^ permalink raw reply related
* Re: [PATCH net] sfc: pass valid pointers from efx_enqueue_unwind
From: David Miller @ 2017-12-08 18:26 UTC (permalink / raw)
To: bkenward; +Cc: netdev, linux-net-drivers, jarod
In-Reply-To: <d8bd90b3-913e-bd8d-6c4e-8409ecaf9c1a@solarflare.com>
From: Bert Kenward <bkenward@solarflare.com>
Date: Thu, 7 Dec 2017 17:18:58 +0000
> The bytes_compl and pkts_compl pointers passed to efx_dequeue_buffers
> cannot be NULL. Add a paranoid warning to check this condition and fix
> the one case where they were NULL.
>
> efx_enqueue_unwind() is called very rarely, during error handling.
> Without this fix it would fail with a NULL pointer dereference in
> efx_dequeue_buffer, with efx_enqueue_skb in the call stack.
>
> Fixes: e9117e5099ea ("sfc: Firmware-Assisted TSO version 2")
> Reported-by: Jarod Wilson <jarod@redhat.com>
> Signed-off-by: Bert Kenward <bkenward@solarflare.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net] gianfar: Disable EEE autoneg by default
From: David Miller @ 2017-12-08 18:23 UTC (permalink / raw)
To: claudiu.manoil; +Cc: netdev, andrew, Shaohui.Xie
In-Reply-To: <1512665063-1680-1-git-send-email-claudiu.manoil@nxp.com>
From: Claudiu Manoil <claudiu.manoil@nxp.com>
Date: Thu, 7 Dec 2017 18:44:23 +0200
> This controller does not support EEE, but it may connect to a PHY
> which supports EEE and advertises EEE by default, while its link
> partner also advertises EEE. If this happens, the PHY enters low
> power mode when the traffic rate is low and causes packet loss.
> This patch disables EEE advertisement by default for any PHY that
> gianfar connects to, to prevent the above unwanted outcome.
>
> Signed-off-by: Shaohui Xie <Shaohui.Xie@nxp.com>
> Tested-by: Yangbo Lu <Yangbo.lu@nxp.com>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: Disable interrupts if napi_complete_done rescheduled napi
From: David Miller @ 2017-12-08 18:19 UTC (permalink / raw)
To: jasowang; +Cc: mst, makita.toshiaki, netdev, virtualization, edumazet
In-Reply-To: <ffcf1aa3-403e-9ff0-bd51-c507d5a52991@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Thu, 7 Dec 2017 15:08:45 +0800
>
>
> On 2017年12月07日 13:09, Michael S. Tsirkin wrote:
>> On Thu, Dec 07, 2017 at 01:15:15PM +0900, Toshiaki Makita wrote:
>>> Since commit 39e6c8208d7b ("net: solve a NAPI race") napi has been
>>> able
>>> to be rescheduled within napi_complete_done() even in non-busypoll
>>> case,
>>> but virtnet_poll() always enabled interrupts before complete, and when
>>> napi was rescheduled within napi_complete_done() it did not disable
>>> interrupts.
>>> This caused more interrupts when event idx is disabled.
>>>
>>> According to commit cbdadbbf0c79 ("virtio_net: fix race in RX VQ
>>> processing") we cannot place virtqueue_enable_cb_prepare() after
>>> NAPI_STATE_SCHED is cleared, so disable interrupts again if
>>> napi_complete_done() returned false.
>>>
>>> Tested with vhost-user of OVS 2.7 on host, which does not have the
>>> event
>>> idx feature.
...
>>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>>
>> it might make sense in net and not -next since tx napi regressed
>> performance
>> in some configs, this might bring it back at least partially.
>> Jason - what do you think?
>
> No sure, the regression I saw was tested with event idx on. And
> virtqueue_disable_cb() does almost nothing for event idx (or even a
> little bit slower).
>
> The patch looks good.
>
> Acked-by: Jason Wang <jasowang@redhat.com>
I'm going to put this into net-next for now.
^ permalink raw reply
* Re: WireGuard Upstreaming Roadmap (November 2017)
From: Jason A. Donenfeld @ 2017-12-08 18:19 UTC (permalink / raw)
To: David Miller; +Cc: Netdev, WireGuard mailing list, LKML
In-Reply-To: <20171208.103841.516344129530992484.davem@davemloft.net>
Hi Dave,
On Fri, Dec 8, 2017 at 4:38 PM, David Miller <davem@davemloft.net> wrote:
> Sorry, you cannot force the discussion of a feature which will be submitted
> upstream to occur on a private mailing list.
>
> It is _ABSOLUTELY_ appropriate to discss this on netdev since it is the
> netdev community which must consider issues like this when looking at
> whether to accept WireGuard upstream.
>
> Jason, this action and response was entirely inappropriate, and please
> I'd like you to reply properly to questions about your feature here.
Whoops, okay! Very sorry. I'm actually kind of happy to hear that. I
had assumed that you'd be annoyed if WireGuard crypto discussion
spewed over into netdev adding even more message volume there for
something perhaps not directly relevant. But in fact, you're
interested and find it important to discuss there. So, good news. And
sorry for trying to shew it away before. I'll copy and paste the
response I had made on the other list:
> This is covered in the paper:
> https://www.wireguard.com/papers/wireguard.pdf
>
> The basic answer is that WireGuard has message type identifiers, and
> the handshake also hashes into it an identifier of the primitives
> used. If there's ever a problem with those primitives chosen, it will
> be possible to introduce new message type identifiers, if that kind of
> "support everything even the broken garbage" approach is desired by
> misguided people. However, a better approach, of course, is to keep
> your secure network separate from your insecure network, and to not
> allow insecure nodes on secure segments; when you mix the two,
> disaster tends to strike. So, in other words, both approaches to "upgrading"
> are possible, in this fantasy wireguardalypse. Take note, though, that
> neither one of these approaches (support new and retain old protocol
> too for old nodes, or only support new) are "agile" or are anything at
> all like the 90s "cipher agility" -- the user still is not permitted
> to "choose" ciphers.
Regards,
Jason
^ permalink raw reply
* Re: [PATCH] ptr_ring: add barriers
From: David Miller @ 2017-12-08 18:08 UTC (permalink / raw)
To: mst
Cc: linux-kernel, george.cherian, jasowang, edumazet, netdev,
virtualization
In-Reply-To: <1512501990-30029-1-git-send-email-mst@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Tue, 5 Dec 2017 21:29:37 +0200
> Users of ptr_ring expect that it's safe to give the
> data structure a pointer and have it be available
> to consumers, but that actually requires an smb_wmb
> or a stronger barrier.
>
> In absence of such barriers and on architectures that reorder writes,
> consumer might read an un=initialized value from an skb pointer stored
> in the skb array. This was observed causing crashes.
>
> To fix, add memory barriers. The barrier we use is a wmb, the
> assumption being that producers do not need to read the value so we do
> not need to order these reads.
>
> Reported-by: George Cherian <george.cherian@cavium.com>
> Suggested-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> George, could you pls report whether this patch fixes
> the issue for you?
>
> This seems to be needed in stable as well.
I really need some testing feedback for this before I apply it
and queue it up for -stable.
George?
^ permalink raw reply
* Re: [PATCHv2 net-next 04/12] sctp: implement make_datafrag for sctp_stream_interleave
From: Xin Long @ 2017-12-08 17:37 UTC (permalink / raw)
To: David Laight
Cc: Marcelo Ricardo Leitner, Neil Horman, network dev,
linux-sctp@vger.kernel.org, davem@davemloft.net
In-Reply-To: <b727ce7a5f8746219987775dab5d1e1a@AcuMS.aculab.com>
On Sat, Dec 9, 2017 at 1:29 AM, David Laight <David.Laight@aculab.com> wrote:
> From: Xin Long ...
>> Hi, David, Sorry, I'm not sure we're worrying about the cpu cost or
>> codes style now ?
>>
>> For cpu cost, I think 0x848(%r13) operation must be better than the
>> generated code of if-else.
>
> Nope - the call xxx(%ryyy) is likely to be a data cache miss and a complete
> cpu pipeline stall.
>
> The conditional will be a data cache hit and the code (for one branch)
> will be prefetched and speculatively executed.
>
> Some very modern cpu might manage to predict indirect jumps, but for
> most it is a full pipeline stall.
Thanks for the CPU information.
The thing is with if-else can't avoid xxx(%ryyy) in this case, as Marcelo
said above. It seems if-else will just be a extra cost compare to this one.
^ permalink raw reply
* Re: [PATCH] bnxt_en: Fix sources of spurious netpoll warnings
From: Michael Chan @ 2017-12-08 17:35 UTC (permalink / raw)
To: Calvin Owens; +Cc: Netdev, open list, kernel-team
In-Reply-To: <1d2d29c336343cc590c8ba31156511381c39fc84.1512511928.git.calvinowens@fb.com>
On Fri, Dec 8, 2017 at 9:05 AM, Calvin Owens <calvinowens@fb.com> wrote:
> After applying 2270bc5da3497945 ("bnxt_en: Fix netpoll handling") and
> 903649e718f80da2 ("bnxt_en: Improve -ENOMEM logic in NAPI poll loop."),
> we still see the following WARN fire:
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1875170 at net/core/netpoll.c:165 netpoll_poll_dev+0x15a/0x160
> bnxt_poll+0x0/0xd0 exceeded budget in poll
> <snip>
> Call Trace:
> [<ffffffff814be5cd>] dump_stack+0x4d/0x70
> [<ffffffff8107e013>] __warn+0xd3/0xf0
> [<ffffffff8107e07f>] warn_slowpath_fmt+0x4f/0x60
> [<ffffffff8179519a>] netpoll_poll_dev+0x15a/0x160
> [<ffffffff81795f38>] netpoll_send_skb_on_dev+0x168/0x250
> [<ffffffff817962fc>] netpoll_send_udp+0x2dc/0x440
> [<ffffffff815fa9be>] write_ext_msg+0x20e/0x250
> [<ffffffff810c8125>] call_console_drivers.constprop.23+0xa5/0x110
> [<ffffffff810c9549>] console_unlock+0x339/0x5b0
> [<ffffffff810c9a88>] vprintk_emit+0x2c8/0x450
> [<ffffffff810c9d5f>] vprintk_default+0x1f/0x30
> [<ffffffff81173df5>] printk+0x48/0x50
> [<ffffffffa0197713>] edac_raw_mc_handle_error+0x563/0x5c0 [edac_core]
> [<ffffffffa0197b9b>] edac_mc_handle_error+0x42b/0x6e0 [edac_core]
> [<ffffffffa01c3a60>] sbridge_mce_output_error+0x410/0x10d0 [sb_edac]
> [<ffffffffa01c47cc>] sbridge_check_error+0xac/0x130 [sb_edac]
> [<ffffffffa0197f3c>] edac_mc_workq_function+0x3c/0x90 [edac_core]
> [<ffffffff81095f8b>] process_one_work+0x19b/0x480
> [<ffffffff810967ca>] worker_thread+0x6a/0x520
> [<ffffffff8109c7c4>] kthread+0xe4/0x100
> [<ffffffff81884c52>] ret_from_fork+0x22/0x40
>
> This happens because we increment rx_pkts on -ENOMEM and -EIO, resulting
> in rx_pkts > 0. Fix this by only bumping rx_pkts if we were actually
> given a non-zero budget.
>
> Signed-off-by: Calvin Owens <calvinowens@fb.com>
Thanks.
Acked-by: Michael Chan <michael.chan@broadcom.com>
^ permalink raw reply
* RE: [PATCHv2 net-next 04/12] sctp: implement make_datafrag for sctp_stream_interleave
From: David Laight @ 2017-12-08 17:29 UTC (permalink / raw)
To: 'Xin Long'
Cc: Marcelo Ricardo Leitner, Neil Horman, network dev,
linux-sctp@vger.kernel.org, davem@davemloft.net
In-Reply-To: <CADvbK_ct1cGSFZ68RaTh8=T6-Z8v0nFSruMti-L2fA393ti9pA@mail.gmail.com>
From: Xin Long ...
> Hi, David, Sorry, I'm not sure we're worrying about the cpu cost or
> codes style now ?
>
> For cpu cost, I think 0x848(%r13) operation must be better than the
> generated code of if-else.
Nope - the call xxx(%ryyy) is likely to be a data cache miss and a complete
cpu pipeline stall.
The conditional will be a data cache hit and the code (for one branch)
will be prefetched and speculatively executed.
Some very modern cpu might manage to predict indirect jumps, but for
most it is a full pipeline stall.
David
^ permalink raw reply
* Re: [PATCHv2 net-next 04/12] sctp: implement make_datafrag for sctp_stream_interleave
From: Xin Long @ 2017-12-08 17:23 UTC (permalink / raw)
To: David Laight
Cc: Marcelo Ricardo Leitner, Neil Horman, network dev,
linux-sctp@vger.kernel.org, davem@davemloft.net
In-Reply-To: <27aff622f1574b329e18ba21922f6e7e@AcuMS.aculab.com>
On Sat, Dec 9, 2017 at 12:22 AM, David Laight <David.Laight@aculab.com> wrote:
> From: Xin Long
>> Sent: 08 December 2017 16:18
>>
> ...
>> >> Alternatively you could preform the dereference in two steps (i.e. declare an si
>> >> pointer on the stack and set it equal to asoc->stream.si, then deref
>> >> si->make_datafrag at call time. That will at least give the compiler an
>> >> opportunity to preload the first pointer.
>
> You want to save the function pointer itself.
>
> ...
>> Another small difference:
>> as you can see, comparing to (X), (Y) is using 0x28(%rsp) in the loop,
>> instead of %r13.
>>
>> So that's what I can see from the related generated code.
>> If 0x848(%r13) is not worse than 0x28(%rsp) for cpu, I think
>> asoc->stream.si->make_datafrag() is even better. No ?
>
> That code must have far too many life local variables.
> Otherwise there's be a caller saved register available.
>
Hi, David, Sorry, I'm not sure we're worrying about the cpu cost or
codes style now ?
For cpu cost, I think 0x848(%r13) operation must be better than the
generated code of if-else.
For the codes style, comparing to the if-else, I think this one is
more readable. (ignore extendible stuff first, as probably no more
new type of data chunk).
^ 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