* Re: [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode
From: Heiner Kallweit @ 2019-02-15 18:57 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <20190215135714.GI5699@lunn.ch>
On 15.02.2019 14:57, Andrew Lunn wrote:
> On Thu, Feb 14, 2019 at 10:15:31PM +0100, Heiner Kallweit wrote:
>> We have the settings array of modes which is sorted based on aneg
>> priority. Instead of checking each mode manually let's simply iterate
>> over the sorted settings.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> drivers/net/phy/phy-core.c | 43 +++++++-------------------------------
>> 1 file changed, 7 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
>> index cdea028d1..5d43106fe 100644
>> --- a/drivers/net/phy/phy-core.c
>> +++ b/drivers/net/phy/phy-core.c
>> @@ -349,45 +349,16 @@ size_t phy_speeds(unsigned int *speeds, size_t size,
>> void phy_resolve_aneg_linkmode(struct phy_device *phydev)
>> {
>> __ETHTOOL_DECLARE_LINK_MODE_MASK(common);
>> + int i;
>>
>> linkmode_and(common, phydev->lp_advertising, phydev->advertising);
>>
>> - if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, common)) {
>> - phydev->speed = SPEED_10000;
>> - phydev->duplex = DUPLEX_FULL;
>> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
>> - common)) {
>> - phydev->speed = SPEED_5000;
>> - phydev->duplex = DUPLEX_FULL;
>> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
>> - common)) {
>> - phydev->speed = SPEED_2500;
>> - phydev->duplex = DUPLEX_FULL;
>> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
>> - common)) {
>> - phydev->speed = SPEED_1000;
>> - phydev->duplex = DUPLEX_FULL;
>> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
>> - common)) {
>> - phydev->speed = SPEED_1000;
>> - phydev->duplex = DUPLEX_HALF;
>> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
>> - common)) {
>> - phydev->speed = SPEED_100;
>> - phydev->duplex = DUPLEX_FULL;
>> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
>> - common)) {
>> - phydev->speed = SPEED_100;
>> - phydev->duplex = DUPLEX_HALF;
>> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
>> - common)) {
>> - phydev->speed = SPEED_10;
>> - phydev->duplex = DUPLEX_FULL;
>> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
>> - common)) {
>> - phydev->speed = SPEED_10;
>> - phydev->duplex = DUPLEX_HALF;
>> - }
>> + for (i = 0; i < ARRAY_SIZE(settings); i++)
>> + if (test_bit(settings[i].bit, common)) {
>> + phydev->speed = settings[i].speed;
>> + phydev->duplex = settings[i].duplex;
>> + break;
>> + }
>
> Hi Heiner
>
> Nice simplification.
>
> I just have one thought about this. The original code was limited to
> baseT. The new code could in theory return a non BaseT speed. For that
> to happen, it would require that phydev->lp_advertising and
> phydev->advertising contain a non BaseT link mode? Is that possible?
> I don't think it is.
>
Currently we set only BaseT modes because that's what the clause 45
standard registers offer. However drivers may come that use vendor
registers for e.g. backplane auto-negotiation (IIRC clause 73).
Now it's even better because this function shouldn't be (and doesn't
have to be) limited to a specific physical link type.
> Andrew
>
Heiner
^ permalink raw reply
* Re: [PATCH v2 perf,bpf 08/11] perf, bpf: save btf information as headers to perf.data
From: Song Liu @ 2019-02-15 18:59 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Stephane Eranian, Netdev, linux-kernel,
Alexei Starovoitov, daniel@iogearbox.net, Kernel Team,
peterz@infradead.org, namhyung@kernel.org
In-Reply-To: <20190215182047.GL31177@kernel.org>
> On Feb 15, 2019, at 10:20 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> Em Fri, Feb 15, 2019 at 05:47:58PM +0000, Song Liu escreveu:
>>
>>
>>> On Feb 15, 2019, at 9:40 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>>>
>>> Em Fri, Feb 15, 2019 at 05:25:01PM +0000, Song Liu escreveu:
>>>>> On Feb 15, 2019, at 6:26 AM, Arnaldo Carvalho de Melo <acme@redhat.com> wrote:
>>>>> Em Thu, Feb 14, 2019 at 04:00:09PM -0800, Song Liu escreveu:
>>>>>> This patch enables perf-record to save btf information as headers to
>>>>>> perf.data A new header type HEADER_BTF is introduced for this data.
>>>
>>>>> Wouldn't it be better for this HEADER_BTF to be introduced
>>>>> already as an user space event, Song, see:
>>>
>>>>> tools/perf/util/event.h
>>>
>>>>> and:
>>>
>>>>> tools/perf/util/event.c
>>>
>>>>> perf_event__synthesize_cpu_map()
>>>
>>>> BTF would be short living for short living BPF programs. I guess
>>>> saving them as header is easier than merging them with samples.
>>>
>>>> What's the benefit of saving them as user space events?
>>>
>>> When we work with pipe mode, i.e.:
>>>
>>> perf record -o - | perf report -i -
>>>
>>> and other combinations (with 'perf script', 'perf inject', etc), we need
>>> a way to pass the headers to the other side, and the way was via user
>>> space events.
>>>
>>> This is something Stephane and Jiri have been discussing recently,
>>> probably they have more justifications, Stephane, Jiri?
>>>
>>> - Arnaldo
>>
>> I see. In this case, we will need some synchronization between main
>> thread and the polling thread, as they are both writing to the same
>> pipe.
>
> So, the whole context is that we need to have 'perf record' to start a
> thread per CPU and then read the already per-cpu mmap buffers in the
> matching thread, with the right affinity, numa settings to have the
> record phase not cause contention, etc, so it ends up dumping one stream
> per CPU in a separate file in a 'perf.data' directory instead of a
> perf.data file.
>
> Jiri is working on that, so, if you dump one more stream into that
> directory, it would, at post processing time, be ordered together with
> the other stream, the per-cpu ones.
>
> - Arnaldo
I see. This solution looks great.
For this set, how about I keep this part as-is (at least for v3)? In
this case, it will goes to the header file after Jiri's change. Once
Jiri's work is done, I will move them into per-cpu files.
Thanks,
Song
^ permalink raw reply
* Re: [PATCH] NETWORKING: avoid use IPCB in cipso_v4_error
From: Paul Moore @ 2019-02-15 19:02 UTC (permalink / raw)
To: David Miller; +Cc: s-nazarov, netdev, linux-security-module, kuznet, yoshfuji
In-Reply-To: <20190214.084343.1138362153341500718.davem@davemloft.net>
On Thu, Feb 14, 2019 at 11:43 AM David Miller <davem@davemloft.net> wrote:
> From: Nazarov Sergey <s-nazarov@yandex.ru>
> Date: Tue, 12 Feb 2019 18:10:03 +0300
>
> > Since cipso_v4_error might be called from different network stack layers, we can't safely use icmp_send there.
> > icmp_send copies IP options with ip_option_echo, which uses IPCB to take access to IP header compiled data.
> > But after commit 971f10ec ("tcp: better TCP_SKB_CB layout to reduce cache line misses"), IPCB can't be used
> > above IP layer.
> > This patch fixes the problem by creating in cipso_v4_error a local copy of compiled IP options and using it with
> > introduced __icmp_send function. This looks some overloaded, but in quite rare error conditions only.
> >
> > The original discussion is here:
> > https://lore.kernel.org/linux-security-module/16659801547571984@sas1-890ba5c2334a.qloud-c.yandex.net/
> >
> > Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>
>
> This problem is not unique to Cipso, net/atm/clip.c's error handler
> has the same exact issue.
>
> I didn't scan more of the tree, there are probably a couple more
> locations as well.
David, are you happy with Sergey's solution as a fix for this?
If so, would you prefer a respin of this patch to apply the to the
other broken callers (e.g. net/atm/clip.c), or would you rather merge
this patch and deal with the other callers in separate patches?
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: Three questions about busy poll
From: Cong Wang @ 2019-02-15 19:04 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Alexander Duyck, Eric Dumazet, sridhar.samudrala,
Linux Kernel Network Developers
In-Reply-To: <CA+FuTSdEhZ3GdODpvdkw8_icnTrawf45WNNtohHCkje4TpqD4g@mail.gmail.com>
On Thu, Feb 14, 2019 at 4:39 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Thu, Feb 14, 2019 at 3:15 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> >
> > Hello,
> >
> > While looking into the busy polling in Linux kernel, three questions
> > come into my mind:
> >
> > 1. In the document[1], it claims sysctl.net.busy_poll depends on
> > either SO_BUSY_POLL or sysctl.net.busy_read. However, from the code in
> > ep_set_busy_poll_napi_id(), I don't see such a dependency. It simply
> > checks sysctl_net_busy_poll and sk->sk_napi_id, but sk->sk_napi_id is
> > always set as long as we enable CONFIG_NET_RX_BUSY_POLL. So what I am
> > missing here?
>
> That documentation refers to sock_poll. This does call sk_busy_loop
> individually on each socket in the pollset and thus respects those values.
> Epoll was added later, after both sock_poll and that documentation.
Ah, yeah, this explains my confusion. I thought busy_poll refers to
all polling related syscalls, that is select()/poll()/epoll(), it looks like
epoll() is so special here. Probably we need some clarification in
net.txt.
>
> > 2. Why there is no socket option for sysctl.net.busy_poll? Clearly
> > sysctl_net_busy_poll is global and SO_BUSY_POLL only works for
> > sysctl.net.busy_read.
>
> I guess because of how sock_poll works. In that case it is not needed.
> The poll duration applies more to the pollset than any of the
> individual sockets, too.
Good point, it's probably like struct eventpoll vs. struct epitem.
The reason why I am looking for a per-socket tuning is to minimize
the impact of setting busy_poll. I don't know if it is possible to somehow
make this per-socket via epoll interfaces, perhaps fundamentally
it is impossible?
>
> > 3. How is SO_INCOMING_NAPI_ID supposed to be used? I can't find any
> > useful documents online. Any example or more detailed doc?
>
> From the commit message of 6d4339028b35 ("net: Introduce
> SO_INCOMING_NAPI_ID") it sounds like a sharding mechanism that
> maintains flow affinity by sharding based on rxqueue (assuming that
> something like RSS was used to ensure flow affinity in the first
> place).
That commit message is the only thing I can find too. I kinda
need a formal documentation in man page and hopefully
an example too.
Thanks for your explanations!
^ permalink raw reply
* Re: Three questions about busy poll
From: Cong Wang @ 2019-02-15 19:18 UTC (permalink / raw)
To: Samudrala, Sridhar
Cc: Alexander Duyck, Eric Dumazet, Linux Kernel Network Developers
In-Reply-To: <bfaff1c6-1269-1c33-31e1-0a78f78d7214@intel.com>
On Thu, Feb 14, 2019 at 4:43 PM Samudrala, Sridhar
<sridhar.samudrala@intel.com> wrote:
>
>
> On 2/14/2019 12:15 PM, Cong Wang wrote:
>
> Hello,
>
> While looking into the busy polling in Linux kernel, three questions
> come into my mind:
>
> 1. In the document[1], it claims sysctl.net.busy_poll depends on
> either SO_BUSY_POLL or sysctl.net.busy_read. However, from the code in
> ep_set_busy_poll_napi_id(), I don't see such a dependency. It simply
> checks sysctl_net_busy_poll and sk->sk_napi_id, but sk->sk_napi_id is
> always set as long as we enable CONFIG_NET_RX_BUSY_POLL. So what I am
> missing here?
>
> epoll based busypoll is only based on global sysctl_net_busy_poll.
> busy_poll value is used with poll()/select()/epoll and and busy_read is used
> with socket recvmsg
Right, I was confused by what the term "poll" refers to.
>
> 2. Why there is no socket option for sysctl.net.busy_poll? Clearly
> sysctl_net_busy_poll is global and SO_BUSY_POLL only works for
> sysctl.net.busy_read.
>
>
> Not sure if it is useful to make it a per socket option. I think it could
> be a per poll/epoll fd option
It is useful, however, from Willem's reply it looks like hard to push it
down from epoll() interface to each socket.
>
> 3. How is SO_INCOMING_NAPI_ID supposed to be used? I can't find any
> useful documents online. Any example or more detailed doc?
>
>
> A app can create one worker thread per device queue and a worker thread
> for an incoming connection can be selected based on SO_INCOMING_NAPI_ID so that
> all connections coming on a queue are processed by the same thread. This will
> allow epoll from a thread to be associated with sockets that receive packets
> from a single queue allowing busy polling.
>
This information is very useful. It also requires each thread pinning to each
CPU/RX queue, right?
Anyway, I will add this information to socket.7 man page.
Thanks!
^ permalink raw reply
* Re: [PATCH net] net: ip6_gre: initialize erspan_ver just for erspan tunnels
From: Gregory Rose @ 2019-02-15 19:58 UTC (permalink / raw)
To: Lorenzo Bianconi, davem; +Cc: netdev, petrm
In-Reply-To: <a08d2d1dbe3a498244867308adf37259bed30f37.1550239457.git.lorenzo.bianconi@redhat.com>
On 2/15/2019 6:10 AM, Lorenzo Bianconi wrote:
> After commit c706863bc890 ("net: ip6_gre: always reports o_key to
> userspace"), ip6gre and ip6gretap tunnels started reporting TUNNEL_KEY
> output flag even if it is not configured.
> ip6gre_fill_info checks erspan_ver value to add TUNNEL_KEY for
> erspan tunnels, however in commit 84581bdae9587 ("erspan: set
> erspan_ver to 1 by default when adding an erspan dev")
> erspan_ver is initialized to 1 even for ip6gre or ip6gretap
> Fix the issue moving erspan_ver initialization in a dedicated routine
>
> Fixes: c706863bc890 ("net: ip6_gre: always reports o_key to userspace")
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> ---
> net/ipv6/ip6_gre.c | 34 ++++++++++++++++++++--------------
> 1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index 801a9a0c217e..43890898b0b5 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -1719,6 +1719,24 @@ static int ip6erspan_tap_validate(struct nlattr *tb[], struct nlattr *data[],
> return 0;
> }
>
> +static void ip6erspan_set_version(struct nlattr *data[],
> + struct __ip6_tnl_parm *parms)
> +{
> + parms->erspan_ver = 1;
> + if (data[IFLA_GRE_ERSPAN_VER])
> + parms->erspan_ver = nla_get_u8(data[IFLA_GRE_ERSPAN_VER]);
> +
> + if (parms->erspan_ver == 1) {
> + if (data[IFLA_GRE_ERSPAN_INDEX])
> + parms->index = nla_get_u32(data[IFLA_GRE_ERSPAN_INDEX]);
> + } else if (parms->erspan_ver == 2) {
> + if (data[IFLA_GRE_ERSPAN_DIR])
> + parms->dir = nla_get_u8(data[IFLA_GRE_ERSPAN_DIR]);
> + if (data[IFLA_GRE_ERSPAN_HWID])
> + parms->hwid = nla_get_u16(data[IFLA_GRE_ERSPAN_HWID]);
> + }
> +}
> +
> static void ip6gre_netlink_parms(struct nlattr *data[],
> struct __ip6_tnl_parm *parms)
> {
> @@ -1767,20 +1785,6 @@ static void ip6gre_netlink_parms(struct nlattr *data[],
>
> if (data[IFLA_GRE_COLLECT_METADATA])
> parms->collect_md = true;
> -
> - parms->erspan_ver = 1;
> - if (data[IFLA_GRE_ERSPAN_VER])
> - parms->erspan_ver = nla_get_u8(data[IFLA_GRE_ERSPAN_VER]);
> -
> - if (parms->erspan_ver == 1) {
> - if (data[IFLA_GRE_ERSPAN_INDEX])
> - parms->index = nla_get_u32(data[IFLA_GRE_ERSPAN_INDEX]);
> - } else if (parms->erspan_ver == 2) {
> - if (data[IFLA_GRE_ERSPAN_DIR])
> - parms->dir = nla_get_u8(data[IFLA_GRE_ERSPAN_DIR]);
> - if (data[IFLA_GRE_ERSPAN_HWID])
> - parms->hwid = nla_get_u16(data[IFLA_GRE_ERSPAN_HWID]);
> - }
> }
>
> static int ip6gre_tap_init(struct net_device *dev)
> @@ -2203,6 +2207,7 @@ static int ip6erspan_newlink(struct net *src_net, struct net_device *dev,
> int err;
>
> ip6gre_netlink_parms(data, &nt->parms);
> + ip6erspan_set_version(data, &nt->parms);
> ign = net_generic(net, ip6gre_net_id);
>
> if (nt->parms.collect_md) {
> @@ -2248,6 +2253,7 @@ static int ip6erspan_changelink(struct net_device *dev, struct nlattr *tb[],
> if (IS_ERR(t))
> return PTR_ERR(t);
>
> + ip6erspan_set_version(data, &p);
> ip6gre_tunnel_unlink_md(ign, t);
> ip6gre_tunnel_unlink(ign, t);
> ip6erspan_tnl_change(t, &p, !tb[IFLA_MTU]);
LGTM. Thanks Lorenzo.
Reviewed-by: Greg Rose <gvrose8192@gmail.com>
^ permalink raw reply
* Re: [RESEND PATCH net] mm: page_alloc: fix ref bias in page_frag_alloc() for 1-byte allocs
From: David Miller @ 2019-02-15 19:58 UTC (permalink / raw)
To: alexander.duyck; +Cc: jannh, netdev, linux-kernel
In-Reply-To: <CAKgT0UeOGPQO0viM0w3JD71357uj7uuZ+wX2tCUTP6u_ZQG78Q@mail.gmail.com>
From: Alexander Duyck <alexander.duyck@gmail.com>
Date: Fri, 15 Feb 2019 10:35:18 -0800
> I'll take care of it. I'm kind of annoyed that you resubmitted this to
> netdev before anyone had a chance to even provide review comments
> though.
He was only following instructions from Andrew Morton.
^ permalink raw reply
* Re: [PATCH] NETWORKING: avoid use IPCB in cipso_v4_error
From: David Miller @ 2019-02-15 20:00 UTC (permalink / raw)
To: paul; +Cc: s-nazarov, netdev, linux-security-module, kuznet, yoshfuji
In-Reply-To: <CAHC9VhQK6Cmcx5kwYAPipR4YGyBHdkLEe3UH+VmNYGHu4PWqjQ@mail.gmail.com>
From: Paul Moore <paul@paul-moore.com>
Date: Fri, 15 Feb 2019 14:02:31 -0500
> On Thu, Feb 14, 2019 at 11:43 AM David Miller <davem@davemloft.net> wrote:
>> From: Nazarov Sergey <s-nazarov@yandex.ru>
>> Date: Tue, 12 Feb 2019 18:10:03 +0300
>>
>> > Since cipso_v4_error might be called from different network stack layers, we can't safely use icmp_send there.
>> > icmp_send copies IP options with ip_option_echo, which uses IPCB to take access to IP header compiled data.
>> > But after commit 971f10ec ("tcp: better TCP_SKB_CB layout to reduce cache line misses"), IPCB can't be used
>> > above IP layer.
>> > This patch fixes the problem by creating in cipso_v4_error a local copy of compiled IP options and using it with
>> > introduced __icmp_send function. This looks some overloaded, but in quite rare error conditions only.
>> >
>> > The original discussion is here:
>> > https://lore.kernel.org/linux-security-module/16659801547571984@sas1-890ba5c2334a.qloud-c.yandex.net/
>> >
>> > Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>
>>
>> This problem is not unique to Cipso, net/atm/clip.c's error handler
>> has the same exact issue.
>>
>> I didn't scan more of the tree, there are probably a couple more
>> locations as well.
>
> David, are you happy with Sergey's solution as a fix for this?
>
> If so, would you prefer a respin of this patch to apply the to the
> other broken callers (e.g. net/atm/clip.c), or would you rather merge
> this patch and deal with the other callers in separate patches?
I'd like the other broken callers to be handled.
^ permalink raw reply
* Re: [PATCH] NETWORKING: avoid use IPCB in cipso_v4_error
From: Paul Moore @ 2019-02-15 20:04 UTC (permalink / raw)
To: David Miller; +Cc: s-nazarov, netdev, linux-security-module, kuznet, yoshfuji
In-Reply-To: <20190215.120009.1549205062473501080.davem@davemloft.net>
On Fri, Feb 15, 2019 at 3:00 PM David Miller <davem@davemloft.net> wrote:
> From: Paul Moore <paul@paul-moore.com>
> Date: Fri, 15 Feb 2019 14:02:31 -0500
>
> > On Thu, Feb 14, 2019 at 11:43 AM David Miller <davem@davemloft.net> wrote:
> >> From: Nazarov Sergey <s-nazarov@yandex.ru>
> >> Date: Tue, 12 Feb 2019 18:10:03 +0300
> >>
> >> > Since cipso_v4_error might be called from different network stack layers, we can't safely use icmp_send there.
> >> > icmp_send copies IP options with ip_option_echo, which uses IPCB to take access to IP header compiled data.
> >> > But after commit 971f10ec ("tcp: better TCP_SKB_CB layout to reduce cache line misses"), IPCB can't be used
> >> > above IP layer.
> >> > This patch fixes the problem by creating in cipso_v4_error a local copy of compiled IP options and using it with
> >> > introduced __icmp_send function. This looks some overloaded, but in quite rare error conditions only.
> >> >
> >> > The original discussion is here:
> >> > https://lore.kernel.org/linux-security-module/16659801547571984@sas1-890ba5c2334a.qloud-c.yandex.net/
> >> >
> >> > Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>
> >>
> >> This problem is not unique to Cipso, net/atm/clip.c's error handler
> >> has the same exact issue.
> >>
> >> I didn't scan more of the tree, there are probably a couple more
> >> locations as well.
> >
> > David, are you happy with Sergey's solution as a fix for this?
> >
> > If so, would you prefer a respin of this patch to apply the to the
> > other broken callers (e.g. net/atm/clip.c), or would you rather merge
> > this patch and deal with the other callers in separate patches?
>
> I'd like the other broken callers to be handled.
Sergey, do you think you could fix the other callers too, or do you
want some help with that?
--
paul moore
www.paul-moore.com
^ permalink raw reply
* [PATCH][unix] missing barriers in some of unix_sock ->addr and ->path accesses
From: Al Viro @ 2019-02-15 20:09 UTC (permalink / raw)
To: netdev; +Cc: David Miller
Several u->addr and u->path users are not holding any locks in
common with unix_bind(). unix_state_lock() is useless for those
purposes.
u->addr is assign-once and *(u->addr) is fully set up by the time
we set u->addr (all under unix_table_lock). u->path is also
set in the same critical area, also before setting u->addr, and
any unix_sock with ->path filled will have non-NULL ->addr.
So setting ->addr with smp_store_release() is all we need for those
"lockless" users - just have them fetch ->addr with smp_load_acquire()
and don't even bother looking at ->path if they see NULL ->addr.
Users of ->addr and ->path fall into several classes now:
1) ones that do smp_load_acquire(u->addr) and access *(u->addr)
and u->path only if smp_load_acquire() has returned non-NULL.
2) places holding unix_table_lock. These are guaranteed that
*(u->addr) is seen fully initialized. If unix_sock is in one of the
"bound" chains, so's ->path.
3) unix_sock_destructor() using ->addr is safe. All places
that set u->addr are guaranteed to have seen all stores *(u->addr)
while holding a reference to u and unix_sock_destructor() is called
when (atomic) refcount hits zero.
4) unix_release_sock() using ->path is safe. unix_bind()
is serialized wrt unix_release() (normally - by struct file
refcount), and for the instances that had ->path set by unix_bind()
unix_release_sock() comes from unix_release(), so they are fine.
Instances that had it set in unix_stream_connect() either end up
attached to a socket (in unix_accept()), in which case the call
chain to unix_release_sock() and serialization are the same as in
the previous case, or they never get accept'ed and unix_release_sock()
is called when the listener is shut down and its queue gets purged.
In that case the listener's queue lock provides the barriers needed -
unix_stream_connect() shoves our unix_sock into listener's queue
under that lock right after having set ->path and eventual
unix_release_sock() caller picks them from that queue under the
same lock right before calling unix_release_sock().
5) unix_find_other() use of ->path is pointless, but safe -
it happens with successful lookup by (abstract) name, so ->path.dentry
is guaranteed to be NULL there.
Cc: stable@kernel.org
earlier-variant-reviewed-by: "Paul E. McKenney" <paulmck@linux.ibm.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 74d1eed7cbd4..a95d479caeea 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -890,7 +890,7 @@ static int unix_autobind(struct socket *sock)
addr->hash ^= sk->sk_type;
__unix_remove_socket(sk);
- u->addr = addr;
+ smp_store_release(&u->addr, addr);
__unix_insert_socket(&unix_socket_table[addr->hash], sk);
spin_unlock(&unix_table_lock);
err = 0;
@@ -1060,7 +1060,7 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
err = 0;
__unix_remove_socket(sk);
- u->addr = addr;
+ smp_store_release(&u->addr, addr);
__unix_insert_socket(list, sk);
out_unlock:
@@ -1331,15 +1331,29 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
RCU_INIT_POINTER(newsk->sk_wq, &newu->peer_wq);
otheru = unix_sk(other);
- /* copy address information from listening to new sock*/
- if (otheru->addr) {
- refcount_inc(&otheru->addr->refcnt);
- newu->addr = otheru->addr;
- }
+ /* copy address information from listening to new sock
+ *
+ * The contents of *(otheru->addr) and otheru->path
+ * are seen fully set up here, since we have found
+ * otheru in hash under unix_table_lock. Insertion
+ * into the hash chain we'd found it in had been done
+ * in an earlier critical area protected by unix_table_lock,
+ * the same one where we'd set *(otheru->addr) contents,
+ * as well as otheru->path and otheru->addr itself.
+ *
+ * Using smp_store_release() here to set newu->addr
+ * is enough to make those stores, as well as stores
+ * to newu->path visible to anyone who gets newu->addr
+ * by smp_load_acquire(). IOW, the same warranties
+ * as for unix_sock instances bound in unix_bind() or
+ * in unix_autobind().
+ */
if (otheru->path.dentry) {
path_get(&otheru->path);
newu->path = otheru->path;
}
+ refcount_inc(&otheru->addr->refcnt);
+ smp_store_release(&newu->addr, otheru->addr);
/* Set credentials */
copy_peercred(sk, other);
@@ -1453,7 +1467,7 @@ static int unix_accept(struct socket *sock, struct socket *newsock, int flags,
static int unix_getname(struct socket *sock, struct sockaddr *uaddr, int peer)
{
struct sock *sk = sock->sk;
- struct unix_sock *u;
+ struct unix_address *addr;
DECLARE_SOCKADDR(struct sockaddr_un *, sunaddr, uaddr);
int err = 0;
@@ -1468,19 +1482,15 @@ static int unix_getname(struct socket *sock, struct sockaddr *uaddr, int peer)
sock_hold(sk);
}
- u = unix_sk(sk);
- unix_state_lock(sk);
- if (!u->addr) {
+ addr = smp_load_acquire(&unix_sk(sk)->addr);
+ if (!addr) {
sunaddr->sun_family = AF_UNIX;
sunaddr->sun_path[0] = 0;
err = sizeof(short);
} else {
- struct unix_address *addr = u->addr;
-
err = addr->len;
memcpy(sunaddr, addr->name, addr->len);
}
- unix_state_unlock(sk);
sock_put(sk);
out:
return err;
@@ -2073,11 +2083,11 @@ static int unix_seqpacket_recvmsg(struct socket *sock, struct msghdr *msg,
static void unix_copy_addr(struct msghdr *msg, struct sock *sk)
{
- struct unix_sock *u = unix_sk(sk);
+ struct unix_address *addr = smp_load_acquire(&unix_sk(sk)->addr);
- if (u->addr) {
- msg->msg_namelen = u->addr->len;
- memcpy(msg->msg_name, u->addr->name, u->addr->len);
+ if (addr) {
+ msg->msg_namelen = addr->len;
+ memcpy(msg->msg_name, addr->name, addr->len);
}
}
@@ -2581,15 +2591,14 @@ static int unix_open_file(struct sock *sk)
if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))
return -EPERM;
- unix_state_lock(sk);
+ if (!smp_load_acquire(&unix_sk(sk)->addr))
+ return -ENOENT;
+
path = unix_sk(sk)->path;
- if (!path.dentry) {
- unix_state_unlock(sk);
+ if (!path.dentry)
return -ENOENT;
- }
path_get(&path);
- unix_state_unlock(sk);
fd = get_unused_fd_flags(O_CLOEXEC);
if (fd < 0)
@@ -2830,7 +2839,7 @@ static int unix_seq_show(struct seq_file *seq, void *v)
(s->sk_state == TCP_ESTABLISHED ? SS_CONNECTING : SS_DISCONNECTING),
sock_i_ino(s));
- if (u->addr) {
+ if (u->addr) { // under unix_table_lock here
int i, len;
seq_putc(seq, ' ');
diff --git a/net/unix/diag.c b/net/unix/diag.c
index 384c84e83462..3183d9b8ab33 100644
--- a/net/unix/diag.c
+++ b/net/unix/diag.c
@@ -10,7 +10,8 @@
static int sk_diag_dump_name(struct sock *sk, struct sk_buff *nlskb)
{
- struct unix_address *addr = unix_sk(sk)->addr;
+ /* might or might not have unix_table_lock */
+ struct unix_address *addr = smp_load_acquire(&unix_sk(sk)->addr);
if (!addr)
return 0;
diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index f84001019356..33028c098ef3 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -321,6 +321,7 @@ static void dump_common_audit_data(struct audit_buffer *ab,
if (a->u.net->sk) {
struct sock *sk = a->u.net->sk;
struct unix_sock *u;
+ struct unix_address *addr;
int len = 0;
char *p = NULL;
@@ -351,14 +352,15 @@ static void dump_common_audit_data(struct audit_buffer *ab,
#endif
case AF_UNIX:
u = unix_sk(sk);
+ addr = smp_load_acquire(&u->addr);
+ if (!addr)
+ break;
if (u->path.dentry) {
audit_log_d_path(ab, " path=", &u->path);
break;
}
- if (!u->addr)
- break;
- len = u->addr->len-sizeof(short);
- p = &u->addr->name->sun_path[0];
+ len = addr->len-sizeof(short);
+ p = &addr->name->sun_path[0];
audit_log_format(ab, " path=");
if (*p)
audit_log_untrustedstring(ab, p);
^ permalink raw reply related
* [PATCH 2/2] MIPS: eBPF: Remove REG_32BIT_ZERO_EX
From: Paul Burton @ 2019-02-15 20:14 UTC (permalink / raw)
To: linux-mips@vger.kernel.org, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Jiong Wang, Paul Burton
In-Reply-To: <20190215201321.15725-1-paul.burton@mips.com>
REG_32BIT_ZERO_EX and REG_64BIT are always handled in exactly the same
way, and reg_val_propagate_range() never actually sets any register to
type REG_32BIT_ZERO_EX.
Remove the redundant & unused REG_32BIT_ZERO_EX.
Signed-off-by: Paul Burton <paul.burton@mips.com>
---
arch/mips/net/ebpf_jit.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/arch/mips/net/ebpf_jit.c b/arch/mips/net/ebpf_jit.c
index 715415fa2345..76e9bf88d3b9 100644
--- a/arch/mips/net/ebpf_jit.c
+++ b/arch/mips/net/ebpf_jit.c
@@ -79,8 +79,6 @@ enum reg_val_type {
REG_64BIT_32BIT,
/* 32-bit compatible, need truncation for 64-bit ops. */
REG_32BIT,
- /* 32-bit zero extended. */
- REG_32BIT_ZERO_EX,
/* 32-bit no sign/zero extension needed. */
REG_32BIT_POS
};
@@ -349,7 +347,7 @@ static int build_int_epilogue(struct jit_ctx *ctx, int dest_reg)
if (dest_reg == MIPS_R_RA) {
/* Don't let zero extended value escape. */
td = get_reg_val_type(ctx, prog->len, BPF_REG_0);
- if (td == REG_64BIT || td == REG_32BIT_ZERO_EX)
+ if (td == REG_64BIT)
emit_instr(ctx, sll, r0, r0, 0);
}
@@ -695,7 +693,7 @@ static int build_one_insn(const struct bpf_insn *insn, struct jit_ctx *ctx,
if (dst < 0)
return dst;
td = get_reg_val_type(ctx, this_idx, insn->dst_reg);
- if (td == REG_64BIT || td == REG_32BIT_ZERO_EX) {
+ if (td == REG_64BIT) {
/* sign extend */
emit_instr(ctx, sll, dst, dst, 0);
}
@@ -710,7 +708,7 @@ static int build_one_insn(const struct bpf_insn *insn, struct jit_ctx *ctx,
if (dst < 0)
return dst;
td = get_reg_val_type(ctx, this_idx, insn->dst_reg);
- if (td == REG_64BIT || td == REG_32BIT_ZERO_EX) {
+ if (td == REG_64BIT) {
/* sign extend */
emit_instr(ctx, sll, dst, dst, 0);
}
@@ -724,7 +722,7 @@ static int build_one_insn(const struct bpf_insn *insn, struct jit_ctx *ctx,
if (dst < 0)
return dst;
td = get_reg_val_type(ctx, this_idx, insn->dst_reg);
- if (td == REG_64BIT || td == REG_32BIT_ZERO_EX)
+ if (td == REG_64BIT)
/* sign extend */
emit_instr(ctx, sll, dst, dst, 0);
if (insn->imm == 1) {
@@ -863,13 +861,13 @@ static int build_one_insn(const struct bpf_insn *insn, struct jit_ctx *ctx,
if (src < 0 || dst < 0)
return -EINVAL;
td = get_reg_val_type(ctx, this_idx, insn->dst_reg);
- if (td == REG_64BIT || td == REG_32BIT_ZERO_EX) {
+ if (td == REG_64BIT) {
/* sign extend */
emit_instr(ctx, sll, dst, dst, 0);
}
did_move = false;
ts = get_reg_val_type(ctx, this_idx, insn->src_reg);
- if (ts == REG_64BIT || ts == REG_32BIT_ZERO_EX) {
+ if (ts == REG_64BIT) {
int tmp_reg = MIPS_R_AT;
if (bpf_op == BPF_MOV) {
@@ -1257,8 +1255,7 @@ static int build_one_insn(const struct bpf_insn *insn, struct jit_ctx *ctx,
if (insn->imm == 64 && td == REG_32BIT)
emit_instr(ctx, dinsu, dst, MIPS_R_ZERO, 32, 32);
- if (insn->imm != 64 &&
- (td == REG_64BIT || td == REG_32BIT_ZERO_EX)) {
+ if (insn->imm != 64 && td == REG_64BIT) {
/* sign extend */
emit_instr(ctx, sll, dst, dst, 0);
}
--
2.20.1
^ permalink raw reply related
* [PATCH 1/2] MIPS: eBPF: Always return sign extended 32b values
From: Paul Burton @ 2019-02-15 20:14 UTC (permalink / raw)
To: linux-mips@vger.kernel.org, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Jiong Wang, Paul Burton,
stable@vger.kernel.org
The function prototype used to call JITed eBPF code (ie. the type of the
struct bpf_prog bpf_func field) returns an unsigned int. The MIPS n64
ABI that MIPS64 kernels target defines that 32 bit integers should
always be sign extended when passed in registers as either arguments or
return values.
This means that when returning any value which may not already be sign
extended (ie. of type REG_64BIT or REG_32BIT_ZERO_EX) we need to perform
that sign extension in order to comply with the n64 ABI. Without this we
see strange looking test failures from test_bpf.ko, such as:
test_bpf: #65 ALU64_MOV_X:
dst = 4294967295 jited:1 ret -1 != -1 FAIL (1 times)
Although the return value printed matches the expected value, this is
only because printf is only examining the least significant 32 bits of
the 64 bit register value we returned. The register holding the expected
value is sign extended whilst the v0 register was set to a zero extended
value by our JITed code, so when compared by a conditional branch
instruction the values are not equal.
We already handle this when the return value register is of type
REG_32BIT_ZERO_EX, so simply extend this to also cover REG_64BIT.
Signed-off-by: Paul Burton <paul.burton@mips.com>
Fixes: b6bd53f9c4e8 ("MIPS: Add missing file for eBPF JIT.")
Cc: stable@vger.kernel.org # v4.13+
---
arch/mips/net/ebpf_jit.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/mips/net/ebpf_jit.c b/arch/mips/net/ebpf_jit.c
index b16710a8a9e7..715415fa2345 100644
--- a/arch/mips/net/ebpf_jit.c
+++ b/arch/mips/net/ebpf_jit.c
@@ -343,12 +343,15 @@ static int build_int_epilogue(struct jit_ctx *ctx, int dest_reg)
const struct bpf_prog *prog = ctx->skf;
int stack_adjust = ctx->stack_size;
int store_offset = stack_adjust - 8;
+ enum reg_val_type td;
int r0 = MIPS_R_V0;
- if (dest_reg == MIPS_R_RA &&
- get_reg_val_type(ctx, prog->len, BPF_REG_0) == REG_32BIT_ZERO_EX)
+ if (dest_reg == MIPS_R_RA) {
/* Don't let zero extended value escape. */
- emit_instr(ctx, sll, r0, r0, 0);
+ td = get_reg_val_type(ctx, prog->len, BPF_REG_0);
+ if (td == REG_64BIT || td == REG_32BIT_ZERO_EX)
+ emit_instr(ctx, sll, r0, r0, 0);
+ }
if (ctx->flags & EBPF_SAVE_RA) {
emit_instr(ctx, ld, MIPS_R_RA, store_offset, MIPS_R_SP);
--
2.20.1
^ permalink raw reply related
* [PATCH net 0/5] net: dsa: b53: VLAN and L2 fixes
From: Florian Fainelli @ 2019-02-15 20:16 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
Hi David,
This patch series contains a collection of fixes to the b53 driver in
order to:
- consistently program the same default VLAN ID when a port is bridged
or not
- properly account for VLAN filtering being turned on/off and turning
on ingress VID checking accordingly
- have SYSTEMPORT properly forward BPDU frames to the network stack
(which it did not)
- do not assume that WoL is supported by the DSA master network device
we are connected to
Thank you!
Florian Fainelli (5):
net: dsa: b53: Fix default VLAN ID
net: dsa: b53: Properly account for VLAN filtering
net: systemport: Fix reception of BPDUs
net: dsa: bcm_sf2: Do not assume DSA master supports WoL
net: dsa: b53: Do not program CPU port's PVID
drivers/net/dsa/b53/b53_common.c | 90 +++++++++++++++++-----
drivers/net/dsa/b53/b53_priv.h | 3 +
drivers/net/dsa/bcm_sf2.c | 10 ++-
drivers/net/ethernet/broadcom/bcmsysport.c | 4 +
4 files changed, 84 insertions(+), 23 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net 1/5] net: dsa: b53: Fix default VLAN ID
From: Florian Fainelli @ 2019-02-15 20:16 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20190215201653.20988-1-f.fainelli@gmail.com>
We were not consistent in how the default VID of a given port was
defined, b53_br_leave() would make sure the VLAN ID would be either 0/1
depending on the switch generation, but b53_configure_vlan(), which is
the default configuration would unconditionally set it to 1. The correct
value is 1 for 5325/5365 series and 0 otherwise. To avoid repeating that
mistake ever again, introduce a helper function: b53_default_pvid() to
factor that out.
Fixes: 967dd82ffc52 ("net: dsa: b53: Add support for Broadcom RoboSwitch")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/b53/b53_common.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 0e4bbdcc614f..964a9ec4652a 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -632,15 +632,25 @@ static void b53_enable_mib(struct b53_device *dev)
b53_write8(dev, B53_MGMT_PAGE, B53_GLOBAL_CONFIG, gc);
}
+static u16 b53_default_pvid(struct b53_device *dev)
+{
+ if (is5325(dev) || is5365(dev))
+ return 1;
+ else
+ return 0;
+}
+
int b53_configure_vlan(struct dsa_switch *ds)
{
struct b53_device *dev = ds->priv;
struct b53_vlan vl = { 0 };
- int i;
+ int i, def_vid;
+
+ def_vid = b53_default_pvid(dev);
/* clear all vlan entries */
if (is5325(dev) || is5365(dev)) {
- for (i = 1; i < dev->num_vlans; i++)
+ for (i = def_vid; i < dev->num_vlans; i++)
b53_set_vlan_entry(dev, i, &vl);
} else {
b53_do_vlan_op(dev, VTA_CMD_CLEAR);
@@ -650,7 +660,7 @@ int b53_configure_vlan(struct dsa_switch *ds)
b53_for_each_port(dev, i)
b53_write16(dev, B53_VLAN_PAGE,
- B53_VLAN_PORT_DEF_TAG(i), 1);
+ B53_VLAN_PORT_DEF_TAG(i), def_vid);
if (!is5325(dev) && !is5365(dev))
b53_set_jumbo(dev, dev->enable_jumbo, false);
@@ -1326,12 +1336,8 @@ int b53_vlan_del(struct dsa_switch *ds, int port,
vl->members &= ~BIT(port);
- if (pvid == vid) {
- if (is5325(dev) || is5365(dev))
- pvid = 1;
- else
- pvid = 0;
- }
+ if (pvid == vid)
+ pvid = b53_default_pvid(dev);
if (untagged && !dsa_is_cpu_port(ds, port))
vl->untag &= ~(BIT(port));
@@ -1644,10 +1650,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *br)
b53_write16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), pvlan);
dev->ports[port].vlan_ctl_mask = pvlan;
- if (is5325(dev) || is5365(dev))
- pvid = 1;
- else
- pvid = 0;
+ pvid = b53_default_pvid(dev);
/* Make this port join all VLANs without VLAN entries */
if (is58xx(dev)) {
--
2.17.1
^ permalink raw reply related
* [PATCH net 2/5] net: dsa: b53: Properly account for VLAN filtering
From: Florian Fainelli @ 2019-02-15 20:16 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20190215201653.20988-1-f.fainelli@gmail.com>
VLAN filtering can be built into the kernel, and also dynamically turned
on/off through the bridge master device. Allow re-configuring the switch
appropriately to account for that by deciding whether VLAN table
(v_table) misses should lead to a drop or forward.
Fixes: a2482d2ce349 ("net: dsa: b53: Plug in VLAN support")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/b53/b53_common.c | 59 +++++++++++++++++++++++++++++---
drivers/net/dsa/b53/b53_priv.h | 3 ++
2 files changed, 57 insertions(+), 5 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 964a9ec4652a..2fef4c564420 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -344,7 +344,8 @@ static void b53_set_forwarding(struct b53_device *dev, int enable)
b53_write8(dev, B53_CTRL_PAGE, B53_SWITCH_CTRL, mgmt);
}
-static void b53_enable_vlan(struct b53_device *dev, bool enable)
+static void b53_enable_vlan(struct b53_device *dev, bool enable,
+ bool enable_filtering)
{
u8 mgmt, vc0, vc1, vc4 = 0, vc5;
@@ -369,8 +370,13 @@ static void b53_enable_vlan(struct b53_device *dev, bool enable)
vc0 |= VC0_VLAN_EN | VC0_VID_CHK_EN | VC0_VID_HASH_VID;
vc1 |= VC1_RX_MCST_UNTAG_EN | VC1_RX_MCST_FWD_EN;
vc4 &= ~VC4_ING_VID_CHECK_MASK;
- vc4 |= VC4_ING_VID_VIO_DROP << VC4_ING_VID_CHECK_S;
- vc5 |= VC5_DROP_VTABLE_MISS;
+ if (enable_filtering) {
+ vc4 |= VC4_ING_VID_VIO_DROP << VC4_ING_VID_CHECK_S;
+ vc5 |= VC5_DROP_VTABLE_MISS;
+ } else {
+ vc4 |= VC4_ING_VID_VIO_FWD << VC4_ING_VID_CHECK_S;
+ vc5 &= ~VC5_DROP_VTABLE_MISS;
+ }
if (is5325(dev))
vc0 &= ~VC0_RESERVED_1;
@@ -420,6 +426,9 @@ static void b53_enable_vlan(struct b53_device *dev, bool enable)
}
b53_write8(dev, B53_CTRL_PAGE, B53_SWITCH_MODE, mgmt);
+
+ dev->vlan_enabled = enable;
+ dev->vlan_filtering_enabled = enable_filtering;
}
static int b53_set_jumbo(struct b53_device *dev, bool enable, bool allow_10_100)
@@ -656,7 +665,7 @@ int b53_configure_vlan(struct dsa_switch *ds)
b53_do_vlan_op(dev, VTA_CMD_CLEAR);
}
- b53_enable_vlan(dev, false);
+ b53_enable_vlan(dev, false, dev->vlan_filtering_enabled);
b53_for_each_port(dev, i)
b53_write16(dev, B53_VLAN_PAGE,
@@ -1265,6 +1274,46 @@ EXPORT_SYMBOL(b53_phylink_mac_link_up);
int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
{
+ struct b53_device *dev = ds->priv;
+ struct net_device *bridge_dev;
+ unsigned int i;
+ u16 pvid, new_pvid;
+
+ /* Handle the case were multiple bridges span the same switch device
+ * and one of them has a different setting than what is being requested
+ * which would be breaking filtering semantics for any of the other
+ * bridge devices.
+ */
+ b53_for_each_port(dev, i) {
+ bridge_dev = dsa_to_port(ds, i)->bridge_dev;
+ if (bridge_dev &&
+ bridge_dev != dsa_to_port(ds, port)->bridge_dev &&
+ br_vlan_enabled(bridge_dev) != vlan_filtering) {
+ netdev_err(bridge_dev,
+ "VLAN filtering is global to the switch!\n");
+ return -EINVAL;
+ }
+ }
+
+ b53_read16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(port), &pvid);
+ new_pvid = pvid;
+ if (dev->vlan_filtering_enabled && !vlan_filtering) {
+ /* Filtering is currently enabled, use the default PVID since
+ * the bridge does not expect tagging anymore
+ */
+ dev->ports[port].pvid = pvid;
+ new_pvid = b53_default_pvid(dev);
+ } else if (!dev->vlan_filtering_enabled && vlan_filtering) {
+ /* Filtering is currently disabled, restore the previous PVID */
+ new_pvid = dev->ports[port].pvid;
+ }
+
+ if (pvid != new_pvid)
+ b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(port),
+ new_pvid);
+
+ b53_enable_vlan(dev, dev->vlan_enabled, vlan_filtering);
+
return 0;
}
EXPORT_SYMBOL(b53_vlan_filtering);
@@ -1280,7 +1329,7 @@ int b53_vlan_prepare(struct dsa_switch *ds, int port,
if (vlan->vid_end > dev->num_vlans)
return -ERANGE;
- b53_enable_vlan(dev, true);
+ b53_enable_vlan(dev, true, dev->vlan_filtering_enabled);
return 0;
}
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index ec796482792d..4dc7ee38b258 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -91,6 +91,7 @@ enum {
struct b53_port {
u16 vlan_ctl_mask;
struct ethtool_eee eee;
+ u16 pvid;
};
struct b53_vlan {
@@ -137,6 +138,8 @@ struct b53_device {
unsigned int num_vlans;
struct b53_vlan *vlans;
+ bool vlan_enabled;
+ bool vlan_filtering_enabled;
unsigned int num_ports;
struct b53_port *ports;
};
--
2.17.1
^ permalink raw reply related
* [PATCH net 3/5] net: systemport: Fix reception of BPDUs
From: Florian Fainelli @ 2019-02-15 20:16 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20190215201653.20988-1-f.fainelli@gmail.com>
SYSTEMPORT has its RXCHK parser block that attempts to validate the
packet structures, unfortunately setting the L2 header check bit will
cause Bridge PDUs (BPDUs) to be incorrectly rejected because they look
like LLC/SNAP packets with a non-IPv4 or non-IPv6 Ethernet Type.
Fixes: 4e8aedfe78c7 ("net: systemport: Turn on offloads by default")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 28c9b0bdf2f6..bc3ac369cbe3 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -134,6 +134,10 @@ static void bcm_sysport_set_rx_csum(struct net_device *dev,
priv->rx_chk_en = !!(wanted & NETIF_F_RXCSUM);
reg = rxchk_readl(priv, RXCHK_CONTROL);
+ /* Clear L2 header checks, which would prevent BPDUs
+ * from being received.
+ */
+ reg &= ~RXCHK_L2_HDR_DIS;
if (priv->rx_chk_en)
reg |= RXCHK_EN;
else
--
2.17.1
^ permalink raw reply related
* [PATCH net 4/5] net: dsa: bcm_sf2: Do not assume DSA master supports WoL
From: Florian Fainelli @ 2019-02-15 20:16 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20190215201653.20988-1-f.fainelli@gmail.com>
We assume in the bcm_sf2 driver that the DSA master network device
supports ethtool_ops::{get,set}_wol operations, which is not a given.
Avoid de-referencing potentially non-existent function pointers and
check them as we should.
Fixes: 96e65d7f3f88 ("net: dsa: bcm_sf2: add support for Wake-on-LAN")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 17ec32b0a1cc..14138d423cf1 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -726,10 +726,11 @@ static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int port,
{
struct net_device *p = ds->ports[port].cpu_dp->master;
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
- struct ethtool_wolinfo pwol;
+ struct ethtool_wolinfo pwol = { };
/* Get the parent device WoL settings */
- p->ethtool_ops->get_wol(p, &pwol);
+ if (p->ethtool_ops->get_wol)
+ p->ethtool_ops->get_wol(p, &pwol);
/* Advertise the parent device supported settings */
wol->supported = pwol.supported;
@@ -750,9 +751,10 @@ static int bcm_sf2_sw_set_wol(struct dsa_switch *ds, int port,
struct net_device *p = ds->ports[port].cpu_dp->master;
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
s8 cpu_port = ds->ports[port].cpu_dp->index;
- struct ethtool_wolinfo pwol;
+ struct ethtool_wolinfo pwol = { };
- p->ethtool_ops->get_wol(p, &pwol);
+ if (p->ethtool_ops->get_wol)
+ p->ethtool_ops->get_wol(p, &pwol);
if (wol->wolopts & ~pwol.supported)
return -EINVAL;
--
2.17.1
^ permalink raw reply related
* [PATCH net 5/5] net: dsa: b53: Do not program CPU port's PVID
From: Florian Fainelli @ 2019-02-15 20:16 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20190215201653.20988-1-f.fainelli@gmail.com>
The CPU port is special and does not need to obey VLAN restrictions as
far as untagged traffic goes, also, having the CPU port be part of a
particular PVID is against the idea of keeping it tagged in all VLANs.
Fixes: ca8931948344 ("net: dsa: b53: Keep CPU port as tagged in all VLANs")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/b53/b53_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 2fef4c564420..c76892ac4e69 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1359,7 +1359,7 @@ void b53_vlan_add(struct dsa_switch *ds, int port,
b53_fast_age_vlan(dev, vid);
}
- if (pvid) {
+ if (pvid && !dsa_is_cpu_port(ds, port)) {
b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(port),
vlan->vid_end);
b53_fast_age_vlan(dev, vid);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH bpf-next 2/4] libbpf: Support 32-bit static data loads
From: Y Song @ 2019-02-15 20:18 UTC (permalink / raw)
To: Joe Stringer; +Cc: bpf, netdev, Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <CAOftzPjFbBHabG2SR=QrC053Y8cC+Gi1LvK19oLEvDvR6LSLkw@mail.gmail.com>
On Thu, Feb 14, 2019 at 11:16 PM Joe Stringer <joe@wand.net.nz> wrote:
>
> On Thu, 14 Feb 2019 at 21:39, Y Song <ys114321@gmail.com> wrote:
> >
> > On Mon, Feb 11, 2019 at 4:48 PM Joe Stringer <joe@wand.net.nz> wrote:
> > >
> > > Support loads of static 32-bit data when BPF writers make use of
> > > convenience macros for accessing static global data variables. A later
> > > patch in this series will demonstrate its usage in a selftest.
> > >
> > > As of LLVM-7, this technique only works with 32-bit data, as LLVM will
> > > complain if this technique is attempted with data of other sizes:
> > >
> > > LLVM ERROR: Unsupported relocation: try to compile with -O2 or above,
> > > or check your static variable usage
> >
> > A little bit clarification from compiler side.
> > The above compiler error is to prevent people use static variables since current
> > kernel/libbpf does not handle this. The compiler only warns if .bss or
> > .data section
> > has more than one definitions. The first definition always has section offset 0
> > and the compiler did not warn.
>
> Ah, interesting. I observed that warning when I attempted to define
> global variables of multiple sizes, and I thought also with sizes
> other than 32-bit. This clarifies things a bit, thanks.
>
> For the .bss my observation was that if you had a definition like:
>
> static int a = 0;
>
> Then this will be placed into .bss, hence why I looked into the
> approach from this patch for patch 3 as well.
>
> > The restriction is a little strange. To only work with 32-bit data is
> > not a right
> > statement. The following are some examples.
> >
> > The following static variable definitions will succeed:
> > static int a; /* one in .bss */
> > static long b = 2; /* one in .data */
> >
> > The following definitions will fail as both in .bss.
> > static int a;
> > static int b;
> >
> > The following definitions will fail as both in .data:
> > static char a = 2;
> > static int b = 3;
>
> Are there type restrictions or something? I've been defining multiple
There is no type restrictions.
-bash-4.4$ cat g.c
struct t {
int a;
char b;
long c;
};
static volatile struct t v;
int test() { return v.a + v.b; }
-bash-4.4$ clang -O2 -g -c -target bpf g.c
-bash-4.4$
> static uint32_t and using them per the approach in this patch series
> without hitting this compiler assertion.
-bash-4.4$ cat g1.c
static volatile int a;
static volatile int b;
int test() { return a + b; }
-bash-4.4$ clang -O2 -g -c -target bpf g1.c
fatal error: error in backend: Unsupported relocation: try to compile
with -O2 or above, or check your static variable
usage
-bash-4.4$
>
> > Using global variables can prevent compiler errors.
> > maps are defined as globals and the compiler does not
> > check whether a particular global variable is defining a map or not.
> >
> > If you just use static variable like below
> > static int a = 2;
> > without potential assignment to a, the compiler will replace variable
> > a with 2 at compile time.
> > An alternative is to define like below
> > static volatile int a = 2;
> > You can get a "load" for variable "a" in the bpf load even if there is
> > no assignment to a.
>
> I'll take a closer look at this too.
>
> > Maybe now is the time to remove the compiler assertions as
> > libbpf/kernel starts to
> > handle static variables?
>
> I don't understand why those assertions exists in this form. It
> already allows code which will not load with libbpf (ie generate any
> .data/.bss), does it help prevent unexpected situations for
> developers?
The error is introduced by the following llvm commit:
commit 39184e407cd937f2f20d3f61eec205925bae7b13
Author: Yonghong Song <yhs@fb.com>
Date: Wed Aug 22 21:21:03 2018 +0000
bpf: fix an assertion in BPFAsmBackend applyFixup()
Fix bug https://bugs.llvm.org/show_bug.cgi?id=38643
In BPFAsmBackend applyFixup(), there is an assertion for FixedValue to be 0.
This may not be true, esp. for optimiation level 0.
For example, in the above bug, for the following two
static variables:
@bpf_map_lookup_elem = internal global i8* (i8*, i8*)*
inttoptr (i64 1 to i8* (i8*, i8*)*), align 8
@bpf_map_update_elem = internal global i32 (i8*, i8*, i8*, i64)*
inttoptr (i64 2 to i32 (i8*, i8*, i8*, i64)*), align 8
The static variable @bpf_map_update_elem will have a symbol
offset of 8 and a FK_SecRel_8 with FixupValue 8 will cause
the assertion if llvm is built with -DLLVM_ENABLE_ASSERTIONS=ON.
The above relocations will not exist if the program is compiled
with optimization level -O1 and above as the compiler optimizes
those static variables away. In the below error message, -O2
is suggested as this is the common practice.
Note that FixedValue = 0 in applyFixup() does exist and is valid,
e.g., for the global variable my_map in the above bug. The bpf
loader will process them properly for map_id's before loading
the program into the kernel.
The static variables, which are not optimized away by compiler,
may have FK_SecRel_8 relocation with non-zero FixedValue.
The patch removed the offending assertion and will issue
a hard error as below if the FixedValue in applyFixup()
is not 0.
$ llc -march=bpf -filetype=obj fixup.ll
LLVM ERROR: Unsupported relocation: try to compile with -O2 or above,
or check your static variable usage
Its main purpose is to fix a behavior difference with and without
-DLLVM_ENABLE_ASSERTIONS=ON. The patch generated an error
regardless whether the compiler time assertion is turned on or not.
It does not catch all the cases e.g., only one static variable is defined,
which needs fine tuning as there are legitimate cases (e.g., in some dwarf
sessions) where the Fixup is valid with FixedValue = 0.
If you try to use more than onee static variables, the compiler will
print an error and let you know your potential issues.
The question is since we are on the path to allow static variables
in the bpf programs for later patching, we probably should remove
this compiler fatal error?
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH] e1000e: Disable runtime PM on CNP+
From: Brown, Aaron F @ 2019-02-15 20:37 UTC (permalink / raw)
To: Kai-Heng Feng, Kirsher, Jeffrey T
Cc: netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20190202174016.28539-1-kai.heng.feng@canonical.com>
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> Behalf Of Kai-Heng Feng
> Sent: Saturday, February 2, 2019 9:40 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: netdev@vger.kernel.org; Kai-Heng Feng
> <kai.heng.feng@canonical.com>; intel-wired-lan@lists.osuosl.org; linux-
> kernel@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH] e1000e: Disable runtime PM on CNP+
>
> There are some new e1000e devices can only be woken up from D3 one
> time,
> by plugging ethernet cable. Subsequent cable plugging does set PME bit
> correctly, but it still doesn't get woken up.
>
> Since e1000e connects to the root complex directly, we rely on ACPI to
> wake it up. In this case, the GPE from _PRW only works once and stops
> working after that. Though it appears to be a platform bug, e1000e
> maintainers confirmed that I219 does not support D3.
>
> So disable runtime PM on CNP+ chips. We may need to disable earlier
> generations if this bug also hit older platforms.
>
> Bugzilla: https://bugzilla.kernel.org/attachment.cgi?id=280819
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> drivers/net/ethernet/intel/e1000e/netdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply
* [MERGE HELP] cls_tcindex.c
From: David Miller @ 2019-02-15 20:41 UTC (permalink / raw)
To: netdev; +Cc: xiyou.wangcong, vladbu
I've merged net into net-next.
The worst conflict was cls_tcindex.c as Cong's fixes collided heavily
with Vlad's work.
The interim solution I used for this merge was to revert back to RCU.
Please take a look at what I did and send me followups because I am
absolutely certain that some are necessary :-)))
Thanks!
^ permalink raw reply
* [PATCH net-next 0/2] net: phy: add helpers for handling C45 10GBT AN register values
From: Heiner Kallweit @ 2019-02-15 20:56 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
Similar to the existing helpers for the Clause 22 registers add helpers
to deal with converting Clause 45 advertisement registers to / from
link mode bitmaps.
Note that these helpers are defined in linux/mdio.h, not like the
Clause 22 helpers in linux/mii.h. Reason is that the Clause 45 register
constants are defined in uapi/linux/mdio.h. And uapi/linux/mdio.h
includes linux/mii.h before defining the C45 register constants.
Heiner Kallweit (2):
net: phy: add helpers for handling C45 10GBT AN register values
net: phy: use mii_10gbt_stat_mod_linkmode_lpa_t in genphy_c45_read_lpa
drivers/net/phy/phy-c45.c | 10 +------
include/linux/mdio.h | 63 +++++++++++++++++++++++++++++++++++++++
2 files changed, 64 insertions(+), 9 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH net-next 1/2] net: phy: add helpers for handling C45 10GBT AN register values
From: Heiner Kallweit @ 2019-02-15 20:57 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <60b2e19d-e138-dfa0-fa79-19f3cbece99e@gmail.com>
Similar to the existing helpers for the Clause 22 registers add helpers
to deal with converting Clause 45 advertisement registers to / from
link mode bitmaps.
Note that these helpers are defined in linux/mdio.h, not like the
Clause 22 helpers in linux/mii.h. Reason is that the Clause 45 register
constants are defined in uapi/linux/mdio.h. And uapi/linux/mdio.h
includes linux/mii.h before defining the C45 register constants.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
include/linux/mdio.h | 63 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index bfa711416..ce0d5ddbf 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -261,6 +261,69 @@ static inline u16 ethtool_adv_to_mmd_eee_adv_t(u32 adv)
return reg;
}
+/**
+ * linkmode_adv_to_mii_10gbt_adv_t
+ * @advertising: the linkmode advertisement settings
+ *
+ * A small helper function that translates linkmode advertisement
+ * settings to phy autonegotiation advertisements for the C45
+ * 10GBASE-T AN CONTROL (7.32) register.
+ */
+static inline u32 linkmode_adv_to_mii_10gbt_adv_t(unsigned long *advertising)
+{
+ u32 result = 0;
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
+ advertising))
+ result |= MDIO_AN_10GBT_CTRL_ADV2_5G;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
+ advertising))
+ result |= MDIO_AN_10GBT_CTRL_ADV5G;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+ advertising))
+ result |= MDIO_AN_10GBT_CTRL_ADV10G;
+
+ return result;
+}
+
+/**
+ * mii_10gbt_stat_mod_linkmode_lpa_t
+ * @advertising: target the linkmode advertisement settings
+ * @adv: value of the C45 10GBASE-T AN STATUS register
+ *
+ * A small helper function that translates C45 10GBASE-T AN STATUS register bits
+ * to linkmode advertisement settings. Other bits in advertising aren't changed.
+ */
+static inline void mii_10gbt_stat_mod_linkmode_lpa_t(unsigned long *advertising,
+ u32 lpa)
+{
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
+ advertising, lpa & MDIO_AN_10GBT_STAT_LP2_5G);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
+ advertising, lpa & MDIO_AN_10GBT_STAT_LP5G);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+ advertising, lpa & MDIO_AN_10GBT_STAT_LP10G);
+}
+
+/**
+ * mii_10gbt_adv_mod_linkmode_adv_t
+ * @advertising:pointer to destination link mode.
+ * @adv: value of the C45 10GBASE-T AN CONTROL register
+ *
+ * A small helper function that translates the C45 10GBASE-T AN CONTROL
+ * register to linkmode advertisement settings. Leaves other bits unchanged.
+ */
+static inline void mii_10gbt_adv_mod_linkmode_adv_t(unsigned long *advertising,
+ u32 adv)
+{
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
+ advertising, adv & MDIO_AN_10GBT_CTRL_ADV2_5G);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
+ advertising, adv & MDIO_AN_10GBT_CTRL_ADV5G);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+ advertising, adv & MDIO_AN_10GBT_CTRL_ADV10G);
+}
+
int __mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
int __mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 2/2] net: phy: use mii_10gbt_stat_mod_linkmode_lpa_t in genphy_c45_read_lpa
From: Heiner Kallweit @ 2019-02-15 20:58 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <60b2e19d-e138-dfa0-fa79-19f3cbece99e@gmail.com>
Use mii_10gbt_stat_mod_linkmode_lpa_t() in genphy_c45_read_lpa() to
simplify the code.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy-c45.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 7af5fa81d..bef126344 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -204,15 +204,7 @@ int genphy_c45_read_lpa(struct phy_device *phydev)
if (val < 0)
return val;
- if (val & MDIO_AN_10GBT_STAT_LP2_5G)
- linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
- phydev->lp_advertising);
- if (val & MDIO_AN_10GBT_STAT_LP5G)
- linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
- phydev->lp_advertising);
- if (val & MDIO_AN_10GBT_STAT_LP10G)
- linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
- phydev->lp_advertising);
+ mii_10gbt_stat_mod_linkmode_lpa_t(phydev->lp_advertising, val);
return 0;
}
--
2.20.1
^ permalink raw reply related
* [PATCH] net: mv643xx_eth: disable clk on error path in mv643xx_eth_shared_probe()
From: Alexey Khoroshilov @ 2019-02-15 21:20 UTC (permalink / raw)
To: Sebastian Hesselbarth, David S. Miller
Cc: Alexey Khoroshilov, netdev, linux-kernel, ldv-project
If mv643xx_eth_shared_of_probe() fails, mv643xx_eth_shared_probe()
leaves clk undisabled.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/net/ethernet/marvell/mv643xx_eth.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 2f427271a793..292a668ce88e 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -2879,7 +2879,7 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
ret = mv643xx_eth_shared_of_probe(pdev);
if (ret)
- return ret;
+ goto err_put_clk;
pd = dev_get_platdata(&pdev->dev);
msp->tx_csum_limit = (pd != NULL && pd->tx_csum_limit) ?
@@ -2887,6 +2887,11 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
infer_hw_params(msp);
return 0;
+
+err_put_clk:
+ if (!IS_ERR(msp->clk))
+ clk_disable_unprepare(msp->clk);
+ return ret;
}
static int mv643xx_eth_shared_remove(struct platform_device *pdev)
--
2.7.4
^ permalink raw reply related
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