* [PATCH v2 net-next] liquidio: remove set but not used variable 'is25G'
From: YueHaibing @ 2018-08-13 9:29 UTC (permalink / raw)
To: davem, derek.chickles, satananda.burla, felix.manlunas,
raghu.vatsavayi
Cc: linux-kernel, netdev, YueHaibing
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/net/ethernet/cavium/liquidio/lio_ethtool.c: In function 'lio_set_link_ksettings':
drivers/net/ethernet/cavium/liquidio/lio_ethtool.c:392:6: warning:
variable 'is25G' set but not used [-Wunused-but-set-variable]
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
v2: remove unnecessary braces as Shahed suggested
---
drivers/net/ethernet/cavium/liquidio/lio_ethtool.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c b/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c
index 807ea2c..8e05afd 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c
@@ -389,18 +389,14 @@ static int lio_set_link_ksettings(struct net_device *netdev,
struct lio *lio = GET_LIO(netdev);
struct oct_link_info *linfo;
struct octeon_device *oct;
- u32 is25G = 0;
oct = lio->oct_dev;
linfo = &lio->linfo;
- if (oct->subsystem_id == OCTEON_CN2350_25GB_SUBSYS_ID ||
- oct->subsystem_id == OCTEON_CN2360_25GB_SUBSYS_ID) {
- is25G = 1;
- } else {
+ if (!(oct->subsystem_id == OCTEON_CN2350_25GB_SUBSYS_ID ||
+ oct->subsystem_id == OCTEON_CN2360_25GB_SUBSYS_ID))
return -EOPNOTSUPP;
- }
if (oct->no_speed_setting) {
dev_err(&oct->pci_dev->dev, "%s: Changing speed is not supported\n",
--
2.7.0
^ permalink raw reply related
* Re: [PATCH net-next] virtio_net: remove duplicated include from virtio_net.c
From: Michael S. Tsirkin @ 2018-08-13 9:42 UTC (permalink / raw)
To: YueHaibing; +Cc: netdev, virtualization, davem, linux-kernel
In-Reply-To: <20180813061315.9084-1-yuehaibing@huawei.com>
On Mon, Aug 13, 2018 at 02:13:15PM +0800, YueHaibing wrote:
> Remove duplicated include linux/netdevice.h
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index eb00ae6..7659209 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -30,7 +30,6 @@
> #include <linux/cpu.h>
> #include <linux/average.h>
> #include <linux/filter.h>
> -#include <linux/netdevice.h>
> #include <linux/kernel.h>
> #include <linux/pci.h>
> #include <net/route.h>
> --
> 2.7.0
>
^ permalink raw reply
* [PATCH bpf-next V2] net/xdp: Fix suspicious RCU usage warning
From: Tariq Toukan @ 2018-08-13 7:04 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: netdev, Eran Ben Elisha, Tariq Toukan, Jesper Dangaard Brouer
Fix the warning below by calling rhashtable_lookup_fast.
Also, make some code movements for better quality and human
readability.
[ 342.450870] WARNING: suspicious RCU usage
[ 342.455856] 4.18.0-rc2+ #17 Tainted: G O
[ 342.462210] -----------------------------
[ 342.467202] ./include/linux/rhashtable.h:481 suspicious rcu_dereference_check() usage!
[ 342.476568]
[ 342.476568] other info that might help us debug this:
[ 342.476568]
[ 342.486978]
[ 342.486978] rcu_scheduler_active = 2, debug_locks = 1
[ 342.495211] 4 locks held by modprobe/3934:
[ 342.500265] #0: 00000000e23116b2 (mlx5_intf_mutex){+.+.}, at:
mlx5_unregister_interface+0x18/0x90 [mlx5_core]
[ 342.511953] #1: 00000000ca16db96 (rtnl_mutex){+.+.}, at: unregister_netdev+0xe/0x20
[ 342.521109] #2: 00000000a46e2c4b (&priv->state_lock){+.+.}, at: mlx5e_close+0x29/0x60
[mlx5_core]
[ 342.531642] #3: 0000000060c5bde3 (mem_id_lock){+.+.}, at: xdp_rxq_info_unreg+0x93/0x6b0
[ 342.541206]
[ 342.541206] stack backtrace:
[ 342.547075] CPU: 12 PID: 3934 Comm: modprobe Tainted: G O 4.18.0-rc2+ #17
[ 342.556621] Hardware name: Dell Inc. PowerEdge R730/0H21J3, BIOS 1.5.4 10/002/2015
[ 342.565606] Call Trace:
[ 342.568861] dump_stack+0x78/0xb3
[ 342.573086] xdp_rxq_info_unreg+0x3f5/0x6b0
[ 342.578285] ? __call_rcu+0x220/0x300
[ 342.582911] mlx5e_free_rq+0x38/0xc0 [mlx5_core]
[ 342.588602] mlx5e_close_channel+0x20/0x120 [mlx5_core]
[ 342.594976] mlx5e_close_channels+0x26/0x40 [mlx5_core]
[ 342.601345] mlx5e_close_locked+0x44/0x50 [mlx5_core]
[ 342.607519] mlx5e_close+0x42/0x60 [mlx5_core]
[ 342.613005] __dev_close_many+0xb1/0x120
[ 342.617911] dev_close_many+0xa2/0x170
[ 342.622622] rollback_registered_many+0x148/0x460
[ 342.628401] ? __lock_acquire+0x48d/0x11b0
[ 342.633498] ? unregister_netdev+0xe/0x20
[ 342.638495] rollback_registered+0x56/0x90
[ 342.643588] unregister_netdevice_queue+0x7e/0x100
[ 342.649461] unregister_netdev+0x18/0x20
[ 342.654362] mlx5e_remove+0x2a/0x50 [mlx5_core]
[ 342.659944] mlx5_remove_device+0xe5/0x110 [mlx5_core]
[ 342.666208] mlx5_unregister_interface+0x39/0x90 [mlx5_core]
[ 342.673038] cleanup+0x5/0xbfc [mlx5_core]
[ 342.678094] __x64_sys_delete_module+0x16b/0x240
[ 342.683725] ? do_syscall_64+0x1c/0x210
[ 342.688476] do_syscall_64+0x5a/0x210
[ 342.693025] entry_SYSCALL_64_after_hwframe+0x49/0xbe
Fixes: 8d5d88527587 ("xdp: rhashtable with allocator ID to pointer mapping")
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
---
net/core/xdp.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
V1 -> V2:
* Use rhashtable_lookup_fast and make some code movements, per Daniel's
and Alexei's comments.
Please queue to -stable v4.18.
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 3dd99e1c04f5..8b1c7b699982 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -105,16 +105,9 @@ static void __xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
mutex_lock(&mem_id_lock);
- xa = rhashtable_lookup(mem_id_ht, &id, mem_id_rht_params);
- if (!xa) {
- mutex_unlock(&mem_id_lock);
- return;
- }
-
- err = rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params);
- WARN_ON(err);
-
- call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
+ xa = rhashtable_lookup_fast(mem_id_ht, &id, mem_id_rht_params);
+ if (xa && rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params))
+ call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
mutex_unlock(&mem_id_lock);
}
--
1.8.3.1
^ permalink raw reply related
* the mobile apps
From: Jack Dike @ 2018-08-13 6:14 UTC (permalink / raw)
To: netdev
Do you need mobile apps development? We can do it for you.
We are an India base company. Here are the details about us:
Years in business: 8
Staffs: 125
App developed: 350
We work on Android, iOS, Ionic, and PhoneGap platforms, we have clients
across different kind of industries.
Such like retail, media and entertainment, BFSI, hospitality, social media,
eCommerce, food and beverages, etc.
We do below:
Mobile Apps
Mobile App UI/UX designing
App Maintenance and Support
Website or ecommerce portal development
Please reply back if you are interested in what we do.
We will share our portfolios to you.
Regards,
Jack
^ permalink raw reply
* you need mobile apps
From: Jack Dike @ 2018-08-13 5:38 UTC (permalink / raw)
To: netdev
Do you need mobile apps development? We can do it for you.
We are an India base company. Here are the details about us:
Years in business: 8
Staffs: 125
App developed: 350
We work on Android, iOS, Ionic, and PhoneGap platforms, we have clients
across different kind of industries.
Such like retail, media and entertainment, BFSI, hospitality, social media,
eCommerce, food and beverages, etc.
We do below:
Mobile Apps
Mobile App UI/UX designing
App Maintenance and Support
Website or ecommerce portal development
Please reply back if you are interested in what we do.
We will share our portfolios to you.
Regards,
Jack
^ permalink raw reply
* Re: [PATCH net-next v6 10/11] net: sched: atomically check-allocate action
From: Vlad Buslov @ 2018-08-13 7:55 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Yevgeny Kliteynik, Jiri Pirko
In-Reply-To: <CAM_iQpW2GAFY84K9pDjcXiRB9VVYKdEHBU6mGu83hLyexahaqw@mail.gmail.com>
On Fri 10 Aug 2018 at 21:45, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Fri, Aug 10, 2018 at 3:29 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>>
>> Approach you suggest is valid, but has its own trade-offs:
>>
>> - As you noted, lock granularity becomes coarse-grained due to per-netns
>> scope.
>
> Sure, you acquire idrinfo->lock too, the only difference is how long
> you take it.
>
> The bottleneck of your approach is the same, also you take idrinfo->lock
> twice, so the contention is heavier.
>
>
>>
>> - I am not sure it is possible to call idr_replace() without obtaining
>> idrinfo->lock in this particular case. Concurrent delete of action with
>> same id is possible and, according to idr_replace() description,
>> unlocked execution is not supported for such use-case:
>
> But we can hold its refcnt before releasing idrinfo->lock, so
> idr_replace() can't race with concurrent delete.
Yes, for concurrent delete case I agree. Action is removed from idr only
when last reference is released and, in case of existing action update,
init holds a reference.
What about case when multiple task race to update the same existing
action? I assume idr_replace() can be used for such case, but what would
be the algorithm in case init replaced some other action, and not the
action it actually copied before calling idr_replace()?
>
>
>>
>> - High rate or replace request will generate a lot of unnecessary memory
>> allocations and deallocations.
>>
>
> Yes, this is literally how RCU works, always allocate and copy,
> release upon error.
>
> Also, if this is really a problem, we have SLAB_TYPESAFE_BY_RCU
> too. ;)
Current action update implementation is in-place, so there is no "copy"
stage, besides members of some actions that are RCU-pointers. But I
guess it makes sense if your goal is to refactor all actions to be
updated with RCU.
^ permalink raw reply
* RE: [PATCH] net/phy: Micrel KSZ8061 PHY link failure after cable connect
From: Onnasch, Alexander (EXT) @ 2018-08-13 10:41 UTC (permalink / raw)
To: David Miller
Cc: andrew@lunn.ch, f.fainelli@gmail.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20180730.094534.351189879060038512.davem@davemloft.net>
-----Original Message-----
From: David Miller <davem@davemloft.net>
Sent: Montag, 30. Juli 2018 18:46
To: Onnasch, Alexander (EXT) <Alexander.Onnasch@landisgyr.com>
Cc: andrew@lunn.ch; f.fainelli@gmail.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net/phy: Micrel KSZ8061 PHY link failure after cable connect
From: Alexander Onnasch <alexander.onnasch@landisgyr.com>
Date: Mon, 30 Jul 2018 16:01:27 +0200
> With Micrel KSZ8061 PHY, the link may occasionally not come up after
> Ethernet cable connect. The vendor's (Microchip, former Micrel) errata
> sheet 80000688A.pdf describes the problem and possible workarounds in
> detail, see below.
> The patch implements workaround 1, which permanently fixes the issue.
...
> Signed-off-by: Alexander Onnasch <alexander.onnasch@landisgyr.com>
This patch does not apply cleanly to any of my neworking trees.
Hello David
thanks a lot for reviewing ! How to proceed now ? If there is action required from my side please tell me.
best regards,
Alex
Alexander Onnasch | Landis+Gyr AG | Firmware Engineer
Phone: +41 41 935 6357 | Fax: +41 41 935 6211
Email: alexander.onnasch@landisgyr.com | Site: www.landisgyr.com | Address: Theilerstr. 1, CH-6301 Zug, Switzerland
manage energy better
^ permalink raw reply
* Re: [PATCH] mt76: Enable NL80211_EXT_FEATURE_CQM_RSSI_LIST
From: Lorenzo Bianconi @ 2018-08-13 10:55 UTC (permalink / raw)
To: arend.vanspriel-dY08KVG/lbpWk0Htik3J/w
Cc: Kalle Valo, kristian.evensen-Re5JQEeQqe8AvxtiuMwx3w,
linux-wireless, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5B708025.4090906-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> On 8/12/2018 8:14 PM, Kalle Valo wrote:
> > Kristian Evensen <kristian.evensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> >
> >> Enable the use of CQM with mt76-devices.
> >>
> >> Signed-off-by: Kristian Evensen <kristian.evensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> ---
> >> drivers/net/wireless/mediatek/mt76/mac80211.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
> >> index 029d54bc..3eb328ff 100644
> >> --- a/drivers/net/wireless/mediatek/mt76/mac80211.c
> >> +++ b/drivers/net/wireless/mediatek/mt76/mac80211.c
> >> @@ -305,6 +305,8 @@ int mt76_register_device(struct mt76_dev *dev, bool vht,
> >>
> >> wiphy->features |= NL80211_FEATURE_ACTIVE_MONITOR;
> >>
> >> + wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
> >
> > So have you tested this and with what devices? For example, does it work
> > with recently added USB devices?
>
> I was looking into this as it looks suspicious to me. From reading the
> description of this ext_feature flag it seems this is an extention of CQM:
>
> """
> * @NL80211_EXT_FEATURE_CQM_RSSI_LIST: With this driver the
> * %NL80211_ATTR_CQM_RSSI_THOLD attribute accepts a list of zero or more
> * RSSI threshold values to monitor rather than exactly one threshold.
> """
>
> Also looking at mt76x2_bss_info_changed() it does not handle
> BSS_CHANGED_CQM so I doubt it has support for it (yet). The driver does
> not use IEEE80211_VIF_SUPPORTS_CQM_RSSI which is a requirement for it.
>
> Regards,
> Arend
>
According to my understanding (please correct me if I am wrong)
BSS_CHANGED_CQM is only needed if CQM_RSSI is handled
by the driver/fw, while if it is not set mac80211 will take care of that
in ieee80211_handle_beacon_sig routine.
I am AFK at the moment, I will test that patch when I am back from vacations.
Regards,
Lorenzo
>
> https://elixir.bootlin.com/linux/latest/source/drivers/net/wireless/mediatek/mt76/mt76x2_main.c#L223
>
^ permalink raw reply
* Re: [PATCH] net: macb: do not disable MDIO bus when closing interface
From: Claudiu Beznea @ 2018-08-13 9:08 UTC (permalink / raw)
To: Anssi Hannula, Andrew Lunn; +Cc: Nicolas Ferre, David S. Miller, netdev
In-Reply-To: <edd7aaf4-53dc-6d10-5c4b-2928a4612d99@bitwise.fi>
On 10.08.2018 09:22, Anssi Hannula wrote:
> On 9.8.2018 18:14, Andrew Lunn wrote:
>> Hi Anssi
>
> Hi!
>
>>> macb_reset_hw() is called in init path too,
I only see it in macb_close() and macb_open() called from macb_init_hw().
though, so maybe clearing
>>> all bits is intentional / wanted to get the controller to a known state,
As far as I know the NCR is used in this driver only to control transmit
enable, receive enable, MPE and tx start.
Not clearing NCR in the init phase should not have any impact on IP. Same
for not clearing Tx start on "if down" path (if tx is in progress).
>>> even though the comment only mentions TX/RX?
>> You need to be careful here. Once of_mdiobus_register() is called, the
>> MDIO should be usable. If you happen to have an Ethernet switch on the
>> bus, it could be probed then. The DSA driver will start using the bus.
>> Or if you have a second PHY, connected to some other MAC, it could be
>> used by the other MAC. This all happens in the macb_probe function.
>>
>> Sometime later, the interface will be up'ed. At this point macb_open()
>> is called, which calls macb_init_hw(), which calls
>> macb_reset_hw(). What you don't want happening is changes to the NCR
>> at this point breaking an MDIO transaction which might be going on.
>>
>> Ideally, the MPE should be enabled before of_mdiobus_register(), and
>> left alone until mdiobus_unregister() is called in macb_remove().
>
> Yep, fixing the use case of having PHYs of other MACs is why I wrote the
> patch :)
>
> Currently the reset code disables MPE while other MACs are using PHYs on
> the bus.
MPE is set in the early phase of initialization, in macb_mii_init(), called
from probe() function. Due to the fact that NCR is cleared in
macb_reset_hw() it is set again at the end of macb_init_hw(). So, as per
Andrew comments in this thread, setting it in macb_mii_init() and letting
it unchanged until remove would be the best. So, if you clear only TE and
RE bits in macb_reset_hw() please also remove MACB_BIT(MPE) from below line
at the end of macb_init_hw():
macb_writel(bp, NCR, MACB_BIT(RE) | MACB_BIT(TE) | MACB_BIT(MPE));
Thank you,
Claudiu Beznea
>
^ permalink raw reply
* RE: [PATCH net-next] liquidio: remove set but not used variable 'is25G'
From: Shaikh, Shahed @ 2018-08-13 9:08 UTC (permalink / raw)
To: YueHaibing, davem@davemloft.net, Chickles, Derek,
Burla, Satananda, Manlunas, Felix, Vatsavayi, Raghu
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20180813072118.12448-1-yuehaibing@huawei.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of YueHaibing
> Sent: Monday, August 13, 2018 12:51 PM
> To: davem@davemloft.net; Chickles, Derek <Derek.Chickles@cavium.com>;
> Burla, Satananda <Satananda.Burla@cavium.com>; Manlunas, Felix
> <Felix.Manlunas@cavium.com>; Vatsavayi, Raghu
> <Raghu.Vatsavayi@cavium.com>
> Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org; YueHaibing
> <yuehaibing@huawei.com>
> Subject: [PATCH net-next] liquidio: remove set but not used variable 'is25G'
>
> External Email
>
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/ethernet/cavium/liquidio/lio_ethtool.c: In function
> 'lio_set_link_ksettings':
> drivers/net/ethernet/cavium/liquidio/lio_ethtool.c:392:6: warning:
> variable 'is25G' set but not used [-Wunused-but-set-variable]
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> drivers/net/ethernet/cavium/liquidio/lio_ethtool.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c
> b/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c
> index 807ea2c..5ce604a 100644
> --- a/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c
> +++ b/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c
> @@ -389,16 +389,13 @@ static int lio_set_link_ksettings(struct net_device
> *netdev,
> struct lio *lio = GET_LIO(netdev);
> struct oct_link_info *linfo;
> struct octeon_device *oct;
> - u32 is25G = 0;
>
> oct = lio->oct_dev;
>
> linfo = &lio->linfo;
>
> - if (oct->subsystem_id == OCTEON_CN2350_25GB_SUBSYS_ID ||
> - oct->subsystem_id == OCTEON_CN2360_25GB_SUBSYS_ID) {
> - is25G = 1;
> - } else {
> + if (!(oct->subsystem_id == OCTEON_CN2350_25GB_SUBSYS_ID ||
> + oct->subsystem_id == OCTEON_CN2360_25GB_SUBSYS_ID)) {
> return -EOPNOTSUPP;
> }
You can also remove braces which are not required for single line.
Thanks,
Shahed
^ permalink raw reply
* Re: [PATCH bpf-next V2] net/xdp: Fix suspicious RCU usage warning
From: Jesper Dangaard Brouer @ 2018-08-13 9:13 UTC (permalink / raw)
To: Tariq Toukan
Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Eran Ben Elisha,
brouer
In-Reply-To: <1534143879-8380-1-git-send-email-tariqt@mellanox.com>
On Mon, 13 Aug 2018 10:04:39 +0300
Tariq Toukan <tariqt@mellanox.com> wrote:
> Fix the warning below by calling rhashtable_lookup_fast.
> Also, make some code movements for better quality and human
> readability.
>
> [ 342.450870] WARNING: suspicious RCU usage
> [ 342.455856] 4.18.0-rc2+ #17 Tainted: G O
> [ 342.462210] -----------------------------
> [ 342.467202] ./include/linux/rhashtable.h:481 suspicious rcu_dereference_check() usage!
> [ 342.476568]
> [ 342.476568] other info that might help us debug this:
> [ 342.476568]
> [ 342.486978]
> [ 342.486978] rcu_scheduler_active = 2, debug_locks = 1
> [ 342.495211] 4 locks held by modprobe/3934:
> [ 342.500265] #0: 00000000e23116b2 (mlx5_intf_mutex){+.+.}, at:
> mlx5_unregister_interface+0x18/0x90 [mlx5_core]
> [ 342.511953] #1: 00000000ca16db96 (rtnl_mutex){+.+.}, at: unregister_netdev+0xe/0x20
> [ 342.521109] #2: 00000000a46e2c4b (&priv->state_lock){+.+.}, at: mlx5e_close+0x29/0x60
> [mlx5_core]
> [ 342.531642] #3: 0000000060c5bde3 (mem_id_lock){+.+.}, at: xdp_rxq_info_unreg+0x93/0x6b0
> [ 342.541206]
> [ 342.541206] stack backtrace:
> [ 342.547075] CPU: 12 PID: 3934 Comm: modprobe Tainted: G O 4.18.0-rc2+ #17
> [ 342.556621] Hardware name: Dell Inc. PowerEdge R730/0H21J3, BIOS 1.5.4 10/002/2015
> [ 342.565606] Call Trace:
> [ 342.568861] dump_stack+0x78/0xb3
> [ 342.573086] xdp_rxq_info_unreg+0x3f5/0x6b0
> [ 342.578285] ? __call_rcu+0x220/0x300
> [ 342.582911] mlx5e_free_rq+0x38/0xc0 [mlx5_core]
> [ 342.588602] mlx5e_close_channel+0x20/0x120 [mlx5_core]
> [ 342.594976] mlx5e_close_channels+0x26/0x40 [mlx5_core]
> [ 342.601345] mlx5e_close_locked+0x44/0x50 [mlx5_core]
> [ 342.607519] mlx5e_close+0x42/0x60 [mlx5_core]
> [ 342.613005] __dev_close_many+0xb1/0x120
> [ 342.617911] dev_close_many+0xa2/0x170
> [ 342.622622] rollback_registered_many+0x148/0x460
> [ 342.628401] ? __lock_acquire+0x48d/0x11b0
> [ 342.633498] ? unregister_netdev+0xe/0x20
> [ 342.638495] rollback_registered+0x56/0x90
> [ 342.643588] unregister_netdevice_queue+0x7e/0x100
> [ 342.649461] unregister_netdev+0x18/0x20
> [ 342.654362] mlx5e_remove+0x2a/0x50 [mlx5_core]
> [ 342.659944] mlx5_remove_device+0xe5/0x110 [mlx5_core]
> [ 342.666208] mlx5_unregister_interface+0x39/0x90 [mlx5_core]
> [ 342.673038] cleanup+0x5/0xbfc [mlx5_core]
> [ 342.678094] __x64_sys_delete_module+0x16b/0x240
> [ 342.683725] ? do_syscall_64+0x1c/0x210
> [ 342.688476] do_syscall_64+0x5a/0x210
> [ 342.693025] entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> Fixes: 8d5d88527587 ("xdp: rhashtable with allocator ID to pointer mapping")
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> net/core/xdp.c | 13 +++----------
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
> V1 -> V2:
> * Use rhashtable_lookup_fast and make some code movements, per Daniel's
> and Alexei's comments.
>
> Please queue to -stable v4.18.
>
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 3dd99e1c04f5..8b1c7b699982 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -105,16 +105,9 @@ static void __xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
>
> mutex_lock(&mem_id_lock);
>
> - xa = rhashtable_lookup(mem_id_ht, &id, mem_id_rht_params);
> - if (!xa) {
> - mutex_unlock(&mem_id_lock);
> - return;
> - }
> -
> - err = rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params);
> - WARN_ON(err);
> -
> - call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
> + xa = rhashtable_lookup_fast(mem_id_ht, &id, mem_id_rht_params);
> + if (xa && rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params))
> + call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>
> mutex_unlock(&mem_id_lock);
> }
This is wrong.
The function rhashtable_remove_fast() returns zero on success.
Please, fix and send a V3.
Look at example in [1] section "Object removal"
[1] https://lwn.net/Articles/751374/
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* mobile apps for you
From: Jack Dike @ 2018-08-13 6:29 UTC (permalink / raw)
To: netdev
Do you need mobile apps development? We can do it for you.
We are an India base company. Here are the details about us:
Years in business: 8
Staffs: 125
App developed: 350
We work on Android, iOS, Ionic, and PhoneGap platforms, we have clients
across different kind of industries.
Such like retail, media and entertainment, BFSI, hospitality, social media,
eCommerce, food and beverages, etc.
We do below:
Mobile Apps
Mobile App UI/UX designing
App Maintenance and Support
Website or ecommerce portal development
Please reply back if you are interested in what we do.
We will share our portfolios to you.
Regards,
Jack
^ permalink raw reply
* mobile apps for you
From: Jack Dike @ 2018-08-13 6:29 UTC (permalink / raw)
To: netdev
Do you need mobile apps development? We can do it for you.
We are an India base company. Here are the details about us:
Years in business: 8
Staffs: 125
App developed: 350
We work on Android, iOS, Ionic, and PhoneGap platforms, we have clients
across different kind of industries.
Such like retail, media and entertainment, BFSI, hospitality, social media,
eCommerce, food and beverages, etc.
We do below:
Mobile Apps
Mobile App UI/UX designing
App Maintenance and Support
Website or ecommerce portal development
Please reply back if you are interested in what we do.
We will share our portfolios to you.
Regards,
Jack
^ permalink raw reply
* [PATCH bpf-next V3] net/xdp: Fix suspicious RCU usage warning
From: Tariq Toukan @ 2018-08-13 9:21 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: netdev, Eran Ben Elisha, Tariq Toukan, Jesper Dangaard Brouer
Fix the warning below by calling rhashtable_lookup_fast.
Also, make some code movements for better quality and human
readability.
[ 342.450870] WARNING: suspicious RCU usage
[ 342.455856] 4.18.0-rc2+ #17 Tainted: G O
[ 342.462210] -----------------------------
[ 342.467202] ./include/linux/rhashtable.h:481 suspicious rcu_dereference_check() usage!
[ 342.476568]
[ 342.476568] other info that might help us debug this:
[ 342.476568]
[ 342.486978]
[ 342.486978] rcu_scheduler_active = 2, debug_locks = 1
[ 342.495211] 4 locks held by modprobe/3934:
[ 342.500265] #0: 00000000e23116b2 (mlx5_intf_mutex){+.+.}, at:
mlx5_unregister_interface+0x18/0x90 [mlx5_core]
[ 342.511953] #1: 00000000ca16db96 (rtnl_mutex){+.+.}, at: unregister_netdev+0xe/0x20
[ 342.521109] #2: 00000000a46e2c4b (&priv->state_lock){+.+.}, at: mlx5e_close+0x29/0x60
[mlx5_core]
[ 342.531642] #3: 0000000060c5bde3 (mem_id_lock){+.+.}, at: xdp_rxq_info_unreg+0x93/0x6b0
[ 342.541206]
[ 342.541206] stack backtrace:
[ 342.547075] CPU: 12 PID: 3934 Comm: modprobe Tainted: G O 4.18.0-rc2+ #17
[ 342.556621] Hardware name: Dell Inc. PowerEdge R730/0H21J3, BIOS 1.5.4 10/002/2015
[ 342.565606] Call Trace:
[ 342.568861] dump_stack+0x78/0xb3
[ 342.573086] xdp_rxq_info_unreg+0x3f5/0x6b0
[ 342.578285] ? __call_rcu+0x220/0x300
[ 342.582911] mlx5e_free_rq+0x38/0xc0 [mlx5_core]
[ 342.588602] mlx5e_close_channel+0x20/0x120 [mlx5_core]
[ 342.594976] mlx5e_close_channels+0x26/0x40 [mlx5_core]
[ 342.601345] mlx5e_close_locked+0x44/0x50 [mlx5_core]
[ 342.607519] mlx5e_close+0x42/0x60 [mlx5_core]
[ 342.613005] __dev_close_many+0xb1/0x120
[ 342.617911] dev_close_many+0xa2/0x170
[ 342.622622] rollback_registered_many+0x148/0x460
[ 342.628401] ? __lock_acquire+0x48d/0x11b0
[ 342.633498] ? unregister_netdev+0xe/0x20
[ 342.638495] rollback_registered+0x56/0x90
[ 342.643588] unregister_netdevice_queue+0x7e/0x100
[ 342.649461] unregister_netdev+0x18/0x20
[ 342.654362] mlx5e_remove+0x2a/0x50 [mlx5_core]
[ 342.659944] mlx5_remove_device+0xe5/0x110 [mlx5_core]
[ 342.666208] mlx5_unregister_interface+0x39/0x90 [mlx5_core]
[ 342.673038] cleanup+0x5/0xbfc [mlx5_core]
[ 342.678094] __x64_sys_delete_module+0x16b/0x240
[ 342.683725] ? do_syscall_64+0x1c/0x210
[ 342.688476] do_syscall_64+0x5a/0x210
[ 342.693025] entry_SYSCALL_64_after_hwframe+0x49/0xbe
Fixes: 8d5d88527587 ("xdp: rhashtable with allocator ID to pointer mapping")
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
---
net/core/xdp.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
V2 -> V3:
* Fix return value test for rhashtable_remove_fast, per Jesper's comment.
V1 -> V2:
* Use rhashtable_lookup_fast and make some code movements, per Daniel's
and Alexei's comments.
Please queue to -stable v4.18.
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 3dd99e1c04f5..8b1c7b699982 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -105,16 +105,9 @@ static void __xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
mutex_lock(&mem_id_lock);
- xa = rhashtable_lookup(mem_id_ht, &id, mem_id_rht_params);
- if (!xa) {
- mutex_unlock(&mem_id_lock);
- return;
- }
-
- err = rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params);
- WARN_ON(err);
-
- call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
+ xa = rhashtable_lookup_fast(mem_id_ht, &id, mem_id_rht_params);
+ if (xa && !rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params))
+ call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
mutex_unlock(&mem_id_lock);
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH bpf-next V2] net/xdp: Fix suspicious RCU usage warning
From: Tariq Toukan @ 2018-08-13 9:24 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Tariq Toukan
Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Eran Ben Elisha
In-Reply-To: <20180813111354.1a651bd8@redhat.com>
On 13/08/2018 12:13 PM, Jesper Dangaard Brouer wrote:
> On Mon, 13 Aug 2018 10:04:39 +0300
> Tariq Toukan <tariqt@mellanox.com> wrote:
>
>> Fix the warning below by calling rhashtable_lookup_fast.
>> Also, make some code movements for better quality and human
>> readability.
>>
>> [ 342.450870] WARNING: suspicious RCU usage
>> [ 342.455856] 4.18.0-rc2+ #17 Tainted: G O
>> [ 342.462210] -----------------------------
>> [ 342.467202] ./include/linux/rhashtable.h:481 suspicious rcu_dereference_check() usage!
>> [ 342.476568]
>> [ 342.476568] other info that might help us debug this:
>> [ 342.476568]
>> [ 342.486978]
>> [ 342.486978] rcu_scheduler_active = 2, debug_locks = 1
>> [ 342.495211] 4 locks held by modprobe/3934:
>> [ 342.500265] #0: 00000000e23116b2 (mlx5_intf_mutex){+.+.}, at:
>> mlx5_unregister_interface+0x18/0x90 [mlx5_core]
>> [ 342.511953] #1: 00000000ca16db96 (rtnl_mutex){+.+.}, at: unregister_netdev+0xe/0x20
>> [ 342.521109] #2: 00000000a46e2c4b (&priv->state_lock){+.+.}, at: mlx5e_close+0x29/0x60
>> [mlx5_core]
>> [ 342.531642] #3: 0000000060c5bde3 (mem_id_lock){+.+.}, at: xdp_rxq_info_unreg+0x93/0x6b0
>> [ 342.541206]
>> [ 342.541206] stack backtrace:
>> [ 342.547075] CPU: 12 PID: 3934 Comm: modprobe Tainted: G O 4.18.0-rc2+ #17
>> [ 342.556621] Hardware name: Dell Inc. PowerEdge R730/0H21J3, BIOS 1.5.4 10/002/2015
>> [ 342.565606] Call Trace:
>> [ 342.568861] dump_stack+0x78/0xb3
>> [ 342.573086] xdp_rxq_info_unreg+0x3f5/0x6b0
>> [ 342.578285] ? __call_rcu+0x220/0x300
>> [ 342.582911] mlx5e_free_rq+0x38/0xc0 [mlx5_core]
>> [ 342.588602] mlx5e_close_channel+0x20/0x120 [mlx5_core]
>> [ 342.594976] mlx5e_close_channels+0x26/0x40 [mlx5_core]
>> [ 342.601345] mlx5e_close_locked+0x44/0x50 [mlx5_core]
>> [ 342.607519] mlx5e_close+0x42/0x60 [mlx5_core]
>> [ 342.613005] __dev_close_many+0xb1/0x120
>> [ 342.617911] dev_close_many+0xa2/0x170
>> [ 342.622622] rollback_registered_many+0x148/0x460
>> [ 342.628401] ? __lock_acquire+0x48d/0x11b0
>> [ 342.633498] ? unregister_netdev+0xe/0x20
>> [ 342.638495] rollback_registered+0x56/0x90
>> [ 342.643588] unregister_netdevice_queue+0x7e/0x100
>> [ 342.649461] unregister_netdev+0x18/0x20
>> [ 342.654362] mlx5e_remove+0x2a/0x50 [mlx5_core]
>> [ 342.659944] mlx5_remove_device+0xe5/0x110 [mlx5_core]
>> [ 342.666208] mlx5_unregister_interface+0x39/0x90 [mlx5_core]
>> [ 342.673038] cleanup+0x5/0xbfc [mlx5_core]
>> [ 342.678094] __x64_sys_delete_module+0x16b/0x240
>> [ 342.683725] ? do_syscall_64+0x1c/0x210
>> [ 342.688476] do_syscall_64+0x5a/0x210
>> [ 342.693025] entry_SYSCALL_64_after_hwframe+0x49/0xbe
>>
>> Fixes: 8d5d88527587 ("xdp: rhashtable with allocator ID to pointer mapping")
>> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
>> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
>> ---
>> net/core/xdp.c | 13 +++----------
>> 1 file changed, 3 insertions(+), 10 deletions(-)
>>
>> V1 -> V2:
>> * Use rhashtable_lookup_fast and make some code movements, per Daniel's
>> and Alexei's comments.
>>
>> Please queue to -stable v4.18.
>>
>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>> index 3dd99e1c04f5..8b1c7b699982 100644
>> --- a/net/core/xdp.c
>> +++ b/net/core/xdp.c
>> @@ -105,16 +105,9 @@ static void __xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
>>
>> mutex_lock(&mem_id_lock);
>>
>> - xa = rhashtable_lookup(mem_id_ht, &id, mem_id_rht_params);
>> - if (!xa) {
>> - mutex_unlock(&mem_id_lock);
>> - return;
>> - }
>> -
>> - err = rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params);
>> - WARN_ON(err);
>> -
>> - call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>> + xa = rhashtable_lookup_fast(mem_id_ht, &id, mem_id_rht_params);
>> + if (xa && rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params))
>> + call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>>
>> mutex_unlock(&mem_id_lock);
>> }
>
> This is wrong.
>
> The function rhashtable_remove_fast() returns zero on success.
> Please, fix and send a V3.
>
> Look at example in [1] section "Object removal"
> [1] https://lwn.net/Articles/751374/
>
Right, thanks Jesper!
V3 Sent.
^ permalink raw reply
* Re: [PATCH bpf-next V2] net/xdp: Fix suspicious RCU usage warning
From: Daniel Borkmann @ 2018-08-13 9:26 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Tariq Toukan
Cc: Alexei Starovoitov, netdev, Eran Ben Elisha
In-Reply-To: <20180813111354.1a651bd8@redhat.com>
On 08/13/2018 11:13 AM, Jesper Dangaard Brouer wrote:
> On Mon, 13 Aug 2018 10:04:39 +0300
> Tariq Toukan <tariqt@mellanox.com> wrote:
>
>> Fix the warning below by calling rhashtable_lookup_fast.
>> Also, make some code movements for better quality and human
>> readability.
>>
>> [ 342.450870] WARNING: suspicious RCU usage
>> [ 342.455856] 4.18.0-rc2+ #17 Tainted: G O
>> [ 342.462210] -----------------------------
>> [ 342.467202] ./include/linux/rhashtable.h:481 suspicious rcu_dereference_check() usage!
>> [ 342.476568]
>> [ 342.476568] other info that might help us debug this:
>> [ 342.476568]
>> [ 342.486978]
>> [ 342.486978] rcu_scheduler_active = 2, debug_locks = 1
>> [ 342.495211] 4 locks held by modprobe/3934:
>> [ 342.500265] #0: 00000000e23116b2 (mlx5_intf_mutex){+.+.}, at:
>> mlx5_unregister_interface+0x18/0x90 [mlx5_core]
>> [ 342.511953] #1: 00000000ca16db96 (rtnl_mutex){+.+.}, at: unregister_netdev+0xe/0x20
>> [ 342.521109] #2: 00000000a46e2c4b (&priv->state_lock){+.+.}, at: mlx5e_close+0x29/0x60
>> [mlx5_core]
>> [ 342.531642] #3: 0000000060c5bde3 (mem_id_lock){+.+.}, at: xdp_rxq_info_unreg+0x93/0x6b0
>> [ 342.541206]
>> [ 342.541206] stack backtrace:
>> [ 342.547075] CPU: 12 PID: 3934 Comm: modprobe Tainted: G O 4.18.0-rc2+ #17
>> [ 342.556621] Hardware name: Dell Inc. PowerEdge R730/0H21J3, BIOS 1.5.4 10/002/2015
>> [ 342.565606] Call Trace:
>> [ 342.568861] dump_stack+0x78/0xb3
>> [ 342.573086] xdp_rxq_info_unreg+0x3f5/0x6b0
>> [ 342.578285] ? __call_rcu+0x220/0x300
>> [ 342.582911] mlx5e_free_rq+0x38/0xc0 [mlx5_core]
>> [ 342.588602] mlx5e_close_channel+0x20/0x120 [mlx5_core]
>> [ 342.594976] mlx5e_close_channels+0x26/0x40 [mlx5_core]
>> [ 342.601345] mlx5e_close_locked+0x44/0x50 [mlx5_core]
>> [ 342.607519] mlx5e_close+0x42/0x60 [mlx5_core]
>> [ 342.613005] __dev_close_many+0xb1/0x120
>> [ 342.617911] dev_close_many+0xa2/0x170
>> [ 342.622622] rollback_registered_many+0x148/0x460
>> [ 342.628401] ? __lock_acquire+0x48d/0x11b0
>> [ 342.633498] ? unregister_netdev+0xe/0x20
>> [ 342.638495] rollback_registered+0x56/0x90
>> [ 342.643588] unregister_netdevice_queue+0x7e/0x100
>> [ 342.649461] unregister_netdev+0x18/0x20
>> [ 342.654362] mlx5e_remove+0x2a/0x50 [mlx5_core]
>> [ 342.659944] mlx5_remove_device+0xe5/0x110 [mlx5_core]
>> [ 342.666208] mlx5_unregister_interface+0x39/0x90 [mlx5_core]
>> [ 342.673038] cleanup+0x5/0xbfc [mlx5_core]
>> [ 342.678094] __x64_sys_delete_module+0x16b/0x240
>> [ 342.683725] ? do_syscall_64+0x1c/0x210
>> [ 342.688476] do_syscall_64+0x5a/0x210
>> [ 342.693025] entry_SYSCALL_64_after_hwframe+0x49/0xbe
>>
>> Fixes: 8d5d88527587 ("xdp: rhashtable with allocator ID to pointer mapping")
>> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
>> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
>> ---
>> net/core/xdp.c | 13 +++----------
>> 1 file changed, 3 insertions(+), 10 deletions(-)
>>
>> V1 -> V2:
>> * Use rhashtable_lookup_fast and make some code movements, per Daniel's
>> and Alexei's comments.
>>
>> Please queue to -stable v4.18.
>>
>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>> index 3dd99e1c04f5..8b1c7b699982 100644
>> --- a/net/core/xdp.c
>> +++ b/net/core/xdp.c
>> @@ -105,16 +105,9 @@ static void __xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
>>
>> mutex_lock(&mem_id_lock);
>>
>> - xa = rhashtable_lookup(mem_id_ht, &id, mem_id_rht_params);
>> - if (!xa) {
>> - mutex_unlock(&mem_id_lock);
>> - return;
>> - }
>> -
>> - err = rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params);
>> - WARN_ON(err);
>> -
>> - call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>> + xa = rhashtable_lookup_fast(mem_id_ht, &id, mem_id_rht_params);
>> + if (xa && rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params))
>> + call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>>
>> mutex_unlock(&mem_id_lock);
>> }
>
> This is wrong.
>
> The function rhashtable_remove_fast() returns zero on success.
> Please, fix and send a V3.
Good catch, suggestion in https://patchwork.ozlabs.org/patch/945121/ did have the
check for success, so seems this slipped through while adapting to it.
> Look at example in [1] section "Object removal"
> [1] https://lwn.net/Articles/751374/
^ permalink raw reply
* we do mobile apps
From: Jack Dike @ 2018-08-13 5:28 UTC (permalink / raw)
To: netdev
Do you need mobile apps development? We can do it for you.
We are an India base company. Here are the details about us:
Years in business: 8
Staffs: 125
App developed: 350
We work on Android, iOS, Ionic, and PhoneGap platforms, we have clients
across different kind of industries.
Such like retail, media and entertainment, BFSI, hospitality, social media,
eCommerce, food and beverages, etc.
We do below:
Mobile Apps
Mobile App UI/UX designing
App Maintenance and Support
Website or ecommerce portal development
Please reply back if you are interested in what we do.
We will share our portfolios to you.
Regards,
Jack
^ permalink raw reply
* possible deadlock in flush_work (3)
From: syzbot @ 2018-08-13 12:35 UTC (permalink / raw)
To: christian.brauner, davem, dsahern, fw, jbenc, ktkhai,
linux-kernel, lucien.xin, netdev, nicolas.dichtel, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: d6dd6431591b Merge branch 'fixes' of git://git.kernel.org/..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14e800aa400000
kernel config: https://syzkaller.appspot.com/x/.config?x=152cb8ccd35b1f70
dashboard link: https://syzkaller.appspot.com/bug?extid=a8371264572a6872b8a3
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+a8371264572a6872b8a3@syzkaller.appspotmail.com
8021q: adding VLAN 0 to HW filter on device bond0
======================================================
WARNING: possible circular locking dependency detected
4.18.0-rc8+ #185 Not tainted
------------------------------------------------------
syz-executor2/6421 is trying to acquire lock:
00000000209b4e4b ((wq_completion)bond_dev->name){+.+.}, at:
start_flush_work kernel/workqueue.c:2888 [inline]
00000000209b4e4b ((wq_completion)bond_dev->name){+.+.}, at:
flush_work+0x4b8/0x900 kernel/workqueue.c:2917
but task is already holding lock:
00000000f0f3d47a (rtnl_mutex){+.+.}, at: rtnl_lock+0x17/0x20
net/core/rtnetlink.c:77
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #2 (rtnl_mutex){+.+.}:
__mutex_lock_common kernel/locking/mutex.c:757 [inline]
__mutex_lock+0x176/0x1820 kernel/locking/mutex.c:894
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:909
rtnl_lock+0x17/0x20 net/core/rtnetlink.c:77
bond_netdev_notify drivers/net/bonding/bond_main.c:1310 [inline]
bond_netdev_notify_work+0x44/0xd0
drivers/net/bonding/bond_main.c:1320
process_one_work+0xc73/0x1ba0 kernel/workqueue.c:2153
worker_thread+0x189/0x13c0 kernel/workqueue.c:2296
kthread+0x345/0x410 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
-> #1 ((work_completion)(&(&nnw->work)->work)){+.+.}:
process_one_work+0xc0b/0x1ba0 kernel/workqueue.c:2129
worker_thread+0x189/0x13c0 kernel/workqueue.c:2296
kthread+0x345/0x410 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
-> #0 ((wq_completion)bond_dev->name){+.+.}:
lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
start_flush_work kernel/workqueue.c:2889 [inline]
flush_work+0x4dd/0x900 kernel/workqueue.c:2917
__cancel_work_timer+0x4bd/0x830 kernel/workqueue.c:2989
cancel_delayed_work_sync+0x1a/0x20 kernel/workqueue.c:3121
bond_work_cancel_all drivers/net/bonding/bond_main.c:3318 [inline]
bond_close+0x1b/0x130 drivers/net/bonding/bond_main.c:3381
__dev_close_many+0x21e/0x380 net/core/dev.c:1476
__dev_close net/core/dev.c:1488 [inline]
__dev_change_flags+0x38d/0x9c0 net/core/dev.c:6989
dev_change_flags+0x89/0x150 net/core/dev.c:7060
dev_ifsioc+0x84f/0xb30 net/core/dev_ioctl.c:237
dev_ioctl+0x1b5/0xcc0 net/core/dev_ioctl.c:493
sock_do_ioctl+0x1d3/0x3e0 net/socket.c:993
sock_ioctl+0x30d/0x680 net/socket.c:1094
vfs_ioctl fs/ioctl.c:46 [inline]
file_ioctl fs/ioctl.c:500 [inline]
do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:684
ksys_ioctl+0xa9/0xd0 fs/ioctl.c:701
__do_sys_ioctl fs/ioctl.c:708 [inline]
__se_sys_ioctl fs/ioctl.c:706 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:706
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
other info that might help us debug this:
Chain exists of:
(wq_completion)bond_dev->name --> (work_completion)(&(&nnw->work)->work)
--> rtnl_mutex
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(rtnl_mutex);
lock((work_completion)(&(&nnw->work)->work));
lock(rtnl_mutex);
lock((wq_completion)bond_dev->name);
*** DEADLOCK ***
1 lock held by syz-executor2/6421:
#0: 00000000f0f3d47a (rtnl_mutex){+.+.}, at: rtnl_lock+0x17/0x20
net/core/rtnetlink.c:77
stack backtrace:
CPU: 0 PID: 6421 Comm: syz-executor2 Not tainted 4.18.0-rc8+ #185
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
print_circular_bug.isra.36.cold.57+0x1bd/0x27d
kernel/locking/lockdep.c:1227
check_prev_add kernel/locking/lockdep.c:1867 [inline]
check_prevs_add kernel/locking/lockdep.c:1980 [inline]
validate_chain kernel/locking/lockdep.c:2421 [inline]
__lock_acquire+0x3449/0x5020 kernel/locking/lockdep.c:3435
lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
start_flush_work kernel/workqueue.c:2889 [inline]
flush_work+0x4dd/0x900 kernel/workqueue.c:2917
__cancel_work_timer+0x4bd/0x830 kernel/workqueue.c:2989
cancel_delayed_work_sync+0x1a/0x20 kernel/workqueue.c:3121
bond_work_cancel_all drivers/net/bonding/bond_main.c:3318 [inline]
bond_close+0x1b/0x130 drivers/net/bonding/bond_main.c:3381
__dev_close_many+0x21e/0x380 net/core/dev.c:1476
__dev_close net/core/dev.c:1488 [inline]
__dev_change_flags+0x38d/0x9c0 net/core/dev.c:6989
dev_change_flags+0x89/0x150 net/core/dev.c:7060
dev_ifsioc+0x84f/0xb30 net/core/dev_ioctl.c:237
dev_ioctl+0x1b5/0xcc0 net/core/dev_ioctl.c:493
sock_do_ioctl+0x1d3/0x3e0 net/socket.c:993
kernel msg: ebtables bug: please report to author: Wrong nr of counters
sock_ioctl+0x30d/0x680 net/socket.c:1094
vfs_ioctl fs/ioctl.c:46 [inline]
file_ioctl fs/ioctl.c:500 [inline]
do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:684
ksys_ioctl+0xa9/0xd0 fs/ioctl.c:701
__do_sys_ioctl fs/ioctl.c:708 [inline]
__se_sys_ioctl fs/ioctl.c:706 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:706
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x457089
Code: fd b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 cb b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fee7b2ccc78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00007fee7b2cd6d4 RCX: 0000000000457089
RDX: 0000000020000140 RSI: 0000000000008914 RDI: 0000000000000004
RBP: 00000000009300a0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 00000000004d19e0 R14: 00000000004c7454 R15: 0000000000000000
8021q: adding VLAN 0 to HW filter on device bond0
(unnamed net_device) (uninitialized): option miimon: invalid value
(18446744073709551615)
(unnamed net_device) (uninitialized): option miimon: allowed values 0 -
2147483647
(unnamed net_device) (uninitialized): option miimon: invalid value
(18446744073709551615)
(unnamed net_device) (uninitialized): option miimon: allowed values 0 -
2147483647
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
device bond0 left promiscuous mode
IPv6: ADDRCONF(NETDEV_UP): bond0: link is not ready
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
IPv6: ADDRCONF(NETDEV_UP): bond0: link is not ready
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
IPv6: ADDRCONF(NETDEV_UP): bond0: link is not ready
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
^ permalink raw reply
* Re: [RFC net-next 00/15] net: A socket API for LoRa
From: Alan Cox @ 2018-08-13 12:36 UTC (permalink / raw)
To: Andreas Färber
Cc: Michal Kubeček, Konstantin Böhm, shess, Pieter ROBYNS,
contact@snootlab.com, Xue Liu, Ken Yu, Michael Röder,
Stefan Schmidt, Rob Herring, lora, Alexander Graf, Jan Jongboom,
Janus Piwek, Jon Ortego, Jian-Hong Pan, devicetree, Jiri Pirko,
Hasnain Virk, Daniele Comel, Marcel Holtmann, Mark Brown,
Dollar Chen
In-Reply-To: <11276f7d-0175-05e9-a9bd-c72868067081@suse.de>
> The simple answer is that, inspired by CAN, it uses an ifindex to select
> the interface the user asked to use. That then also answers Alan's next
> question: This ifindex determines which interface it goes out to.
>
> sockaddr_lora was in patch 02/15, latest code here:
> https://git.kernel.org/pub/scm/linux/kernel/git/afaerber/linux-lora.git/tree/include/uapi/linux/lora.h?h=lora-next
And any loopback just becomes an ifindex, likewise any virtual lorawan
device (eg when testing in a cloud or simulating radio properties).
> I still think the user will need to explicitly say which interface they
> want to bind their socket to. AFAIU the device EUI is more comparable to
It does make it very hard for any vaguely complex environment because if
for example you have two interfaces enumerated via USB they will appear
in random order each boot.
CANbus is a bit of a mess in this sense but it's so statically configured
and embedded into industrial devices it's less of a problem.
I just wonder if the name would be a better binding (so you can sort the
order out), or a local physical identifier of some kind so that your
enumeration is consistent.
> Loopback mode would require a separate virtual device driver such as
> fakelr or vlora.
And a tunnel device, which is easy enough if you've got tap support or
similar.
Alan
^ permalink raw reply
* Re: [PATCH] mt76: Enable NL80211_EXT_FEATURE_CQM_RSSI_LIST
From: Arend van Spriel @ 2018-08-13 12:39 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: Kalle Valo, kristian.evensen-Re5JQEeQqe8AvxtiuMwx3w,
linux-wireless, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAJ0CqmVZNAnHwnU4YqRRv7R8DyHi2MiFJr1bvQDPVszUU5Ez3g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 8/13/2018 12:55 PM, Lorenzo Bianconi wrote:
>> On 8/12/2018 8:14 PM, Kalle Valo wrote:
>>> Kristian Evensen <kristian.evensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>>>
>>>> Enable the use of CQM with mt76-devices.
>>>>
>>>> Signed-off-by: Kristian Evensen <kristian.evensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>> drivers/net/wireless/mediatek/mt76/mac80211.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
>>>> index 029d54bc..3eb328ff 100644
>>>> --- a/drivers/net/wireless/mediatek/mt76/mac80211.c
>>>> +++ b/drivers/net/wireless/mediatek/mt76/mac80211.c
>>>> @@ -305,6 +305,8 @@ int mt76_register_device(struct mt76_dev *dev, bool vht,
>>>>
>>>> wiphy->features |= NL80211_FEATURE_ACTIVE_MONITOR;
>>>>
>>>> + wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
>>>
>>> So have you tested this and with what devices? For example, does it work
>>> with recently added USB devices?
>>
>> I was looking into this as it looks suspicious to me. From reading the
>> description of this ext_feature flag it seems this is an extention of CQM:
>>
>> """
>> * @NL80211_EXT_FEATURE_CQM_RSSI_LIST: With this driver the
>> * %NL80211_ATTR_CQM_RSSI_THOLD attribute accepts a list of zero or more
>> * RSSI threshold values to monitor rather than exactly one threshold.
>> """
>>
>> Also looking at mt76x2_bss_info_changed() it does not handle
>> BSS_CHANGED_CQM so I doubt it has support for it (yet). The driver does
>> not use IEEE80211_VIF_SUPPORTS_CQM_RSSI which is a requirement for it.
>>
>> Regards,
>> Arend
>>
>
> According to my understanding (please correct me if I am wrong)
> BSS_CHANGED_CQM is only needed if CQM_RSSI is handled
> by the driver/fw, while if it is not set mac80211 will take care of that
> in ieee80211_handle_beacon_sig routine.
Yeah. That explains it. Seems like mac80211 could actually set the
NL80211_EXT_FEATURE_CQM_RSSI_LIST flag is the driver does not set
IEEE80211_VIF_SUPPORTS_CQM_RSSI. That way all mac80211 drivers support
the list. Just a problem as the ext_feature is not a per-vif flag.
Regards,
Arend
^ permalink raw reply
* Re: [PATCH bpf-next V3] net/xdp: Fix suspicious RCU usage warning
From: Jesper Dangaard Brouer @ 2018-08-13 10:31 UTC (permalink / raw)
To: Tariq Toukan
Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Eran Ben Elisha,
brouer, Neil Brown, Paul E. McKenney
In-Reply-To: <1534152118-15968-1-git-send-email-tariqt@mellanox.com>
On Mon, 13 Aug 2018 12:21:58 +0300
Tariq Toukan <tariqt@mellanox.com> wrote:
> Fix the warning below by calling rhashtable_lookup_fast.
> Also, make some code movements for better quality and human
> readability.
>
> [ 342.450870] WARNING: suspicious RCU usage
> [ 342.455856] 4.18.0-rc2+ #17 Tainted: G O
> [ 342.462210] -----------------------------
> [ 342.467202] ./include/linux/rhashtable.h:481 suspicious rcu_dereference_check() usage!
> [ 342.476568]
> [ 342.476568] other info that might help us debug this:
> [ 342.476568]
> [ 342.486978]
> [ 342.486978] rcu_scheduler_active = 2, debug_locks = 1
> [ 342.495211] 4 locks held by modprobe/3934:
> [ 342.500265] #0: 00000000e23116b2 (mlx5_intf_mutex){+.+.}, at:
> mlx5_unregister_interface+0x18/0x90 [mlx5_core]
> [ 342.511953] #1: 00000000ca16db96 (rtnl_mutex){+.+.}, at: unregister_netdev+0xe/0x20
> [ 342.521109] #2: 00000000a46e2c4b (&priv->state_lock){+.+.}, at: mlx5e_close+0x29/0x60
> [mlx5_core]
> [ 342.531642] #3: 0000000060c5bde3 (mem_id_lock){+.+.}, at: xdp_rxq_info_unreg+0x93/0x6b0
> [ 342.541206]
> [ 342.541206] stack backtrace:
> [ 342.547075] CPU: 12 PID: 3934 Comm: modprobe Tainted: G O 4.18.0-rc2+ #17
> [ 342.556621] Hardware name: Dell Inc. PowerEdge R730/0H21J3, BIOS 1.5.4 10/002/2015
> [ 342.565606] Call Trace:
> [ 342.568861] dump_stack+0x78/0xb3
> [ 342.573086] xdp_rxq_info_unreg+0x3f5/0x6b0
> [ 342.578285] ? __call_rcu+0x220/0x300
> [ 342.582911] mlx5e_free_rq+0x38/0xc0 [mlx5_core]
> [ 342.588602] mlx5e_close_channel+0x20/0x120 [mlx5_core]
> [ 342.594976] mlx5e_close_channels+0x26/0x40 [mlx5_core]
> [ 342.601345] mlx5e_close_locked+0x44/0x50 [mlx5_core]
> [ 342.607519] mlx5e_close+0x42/0x60 [mlx5_core]
> [ 342.613005] __dev_close_many+0xb1/0x120
> [ 342.617911] dev_close_many+0xa2/0x170
> [ 342.622622] rollback_registered_many+0x148/0x460
> [ 342.628401] ? __lock_acquire+0x48d/0x11b0
> [ 342.633498] ? unregister_netdev+0xe/0x20
> [ 342.638495] rollback_registered+0x56/0x90
> [ 342.643588] unregister_netdevice_queue+0x7e/0x100
> [ 342.649461] unregister_netdev+0x18/0x20
> [ 342.654362] mlx5e_remove+0x2a/0x50 [mlx5_core]
> [ 342.659944] mlx5_remove_device+0xe5/0x110 [mlx5_core]
> [ 342.666208] mlx5_unregister_interface+0x39/0x90 [mlx5_core]
> [ 342.673038] cleanup+0x5/0xbfc [mlx5_core]
> [ 342.678094] __x64_sys_delete_module+0x16b/0x240
> [ 342.683725] ? do_syscall_64+0x1c/0x210
> [ 342.688476] do_syscall_64+0x5a/0x210
> [ 342.693025] entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> Fixes: 8d5d88527587 ("xdp: rhashtable with allocator ID to pointer mapping")
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> net/core/xdp.c | 13 +++----------
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
> V2 -> V3:
> * Fix return value test for rhashtable_remove_fast, per Jesper's comment.
>
> V1 -> V2:
> * Use rhashtable_lookup_fast and make some code movements, per Daniel's
> and Alexei's comments.
>
> Please queue to -stable v4.18.
>
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 3dd99e1c04f5..8b1c7b699982 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -105,16 +105,9 @@ static void __xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
>
> mutex_lock(&mem_id_lock);
>
> - xa = rhashtable_lookup(mem_id_ht, &id, mem_id_rht_params);
> - if (!xa) {
> - mutex_unlock(&mem_id_lock);
> - return;
> - }
> -
> - err = rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params);
> - WARN_ON(err);
> -
> - call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
> + xa = rhashtable_lookup_fast(mem_id_ht, &id, mem_id_rht_params);
> + if (xa && !rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params))
> + call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>
> mutex_unlock(&mem_id_lock);
> }
I would personally prefer to write it as in the example as "== 0", look
at example in [1] section "Object removal", but is it semantically the
same to write !rhashtable_remove_fast(). So, I'm fine with this.
In the example[1], the sequence is wrapped in rcu_read_lock/unlock,
while you have not done so. The rhashtable_lookup_fast and
rhashtable_remove_fast calls have their own rcu_read_lock/unlock, but
the outer rcu_read_lock/unlock, makes sure that a RCU period cannot
slip in between the two calls.
I still think your fix is correct, due to the mutex_lock. Given the
mutex sync removal and insert in this rhashtable.
I do wonder if it would be better to add the outer rcu_read_lock/unlock,
calls if someone else reads and copy-paste this code (and don't have an
mutex sync scheme) ?
If you think this is all fine, and want to proceed as is then you have
my ack:
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
[1] https://lwn.net/Articles/751374/
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH net 0/2] net_sched: Fix two tc_index filter init issues
From: Hangbin Liu @ 2018-08-13 10:44 UTC (permalink / raw)
To: netdev; +Cc: David Miller, WANG Cong, Hangbin Liu
These two patches fix two tc_index filter init issues. The first one fixes
missing exts info in new filter, which will cause NULL pointer dereference
when delete tcindex filter. The second one fixes missing res info when create
new filter, which will make filter unbind failed.
Hangbin Liu (2):
net_sched: fix NULL pointer dereference when delete tcindex filter
net_sched: Fix missing res info when create new filter
net/sched/cls_tcindex.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
--
2.5.5
^ permalink raw reply
* [PATCH net 1/2] net_sched: fix NULL pointer dereference when delete tcindex filter
From: Hangbin Liu @ 2018-08-13 10:44 UTC (permalink / raw)
To: netdev; +Cc: David Miller, WANG Cong, Hangbin Liu
In-Reply-To: <1534157044-19753-1-git-send-email-liuhangbin@gmail.com>
Li Shuang reported the following crash:
[ 71.267724] BUG: unable to handle kernel NULL pointer dereference at 0000000000000004
[ 71.276456] PGD 800000085d9bd067 P4D 800000085d9bd067 PUD 859a0b067 PMD 0
[ 71.284127] Oops: 0000 [#1] SMP PTI
[ 71.288015] CPU: 12 PID: 2386 Comm: tc Not tainted 4.18.0-rc8.latest+ #131
[ 71.295686] Hardware name: Dell Inc. PowerEdge R730/0WCJNT, BIOS 2.1.5 04/11/2016
[ 71.304037] RIP: 0010:tcindex_delete+0x72/0x280 [cls_tcindex]
[ 71.310446] Code: 00 31 f6 48 87 75 20 48 85 f6 74 11 48 8b 47 18 48 8b 40 08 48 8b 40 50 e8 fb a6 f8 fc 48 85 db 0f 84 dc 00 00 00 48 8b 73 18 <8b> 56 04 48 8d 7e 04 85 d2 0f 84 7b 01 00
[ 71.331517] RSP: 0018:ffffb45207b3f898 EFLAGS: 00010282
[ 71.337345] RAX: ffff8ad3d72d6360 RBX: ffff8acc84393680 RCX: 000000000000002e
[ 71.345306] RDX: ffff8ad3d72c8570 RSI: 0000000000000000 RDI: ffff8ad847a45800
[ 71.353277] RBP: ffff8acc84393688 R08: ffff8ad3d72c8400 R09: 0000000000000000
[ 71.361238] R10: ffff8ad3de786e00 R11: 0000000000000000 R12: ffffb45207b3f8c7
[ 71.369199] R13: ffff8ad3d93bd2a0 R14: 000000000000002e R15: ffff8ad3d72c9600
[ 71.377161] FS: 00007f9d3ec3e740(0000) GS:ffff8ad3df980000(0000) knlGS:0000000000000000
[ 71.386188] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 71.392597] CR2: 0000000000000004 CR3: 0000000852f06003 CR4: 00000000001606e0
[ 71.400558] Call Trace:
[ 71.403299] tcindex_destroy_element+0x25/0x40 [cls_tcindex]
[ 71.409611] tcindex_walk+0xbb/0x110 [cls_tcindex]
[ 71.414953] tcindex_destroy+0x44/0x90 [cls_tcindex]
[ 71.420492] ? tcindex_delete+0x280/0x280 [cls_tcindex]
[ 71.426323] tcf_proto_destroy+0x16/0x40
[ 71.430696] tcf_chain_flush+0x51/0x70
[ 71.434876] tcf_block_put_ext.part.30+0x8f/0x1b0
[ 71.440122] tcf_block_put+0x4d/0x70
[ 71.444108] cbq_destroy+0x4d/0xd0 [sch_cbq]
[ 71.448869] qdisc_destroy+0x62/0x130
[ 71.452951] dsmark_destroy+0x2a/0x70 [sch_dsmark]
[ 71.458300] qdisc_destroy+0x62/0x130
[ 71.462373] qdisc_graft+0x3ba/0x470
[ 71.466359] tc_get_qdisc+0x2a6/0x2c0
[ 71.470443] ? cred_has_capability+0x7d/0x130
[ 71.475307] rtnetlink_rcv_msg+0x263/0x2d0
[ 71.479875] ? rtnl_calcit.isra.30+0x110/0x110
[ 71.484832] netlink_rcv_skb+0x4d/0x130
[ 71.489109] netlink_unicast+0x1a3/0x250
[ 71.493482] netlink_sendmsg+0x2ae/0x3a0
[ 71.497859] sock_sendmsg+0x36/0x40
[ 71.501748] ___sys_sendmsg+0x26f/0x2d0
[ 71.506029] ? handle_pte_fault+0x586/0xdf0
[ 71.510694] ? __handle_mm_fault+0x389/0x500
[ 71.515457] ? __sys_sendmsg+0x5e/0xa0
[ 71.519636] __sys_sendmsg+0x5e/0xa0
[ 71.523626] do_syscall_64+0x5b/0x180
[ 71.527711] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 71.533345] RIP: 0033:0x7f9d3e257f10
[ 71.537331] Code: c3 48 8b 05 82 6f 2c 00 f7 db 64 89 18 48 83 cb ff eb dd 0f 1f 80 00 00 00 00 83 3d 8d d0 2c 00 00 75 10 b8 2e 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8
[ 71.558401] RSP: 002b:00007fff6f893398 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[ 71.566848] RAX: ffffffffffffffda RBX: 000000005b71274d RCX: 00007f9d3e257f10
[ 71.574810] RDX: 0000000000000000 RSI: 00007fff6f8933e0 RDI: 0000000000000003
[ 71.582770] RBP: 00007fff6f8933e0 R08: 000000000000ffff R09: 0000000000000003
[ 71.590729] R10: 00007fff6f892e20 R11: 0000000000000246 R12: 0000000000000000
[ 71.598689] R13: 0000000000662ee0 R14: 0000000000000000 R15: 0000000000000000
[ 71.606651] Modules linked in: sch_cbq cls_tcindex sch_dsmark xt_CHECKSUM iptable_mangle ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_coni
[ 71.685425] libahci i2c_algo_bit i2c_core i40e libata dca mdio megaraid_sas dm_mirror dm_region_hash dm_log dm_mod
[ 71.697075] CR2: 0000000000000004
[ 71.700792] ---[ end trace f604eb1acacd978b ]---
Reproducer:
tc qdisc add dev lo handle 1:0 root dsmark indices 64 set_tc_index
tc filter add dev lo parent 1:0 protocol ip prio 1 tcindex mask 0xfc shift 2
tc qdisc add dev lo parent 1:0 handle 2:0 cbq bandwidth 10Mbit cell 8 avpkt 1000 mpu 64
tc class add dev lo parent 2:0 classid 2:1 cbq bandwidth 10Mbit rate 1500Kbit avpkt 1000 prio 1 bounded isolated allot 1514 weight 1 maxburst 10
tc filter add dev lo parent 2:0 protocol ip prio 1 handle 0x2e tcindex classid 2:1 pass_on
tc qdisc add dev lo parent 2:1 pfifo limit 5
tc qdisc del dev lo root
This is because in tcindex_set_parms, when there is no old_r, we set new
exts to cr.exts. And we didn't set it to filter when r == &new_filter_result.
Then in tcindex_delete() -> tcf_exts_get_net(), we will get NULL pointer
dereference as we didn't init exts.
Fix it by moving tcf_exts_change() after "if (old_r && old_r != r)" check.
Then we don't need "cr" as there is no errout after that.
Fixes: bf63ac73b3e13 ("net_sched: fix an oops in tcindex filter")
Reported-by: Li Shuang <shuali@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
net/sched/cls_tcindex.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index 32f4bbd..ddaa4e6 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -447,11 +447,6 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
tcf_bind_filter(tp, &cr.res, base);
}
- if (old_r)
- tcf_exts_change(&r->exts, &e);
- else
- tcf_exts_change(&cr.exts, &e);
-
if (old_r && old_r != r) {
err = tcindex_filter_result_init(old_r);
if (err < 0) {
@@ -462,6 +457,8 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
oldp = p;
r->res = cr.res;
+ tcf_exts_change(&r->exts, &e);
+
rcu_assign_pointer(tp->root, cp);
if (r == &new_filter_result) {
--
2.5.5
^ permalink raw reply related
* [PATCH net 2/2] net_sched: Fix missing res info when create new tc_index filter
From: Hangbin Liu @ 2018-08-13 10:44 UTC (permalink / raw)
To: netdev; +Cc: David Miller, WANG Cong, Hangbin Liu
In-Reply-To: <1534157044-19753-1-git-send-email-liuhangbin@gmail.com>
Li Shuang reported the following warn:
[ 733.484610] WARNING: CPU: 6 PID: 21123 at net/sched/sch_cbq.c:1418 cbq_destroy_class+0x5d/0x70 [sch_cbq]
[ 733.495190] Modules linked in: sch_cbq cls_tcindex sch_dsmark rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache xt_CHECKSUM iptable_mangle ipt_MASQUERADE iptable_nat l
[ 733.574155] syscopyarea sysfillrect sysimgblt fb_sys_fops ttm drm igb ixgbe ahci libahci i2c_algo_bit libata i40e i2c_core dca mdio megaraid_sas dm_mirror dm_region_hash dm_log dm_mod
[ 733.592500] CPU: 6 PID: 21123 Comm: tc Not tainted 4.18.0-rc8.latest+ #131
[ 733.600169] Hardware name: Dell Inc. PowerEdge R730/0WCJNT, BIOS 2.1.5 04/11/2016
[ 733.608518] RIP: 0010:cbq_destroy_class+0x5d/0x70 [sch_cbq]
[ 733.614734] Code: e7 d9 d2 48 8b 7b 48 e8 61 05 da d2 48 8d bb f8 00 00 00 e8 75 ae d5 d2 48 39 eb 74 0a 48 89 df 5b 5d e9 16 6c 94 d2 5b 5d c3 <0f> 0b eb b6 0f 1f 44 00 00 66 2e 0f 1f 84
[ 733.635798] RSP: 0018:ffffbfbb066bb9d8 EFLAGS: 00010202
[ 733.641627] RAX: 0000000000000001 RBX: ffff9cdd17392800 RCX: 000000008010000f
[ 733.649588] RDX: ffff9cdd1df547e0 RSI: ffff9cdd17392800 RDI: ffff9cdd0f84c800
[ 733.657547] RBP: ffff9cdd0f84c800 R08: 0000000000000001 R09: 0000000000000000
[ 733.665508] R10: ffff9cdd0f84d000 R11: 0000000000000001 R12: 0000000000000001
[ 733.673469] R13: 0000000000000000 R14: 0000000000000001 R15: ffff9cdd17392200
[ 733.681430] FS: 00007f911890a740(0000) GS:ffff9cdd1f8c0000(0000) knlGS:0000000000000000
[ 733.690456] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 733.696864] CR2: 0000000000b5544c CR3: 0000000859374002 CR4: 00000000001606e0
[ 733.704826] Call Trace:
[ 733.707554] cbq_destroy+0xa1/0xd0 [sch_cbq]
[ 733.712318] qdisc_destroy+0x62/0x130
[ 733.716401] dsmark_destroy+0x2a/0x70 [sch_dsmark]
[ 733.721745] qdisc_destroy+0x62/0x130
[ 733.725829] qdisc_graft+0x3ba/0x470
[ 733.729817] tc_get_qdisc+0x2a6/0x2c0
[ 733.733901] ? cred_has_capability+0x7d/0x130
[ 733.738761] rtnetlink_rcv_msg+0x263/0x2d0
[ 733.743330] ? rtnl_calcit.isra.30+0x110/0x110
[ 733.748287] netlink_rcv_skb+0x4d/0x130
[ 733.752576] netlink_unicast+0x1a3/0x250
[ 733.756949] netlink_sendmsg+0x2ae/0x3a0
[ 733.761324] sock_sendmsg+0x36/0x40
[ 733.765213] ___sys_sendmsg+0x26f/0x2d0
[ 733.769493] ? handle_pte_fault+0x586/0xdf0
[ 733.774158] ? __handle_mm_fault+0x389/0x500
[ 733.778919] ? __sys_sendmsg+0x5e/0xa0
[ 733.783099] __sys_sendmsg+0x5e/0xa0
[ 733.787087] do_syscall_64+0x5b/0x180
[ 733.791171] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 733.796805] RIP: 0033:0x7f9117f23f10
[ 733.800791] Code: c3 48 8b 05 82 6f 2c 00 f7 db 64 89 18 48 83 cb ff eb dd 0f 1f 80 00 00 00 00 83 3d 8d d0 2c 00 00 75 10 b8 2e 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8
[ 733.821873] RSP: 002b:00007ffe96818398 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[ 733.830319] RAX: ffffffffffffffda RBX: 000000005b71244c RCX: 00007f9117f23f10
[ 733.838280] RDX: 0000000000000000 RSI: 00007ffe968183e0 RDI: 0000000000000003
[ 733.846241] RBP: 00007ffe968183e0 R08: 000000000000ffff R09: 0000000000000003
[ 733.854202] R10: 00007ffe96817e20 R11: 0000000000000246 R12: 0000000000000000
[ 733.862161] R13: 0000000000662ee0 R14: 0000000000000000 R15: 0000000000000000
[ 733.870121] ---[ end trace 28edd4aad712ddca ]---
This is because we didn't update f->result.res when create new filter. Then in
tcindex_delete() -> tcf_unbind_filter(), we will failed to find out the res
and unbind filter, which will trigger the WARN_ON() in cbq_destroy_class().
Fix it by updating f->result.res when create new filter.
Fixes: 6e0565697a106 ("net_sched: fix another crash in cls_tcindex")
Reported-by: Li Shuang <shuali@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
net/sched/cls_tcindex.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index ddaa4e6..9ccc93f 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -465,6 +465,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
struct tcindex_filter *nfp;
struct tcindex_filter __rcu **fp;
+ f->result.res = r->res;
tcf_exts_change(&f->result.exts, &r->exts);
fp = cp->h + (handle % cp->hash);
--
2.5.5
^ permalink raw reply related
* Re: [PATCH bpf-next V3] net/xdp: Fix suspicious RCU usage warning
From: Tariq Toukan @ 2018-08-13 10:57 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Tariq Toukan
Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Eran Ben Elisha,
Neil Brown, Paul E. McKenney
In-Reply-To: <20180813123159.1f447108@redhat.com>
On 13/08/2018 1:31 PM, Jesper Dangaard Brouer wrote:
> On Mon, 13 Aug 2018 12:21:58 +0300
> Tariq Toukan <tariqt@mellanox.com> wrote:
>
>> Fix the warning below by calling rhashtable_lookup_fast.
>> Also, make some code movements for better quality and human
>> readability.
>>
>> [ 342.450870] WARNING: suspicious RCU usage
>> [ 342.455856] 4.18.0-rc2+ #17 Tainted: G O
>> [ 342.462210] -----------------------------
>> [ 342.467202] ./include/linux/rhashtable.h:481 suspicious rcu_dereference_check() usage!
>> [ 342.476568]
>> [ 342.476568] other info that might help us debug this:
>> [ 342.476568]
>> [ 342.486978]
>> [ 342.486978] rcu_scheduler_active = 2, debug_locks = 1
>> [ 342.495211] 4 locks held by modprobe/3934:
>> [ 342.500265] #0: 00000000e23116b2 (mlx5_intf_mutex){+.+.}, at:
>> mlx5_unregister_interface+0x18/0x90 [mlx5_core]
>> [ 342.511953] #1: 00000000ca16db96 (rtnl_mutex){+.+.}, at: unregister_netdev+0xe/0x20
>> [ 342.521109] #2: 00000000a46e2c4b (&priv->state_lock){+.+.}, at: mlx5e_close+0x29/0x60
>> [mlx5_core]
>> [ 342.531642] #3: 0000000060c5bde3 (mem_id_lock){+.+.}, at: xdp_rxq_info_unreg+0x93/0x6b0
>> [ 342.541206]
>> [ 342.541206] stack backtrace:
>> [ 342.547075] CPU: 12 PID: 3934 Comm: modprobe Tainted: G O 4.18.0-rc2+ #17
>> [ 342.556621] Hardware name: Dell Inc. PowerEdge R730/0H21J3, BIOS 1.5.4 10/002/2015
>> [ 342.565606] Call Trace:
>> [ 342.568861] dump_stack+0x78/0xb3
>> [ 342.573086] xdp_rxq_info_unreg+0x3f5/0x6b0
>> [ 342.578285] ? __call_rcu+0x220/0x300
>> [ 342.582911] mlx5e_free_rq+0x38/0xc0 [mlx5_core]
>> [ 342.588602] mlx5e_close_channel+0x20/0x120 [mlx5_core]
>> [ 342.594976] mlx5e_close_channels+0x26/0x40 [mlx5_core]
>> [ 342.601345] mlx5e_close_locked+0x44/0x50 [mlx5_core]
>> [ 342.607519] mlx5e_close+0x42/0x60 [mlx5_core]
>> [ 342.613005] __dev_close_many+0xb1/0x120
>> [ 342.617911] dev_close_many+0xa2/0x170
>> [ 342.622622] rollback_registered_many+0x148/0x460
>> [ 342.628401] ? __lock_acquire+0x48d/0x11b0
>> [ 342.633498] ? unregister_netdev+0xe/0x20
>> [ 342.638495] rollback_registered+0x56/0x90
>> [ 342.643588] unregister_netdevice_queue+0x7e/0x100
>> [ 342.649461] unregister_netdev+0x18/0x20
>> [ 342.654362] mlx5e_remove+0x2a/0x50 [mlx5_core]
>> [ 342.659944] mlx5_remove_device+0xe5/0x110 [mlx5_core]
>> [ 342.666208] mlx5_unregister_interface+0x39/0x90 [mlx5_core]
>> [ 342.673038] cleanup+0x5/0xbfc [mlx5_core]
>> [ 342.678094] __x64_sys_delete_module+0x16b/0x240
>> [ 342.683725] ? do_syscall_64+0x1c/0x210
>> [ 342.688476] do_syscall_64+0x5a/0x210
>> [ 342.693025] entry_SYSCALL_64_after_hwframe+0x49/0xbe
>>
>> Fixes: 8d5d88527587 ("xdp: rhashtable with allocator ID to pointer mapping")
>> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
>> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
>> ---
>> net/core/xdp.c | 13 +++----------
>> 1 file changed, 3 insertions(+), 10 deletions(-)
>>
>> V2 -> V3:
>> * Fix return value test for rhashtable_remove_fast, per Jesper's comment.
>>
>> V1 -> V2:
>> * Use rhashtable_lookup_fast and make some code movements, per Daniel's
>> and Alexei's comments.
>>
>> Please queue to -stable v4.18.
>>
>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>> index 3dd99e1c04f5..8b1c7b699982 100644
>> --- a/net/core/xdp.c
>> +++ b/net/core/xdp.c
>> @@ -105,16 +105,9 @@ static void __xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
>>
>> mutex_lock(&mem_id_lock);
>>
>> - xa = rhashtable_lookup(mem_id_ht, &id, mem_id_rht_params);
>> - if (!xa) {
>> - mutex_unlock(&mem_id_lock);
>> - return;
>> - }
>> -
>> - err = rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params);
>> - WARN_ON(err);
>> -
>> - call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>> + xa = rhashtable_lookup_fast(mem_id_ht, &id, mem_id_rht_params);
>> + if (xa && !rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params))
>> + call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>>
>> mutex_unlock(&mem_id_lock);
>> }
>
> I would personally prefer to write it as in the example as "== 0", look
> at example in [1] section "Object removal", but is it semantically the
> same to write !rhashtable_remove_fast(). So, I'm fine with this.
>
I thought the coding convention is to not explicitly compare to zero,
just like we do not compare to NULL on page allocation, but do:
if (!page)
But I don't mind changing this one.
> In the example[1], the sequence is wrapped in rcu_read_lock/unlock,
> while you have not done so. The rhashtable_lookup_fast and
> rhashtable_remove_fast calls have their own rcu_read_lock/unlock, but
> the outer rcu_read_lock/unlock, makes sure that a RCU period cannot
> slip in between the two calls.
>
> I still think your fix is correct, due to the mutex_lock. Given the
> mutex sync removal and insert in this rhashtable.
>
Right, we rely here on the mutex to avoid the scenario you described.
So the outer rcu lock calls are not necessary.
> I do wonder if it would be better to add the outer rcu_read_lock/unlock,
> calls if someone else reads and copy-paste this code (and don't have an
> mutex sync scheme) ?
>
Yeah it'll be safer for future unaware developers, but I think reviewers
should always comment and make it clear that the best generic reference
is [1], not any specific/optimized use case.
If you guys still want to me to fix this then please let me know and
I'll re-spin.
> If you think this is all fine, and want to proceed as is then you have
> my ack:
>
> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
>
> [1] https://lwn.net/Articles/751374/
>
^ 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