* Re: [PATCH net-next 4/8] mlxsw: spectrum_switchdev: Handle SWITCHDEV_PORT_ATTR_SET
From: Ido Schimmel @ 2019-02-23 10:36 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev@vger.kernel.org, David S. Miller, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE,
Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <20190222235927.10295-5-f.fainelli@gmail.com>
On Fri, Feb 22, 2019 at 03:59:22PM -0800, Florian Fainelli wrote:
> Following patches will change the way we communicate setting a port's
> attribute and use a notifier to perform those tasks.
>
> Prepare mlxsw to support receiving notifier events targeting
> SWITCHDEV_PORT_ATTR_SET and utilize the switchdev_handle_port_attr_set()
> to handle stacking of devices.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> .../net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> index 766f5b5f1cf5..c1aedfea3a31 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> @@ -3123,6 +3123,13 @@ static int mlxsw_sp_switchdev_event(struct notifier_block *unused,
> struct net_device *br_dev;
> int err;
>
> + if (event == SWITCHDEV_PORT_ATTR_SET) {
> + err = switchdev_handle_port_attr_set(dev, ptr,
> + mlxsw_sp_port_dev_check,
> + mlxsw_sp_port_attr_set);
> + return notifier_from_errno(err);
I don't think this code is ever executed. And if it was executed, we
would have problems because switchdev_handle_port_attr_set() might
block.
> + }
> +
> /* Tunnel devices are not our uppers, so check their master instead */
> br_dev = netdev_master_upper_dev_get_rcu(dev);
> if (!br_dev)
> @@ -3446,6 +3453,11 @@ static int mlxsw_sp_switchdev_blocking_event(struct notifier_block *unused,
> mlxsw_sp_port_dev_check,
> mlxsw_sp_port_obj_del);
> return notifier_from_errno(err);
> + case SWITCHDEV_PORT_ATTR_SET:
> + err = switchdev_handle_port_attr_set(dev, ptr,
> + mlxsw_sp_port_dev_check,
> + mlxsw_sp_port_attr_set);
> + return notifier_from_errno(err);
> }
>
> return NOTIFY_DONE;
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH net-next 1/2] xdp: Always use a devmap for XDP_REDIRECT to a device
From: Jesper Dangaard Brouer @ 2019-02-23 10:43 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Toke Høiland-Jørgensen, David Miller, netdev,
Daniel Borkmann, Alexei Starovoitov, brouer
In-Reply-To: <20190222133734.1880a88d@cakuba.netronome.com>
On Fri, 22 Feb 2019 13:37:34 -0800 Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> On Fri, 22 Feb 2019 11:13:50 +0100, Toke Høiland-Jørgensen wrote:
> > Jakub Kicinski <jakub.kicinski@netronome.com> writes:
> > > On Thu, 21 Feb 2019 12:56:54 +0100, Toke Høiland-Jørgensen wrote:
[...]
> > >
> > > BPF programs don't obey by netns boundaries. The fact the program is
> > > verified in one ns doesn't mean this is the only ns it will be used in :(
> > > Meaning if any program is using the redirect map you may need a secret
> > > map in every ns.. no?
> >
> > Ah, yes, good point. Totally didn't think about the fact that load and
> > attach are decoupled. Hmm, guess I'll just have to move the call to
> > alloc_default_map() to the point where the program is attached to an
> > interface, then...
>
> Possibly.. and you also need to handle the case where interface with a
> program attached is moved, no?
True, we need to handle if e.g. a veth gets an XDP program attached and
then is moved into a network namespace (as I've already explained to
Toke in a meeting).
I'm still not sure how to handle this...
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH] net: ax25: Add SPDX License to fix missing License warning
From: Jules Irenge @ 2019-02-23 11:11 UTC (permalink / raw)
To: jreuter
Cc: linux-hams, linux-kernel, davem, ralf, netdev, outreachy-kernel,
gregkh, jbi.octave
Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
net/ax25/af_ax25.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 5d01edf8d819..5048790a75f6 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
--
2.20.1
^ permalink raw reply related
* [PATCH] net: ax25: Fix multiple blank lines warning
From: Jules Irenge @ 2019-02-23 11:17 UTC (permalink / raw)
To: jreuter
Cc: linux-hams, linux-kernel, davem, ralf, netdev, outreachy-kernel,
jbi.octave
Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
net/ax25/af_ax25.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 5048790a75f6..2da349b7854a 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -49,8 +49,6 @@
#include <net/ip.h>
#include <net/arp.h>
-
-
HLIST_HEAD(ax25_list);
DEFINE_SPINLOCK(ax25_list_lock);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next 2/2] xdp: Add devmap_idx map type for looking up devices by ifindex
From: Toke Høiland-Jørgensen @ 2019-02-23 11:52 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David Miller, netdev, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov
In-Reply-To: <20190222133029.131d066d@cakuba.netronome.com>
Jakub Kicinski <jakub.kicinski@netronome.com> writes:
> On Fri, 22 Feb 2019 10:47:10 +0100, Toke Høiland-Jørgensen wrote:
>> Jakub Kicinski <jakub.kicinski@netronome.com> writes:
>>
>> > On Fri, 22 Feb 2019 00:02:23 +0100, Toke Høiland-Jørgensen wrote:
>> >> Jakub Kicinski <jakub.kicinski@netronome.com> writes:
>> >>
>> >> > On Thu, 21 Feb 2019 12:56:54 +0100, Toke Høiland-Jørgensen wrote:
>> >> >> A common pattern when using xdp_redirect_map() is to create a device map
>> >> >> where the lookup key is simply ifindex. Because device maps are arrays,
>> >> >> this leaves holes in the map, and the map has to be sized to fit the
>> >> >> largest ifindex, regardless of how many devices actually are actually
>> >> >> needed in the map.
>> >> >>
>> >> >> This patch adds a second type of device map where the key is interpreted as
>> >> >> an ifindex and looked up using a hashmap, instead of being used as an array
>> >> >> index. This leads to maps being densely packed, so they can be smaller.
>> >> >>
>> >> >> The default maps used by xdp_redirect() are changed to use the new map
>> >> >> type, which means that xdp_redirect() is no longer limited to ifindex < 64,
>> >> >> but instead to 64 total simultaneous interfaces per network namespace. This
>> >> >> also provides an easy way to compare the performance of devmap and
>> >> >> devmap_idx:
>> >> >>
>> >> >> xdp_redirect_map (devmap): 8394560 pkt/s
>> >> >> xdp_redirect (devmap_idx): 8179480 pkt/s
>> >> >>
>> >> >> Difference: 215080 pkt/s or 3.1 nanoseconds per packet.
>> >> >
>> >> > Could you share what the ifindex mix was here, to arrive at these
>> >> > numbers? How does it compare to using an array but not keying with
>> >> > ifindex?
>> >>
>> >> Just the standard set on my test machine; ifindex 1 through 9, except 8
>> >> in this case. So certainly no more than 1 ifindex in each hash bucket
>> >> for those numbers.
>> >
>> > Oh, I clearly misread your numbers, it's still slower than array, you
>> > just don't need the size limit.
>>
>> Yeah, this is not about speeding up devmap, it's about lifting the size
>> restriction.
>>
>> >> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> >> >
>> >> >> +static int dev_map_idx_update_elem(struct bpf_map *map, void *key, void *value,
>> >> >> + u64 map_flags)
>> >> >> +{
>> >> >> + struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
>> >> >> + struct bpf_dtab_netdev *dev, *old_dev;
>> >> >> + u32 idx = *(u32 *)key;
>> >> >> + u32 val = *(u32 *)value;
>> >> >> + u32 bit;
>> >> >> +
>> >> >> + if (unlikely(map_flags > BPF_EXIST))
>> >> >> + return -EINVAL;
>> >> >> + if (unlikely(map_flags == BPF_NOEXIST))
>> >> >> + return -EEXIST;
>> >> >> +
>> >> >> + old_dev = __dev_map_idx_lookup_elem(map, idx);
>> >> >> + if (!val) {
>> >> >> + if (!old_dev)
>> >> >> + return 0;
>> >> >
>> >> > IMHO this is a fairly strange mix of array and hashmap semantics. I
>> >> > think you should stick to hashmap behaviour AFA flags and
>> >> > update/delete goes.
>> >>
>> >> Yeah, the double book-keeping is a bit strange, but it allows the actual
>> >> forwarding and flush code to be reused between both types of maps. I
>> >> think this is worth the slight semantic confusion :)
>> >
>> > I'm not sure I was clear, let me try again :) Your get_next_key only
>> > reports existing indexes if I read the code right, so that's not an
>> > array - in an array indexes always exist. What follows inserting 0
>> > should not be equivalent to delete and BPF_NOEXIST should be handled
>> > appropriately.
>>
>> Ah, I see what you mean. Yeah, sure, I guess I can restrict deletion to
>> only working through explicit delete.
>>
>> I could also add a fail on NOEXIST, but since each index is tied to a
>> particular value, you can't actually change the contents of each index,
>> only insert and remove. So why would you ever set that flag?
>
> The reason user would have for setting the flag is not clear :) But
> if you want to reject it because it's unsupported/makes no sense, you
> should do EINVAL, not EEXIST ;)
>
>> > Different maps behave differently, I think it's worth trying to limit
>> > the divergence in how things behave to the basic array and a hashmap
>> > models when possible.
>>
>> So I don't actually think of this as a hashmap in the general sense;
>> after all, you can only store ifindexes in it, and key and value are
>> tied to one another. So it's an ifindex'ed devmap (which is also why I
>> named it devmap_idx and not devmap_hash); the fact that it's implemented
>> as a hashmap is just incidental.
>>
>> So I guess it's a choice between being consistent with the other devmap
>> type, or with a general hashmap. I'm not actually sure that the latter
>> is less surprising? :)
>
> The distinction is that if entry is not in the map get_next won't
> return its key. As you say the construct is not really a hash map
> (probably a set is the closest) but it most definitely is not an
> array, so no hard EEXIST on NOEXIST flag :)
Yeah, I thought about it some more and I agree it makes sense to change
the update semantics to be a bit more hashmap-like :)
-Toke
^ permalink raw reply
* [PATCH] tun: fix blocking read
From: Timur Celik @ 2019-02-23 11:53 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, Timur Celik
This patch moves setting of the current state into the loop. Otherwise
the task may end up in a busy wait loop if none of the break conditions
are met.
Signed-off-by: Timur Celik <mail@timurcelik.de>
---
drivers/net/tun.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index fed298c0cb39..d291762b9e9d 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2167,9 +2167,9 @@ static void *tun_ring_recv(struct tun_file *tfile, int noblock, int *err)
}
add_wait_queue(&tfile->wq.wait, &wait);
- current->state = TASK_INTERRUPTIBLE;
while (1) {
+ set_current_state(TASK_INTERRUPTIBLE);
ptr = ptr_ring_consume(&tfile->tx_ring);
if (ptr)
break;
@@ -2185,7 +2185,7 @@ static void *tun_ring_recv(struct tun_file *tfile, int noblock, int *err)
schedule();
}
- current->state = TASK_RUNNING;
+ set_current_state(TASK_RUNNING);
remove_wait_queue(&tfile->wq.wait, &wait);
out:
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next 1/2] xdp: Always use a devmap for XDP_REDIRECT to a device
From: Toke Høiland-Jørgensen @ 2019-02-23 12:11 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Jakub Kicinski
Cc: David Miller, netdev, Daniel Borkmann, Alexei Starovoitov, brouer
In-Reply-To: <20190223114343.5813f18a@carbon>
Jesper Dangaard Brouer <brouer@redhat.com> writes:
> On Fri, 22 Feb 2019 13:37:34 -0800 Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
>
>> On Fri, 22 Feb 2019 11:13:50 +0100, Toke Høiland-Jørgensen wrote:
>> > Jakub Kicinski <jakub.kicinski@netronome.com> writes:
>> > > On Thu, 21 Feb 2019 12:56:54 +0100, Toke Høiland-Jørgensen wrote:
> [...]
>> > >
>> > > BPF programs don't obey by netns boundaries. The fact the program is
>> > > verified in one ns doesn't mean this is the only ns it will be used in :(
>> > > Meaning if any program is using the redirect map you may need a secret
>> > > map in every ns.. no?
>> >
>> > Ah, yes, good point. Totally didn't think about the fact that load and
>> > attach are decoupled. Hmm, guess I'll just have to move the call to
>> > alloc_default_map() to the point where the program is attached to an
>> > interface, then...
>>
>> Possibly.. and you also need to handle the case where interface with a
>> program attached is moved, no?
Yup, alloc on attach was easy enough; the moving turns out to be the
tricky part :)
> True, we need to handle if e.g. a veth gets an XDP program attached and
> then is moved into a network namespace (as I've already explained to
> Toke in a meeting).
Yeah, I had somehow convinced myself that the XDP program was being
removed when the interface was being torn down before moving between
namespaces. Jesper pointed out that this was not in fact the case... :P
> I'm still not sure how to handle this...
There are a couple of options, I think. At least:
1. Maintain a flag on struct net_device indicating that this device
needs the redirect map allocated, and react to that when interfaces
are being moved.
2. Lookup the BPF program by ID (which we can get from the driver) on
move, and react to the program flag.
3. Keep the allocation on program load, but allocate maps for all active
namespaces (which would probably need a refcnt mechanism to
deallocate things again).
I think I'm leaning towards #2; possibly combined with a refcnt so we
can actually deallocate the map in the root namespace when it's not
needed anymore.
-Toke
^ permalink raw reply
* Re: [PATCH] net: ax25: Add SPDX License to fix missing License warning
From: Greg KH @ 2019-02-23 12:58 UTC (permalink / raw)
To: Jules Irenge
Cc: jreuter, linux-hams, linux-kernel, davem, ralf, netdev,
outreachy-kernel
In-Reply-To: <20190223111100.13983-1-jbi.octave@gmail.com>
On Sat, Feb 23, 2019 at 11:11:00AM +0000, Jules Irenge wrote:
> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
I know I never take patches without any changelog text :(
> ---
> net/ax25/af_ax25.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
> index 5d01edf8d819..5048790a75f6 100644
> --- a/net/ax25/af_ax25.c
> +++ b/net/ax25/af_ax25.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0
That is the incorrect license of this file!
NEVER add or modify a SPDX line if you do not really really really know
what you are doing. Putting the wrong license tag on a file is worse
than not having any tag at all.
BE VERY CAREFUL ABOUT THESE THINGS, they matter. If you get them wrong,
lawyers get very grumpy :)
thanks,
greg k-h
^ permalink raw reply
* Re: [Outreachy kernel] [PATCH] net: ax25: Fix multiple blank lines warning
From: Greg KH @ 2019-02-23 12:59 UTC (permalink / raw)
To: Jules Irenge
Cc: jreuter, linux-hams, linux-kernel, davem, ralf, netdev,
outreachy-kernel
In-Reply-To: <20190223111751.14620-1-jbi.octave@gmail.com>
On Sat, Feb 23, 2019 at 11:17:51AM +0000, Jules Irenge wrote:
> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> ---
> net/ax25/af_ax25.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
> index 5048790a75f6..2da349b7854a 100644
> --- a/net/ax25/af_ax25.c
> +++ b/net/ax25/af_ax25.c
> @@ -49,8 +49,6 @@
> #include <net/ip.h>
> #include <net/arp.h>
>
> -
> -
> HLIST_HEAD(ax25_list);
> DEFINE_SPINLOCK(ax25_list_lock);
Why are you doing outreachy application patches in net/? We only accept
them in drivers/staging/ Always start there first.
thanks,
greg k-h
^ permalink raw reply
* Re: [Outreachy kernel] [PATCH] net: ax25: Fix multiple blank lines warning
From: Ivan Ivanov @ 2019-02-23 13:17 UTC (permalink / raw)
To: Jules Irenge, Greg KH, jreuter, linux-hams,
Linux Kernel Mailing List, ralf, davem, netdev, outreachy-kernel
In-Reply-To: <20190223125932.GC16686@kroah.com>
as usual, Outreachy is doing stiuupid almost-useless commits like
remove two blank lines or fix a tiny typo. They are trying hard to
pretend that they are useful by doing many lame patches like this,
while promoting their SJW madness, anti-male sieexism and anti-white
riaacism. But with the lame stuff like this patch, they are failing
miserably. Maybe hire a ghostwriter (from NSA) to write some drivers
for you?
P.S. and no I am not going to write them for you because I disagree
with your Ci0ck of Conduct and now have to write from anonymous
account
On Sat, Feb 23, 2019, Greg KH <greg@kroah.com> wrote:
>
> On Sat, Feb 23, 2019 at 11:17:51AM +0000, Jules Irenge wrote:
> > Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> > ---
> > net/ax25/af_ax25.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
> > index 5048790a75f6..2da349b7854a 100644
> > --- a/net/ax25/af_ax25.c
> > +++ b/net/ax25/af_ax25.c
> > @@ -49,8 +49,6 @@
> > #include <net/ip.h>
> > #include <net/arp.h>
> >
> > -
> > -
> > HLIST_HEAD(ax25_list);
> > DEFINE_SPINLOCK(ax25_list_lock);
>
> Why are you doing outreachy application patches in net/? We only accept
> them in drivers/staging/ Always start there first.
>
> thanks,
>
> greg k-h
^ permalink raw reply
* Re: [Outreachy kernel] [PATCH] net: ax25: Fix multiple blank lines warning
From: Julia Lawall @ 2019-02-23 13:22 UTC (permalink / raw)
To: Jules Irenge
Cc: jreuter, linux-hams, linux-kernel, davem, ralf, netdev,
outreachy-kernel
In-Reply-To: <20190223111751.14620-1-jbi.octave@gmail.com>
On Sat, 23 Feb 2019, Jules Irenge wrote:
You need a commit message here.
> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> ---
> net/ax25/af_ax25.c | 2 --
This is the same file as in your previous patch, so they should be in a
series.
But please work on files in drivers/staging instead.
julia
> 1 file changed, 2 deletions(-)
>
> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
> index 5048790a75f6..2da349b7854a 100644
> --- a/net/ax25/af_ax25.c
> +++ b/net/ax25/af_ax25.c
> @@ -49,8 +49,6 @@
> #include <net/ip.h>
> #include <net/arp.h>
>
> -
> -
> HLIST_HEAD(ax25_list);
> DEFINE_SPINLOCK(ax25_list_lock);
>
> --
> 2.20.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20190223111751.14620-1-jbi.octave%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply
* [PATCH net-next v4] ip_tunnel: Add dst_cache support in lwtunnel_state of ip tunnel
From: wenxu @ 2019-02-23 13:32 UTC (permalink / raw)
To: davem; +Cc: netdev
From: wenxu <wenxu@ucloud.cn>
The lwtunnel_state is not init the dst_cache Which make the
ip_md_tunnel_xmit can't use the dst_cache. It will lookup
route table every packets.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
net/core/lwtunnel.c | 16 ++++++++--------
net/ipv4/ip_tunnel_core.c | 18 ++++++++++++++++++
2 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
index 0b17175..4109095 100644
--- a/net/core/lwtunnel.c
+++ b/net/core/lwtunnel.c
@@ -122,18 +122,18 @@ int lwtunnel_build_state(u16 encap_type,
ret = -EOPNOTSUPP;
rcu_read_lock();
ops = rcu_dereference(lwtun_encaps[encap_type]);
- if (likely(ops && ops->build_state && try_module_get(ops->owner))) {
+ if (likely(ops && ops->build_state && try_module_get(ops->owner)))
found = true;
+ rcu_read_unlock();
+
+ if (found) {
ret = ops->build_state(encap, family, cfg, lws, extack);
if (ret)
module_put(ops->owner);
- }
- rcu_read_unlock();
-
- /* don't rely on -EOPNOTSUPP to detect match as build_state
- * handlers could return it
- */
- if (!found) {
+ } else {
+ /* don't rely on -EOPNOTSUPP to detect match as build_state
+ * handlers could return it
+ */
NL_SET_ERR_MSG_ATTR(extack, encap,
"LWT encapsulation type not supported");
}
diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index 9a0e67b..c3f3d28 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -252,6 +252,14 @@ static int ip_tun_build_state(struct nlattr *attr,
tun_info = lwt_tun_info(new_state);
+#ifdef CONFIG_DST_CACHE
+ err = dst_cache_init(&tun_info->dst_cache, GFP_KERNEL);
+ if (err) {
+ lwtstate_free(new_state);
+ return err;
+ }
+#endif
+
if (tb[LWTUNNEL_IP_ID])
tun_info->key.tun_id = nla_get_be64(tb[LWTUNNEL_IP_ID]);
@@ -278,6 +286,15 @@ static int ip_tun_build_state(struct nlattr *attr,
return 0;
}
+static void ip_tun_destroy_state(struct lwtunnel_state *lwtstate)
+{
+#ifdef CONFIG_DST_CACHE
+ struct ip_tunnel_info *tun_info = lwt_tun_info(lwtstate);
+
+ dst_cache_destroy(&tun_info->dst_cache);
+#endif
+}
+
static int ip_tun_fill_encap_info(struct sk_buff *skb,
struct lwtunnel_state *lwtstate)
{
@@ -313,6 +330,7 @@ static int ip_tun_cmp_encap(struct lwtunnel_state *a, struct lwtunnel_state *b)
static const struct lwtunnel_encap_ops ip_tun_lwt_ops = {
.build_state = ip_tun_build_state,
+ .destroy_state = ip_tun_destroy_state,
.fill_encap = ip_tun_fill_encap_info,
.get_encap_size = ip_tun_encap_nlsize,
.cmp_encap = ip_tun_cmp_encap,
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3] route: Add multipath_hash in flowi_common to make user-define hash
From: wenxu @ 2019-02-23 14:53 UTC (permalink / raw)
To: nikolay, davem; +Cc: netdev
From: wenxu <wenxu@ucloud.cn>
Current fib_multipath_hash_policy can make hash based on the L3 or
L4. But it only work on the outer IP. So a specific tunnel always
has the same hash value. But a specific tunnel may contain so many
inner connections.
This patch provide a generic multipath_hash in floi_common. It can
make a user-define hash which can mix with L3 or L4 hash.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c | 2 +-
include/net/flow.h | 2 ++
include/net/ip_tunnels.h | 3 ++-
net/ipv4/ip_gre.c | 2 +-
net/ipv4/ip_tunnel.c | 6 +++---
net/ipv4/route.c | 4 ++++
6 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
index ad5a9b9..536c23c 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
@@ -305,7 +305,7 @@ static int mlxsw_sp_span_dmac(struct neigh_table *tbl,
parms = mlxsw_sp_ipip_netdev_parms4(to_dev);
ip_tunnel_init_flow(&fl4, parms.iph.protocol, *daddrp, *saddrp,
- 0, 0, parms.link, tun->fwmark);
+ 0, 0, parms.link, tun->fwmark, 0);
rt = ip_route_output_key(tun->net, &fl4);
if (IS_ERR(rt))
diff --git a/include/net/flow.h b/include/net/flow.h
index 93f2c9a..4da503a 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -40,6 +40,7 @@ struct flowi_common {
__u32 flowic_secid;
kuid_t flowic_uid;
struct flowi_tunnel flowic_tun_key;
+ __be32 flowic_multipath_hash;
};
union flowi_uli {
@@ -78,6 +79,7 @@ struct flowi4 {
#define flowi4_secid __fl_common.flowic_secid
#define flowi4_tun_key __fl_common.flowic_tun_key
#define flowi4_uid __fl_common.flowic_uid
+#define flowi4_multipath_hash __fl_common.flowic_multipath_hash
/* (saddr,daddr) must be grouped, same order as in IP header */
__be32 saddr;
diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index f069f64..af64560 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -241,7 +241,7 @@ static inline void ip_tunnel_init_flow(struct flowi4 *fl4,
int proto,
__be32 daddr, __be32 saddr,
__be32 key, __u8 tos, int oif,
- __u32 mark)
+ __u32 mark, __u32 tun_inner_hash)
{
memset(fl4, 0, sizeof(*fl4));
fl4->flowi4_oif = oif;
@@ -251,6 +251,7 @@ static inline void ip_tunnel_init_flow(struct flowi4 *fl4,
fl4->flowi4_proto = proto;
fl4->fl4_gre_key = key;
fl4->flowi4_mark = mark;
+ fl4->flowi4_multipath_hash = tun_inner_hash;
}
int ip_tunnel_init(struct net_device *dev);
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index ccee941..88ad95e 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -578,7 +578,7 @@ static int gre_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
key = &info->key;
ip_tunnel_init_flow(&fl4, IPPROTO_GRE, key->u.ipv4.dst, key->u.ipv4.src,
tunnel_id_to_key32(key->tun_id), key->tos, 0,
- skb->mark);
+ skb->mark, skb_get_hash(skb));
rt = ip_route_output_key(dev_net(dev), &fl4);
if (IS_ERR(rt))
return PTR_ERR(rt);
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 893f013..3a03114 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -310,7 +310,7 @@ static int ip_tunnel_bind_dev(struct net_device *dev)
ip_tunnel_init_flow(&fl4, iph->protocol, iph->daddr,
iph->saddr, tunnel->parms.o_key,
RT_TOS(iph->tos), tunnel->parms.link,
- tunnel->fwmark);
+ tunnel->fwmark, 0);
rt = ip_route_output_key(tunnel->net, &fl4);
if (!IS_ERR(rt)) {
@@ -584,7 +584,7 @@ void ip_md_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
}
ip_tunnel_init_flow(&fl4, proto, key->u.ipv4.dst, key->u.ipv4.src,
tunnel_id_to_key32(key->tun_id), RT_TOS(tos),
- 0, skb->mark);
+ 0, skb->mark, skb_get_hash(skb));
if (tunnel->encap.type != TUNNEL_ENCAP_NONE)
goto tx_error;
@@ -738,7 +738,7 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
ip_tunnel_init_flow(&fl4, protocol, dst, tnl_params->saddr,
tunnel->parms.o_key, RT_TOS(tos), tunnel->parms.link,
- tunnel->fwmark);
+ tunnel->fwmark, skb_get_hash(skb));
if (ip_tunnel_encap(skb, tunnel, &protocol, &fl4) < 0)
goto tx_error;
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index ecc12a7..6b9a8b7 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1820,6 +1820,7 @@ static void ip_multipath_l3_keys(const struct sk_buff *skb,
int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4,
const struct sk_buff *skb, struct flow_keys *flkeys)
{
+ u32 multipath_hash = fl4->flowi4_multipath_hash;
struct flow_keys hash_keys;
u32 mhash;
@@ -1869,6 +1870,9 @@ int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4,
break;
}
mhash = flow_hash_from_keys(&hash_keys);
+
+ if (multipath_hash)
+ mhash = jhash_2words(mhash, multipath_hash, 0);
return mhash >> 1;
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] net: phy: realtek: Dummy IRQ calls for RTL8366RB
From: Andrew Lunn @ 2019-02-23 15:17 UTC (permalink / raw)
To: Linus Walleij; +Cc: Florian Fainelli, netdev, Heiner Kallweit
In-Reply-To: <20190223023639.27038-1-linus.walleij@linaro.org>
On Sat, Feb 23, 2019 at 03:36:39AM +0100, Linus Walleij wrote:
> This fixes a regression introduced by
> commit 0d2e778e38e0ddffab4bb2b0e9ed2ad5165c4bf7
> "net: phy: replace PHY_HAS_INTERRUPT with a check for
> config_intr and ack_interrupt".
>
> This assumes that a PHY cannot trigger interrupt unless
> it has .config_intr() or .ack_interrupt() implemented.
> A later patch makes the code assume both need to be
> implemented for interrupts to be present.
>
> But this PHY (which is inside a DSA) will happily
> fire interrupts without either callback.
Hi Linus
Can you disable these interrupts?
If you have dummy implementations, what is clearing the interrupt?
Andrew
^ permalink raw reply
* Re: [PATCH 2/3] net: ethernet: add c45 PHY support in MDIO read/write functions.
From: Andrew Lunn @ 2019-02-23 15:23 UTC (permalink / raw)
To: Parshuram Raju Thombare
Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
netdev@vger.kernel.org, f.fainelli@gmail.com,
hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
Rafal Ciepiela, Piotr Sroka, Jan Kotas
In-Reply-To: <CO2PR07MB2469D90FD13642D668F8D281C1780@CO2PR07MB2469.namprd07.prod.outlook.com>
> >On Fri, Feb 22, 2019 at 08:12:42PM +0000, Parshuram Thombare wrote:
> >> This patch modify MDIO read/write functions to support communication
> >> with C45 PHY in Cadence ethernet controller driver.
> >
> >Hi Parshuram
> >
> >Are all versions of the MDIO controller capable of doing C45?
> >
> > Andrew
> Now driver support c22 and c45 PHY.
> Are you suggesting to add check for C45 PHY using is_c45 in phydev ?
You are unconditionally supporting C45. Are there versions of the
hardware which don't actually support C45? You have this endless loop:
+ /* wait for end of transfer */
+ while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
+ cpu_relax();
If there is hardware which does not support C45, will this loop
forever?
Andrew
^ permalink raw reply
* Re: [PATCH 2/3] net: ethernet: add c45 PHY support in MDIO read/write functions.
From: Andrew Lunn @ 2019-02-23 15:25 UTC (permalink / raw)
To: Parshuram Thombare
Cc: nicolas.ferre, davem, netdev, f.fainelli, hkallweit1,
linux-kernel, rafalc, piotrs, jank
In-Reply-To: <20190222201242.GA20889@lvlogina.cadence.com>
On Fri, Feb 22, 2019 at 08:12:42PM +0000, Parshuram Thombare wrote:
> This patch modify MDIO read/write functions to support
> communication with C45 PHY in Cadence ethernet controller driver.
>
> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
> ---
> drivers/net/ethernet/cadence/macb.h | 15 +++++--
> drivers/net/ethernet/cadence/macb_main.c | 61 ++++++++++++++++++++++++-----
> 2 files changed, 61 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index bed4ded..59c23e0 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -636,10 +636,17 @@
> #define GEM_CLK_DIV96 5
>
> /* Constants for MAN register */
> -#define MACB_MAN_SOF 1
> -#define MACB_MAN_WRITE 1
> -#define MACB_MAN_READ 2
> -#define MACB_MAN_CODE 2
> +#define MACB_MAN_C22_SOF 1
> +#define MACB_MAN_C22_WRITE 1
> +#define MACB_MAN_C22_READ 2
> +#define MACB_MAN_C22_CODE 2
> +
> +#define MACB_MAN_C45_SOF 0
> +#define MACB_MAN_C45_ADDR 0
> +#define MACB_MAN_C45_WRITE 1
> +#define MACB_MAN_C45_POST_READ_INCR 2
> +#define MACB_MAN_C45_READ 3
> +#define MACB_MAN_C45_CODE 2
>
> /* Capability mask bits */
> #define MACB_CAPS_ISR_CLEAR_ON_WRITE 0x00000001
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 4f4f8e5..2494abf 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -323,11 +323,30 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
> struct macb *bp = bus->priv;
> int value;
>
> - macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
> - | MACB_BF(RW, MACB_MAN_READ)
> - | MACB_BF(PHYA, mii_id)
> - | MACB_BF(REGA, regnum)
> - | MACB_BF(CODE, MACB_MAN_CODE)));
> + if (regnum & MII_ADDR_C45) {
> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
> + | MACB_BF(RW, MACB_MAN_C45_ADDR)
> + | MACB_BF(PHYA, mii_id)
> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
> + | MACB_BF(DATA, regnum & 0xFFFF)
> + | MACB_BF(CODE, MACB_MAN_C45_CODE)));
> +
> + /* wait for end of transfer */
> + while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
> + cpu_relax();
You need a timeout here, and anywhere you wait for the hardware to
complete. Try to make use of readx_poll_timeout() variants.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 1/7] net: phy: marvell10g: Use get_features to get the PHY abilities
From: Andrew Lunn @ 2019-02-23 15:36 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, Florian Fainelli, Heiner Kallweit,
Russell King, linux-arm-kernel, Antoine Tenart, thomas.petazzoni,
gregory.clement, miquel.raynal, nadavh, stefanc, mw
In-Reply-To: <20190222233744.25735-2-maxime.chevallier@bootlin.com>
On Sat, Feb 23, 2019 at 12:37:38AM +0100, Maxime Chevallier wrote:
> The Alaska family of 10G PHYs has more abilities than the ones listed in
> PHY_10GBIT_FULL_FEATURES, the exact list depending on the model.
>
> Make use of the newly introduced .get_features call to build this list,
> using genphy_c45_pma_read_abilities to build the list of supported
> linkmodes, and adding autoneg ability based on what's reported by the AN
> MMD.
>
> .config_init is still used to validate the interface_mode.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 2/7] net: phy: marvell10g: Use linkmode_set_bit helper instead of __set_bit
From: Andrew Lunn @ 2019-02-23 15:37 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, Florian Fainelli, Heiner Kallweit,
Russell King, linux-arm-kernel, Antoine Tenart, thomas.petazzoni,
gregory.clement, miquel.raynal, nadavh, stefanc, mw
In-Reply-To: <20190222233744.25735-3-maxime.chevallier@bootlin.com>
On Sat, Feb 23, 2019 at 12:37:39AM +0100, Maxime Chevallier wrote:
> Cosmetic patch making use of helpers dedicated to linkmodes handling.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 3/7] net: phy: marvell10g: Use 2500BASEX when using 2.5GBASET
From: Andrew Lunn @ 2019-02-23 15:38 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, Florian Fainelli, Heiner Kallweit,
Russell King, linux-arm-kernel, Antoine Tenart, thomas.petazzoni,
gregory.clement, miquel.raynal, nadavh, stefanc, mw
In-Reply-To: <20190222233744.25735-4-maxime.chevallier@bootlin.com>
On Sat, Feb 23, 2019 at 12:37:40AM +0100, Maxime Chevallier wrote:
> The Marvell Alaska family of PHYs supports 2.5GBaseT and 5GBaseT modes,
> as defined in the 802.3bz specification.
>
> Upon establishing a 2.5GBASET link, the PHY will reconfigure it's MII
> interface to 2500BASEX.
>
> At 5G, the PHY will reconfigure it's interface to 5GBASE-R, but this
> mode isn't supported by any MAC for now.
>
> This was tested with :
> - The 88X3310, which is on the MacchiatoBin
> - The 88E2010, an Alaska PHY that has no fiber interfaces, and is
> limited to 5G maximum speed.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 4/7] net: phy: marvell10g: Use a #define for 88X3310 family id
From: Andrew Lunn @ 2019-02-23 15:39 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, Florian Fainelli, Heiner Kallweit,
Russell King, linux-arm-kernel, Antoine Tenart, thomas.petazzoni,
gregory.clement, miquel.raynal, nadavh, stefanc, mw
In-Reply-To: <20190222233744.25735-5-maxime.chevallier@bootlin.com>
On Sat, Feb 23, 2019 at 12:37:41AM +0100, Maxime Chevallier wrote:
> The PHY ID corresponding to the 88X3310 is also used for other PHYs in
> the same family, such as the 88E2010. Use a #define for the PHY id, that
> ignores the last nibble.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 5/7] net: phy: marvell10g: Force reading of 2.5/5G
From: Andrew Lunn @ 2019-02-23 15:40 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, Florian Fainelli, Heiner Kallweit,
Russell King, linux-arm-kernel, Antoine Tenart, thomas.petazzoni,
gregory.clement, miquel.raynal, nadavh, stefanc, mw
In-Reply-To: <20190222233744.25735-6-maxime.chevallier@bootlin.com>
On Sat, Feb 23, 2019 at 12:37:42AM +0100, Maxime Chevallier wrote:
> As per 802.3bz, if bit 14 of (1.11) "PMA Extended Abilities" indicates
> whether or not we should read register (1.21) "2.52/5G PMA Extended
> Abilities", which contains information on the support of 2.5GBASET and
> 5GBASET.
>
> After testing on several variants of PHYS of this family, it appears
> that bit 14 in (1.11) isn't always set when it should be.
>
> PHYs 88X3310 (on MacchiatoBin) and 88E2010 do support 2.5G and 5GBASET,
> but don't have 1.11.14 set. Their register 1.21 is filled with the
> correct values, indicating 2.5G and 5G support.
>
> PHYs 88E2110 do have their 1.11.14 bit set, as it should.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 7/7] net: phy: marvell10g: add support for the 88x2110 PHY
From: Andrew Lunn @ 2019-02-23 15:41 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, Florian Fainelli, Heiner Kallweit,
Russell King, linux-arm-kernel, Antoine Tenart, thomas.petazzoni,
gregory.clement, miquel.raynal, nadavh, stefanc, mw
In-Reply-To: <20190222233744.25735-8-maxime.chevallier@bootlin.com>
On Sat, Feb 23, 2019 at 12:37:44AM +0100, Maxime Chevallier wrote:
> This patch adds support for the 88x2110 PHY, which is similar to the
> already supported 88x3310 PHY without the SFP interface.
>
> It supports 10/100/1000BASET along with 2.5GBASET, 5GBASET and 10GBASET,
> with the same interface modes that are used by the 3310.
>
> This PHY don't have the same issue as the 88x3310 regarding 2.5/5G
> abilities, and correctly follows the 802.3bz standard to list the
> supported abilities.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Suggested-by: Antoine Tenart <antoine.tenart@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* [PATCH net-next 2/2] net: dsa: mv88e6xxx: Release lock while requesting IRQ
From: Andrew Lunn @ 2019-02-23 16:43 UTC (permalink / raw)
To: David Miller; +Cc: Vivien Didelot, Russell King, netdev, Andrew Lunn
In-Reply-To: <20190223164357.17284-1-andrew@lunn.ch>
There is no need to hold the register lock while requesting the GPIO
interrupt. By not holding it we can also avoid a false positive
lockdep splat.
Reported-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/chip.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 7ee8607bf3a6..9cb17d3ef590 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -456,10 +456,12 @@ static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip)
*/
irq_set_lockdep_class(chip->irq, &lock_key, &request_key);
+ mutex_unlock(&chip->reg_lock);
err = request_threaded_irq(chip->irq, NULL,
mv88e6xxx_g1_irq_thread_fn,
IRQF_ONESHOT | IRQF_SHARED,
dev_name(chip->dev), chip);
+ mutex_lock(&chip->reg_lock);
if (err)
mv88e6xxx_g1_irq_free_common(chip);
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 1/2] net: dsa: mv88e6xxx: Add lockdep classes to fix false positive splat
From: Andrew Lunn @ 2019-02-23 16:43 UTC (permalink / raw)
To: David Miller; +Cc: Vivien Didelot, Russell King, netdev, Andrew Lunn
In-Reply-To: <20190223164357.17284-1-andrew@lunn.ch>
The following false positive lockdep splat has been observed.
======================================================
WARNING: possible circular locking dependency detected
4.20.0+ #302 Not tainted
------------------------------------------------------
systemd-udevd/160 is trying to acquire lock:
edea6080 (&chip->reg_lock){+.+.}, at: __setup_irq+0x640/0x704
but task is already holding lock:
edff0340 (&desc->request_mutex){+.+.}, at: __setup_irq+0xa0/0x704
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&desc->request_mutex){+.+.}:
mutex_lock_nested+0x1c/0x24
__setup_irq+0xa0/0x704
request_threaded_irq+0xd0/0x150
mv88e6xxx_probe+0x41c/0x694 [mv88e6xxx]
mdio_probe+0x2c/0x54
really_probe+0x200/0x2c4
driver_probe_device+0x5c/0x174
__driver_attach+0xd8/0xdc
bus_for_each_dev+0x58/0x7c
bus_add_driver+0xe4/0x1f0
driver_register+0x7c/0x110
mdio_driver_register+0x24/0x58
do_one_initcall+0x74/0x2e8
do_init_module+0x60/0x1d0
load_module+0x1968/0x1ff4
sys_finit_module+0x8c/0x98
ret_fast_syscall+0x0/0x28
0xbedf2ae8
-> #0 (&chip->reg_lock){+.+.}:
__mutex_lock+0x50/0x8b8
mutex_lock_nested+0x1c/0x24
__setup_irq+0x640/0x704
request_threaded_irq+0xd0/0x150
mv88e6xxx_g2_irq_setup+0xcc/0x1b4 [mv88e6xxx]
mv88e6xxx_probe+0x44c/0x694 [mv88e6xxx]
mdio_probe+0x2c/0x54
really_probe+0x200/0x2c4
driver_probe_device+0x5c/0x174
__driver_attach+0xd8/0xdc
bus_for_each_dev+0x58/0x7c
bus_add_driver+0xe4/0x1f0
driver_register+0x7c/0x110
mdio_driver_register+0x24/0x58
do_one_initcall+0x74/0x2e8
do_init_module+0x60/0x1d0
load_module+0x1968/0x1ff4
sys_finit_module+0x8c/0x98
ret_fast_syscall+0x0/0x28
0xbedf2ae8
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&desc->request_mutex);
lock(&chip->reg_lock);
lock(&desc->request_mutex);
lock(&chip->reg_lock);
&desc->request_mutex refer to two different mutex. #1 is the GPIO for
the chip interrupt. #2 is the chained interrupt between global 1 and
global 2.
Add lockdep classes to the GPIO interrupt to avoid this.
Reported-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/chip.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index cc7ce06b6d58..7ee8607bf3a6 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -442,12 +442,20 @@ static int mv88e6xxx_g1_irq_setup_common(struct mv88e6xxx_chip *chip)
static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip)
{
+ static struct lock_class_key lock_key;
+ static struct lock_class_key request_key;
int err;
err = mv88e6xxx_g1_irq_setup_common(chip);
if (err)
return err;
+ /* These lock classes tells lockdep that global 1 irqs are in
+ * a different category than their parent GPIO, so it won't
+ * report false recursion.
+ */
+ irq_set_lockdep_class(chip->irq, &lock_key, &request_key);
+
err = request_threaded_irq(chip->irq, NULL,
mv88e6xxx_g1_irq_thread_fn,
IRQF_ONESHOT | IRQF_SHARED,
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 0/2] mv88e6xxx: Avoid false positive Lockdep splats
From: Andrew Lunn @ 2019-02-23 16:43 UTC (permalink / raw)
To: David Miller; +Cc: Vivien Didelot, Russell King, netdev, Andrew Lunn
When acquiring the GPIO interrupt line for the switch, it is possible
to trigger lockdep splats. These are false positives, the mutex is in
a different IRQ descriptor. But fix it anyway, since it could mask
real locking issues.
Andrew Lunn (2):
net: dsa: mv88e6xxx: Add lockdep classes to fix false positive splat
net: dsa: mv88e6xxx: Release lock while requesting IRQ
drivers/net/dsa/mv88e6xxx/chip.c | 10 ++++++++++
1 file changed, 10 insertions(+)
--
2.20.1
^ 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