* Re: [PATCH v3 7/8] net: phy: Add support to configure clock in Broadcom iProc mdio mux
From: Russell King - ARM Linux @ 2018-08-01 18:46 UTC (permalink / raw)
To: Florian Fainelli
Cc: Arun Parameswaran, David S. Miller, Andrew Lunn, Rob Herring,
Mark Rutland, Ray Jui, Scott Branden, Catalin Marinas,
Will Deacon, netdev, bcm-kernel-feedback-list, linux-kernel,
linux-arm-kernel, devicetree
In-Reply-To: <0cc6db4f-7008-0ad3-58d7-9e93060f152f@gmail.com>
On Wed, Aug 01, 2018 at 11:40:33AM -0700, Florian Fainelli wrote:
> On 08/01/2018 10:56 AM, Arun Parameswaran wrote:
> > static int iproc_mdio_wait_for_idle(void __iomem *base, bool result)
> > @@ -204,6 +225,20 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
> > return -ENOMEM;
> > }
> >
> > + md->core_clk = devm_clk_get(&pdev->dev, NULL);
> > + if (IS_ERR(md->core_clk)) {
> > + if (PTR_ERR(md->core_clk) == -EPROBE_DEFER)
> > + return -EPROBE_DEFER;
> > +
> > + md->core_clk = NULL;
>
> I would simplify this a bit:
>
> if (IS_ERR(md->core_clk) && PTR_ERR(md->core_clk) == -EPROBE_DEFER)
Even better is:
if (md->core_clk == ERR_PTR(-EPROBE_DEFER))
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up
According to speedtest.net: 13Mbps down 490kbps up
^ permalink raw reply
* Re: [PATCH net] rxrpc: Fix user call ID check in rxrpc_service_prealloc_one
From: David Miller @ 2018-08-01 18:52 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-afs, linux-kernel
In-Reply-To: <153312644299.23453.6879704964860340038.stgit@warthog.procyon.org.uk>
From: David Howells <dhowells@redhat.com>
Date: Wed, 01 Aug 2018 13:27:23 +0100
> From: YueHaibing <yuehaibing@huawei.com>
>
> There just check the user call ID isn't already in use, hence should
> compare user_call_ID with xcall->user_call_ID, which is current
> node's user_call_ID.
>
> Fixes: 540b1c48c37a ("rxrpc: Fix deadlock between call creation and sendmsg/recvmsg")
> Suggested-by: David Howells <dhowells@redhat.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net-next 00/10] rxrpc: Development
From: David Miller @ 2018-08-01 18:52 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-afs, linux-kernel
In-Reply-To: <153312832565.27702.15302541876382957550.stgit@warthog.procyon.org.uk>
From: David Howells <dhowells@redhat.com>
Date: Wed, 01 Aug 2018 13:58:45 +0100
>
> Here are some patches that add some more tracepoints to AF_RXRPC and fix
> some issues therein. The most significant points are:
...
> The patches are tagged here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
> rxrpc-next-20180801
Pulled, thanks David.
^ permalink raw reply
* Re: linux-next: manual merge of the net-next tree with the rdma tree
From: Jason Gunthorpe @ 2018-08-01 17:13 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Parav Pandit, David Miller, Networking, Doug Ledford,
Linux-Next Mailing List, Linux Kernel Mailing List, Ursula Braun,
Leon Romanovsky, linux-rdma@vger.kernel.org
In-Reply-To: <20180801153345.22692adb@canb.auug.org.au>
On Wed, Aug 01, 2018 at 03:33:45PM +1000, Stephen Rothwell wrote:
> Hi Parav,
>
> On Tue, 31 Jul 2018 21:12:00 +0000 Parav Pandit <parav@mellanox.com> wrote:
> >
> > You might want to consider this compatibility patch in Linux-rdma
> > tree to avoid a merge conflict of smc.
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit/?h=for-next&id=7aaa1807e698f73094b78f0ef25b1a37a4409a55
>
> Ok, since commit that is now in the rdma tree, my resolution of the
> original conflicts comes down to dropping all the changes to
> net/smc/smc_core.c net/smc/smc_ib.c that come from the rdma tree and
> adding
>
> #include <rdma/ib_cache.h>
>
> to net/smc/smc_ib.c.
Oh, that means I put the compat inline in the wrong header? Sigh.
Jason
^ permalink raw reply
* Re: SLAB_TYPESAFE_BY_RCU without constructors (was Re: [PATCH v4 13/17] khwasan: add hooks implementation)
From: Dmitry Vyukov @ 2018-08-01 16:47 UTC (permalink / raw)
To: Eric Dumazet
Cc: Christopher Lameter, Eric Dumazet, Andrey Ryabinin,
Linus Torvalds, Theodore Ts'o, Jan Kara, linux-ext4,
Greg Kroah-Hartman, Pablo Neira Ayuso, Jozsef Kadlecsik,
Florian Westphal, David Miller, NetFilter, coreteam, netdev,
Gerrit Renker, dccp, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
David
In-Reply-To: <bf435a04-b689-ec5a-f5df-f47807b43316@gmail.com>
On Wed, Aug 1, 2018 at 6:25 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On 08/01/2018 09:22 AM, Christopher Lameter wrote:
>> On Wed, 1 Aug 2018, Eric Dumazet wrote:
>>
>>> The idea of having a ctor() would only be a win if all the fields that
>>> can be initialized in the ctor are contiguous and fill an integral
>>> number of cache lines.
>>
>> Ok. Its reducing code size and makes the object status more consistent.
>> Isn't that enough?
>>
>
> Prove it ;)
>
> I yet have to seen actual numbers.
Proving with numbers is required for a claimed performance improvement
at the cost of code degradation/increase. For a win-win change there
is really nothing to prove.
^ permalink raw reply
* Re: [Patch net-next] net: hns3: fix return value error while hclge_cmd_csq_clean failed
From: David Miller @ 2018-08-01 17:03 UTC (permalink / raw)
To: tanhuazhong; +Cc: netdev, linuxarm
In-Reply-To: <1533117208-164535-1-git-send-email-tanhuazhong@huawei.com>
From: Huazhong Tan <tanhuazhong@huawei.com>
Date: Wed, 1 Aug 2018 17:53:28 +0800
> From: fredalu <fredalu@yeah.net>
>
> While cleaning the command queue, the value of the HEAD register is not
> in the range of next_to_clean and next_to_use, meaning that this value
> is invalid. This also means that there is a hardware error and the
> hardware will trigger a reset soon. At this time we should return an
> error code instead of 0, and HCLGE_STATE_CMD_DISABLE needs to be set to
> prevent sending command again.
>
> Fixes: 3ff504908f95 ("net: hns3: fix a dead loop in hclge_cmd_csq_clean")
> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>wq
---> ^^^
I think I know what text editor you use... :-)
Applied with 'wq' removed from your signoff... Thanks.
^ permalink raw reply
* Re: [PATCH net-next] net: change Exar/Neterion menu items to be alphabetical
From: David Miller @ 2018-08-01 16:50 UTC (permalink / raw)
To: jdmason; +Cc: helgaas, netdev, linux-kernel
In-Reply-To: <20180731155619.16310-1-jdmason@kudzu.us>
From: Jon Mason <jdmason@kudzu.us>
Date: Tue, 31 Jul 2018 11:56:19 -0400
> Neterion was standalone for several years, then acquired by Exar and
> shutdown in 11 months without ever making any new Exar branded adapters.
> Users would probably think of them as Neterion and not Exar (as there
> have been no follow-on adapters and the vast majority ever sold were
> under the Neterion name).
>
> 6c541b4595a2 ("net: ethernet: Sort Kconfig sourcing alphabetically")
> sorted Kconfig sourcing based on directory names, but in a couple cases,
> the menu item text is quite different from the directory name and is not
> sorted correctly:
>
> drivers/net/ethernet/neterion/Kconfig => "Exar devices"
>
> To address that and clear up any confusion about the name, "Exar" was
> changed to "Neterion (Exar)" and the relevant entries in the Makefile
> and Kconfig were reordered to match the alphabetical organization.
>
> Inspired-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Jon Mason <jdmason@kudzu.us>
Applied.
^ permalink raw reply
* Re: [patch net-next v2 0/3] net: sched: couple of adjustments/fixes
From: David Miller @ 2018-08-01 17:06 UTC (permalink / raw)
To: jiri; +Cc: netdev, jhs, xiyou.wangcong, jakub.kicinski, mlxsw
In-Reply-To: <20180801103657.10532-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 1 Aug 2018 12:36:54 +0200
> From: Jiri Pirko <jiri@mellanox.com>
>
> Jiri Pirko (3):
> net: sched: change name of zombie chain to "held_by_acts_only"
> net: sched: fix notifications for action-held chains
> net: sched: make tcf_chain_{get,put}() static
Series applied, thanks Jiri.
^ permalink raw reply
* Re: [PATCH net-next] cxgb4: fix endian to test F_FW_PORT_CMD_DCBXDIS32
From: David Miller @ 2018-08-01 17:08 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan, indranil, leedom
In-Reply-To: <1533127532-6568-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Wed, 1 Aug 2018 18:15:32 +0530
> For FW_PORT_ACTION_GET_PORT_INFO32 messages, the
> u.info32.lstatus32_to_cbllen32 is 32-bit Big Endian.
> We need to translate that to CPU Endian in order to
> test F_FW_PORT_CMD_DCBXDIS32.
>
> Signed-off-by: Casey Leedom <leedom@chelsio.com>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH] netlink: Fix spectre v1 gadget in netlink_create()
From: David Miller @ 2018-08-01 16:51 UTC (permalink / raw)
To: jcline; +Cc: netdev, linux-kernel, jpoimboe
In-Reply-To: <20180731211316.12971-1-jcline@redhat.com>
From: Jeremy Cline <jcline@redhat.com>
Date: Tue, 31 Jul 2018 21:13:16 +0000
> 'protocol' is a user-controlled value, so sanitize it after the bounds
> check to avoid using it for speculative out-of-bounds access to arrays
> indexed by it.
>
> This addresses the following accesses detected with the help of smatch:
>
> * net/netlink/af_netlink.c:654 __netlink_create() warn: potential
> spectre issue 'nlk_cb_mutex_keys' [w]
>
> * net/netlink/af_netlink.c:654 __netlink_create() warn: potential
> spectre issue 'nlk_cb_mutex_key_strings' [w]
>
> * net/netlink/af_netlink.c:685 netlink_create() warn: potential spectre
> issue 'nl_table' [w] (local cap)
>
> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> Signed-off-by: Jeremy Cline <jcline@redhat.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net-next v1] net: add helpers checking if socket can be bound to nonlocal address
From: David Miller @ 2018-08-01 16:50 UTC (permalink / raw)
To: vincent; +Cc: kuznet, yoshfuji, netdev, tom
In-Reply-To: <20180731191810.5493-1-vincent@bernat.im>
From: Vincent Bernat <vincent@bernat.im>
Date: Tue, 31 Jul 2018 21:18:11 +0200
> The construction "net->ipv4.sysctl_ip_nonlocal_bind || inet->freebind
> || inet->transparent" is present three times and its IPv6 counterpart
> is also present three times. We introduce two small helpers to
> characterize these tests uniformly.
>
> Signed-off-by: Vincent Bernat <vincent@bernat.im>
Looks great, thanks for doing this.
Applied.
^ permalink raw reply
* Re: [PATCH net-next v2 0/7] ipv4: Control SKB reprioritization after forwarding
From: David Miller @ 2018-08-01 16:52 UTC (permalink / raw)
To: petrm
Cc: netdev, linux-doc, linux-kselftest, corbet, jiri, idosch, kuznet,
yoshfuji, shuah, nikolay, dsahern
In-Reply-To: <cover.1533076217.git.petrm@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
Date: Wed, 01 Aug 2018 00:35:53 +0200
> After IPv4 packets are forwarded, the priority of the corresponding SKB
> is updated according to the TOS field of IPv4 header. This overrides any
> prioritization done earlier by e.g. an skbedit action or ingress-qos-map
> defined at a vlan device.
>
> Such overriding may not always be desirable. Even if the packet ends up
> being routed, which implies this is an L3 network node, an administrator
> may wish to preserve whatever prioritization was done earlier on in the
> pipeline.
>
> Therefore this patch set introduces a sysctl that controls this
> behavior, net.ipv4.ip_forward_update_priority. It's value is 1 by
> default to preserve the current behavior.
>
> All of the above is implemented in patch #1.
>
> Value changes prompt a new NETEVENT_IPV4_FWD_UPDATE_PRIORITY_UPDATE
> notification, so that the drivers can hook up whatever logic may depend
> on this value. That is implemented in patch #2.
>
> In patches #3 and #4, mlxsw is adapted to recognize the sysctl. On
> initialization, the RGCR register that handles router configuration is
> set in accordance with the sysctl. The new notification is listened to
> and RGCR is reconfigured as necessary.
>
> In patches #5 to #7, a selftest is added to verify that mlxsw reflects
> the sysctl value as necessary. The test is expressed in terms of the
> recently-introduced ieee_setapp support, and works by observing how DSCP
> value gets rewritten depending on packet priority. For this reason, the
> test is added to the subdirectory drivers/net/mlxsw. Even though it's
> not particularly specific to mlxsw, it's not suitable for running on
> soft devices (which don't support the ieee_setapp et.al.).
...
Series applied, thank you.
^ permalink raw reply
* Re: how PHY driver is notified that cable is unplugged? (possibly related to IFF_RUNNING flag)
From: Florian Fainelli @ 2018-08-01 17:31 UTC (permalink / raw)
To: rpjday, netdev, andrew
In-Reply-To: <20180801085826.Horde.rj4qsfJlofF1IKzzUQgwFrO@crashcourse.ca>
On 08/01/2018 05:58 AM, rpjday@crashcourse.ca wrote:
>
> (warning that i have a few questions that are probably trivial until i
> get up to speed
> with networking code.)
>
> a colleague asked for advice about the following -- apparently a new
> PHY driver works
> properly when being brought up with "ifconfig <ifname> up", part of that
> process
> apparently setting the IFF_RUNNING net_device flags bit. so far, so good.
>
> now, when the cable is physically unplugged, the claim is that there
> is no obvious
> status change for that port, accompanied by the suggestion that it is
> that IFF_RUNNING
> flag bit that is not being unset.
>
> asking a more general question, where can i read up on the proper
> protocol for
> a driver being notified of, and properly handling, physical
> disconnection on that
> port? and, of course, the cable being plugged back in.
The basic mechanism used by the PHY library is to read the standard
Basic Mode Status Register and check the Link status bit to determine
what the state of the link is set. This event can be triggered either
through polling, or the use of an interrupt that the PHY is generating.
Once the link state is determined, because the PHY device is "connected"
to a network device, the PHY library can call netif_carrier_{on,off}
against the network device attached to the PHY and that propagates
through the networking stack and sets the appropriate bits, including
IFF_RUNNING.
Hope this helps.
--
Florian
^ permalink raw reply
* Re: [PATCH v1 2/3] zinc: Introduce minimal cryptography library
From: Andy Lutomirski @ 2018-08-01 17:02 UTC (permalink / raw)
To: Eric Biggers
Cc: Jason A. Donenfeld, Linux Crypto Mailing List, LKML,
Network Development, David S. Miller, Andy Lutomirski, Greg KH,
Samuel Neves, D . J . Bernstein, Tanja Lange,
Jean-Philippe Aumasson, Karthikeyan Bhargavan
In-Reply-To: <20180801072246.GA15677@sol.localdomain>
[I reordered the snippets below for readability.]
> On Aug 1, 2018, at 12:22 AM, Eric Biggers <ebiggers3@gmail.com> wrote:
> In general this is great work, and I'm very excited for WireGuard to be
> upstreamed! But for the new crypto code, I think a few things are on
> the wrong track, for example treating it is a special library. Even the
> name is contradicting itself: Zinc is "not crypto/", yet as you stated
> it's intended that the "Zinc" algorithms will be exposed through the
> crypto API -- just like how most of the existing crypto code in lib/ is
> also exposed through the crypto API. So, I think that what you're doing
> isn't actually *that* different from what already exists in some cases;
> and pretending that it is very different is just going to cause
> problems. Rather, the actual truly new thing seems to be that the
> dispatch to architecture specific implementations is done at the lib/
> level instead of handled by the crypto API priority numbers.
...
>
>
> I think the above changes would also naturally lead to a much saner
> patch series where each algorithm is added by its own patch, rather than
> one monster patch that adds many algorithms and 24000 lines of code.
>
Yes, please.
>
> As for doing the architecture-specific dispatch in lib/ rather than
> through the crypto API, there definitely are some arguments in favor of
> it. The main problem, though, is that your code is a mess due to all
> the #ifdefs, and it will only get worse as people add more
> architectures. You may think you already added all the architectures
> that matter, but tomorrow people will come and want to add PowerPC,
> RISC-V, etc. I really think you should consider splitting up
> implementations by architecture; this would *not*, however, preclude the
> implementations from still being accessed through a single top-level
> "glue" function. For example chacha20() could look like:
>
> void chacha20(struct chacha20_ctx *state, u8 *dst, const u8 *src, u32 len,
> bool have_simd)
> {
> if (chacha20_arch(dst, src, len, state->key, state->counter, have_simd))
> goto out;
>
> chacha20_generic(dst, src, len, state->key, state->counter);
>
> out:
> state->counter[0] += (len + 63) / 64;
> }
I like this a *lot*. (But why are you passing have_simd? Shouldn't
that check live in chacha20_arch? If there's some init code needed,
then chacha20_arch() should just return false before the init code
runs. Once the arch does whatever feature detection it needs, it can
make chacha20_arch() start returning true.)
As I see it, there there are two truly new thing in the zinc patchset:
the direct (in the direct call sense) arch dispatch, and the fact that
the functions can be called directly, without allocating contexts,
using function pointers, etc.
In fact, I had a previous patch set that added such an interface for SHA256.
https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=crypto/sha256_bpf&id=8c59a4dd8b7ba4f2e5a6461132bbd16c83ff7c1f
https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=crypto/sha256_bpf&id=7e5fbc02972b03727b71bc71f84175c36cbf01f5
>
> Your patch description is also missing any mention of crypto accelerator
> hardware. Quite a bit of the complexity in the crypto API, such as
> scatterlist support and asynchronous execution, exists because it
> supports crypto accelerators. AFAICS your new APIs cannot support
> crypto accelerators, as your APIs are synchronous and operate on virtual
> addresses. I assume your justification is that "djb algorithms" like
> ChaCha and Poly1305 don't need crypto accelerators as they are fast in
> software. But you never explicitly stated this and discussed the
> tradeoffs. Since this is basically the foundation for the design you've
> chosen, it really needs to be addressed.
I see this as an advantage, not a disadvantage. A very large majority
of in-kernel crypto users (by number of call sites under a *very*
brief survey, not by number of CPU cycles) just want to do some
synchronous crypto on a buffer that is addressed by a regular pointer.
Most of these users would be slowed down if they used any form of
async crypto, since the CPU can complete the whole operation faster
than it could plausibly initiate and complete anything asynchronous.
And, right now, they suffer the full overhead of allocating a context
(often with alloca!), looking up (or caching) some crypto API data
structures, dispatching the operation, and cleaning up.
So I think the right way to do it is to have directly callable
functions like zinc uses and to have the fancy crypto API layer on top
of them. So if you actually want async accelerated crypto with
scatterlists or whatever, you can call into the fancy API, and the
fancy API can dispatch to hardware or it can dispatch to the normal
static API.
In fact, this is exactly what my patch above did. But Jason's work is
more complete than mine, and mine wasn't really done because it had
some configury issues.
All that being said, for algorithms where the crypto API already has a
reasonable implementation, I think the patch series should first
restructure the code (so the actual software implementation is moved
away from the crypto API wrappers) and then, if needed, replace the
implementation with something better. The latter should be its own
patch with its own justification.
^ permalink raw reply
* Re: [PATCH net-next] tcp: remove set but not used variable 'skb_size'
From: David Miller @ 2018-08-01 16:57 UTC (permalink / raw)
To: weiyongjun1; +Cc: edumazet, kuznet, yoshfuji, netdev, kernel-janitors
In-Reply-To: <1533088796-149451-1-git-send-email-weiyongjun1@huawei.com>
From: Wei Yongjun <weiyongjun1@huawei.com>
Date: Wed, 1 Aug 2018 01:59:56 +0000
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> net/ipv4/tcp_output.c: In function 'tcp_collapse_retrans':
> net/ipv4/tcp_output.c:2700:6: warning:
> variable 'skb_size' set but not used [-Wunused-but-set-variable]
> int skb_size, next_skb_size;
> ^
>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH v2 net-next 0/5] tcp: add 4 new stats
From: David Miller @ 2018-08-01 16:56 UTC (permalink / raw)
To: weiwan; +Cc: netdev, edumazet, ncardwell, soheil, ycheng
In-Reply-To: <20180801004624.154346-1-tracywwnj@gmail.com>
From: Wei Wang <weiwan@google.com>
Date: Tue, 31 Jul 2018 17:46:19 -0700
> From: Wei Wang <weiwan@google.com>
>
> This patch series adds 3 RFC4898 stats:
> 1. tcpEStatsPerfHCDataOctetsOut
> 2. tcpEStatsPerfOctetsRetrans
> 3. tcpEStatsStackDSACKDups
> and an addtional stat to record the number of data packet reordering
> events seen:
> 4. tcp_reord_seen
>
> Together with the existing stats, application can use them to measure
> the retransmission rate in bytes, exclude spurious retransmissions
> reflected by DSACK, and keep track of the reordering events on live
> connections.
> In particular the networks with different MTUs make bytes-based loss stats
> more useful. Google servers have been using these stats for many years to
> instrument transport and network performance.
>
> Note: The first patch is a refactor to add a helper to calculate
> opt_stats size in order to make later changes cleaner.
Series applied, thank you.
^ permalink raw reply
* Re: KASAN: use-after-free Read in rtnetlink_put_metrics
From: Sabrina Dubroca @ 2018-08-01 19:26 UTC (permalink / raw)
To: David Miller
Cc: xiyou.wangcong, eric.dumazet, syzbot+41f9c04b50ef70c66947,
christian.brauner, dsahern, fw, jbenc, ktkhai, linux-kernel,
lucien.xin, netdev, syzkaller-bugs
In-Reply-To: <20180801.114636.279269263935333136.davem@davemloft.net>
2018-08-01, 11:46:36 -0700, David Miller wrote:
> From: Cong Wang <xiyou.wangcong@gmail.com>
> Date: Tue, 31 Jul 2018 16:03:13 -0700
>
> > Looks like this commit is completely unnecessary,
> > fib6_drop_pcpu_from() calls fib6_info_release()
> > which calls fib6_info_destroy_rcu(), so this metrics
> > will be released twice...
>
> And even if there was a leak here, it's illegal to free this
> metrics memory synchronously since it is RCU protected.
Yeah, I noticed that today, but I don't think that's the problem we're
seeing here.
> That's why it normally goes through fib6_info_destroy_rcu().
>
> Sabrina, I'm going to revert your changes unless I see some
> progress here by the end of today.
Yeah, I'm fine with a revert, we can fix the leak later.
syzbot hasn't found a reproducer so I'm not sure it's the same issue,
but I ran into this: we can create a route, start using it, and then
give it some metrics. In that case, we'll hit rt6_set_from() with the
default metrics, so we don't refcount them. Then fib6_metric_set()
will assign the new metrics to the parent route.
Then fib6_drop_pcpu_from will see that the parent route has
non-default metrics, and try to release this, but the percpu copy
doesn't actually hold a reference. Bandaid would be to put a
DST_METRICS_REFCOUNTED check in fib6_drop_pcpu_from().
Looking at rt6_set_from(), it seems we can also do dst_init_metrics
with the old metrics, then refcount the new metrics.
And I'm not sure whether the refcount_set in fib6_metric_set() can't
be reordered so that rt6_set_from() might see the new metrics pointer,
increment the refcount, then fib6_metric_set() would do its
refcount_set, stepping over the previous increment.
--
Sabrina
^ permalink raw reply
* Re: [PATCH] net/mlx5e: Fix uninitialized variable
From: Gustavo A. R. Silva @ 2018-08-01 17:19 UTC (permalink / raw)
To: David Miller; +Cc: tariqt, saeedm, leon, netdev, linux-rdma, linux-kernel
In-Reply-To: <20180801.093824.692154182807362672.davem@davemloft.net>
On 08/01/2018 11:38 AM, David Miller wrote:
> From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
> Date: Tue, 31 Jul 2018 09:21:57 -0500
>
>> There is a potential execution path in which variable *err* is returned
>> without being properly initialized previously.
>>
>> Fix this by initializing variable *err* to 0.
>>
>> Addresses-Coverity-ID: 1472116 ("Uninitialized scalar variable")
>> Fixes: 0ec13877ce95 ("net/mlx5e: Gather all XDP pre-requisite checks in a single function")
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>
> Applied to net-next.
>
David,
Sorry I failed to tag this patch for net-next. I got a bit confused, once based on the logs,
the usual committer for this driver is Saeed Mahameed; not you. So, I wasn't quite sure about
how to proceed.
Thanks
^ permalink raw reply
* [PATCH v3 1/8] dt-bindings: net: Fix Broadcom iProc mdio mux driver base address
From: Arun Parameswaran @ 2018-08-01 17:56 UTC (permalink / raw)
To: David S. Miller, Florian Fainelli, Andrew Lunn, Rob Herring,
Mark Rutland, Ray Jui, Scott Branden, Catalin Marinas,
Will Deacon
Cc: netdev, devicetree, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list, Arun Parameswaran
In-Reply-To: <1533146186-8374-1-git-send-email-arun.parameswaran@broadcom.com>
Modify the base address of the Broadcom iProc MDIO mux driver to
point to the start of the block's register address space.
Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt b/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
index dfe287a..dc8aa68 100644
--- a/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
+++ b/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
@@ -18,9 +18,9 @@ at- Documentation/devicetree/bindings/net/mdio-mux.txt
for example:
- mdio_mux_iproc: mdio-mux@6602023c {
+ mdio_mux_iproc: mdio-mux@66020000 {
compatible = "brcm,mdio-mux-iproc";
- reg = <0x6602023c 0x14>;
+ reg = <0x66020000 0x250>;
#address-cells = <1>;
#size-cells = <0>;
--
1.9.1
^ permalink raw reply related
* [PATCH v3 2/8] net: phy: Fix the register offsets in Broadcom iProc mdio mux driver
From: Arun Parameswaran @ 2018-08-01 17:56 UTC (permalink / raw)
To: David S. Miller, Florian Fainelli, Andrew Lunn, Rob Herring,
Mark Rutland, Ray Jui, Scott Branden, Catalin Marinas,
Will Deacon
Cc: netdev, devicetree, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list, Arun Parameswaran
In-Reply-To: <1533146186-8374-1-git-send-email-arun.parameswaran@broadcom.com>
Modify the register offsets in the Broadcom iProc mdio mux to start
from the top of the register address space.
Earlier, the base address pointed to the end of the block's register
space. The base address will now point to the start of the mdio's
address space. The offsets have been fixed to match this.
Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/mdio-mux-bcm-iproc.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/mdio-mux-bcm-iproc.c b/drivers/net/phy/mdio-mux-bcm-iproc.c
index 0831b71..48bb74a 100644
--- a/drivers/net/phy/mdio-mux-bcm-iproc.c
+++ b/drivers/net/phy/mdio-mux-bcm-iproc.c
@@ -22,7 +22,7 @@
#include <linux/mdio-mux.h>
#include <linux/delay.h>
-#define MDIO_PARAM_OFFSET 0x00
+#define MDIO_PARAM_OFFSET 0x23c
#define MDIO_PARAM_MIIM_CYCLE 29
#define MDIO_PARAM_INTERNAL_SEL 25
#define MDIO_PARAM_BUS_ID 22
@@ -30,20 +30,22 @@
#define MDIO_PARAM_PHY_ID 16
#define MDIO_PARAM_PHY_DATA 0
-#define MDIO_READ_OFFSET 0x04
+#define MDIO_READ_OFFSET 0x240
#define MDIO_READ_DATA_MASK 0xffff
-#define MDIO_ADDR_OFFSET 0x08
+#define MDIO_ADDR_OFFSET 0x244
-#define MDIO_CTRL_OFFSET 0x0C
+#define MDIO_CTRL_OFFSET 0x248
#define MDIO_CTRL_WRITE_OP 0x1
#define MDIO_CTRL_READ_OP 0x2
-#define MDIO_STAT_OFFSET 0x10
+#define MDIO_STAT_OFFSET 0x24c
#define MDIO_STAT_DONE 1
#define BUS_MAX_ADDR 32
#define EXT_BUS_START_ADDR 16
+#define MDIO_REG_ADDR_SPACE_SIZE 0x250
+
struct iproc_mdiomux_desc {
void *mux_handle;
void __iomem *base;
@@ -169,6 +171,14 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
md->dev = &pdev->dev;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (res->start & 0xfff) {
+ /* For backward compatibility in case the
+ * base address is specified with an offset.
+ */
+ dev_info(&pdev->dev, "fix base address in dt-blob\n");
+ res->start &= ~0xfff;
+ res->end = res->start + MDIO_REG_ADDR_SPACE_SIZE - 1;
+ }
md->base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(md->base)) {
dev_err(&pdev->dev, "failed to ioremap register\n");
--
1.9.1
^ permalink raw reply related
* [PATCH v3 4/8] net: phy: Disable external master access in bcm mdio mux driver
From: Arun Parameswaran @ 2018-08-01 17:56 UTC (permalink / raw)
To: David S. Miller, Florian Fainelli, Andrew Lunn, Rob Herring,
Mark Rutland, Ray Jui, Scott Branden, Catalin Marinas,
Will Deacon
Cc: netdev, devicetree, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list, Arun Parameswaran
In-Reply-To: <1533146186-8374-1-git-send-email-arun.parameswaran@broadcom.com>
Configure the scan control register in the Broadcom iProc
mdio mux driver to disable access to external master.
In some SoC's, the scan control register defaults to an incorrect
value.
Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/mdio-mux-bcm-iproc.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/net/phy/mdio-mux-bcm-iproc.c b/drivers/net/phy/mdio-mux-bcm-iproc.c
index 48bb74a..c36ce4b 100644
--- a/drivers/net/phy/mdio-mux-bcm-iproc.c
+++ b/drivers/net/phy/mdio-mux-bcm-iproc.c
@@ -22,6 +22,9 @@
#include <linux/mdio-mux.h>
#include <linux/delay.h>
+#define MDIO_SCAN_CTRL_OFFSET 0x008
+#define MDIO_SCAN_CTRL_OVRIDE_EXT_MSTR 28
+
#define MDIO_PARAM_OFFSET 0x23c
#define MDIO_PARAM_MIIM_CYCLE 29
#define MDIO_PARAM_INTERNAL_SEL 25
@@ -53,6 +56,16 @@ struct iproc_mdiomux_desc {
struct mii_bus *mii_bus;
};
+static void mdio_mux_iproc_config(struct iproc_mdiomux_desc *md)
+{
+ u32 val;
+
+ /* Disable external mdio master access */
+ val = readl(md->base + MDIO_SCAN_CTRL_OFFSET);
+ val |= BIT(MDIO_SCAN_CTRL_OVRIDE_EXT_MSTR);
+ writel(val, md->base + MDIO_SCAN_CTRL_OFFSET);
+}
+
static int iproc_mdio_wait_for_idle(void __iomem *base, bool result)
{
unsigned int timeout = 1000; /* loop for 1s */
@@ -216,6 +229,8 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
goto out_register;
}
+ mdio_mux_iproc_config(md);
+
dev_info(md->dev, "iProc mdiomux registered\n");
return 0;
--
1.9.1
^ permalink raw reply related
* Re: [net-next 10/16] net/mlx5: Support PCIe buffer congestion handling via Devlink
From: Moshe Shemesh @ 2018-08-01 18:28 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Alexander Duyck, Jiri Pirko, Jakub Kicinski, Eran Ben Elisha,
Saeed Mahameed, David S. Miller, netdev@vger.kernel.org,
linux-pci
In-Reply-To: <20180731110649.GI45322@bhelgaas-glaptop.roam.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 15208 bytes --]
On Tue, Jul 31, 2018 at 2:06 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Mon, Jul 30, 2018 at 08:19:50PM -0700, Alexander Duyck wrote:
> > On Mon, Jul 30, 2018 at 7:33 PM, Bjorn Helgaas <helgaas@kernel.org>
> wrote:
> > > On Mon, Jul 30, 2018 at 08:02:48AM -0700, Alexander Duyck wrote:
> > >> On Mon, Jul 30, 2018 at 7:07 AM, Bjorn Helgaas <helgaas@kernel.org>
> wrote:
> > >> > On Sun, Jul 29, 2018 at 03:00:28PM -0700, Alexander Duyck wrote:
> > >> >> On Sun, Jul 29, 2018 at 2:23 AM, Moshe Shemesh <
> moshes20.il@gmail.com> wrote:
> > >> >> > On Sat, Jul 28, 2018 at 7:06 PM, Bjorn Helgaas <
> helgaas@kernel.org> wrote:
> > >> >> >> On Thu, Jul 26, 2018 at 07:00:20AM -0700, Alexander Duyck wrote:
> > >> >> >> > On Thu, Jul 26, 2018 at 12:14 AM, Jiri Pirko <
> jiri@resnulli.us> wrote:
> > >> >> >> > > Thu, Jul 26, 2018 at 02:43:59AM CEST,
> jakub.kicinski@netronome.com
> > >> >> >> > > wrote:
> > >> >> >> > >>On Wed, 25 Jul 2018 08:23:26 -0700, Alexander Duyck wrote:
> > >> >> >> > >>> On Wed, Jul 25, 2018 at 5:31 AM, Eran Ben Elisha wrote:
> > >> >> >> > >>> > On 7/24/2018 10:51 PM, Jakub Kicinski wrote:
> > >> >> >> > >>> >>>> The devlink params haven't been upstream even for a
> full cycle
> > >> >> >> > >>> >>>> and
> > >> >> >> > >>> >>>> already you guys are starting to use them to
> configure standard
> > >> >> >> > >>> >>>> features like queuing.
> > >> >> >> > >>> >>>
> > >> >> >> > >>> >>> We developed the devlink params in order to support
> non-standard
> > >> >> >> > >>> >>> configuration only. And for non-standard, there are
> generic and
> > >> >> >> > >>> >>> vendor
> > >> >> >> > >>> >>> specific options.
> > >> >> >> > >>> >>
> > >> >> >> > >>> >> I thought it was developed for performing non-standard
> and
> > >> >> >> > >>> >> possibly
> > >> >> >> > >>> >> vendor specific configuration. Look at
> DEVLINK_PARAM_GENERIC_*
> > >> >> >> > >>> >> for
> > >> >> >> > >>> >> examples of well justified generic options for which
> we have no
> > >> >> >> > >>> >> other API. The vendor mlx4 options look fairly vendor
> specific
> > >> >> >> > >>> >> if you
> > >> >> >> > >>> >> ask me, too.
> > >> >> >> > >>> >>
> > >> >> >> > >>> >> Configuring queuing has an API. The question is it
> acceptable to
> > >> >> >> > >>> >> enter
> > >> >> >> > >>> >> into the risky territory of controlling offloads via
> devlink
> > >> >> >> > >>> >> parameters
> > >> >> >> > >>> >> or would we rather make vendors take the time and
> effort to model
> > >> >> >> > >>> >> things to (a subset) of existing APIs. The HW never
> fits the
> > >> >> >> > >>> >> APIs
> > >> >> >> > >>> >> perfectly.
> > >> >> >> > >>> >
> > >> >> >> > >>> > I understand what you meant here, I would like to
> highlight that
> > >> >> >> > >>> > this
> > >> >> >> > >>> > mechanism was not meant to handle SRIOV, Representors,
> etc.
> > >> >> >> > >>> > The vendor specific configuration suggested here is to
> handle a
> > >> >> >> > >>> > congestion
> > >> >> >> > >>> > state in Multi Host environment (which includes PF and
> multiple
> > >> >> >> > >>> > VFs per
> > >> >> >> > >>> > host), where one host is not aware to the other hosts,
> and each is
> > >> >> >> > >>> > running
> > >> >> >> > >>> > on its own pci/driver. It is a device working mode
> configuration.
> > >> >> >> > >>> >
> > >> >> >> > >>> > This couldn't fit into any existing API, thus creating
> this
> > >> >> >> > >>> > vendor specific
> > >> >> >> > >>> > unique API is needed.
> > >> >> >> > >>>
> > >> >> >> > >>> If we are just going to start creating devlink interfaces
> in for
> > >> >> >> > >>> every
> > >> >> >> > >>> one-off option a device wants to add why did we even
> bother with
> > >> >> >> > >>> trying to prevent drivers from using sysfs? This just
> feels like we
> > >> >> >> > >>> are back to the same arguments we had back in the day
> with it.
> > >> >> >> > >>>
> > >> >> >> > >>> I feel like the bigger question here is if devlink is how
> we are
> > >> >> >> > >>> going
> > >> >> >> > >>> to deal with all PCIe related features going forward, or
> should we
> > >> >> >> > >>> start looking at creating a new interface/tool for
> PCI/PCIe related
> > >> >> >> > >>> features? My concern is that we have already had features
> such as
> > >> >> >> > >>> DMA
> > >> >> >> > >>> Coalescing that didn't really fit into anything and now
> we are
> > >> >> >> > >>> starting to see other things related to DMA and PCIe bus
> credits.
> > >> >> >> > >>> I'm
> > >> >> >> > >>> wondering if we shouldn't start looking at a
> tool/interface to
> > >> >> >> > >>> configure all the PCIe related features such as
> interrupts, error
> > >> >> >> > >>> reporting, DMA configuration, power management, etc.
> Maybe we could
> > >> >> >> > >>> even look at sharing it across subsystems and include
> things like
> > >> >> >> > >>> storage, graphics, and other subsystems in the
> conversation.
> > >> >> >> > >>
> > >> >> >> > >>Agreed, for actual PCIe configuration (i.e. not ECN
> marking) we do
> > >> >> >> > >> need
> > >> >> >> > >>to build up an API. Sharing it across subsystems would be
> very cool!
> > >> >> >>
> > >> >> >> I read the thread (starting at [1], for anybody else coming in
> late)
> > >> >> >> and I see this has something to do with "configuring outbound
> PCIe
> > >> >> >> buffers", but I haven't seen the connection to PCIe protocol or
> > >> >> >> features, i.e., I can't connect this to anything in the PCIe
> spec.
> > >> >> >>
> > >> >> >> Can somebody help me understand how the PCI core is relevant?
> If
> > >> >> >> there's some connection with a feature defined by PCIe, or if it
> > >> >> >> affects the PCIe transaction protocol somehow, I'm definitely
> > >> >> >> interested in this. But if this only affects the data
> transferred
> > >> >> >> over PCIe, i.e., the data payloads of PCIe TLP packets, then
> I'm not
> > >> >> >> sure why the PCI core should care.
> > >> >> >
> > >> >> > As you wrote, this is not a PCIe feature or affects the PCIe
> transaction
> > >> >> > protocol.
> > >> >> >
> > >> >> > Actually, due to hardware limitation in current device, we have
> enabled a
> > >> >> > workaround in hardware.
> > >> >> >
> > >> >> > This mode is proprietary and not relevant to other PCIe devices,
> thus is set
> > >> >> > using driver-specific parameter in devlink
> > >> >>
> > >> >> Essentially what this feature is doing is communicating the need
> for
> > >> >> PCIe back-pressure to the network fabric. So as the buffers on the
> > >> >> device start to fill because the device isn't able to get back PCIe
> > >> >> credits fast enough it will then start to send congestion
> > >> >> notifications to the network stack itself if I understand this
> > >> >> correctly.
> > >> >
> > >> > This sounds like a hook that allows the device to tell its driver
> > >> > about PCIe flow control credits, and the driver can pass that on to
> > >> > the network stack. IIUC, that would be a device-specific feature
> > >> > outside the scope of the PCI core.
> > >> >
> > >> >> For now there are no major conflicts, but when we start getting
> into
> > >> >> stuff like PCIe DMA coalescing, and on a more general basis just
> PCIe
> > >> >> active state power management that is going to start making things
> > >> >> more complicated going forward.
> > >> >
> > >> > We do support ASPM already in the PCI core, and we do have the
> > >> > pci_disable_link_state() interface, which is currently the only way
> > >> > drivers can influence it. There are several drivers that do their
> own
> > >> > ASPM configuration, but this is not safe because it's not
> coordinated
> > >> > with what the PCI core does. If/when drivers need more control, we
> > >> > should enhance the PCI core interfaces.
> > >>
> > >> This is kind of what I was getting at. It would be useful to have an
> > >> interface of some sort so that drivers get notified when a user is
> > >> making changes to configuration space and I don't know if anything
> > >> like that exists now.
> > >
> > > You mean something like this?
> > >
> > > - driver registers a callback with PCI core
> > > - user runs setpci, which writes PCI config space via sysfs
> > > - kernel hook in pci_write_config() notices write and calls driver
> > > callback
> > > - driver callback receives config address and data written
> > > - driver parses PCI capability lists to identify register
> > >
> > > Nothing like that exists today, and this is not what I had in mind by
> > > "enhance the PCI core interfaces". I'm not sure what the utility of
> > > this is (but I'm not a networking guy by any means).
> >
> > Well in general I have been wondering if setpci is really the cleanest
> > way to do any of this. I have found it can be a fast way to really
> > mess things up. For example using setpci to trigger an FLR is a fast
> > way to cripple an interface. I recall using that approach when testing
> > the fm10k driver to deal with surprise resets.
> >
> > The problem is setpci is REALLY powerful. It lets you change a ton
> > that can impact the driver, and many drivers just read the config
> > space at the probe function and assume it is static (which in most
> > cases it is). That is why I was thinking as a small step it might be
> > useful to have the notifications delivered to the driver if it is
> > registered on the interface so it could prepare or clean-up after a
> > change to the PCI configuration space.
>
> Yep, setpci is very powerful. I doubt it's worth having drivers try
> to react, just because setpci can do completely arbitrary things, many
> of which are not recoverable even in principle. But I do think we
> should make it taint the kernel so we have a clue when things go
> wrong.
>
> > > I think it's a bad idea for drivers to directly write config space.
> > > It would be much better to provide a PCI core interface so we can
> > > implement things once and coordinate things that need to be
> > > coordinated.
> >
> > I agree with that.
> >
> > >> > I don't know what PCIe DMA coalescing means, so I can't comment on
> > >> > that.
> > >>
> > >> There are devices, specifically network devices, that will hold off on
> > >> switching between either L0s or L1 and L0 by deferring DMA operations.
> > >> Basically the idea is supposed to be to hold off bringing the link up
> > >> for as long as possible in order to maximize power savings for the
> > >> ASPM state. This is something that has come up in the past, and I
> > >> don't know if there has been any interface determined for how to
> > >> handle this sort of configuration. Most of it occurs through MMIO.
> > >
> > > The device can certainly delay L0s or L1 exit if it wants to. If
> > > there are knobs to control this, e.g., how long it can defer a DMA, it
> > > makes sense that they would be device-specific and in MMIO space. The
> > > PCI core can't be involved in that because in general it knows nothing
> > > about the contents of MMIO BARs. Presumably those knobs would work
> > > within the framework of ASPM as defined by the PCIe spec, e.g., if we
> > > disable ASPM, the knobs would do nothing because there is no L0s or L1
> > > at all.
> > >
> > > That's not to say that device designers couldn't get together and
> > > define a common model for such knobs that puts them at well-known
> > > offsets in well-known BARs. All I'm saying is that this sounds like
> > > it's currently outside the realm of the PCI specs and the PCI core.
> >
> > Some of this was already handled in LTR and OBFF. I think the DMA
> > coalescing was meant to work either with those, or optionally on its
> > own. As far as being ASPM dependent or not I don't think it really
> > cared all that much other than knowing if ASPM was enabled and how
> > long it takes for a given device to come out of either of those
> > states. The general idea with DMA coalescing was to try and save power
> > on the CPU and then possibly the PCIe link if the value was set high
> > enough and ASPM was enabled.
> >
> > Anyway we have kind of gotten off into a tangent as I was just citing
> > something that might end up having an interaction with a feature such
> > as notifying the stack that the Rx buffer on a given device has become
> > congested.
> >
> > >> >> I assume the devices we are talking about supporting this new
> feature
> > >> >> on either don't deal with ASPM or assume a quick turnaround to get
> out
> > >> >> of the lower power states? Otherwise that would definitely cause
> some
> > >> >> back-pressure buildups that would hurt performance.
> > >> >
> > >> > Devices can communicate the ASPM exit latency they can tolerate via
> > >> > the Device Capabilities register (PCIe r4.0, sec 76.5.3.3). Linux
> > >> > should be configuring ASPM to respect those device requirements.
> > >>
> > >> Right. But my point was something like ASPM will add extra complexity
> > >> to a feature such as what has been described here. My concern is that
> > >> I don't want us implementing stuff on a per-driver basis that is not
> > >> all that unique to the device. I don't really see the feature that was
> > >> described above as being something that will stay specific to this one
> > >> device for very long, especially if it provides added value. Basically
> > >> all it is doing is allowing exposing PCIe congestion management to
> > >> upper levels in the network stack. I don't even necessarily see it as
> > >> being networking specific as I would imagine there might be other
> > >> types of devices that could make use of knowing how many transactions
> > >> and such they could process at the same time.
> > >
> > > It sounds to me like you need a library that can be used by all the
> > > drivers that need this functionality. Unless it's something
> > > documented in the PCIe specs so we can rely on a standard way of
> > > discovering and configuring things, I don't see how the PCI core can
> > > really be involved.
> > >
> > > Bjorn
> >
> > I am kind of thinking that a common library would be a preferred way
> > to go, or at least a common interface to share between the drivers. It
> > wasn't my intention to imply that the PCI core needed to get involved.
> > I was including the linux-pci list as more of a way of checking to get
> > the broader audience's input since we were just discussing this on
> > netdev.
> >
> > My main concern is that those of us on netdev have historically we
> > have been dealing with features such as SR-IOV as a networking thing,
> > when really it has been a combination of a PCI feature and some
> > network switching. I might have ratholed this a bit, as I kind of see
> > this topic as something similar.
>
> OK, thanks. Sounds like we have similar perspectives and I appreciate
> being looped in!
>
> Bjorn
>
Thanks for your comments, as already understood this is not a pci core
feature, this is a NIC specific hardware limitation workaround.
It is not common to drivers, not even within Mellanox drivers. The previous
NIC generation driver mlx4 does not need it. The next NIC won't need it
too, it should be handled by hardware automatically.
We are sending version 2 with clarification on this in cover letter.
[-- Attachment #2: Type: text/html, Size: 21975 bytes --]
^ permalink raw reply
* Re: [PATCH v4 bpf-next 08/14] bpf: introduce the bpf_get_local_storage() helper function
From: Daniel Borkmann @ 2018-08-01 20:15 UTC (permalink / raw)
To: Roman Gushchin; +Cc: netdev, linux-kernel, kernel-team, Alexei Starovoitov
In-Reply-To: <20180801002831.GA25953@castle.DHCP.thefacebook.com>
On 08/01/2018 02:28 AM, Roman Gushchin wrote:
> On Wed, Aug 01, 2018 at 12:50:16AM +0200, Daniel Borkmann wrote:
>> On 07/27/2018 11:52 PM, Roman Gushchin wrote:
>> [...]
>>> @@ -2533,6 +2541,16 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
>>> }
>>>
>>> regs = cur_regs(env);
>>> +
>>> + /* check that flags argument in get_local_storage(map, flags) is 0,
>>> + * this is required because get_local_storage() can't return an error.
>>> + */
>>> + if (func_id == BPF_FUNC_get_local_storage &&
>>> + !tnum_equals_const(regs[BPF_REG_2].var_off, 0)) {
>>> + verbose(env, "get_local_storage() doesn't support non-zero flags\n");
>>> + return -EINVAL;
>>> + }
>>
>> Hmm, this check is actually not correct. You will still be able to pass non-zero
>> values in there. arg2_type from the helper is ARG_ANYTHING, so the register type
>> could for example be one of the pointer types and it will still pass the verifier.
>> The correct way to check would be to use register_is_null().
>>
>>> +
>>> /* reset caller saved regs */
>>> for (i = 0; i < CALLER_SAVED_REGS; i++) {
>>> mark_reg_not_init(env, regs, caller_saved[i]);
>
> Oh, perfect catch!
> The diff is below. Please, let me know if you prefer me to resend
> the whole patch/patchset.
Yeah, please resend at that point. There are also some other minor things which
would be great if you could roll them in as well in a respin along with this fix
and the uapi helper description adjustment with test case fix:
- patch 1: bpf_map_release_memlock() should also use bpf_uncharge_memlock() directly
- patch 2: cgroup_storage_map_alloc() only checks attr->key_size and attr->value_size
but what about attr->max_entries and attr->map_flags? Should attr->max_entries be
forced to 0 and at least attr->map_flags that don't make any sense in this context
get rejected on map creation?
- patch 9: not all uapi changes were copied over into tools' uapi header
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH v3 7/8] net: phy: Add support to configure clock in Broadcom iProc mdio mux
From: Florian Fainelli @ 2018-08-01 18:40 UTC (permalink / raw)
To: Arun Parameswaran, David S. Miller, Florian Fainelli, Andrew Lunn,
Rob Herring, Mark Rutland, Ray Jui, Scott Branden,
Catalin Marinas, Will Deacon
Cc: netdev, devicetree, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list
In-Reply-To: <1533146186-8374-8-git-send-email-arun.parameswaran@broadcom.com>
On 08/01/2018 10:56 AM, Arun Parameswaran wrote:
> Add support to configure the internal rate adjust register based on the
> core clock supplied through device tree in the Broadcom iProc mdio mux.
>
> The operating frequency of the mdio mux block is 11MHz. This is derrived
> by dividing the clock to the mdio mux with the rate adjust register.
>
> In some SoC's the default values of the rate adjust register do not yield
> 11MHz. These SoC's are required to specify the clock via the device tree
> for proper operation.
>
> Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
> ---
[snip]
> static int iproc_mdio_wait_for_idle(void __iomem *base, bool result)
> @@ -204,6 +225,20 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
> return -ENOMEM;
> }
>
> + md->core_clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(md->core_clk)) {
> + if (PTR_ERR(md->core_clk) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + md->core_clk = NULL;
I would simplify this a bit:
if (IS_ERR(md->core_clk) && PTR_ERR(md->core_clk) == -EPROBE_DEFER)
return PTR_ERR(md->core_clk);
else
md->core_clk = NULL;
rc = clk_prepare_enable(md->core_clk);
and continue that way.
> + } else {
> + rc = clk_prepare_enable(md->core_clk);
> + if (rc) {
> + dev_err(&pdev->dev, "failed to enable core clk\n");
> + return rc;
> + }
> + }
> +
> bus = md->mii_bus;
> bus->priv = md;
> bus->name = "iProc MDIO mux bus";
> @@ -217,7 +252,7 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
> rc = mdiobus_register(bus);
> if (rc) {
> dev_err(&pdev->dev, "mdiomux registration failed\n");
> - return rc;
> + goto out_clk;
> }
>
> platform_set_drvdata(pdev, md);
> @@ -237,6 +272,8 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
>
> out_register:
> mdiobus_unregister(bus);
> +out_clk:
> + clk_disable_unprepare(md->core_clk);
> return rc;
> }
>
> @@ -246,6 +283,7 @@ static int mdio_mux_iproc_remove(struct platform_device *pdev)
>
> mdio_mux_uninit(md->mux_handle);
> mdiobus_unregister(md->mii_bus);
> + clk_disable_unprepare(md->core_clk);
> platform_set_drvdata(pdev, NULL);
>
> return 0;
>
--
Florian
^ permalink raw reply
* Re: [PATCH v3 7/8] net: phy: Add support to configure clock in Broadcom iProc mdio mux
From: Andrew Lunn @ 2018-08-01 20:38 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Arun Parameswaran, Florian Fainelli, David S. Miller, Rob Herring,
Mark Rutland, Ray Jui, Scott Branden, Catalin Marinas,
Will Deacon, netdev, bcm-kernel-feedback-list, linux-kernel,
linux-arm-kernel, devicetree
In-Reply-To: <20180801202313.GH30658@n2100.armlinux.org.uk>
On Wed, Aug 01, 2018 at 09:23:14PM +0100, Russell King - ARM Linux wrote:
> On Wed, Aug 01, 2018 at 10:07:12PM +0200, Andrew Lunn wrote:
> > You might want to consider adding clk_optional_get() and
> > devm_clk_optional_get().
>
> I think there's attempts to add such APIs but I don't think it's
> trivial - it seems to require a _lot_ of discussion.
>
> I think part of that is because of the quirky use of error codes.
> If you look at clk_get(), it calls __of_clk_get_by_name() which
> returns:
>
> -ENOENT if DT is disabled
> -ENOENT if the device has no DT node
> -EPROBE_DEFER if the lookup in DT succeeds but there's no registered
> clock
> -EINVAL if the device has a DT node but the lookup of the name
> failed (in otherwords, the optional clock was omitted)
> -ENOENT if the clocks = property has not enough clocks for the
> clock-names property
> -ENOMEM if we fail to allocate the clk
> -ENOENT if __clk_get() fails
That makes it hard. I added phy_optional_get() early on, when the
error cases were simple. Hopefully they remain simple...
Andrew
^ 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