* [PATCH net 1/4] udpv6: add the required annotation to mib type
From: Paolo Abeni @ 2019-02-21 16:43 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Stefano Brivio
In-Reply-To: <cover.1550767274.git.pabeni@redhat.com>
In commit 029a37434880 ("udp6: cleanup stats accounting in recvmsg()")
I forgot to add the percpu annotation for the mib pointer. Add it, and
make sparse happy.
Fixes: 029a37434880 ("udp6: cleanup stats accounting in recvmsg()")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/ipv6/udp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 2596ffdeebea..e6c52c27f354 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -288,8 +288,8 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
int peeked, peeking, off;
int err;
int is_udplite = IS_UDPLITE(sk);
+ struct udp_mib __percpu *mib;
bool checksum_valid = false;
- struct udp_mib *mib;
int is_udp4;
if (flags & MSG_ERRQUEUE)
--
2.20.1
^ permalink raw reply related
* [PATCH net 2/4] fou6: fix proto error handler argument type
From: Paolo Abeni @ 2019-02-21 16:43 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Stefano Brivio
In-Reply-To: <cover.1550767274.git.pabeni@redhat.com>
Last argument of gue6_err_proto_handler() has a wrong type annotation,
fix it and make sparse happy again.
Fixes: b8a51b38e4d4 ("fou, fou6: ICMP error handlers for FoU and GUE")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Acked-by: Stefano Brivio <sbrivio@redhat.com>
---
net/ipv6/fou6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/fou6.c b/net/ipv6/fou6.c
index b858bd5280bf..867474abe269 100644
--- a/net/ipv6/fou6.c
+++ b/net/ipv6/fou6.c
@@ -72,7 +72,7 @@ static int gue6_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
static int gue6_err_proto_handler(int proto, struct sk_buff *skb,
struct inet6_skb_parm *opt,
- u8 type, u8 code, int offset, u32 info)
+ u8 type, u8 code, int offset, __be32 info)
{
const struct inet6_protocol *ipprot;
--
2.20.1
^ permalink raw reply related
* [PATCH net 3/4] udpv6: fix possible user after free in error handler
From: Paolo Abeni @ 2019-02-21 16:43 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Stefano Brivio
In-Reply-To: <cover.1550767274.git.pabeni@redhat.com>
Before derefencing the encap pointer, commit e7cc082455cb ("udp: Support
for error handlers of tunnels with arbitrary destination port") checks
for a NULL value, but the two fetch operation can race with removal.
Fix the above using a single access.
Also fix a couple of type annotations, to make sparse happy.
Fixes: e7cc082455cb ("udp: Support for error handlers of tunnels with arbitrary destination port")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Acked-by: Stefano Brivio <sbrivio@redhat.com>
---
net/ipv6/udp.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index e6c52c27f354..b444483cdb2b 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -420,17 +420,19 @@ EXPORT_SYMBOL(udpv6_encap_enable);
*/
static int __udp6_lib_err_encap_no_sk(struct sk_buff *skb,
struct inet6_skb_parm *opt,
- u8 type, u8 code, int offset, u32 info)
+ u8 type, u8 code, int offset, __be32 info)
{
int i;
for (i = 0; i < MAX_IPTUN_ENCAP_OPS; i++) {
int (*handler)(struct sk_buff *skb, struct inet6_skb_parm *opt,
- u8 type, u8 code, int offset, u32 info);
+ u8 type, u8 code, int offset, __be32 info);
+ const struct ip6_tnl_encap_ops *encap;
- if (!ip6tun_encaps[i])
+ encap = rcu_dereference(ip6tun_encaps[i]);
+ if (!encap)
continue;
- handler = rcu_dereference(ip6tun_encaps[i]->err_handler);
+ handler = encap->err_handler;
if (handler && !handler(skb, opt, type, code, offset, info))
return 0;
}
--
2.20.1
^ permalink raw reply related
* [PATCH net 4/4] udp: fix possible user after free in error handler
From: Paolo Abeni @ 2019-02-21 16:44 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Stefano Brivio
In-Reply-To: <cover.1550767274.git.pabeni@redhat.com>
Similar to the previous commit, this addresses the same issue for
ipv4: use a single fetch operation and use the correct rcu
annotation.
Fixes: e7cc082455cb ("udp: Support for error handlers of tunnels with arbitrary destination port")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Acked-by: Stefano Brivio <sbrivio@redhat.com>
---
net/ipv4/udp.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 5c3cd5d84a6f..372fdc5381a9 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -562,10 +562,12 @@ static int __udp4_lib_err_encap_no_sk(struct sk_buff *skb, u32 info)
for (i = 0; i < MAX_IPTUN_ENCAP_OPS; i++) {
int (*handler)(struct sk_buff *skb, u32 info);
+ const struct ip_tunnel_encap_ops *encap;
- if (!iptun_encaps[i])
+ encap = rcu_dereference(iptun_encaps[i]);
+ if (!encap)
continue;
- handler = rcu_dereference(iptun_encaps[i]->err_handler);
+ handler = encap->err_handler;
if (handler && !handler(skb, info))
return 0;
}
--
2.20.1
^ permalink raw reply related
* [PATCH net] net: phy: marvell10g: Fix Multi-G advertisement to only advertise 10G
From: Maxime Chevallier @ 2019-02-21 16:54 UTC (permalink / raw)
To: davem
Cc: Maxime Chevallier, netdev, linux-kernel, Andrew Lunn,
Florian Fainelli, Heiner Kallweit, Russell King, linux-arm-kernel,
Antoine Tenart, thomas.petazzoni
Some Marvell Alaska PHYs support 2.5G, 5G and 10G BaseT links. Their
default behaviour is to advertise all of these modes, but at the moment,
only 10GBaseT is supported. To prevent link partners from establishing
link at that speed, clear these modes upon configuring aneg parameters.
Fixes: 20b2af32ff3f ("net: phy: add Marvell Alaska X 88X3310 10Gigabit PHY support")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reported-by: Russell King <linux@armlinux.org.uk>
---
Dave,
This patch will conflict when merging net into net-next, and is actually
not needed there. In net-next, this issue is fixed by the recent work
done by Andrew and Heiner, that introduce more generic ways to handle aneg
configuration for C45 PHYs.
The commit fixing this would be :
3de97f3c6308 ("net: phy: marvell10g: use genphy_c45_an_config_aneg")
However this fix should be backported, hence why it was implemented in a
small standalone patch here.
Please tell me if you have any issue with this.
Thanks,
Maxime
drivers/net/phy/marvell10g.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 82ab6ed3b74e..6bac602094bd 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -26,6 +26,8 @@
#include <linux/marvell_phy.h>
#include <linux/phy.h>
+#define MDIO_AN_10GBT_CTRL_ADV_NBT_MASK 0x01e0
+
enum {
MV_PCS_BASE_T = 0x0000,
MV_PCS_BASE_R = 0x1000,
@@ -386,8 +388,10 @@ static int mv3310_config_aneg(struct phy_device *phydev)
else
reg = 0;
+ /* Make sure we clear unsupported 2.5G/5G advertising */
ret = mv3310_modify(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_CTRL,
- MDIO_AN_10GBT_CTRL_ADV10G, reg);
+ MDIO_AN_10GBT_CTRL_ADV10G |
+ MDIO_AN_10GBT_CTRL_ADV_NBT_MASK, reg);
if (ret < 0)
return ret;
if (ret > 0)
--
2.19.2
^ permalink raw reply related
* Re: [PATCH net-next 5/5] net/mlx5e: Support enable/disable VFs link state on switchdev mode
From: Or Gerlitz @ 2019-02-21 17:03 UTC (permalink / raw)
To: Tonghao Zhang; +Cc: Saeed Mahameed, Linux Netdev List
In-Reply-To: <1550715283-23579-5-git-send-email-xiangxia.m.yue@gmail.com>
On Thu, Feb 21, 2019 at 3:42 PM <xiangxia.m.yue@gmail.com> wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> This patch allow users to enable/disable VFs link state
> on switchdev mode.
NAK
We do it with the reps, if you change the administrative link state of a VF rep
it will effect the operational link state of the VF, see upstream commit
20a1ea674783 net/mlx5e: Support VF vport link state control for SRIOV
switchdev mode
^ permalink raw reply
* Re: [PATCH net-next v4 07/17] net: sched: protect filter_chain list with filter_chain_lock mutex
From: Vlad Buslov @ 2019-02-21 17:11 UTC (permalink / raw)
To: Cong Wang
Cc: Ido Schimmel, netdev@vger.kernel.org, jhs@mojatatu.com,
jiri@resnulli.us, davem@davemloft.net, ast@kernel.org,
daniel@iogearbox.net
In-Reply-To: <CAM_iQpXm9-mpANJ77pykotHO+OEP1EnsY1SwZygbnNczdp0vPQ@mail.gmail.com>
On Wed 20 Feb 2019 at 23:00, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, Feb 19, 2019 at 7:20 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>>
>>
>> On Tue 19 Feb 2019 at 05:08, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> > On Fri, Feb 15, 2019 at 2:02 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>> >>
>> >> I looked at the code and problem seems to be matchall classifier
>> >> specific. My implementation of unlocked cls API assumes that concurrent
>> >> insertions are possible and checks for it when deleting "empty" tp.
>> >> Since classifiers don't expose number of elements, the only way to test
>> >> this is to do tp->walk() on them and assume that walk callback is called
>> >> once per filter on every classifier. In your example new tp is created
>> >> for second filter, filter insertion fails, number of elements on newly
>> >> created tp is checked with tp->walk() before deleting it. However,
>> >> matchall classifier always calls the tp->walk() callback once, even when
>> >> it doesn't have a valid filter (in this case with NULL filter pointer).
>> >
>> > Again, this can be eliminated by just switching to normal
>> > non-retry logic. This is yet another headache to review this
>> > kind of unlock-and-retry logic, I have no idea why you are such
>> > a big fan of it.
>>
>> The retry approach was suggested to me multiple times by Jiri on
>> previous code reviews so I assumed it is preferred approach in such
>> cases. I don't have a strong preference in this regard, but locking
>> whole tp on filter update will remove any parallelism when updating same
>> classifier instance concurrently. The goal of these changes is to allow
>> parallel rule update and to achieve that I had to introduce some
>> complexity into the code.
>
> Yeah, but with unlock-and-retry it would waste more time when
> retry occurs. So it can't be better in the worst scenario.
>
> The question is essentially that do we want to waste CPU cycles
> when conflicts occurs or just block there until it is safe to enter
> the critical section?
>
> And, is the retry bound? Is it possible that we would retry infinitely
> as long as we time it correctly?
>
>
>>
>> Now let me explain why these two approaches result completely different
>> performance in this case. Lets start with a list of most CPU-consuming
>> parts in new filter creation process in descending order (raw data at
>> the end of this mail):
>>
>> 1) Hardware offload - if available and no skip_hw.
>> 2) Exts (actions) initalization - most expensive part even with single
>> action, CPU usage increases with number of actions per filter.
>> 3) cls API.
>> 4) Flower classifier data structure initialization.
>>
>> Note that 1)+2) is ~80% of cost of creating a flower filter. So if we
>> just lock the whole flower classifier instance during rule update we
>> serialize 1, 2 and 4, and only cls API (~13% of CPU cost) can be
>> executed concurrently. However, in proposed flower implementation hw
>> offloading and action initialization code is called without any locks
>> and tp->lock is only obtained when modifying flower data structures,
>> which means that only 3) is serialized and everything else (87% of CPU
>> cost) can be executed in parallel.
>
> What about when conflicts detected and retry the whole change?
> And, of course, how often do conflicts happen?
>
> Thanks.
I had similar concerns when designing this change. Lets look at two
cases when this retry is needed.
One process creates first filter on classifier and fails, while other
processes are trying to concurrently add filter to same block/chain/tp:
1) Process obtains filter_chain_lock, performs unsuccessful tp lookup,
releases the lock.
2) Calls tcf_chain_tp_insert_unique() which obtains filter_chain_lock,
inserts new tp, releases the lock.
3) Calls tp->ops->change() that returns an error.
4) Calls tcf_chain_tp_delete_empty() which takes filter_chain_lock, verifies that no
filters were added to tp concurrently, sets tp->deleting flag, removes
tp from chain.
This is supposed to be very rare occurrence because for retry to happen
it not only requires concurrent insertions to same block/chain/tp, but
also that tp with requested prio didn't exist before and no concurrent
process succeeded in adding at least one filter to tp during step 3
before it is marked for deletion in step 4 (otherwise
tcf_proto_check_delete() fails and concurrent threads don't perform
retry).
Another case is when last filter is being deleted while concurrent
processes adding new filters to same block/chain/tp:
1) tc_del_tfilter() gets last filter with tp->ops->get()
2) Deletes it with tp->ops->delete()...
3) ... that return 'last' hint set to true.
4) Calls tcf_chain_tp_delete_empty() which takes filter_chain_lock, verifies that no
filters were added to tp concurrently, sets tp->deleting flag, removes
tp from chain.
This case is also quite rare because it requires concurrent users to
successfully lookup tp before tp->deleting is set to true and tp is
removed from chain, but not create any new filters on tp during that
time.
After considering this I decided that it is not worth it to penalize
common case of updating filters by completely removing parallelism when
updates target same tp instance for such rare corner cases as described
above.
Now regarding forcing users to retry indefinitely. In later cases no
more than one retry is possible because concurrent add processes create
new tp on first retry. In former case multiple retries are possible, but
to block concurrent users indefinitely would require malicious process
to somehow always have priority when obtaining filter_chain_lock during
steps 1, 2, then wait to allow all concurrent users to lookup the tp,
then obtain filter_chain_lock in step 4 and initiate tp deletion before
any of concurrent users that have a reference to this new tp instance
can insert any single filter on it, then go back to step 1, obtain lock
first and repeat. I don't see how this can be timed from userspace
repeatedly as creating first filter on new tp involves multiple cycles
of getting and releasing filter_chain_lock and each of them require
attacker to "influence" kernel scheduler to behave in very specific
fashion.
Regards,
Vlad
^ permalink raw reply
* Re: [PATCH RFC 3/5] sched/cpufreq: Fix incorrect RCU API usage
From: Joel Fernandes @ 2019-02-21 17:13 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Paul E. McKenney, linux-kernel, Alexei Starovoitov,
Christian Brauner, Daniel Borkmann, David Ahern, David S. Miller,
Ido Schimmel, Ingo Molnar, moderated list:INTEL ETHERNET DRIVERS,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, Lai Jiangshan,
Martin KaFai Lau, Mathieu Desnoyers, netdev, rcu, Song Liu,
Steven Rostedt, xdp-newbies, Yonghong Song
In-Reply-To: <20190221161144.GU32494@hirez.programming.kicks-ass.net>
On Thu, Feb 21, 2019 at 05:11:44PM +0100, Peter Zijlstra wrote:
> On Thu, Feb 21, 2019 at 07:52:18AM -0800, Paul E. McKenney wrote:
> > On Thu, Feb 21, 2019 at 04:31:17PM +0100, Peter Zijlstra wrote:
> > > On Thu, Feb 21, 2019 at 10:21:39AM -0500, Joel Fernandes wrote:
> > > > On Thu, Feb 21, 2019 at 10:18:05AM +0100, Peter Zijlstra wrote:
> > > > > On Thu, Feb 21, 2019 at 12:49:40AM -0500, Joel Fernandes (Google) wrote:
> > > > > > @@ -34,8 +34,12 @@ void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
> > > > > > if (WARN_ON(!data || !func))
> > > > > > return;
> > > > > >
> > > > > > - if (WARN_ON(per_cpu(cpufreq_update_util_data, cpu)))
> > > > > > + rcu_read_lock();
> > > > > > + if (WARN_ON(rcu_dereference(per_cpu(cpufreq_update_util_data, cpu)))) {
> > > > > > + rcu_read_unlock();
> > > > > > return;
> > > > > > + }
> > > > > > + rcu_read_unlock();
> > > > > >
> > > > > > data->func = func;
> > > > > > rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
>
> > For whatever it is worth, in that case it could use rcu_access_pointer().
> > And this primitive does not do the lockdep check for being within an RCU
> > read-side critical section. As Peter says, if there is no dereferencing,
> > there can be no use-after-free bug, so the RCU read-side critical is
> > not needed.
>
> On top of that, I suspect this is under the write-side lock (we're doing
> assignment after all).
Yes it is under a policy->rwsem, just confirmed that.
And indeed rcu_read_lock() is not needed here in this patch, thanks for
pointing that out. Sometimes the word "dereference" plays some visual tricks
and in this case tempted me to add an RCU reader section ;-) Assuming you
guys are in agreement with usage of rcu_access_pointer() here to keep sparse
happy, I'll rework the patch accordingly and resubmit with that.
thanks!
- Joel
^ permalink raw reply
* Re: [PATCH RFC 4/5] sched/topology: Annonate RCU pointers properly
From: Joel Fernandes @ 2019-02-21 17:17 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ido Schimmel,
Ingo Molnar, moderated list:INTEL ETHERNET DRIVERS,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, Lai Jiangshan,
Martin KaFai Lau, Mathieu Desnoyers, netdev, Paul E. McKenney,
rcu, Song Liu, Steven Rostedt, xdp-newbies, Yonghong Song
In-Reply-To: <20190221152944.GS32494@hirez.programming.kicks-ass.net>
On Thu, Feb 21, 2019 at 04:29:44PM +0100, Peter Zijlstra wrote:
> On Thu, Feb 21, 2019 at 10:10:57AM -0500, Joel Fernandes wrote:
> > Hi Peter,
> >
> > Thanks for taking a look.
> >
> > On Thu, Feb 21, 2019 at 10:19:44AM +0100, Peter Zijlstra wrote:
> > > On Thu, Feb 21, 2019 at 12:49:41AM -0500, Joel Fernandes (Google) wrote:
> > >
> > > > Also replace rcu_assign_pointer call on rq->sd with WRITE_ONCE. This
> > > > should be sufficient for the rq->sd initialization.
> > >
> > > > @@ -668,7 +668,7 @@ cpu_attach_domain(struct sched_domain *sd, struct root_domain *rd, int cpu)
> > > >
> > > > rq_attach_root(rq, rd);
> > > > tmp = rq->sd;
> > > > - rcu_assign_pointer(rq->sd, sd);
> > > > + WRITE_ONCE(rq->sd, sd);
> > > > dirty_sched_domain_sysctl(cpu);
> > > > destroy_sched_domains(tmp);
> > >
> > > Where did the RELEASE barrier go?
> > >
> > > That was a publish operation, now it is not.
> >
> > Funny thing is, initially I had written this patch with smp_store_release()
> > instead of WRITE_ONCE, but checkpatch complaints with that since it needs a
> > comment on top of it, and I wasn't sure if RELEASE barrier was the intent of
> > using rcu_assign_pointer (all the more reason to replace it with something
> > more explicit).
> >
> > I will replace it with the following and resubmit it then:
> >
> > /* Release barrier */
> > smp_store_release(&rq->sd, sd);
> >
> > Or do we want to just drop the "Release barrier" comment and live with the
> > checkpatch warning?
>
> How about we keep using rcu_assign_pointer(), the whole sched domain
> tree is under rcu; peruse that destroy_sched_domains() function for
> instance.
>
> Also check how for_each_domain() uses rcu_dereference().
May be then, all those pointers should be made __rcu as well. Then we can use
rcu_assign_pointer() here. I will look more into it and study these functions
as you are suggesting.
thanks,
- Joel
^ permalink raw reply
* Re: stmmac / meson8b-dwmac
From: Simon Huelck @ 2019-02-21 17:27 UTC (permalink / raw)
To: Jerome Brunet, Jose Abreu, Martin Blumenstingl
Cc: linux-amlogic, netdev, alexandre.torgue, Emiliano Ingrassia,
Gpeppe.cavallaro
In-Reply-To: <1426d8ed40be0927c135aff25dcf989a11326932.camel@baylibre.com>
Am 21.02.2019 um 15:21 schrieb Jerome Brunet:
> On Tue, 2019-02-19 at 20:41 +0100, Simon Huelck wrote:
>> Am 19.02.2019 um 09:47 schrieb Jose Abreu:
>>> Hi Simon,
>>>
>>> On 2/18/2019 6:05 PM, Simon Huelck wrote:
>>>> disabling EEE doesnt help ( did it via the entry in the .dtb / .dts ),
>>>> the results are the same. I can confirm the LPI counters are zero or one
>>>> only after the test.....
>>> It's interesting to see that you have a lot of RX packets but few
>>> RX interrupts. This can either be due to mis-configuration of
>>> interrupt flags or RX Watchdog.
>>>
>>> 1. For interrupt flags you should be using LEVEL triggered IRQ.
>>> 2. For RX Watchdog you can try disabling it by adding the
>>> following in your platform wrapper driver (which will be
>>> "dwmac-meson8b.c", at probe):
>>> "plat_data->force_sf_dma_mode = 1;" and
>>> "plat_data->riwt_off = 1;"
>>>
>>> Thanks,
>>> Jose Miguel Abreu
>> Hi,
>>
>>
>> are you aware that odroid c2 dts is using level irq ?
> The odroid-c2 (meson-gxl) is using level triggered irq, indeed.
> For a reason that still eludes me, The odroid-c1 (meson8b) isn't.
>
> This is confusing since both boards have been mentionned in this thread.
>
>> at which numbers do you estimage that RX irqs are low in numbers ?
>>
>>
>> regards,
>>
>> Simon
>>
>>
>> _______________________________________________
>> linux-amlogic mailing list
>> linux-amlogic@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
>
Hi,
this was changed recently, with a patch for the EEE stuff , see here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.0-rc7&id=e35e26b26e955c53e61c154ba26b9bb15da6b858
before that the IRQ stuff was differently. I tried to revert this patch
via git locally, but had no success ( i guess git tried to revert this
patch on the server :-P)
regards,
Simon
^ permalink raw reply
* Re: [PATCH net-next v2 1/7] net: Don't set transport offset to invalid value
From: Willem de Bruijn @ 2019-02-21 17:28 UTC (permalink / raw)
To: Maxim Mikityanskiy
Cc: David S. Miller, Saeed Mahameed, Willem de Bruijn, Jason Wang,
Eric Dumazet, netdev@vger.kernel.org, Eran Ben Elisha,
Tariq Toukan
In-Reply-To: <20190221123908.7196-2-maximmi@mellanox.com>
On Thu, Feb 21, 2019 at 7:40 AM Maxim Mikityanskiy <maximmi@mellanox.com> wrote:
>
> If the socket was created with socket(AF_PACKET, SOCK_RAW, 0),
> skb->protocol will be unset, __skb_flow_dissect() will fail, and
> skb_probe_transport_header() will fall back to the offset_hint, making
> the resulting skb_transport_offset incorrect.
>
> If, however, there is no transport header in the packet,
> transport_header shouldn't be set to an arbitrary value.
>
> Fix it by leaving the transport offset unset if it couldn't be found, to
> be explicit rather than to fill it with some wrong value. It changes the
> behavior, but if some code relied on the old behavior, it would be
> broken anyway, as the old one is incorrect.
>
> Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
qdisc_pkt_len_init also expects skb_transport_header(skb) to always be
set for gso packets.
Once net is merged into net-next, commit d5be7f632bad ("net: validate
untrusted gso packets without csum offload") will ensure that packets
that fail flow dissection do not make it into the stack. But we have
to skip dissection in some cases, like tun [1].
I think we need to add a check in qdisc_pkt_len_init to skip the gso
size estimation branch if !skb_transport_header_was_set(skb).
Otherwise this patch set looks good to me. To avoid resubmitting
everything we can fix up the qdisc_pkt_len_init in a follow-up, in
which case I'm happy to add my Acked-by to this series.
[1] http://patchwork.ozlabs.org/patch/1044429/
^ permalink raw reply
* Re: [PATCH RFC 3/5] sched/cpufreq: Fix incorrect RCU API usage
From: Paul E. McKenney @ 2019-02-21 17:29 UTC (permalink / raw)
To: Joel Fernandes
Cc: Peter Zijlstra, linux-kernel, Alexei Starovoitov,
Christian Brauner, Daniel Borkmann, David Ahern, David S. Miller,
Ido Schimmel, Ingo Molnar, moderated list:INTEL ETHERNET DRIVERS,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, Lai Jiangshan,
Martin KaFai Lau, Mathieu Desnoyers, netdev, rcu, Song Liu,
Steven Rostedt, xdp-newbies, Yonghong Song
In-Reply-To: <20190221171311.GA118415@google.com>
On Thu, Feb 21, 2019 at 12:13:11PM -0500, Joel Fernandes wrote:
> On Thu, Feb 21, 2019 at 05:11:44PM +0100, Peter Zijlstra wrote:
> > On Thu, Feb 21, 2019 at 07:52:18AM -0800, Paul E. McKenney wrote:
> > > On Thu, Feb 21, 2019 at 04:31:17PM +0100, Peter Zijlstra wrote:
> > > > On Thu, Feb 21, 2019 at 10:21:39AM -0500, Joel Fernandes wrote:
> > > > > On Thu, Feb 21, 2019 at 10:18:05AM +0100, Peter Zijlstra wrote:
> > > > > > On Thu, Feb 21, 2019 at 12:49:40AM -0500, Joel Fernandes (Google) wrote:
> > > > > > > @@ -34,8 +34,12 @@ void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
> > > > > > > if (WARN_ON(!data || !func))
> > > > > > > return;
> > > > > > >
> > > > > > > - if (WARN_ON(per_cpu(cpufreq_update_util_data, cpu)))
> > > > > > > + rcu_read_lock();
> > > > > > > + if (WARN_ON(rcu_dereference(per_cpu(cpufreq_update_util_data, cpu)))) {
> > > > > > > + rcu_read_unlock();
> > > > > > > return;
> > > > > > > + }
> > > > > > > + rcu_read_unlock();
> > > > > > >
> > > > > > > data->func = func;
> > > > > > > rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
> >
> > > For whatever it is worth, in that case it could use rcu_access_pointer().
> > > And this primitive does not do the lockdep check for being within an RCU
> > > read-side critical section. As Peter says, if there is no dereferencing,
> > > there can be no use-after-free bug, so the RCU read-side critical is
> > > not needed.
> >
> > On top of that, I suspect this is under the write-side lock (we're doing
> > assignment after all).
>
> Yes it is under a policy->rwsem, just confirmed that.
>
> And indeed rcu_read_lock() is not needed here in this patch, thanks for
> pointing that out. Sometimes the word "dereference" plays some visual tricks
> and in this case tempted me to add an RCU reader section ;-) Assuming you
> guys are in agreement with usage of rcu_access_pointer() here to keep sparse
> happy, I'll rework the patch accordingly and resubmit with that.
Works for me!
Thanx, Paul
^ permalink raw reply
* Re: [RFC PATCH net-next v3 15/21] ethtool: provide link settings and link modes in GET_SETTINGS request
From: Florian Fainelli @ 2019-02-21 17:40 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, David Miller, Andrew Lunn, Jakub Kicinski, Jiri Pirko,
linux-kernel
In-Reply-To: <20190221101425.GO23151@unicorn.suse.cz>
On 2/21/19 2:14 AM, Michal Kubecek wrote:
> On Wed, Feb 20, 2019 at 07:14:50PM -0800, Florian Fainelli wrote:
>> On 2/18/2019 10:22 AM, Michal Kubecek wrote:
>>> +#define ETH_SETTINGS_IM_LINKINFO 0x01
>>> +#define ETH_SETTINGS_IM_LINKMODES 0x02
>>> +
>>> +#define ETH_SETTINGS_IM_ALL 0x03
>>
>> You could define ETH_SETTINGS_IM_ALL as:
>>
>> #define ETH_SETTING_IM_ALL \
>> (ETH_SETTINGS_IM_LINKINFO |
>> ETH_SETTINGS_IM_LINMODES)
>>
>> that would scale better IMHO, especially given that you have to keep
>> bumping that mask with new bits in subsequent patches.
>
> I'm considering going even further and using something similar to what
> is used for NETIF_F_* constants so that the *_ALL value would be
> calculated automatically. But I'm not sure if it's not too fancy for
> a uapi header file.
Adding new netdev features still requires defining two constants: one in
the enumeration, and one that resolves the bit to bitmask constant, so
this would amount to the same possible mistakes/errors being made here
by changing two lines.
>
>>> + if (tb[ETHA_SETTINGS_INFOMASK])
>>> + req_info->req_mask = nla_get_u32(tb[ETHA_SETTINGS_INFOMASK]);
>>> + if (tb[ETHA_SETTINGS_COMPACT])
>>> + req_info->compact = true;
>>> + if (req_info->req_mask == 0)
>>> + req_info->req_mask = ETH_SETTINGS_IM_ALL;
>>
>> What if userland is newer than the kernel and specifies a req_mask with
>> bits set that you don't support? Should not you always do an &
>> ETH_SETTINGS_IM_ALL here?
>
> In that case only known bits would be handled and the check at the end
> of prepare_info() would add a warning to extack that part of the
> information couldn't be provided (same as if some of the recognized
> parts didn't have necessary ethtool_ops handlers or if they failed).
I guess that is fair, I was just wondering if it made sense for the
kernel to report that there is one particular bitmask of settings that
it does not support at all and report that through netlink extended ack,
as opposed to silently not processing the bits it does not know.
--
Florian
^ permalink raw reply
* [PATCH] sysctl: add proc_do_large_bitmap test node
From: Eric Sandeen @ 2019-02-21 17:45 UTC (permalink / raw)
To: Eric Sandeen, Linux Kernel Mailing List, fsdevel, netdev
Cc: Luis Chamberlain, Kees Cook
In-Reply-To: <53be40fc-6ec4-c714-a64e-f69c96f7058f@redhat.com>
Add a test node for proc_do_large_bitmap to the test_sysctl.c
infrastructure. It's sized the same as the one existing user.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/lib/test_sysctl.c b/lib/test_sysctl.c
index 3dd801c1c85b..1263be4ebfaf 100644
--- a/lib/test_sysctl.c
+++ b/lib/test_sysctl.c
@@ -47,6 +47,9 @@ struct test_sysctl_data {
unsigned int uint_0001;
char string_0001[65];
+
+#define SYSCTL_TEST_BITMAP_SIZE 65536
+ unsigned long *bitmap_0001;
};
static struct test_sysctl_data test_data = {
@@ -102,6 +106,13 @@ static struct ctl_table test_table[] = {
.mode = 0644,
.proc_handler = proc_dostring,
},
+ {
+ .procname = "bitmap_0001",
+ .data = &test_data.bitmap_0001,
+ .maxlen = SYSCTL_TEST_BITMAP_SIZE,
+ .mode = 0644,
+ .proc_handler = proc_do_large_bitmap,
+ },
{ }
};
@@ -129,15 +140,21 @@ static struct ctl_table_header *test_sysctl_header;
static int __init test_sysctl_init(void)
{
+ test_data.bitmap_0001 = kzalloc(SYSCTL_TEST_BITMAP_SIZE/8, GFP_KERNEL);
+ if (!test_data.bitmap_0001)
+ return -ENOMEM;
test_sysctl_header = register_sysctl_table(test_sysctl_root_table);
- if (!test_sysctl_header)
+ if (!test_sysctl_header) {
+ kfree(test_data.bitmap_0001);
return -ENOMEM;
+ }
return 0;
}
late_initcall(test_sysctl_init);
static void __exit test_sysctl_exit(void)
{
+ kfree(test_data.bitmap_0001);
if (test_sysctl_header)
unregister_sysctl_table(test_sysctl_header);
}
^ permalink raw reply related
* Re: [PATCH net-next 01/12] net: sched: flower: don't check for rtnl on head dereference
From: Vlad Buslov @ 2019-02-21 17:45 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <CAM_iQpVSXVbLWcYDo8GHXn=aOPao=XufF5m=GhKAp9BhbJ=E9w@mail.gmail.com>
On Wed 20 Feb 2019 at 22:33, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, Feb 19, 2019 at 1:46 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>>
>>
>> On Mon 18 Feb 2019 at 19:08, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> > On Wed, Feb 13, 2019 at 11:47 PM Vlad Buslov <vladbu@mellanox.com> wrote:
>> >>
>> >> Flower classifier only changes root pointer during init and destroy. Cls
>> >> API implements reference counting for tcf_proto, so there is no danger of
>> >> concurrent access to tp when it is being destroyed, even without protection
>> >> provided by rtnl lock.
>> >
>> > How about atomicity? Refcnt doesn't guarantee atomicity, how do
>> > you make sure two concurrent modifications are atomic?
>>
>> In order to guarantee atomicity I lock shared flower classifier data
>> structures with tp->lock in following patches.
>
> Sure, I meant the atomicity of the _whole_ change, as you know
> the TC filters are stored in hierarchical structures: a block, a chain,
> a tp struct, some type-specific data structure like a hash table.
>
> Locking tp only solves a partial of the atomicity here. Are you
> going to restart the whole change from top down to the bottom?
You mean in cases where there is a conflict? Yes, processing EAGAIN in
tc_new_tfilter() involves releasing and re-obtaining all locks and
references.
>
>
>>
>> >
>> >
>> >>
>> >> Implement new function fl_head_dereference() to dereference tp->root
>> >> without checking for rtnl lock. Use it in all flower function that obtain
>> >> head pointer instead of rtnl_dereference().
>> >>
>> >
>> > So what lock protects RCU writers after this patch?
>>
>> I explained it in comment for fl_head_dereference(), but should have
>> copied this information to changelog as well:
>> Flower classifier only changes root pointer during init and destroy.
>> Cls API implements reference counting for tcf_proto, so there is no
>> danger of concurrent access to tp when it is being destroyed, even
>> without protection provided by rtnl lock.
>
> So you are saying an RCU pointer is okay to deference without
> any lock eve without RCU read lock, right?
>
>
>>
>> In initial version of this change I used tp->lock to protect tp->root
>> access and verified it with lockdep, but during internal review Jiri
>> noted that this is not needed in current flower implementation.
>
> Let's see what you have on top of your own branch
> unlocked_flower_cong_1:
>
> 1458 static int fl_change(struct net *net, struct sk_buff *in_skb,
> 1459 struct tcf_proto *tp, unsigned long base,
> 1460 u32 handle, struct nlattr **tca,
> 1461 void **arg, bool ovr, bool rtnl_held,
> 1462 struct netlink_ext_ack *extack)
> 1463 {
> 1464 struct cls_fl_head *head = fl_head_dereference(tp);
>
> At the point of line 1464, there is no lock taken, tp->lock is taken
> after it, block->lock or chain lock is already unlocked before ->change().
>
> So, what protects this RCU structure? According to RCU, it must be
> either RCU read lock or some writer lock. I see none here. :(
>
> What am I missing?
Initially I had flower implementation that protected tp->root access
with tp->lock, re-obtaining lock and head reference each time it was
used, sometimes multiple times per function (head reference is usually
obtained in beginning of every flower API function and used multiple
times afterwards). This complicated the code and reduced parallelism.
However, in case of flower classifier tp->root is never reassigned after
init, so essentially there is no "CU" part in this "RCU" pointer. This
realization allowed me to refactor unlocked flower code to look much
nicer and perform better. Am I missing something in flower classifier
implementation?
^ permalink raw reply
* [PATCH net-next 0/5] devlink: make ethtool compat reliable
From: Jakub Kicinski @ 2019-02-21 17:46 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
Hi!
This is a follow up to the series which added device flash
updates via devlink. I went with the approach of adding a
new NDO in the end. It seems to end up looking cleaner
(the port favour patches are coming regardless ;)).
First patch removes the option to build devlink as a module.
Users can still decide to not build it, but the module option
ends up not being worth the maintenance cost.
Second patch fixes a potential issue with ethtool code, which
will become even more problematic as the NDO introduced by
the third patch does not hold RTNL. The new NDO allows the core
to get the devlink instance based on a netdev pointer.
Next the NDO is implemented in the NFP, and ethtool flashing
ops removed.
Jakub Kicinski (5):
net: devlink: turn devlink into a built-in
ethtool: hold a reference to the netdevice around devlink compat
devlink: create a special NDO for getting the devlink instance
nfp: add .ndo_get_devlink
nfp: remove ethtool flashing fallback
drivers/infiniband/hw/bnxt_re/Kconfig | 1 -
drivers/infiniband/hw/mlx4/Kconfig | 1 -
drivers/net/Kconfig | 1 -
drivers/net/ethernet/broadcom/Kconfig | 1 -
drivers/net/ethernet/cavium/Kconfig | 1 -
drivers/net/ethernet/mellanox/mlx4/Kconfig | 1 -
.../net/ethernet/mellanox/mlx5/core/Kconfig | 1 -
drivers/net/ethernet/mellanox/mlxsw/Kconfig | 1 -
drivers/net/ethernet/netronome/Kconfig | 1 -
drivers/net/ethernet/netronome/nfp/nfp_app.h | 2 +
.../net/ethernet/netronome/nfp/nfp_devlink.c | 11 +++
.../ethernet/netronome/nfp/nfp_net_common.c | 1 +
.../ethernet/netronome/nfp/nfp_net_ethtool.c | 24 ------
.../net/ethernet/netronome/nfp/nfp_net_repr.c | 1 +
include/linux/netdevice.h | 6 ++
include/net/devlink.h | 10 +--
net/Kconfig | 11 +--
net/core/devlink.c | 77 ++++++-------------
net/core/ethtool.c | 11 ++-
net/dsa/Kconfig | 2 +-
20 files changed, 57 insertions(+), 108 deletions(-)
--
2.19.2
^ permalink raw reply
* [PATCH net-next 1/5] net: devlink: turn devlink into a built-in
From: Jakub Kicinski @ 2019-02-21 17:46 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190221174620.12144-1-jakub.kicinski@netronome.com>
Being able to build devlink as a module causes growing pains.
First all drivers had to add a meta dependency to make sure
they are not built in when devlink is built as a module. Now
we are struggling to invoke ethtool compat code reliably.
Make devlink code built-in, users can still not build it at
all but the dynamically loadable module option is removed.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/infiniband/hw/bnxt_re/Kconfig | 1 -
drivers/infiniband/hw/mlx4/Kconfig | 1 -
drivers/net/Kconfig | 1 -
drivers/net/ethernet/broadcom/Kconfig | 1 -
drivers/net/ethernet/cavium/Kconfig | 1 -
drivers/net/ethernet/mellanox/mlx4/Kconfig | 1 -
drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 1 -
drivers/net/ethernet/mellanox/mlxsw/Kconfig | 1 -
drivers/net/ethernet/netronome/Kconfig | 1 -
include/net/devlink.h | 10 ++++------
net/Kconfig | 11 +----------
net/core/devlink.c | 15 ++-------------
net/dsa/Kconfig | 2 +-
13 files changed, 8 insertions(+), 39 deletions(-)
diff --git a/drivers/infiniband/hw/bnxt_re/Kconfig b/drivers/infiniband/hw/bnxt_re/Kconfig
index 18f5ed082f41..19982a4a9bba 100644
--- a/drivers/infiniband/hw/bnxt_re/Kconfig
+++ b/drivers/infiniband/hw/bnxt_re/Kconfig
@@ -1,7 +1,6 @@
config INFINIBAND_BNXT_RE
tristate "Broadcom Netxtreme HCA support"
depends on ETHERNET && NETDEVICES && PCI && INET && DCB
- depends on MAY_USE_DEVLINK
select NET_VENDOR_BROADCOM
select BNXT
---help---
diff --git a/drivers/infiniband/hw/mlx4/Kconfig b/drivers/infiniband/hw/mlx4/Kconfig
index d1de3285fd88..4e9936731867 100644
--- a/drivers/infiniband/hw/mlx4/Kconfig
+++ b/drivers/infiniband/hw/mlx4/Kconfig
@@ -2,7 +2,6 @@ config MLX4_INFINIBAND
tristate "Mellanox ConnectX HCA support"
depends on NETDEVICES && ETHERNET && PCI && INET
depends on INFINIBAND_USER_ACCESS || !INFINIBAND_USER_ACCESS
- depends on MAY_USE_DEVLINK
select NET_VENDOR_MELLANOX
select MLX4_CORE
---help---
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 6210757772ed..5e4ca082cfcd 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -505,7 +505,6 @@ source "drivers/net/hyperv/Kconfig"
config NETDEVSIM
tristate "Simulated networking device"
depends on DEBUG_FS
- depends on MAY_USE_DEVLINK
help
This driver is a developer testing tool and software model that can
be used to test various control path networking APIs, especially
diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
index c1d3ee9baf7e..716bfbba59cf 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -194,7 +194,6 @@ config SYSTEMPORT
config BNXT
tristate "Broadcom NetXtreme-C/E support"
depends on PCI
- depends on MAY_USE_DEVLINK
select FW_LOADER
select LIBCRC32C
---help---
diff --git a/drivers/net/ethernet/cavium/Kconfig b/drivers/net/ethernet/cavium/Kconfig
index 05f4a3b21e29..6650e2a5f171 100644
--- a/drivers/net/ethernet/cavium/Kconfig
+++ b/drivers/net/ethernet/cavium/Kconfig
@@ -64,7 +64,6 @@ config CAVIUM_PTP
config LIQUIDIO
tristate "Cavium LiquidIO support"
depends on 64BIT && PCI
- depends on MAY_USE_DEVLINK
depends on PCI
imply PTP_1588_CLOCK
select FW_LOADER
diff --git a/drivers/net/ethernet/mellanox/mlx4/Kconfig b/drivers/net/ethernet/mellanox/mlx4/Kconfig
index f200b8c420d5..ff8057ed97ee 100644
--- a/drivers/net/ethernet/mellanox/mlx4/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx4/Kconfig
@@ -4,7 +4,6 @@
config MLX4_EN
tristate "Mellanox Technologies 1/10/40Gbit Ethernet support"
- depends on MAY_USE_DEVLINK
depends on PCI && NETDEVICES && ETHERNET && INET
select MLX4_CORE
imply PTP_1588_CLOCK
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
index 37a551436e4a..6debffb8336b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
@@ -4,7 +4,6 @@
config MLX5_CORE
tristate "Mellanox 5th generation network adapters (ConnectX series) core driver"
- depends on MAY_USE_DEVLINK
depends on PCI
imply PTP_1588_CLOCK
imply VXLAN
diff --git a/drivers/net/ethernet/mellanox/mlxsw/Kconfig b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
index b9a25aed5d11..9c195dfed031 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
@@ -4,7 +4,6 @@
config MLXSW_CORE
tristate "Mellanox Technologies Switch ASICs support"
- depends on MAY_USE_DEVLINK
---help---
This driver supports Mellanox Technologies Switch ASICs family.
diff --git a/drivers/net/ethernet/netronome/Kconfig b/drivers/net/ethernet/netronome/Kconfig
index 66f15b05b65e..549898d5d450 100644
--- a/drivers/net/ethernet/netronome/Kconfig
+++ b/drivers/net/ethernet/netronome/Kconfig
@@ -19,7 +19,6 @@ config NFP
tristate "Netronome(R) NFP4000/NFP6000 NIC driver"
depends on PCI && PCI_MSI
depends on VXLAN || VXLAN=n
- depends on MAY_USE_DEVLINK
---help---
This driver supports the Netronome(R) NFP4000/NFP6000 based
cards working as a advanced Ethernet NIC. It works with both
diff --git a/include/net/devlink.h b/include/net/devlink.h
index a2da49dd9147..f9f7fe974652 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -707,6 +707,10 @@ devlink_health_reporter_priv(struct devlink_health_reporter *reporter);
int devlink_health_report(struct devlink_health_reporter *reporter,
const char *msg, void *priv_ctx);
+void devlink_compat_running_version(struct net_device *dev,
+ char *buf, size_t len);
+int devlink_compat_flash_update(struct net_device *dev, const char *file_name);
+
#else
static inline struct devlink *devlink_alloc(const struct devlink_ops *ops,
@@ -1190,13 +1194,7 @@ devlink_health_report(struct devlink_health_reporter *reporter,
{
return 0;
}
-#endif
-#if IS_REACHABLE(CONFIG_NET_DEVLINK)
-void devlink_compat_running_version(struct net_device *dev,
- char *buf, size_t len);
-int devlink_compat_flash_update(struct net_device *dev, const char *file_name);
-#else
static inline void
devlink_compat_running_version(struct net_device *dev, char *buf, size_t len)
{
diff --git a/net/Kconfig b/net/Kconfig
index 62da6148e9f8..1efe1f9ee492 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -429,21 +429,12 @@ config NET_SOCK_MSG
with the help of BPF programs.
config NET_DEVLINK
- tristate "Network physical/parent device Netlink interface"
+ bool "Network physical/parent device Netlink interface"
help
Network physical/parent device Netlink interface provides
infrastructure to support access to physical chip-wide config and
monitoring.
-config MAY_USE_DEVLINK
- tristate
- default m if NET_DEVLINK=m
- default y if NET_DEVLINK=y || NET_DEVLINK=n
- help
- Drivers using the devlink infrastructure should have a dependency
- on MAY_USE_DEVLINK to ensure they do not cause link errors when
- devlink is a loadable module and the driver using it is built-in.
-
config PAGE_POOL
bool
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 04d98550c78c..fcf09e02f4c6 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6480,20 +6480,9 @@ int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
return -EOPNOTSUPP;
}
-static int __init devlink_module_init(void)
+static int __init devlink_init(void)
{
return genl_register_family(&devlink_nl_family);
}
-static void __exit devlink_module_exit(void)
-{
- genl_unregister_family(&devlink_nl_family);
-}
-
-module_init(devlink_module_init);
-module_exit(devlink_module_exit);
-
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Jiri Pirko <jiri@mellanox.com>");
-MODULE_DESCRIPTION("Network physical device Netlink interface");
-MODULE_ALIAS_GENL_FAMILY(DEVLINK_GENL_NAME);
+subsys_initcall(devlink_init);
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 91e52973ee13..fab49132345f 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -6,7 +6,7 @@ config HAVE_NET_DSA
config NET_DSA
tristate "Distributed Switch Architecture"
- depends on HAVE_NET_DSA && MAY_USE_DEVLINK
+ depends on HAVE_NET_DSA
depends on BRIDGE || BRIDGE=n
select NET_SWITCHDEV
select PHYLINK
--
2.19.2
^ permalink raw reply related
* [PATCH net-next 2/5] ethtool: hold a reference to the netdevice around devlink compat
From: Jakub Kicinski @ 2019-02-21 17:46 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190221174620.12144-1-jakub.kicinski@netronome.com>
When calling into devlink compat code make sure we have a reference
on the netdevice on which the operation was invoked.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
net/core/ethtool.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 1320e8dce559..6832476dfcaf 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -805,11 +805,14 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
if (ops->get_eeprom_len)
info.eedump_len = ops->get_eeprom_len(dev);
- rtnl_unlock();
- if (!info.fw_version[0])
+ if (!info.fw_version[0]) {
+ dev_hold(dev);
+ rtnl_unlock();
devlink_compat_running_version(dev, info.fw_version,
sizeof(info.fw_version));
- rtnl_lock();
+ rtnl_lock();
+ dev_put(dev);
+ }
if (copy_to_user(useraddr, &info, sizeof(info)))
return -EFAULT;
@@ -2043,9 +2046,11 @@ static noinline_for_stack int ethtool_flash_device(struct net_device *dev,
if (!dev->ethtool_ops->flash_device) {
int ret;
+ dev_hold(dev);
rtnl_unlock();
ret = devlink_compat_flash_update(dev, efl.data);
rtnl_lock();
+ dev_put(dev);
return ret;
}
--
2.19.2
^ permalink raw reply related
* [PATCH net-next 3/5] devlink: create a special NDO for getting the devlink instance
From: Jakub Kicinski @ 2019-02-21 17:46 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190221174620.12144-1-jakub.kicinski@netronome.com>
Instead of iterating over all devlink ports add a NDO which
will return the devlink instance from the driver.
Suggested-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
include/linux/netdevice.h | 6 ++++
net/core/devlink.c | 62 +++++++++++++--------------------------
2 files changed, 26 insertions(+), 42 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index aab4d9f6613d..eebcef6b8191 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -940,6 +940,8 @@ struct dev_ifalias {
char ifalias[];
};
+struct devlink;
+
/*
* This structure defines the management hooks for network devices.
* The following hooks can be defined; unless noted otherwise, they are
@@ -1248,6 +1250,9 @@ struct dev_ifalias {
* that got dropped are freed/returned via xdp_return_frame().
* Returns negative number, means general error invoking ndo, meaning
* no frames were xmit'ed and core-caller will free all frames.
+ * struct devlink *(*ndo_get_devlink)(struct net_device *dev);
+ * Get devlink instance associated with a given netdev.
+ * Called with a reference on the device only, rtnl_lock is not held.
*/
struct net_device_ops {
int (*ndo_init)(struct net_device *dev);
@@ -1446,6 +1451,7 @@ struct net_device_ops {
u32 flags);
int (*ndo_xsk_async_xmit)(struct net_device *dev,
u32 queue_id);
+ struct devlink * (*ndo_get_devlink)(struct net_device *dev);
};
/**
diff --git a/net/core/devlink.c b/net/core/devlink.c
index fcf09e02f4c6..050999a68c35 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6395,9 +6395,6 @@ static void __devlink_compat_running_version(struct devlink *devlink,
struct sk_buff *msg;
int rem, err;
- if (!devlink->ops->info_get)
- return;
-
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
if (!msg)
return;
@@ -6429,55 +6426,36 @@ static void __devlink_compat_running_version(struct devlink *devlink,
void devlink_compat_running_version(struct net_device *dev,
char *buf, size_t len)
{
- struct devlink_port *devlink_port;
struct devlink *devlink;
- mutex_lock(&devlink_mutex);
- list_for_each_entry(devlink, &devlink_list, list) {
- mutex_lock(&devlink->lock);
- list_for_each_entry(devlink_port, &devlink->port_list, list) {
- if (devlink_port->type == DEVLINK_PORT_TYPE_ETH &&
- devlink_port->type_dev == dev) {
- __devlink_compat_running_version(devlink,
- buf, len);
- mutex_unlock(&devlink->lock);
- goto out;
- }
- }
- mutex_unlock(&devlink->lock);
- }
-out:
- mutex_unlock(&devlink_mutex);
+ if (!dev->netdev_ops->ndo_get_devlink)
+ return;
+
+ devlink = dev->netdev_ops->ndo_get_devlink(dev);
+ if (!devlink || !devlink->ops->info_get)
+ return;
+
+ mutex_lock(&devlink->lock);
+ __devlink_compat_running_version(devlink, buf, len);
+ mutex_unlock(&devlink->lock);
}
int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
{
- struct devlink_port *devlink_port;
struct devlink *devlink;
+ int ret;
- mutex_lock(&devlink_mutex);
- list_for_each_entry(devlink, &devlink_list, list) {
- mutex_lock(&devlink->lock);
- list_for_each_entry(devlink_port, &devlink->port_list, list) {
- int ret = -EOPNOTSUPP;
-
- if (devlink_port->type != DEVLINK_PORT_TYPE_ETH ||
- devlink_port->type_dev != dev)
- continue;
+ if (!dev->netdev_ops->ndo_get_devlink)
+ return -EOPNOTSUPP;
- mutex_unlock(&devlink_mutex);
- if (devlink->ops->flash_update)
- ret = devlink->ops->flash_update(devlink,
- file_name,
- NULL, NULL);
- mutex_unlock(&devlink->lock);
- return ret;
- }
- mutex_unlock(&devlink->lock);
- }
- mutex_unlock(&devlink_mutex);
+ devlink = dev->netdev_ops->ndo_get_devlink(dev);
+ if (!devlink || !devlink->ops->flash_update)
+ return -EOPNOTSUPP;
- return -EOPNOTSUPP;
+ mutex_lock(&devlink->lock);
+ ret = devlink->ops->flash_update(devlink, file_name, NULL, NULL);
+ mutex_unlock(&devlink->lock);
+ return ret;
}
static int __init devlink_init(void)
--
2.19.2
^ permalink raw reply related
* [PATCH net-next 4/5] nfp: add .ndo_get_devlink
From: Jakub Kicinski @ 2019-02-21 17:46 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190221174620.12144-1-jakub.kicinski@netronome.com>
Support getting devlink instance from a new NDO.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/nfp_app.h | 2 ++
drivers/net/ethernet/netronome/nfp/nfp_devlink.c | 11 +++++++++++
drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 1 +
drivers/net/ethernet/netronome/nfp/nfp_net_repr.c | 1 +
4 files changed, 15 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.h b/drivers/net/ethernet/netronome/nfp/nfp_app.h
index d578d856a009..f8d422713705 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_app.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_app.h
@@ -433,4 +433,6 @@ int nfp_app_nic_vnic_alloc(struct nfp_app *app, struct nfp_net *nn,
int nfp_app_nic_vnic_init_phy_port(struct nfp_pf *pf, struct nfp_app *app,
struct nfp_net *nn, unsigned int id);
+struct devlink *nfp_devlink_get_devlink(struct net_device *netdev);
+
#endif
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index db2da99f6aa7..e9eca99cf493 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -376,3 +376,14 @@ void nfp_devlink_port_unregister(struct nfp_port *port)
{
devlink_port_unregister(&port->dl_port);
}
+
+struct devlink *nfp_devlink_get_devlink(struct net_device *netdev)
+{
+ struct nfp_app *app;
+
+ app = nfp_app_from_netdev(netdev);
+ if (!app)
+ return NULL;
+
+ return priv_to_devlink(app->pf);
+}
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 776f6c07701b..6d1b8816552e 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3531,6 +3531,7 @@ const struct net_device_ops nfp_net_netdev_ops = {
.ndo_udp_tunnel_del = nfp_net_del_vxlan_port,
.ndo_bpf = nfp_net_xdp,
.ndo_get_port_parent_id = nfp_port_get_port_parent_id,
+ .ndo_get_devlink = nfp_devlink_get_devlink,
};
/**
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
index 62839807e21e..d2c803bb4e56 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
@@ -273,6 +273,7 @@ const struct net_device_ops nfp_repr_netdev_ops = {
.ndo_set_features = nfp_port_set_features,
.ndo_set_mac_address = eth_mac_addr,
.ndo_get_port_parent_id = nfp_port_get_port_parent_id,
+ .ndo_get_devlink = nfp_devlink_get_devlink,
};
void
--
2.19.2
^ permalink raw reply related
* [PATCH net-next 5/5] nfp: remove ethtool flashing fallback
From: Jakub Kicinski @ 2019-02-21 17:46 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190221174620.12144-1-jakub.kicinski@netronome.com>
Now that devlink fallback will be called reliably, we can remove
the ethtool flashing code.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../ethernet/netronome/nfp/nfp_net_ethtool.c | 24 -------------------
1 file changed, 24 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index 8f189149efc5..690b62718dbb 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -1234,28 +1234,6 @@ static int nfp_net_set_channels(struct net_device *netdev,
return nfp_net_set_num_rings(nn, total_rx, total_tx);
}
-static int
-nfp_net_flash_device(struct net_device *netdev, struct ethtool_flash *flash)
-{
- struct nfp_app *app;
- int ret;
-
- if (flash->region != ETHTOOL_FLASH_ALL_REGIONS)
- return -EOPNOTSUPP;
-
- app = nfp_app_from_netdev(netdev);
- if (!app)
- return -EOPNOTSUPP;
-
- dev_hold(netdev);
- rtnl_unlock();
- ret = nfp_flash_update_common(app->pf, flash->data, NULL);
- rtnl_lock();
- dev_put(netdev);
-
- return ret;
-}
-
static const struct ethtool_ops nfp_net_ethtool_ops = {
.get_drvinfo = nfp_net_get_drvinfo,
.get_link = ethtool_op_get_link,
@@ -1266,7 +1244,6 @@ static const struct ethtool_ops nfp_net_ethtool_ops = {
.get_sset_count = nfp_net_get_sset_count,
.get_rxnfc = nfp_net_get_rxnfc,
.set_rxnfc = nfp_net_set_rxnfc,
- .flash_device = nfp_net_flash_device,
.get_rxfh_indir_size = nfp_net_get_rxfh_indir_size,
.get_rxfh_key_size = nfp_net_get_rxfh_key_size,
.get_rxfh = nfp_net_get_rxfh,
@@ -1292,7 +1269,6 @@ const struct ethtool_ops nfp_port_ethtool_ops = {
.get_strings = nfp_port_get_strings,
.get_ethtool_stats = nfp_port_get_stats,
.get_sset_count = nfp_port_get_sset_count,
- .flash_device = nfp_net_flash_device,
.set_dump = nfp_app_set_dump,
.get_dump_flag = nfp_app_get_dump_flag,
.get_dump_data = nfp_app_get_dump_data,
--
2.19.2
^ permalink raw reply related
* Re: stmmac / meson8b-dwmac
From: Jerome Brunet @ 2019-02-21 17:46 UTC (permalink / raw)
To: Simon Huelck, Jose Abreu, Martin Blumenstingl
Cc: linux-amlogic, Gpeppe.cavallaro, alexandre.torgue,
Emiliano Ingrassia, netdev
In-Reply-To: <9074d29b-4cc9-87b6-009f-48280a4692c0@gmx.de>
On Thu, 2019-02-21 at 18:27 +0100, Simon Huelck wrote:
> Hi,
>
>
>
> this was changed recently, with a patch for the EEE stuff , see here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.0-rc7&id=e35e26b26e955c53e61c154ba26b9bb15da6b858
Hu, I was not aware this finally went through. Good !
As explained in the patch and by Jose, the GMAC should be using IRQ_LEVEL.
The realtek PHY has EEE enabled by default. Having this enabled generates a
lot of (Low Power) Interrupts.
Previously, when the GMAC used IRQ_EDGE. Because it is wrong, we would
eventually miss an IRQ and the interface would just die. Unfortunately, it was
not that easy find out.
2 years ago, we just noticed that disabling EEE would make the failure go
away. Forcing this EEE feature off through DT was merely a work around.
Now that the real cause of the problem is known, there is no reason to keep
this hack around.
Whether EEE adds a performance penality and why, is another topic.
As Jose pointed out, you can disable EEE at runtime, using ethtool.
Jerome
^ permalink raw reply
* Re: [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers
From: Eric Sandeen @ 2019-02-21 17:47 UTC (permalink / raw)
To: Luis Chamberlain, Eric Sandeen, Andrew Morton
Cc: Linux Kernel Mailing List, fsdevel, netdev, Kees Cook
In-Reply-To: <20190221151854.GJ11489@garbanzo.do-not-panic.com>
On 2/21/19 9:18 AM, Luis Chamberlain wrote:
> On Wed, Feb 20, 2019 at 05:35:04PM -0600, Eric Sandeen wrote:
>> Here's a pretty hacky test script to test this code via
>> ip_local_reserved_ports
>
> Thanks Eric!
>
> So /proc/sys/net/ipv4/ip_local_reserved_ports is a production knob, and
> if we wanted to stress test it with a selftest it could break other self
> tests or change the system behaviour. Because of this we have now have
> lib/test_sysctl.c, and we test this with the script:
>
> tools/testing/selftests/sysctl/sysctl.sh
>
> Any chance you can extend lib/test_sysctl.c with a new respective bitmap
> knob,
Done
> and add a respective test? This will ensure we don't regress
> later. 0-day runs sysctl.sh so it should catch any regressions in the
> future.
As you know, learning somebody else's test harness infra is a PITA. ;)
Can you find me off-list and give me a hand with this?
Thanks,
-Eric
^ permalink raw reply
* Re: [PATCH net v2] ipv6: route: purge exception on removal
From: Paolo Abeni @ 2019-02-21 17:48 UTC (permalink / raw)
To: David Ahern, netdev; +Cc: David S. Miller
In-Reply-To: <cf0a4167-a61a-1d6d-b82c-c7c98c31ba92@gmail.com>
On Thu, 2019-02-21 at 10:10 -0500, David Ahern wrote:
> I am surprised this was not found by the existing pmtu script which
> creates exceptions. Please add this test case to selftests to capture
> this specific set of events.
Sure, I'll have a look at the self-tests. Should I target net (since
the fix is there) or net-next, since it'a new "feature" (test)?
Thank you for reviewing,
Paolo
^ permalink raw reply
* Re: [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers
From: Luis Chamberlain @ 2019-02-21 17:52 UTC (permalink / raw)
To: Eric Sandeen
Cc: Eric Sandeen, Andrew Morton, Linux Kernel Mailing List, fsdevel,
netdev, Kees Cook
In-Reply-To: <72a6d9bc-cfb9-a21a-960d-bb01331c0dd7@redhat.com>
On Thu, Feb 21, 2019 at 11:47:49AM -0600, Eric Sandeen wrote:
> On 2/21/19 9:18 AM, Luis Chamberlain wrote:
> > On Wed, Feb 20, 2019 at 05:35:04PM -0600, Eric Sandeen wrote:
> >> Here's a pretty hacky test script to test this code via
> >> ip_local_reserved_ports
> >
> > Thanks Eric!
> >
> > So /proc/sys/net/ipv4/ip_local_reserved_ports is a production knob, and
> > if we wanted to stress test it with a selftest it could break other self
> > tests or change the system behaviour. Because of this we have now have
> > lib/test_sysctl.c, and we test this with the script:
> >
> > tools/testing/selftests/sysctl/sysctl.sh
> >
> > Any chance you can extend lib/test_sysctl.c with a new respective bitmap
> > knob,
>
> Done
Thanks!
> > and add a respective test? This will ensure we don't regress
> > later. 0-day runs sysctl.sh so it should catch any regressions in the
> > future.
>
> As you know, learning somebody else's test harness infra is a PITA. ;)
> Can you find me off-list and give me a hand with this?
Sure, its actually quite simple, just as root, run the script.
Luis
^ 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