* Re: kernel/bpf/core.o: warning: objtool: ___bpf_prog_run.cold()+0x7: call without frame pointer save/setup
From: Josh Poimboeuf @ 2019-08-27 14:58 UTC (permalink / raw)
To: He Zhe
Cc: ast, daniel, kafai, songliubraving, yhs, ndesaulniers,
miguel.ojeda.sandonis, luc.vanoostenryck, schwidefsky, gregkh,
mst, gor, andreyknvl, liuxiaozhou, yamada.masahiro, linux-kernel,
netdev, bpf
In-Reply-To: <2c416fe7-f6be-440b-b476-9fede1ea123c@windriver.com>
On Tue, Aug 27, 2019 at 10:43:27AM +0800, He Zhe wrote:
>
>
> On 8/26/19 11:18 PM, Josh Poimboeuf wrote:
> > On Mon, Aug 26, 2019 at 10:42:53PM +0800, He Zhe wrote:
> >> Hi All,
> >>
> >> Since 3193c0836f20 ("bpf: Disable GCC -fgcse optimization for ___bpf_prog_run()"),
> >> We have got the following warning,
> >> kernel/bpf/core.o: warning: objtool: ___bpf_prog_run.cold()+0x7: call without frame pointer save/setup
> >>
> >> If reverting the above commit, we will get the following warning,
> >> kernel/bpf/core.o: warning: objtool: ___bpf_prog_run()+0x8b9: sibling call from callable instruction with modified stack frame
> >> if CONFIG_RETPOLINE=n, and no warning if CONFIG_RETPOLINE=y
> > Can you please share the following:
> >
> > - core.o file
>
> Attached.
>
> >
> > The following would also be helpful for me to try to recreate it:
> >
> > - config file
> > - compiler version
> > - kernel version
>
> I pasted them in the other reply.
Thanks. I was able to recreate. I reduced it to:
void a(b);
__attribute__((optimize(""))) c(void) { a(); }
Apparently '__attribute__((optimize()))' is overwriting GCC cmdline
flags, including -fno-omit-frame-pointer. I had assumed it would append
instead of replace.
I'm guessing this is a GCC "feature" instead of a bug. I'll need to
follow up.
--
Josh
^ permalink raw reply
* Re: [PATCH v1 net-next 4/4] net: stmmac: setup higher frequency clk support for EHL & TGL
From: Florian Fainelli @ 2019-08-27 15:08 UTC (permalink / raw)
To: Voon, Weifeng, Andrew Lunn
Cc: David S. Miller, Maxime Coquelin, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Jose Abreu, Giuseppe Cavallaro,
Alexandre Torgue, Ong, Boon Leong
In-Reply-To: <D6759987A7968C4889FDA6FA91D5CBC814758DB5@PGSMSX103.gar.corp.intel.com>
On 8/27/2019 3:38 AM, Voon, Weifeng wrote:
>>>> +#include <linux/clk-provider.h>
>>>> #include <linux/pci.h>
>>>> #include <linux/dmi.h>
>>>>
>>>> @@ -174,6 +175,19 @@ static int intel_mgbe_common_data(struct
>> pci_dev *pdev,
>>>> plat->axi->axi_blen[1] = 8;
>>>> plat->axi->axi_blen[2] = 16;
>>>>
>>>> + plat->ptp_max_adj = plat->clk_ptp_rate;
>>>> +
>>>> + /* Set system clock */
>>>> + plat->stmmac_clk = clk_register_fixed_rate(&pdev->dev,
>>>> + "stmmac-clk", NULL, 0,
>>>> + plat->clk_ptp_rate);
>>>> +
>>>> + if (IS_ERR(plat->stmmac_clk)) {
>>>> + dev_warn(&pdev->dev, "Fail to register stmmac-clk\n");
>>>> + plat->stmmac_clk = NULL;
>>>
>>> Don't you need to propagate at least EPROBE_DEFER here?
>>
>> Hi Florian
>>
>> Isn't a fixed rate clock a complete fake. There is no hardware behind it.
>> So can it return EPROBE_DEFER?
>>
>> Andrew
>
> Yes, there is no hardware behind it. So, I don't think we need to deferred probe
> and a warning message should be sufficient. Anyhow, please point it out if I miss
> out anything.
Looks good to me, thanks both for clarifying.
--
Florian
^ permalink raw reply
* Re: BUG_ON in skb_segment, after bpf_skb_change_proto was applied
From: Eric Dumazet @ 2019-08-27 15:09 UTC (permalink / raw)
To: Shmulik Ladkani
Cc: Daniel Borkmann, netdev, Alexander Duyck, Alexei Starovoitov,
Yonghong Song, Steffen Klassert, shmulik, eyal
In-Reply-To: <20190827144218.5b098eac@pixies>
On 8/27/19 1:42 PM, Shmulik Ladkani wrote:
> On Mon, 26 Aug 2019 19:47:40 +0200
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> On 8/26/19 4:07 PM, Shmulik Ladkani wrote:
>>> - ipv4 forwarding to dummy1, where eBPF nat4-to-6 program is attached
>>> at TC Egress (calls 'bpf_skb_change_proto()'), then redirect to ingress
>>> on same device.
>>> NOTE: 'bpf_skb_proto_4_to_6()' mangles 'shinfo->gso_size'
>>
>> Doing this on an skb with a frag_list is doomed, in current gso_segment() state.
>>
>> A rewrite would be needed (I believe I did so at some point, but Herbert Xu fought hard against it)
>
> Thanks Eric,
>
> - If a rewrite is still considered out of the question, how can one use
> eBPF's bpf_skb_change_proto() safely without disabling GRO completely?
> - e.g. is there a way to force the GROed skbs to fit into a layout that is
> tolerated by skb_segment?
> - alternatively can eBPF layer somehow enforce segmentation of the
> original GROed skb before mangling the gso_size?
>
> - Another thing that puzzles me is that we hit the BUG_ON rather rarely
> and cannot yet reproduce synthetically. If skb_segment's handling of
> skbs with a frag_list (that have gso_size mangled) is broken, I'd expect
> to hit this more often... Any ideas?
skb_segment of a gro packet (especially with frag_list) is only supported
if the geometry of the individual segments is not changed,
meaning that gso_size must remain the same.
>
> - Suppose going for a rewrite, care to elaborate what's exactly missing
> in skb_segment's logic?
> I must admit I do not fully understand all the different code flows in
> this function, it seems to support many different input skbs - any
> assistance is highly appreciated.
Well, this is the point really.
The complexity of this function is so high that very few of us dare to touch it.
^ permalink raw reply
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: Roopa Prabhu @ 2019-08-27 15:14 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Miller, Jakub Kicinski, David Ahern, netdev,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
In-Reply-To: <20190827093525.GB2250@nanopsycho>
On Tue, Aug 27, 2019 at 2:35 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Tue, Aug 27, 2019 at 10:22:42AM CEST, davem@davemloft.net wrote:
> >From: Jiri Pirko <jiri@resnulli.us>
> >Date: Tue, 27 Aug 2019 09:08:08 +0200
> >
> >> Okay, so if I understand correctly, on top of separate commands for
> >> add/del of alternative names, you suggest also get/dump to be separate
> >> command and don't fill this up in existing newling/getlink command.
> >
> >I'm not sure what to do yet.
> >
> >David has a point, because the only way these ifnames are useful is
> >as ways to specify and choose net devices. So based upon that I'm
> >slightly learning towards not using separate commands.
>
> Well yeah, one can use it to handle existing commands instead of
> IFLA_NAME.
>
> But why does it rule out separate commands? I think it is cleaner than
> to put everything in poor setlink messages :/ The fact that we would
> need to add "OP" to the setlink message just feels of. Other similar
> needs may show up in the future and we may endup in ridiculous messages
> like:
>
> SETLINK
> IFLA_NAME eth0
> IFLA_ATLNAME_LIST (nest)
> IFLA_ALTNAME_OP add
> IFLA_ALTNAME somereallylongname
> IFLA_ALTNAME_OP del
> IFLA_ALTNAME somereallyreallylongname
> IFLA_ALTNAME_OP add
> IFLA_ALTNAME someotherreallylongname
> IFLA_SOMETHING_ELSE_LIST (nest)
> IFLA_SOMETHING_ELSE_OP add
> ...
> IFLA_SOMETHING_ELSE_OP del
> ...
> IFLA_SOMETHING_ELSE_OP add
> ...
>
> I don't know what to think about it. Rollbacks are going to be pure hell :/
I don't see a huge problem with the above. We need a way to solve this
anyways for other list types in the future correct ?.
The approach taken by this series will not scale if we have to add a
new msg type and header for every such list attribute in the future.
A good parallel here is bridge vlan which uses RTM_SETLINK and
RTM_DELLINK for vlan add and deletes. But it does have an advantage of
a separate
msg space under AF_BRIDGE which makes it cleaner. Maybe something
closer to that can be made to work (possibly with a msg flag) ?.
Would be good to have a consistent way to update list attributes for
future needs too.
^ permalink raw reply
* Re: [PATCH] ipv6: Not to probe neighbourless routes
From: Eric Dumazet @ 2019-08-27 15:22 UTC (permalink / raw)
To: Yi Wang, davem
Cc: kuznet, yoshfuji, netdev, linux-kernel, xue.zhihong, wang.liang82,
Cheng Lin
In-Reply-To: <1566896907-5121-1-git-send-email-wang.yi59@zte.com.cn>
On 8/27/19 11:08 AM, Yi Wang wrote:
> From: Cheng Lin <cheng.lin130@zte.com.cn>
>
> Originally, Router Reachability Probing require a neighbour entry
> existed. Commit 2152caea7196 ("ipv6: Do not depend on rt->n in
> rt6_probe().") removed the requirement for a neighbour entry. And
> commit f547fac624be ("ipv6: rate-limit probes for neighbourless
> routes") adds rate-limiting for neighbourless routes.
>
> And, the Neighbor Discovery for IP version 6 (IPv6)(rfc4861) says,
> "
> 7.2.5. Receipt of Neighbor Advertisements
>
> When a valid Neighbor Advertisement is received (either solicited or
> unsolicited), the Neighbor Cache is searched for the target's entry.
> If no entry exists, the advertisement SHOULD be silently discarded.
> There is no need to create an entry if none exists, since the
> recipient has apparently not initiated any communication with the
> target.
> ".
>
> In rt6_probe(), just a Neighbor Solicitation message are transmited.
> When receiving a Neighbor Advertisement, the node does nothing in a
> Neighborless condition.
>
> Not sure it's needed to create a neighbor entry in Router
> Reachability Probing. And the Original way may be the right way.
>
> This patch recover the requirement for a neighbour entry.
>
> Signed-off-by: Cheng Lin <cheng.lin130@zte.com.cn>
> ---
> include/net/ip6_fib.h | 5 -----
> net/ipv6/route.c | 5 +----
> 2 files changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
> index 4b5656c..8c2e022 100644
> --- a/include/net/ip6_fib.h
> +++ b/include/net/ip6_fib.h
> @@ -124,11 +124,6 @@ struct rt6_exception {
>
> struct fib6_nh {
> struct fib_nh_common nh_common;
> -
> -#ifdef CONFIG_IPV6_ROUTER_PREF
> - unsigned long last_probe;
> -#endif
> -
> struct rt6_info * __percpu *rt6i_pcpu;
> struct rt6_exception_bucket __rcu *rt6i_exception_bucket;
> };
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index fd059e0..c4bcffc 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -639,12 +639,12 @@ static void rt6_probe(struct fib6_nh *fib6_nh)
> nh_gw = &fib6_nh->fib_nh_gw6;
> dev = fib6_nh->fib_nh_dev;
> rcu_read_lock_bh();
> - idev = __in6_dev_get(dev);
> neigh = __ipv6_neigh_lookup_noref(dev, nh_gw);
> if (neigh) {
> if (neigh->nud_state & NUD_VALID)
> goto out;
>
> + idev = __in6_dev_get(dev);
> write_lock(&neigh->lock);
> if (!(neigh->nud_state & NUD_VALID) &&
> time_after(jiffies,
> @@ -654,9 +654,6 @@ static void rt6_probe(struct fib6_nh *fib6_nh)
> __neigh_set_probe_once(neigh);
> }
> write_unlock(&neigh->lock);
> - } else if (time_after(jiffies, fib6_nh->last_probe +
> - idev->cnf.rtr_probe_interval)) {
> - work = kmalloc(sizeof(*work), GFP_ATOMIC);
> }
>
> if (work) {
>
Have you really compiled this patch ?
^ permalink raw reply
* RE: [PATCH v1 net-next] net: phy: mdio_bus: make mdiobus_scan also cover PHY that only talks C45
From: Voon, Weifeng @ 2019-08-27 15:23 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli
Cc: David S. Miller, Maxime Coquelin, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Jose Abreu, Heiner Kallweit,
Ong, Boon Leong
In-Reply-To: <20190826185418.GG2168@lunn.ch>
> > > Make mdiobus_scan() to try harder to look for any PHY that only
> talks C45.
> > If you are not using Device Tree or ACPI, and you are letting the MDIO
> > bus be scanned, it sounds like there should be a way for you to
> > provide a hint as to which addresses should be scanned (that's
> > mii_bus::phy_mask) and possibly enhance that with a mask of possible
> > C45 devices?
>
> Yes, i don't like this unconditional c45 scanning. A lot of MDIO bus
> drivers don't look for the MII_ADDR_C45. They are going to do a C22
> transfer, and maybe not mask out the MII_ADDR_C45 from reg, causing an
> invalid register write. Bad things can then happen.
>
> With DT and ACPI, we have an explicit indication that C45 should be used,
> so we know on this platform C45 is safe to use. We need something
> similar when not using DT or ACPI.
>
> Andrew
Florian and Andrew,
The mdio c22 is using the start-of-frame ST=01 while mdio c45 is using ST=00
as identifier. So mdio c22 device will not response to mdio c45 protocol.
As in IEEE 802.1ae-2002 Annex 45A.3 mention that:
" Even though the Clause 45 MDIO frames using the ST=00 frame code
will also be driven on to the Clause 22 MII Management interface,
the Clause 22 PHYs will ignore the frames. "
Hence, I am not seeing any concern that the c45 scanning will mess up with
c22 devices.
Weifeng
^ permalink raw reply
* Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
From: Alex Williamson @ 2019-08-27 15:23 UTC (permalink / raw)
To: Parav Pandit
Cc: Mark Bloch, Jiri Pirko, kwankhede@nvidia.com, cohuck@redhat.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB4866BB4736D265EF28280014D1A00@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Tue, 27 Aug 2019 04:28:37 +0000
Parav Pandit <parav@mellanox.com> wrote:
> Hi Mark,
>
> > -----Original Message-----
> > From: Mark Bloch <markb@mellanox.com>
> > Sent: Tuesday, August 27, 2019 4:32 AM
> > To: Parav Pandit <parav@mellanox.com>; alex.williamson@redhat.com; Jiri
> > Pirko <jiri@mellanox.com>; kwankhede@nvidia.com; cohuck@redhat.com;
> > davem@davemloft.net
> > Cc: kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
> > netdev@vger.kernel.org
> > Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
> >
> >
> >
> > On 8/26/19 1:41 PM, Parav Pandit wrote:
> > > Mdev alias should be unique among all the mdevs, so that when such
> > > alias is used by the mdev users to derive other objects, there is no
> > > collision in a given system.
> > >
> > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > ---
> > > drivers/vfio/mdev/mdev_core.c | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/vfio/mdev/mdev_core.c
> > > b/drivers/vfio/mdev/mdev_core.c index e825ff38b037..6eb37f0c6369
> > > 100644
> > > --- a/drivers/vfio/mdev/mdev_core.c
> > > +++ b/drivers/vfio/mdev/mdev_core.c
> > > @@ -375,6 +375,11 @@ int mdev_device_create(struct kobject *kobj,
> > struct device *dev,
> > > ret = -EEXIST;
> > > goto mdev_fail;
> > > }
> > > + if (tmp->alias && strcmp(tmp->alias, alias) == 0) {
> >
> > alias can be NULL here no?
> >
> If alias is NULL, tmp->alias would also be null because for given parent either we have alias or we don’t.
> So its not possible to have tmp->alias as null and alias as non null.
> But it may be good/defensive to add check for both.
mdev_list is a global list of all mdev devices, how can we make any
assumptions that an element has the same parent? Thanks,
Alex
> > > + mutex_unlock(&mdev_list_lock);
> > > + ret = -EEXIST;
> > > + goto mdev_fail;
> > > + }
> > > }
> > >
> > > mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
> > >
> >
> > Mark
^ permalink raw reply
* Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
From: Alex Williamson @ 2019-08-27 15:28 UTC (permalink / raw)
To: Cornelia Huck
Cc: Parav Pandit, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190827132946.0b92d259.cohuck@redhat.com>
On Tue, 27 Aug 2019 13:29:46 +0200
Cornelia Huck <cohuck@redhat.com> wrote:
> On Tue, 27 Aug 2019 11:08:59 +0000
> Parav Pandit <parav@mellanox.com> wrote:
>
> > > -----Original Message-----
> > > From: Cornelia Huck <cohuck@redhat.com>
> > > Sent: Tuesday, August 27, 2019 3:59 PM
> > > To: Parav Pandit <parav@mellanox.com>
> > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > > kernel@vger.kernel.org; netdev@vger.kernel.org
> > > Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
> > >
> > > On Mon, 26 Aug 2019 15:41:17 -0500
> > > Parav Pandit <parav@mellanox.com> wrote:
> > >
> > > > Mdev alias should be unique among all the mdevs, so that when such
> > > > alias is used by the mdev users to derive other objects, there is no
> > > > collision in a given system.
> > > >
> > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > > ---
> > > > drivers/vfio/mdev/mdev_core.c | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/vfio/mdev/mdev_core.c
> > > > b/drivers/vfio/mdev/mdev_core.c index e825ff38b037..6eb37f0c6369
> > > > 100644
> > > > --- a/drivers/vfio/mdev/mdev_core.c
> > > > +++ b/drivers/vfio/mdev/mdev_core.c
> > > > @@ -375,6 +375,11 @@ int mdev_device_create(struct kobject *kobj, struct
> > > device *dev,
> > > > ret = -EEXIST;
> > > > goto mdev_fail;
> > > > }
> > > > + if (tmp->alias && strcmp(tmp->alias, alias) == 0) {
> > >
> > > Any way we can relay to the caller that the uuid was fine, but that we had a
> > > hash collision? Duplicate uuids are much more obvious than a collision here.
> > >
> > How do you want to relay this rare event?
> > Netlink interface has way to return the error message back, but sysfs is limited due to its error code based interface.
>
> I don't know, that's why I asked :)
>
> The problem is that "uuid already used" and "hash collision" are
> indistinguishable. While "use a different uuid" will probably work in
> both cases, "increase alias length" might be a good alternative in some
> cases.
>
> But if there is no good way to relay the problem, we can live with it.
It's a rare event, maybe just dev_dbg(dev, "Hash collision creating alias \"%s\" for mdev device %pUl\n",...
Thanks,
Alex
> > > > + mutex_unlock(&mdev_list_lock);
> > > > + ret = -EEXIST;
> > > > + goto mdev_fail;
> > > > + }
> > > > }
> > > >
> > > > mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
> >
>
^ permalink raw reply
* Re: [net-next] net: sched: pie: enable timestamp based delay calculation
From: Eric Dumazet @ 2019-08-27 15:34 UTC (permalink / raw)
To: Gautam Ramakrishnan, netdev
Cc: jhs, davem, xiyou.wangcong, Leslie Monis, Mohit P . Tahiliani,
Dave Taht
In-Reply-To: <20190827141938.23483-1-gautamramk@gmail.com>
On 8/27/19 4:19 PM, Gautam Ramakrishnan wrote:
> RFC 8033 suggests an alternative approach to calculate the queue
> delay in PIE by using per packet timestamps. This patch enables the
> PIE implementation to do this.
>
> The calculation of queue delay is as follows:
> qdelay = now - packet_enqueue_time
>
> To enable the use of timestamps:
> modprobe sch_pie use_timestamps=1
No module parameter is accepted these days.
Please add a new attribute instead,
so that pie can be used in both mode on the same host.
For a typical example of attribute addition, please take
a look at commit 48872c11b77271ef9b070bdc50afe6655c4eb9aa
("net_sched: sch_fq: add dctcp-like marking")
^ permalink raw reply
* Re: [PATCH v1 net-next] net: phy: mdio_bus: make mdiobus_scan also cover PHY that only talks C45
From: Florian Fainelli @ 2019-08-27 15:37 UTC (permalink / raw)
To: Voon, Weifeng, Andrew Lunn
Cc: David S. Miller, Maxime Coquelin, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Jose Abreu, Heiner Kallweit,
Ong, Boon Leong
In-Reply-To: <D6759987A7968C4889FDA6FA91D5CBC814758ED8@PGSMSX103.gar.corp.intel.com>
On 8/27/2019 8:23 AM, Voon, Weifeng wrote:
>>>> Make mdiobus_scan() to try harder to look for any PHY that only
>> talks C45.
>>> If you are not using Device Tree or ACPI, and you are letting the MDIO
>>> bus be scanned, it sounds like there should be a way for you to
>>> provide a hint as to which addresses should be scanned (that's
>>> mii_bus::phy_mask) and possibly enhance that with a mask of possible
>>> C45 devices?
>>
>> Yes, i don't like this unconditional c45 scanning. A lot of MDIO bus
>> drivers don't look for the MII_ADDR_C45. They are going to do a C22
>> transfer, and maybe not mask out the MII_ADDR_C45 from reg, causing an
>> invalid register write. Bad things can then happen.
>>
>> With DT and ACPI, we have an explicit indication that C45 should be used,
>> so we know on this platform C45 is safe to use. We need something
>> similar when not using DT or ACPI.
>>
>> Andrew
>
> Florian and Andrew,
> The mdio c22 is using the start-of-frame ST=01 while mdio c45 is using ST=00
> as identifier. So mdio c22 device will not response to mdio c45 protocol.
> As in IEEE 802.1ae-2002 Annex 45A.3 mention that:
> " Even though the Clause 45 MDIO frames using the ST=00 frame code
> will also be driven on to the Clause 22 MII Management interface,
> the Clause 22 PHYs will ignore the frames. "
>
> Hence, I am not seeing any concern that the c45 scanning will mess up with
> c22 devices.
It is not so much the messing up that concerns me other than the
increased scan time. Assuming you are making this change to support your
stmmac PCI patch series with SGMII/RGMII, etc. cannot you introduce a
bitmask of C45 PHY addresses that should be scanned and the logic could
look like (pseudo code):
- for each bit clear in mii_bus::phy_mask, scan it as C22
- for each bit clear in mii_bus::phy_c45_mask, scan it as C45
or something along those lines?
--
Florian
^ permalink raw reply
* Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
From: Cornelia Huck @ 2019-08-27 15:39 UTC (permalink / raw)
To: Alex Williamson
Cc: Parav Pandit, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190827092855.29702347@x1.home>
On Tue, 27 Aug 2019 09:28:55 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:
> On Tue, 27 Aug 2019 13:29:46 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
>
> > On Tue, 27 Aug 2019 11:08:59 +0000
> > Parav Pandit <parav@mellanox.com> wrote:
> >
> > > > -----Original Message-----
> > > > From: Cornelia Huck <cohuck@redhat.com>
> > > > Sent: Tuesday, August 27, 2019 3:59 PM
> > > > To: Parav Pandit <parav@mellanox.com>
> > > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > > > kernel@vger.kernel.org; netdev@vger.kernel.org
> > > > Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
> > > >
> > > > On Mon, 26 Aug 2019 15:41:17 -0500
> > > > Parav Pandit <parav@mellanox.com> wrote:
> > > >
> > > > > Mdev alias should be unique among all the mdevs, so that when such
> > > > > alias is used by the mdev users to derive other objects, there is no
> > > > > collision in a given system.
> > > > >
> > > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > > > ---
> > > > > drivers/vfio/mdev/mdev_core.c | 5 +++++
> > > > > 1 file changed, 5 insertions(+)
> > > > >
> > > > > diff --git a/drivers/vfio/mdev/mdev_core.c
> > > > > b/drivers/vfio/mdev/mdev_core.c index e825ff38b037..6eb37f0c6369
> > > > > 100644
> > > > > --- a/drivers/vfio/mdev/mdev_core.c
> > > > > +++ b/drivers/vfio/mdev/mdev_core.c
> > > > > @@ -375,6 +375,11 @@ int mdev_device_create(struct kobject *kobj, struct
> > > > device *dev,
> > > > > ret = -EEXIST;
> > > > > goto mdev_fail;
> > > > > }
> > > > > + if (tmp->alias && strcmp(tmp->alias, alias) == 0) {
> > > >
> > > > Any way we can relay to the caller that the uuid was fine, but that we had a
> > > > hash collision? Duplicate uuids are much more obvious than a collision here.
> > > >
> > > How do you want to relay this rare event?
> > > Netlink interface has way to return the error message back, but sysfs is limited due to its error code based interface.
> >
> > I don't know, that's why I asked :)
> >
> > The problem is that "uuid already used" and "hash collision" are
> > indistinguishable. While "use a different uuid" will probably work in
> > both cases, "increase alias length" might be a good alternative in some
> > cases.
> >
> > But if there is no good way to relay the problem, we can live with it.
>
> It's a rare event, maybe just dev_dbg(dev, "Hash collision creating alias \"%s\" for mdev device %pUl\n",...
Sounds good to me.
^ permalink raw reply
* Re: [PATCH rdma-next v3 0/3] ODP support for mlx5 DC QPs
From: Jason Gunthorpe @ 2019-08-27 15:51 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list,
Michael Guralnik, Saeed Mahameed, linux-netdev
In-Reply-To: <20190819120815.21225-1-leon@kernel.org>
On Mon, Aug 19, 2019 at 03:08:12PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Changelog
> v3:
> * Rewrote patches to expose through DEVX without need to change mlx5-abi.h at all.
> v2: https://lore.kernel.org/linux-rdma/20190806074807.9111-1-leon@kernel.org
> * Fixed reserved_* field wrong name (Saeed M.)
> * Split first patch to two patches, one for mlx5-next and one for rdma-next. (Saeed M.)
> v1: https://lore.kernel.org/linux-rdma/20190804100048.32671-1-leon@kernel.org
> * Fixed alignment to u64 in mlx5-abi.h (Gal P.)
> v0: https://lore.kernel.org/linux-rdma/20190801122139.25224-1-leon@kernel.org
>
> >From Michael,
>
> The series adds support for on-demand paging for DC transport.
>
> As DC is mlx-only transport, the capabilities are exposed
> to the user using DEVX objects and later on through mlx5dv_query_device.
>
> Thanks
>
> Michael Guralnik (3):
> net/mlx5: Set ODP capabilities for DC transport to max
> IB/mlx5: Remove check of FW capabilities in ODP page fault handling
> IB/mlx5: Add page fault handler for DC initiator WQE
This seems fine, can you put the commit on the shared branch?
Thanks,
Jason
^ permalink raw reply
* [PATCH net] ibmvnic: Do not process reset during or after device removal
From: Thomas Falcon @ 2019-08-27 16:10 UTC (permalink / raw)
To: netdev; +Cc: linuxppc-dev, Thomas Falcon
Currently, the ibmvnic driver will not schedule device resets
if the device is being removed, but does not check the device
state before the reset is actually processed. This leads to a race
where a reset is scheduled with a valid device state but is
processed after the driver has been removed, resulting in an oops.
Fix this by checking the device state before processing a queued
reset event.
Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
Tested-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index cebd20f..fa4bb94 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1983,6 +1983,10 @@ static void __ibmvnic_reset(struct work_struct *work)
rwi = get_next_rwi(adapter);
while (rwi) {
+ if (adapter->state == VNIC_REMOVING ||
+ adapter->state == VNIC_REMOVED)
+ goto out;
+
if (adapter->force_reset_recovery) {
adapter->force_reset_recovery = false;
rc = do_hard_reset(adapter, rwi, reset_state);
@@ -2007,7 +2011,7 @@ static void __ibmvnic_reset(struct work_struct *work)
netdev_dbg(adapter->netdev, "Reset failed\n");
free_all_rwi(adapter);
}
-
+out:
adapter->resetting = false;
if (we_lock_rtnl)
rtnl_unlock();
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v6 1/4] dt-bindings: net: phy: Add subnode for LED configuration
From: Rob Herring @ 2019-08-27 16:12 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: David S . Miller, Mark Rutland, Andrew Lunn, Florian Fainelli,
Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190813191147.19936-2-mka@chromium.org>
On Tue, Aug 13, 2019 at 12:11:44PM -0700, Matthias Kaehlcke wrote:
> The LED behavior of some Ethernet PHYs is configurable. Add an
> optional 'leds' subnode with a child node for each LED to be
> configured. The binding aims to be compatible with the common
> LED binding (see devicetree/bindings/leds/common.txt).
>
> A LED can be configured to be:
>
> - 'on' when a link is active, some PHYs allow configuration for
> certain link speeds
> speeds
> - 'off'
> - blink on RX/TX activity, some PHYs allow configuration for
> certain link speeds
>
> For the configuration to be effective it needs to be supported by
> the hardware and the corresponding PHY driver.
>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v6:
> - none
>
> Changes in v5:
> - renamed triggers from 'phy_link_<speed>_active' to 'phy-link-<speed>'
> - added entries for 'phy-link-<speed>-activity'
> - added 'phy-link' and 'phy-link-activity' for triggers with any link
> speed
> - added entry for trigger 'none'
>
> Changes in v4:
> - patch added to the series
> ---
> .../devicetree/bindings/net/ethernet-phy.yaml | 59 +++++++++++++++++++
> 1 file changed, 59 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index f70f18ff821f..98ba320f828b 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -153,6 +153,50 @@ properties:
> Delay after the reset was deasserted in microseconds. If
> this property is missing the delay will be skipped.
>
> +patternProperties:
> + "^leds$":
> + type: object
> + description:
> + Subnode with configuration of the PHY LEDs.
#address-cells and #size-cells needed.
> +
> + patternProperties:
> + "^led@[0-9]+$":
Need to allow for the case of 1 LED which doesn't need a unit-address
nor reg.
> + type: object
> + description:
> + Subnode with the configuration of a single PHY LED.
> +
> + properties:
> + reg:
> + description:
> + The ID number of the LED, typically corresponds to a hardware ID.
> + $ref: "/schemas/types.yaml#/definitions/uint32"
Standard properties already have a type definition. What's needed is
'maxItems: 1'.
> +
> + linux,default-trigger:
> + description:
> + This parameter, if present, is a string specifying the trigger
> + assigned to the LED. Supported triggers are:
> + "none" - LED will be solid off
> + "phy-link" - LED will be solid on when a link is active
> + "phy-link-10m" - LED will be solid on when a 10Mb/s link is active
> + "phy-link-100m" - LED will be solid on when a 100Mb/s link is active
> + "phy-link-1g" - LED will be solid on when a 1Gb/s link is active
> + "phy-link-10g" - LED will be solid on when a 10Gb/s link is active
> + "phy-link-activity" - LED will be on when link is active and blink
> + off with activity.
> + "phy-link-10m-activity" - LED will be on when 10Mb/s link is active
> + and blink off with activity.
> + "phy-link-100m-activity" - LED will be on when 100Mb/s link is
> + active and blink off with activity.
> + "phy-link-1g-activity" - LED will be on when 1Gb/s link is active
> + and blink off with activity.
> + "phy-link-10g-activity" - LED will be on when 10Gb/s link is active
> + and blink off with activity.
These strings all need to be in an enum.
The led binding is moving away from linux,default-trigger to 'function'
and 'color' properties. You probably want to do that here.
> +
> + $ref: "/schemas/types.yaml#/definitions/string"
> +
> + required:
> + - reg
> +
> required:
> - reg
>
> @@ -173,5 +217,20 @@ examples:
> reset-gpios = <&gpio1 4 1>;
> reset-assert-us = <1000>;
> reset-deassert-us = <2000>;
> +
> + leds {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + led@0 {
> + reg = <0>;
> + linux,default-trigger = "phy-link-1g";
> + };
> +
> + led@1 {
> + reg = <1>;
> + linux,default-trigger = "phy-link-100m-activity";
> + };
> + };
> };
> };
> --
> 2.23.0.rc1.153.gdeed80330f-goog
>
^ permalink raw reply
* Re: [PATCH v1 net-next] net: phy: mdio_bus: make mdiobus_scan also cover PHY that only talks C45
From: Andrew Lunn @ 2019-08-27 15:49 UTC (permalink / raw)
To: Voon, Weifeng
Cc: Florian Fainelli, David S. Miller, Maxime Coquelin,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Jose Abreu,
Heiner Kallweit, Ong, Boon Leong
In-Reply-To: <D6759987A7968C4889FDA6FA91D5CBC814758ED8@PGSMSX103.gar.corp.intel.com>
On Tue, Aug 27, 2019 at 03:23:34PM +0000, Voon, Weifeng wrote:
> > > > Make mdiobus_scan() to try harder to look for any PHY that only
> > talks C45.
> > > If you are not using Device Tree or ACPI, and you are letting the MDIO
> > > bus be scanned, it sounds like there should be a way for you to
> > > provide a hint as to which addresses should be scanned (that's
> > > mii_bus::phy_mask) and possibly enhance that with a mask of possible
> > > C45 devices?
> >
> > Yes, i don't like this unconditional c45 scanning. A lot of MDIO bus
> > drivers don't look for the MII_ADDR_C45. They are going to do a C22
> > transfer, and maybe not mask out the MII_ADDR_C45 from reg, causing an
> > invalid register write. Bad things can then happen.
> >
> > With DT and ACPI, we have an explicit indication that C45 should be used,
> > so we know on this platform C45 is safe to use. We need something
> > similar when not using DT or ACPI.
> >
> > Andrew
>
> Florian and Andrew,
> The mdio c22 is using the start-of-frame ST=01 while mdio c45 is using ST=00
> as identifier. So mdio c22 device will not response to mdio c45 protocol.
> As in IEEE 802.1ae-2002 Annex 45A.3 mention that:
> " Even though the Clause 45 MDIO frames using the ST=00 frame code
> will also be driven on to the Clause 22 MII Management interface,
> the Clause 22 PHYs will ignore the frames. "
>
> Hence, I am not seeing any concern that the c45 scanning will mess up with
> c22 devices.
Hi Voon
Take for example mdio-hisi-femac.c
static int hisi_femac_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
{
struct hisi_femac_mdio_data *data = bus->priv;
int ret;
ret = hisi_femac_mdio_wait_ready(data);
if (ret)
return ret;
writel((mii_id << BIT_PHY_ADDR_OFFSET) | regnum,
data->membase + MDIO_RWCTRL);
There is no check here for MII_ADDR_C45. So it will perform a C22
transfer. And regnum will still have MII_ADDR_C45 in it, so the
writel() is going to set bit 30, since #define MII_ADDR_C45
(1<<30). What happens on this hardware under these conditions?
You cannot unconditionally ask an MDIO driver to do a C45
transfer. Some drivers are going to do bad things.
Andrew
^ permalink raw reply
* RE: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
From: Parav Pandit @ 2019-08-27 16:13 UTC (permalink / raw)
To: Alex Williamson, Cornelia Huck
Cc: Jiri Pirko, kwankhede@nvidia.com, davem@davemloft.net,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <20190827092855.29702347@x1.home>
> -----Original Message-----
> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: Tuesday, August 27, 2019 8:59 PM
> To: Cornelia Huck <cohuck@redhat.com>
> Cc: Parav Pandit <parav@mellanox.com>; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
>
> On Tue, 27 Aug 2019 13:29:46 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
>
> > On Tue, 27 Aug 2019 11:08:59 +0000
> > Parav Pandit <parav@mellanox.com> wrote:
> >
> > > > -----Original Message-----
> > > > From: Cornelia Huck <cohuck@redhat.com>
> > > > Sent: Tuesday, August 27, 2019 3:59 PM
> > > > To: Parav Pandit <parav@mellanox.com>
> > > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > > Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all
> > > > mdevs
> > > >
> > > > On Mon, 26 Aug 2019 15:41:17 -0500 Parav Pandit
> > > > <parav@mellanox.com> wrote:
> > > >
> > > > > Mdev alias should be unique among all the mdevs, so that when
> > > > > such alias is used by the mdev users to derive other objects,
> > > > > there is no collision in a given system.
> > > > >
> > > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > > > ---
> > > > > drivers/vfio/mdev/mdev_core.c | 5 +++++
> > > > > 1 file changed, 5 insertions(+)
> > > > >
> > > > > diff --git a/drivers/vfio/mdev/mdev_core.c
> > > > > b/drivers/vfio/mdev/mdev_core.c index e825ff38b037..6eb37f0c6369
> > > > > 100644
> > > > > --- a/drivers/vfio/mdev/mdev_core.c
> > > > > +++ b/drivers/vfio/mdev/mdev_core.c
> > > > > @@ -375,6 +375,11 @@ int mdev_device_create(struct kobject *kobj,
> struct
> > > > device *dev,
> > > > > ret = -EEXIST;
> > > > > goto mdev_fail;
> > > > > }
> > > > > + if (tmp->alias && strcmp(tmp->alias, alias) == 0) {
> > > >
> > > > Any way we can relay to the caller that the uuid was fine, but
> > > > that we had a hash collision? Duplicate uuids are much more obvious than
> a collision here.
> > > >
> > > How do you want to relay this rare event?
> > > Netlink interface has way to return the error message back, but sysfs is
> limited due to its error code based interface.
> >
> > I don't know, that's why I asked :)
> >
> > The problem is that "uuid already used" and "hash collision" are
> > indistinguishable. While "use a different uuid" will probably work in
> > both cases, "increase alias length" might be a good alternative in
> > some cases.
> >
> > But if there is no good way to relay the problem, we can live with it.
>
> It's a rare event, maybe just dev_dbg(dev, "Hash collision creating alias \"%s\"
> for mdev device %pUl\n",...
>
Ok.
dev_dbg_once() to avoid message flood.
> Thanks,
> Alex
>
> > > > > + mutex_unlock(&mdev_list_lock);
> > > > > + ret = -EEXIST;
> > > > > + goto mdev_fail;
> > > > > + }
> > > > > }
> > > > >
> > > > > mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
> > >
> >
^ permalink raw reply
* RE: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
From: Parav Pandit @ 2019-08-27 16:16 UTC (permalink / raw)
To: Alex Williamson
Cc: Mark Bloch, Jiri Pirko, kwankhede@nvidia.com, cohuck@redhat.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190827092346.66bb73f1@x1.home>
> -----Original Message-----
> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: Tuesday, August 27, 2019 8:54 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: Mark Bloch <markb@mellanox.com>; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; cohuck@redhat.com; davem@davemloft.net;
> kvm@vger.kernel.org; linux-kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
>
> On Tue, 27 Aug 2019 04:28:37 +0000
> Parav Pandit <parav@mellanox.com> wrote:
>
> > Hi Mark,
> >
> > > -----Original Message-----
> > > From: Mark Bloch <markb@mellanox.com>
> > > Sent: Tuesday, August 27, 2019 4:32 AM
> > > To: Parav Pandit <parav@mellanox.com>; alex.williamson@redhat.com;
> > > Jiri Pirko <jiri@mellanox.com>; kwankhede@nvidia.com;
> > > cohuck@redhat.com; davem@davemloft.net
> > > Cc: kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > netdev@vger.kernel.org
> > > Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all
> > > mdevs
> > >
> > >
> > >
> > > On 8/26/19 1:41 PM, Parav Pandit wrote:
> > > > Mdev alias should be unique among all the mdevs, so that when such
> > > > alias is used by the mdev users to derive other objects, there is
> > > > no collision in a given system.
> > > >
> > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > > ---
> > > > drivers/vfio/mdev/mdev_core.c | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/vfio/mdev/mdev_core.c
> > > > b/drivers/vfio/mdev/mdev_core.c index e825ff38b037..6eb37f0c6369
> > > > 100644
> > > > --- a/drivers/vfio/mdev/mdev_core.c
> > > > +++ b/drivers/vfio/mdev/mdev_core.c
> > > > @@ -375,6 +375,11 @@ int mdev_device_create(struct kobject *kobj,
> > > struct device *dev,
> > > > ret = -EEXIST;
> > > > goto mdev_fail;
> > > > }
> > > > + if (tmp->alias && strcmp(tmp->alias, alias) == 0) {
> > >
> > > alias can be NULL here no?
> > >
> > If alias is NULL, tmp->alias would also be null because for given parent either
> we have alias or we don’t.
> > So its not possible to have tmp->alias as null and alias as non null.
> > But it may be good/defensive to add check for both.
>
> mdev_list is a global list of all mdev devices, how can we make any
> assumptions that an element has the same parent? Thanks,
>
Oh yes, right. If tmp->alias is not_null but alias can be NULL.
I will fix the check.
> Alex
>
> > > > + mutex_unlock(&mdev_list_lock);
> > > > + ret = -EEXIST;
> > > > + goto mdev_fail;
> > > > + }
> > > > }
> > > >
> > > > mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
> > > >
> > >
> > > Mark
^ permalink raw reply
* Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
From: Alex Williamson @ 2019-08-27 16:24 UTC (permalink / raw)
To: Parav Pandit
Cc: Cornelia Huck, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB486671BB1CD562D070F0C0F2D1A00@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Tue, 27 Aug 2019 16:13:27 +0000
Parav Pandit <parav@mellanox.com> wrote:
> > -----Original Message-----
> > From: Alex Williamson <alex.williamson@redhat.com>
> > Sent: Tuesday, August 27, 2019 8:59 PM
> > To: Cornelia Huck <cohuck@redhat.com>
> > Cc: Parav Pandit <parav@mellanox.com>; Jiri Pirko <jiri@mellanox.com>;
> > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org
> > Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
> >
> > On Tue, 27 Aug 2019 13:29:46 +0200
> > Cornelia Huck <cohuck@redhat.com> wrote:
> >
> > > On Tue, 27 Aug 2019 11:08:59 +0000
> > > Parav Pandit <parav@mellanox.com> wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Cornelia Huck <cohuck@redhat.com>
> > > > > Sent: Tuesday, August 27, 2019 3:59 PM
> > > > > To: Parav Pandit <parav@mellanox.com>
> > > > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > > > Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all
> > > > > mdevs
> > > > >
> > > > > On Mon, 26 Aug 2019 15:41:17 -0500 Parav Pandit
> > > > > <parav@mellanox.com> wrote:
> > > > >
> > > > > > Mdev alias should be unique among all the mdevs, so that when
> > > > > > such alias is used by the mdev users to derive other objects,
> > > > > > there is no collision in a given system.
> > > > > >
> > > > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > > > > ---
> > > > > > drivers/vfio/mdev/mdev_core.c | 5 +++++
> > > > > > 1 file changed, 5 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/vfio/mdev/mdev_core.c
> > > > > > b/drivers/vfio/mdev/mdev_core.c index e825ff38b037..6eb37f0c6369
> > > > > > 100644
> > > > > > --- a/drivers/vfio/mdev/mdev_core.c
> > > > > > +++ b/drivers/vfio/mdev/mdev_core.c
> > > > > > @@ -375,6 +375,11 @@ int mdev_device_create(struct kobject *kobj,
> > struct
> > > > > device *dev,
> > > > > > ret = -EEXIST;
> > > > > > goto mdev_fail;
> > > > > > }
> > > > > > + if (tmp->alias && strcmp(tmp->alias, alias) == 0) {
> > > > >
> > > > > Any way we can relay to the caller that the uuid was fine, but
> > > > > that we had a hash collision? Duplicate uuids are much more obvious than
> > a collision here.
> > > > >
> > > > How do you want to relay this rare event?
> > > > Netlink interface has way to return the error message back, but sysfs is
> > limited due to its error code based interface.
> > >
> > > I don't know, that's why I asked :)
> > >
> > > The problem is that "uuid already used" and "hash collision" are
> > > indistinguishable. While "use a different uuid" will probably work in
> > > both cases, "increase alias length" might be a good alternative in
> > > some cases.
> > >
> > > But if there is no good way to relay the problem, we can live with it.
> >
> > It's a rare event, maybe just dev_dbg(dev, "Hash collision creating alias \"%s\"
> > for mdev device %pUl\n",...
> >
> Ok.
> dev_dbg_once() to avoid message flood.
I'd suggest a rate-limit rather than a once. The fact that the kernel
may have experienced a collision at some time in the past does not help
someone debug why they can't create a device now. The only way we're
going to get a flood is if a user sufficiently privileged to create
mdev devices stumbles onto a collision and continues to repeat the same
operation. That falls into shoot-yourself-in-the-foot behavior imo.
Thanks,
Alex
^ permalink raw reply
* [PATCH v2] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for wider use
From: Andy Shevchenko @ 2019-08-27 16:37 UTC (permalink / raw)
To: Mark Fasheh, Joel Becker, Joseph Qi, ocfs2-devel, Ariel Elior,
Sudarsana Kalluru, GR-everest-linux-l2, David S. Miller, netdev,
Colin Ian King
Cc: Andy Shevchenko
There are users already and will be more of BITS_TO_BYTES() macro.
Move it to bitops.h for wider use.
In the case of ocfs2 the replacement is identical.
As for bnx2x, there are two places where floor version is used.
In the first case to calculate the amount of structures that can fit
one memory page. In this case obviously the ceiling variant is correct and
original code might have a potential bug, if amount of bits % 8 is not 0.
In the second case the macro is used to calculate bytes transmitted in one
microsecond. This will work for all speeds which is multiply of 1Gbps without
any change, for the rest new code will give ceiling value, for instance 100Mbps
will give 13 bytes, while old code gives 12 bytes and the arithmetically
correct one is 12.5 bytes. Further the value is used to setup timer threshold
which in any case has its own margins due to certain resolution. I don't see
here an issue with slightly shifting thresholds for low speed connections, the
card is supposed to utilize highest available rate, which is usually 10Gbps.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
- described bnx2x cases in the commit message
- appended Rb (for ocfs2)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h | 1 -
fs/ocfs2/dlm/dlmcommon.h | 4 ----
include/linux/bitops.h | 1 +
3 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
index 066765fbef06..0a59a09ef82f 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
@@ -296,7 +296,6 @@ static inline void bnx2x_dcb_config_qm(struct bnx2x *bp, enum cos_mode mode,
* possible, the driver should only write the valid vnics into the internal
* ram according to the appropriate port mode.
*/
-#define BITS_TO_BYTES(x) ((x)/8)
/* CMNG constants, as derived from system spec calculations */
diff --git a/fs/ocfs2/dlm/dlmcommon.h b/fs/ocfs2/dlm/dlmcommon.h
index aaf24548b02a..0463dce65bb2 100644
--- a/fs/ocfs2/dlm/dlmcommon.h
+++ b/fs/ocfs2/dlm/dlmcommon.h
@@ -688,10 +688,6 @@ struct dlm_begin_reco
__be32 pad2;
};
-
-#define BITS_PER_BYTE 8
-#define BITS_TO_BYTES(bits) (((bits)+BITS_PER_BYTE-1)/BITS_PER_BYTE)
-
struct dlm_query_join_request
{
u8 node_idx;
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index cf074bce3eb3..79d80f5ddf7b 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -5,6 +5,7 @@
#include <linux/bits.h>
#define BITS_PER_TYPE(type) (sizeof(type) * BITS_PER_BYTE)
+#define BITS_TO_BYTES(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE)
#define BITS_TO_LONGS(nr) DIV_ROUND_UP(nr, BITS_PER_TYPE(long))
extern unsigned int __sw_hweight8(unsigned int w);
--
2.23.0.rc1
^ permalink raw reply related
* [net-next 00/15][pull request] 100GbE Intel Wired LAN Driver Updates 2019-08-26
From: Jeff Kirsher @ 2019-08-27 16:38 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann
This series contains updates to ice driver only.
Usha fixes the statistics reported on 4 port NICs which were reporting
the incorrect statistics due to using the incorrect port identifier.
Victor fixes an issue when trying to traverse to the first node of a
requested layer by adding a sibling head pointer for each layer per
traffic class.
Anirudh cleans up the locking and logic for enabling and disabling
VSI's to make it more consistent. Updates the driver to do dynamic
allocation of queue management bitmaps and arrays, rather than
statically allocating them which consumes more memory than required.
Refactor the logic in ice_ena_msix_range() for clarity and add
additional checks for when requested resources exceed what is available.
Jesse updates the debugging print statements to make it more useful when
dealing with link and PHY related issues.
Krzysztof adds a local variable to the VSI rebuild path to improve
readability.
Akeem limits the reporting of MDD events from VFs so that the kernel
log is not clogged up with MDD events which are duplicate or potentially
false positives. Fixed a reset issue that would result in the system
getting into a state that could only be resolved by a reboot by
testing if the VF is in a disabled state during a reset.
Michal adds a check to avoid trying to access memory that has not be
allocated by checking the number of queue pairs.
Jake fixes a static analysis warning due to a cast of a u8 to unsigned
long, so just update ice_is_tc_ena() to take a unsigned long so that a
cast is not necessary.
Colin Ian King fixes a potential infinite loop where a u8 is being
compared to an int.
Maciej refactors the queue handling functions that work on queue arrays
so that the logic can be done for a single queue.
Paul adds support for VFs to enable and disable single queues.
Henry fixed the order of operations in ice_remove() which was trying to
use adminq operations that were already disabled.
The following are changes since commit d00ee466a07eb9182ad3caf6140c7ebb527b4c64:
nfp: add AMDA0058 boards to firmware list
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 100GbE
Akeem G Abodunrin (2):
ice: Don't clog kernel debug log with VF MDD events errors
ice: Fix VF configuration issues due to reset
Anirudh Venkataramanan (3):
ice: Sanitize ice_ena_vsi and ice_dis_vsi
ice: Alloc queue management bitmaps and arrays dynamically
ice: Rework ice_ena_msix_range
Colin Ian King (1):
ice: fix potential infinite loop
Henry Tieman (1):
ice: fix adminq calls during remove
Jacob Keller (1):
ice: fix ice_is_tc_ena
Jesse Brandeburg (1):
ice: shorten local and add debug prints
Krzysztof Kazimierczak (1):
ice: Introduce a local variable for a VSI in the rebuild path
Maciej Fijalkowski (1):
ice: add support for enabling/disabling single queues
Michal Swiatkowski (1):
ice: add validation in OP_CONFIG_VSI_QUEUES VF message
Paul Greenwalt (1):
ice: add support for virtchnl_queue_select.[tx|rx]_queues bitmap
Usha Ketineni (1):
ice: Fix ethtool port and PFC stats for 4x25G cards
Victor Raj (1):
ice: added sibling head to parse nodes
drivers/net/ethernet/intel/ice/ice.h | 12 +-
drivers/net/ethernet/intel/ice/ice_common.c | 63 ++-
drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 13 +-
drivers/net/ethernet/intel/ice/ice_lib.c | 398 +++++++++++-------
drivers/net/ethernet/intel/ice/ice_lib.h | 28 +-
drivers/net/ethernet/intel/ice/ice_main.c | 205 +++++----
drivers/net/ethernet/intel/ice/ice_sched.c | 57 +--
drivers/net/ethernet/intel/ice/ice_type.h | 6 +-
.../net/ethernet/intel/ice/ice_virtchnl_pf.c | 279 ++++++++----
.../net/ethernet/intel/ice/ice_virtchnl_pf.h | 14 +-
10 files changed, 688 insertions(+), 387 deletions(-)
--
2.21.0
^ permalink raw reply
* [net-next 03/15] ice: Sanitize ice_ena_vsi and ice_dis_vsi
From: Jeff Kirsher @ 2019-08-27 16:38 UTC (permalink / raw)
To: davem
Cc: Anirudh Venkataramanan, netdev, nhorman, sassmann, Andrew Bowers,
Jeff Kirsher
In-Reply-To: <20190827163832.8362-1-jeffrey.t.kirsher@intel.com>
From: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
1. ndo_open and ndo_stop are implemented by ice_open and ice_stop
respectively. When enabling/disabling VSIs, just call
ice_open/ice_stop instead of ndo_open/ndo_stop.
2. Rework logic around rtnl_lock/rtnl_unlock
3. In ice_ena_vsi, remove an unnecessary stack variable and return
0 instead of err when __ICE_NEEDS_RESTART is not set.
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ice/ice_main.c | 24 +++++++++++------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 76a647cc2dbd..2f6125bfd991 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -436,13 +436,13 @@ static void ice_dis_vsi(struct ice_vsi *vsi, bool locked)
if (vsi->type == ICE_VSI_PF && vsi->netdev) {
if (netif_running(vsi->netdev)) {
- if (!locked) {
+ if (!locked)
rtnl_lock();
- vsi->netdev->netdev_ops->ndo_stop(vsi->netdev);
+
+ ice_stop(vsi->netdev);
+
+ if (!locked)
rtnl_unlock();
- } else {
- vsi->netdev->netdev_ops->ndo_stop(vsi->netdev);
- }
} else {
ice_vsi_close(vsi);
}
@@ -3654,21 +3654,19 @@ static int ice_ena_vsi(struct ice_vsi *vsi, bool locked)
int err = 0;
if (!test_bit(__ICE_NEEDS_RESTART, vsi->state))
- return err;
+ return 0;
clear_bit(__ICE_NEEDS_RESTART, vsi->state);
if (vsi->netdev && vsi->type == ICE_VSI_PF) {
- struct net_device *netd = vsi->netdev;
-
if (netif_running(vsi->netdev)) {
- if (locked) {
- err = netd->netdev_ops->ndo_open(netd);
- } else {
+ if (!locked)
rtnl_lock();
- err = netd->netdev_ops->ndo_open(netd);
+
+ err = ice_open(vsi->netdev);
+
+ if (!locked)
rtnl_unlock();
- }
}
}
--
2.21.0
^ permalink raw reply related
* [net-next 09/15] ice: fix potential infinite loop
From: Jeff Kirsher @ 2019-08-27 16:38 UTC (permalink / raw)
To: davem
Cc: Colin Ian King, netdev, nhorman, sassmann, Andrew Bowers,
Jeff Kirsher
In-Reply-To: <20190827163832.8362-1-jeffrey.t.kirsher@intel.com>
From: Colin Ian King <colin.king@canonical.com>
The loop counter of a for-loop is a u8 however this is being compared
to an int upper bound and this can lead to an infinite loop if the
upper bound is greater than 255 since the loop counter will wrap back
to zero. Fix this potential issue by making the loop counter an int.
Addresses-Coverity: ("Infinite loop")
Fixes: c7aeb4d1b9bf ("ice: Disable VFs until reset is completed")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 67cbebe1ff3f..0398c86226f0 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -477,7 +477,7 @@ static void
ice_prepare_for_reset(struct ice_pf *pf)
{
struct ice_hw *hw = &pf->hw;
- u8 i;
+ int i;
/* already prepared for reset */
if (test_bit(__ICE_PREPARED_FOR_RESET, pf->state))
--
2.21.0
^ permalink raw reply related
* [net-next 01/15] ice: Fix ethtool port and PFC stats for 4x25G cards
From: Jeff Kirsher @ 2019-08-27 16:38 UTC (permalink / raw)
To: davem; +Cc: Usha Ketineni, netdev, nhorman, sassmann, Andrew Bowers,
Jeff Kirsher
In-Reply-To: <20190827163832.8362-1-jeffrey.t.kirsher@intel.com>
From: Usha Ketineni <usha.k.ketineni@intel.com>
This patch fixes the issue where port and PFC statistics counters are
incrementing at the wrong port with 4x25G cards.
Read the GLPRT port registers using lport parameter instead of pf_id to
update the statistics otherwise the pf_ids are flipped for ports 2 and 3
when read from the HW register PF_FUNC_RID and this is expected as per
hardware specification.
Signed-off-by: Usha Ketineni <usha.k.ketineni@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 13 ++--
drivers/net/ethernet/intel/ice/ice_main.c | 76 ++++++++++----------
2 files changed, 45 insertions(+), 44 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
index 734cef8eed9e..d9578919aad8 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
@@ -500,30 +500,31 @@ void ice_update_dcb_stats(struct ice_pf *pf)
{
struct ice_hw_port_stats *prev_ps, *cur_ps;
struct ice_hw *hw = &pf->hw;
- u8 pf_id = hw->pf_id;
+ u8 port;
int i;
+ port = hw->port_info->lport;
prev_ps = &pf->stats_prev;
cur_ps = &pf->stats;
for (i = 0; i < 8; i++) {
- ice_stat_update32(hw, GLPRT_PXOFFRXC(pf_id, i),
+ ice_stat_update32(hw, GLPRT_PXOFFRXC(port, i),
pf->stat_prev_loaded,
&prev_ps->priority_xoff_rx[i],
&cur_ps->priority_xoff_rx[i]);
- ice_stat_update32(hw, GLPRT_PXONRXC(pf_id, i),
+ ice_stat_update32(hw, GLPRT_PXONRXC(port, i),
pf->stat_prev_loaded,
&prev_ps->priority_xon_rx[i],
&cur_ps->priority_xon_rx[i]);
- ice_stat_update32(hw, GLPRT_PXONTXC(pf_id, i),
+ ice_stat_update32(hw, GLPRT_PXONTXC(port, i),
pf->stat_prev_loaded,
&prev_ps->priority_xon_tx[i],
&cur_ps->priority_xon_tx[i]);
- ice_stat_update32(hw, GLPRT_PXOFFTXC(pf_id, i),
+ ice_stat_update32(hw, GLPRT_PXOFFTXC(port, i),
pf->stat_prev_loaded,
&prev_ps->priority_xoff_tx[i],
&cur_ps->priority_xoff_tx[i]);
- ice_stat_update32(hw, GLPRT_RXON2OFFCNT(pf_id, i),
+ ice_stat_update32(hw, GLPRT_RXON2OFFCNT(port, i),
pf->stat_prev_loaded,
&prev_ps->priority_xon_2_xoff[i],
&cur_ps->priority_xon_2_xoff[i]);
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index f3923dec32b7..76a647cc2dbd 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3262,25 +3262,25 @@ void ice_update_pf_stats(struct ice_pf *pf)
{
struct ice_hw_port_stats *prev_ps, *cur_ps;
struct ice_hw *hw = &pf->hw;
- u8 pf_id;
+ u8 port;
+ port = hw->port_info->lport;
prev_ps = &pf->stats_prev;
cur_ps = &pf->stats;
- pf_id = hw->pf_id;
- ice_stat_update40(hw, GLPRT_GORCL(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_GORCL(port), pf->stat_prev_loaded,
&prev_ps->eth.rx_bytes,
&cur_ps->eth.rx_bytes);
- ice_stat_update40(hw, GLPRT_UPRCL(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_UPRCL(port), pf->stat_prev_loaded,
&prev_ps->eth.rx_unicast,
&cur_ps->eth.rx_unicast);
- ice_stat_update40(hw, GLPRT_MPRCL(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_MPRCL(port), pf->stat_prev_loaded,
&prev_ps->eth.rx_multicast,
&cur_ps->eth.rx_multicast);
- ice_stat_update40(hw, GLPRT_BPRCL(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_BPRCL(port), pf->stat_prev_loaded,
&prev_ps->eth.rx_broadcast,
&cur_ps->eth.rx_broadcast);
@@ -3288,109 +3288,109 @@ void ice_update_pf_stats(struct ice_pf *pf)
&prev_ps->eth.rx_discards,
&cur_ps->eth.rx_discards);
- ice_stat_update40(hw, GLPRT_GOTCL(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_GOTCL(port), pf->stat_prev_loaded,
&prev_ps->eth.tx_bytes,
&cur_ps->eth.tx_bytes);
- ice_stat_update40(hw, GLPRT_UPTCL(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_UPTCL(port), pf->stat_prev_loaded,
&prev_ps->eth.tx_unicast,
&cur_ps->eth.tx_unicast);
- ice_stat_update40(hw, GLPRT_MPTCL(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_MPTCL(port), pf->stat_prev_loaded,
&prev_ps->eth.tx_multicast,
&cur_ps->eth.tx_multicast);
- ice_stat_update40(hw, GLPRT_BPTCL(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_BPTCL(port), pf->stat_prev_loaded,
&prev_ps->eth.tx_broadcast,
&cur_ps->eth.tx_broadcast);
- ice_stat_update32(hw, GLPRT_TDOLD(pf_id), pf->stat_prev_loaded,
+ ice_stat_update32(hw, GLPRT_TDOLD(port), pf->stat_prev_loaded,
&prev_ps->tx_dropped_link_down,
&cur_ps->tx_dropped_link_down);
- ice_stat_update40(hw, GLPRT_PRC64L(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_PRC64L(port), pf->stat_prev_loaded,
&prev_ps->rx_size_64, &cur_ps->rx_size_64);
- ice_stat_update40(hw, GLPRT_PRC127L(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_PRC127L(port), pf->stat_prev_loaded,
&prev_ps->rx_size_127, &cur_ps->rx_size_127);
- ice_stat_update40(hw, GLPRT_PRC255L(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_PRC255L(port), pf->stat_prev_loaded,
&prev_ps->rx_size_255, &cur_ps->rx_size_255);
- ice_stat_update40(hw, GLPRT_PRC511L(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_PRC511L(port), pf->stat_prev_loaded,
&prev_ps->rx_size_511, &cur_ps->rx_size_511);
- ice_stat_update40(hw, GLPRT_PRC1023L(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_PRC1023L(port), pf->stat_prev_loaded,
&prev_ps->rx_size_1023, &cur_ps->rx_size_1023);
- ice_stat_update40(hw, GLPRT_PRC1522L(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_PRC1522L(port), pf->stat_prev_loaded,
&prev_ps->rx_size_1522, &cur_ps->rx_size_1522);
- ice_stat_update40(hw, GLPRT_PRC9522L(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_PRC9522L(port), pf->stat_prev_loaded,
&prev_ps->rx_size_big, &cur_ps->rx_size_big);
- ice_stat_update40(hw, GLPRT_PTC64L(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_PTC64L(port), pf->stat_prev_loaded,
&prev_ps->tx_size_64, &cur_ps->tx_size_64);
- ice_stat_update40(hw, GLPRT_PTC127L(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_PTC127L(port), pf->stat_prev_loaded,
&prev_ps->tx_size_127, &cur_ps->tx_size_127);
- ice_stat_update40(hw, GLPRT_PTC255L(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_PTC255L(port), pf->stat_prev_loaded,
&prev_ps->tx_size_255, &cur_ps->tx_size_255);
- ice_stat_update40(hw, GLPRT_PTC511L(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_PTC511L(port), pf->stat_prev_loaded,
&prev_ps->tx_size_511, &cur_ps->tx_size_511);
- ice_stat_update40(hw, GLPRT_PTC1023L(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_PTC1023L(port), pf->stat_prev_loaded,
&prev_ps->tx_size_1023, &cur_ps->tx_size_1023);
- ice_stat_update40(hw, GLPRT_PTC1522L(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_PTC1522L(port), pf->stat_prev_loaded,
&prev_ps->tx_size_1522, &cur_ps->tx_size_1522);
- ice_stat_update40(hw, GLPRT_PTC9522L(pf_id), pf->stat_prev_loaded,
+ ice_stat_update40(hw, GLPRT_PTC9522L(port), pf->stat_prev_loaded,
&prev_ps->tx_size_big, &cur_ps->tx_size_big);
- ice_stat_update32(hw, GLPRT_LXONRXC(pf_id), pf->stat_prev_loaded,
+ ice_stat_update32(hw, GLPRT_LXONRXC(port), pf->stat_prev_loaded,
&prev_ps->link_xon_rx, &cur_ps->link_xon_rx);
- ice_stat_update32(hw, GLPRT_LXOFFRXC(pf_id), pf->stat_prev_loaded,
+ ice_stat_update32(hw, GLPRT_LXOFFRXC(port), pf->stat_prev_loaded,
&prev_ps->link_xoff_rx, &cur_ps->link_xoff_rx);
- ice_stat_update32(hw, GLPRT_LXONTXC(pf_id), pf->stat_prev_loaded,
+ ice_stat_update32(hw, GLPRT_LXONTXC(port), pf->stat_prev_loaded,
&prev_ps->link_xon_tx, &cur_ps->link_xon_tx);
- ice_stat_update32(hw, GLPRT_LXOFFTXC(pf_id), pf->stat_prev_loaded,
+ ice_stat_update32(hw, GLPRT_LXOFFTXC(port), pf->stat_prev_loaded,
&prev_ps->link_xoff_tx, &cur_ps->link_xoff_tx);
ice_update_dcb_stats(pf);
- ice_stat_update32(hw, GLPRT_CRCERRS(pf_id), pf->stat_prev_loaded,
+ ice_stat_update32(hw, GLPRT_CRCERRS(port), pf->stat_prev_loaded,
&prev_ps->crc_errors, &cur_ps->crc_errors);
- ice_stat_update32(hw, GLPRT_ILLERRC(pf_id), pf->stat_prev_loaded,
+ ice_stat_update32(hw, GLPRT_ILLERRC(port), pf->stat_prev_loaded,
&prev_ps->illegal_bytes, &cur_ps->illegal_bytes);
- ice_stat_update32(hw, GLPRT_MLFC(pf_id), pf->stat_prev_loaded,
+ ice_stat_update32(hw, GLPRT_MLFC(port), pf->stat_prev_loaded,
&prev_ps->mac_local_faults,
&cur_ps->mac_local_faults);
- ice_stat_update32(hw, GLPRT_MRFC(pf_id), pf->stat_prev_loaded,
+ ice_stat_update32(hw, GLPRT_MRFC(port), pf->stat_prev_loaded,
&prev_ps->mac_remote_faults,
&cur_ps->mac_remote_faults);
- ice_stat_update32(hw, GLPRT_RLEC(pf_id), pf->stat_prev_loaded,
+ ice_stat_update32(hw, GLPRT_RLEC(port), pf->stat_prev_loaded,
&prev_ps->rx_len_errors, &cur_ps->rx_len_errors);
- ice_stat_update32(hw, GLPRT_RUC(pf_id), pf->stat_prev_loaded,
+ ice_stat_update32(hw, GLPRT_RUC(port), pf->stat_prev_loaded,
&prev_ps->rx_undersize, &cur_ps->rx_undersize);
- ice_stat_update32(hw, GLPRT_RFC(pf_id), pf->stat_prev_loaded,
+ ice_stat_update32(hw, GLPRT_RFC(port), pf->stat_prev_loaded,
&prev_ps->rx_fragments, &cur_ps->rx_fragments);
- ice_stat_update32(hw, GLPRT_ROC(pf_id), pf->stat_prev_loaded,
+ ice_stat_update32(hw, GLPRT_ROC(port), pf->stat_prev_loaded,
&prev_ps->rx_oversize, &cur_ps->rx_oversize);
- ice_stat_update32(hw, GLPRT_RJC(pf_id), pf->stat_prev_loaded,
+ ice_stat_update32(hw, GLPRT_RJC(port), pf->stat_prev_loaded,
&prev_ps->rx_jabber, &cur_ps->rx_jabber);
pf->stat_prev_loaded = true;
--
2.21.0
^ permalink raw reply related
* [net-next 12/15] ice: Alloc queue management bitmaps and arrays dynamically
From: Jeff Kirsher @ 2019-08-27 16:38 UTC (permalink / raw)
To: davem
Cc: Anirudh Venkataramanan, netdev, nhorman, sassmann, Andrew Bowers,
Jeff Kirsher
In-Reply-To: <20190827163832.8362-1-jeffrey.t.kirsher@intel.com>
From: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
The total number of queues available on the device is divided between
multiple physical functions (PF) in the firmware and provided to the
driver when it gets function capabilities from the firmware. Thus
each PF knows how many Tx/Rx queues it has. These queues are then
doled out to different VSIs (for LAN traffic, SR-IOV VF traffic, etc.)
To track usage of these queues at the PF level, the driver uses two
bitmaps avail_txqs and avail_rxqs. At the VSI level (i.e. struct ice_vsi
instances) the driver uses two arrays txq_map and rxq_map, to track
ownership of VSIs' queues in avail_txqs and avail_rxqs respectively.
The aforementioned bitmaps and arrays should be allocated dynamically,
because the number of queues supported by a PF is only available once
function capabilities have been queried. The current static allocation
consumes way more memory than required.
This patch removes the DECLARE_BITMAP for avail_txqs and avail_rxqs
and instead uses bitmap_zalloc to allocate the bitmaps during init.
Similarly txq_map and rxq_map are now allocated in ice_vsi_alloc_arrays.
As a result ICE_MAX_TXQS and ICE_MAX_RXQS defines are no longer needed.
Also as txq_map and rxq_map are now allocated and freed, some code
reordering was required in ice_vsi_rebuild for correct functioning.
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ice/ice.h | 12 +++---
drivers/net/ethernet/intel/ice/ice_lib.c | 45 ++++++++++++++++++-----
drivers/net/ethernet/intel/ice/ice_main.c | 40 ++++++++++++++++----
3 files changed, 74 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index 97d0f61cf52b..fb2bc836b20a 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -73,8 +73,6 @@ extern const char ice_drv_ver[];
#define ICE_MBXRQ_LEN 512
#define ICE_MIN_MSIX 2
#define ICE_NO_VSI 0xffff
-#define ICE_MAX_TXQS 2048
-#define ICE_MAX_RXQS 2048
#define ICE_VSI_MAP_CONTIG 0
#define ICE_VSI_MAP_SCATTER 1
#define ICE_MAX_SCATTER_TXQS 16
@@ -284,8 +282,8 @@ struct ice_vsi {
/* queue information */
u8 tx_mapping_mode; /* ICE_MAP_MODE_[CONTIG|SCATTER] */
u8 rx_mapping_mode; /* ICE_MAP_MODE_[CONTIG|SCATTER] */
- u16 txq_map[ICE_MAX_TXQS]; /* index in pf->avail_txqs */
- u16 rxq_map[ICE_MAX_RXQS]; /* index in pf->avail_rxqs */
+ u16 *txq_map; /* index in pf->avail_txqs */
+ u16 *rxq_map; /* index in pf->avail_rxqs */
u16 alloc_txq; /* Allocated Tx queues */
u16 num_txq; /* Used Tx queues */
u16 alloc_rxq; /* Allocated Rx queues */
@@ -355,9 +353,9 @@ struct ice_pf {
u16 num_vf_qps; /* num queue pairs per VF */
u16 num_vf_msix; /* num vectors per VF */
DECLARE_BITMAP(state, __ICE_STATE_NBITS);
- DECLARE_BITMAP(avail_txqs, ICE_MAX_TXQS);
- DECLARE_BITMAP(avail_rxqs, ICE_MAX_RXQS);
DECLARE_BITMAP(flags, ICE_PF_FLAGS_NBITS);
+ unsigned long *avail_txqs; /* bitmap to track PF Tx queue usage */
+ unsigned long *avail_rxqs; /* bitmap to track PF Rx queue usage */
unsigned long serv_tmr_period;
unsigned long serv_tmr_prev;
struct timer_list serv_tmr;
@@ -368,6 +366,8 @@ struct ice_pf {
u32 hw_csum_rx_error;
u32 oicr_idx; /* Other interrupt cause MSIX vector index */
u32 num_avail_sw_msix; /* remaining MSIX SW vectors left unclaimed */
+ u16 max_pf_txqs; /* Total Tx queues PF wide */
+ u16 max_pf_rxqs; /* Total Rx queues PF wide */
u32 num_lan_msix; /* Total MSIX vectors for base driver */
u16 num_lan_tx; /* num LAN Tx queues setup */
u16 num_lan_rx; /* num LAN Rx queues setup */
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index fb866be84088..a39767e8c2a2 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -263,12 +263,24 @@ static int ice_vsi_alloc_arrays(struct ice_vsi *vsi)
vsi->tx_rings = devm_kcalloc(&pf->pdev->dev, vsi->alloc_txq,
sizeof(*vsi->tx_rings), GFP_KERNEL);
if (!vsi->tx_rings)
- goto err_txrings;
+ return -ENOMEM;
vsi->rx_rings = devm_kcalloc(&pf->pdev->dev, vsi->alloc_rxq,
sizeof(*vsi->rx_rings), GFP_KERNEL);
if (!vsi->rx_rings)
- goto err_rxrings;
+ goto err_rings;
+
+ vsi->txq_map = devm_kcalloc(&pf->pdev->dev, vsi->alloc_txq,
+ sizeof(*vsi->txq_map), GFP_KERNEL);
+
+ if (!vsi->txq_map)
+ goto err_txq_map;
+
+ vsi->rxq_map = devm_kcalloc(&pf->pdev->dev, vsi->alloc_rxq,
+ sizeof(*vsi->rxq_map), GFP_KERNEL);
+ if (!vsi->rxq_map)
+ goto err_rxq_map;
+
/* There is no need to allocate q_vectors for a loopback VSI. */
if (vsi->type == ICE_VSI_LB)
@@ -283,10 +295,13 @@ static int ice_vsi_alloc_arrays(struct ice_vsi *vsi)
return 0;
err_vectors:
+ devm_kfree(&pf->pdev->dev, vsi->rxq_map);
+err_rxq_map:
+ devm_kfree(&pf->pdev->dev, vsi->txq_map);
+err_txq_map:
devm_kfree(&pf->pdev->dev, vsi->rx_rings);
-err_rxrings:
+err_rings:
devm_kfree(&pf->pdev->dev, vsi->tx_rings);
-err_txrings:
return -ENOMEM;
}
@@ -433,6 +448,14 @@ static void ice_vsi_free_arrays(struct ice_vsi *vsi)
devm_kfree(&pf->pdev->dev, vsi->rx_rings);
vsi->rx_rings = NULL;
}
+ if (vsi->txq_map) {
+ devm_kfree(&pf->pdev->dev, vsi->txq_map);
+ vsi->txq_map = NULL;
+ }
+ if (vsi->rxq_map) {
+ devm_kfree(&pf->pdev->dev, vsi->rxq_map);
+ vsi->rxq_map = NULL;
+ }
}
/**
@@ -664,7 +687,7 @@ static int ice_vsi_get_qs(struct ice_vsi *vsi)
struct ice_qs_cfg tx_qs_cfg = {
.qs_mutex = &pf->avail_q_mutex,
.pf_map = pf->avail_txqs,
- .pf_map_size = ICE_MAX_TXQS,
+ .pf_map_size = pf->max_pf_txqs,
.q_count = vsi->alloc_txq,
.scatter_count = ICE_MAX_SCATTER_TXQS,
.vsi_map = vsi->txq_map,
@@ -674,7 +697,7 @@ static int ice_vsi_get_qs(struct ice_vsi *vsi)
struct ice_qs_cfg rx_qs_cfg = {
.qs_mutex = &pf->avail_q_mutex,
.pf_map = pf->avail_rxqs,
- .pf_map_size = ICE_MAX_RXQS,
+ .pf_map_size = pf->max_pf_rxqs,
.q_count = vsi->alloc_rxq,
.scatter_count = ICE_MAX_SCATTER_RXQS,
.vsi_map = vsi->rxq_map,
@@ -3018,6 +3041,7 @@ int ice_vsi_rebuild(struct ice_vsi *vsi)
vsi->base_vector = 0;
}
+ ice_vsi_put_qs(vsi);
ice_vsi_clear_rings(vsi);
ice_vsi_free_arrays(vsi);
ice_dev_onetime_setup(&pf->hw);
@@ -3025,6 +3049,12 @@ int ice_vsi_rebuild(struct ice_vsi *vsi)
ice_vsi_set_num_qs(vsi, vf->vf_id);
else
ice_vsi_set_num_qs(vsi, ICE_INVAL_VFID);
+
+ ret = ice_vsi_alloc_arrays(vsi);
+ if (ret < 0)
+ goto err_vsi;
+
+ ice_vsi_get_qs(vsi);
ice_vsi_set_tc_cfg(vsi);
/* Initialize VSI struct elements and create VSI in FW */
@@ -3032,9 +3062,6 @@ int ice_vsi_rebuild(struct ice_vsi *vsi)
if (ret < 0)
goto err_vsi;
- ret = ice_vsi_alloc_arrays(vsi);
- if (ret < 0)
- goto err_vsi;
switch (vsi->type) {
case ICE_VSI_PF:
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index e47aab6d998d..2499c7ee5038 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -2207,13 +2207,23 @@ static void ice_deinit_pf(struct ice_pf *pf)
ice_service_task_stop(pf);
mutex_destroy(&pf->sw_mutex);
mutex_destroy(&pf->avail_q_mutex);
+
+ if (pf->avail_txqs) {
+ bitmap_free(pf->avail_txqs);
+ pf->avail_txqs = NULL;
+ }
+
+ if (pf->avail_rxqs) {
+ bitmap_free(pf->avail_rxqs);
+ pf->avail_rxqs = NULL;
+ }
}
/**
* ice_init_pf - Initialize general software structures (struct ice_pf)
* @pf: board private structure to initialize
*/
-static void ice_init_pf(struct ice_pf *pf)
+static int ice_init_pf(struct ice_pf *pf)
{
bitmap_zero(pf->flags, ICE_PF_FLAGS_NBITS);
#ifdef CONFIG_PCI_IOV
@@ -2229,12 +2239,6 @@ static void ice_init_pf(struct ice_pf *pf)
mutex_init(&pf->sw_mutex);
mutex_init(&pf->avail_q_mutex);
- /* Clear avail_[t|r]x_qs bitmaps (set all to avail) */
- mutex_lock(&pf->avail_q_mutex);
- bitmap_zero(pf->avail_txqs, ICE_MAX_TXQS);
- bitmap_zero(pf->avail_rxqs, ICE_MAX_RXQS);
- mutex_unlock(&pf->avail_q_mutex);
-
if (pf->hw.func_caps.common_cap.rss_table_size)
set_bit(ICE_FLAG_RSS_ENA, pf->flags);
@@ -2243,6 +2247,22 @@ static void ice_init_pf(struct ice_pf *pf)
pf->serv_tmr_period = HZ;
INIT_WORK(&pf->serv_task, ice_service_task);
clear_bit(__ICE_SERVICE_SCHED, pf->state);
+
+ pf->max_pf_txqs = pf->hw.func_caps.common_cap.num_txq;
+ pf->max_pf_rxqs = pf->hw.func_caps.common_cap.num_rxq;
+
+ pf->avail_txqs = bitmap_zalloc(pf->max_pf_txqs, GFP_KERNEL);
+ if (!pf->avail_txqs)
+ return -ENOMEM;
+
+ pf->avail_rxqs = bitmap_zalloc(pf->max_pf_rxqs, GFP_KERNEL);
+ if (!pf->avail_rxqs) {
+ devm_kfree(&pf->pdev->dev, pf->avail_txqs);
+ pf->avail_txqs = NULL;
+ return -ENOMEM;
+ }
+
+ return 0;
}
/**
@@ -2467,7 +2487,11 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
hw->fw_maj_ver, hw->fw_min_ver, hw->fw_build,
hw->api_maj_ver, hw->api_min_ver);
- ice_init_pf(pf);
+ err = ice_init_pf(pf);
+ if (err) {
+ dev_err(dev, "ice_init_pf failed: %d\n", err);
+ goto err_init_pf_unroll;
+ }
err = ice_init_pf_dcb(pf, false);
if (err) {
--
2.21.0
^ permalink raw reply related
* [net-next 05/15] ice: Introduce a local variable for a VSI in the rebuild path
From: Jeff Kirsher @ 2019-08-27 16:38 UTC (permalink / raw)
To: davem
Cc: Krzysztof Kazimierczak, netdev, nhorman, sassmann, Andrew Bowers,
Jeff Kirsher
In-Reply-To: <20190827163832.8362-1-jeffrey.t.kirsher@intel.com>
From: Krzysztof Kazimierczak <krzysztof.kazimierczak@intel.com>
When a VSI is accessed inside the ice_for_each_vsi macro in the rebuild
path (ice_vsi_rebuild_all() and ice_vsi_replay_all()), it is referred to
as pf->vsi[i]. Introduce local variables to improve readability.
Signed-off-by: Krzysztof Kazimierczak <krzysztof.kazimierczak@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ice/ice_main.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 2f6125bfd991..2c6b2bc4e30e 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3704,22 +3704,23 @@ static int ice_vsi_rebuild_all(struct ice_pf *pf)
/* loop through pf->vsi array and reinit the VSI if found */
ice_for_each_vsi(pf, i) {
+ struct ice_vsi *vsi = pf->vsi[i];
int err;
- if (!pf->vsi[i])
+ if (!vsi)
continue;
- err = ice_vsi_rebuild(pf->vsi[i]);
+ err = ice_vsi_rebuild(vsi);
if (err) {
dev_err(&pf->pdev->dev,
"VSI at index %d rebuild failed\n",
- pf->vsi[i]->idx);
+ vsi->idx);
return err;
}
dev_info(&pf->pdev->dev,
"VSI at index %d rebuilt. vsi_num = 0x%x\n",
- pf->vsi[i]->idx, pf->vsi[i]->vsi_num);
+ vsi->idx, vsi->vsi_num);
}
return 0;
@@ -3737,25 +3738,27 @@ static int ice_vsi_replay_all(struct ice_pf *pf)
/* loop through pf->vsi array and replay the VSI if found */
ice_for_each_vsi(pf, i) {
- if (!pf->vsi[i])
+ struct ice_vsi *vsi = pf->vsi[i];
+
+ if (!vsi)
continue;
- ret = ice_replay_vsi(hw, pf->vsi[i]->idx);
+ ret = ice_replay_vsi(hw, vsi->idx);
if (ret) {
dev_err(&pf->pdev->dev,
"VSI at index %d replay failed %d\n",
- pf->vsi[i]->idx, ret);
+ vsi->idx, ret);
return -EIO;
}
/* Re-map HW VSI number, using VSI handle that has been
* previously validated in ice_replay_vsi() call above
*/
- pf->vsi[i]->vsi_num = ice_get_hw_vsi_num(hw, pf->vsi[i]->idx);
+ vsi->vsi_num = ice_get_hw_vsi_num(hw, vsi->idx);
dev_info(&pf->pdev->dev,
"VSI at index %d filter replayed successfully - vsi_num %i\n",
- pf->vsi[i]->idx, pf->vsi[i]->vsi_num);
+ vsi->idx, vsi->vsi_num);
}
/* Clean up replay filter after successful re-configuration */
--
2.21.0
^ 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