* latest netdev net-next kernel panic
From: Paweł Staszewski @ 2017-09-07 21:30 UTC (permalink / raw)
To: Linux Kernel Network Developers
I want to mention also here cause last trace for RIP was dst_dev_put
https://bugzilla.kernel.org/attachment.cgi?id=258259
^ permalink raw reply
* Re: [PATCH net-next] mdio_bus: Remove unneeded gpiod NULL check
From: Linus Walleij @ 2017-09-07 21:33 UTC (permalink / raw)
To: Woojung.Huh
Cc: Andrew Lunn, Fabio Estevam, Sergei Shtylyov, David S. Miller,
Florian Fainelli, netdev@vger.kernel.org, Fabio Estevam
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40B1B424@CHN-SV-EXMX02.mchp-main.com>
On Thu, Sep 7, 2017 at 11:30 PM, <Woojung.Huh@microchip.com> wrote:
>> If someone is using GPIO descriptors with GPIO disabled, i.e. calling
>> gpiod_get() and friends, this is very likely to be a bug, and what
>> the driver wants to do is:
>>
>> depends on GPIOLIB
>>
>> or
>>
>> select GPIOLIB
>>
>> in Kconfig. The whole optional thing is mainly a leftover from when it
>> was possible to have a local implementation of the GPIOLIB API in
>> some custom header file, noone sane should be doing that anymore,
>> and if they do, they can very well face the warnings.
>>
>> If someone is facing a lot of WARN_ON() messages to this, it is a clear
>> indication that they need to fix their Kconfig and in that case it is proper.
> Linus & Andrew,
>
> I knew that it is already in David's pulling request.
> Configuring GPIOLIB is the right solution even if platform doesn't use it?
I guess?
"Platform doesn't use it" what does that mean?
Does it mean it does not call the
APIs of the GPIOLIB, does it mean it doesn't have a GPIO driver
at probe (but may have one by having it probed from a module)
or does it mean the platform can never have it?
If it calls the APIs, it is using it.
Yours,
Linus Walleij
^ permalink raw reply
* Re: latest netdev net-next kernel panic
From: Eric Dumazet @ 2017-09-07 21:39 UTC (permalink / raw)
To: Paweł Staszewski; +Cc: Linux Kernel Network Developers
In-Reply-To: <9f4c89c9-1a1b-39ec-6d8c-2c9e4c15911a@itcare.pl>
On Thu, 2017-09-07 at 23:30 +0200, Paweł Staszewski wrote:
> I want to mention also here cause last trace for RIP was dst_dev_put
>
> https://bugzilla.kernel.org/attachment.cgi?id=258259
Can you provide a bit more context ?
Thanks !
^ permalink raw reply
* Re: [PATCH net-next] mdio_bus: Remove unneeded gpiod NULL check
From: Florian Fainelli @ 2017-09-07 21:39 UTC (permalink / raw)
To: Linus Walleij, Woojung.Huh
Cc: Andrew Lunn, Fabio Estevam, Sergei Shtylyov, David S. Miller,
netdev@vger.kernel.org, Fabio Estevam
In-Reply-To: <CACRpkdbK03cwzmGs=0nVeCp6NnX0-41dhEotxfDT46O4xDzagQ@mail.gmail.com>
On 09/07/2017 02:33 PM, Linus Walleij wrote:
> On Thu, Sep 7, 2017 at 11:30 PM, <Woojung.Huh@microchip.com> wrote:
>>> If someone is using GPIO descriptors with GPIO disabled, i.e. calling
>>> gpiod_get() and friends, this is very likely to be a bug, and what
>>> the driver wants to do is:
>>>
>>> depends on GPIOLIB
>>>
>>> or
>>>
>>> select GPIOLIB
>>>
>>> in Kconfig. The whole optional thing is mainly a leftover from when it
>>> was possible to have a local implementation of the GPIOLIB API in
>>> some custom header file, noone sane should be doing that anymore,
>>> and if they do, they can very well face the warnings.
>>>
>>> If someone is facing a lot of WARN_ON() messages to this, it is a clear
>>> indication that they need to fix their Kconfig and in that case it is proper.
>> Linus & Andrew,
>>
>> I knew that it is already in David's pulling request.
>> Configuring GPIOLIB is the right solution even if platform doesn't use it?
>
> I guess?
>
> "Platform doesn't use it" what does that mean?
>
> Does it mean it does not call the
> APIs of the GPIOLIB, does it mean it doesn't have a GPIO driver
> at probe (but may have one by having it probed from a module)
> or does it mean the platform can never have it?
I think it means CONFIG_GPIOLIB=n in the kernel because it's not needed,
yet you run code (like drivers/net/phy/mdio_bus.c) that unconditionally
calls into GPIOLIB and attempts to configure a given GPIO if available.
This thread is actually what prompted me to write this email:
https://lkml.org/lkml/2017/9/2/3
So that's the takeway should we just conditionalize all the GPIOLIB
calls from drivers/net/phy/mdio_bus.c under an #if
IS_ENABLED(CONFIG_GPIOLIB) of some sort?
>
> If it calls the APIs, it is using it.
>
It's more subtle than that, the GPIO resource may not be available just
like you have disabled support for GPIOLIB in the kernel, in which case,
the calls are still there, they default to their inline stubs, you get
warnings, everyone panics and screams.
--
Florian
^ permalink raw reply
* Re: [PATCH RFC 5/6] Switch SPI driver calls its own driver switch register function
From: Andrew Lunn @ 2017-09-07 21:40 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121A1B@CHN-SV-EXMX02.mchp-main.com>
On Thu, Sep 07, 2017 at 09:09:22PM +0000, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> SPI driver calls own specific switch register function.
> Shutdown callback function is added to reset switch to default state.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> ---
> diff --git a/drivers/net/dsa/microchip/ksz_spi.c b/drivers/net/dsa/microchip/ksz_spi.c
> index c519469..d03eb83 100644
> --- a/drivers/net/dsa/microchip/ksz_spi.c
> +++ b/drivers/net/dsa/microchip/ksz_spi.c
> @@ -25,6 +25,8 @@
>
> #include "ksz_priv.h"
>
> +int ksz9477_switch_register(struct ksz_device *dev);
Hi Tristram
This should be placed in a header file somewhere, not here.
> +static void ksz_spi_shutdown(struct spi_device *spi) {
> + struct ksz_device *dev = spi_get_drvdata(spi);
> +
> + if (dev)
> + dev->dev_ops->reset(dev);
shutdown seems like a better name for this op, not reset.
> +}
> +
> static const struct of_device_id ksz_dt_ids[] = {
> { .compatible = "microchip,ksz9477" },
> {},
> @@ -207,6 +217,7 @@ static int ksz_spi_remove(struct spi_device *spi)
> },
> .probe = ksz_spi_probe,
> .remove = ksz_spi_remove,
> + .shutdown = ksz_spi_shutdown,
> };
>
> module_spi_driver(ksz_spi_driver);
^ permalink raw reply
* Re: [PATCH RFC 6/6] Modify tag_ksz.c to support other KSZ switch drivers
From: Andrew Lunn @ 2017-09-07 21:48 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121A22@CHN-SV-EXMX02.mchp-main.com>
On Thu, Sep 07, 2017 at 09:09:30PM +0000, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> The KSZ tail tag code can be used by different KSZ switch drivers.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> ---
> diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c index 010ca0a..d5faf14 100644
> --- a/net/dsa/tag_ksz.c
> +++ b/net/dsa/tag_ksz.c
> @@ -13,35 +13,21 @@
> #include <linux/slab.h>
> #include <net/dsa.h>
> #include "dsa_priv.h"
> -
> -/* For Ingress (Host -> KSZ), 2 bytes are added before FCS.
> - * ---------------------------------------------------------------------------
> - * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes)
> - * ---------------------------------------------------------------------------
> - * tag0 : Prioritization (not used now)
> - * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x10=port5)
> - *
> - * For Egress (KSZ -> Host), 1 byte is added before FCS.
> - * ---------------------------------------------------------------------------
> - * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
> - * ---------------------------------------------------------------------------
> - * tag0 : zero-based value represents port
> - * (eg, 0x00=port1, 0x02=port3, 0x06=port7)
> - */
> -
> -#define KSZ_INGRESS_TAG_LEN 2
> -#define KSZ_EGRESS_TAG_LEN 1
> +#include "../../drivers/net/dsa/microchip/ksz_priv.h"
No.
Move the header file to somewhere under include/linux. You can split
it into private and public parts if you want, and just put the public
bits in include/linux.
> static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev) {
> struct dsa_slave_priv *p = netdev_priv(dev);
> + struct dsa_switch *ds = p->dp->ds;
> + struct ksz_device *swdev = ds->priv;
> struct sk_buff *nskb;
> + int len;
> int padlen;
> - u8 *tag;
>
> padlen = (skb->len >= ETH_ZLEN) ? 0 : ETH_ZLEN - skb->len;
>
> - if (skb_tailroom(skb) >= padlen + KSZ_INGRESS_TAG_LEN) {
> + len = swdev->dev_ops->get_tx_len(swdev);
This is ugly. We have a clean separation between a switch driver and a
tag driver. Here you are mixing them together. Don't. Look at the
mailing lists for what Florian and I suggested to Pavel. It will also
solve your include file issue above.
Andrew
^ permalink raw reply
* Re: latest netdev net-next kernel panic
From: Paweł Staszewski @ 2017-09-07 21:47 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Linux Kernel Network Developers
In-Reply-To: <1504820388.15310.71.camel@edumazet-glaptop3.roam.corp.google.com>
Hi Eric
today upgraded some host from 4.13.0-rc7+ to latest net-next from git
and run some tests simulate clients traffic 40Gbit / 5Mpps mixed tcp + udp
There is also bgpd running that feeds FIB with full bgp feed - about
700k prefixes (so routing tables is about 700k routes with about 300
nexthops)
There is also simulated about 1000 ip neigh hosts (arp) with about 300
ip interfaces assigned on vlans
With kernel 4.13.0-rc7+ (git from 05.09.2017) all tests are good
(besides that there is panic after about 10 hours - every time same like
in this link:
https://bugzilla.kernel.org/attachment.cgi?id=258257
But looks like acpi related or c/p states with new intel cpus... dono now.
W dniu 2017-09-07 o 23:39, Eric Dumazet pisze:
> On Thu, 2017-09-07 at 23:30 +0200, Paweł Staszewski wrote:
>> I want to mention also here cause last trace for RIP was dst_dev_put
>>
>> https://bugzilla.kernel.org/attachment.cgi?id=258259
> Can you provide a bit more context ?
>
> Thanks !
>
>
>
^ permalink raw reply
* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Guedes, Andre @ 2017-09-07 21:51 UTC (permalink / raw)
To: richardcochran@gmail.com, henrik@austad.us
Cc: jiri@resnulli.us, jhs@mojatatu.com, Ong, Boon Leong,
xiyou.wangcong@gmail.com, Gomes, Vinicius,
Sanchez-Palencia, Jesus, netdev@vger.kernel.org, Briano, Ivan,
intel-wired-lan@lists.osuosl.org
In-Reply-To: <20170907161838.GB9064@sisyphus.home.austad.us>
[-- Attachment #1: Type: text/plain, Size: 1077 bytes --]
On Thu, 2017-09-07 at 18:18 +0200, Henrik Austad wrote:
> On Thu, Sep 07, 2017 at 05:53:15PM +0200, Richard Cochran wrote:
> > On Thu, Sep 07, 2017 at 05:27:51PM +0200, Henrik Austad wrote:
> > > On Thu, Sep 07, 2017 at 02:40:18PM +0200, Richard Cochran wrote:
> > > And if you want to this driver to act as a bridge, how do you accomodate
> > > change in network requirements? (i.e. how does this work with switchdev?)
> >
> > To my understanding, this Qdisc idea provides QoS for the host's
> > transmitted traffic, and nothing more.
>
> Ok, then we're on the same page.
>
> > > - Or am I overthinking this?
> >
> > Being able to configure the external ports of a switchdev is probably
> > a nice feature, but that is another story. (But maybe I misunderstood
> > the authors' intent!)
>
> ok, chalk that one up for later perhaps
Just to clarify, we've been most focused on end-station use-cases. We've
considered some bridge use-cases as well just to verify that the proposed
design won't be an issue if someone else goes for it.
- Andre
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3262 bytes --]
^ permalink raw reply
* RE: [PATCH net-next] mdio_bus: Remove unneeded gpiod NULL check
From: Woojung.Huh @ 2017-09-07 21:51 UTC (permalink / raw)
To: f.fainelli, linus.walleij
Cc: andrew, festevam, sergei.shtylyov, davem, netdev, fabio.estevam
In-Reply-To: <81338659-4f46-0f2f-13c3-135b3ac876d7@gmail.com>
> >>> If someone is using GPIO descriptors with GPIO disabled, i.e. calling
> >>> gpiod_get() and friends, this is very likely to be a bug, and what
> >>> the driver wants to do is:
> >>>
> >>> depends on GPIOLIB
> >>>
> >>> or
> >>>
> >>> select GPIOLIB
> >>>
> >>> in Kconfig. The whole optional thing is mainly a leftover from when it
> >>> was possible to have a local implementation of the GPIOLIB API in
> >>> some custom header file, noone sane should be doing that anymore,
> >>> and if they do, they can very well face the warnings.
> >>>
> >>> If someone is facing a lot of WARN_ON() messages to this, it is a clear
> >>> indication that they need to fix their Kconfig and in that case it is proper.
> >> Linus & Andrew,
> >>
> >> I knew that it is already in David's pulling request.
> >> Configuring GPIOLIB is the right solution even if platform doesn't use it?
> >
> > I guess?
> >
> > "Platform doesn't use it" what does that mean?
> >
> > Does it mean it does not call the
> > APIs of the GPIOLIB, does it mean it doesn't have a GPIO driver
> > at probe (but may have one by having it probed from a module)
> > or does it mean the platform can never have it?
>
> I think it means CONFIG_GPIOLIB=n in the kernel because it's not needed,
> yet you run code (like drivers/net/phy/mdio_bus.c) that unconditionally
> calls into GPIOLIB and attempts to configure a given GPIO if available.
Yes. I'm facing issue on PC which won't need GPIOLIB as default.
Warning message goes away when GPIOLIB is enabled, and fortunately,
Ubuntu default config has it.
So, it may not be seen by many users when with full/default configuration.
> This thread is actually what prompted me to write this email:
>
> https://lkml.org/lkml/2017/9/2/3
Thanks for the link.
^ permalink raw reply
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added
From: Andrew Lunn @ 2017-09-07 21:54 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121A5B@CHN-SV-EXMX02.mchp-main.com>
> -- compatible: For external switch chips, compatible string must be exactly one
> - of: "microchip,ksz9477"
> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip,
> + "microchip,ksz8795" for KSZ8795 chip,
> + "microchip,ksz8794" for KSZ8794 chip,
> + "microchip,ksz8765" for KSZ8765 chip,
> + "microchip,ksz8895" for KSZ8895 chip,
> + "microchip,ksz8864" for KSZ8864 chip,
> + "microchip,ksz8873" for KSZ8873 chip,
> + "microchip,ksz8863" for KSZ8863 chip,
> + "microchip,ksz8463" for KSZ8463 chip
This part of this patch should be in a patch of the series that
actually adds support for these chips. Don't document chips until you
actually support them.
> See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional required and optional properties.
> @@ -13,60 +20,60 @@ Examples:
>
> Ethernet switch connected via SPI to the host, CPU port wired to eth0:
>
> - eth0: ethernet@10001000 {
> - fixed-link {
> - speed = <1000>;
> - full-duplex;
> - };
> - };
> + eth0: ethernet@10001000 {
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
>
> - spi1: spi@f8008000 {
> - pinctrl-0 = <&pinctrl_spi_ksz>;
> - cs-gpios = <&pioC 25 0>;
> - id = <1>;
> - status = "okay";
> + spi1: spi@f8008000 {
> + cs-gpios = <&pioC 25 0>;
> + id = <1>;
> + status = "okay";
>
> - ksz9477: ksz9477@0 {
> - compatible = "microchip,ksz9477";
> - reg = <0>;
> + ksz9477: ksz9477@0 {
> + compatible = "microchip,ksz9477";
> + reg = <0>;
>
> - spi-max-frequency = <44000000>;
> - spi-cpha;
> - spi-cpol;
> + spi-max-frequency = <44000000>;
> + spi-cpha;
> + spi-cpol;
> +
> + status = "okay";
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + label = "lan1";
> + };
> + port@1 {
> + reg = <1>;
> + label = "lan2";
> + };
> + port@2 {
> + reg = <2>;
> + label = "lan3";
> + };
> + port@3 {
> + reg = <3>;
> + label = "lan4";
> + };
> + port@4 {
> + reg = <4>;
> + label = "lan5";
> + };
> + port@5 {
> + reg = <5>;
> + label = "cpu";
> + ethernet = <ð0>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + };
> + };
> + };
>
> - status = "okay";
> - ports {
> - #address-cells = <1>;
> - #size-cells = <0>;
> - port@0 {
> - reg = <0>;
> - label = "lan1";
> - };
> - port@1 {
> - reg = <1>;
> - label = "lan2";
> - };
> - port@2 {
> - reg = <2>;
> - label = "lan3";
> - };
> - port@3 {
> - reg = <3>;
> - label = "lan4";
> - };
> - port@4 {
> - reg = <4>;
> - label = "lan5";
> - };
> - port@5 {
> - reg = <5>;
> - label = "cpu";
> - ethernet = <ð0>;
> - fixed-link {
> - speed = <1000>;
> - full-duplex;
> - };
> - };
> - };
> - };
> - };
This part however is a nice cleanup. You can submit this patch as a
separate patch, once netdev has opened again in about 10 days time.
Andrew
^ permalink raw reply
* Re: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile
From: Andrew Lunn @ 2017-09-07 21:55 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121A80@CHN-SV-EXMX02.mchp-main.com>
On Thu, Sep 07, 2017 at 09:17:10PM +0000, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Add KSZ8795 switch support with SPI access.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> ---
> diff --git a/drivers/net/dsa/microchip/Makefile b/drivers/net/dsa/microchip/Makefile
> index 0961c30..0d8ba48 100644
> --- a/drivers/net/dsa/microchip/Makefile
> +++ b/drivers/net/dsa/microchip/Makefile
> @@ -1,2 +1,4 @@
> obj-$(CONFIG_MICROCHIP_KSZ) += ksz9477.o ksz_common.o
> obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
> +obj-$(CONFIG_MICROCHIP_KSZ8795) += ksz8795.o ksz_common.o
> +obj-$(CONFIG_MICROCHIP_KSZ8795_SPI_DRIVER) += ksz8795_spi.o
I've not tried it, but i think this breaks the build
Andrew
^ permalink raw reply
* Re: latest netdev net-next kernel panic
From: Paweł Staszewski @ 2017-09-07 22:08 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Linux Kernel Network Developers
In-Reply-To: <2e7d813e-0ee9-ede6-7e0e-db2814b3cf94@itcare.pl>
Also this panic occured about 3 minutes after traffic rise to 40G
before host was working without trafic for about 30minutes - after
traffic start to flow this bug start.
Now i have this host without traffic and it is working for almost 1 hour :)
W dniu 2017-09-07 o 23:47, Paweł Staszewski pisze:
> Hi Eric
>
>
> today upgraded some host from 4.13.0-rc7+ to latest net-next from git
>
> and run some tests simulate clients traffic 40Gbit / 5Mpps mixed tcp +
> udp
>
> There is also bgpd running that feeds FIB with full bgp feed - about
> 700k prefixes (so routing tables is about 700k routes with about 300
> nexthops)
>
>
> There is also simulated about 1000 ip neigh hosts (arp) with about 300
> ip interfaces assigned on vlans
>
>
>
> With kernel 4.13.0-rc7+ (git from 05.09.2017) all tests are good
> (besides that there is panic after about 10 hours - every time same
> like in this link:
>
> https://bugzilla.kernel.org/attachment.cgi?id=258257
>
> But looks like acpi related or c/p states with new intel cpus... dono
> now.
>
>
>
>
> W dniu 2017-09-07 o 23:39, Eric Dumazet pisze:
>> On Thu, 2017-09-07 at 23:30 +0200, Paweł Staszewski wrote:
>>> I want to mention also here cause last trace for RIP was dst_dev_put
>>>
>>> https://bugzilla.kernel.org/attachment.cgi?id=258259
>> Can you provide a bit more context ?
>>
>> Thanks !
>>
>>
>>
>
>
^ permalink raw reply
* Re: [PATCH iproute2 3/4] devlink: Update devlink UAPI file
From: Stephen Hemminger @ 2017-09-07 22:12 UTC (permalink / raw)
To: Arkadi Sharshevsky; +Cc: netdev, davem, jiri, mlxsw, andrew, Jiri Pirko
In-Reply-To: <1504794403-45690-4-git-send-email-arkadis@mellanox.com>
On Thu, 7 Sep 2017 17:26:42 +0300
Arkadi Sharshevsky <arkadis@mellanox.com> wrote:
> Update devlink UAPI file.
>
> Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> include/linux/devlink.h | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/include/linux/devlink.h b/include/linux/devlink.h
> index 7644005..a62695e 100644
> --- a/include/linux/devlink.h
> +++ b/include/linux/devlink.h
> @@ -226,4 +226,22 @@ enum devlink_dpipe_action_type {
> DEVLINK_DPIPE_ACTION_TYPE_FIELD_MODIFY,
> };
>
> +enum devlink_dpipe_field_ethernet_id {
> + DEVLINK_DPIPE_FIELD_ETHERNET_DST_MAC,
> +};
> +
> +enum devlink_dpipe_field_ipv4_id {
> + DEVLINK_DPIPE_FIELD_IPV4_DST_IP,
> +};
> +
> +enum devlink_dpipe_field_ipv6_id {
> + DEVLINK_DPIPE_FIELD_IPV6_DST_IP,
> +};
> +
> +enum devlink_dpipe_header_id {
> + DEVLINK_DPIPE_HEADER_ETHERNET,
> + DEVLINK_DPIPE_HEADER_IPV4,
> + DEVLINK_DPIPE_HEADER_IPV6,
> +};
> +
> #endif /* _LINUX_DEVLINK_H_ */
This patch was unnecessary. Already had updated file present.
^ permalink raw reply
* Re: [PATCH iproute2 0/4] Add support for dpipe's global header formatting
From: Stephen Hemminger @ 2017-09-07 22:13 UTC (permalink / raw)
To: Arkadi Sharshevsky; +Cc: netdev, davem, jiri, mlxsw, andrew
In-Reply-To: <1504794403-45690-1-git-send-email-arkadis@mellanox.com>
On Thu, 7 Sep 2017 17:26:39 +0300
Arkadi Sharshevsky <arkadis@mellanox.com> wrote:
> Some dpipe's global header values need special formatting, for example
> Ethernet and IP addresses. This patchset adds support for IPv4/6 and
> Ethernet's special format.
>
> Arkadi Sharshevsky (4):
> devlink: Make match/action parsing more flexible
> devlink: Add support for special format protocol headers
> devlink: Update devlink UAPI file
> devlink: Add support for protocol IPv4/IPv6/Ethernet special formats
>
> devlink/devlink.c | 319 +++++++++++++++++++++++++++++++++++++-----------
> include/linux/devlink.h | 18 +++
> 2 files changed, 265 insertions(+), 72 deletions(-)
>
Applied 1,2 and 4. #3 was not needed.
^ permalink raw reply
* [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs
From: Daniel Borkmann @ 2017-09-07 22:14 UTC (permalink / raw)
To: davem; +Cc: ast, john.fastabend, brouer, andy, netdev, Daniel Borkmann
We can potentially run into a couple of issues with the XDP
bpf_redirect_map() helper. The ri->map in the per CPU storage
can become stale in several ways, mostly due to misuse, where
we can then trigger a use after free on the map:
i) prog A is calling bpf_redirect_map(), returning XDP_REDIRECT
and running on a driver not supporting XDP_REDIRECT yet. The
ri->map on that CPU becomes stale when the XDP program is unloaded
on the driver, and a prog B loaded on a different driver which
supports XDP_REDIRECT return code. prog B would have to omit
calling to bpf_redirect_map() and just return XDP_REDIRECT, which
would then access the freed map in xdp_do_redirect() since not
cleared for that CPU.
ii) prog A is calling bpf_redirect_map(), returning a code other
than XDP_REDIRECT. prog A is then detached, which triggers release
of the map. prog B is attached which, similarly as in i), would
just return XDP_REDIRECT without having called bpf_redirect_map()
and thus be accessing the freed map in xdp_do_redirect() since
not cleared for that CPU.
iii) prog A is attached to generic XDP, calling the bpf_redirect_map()
helper and returning XDP_REDIRECT. xdp_do_generic_redirect() is
currently not handling ri->map (will be fixed by Jesper), so it's
not being reset. Later loading a e.g. native prog B which would,
say, call bpf_xdp_redirect() and then returns XDP_REDIRECT would
find in xdp_do_redirect() that a map was set and uses that causing
use after free on map access.
Fix thus needs to avoid accessing stale ri->map pointers, naive
way would be to call a BPF function from drivers that just resets
it to NULL for all XDP return codes but XDP_REDIRECT and including
XDP_REDIRECT for drivers not supporting it yet (and let ri->map
being handled in xdp_do_generic_redirect()). There is a less
intrusive way w/o letting drivers call a reset for each BPF run.
The verifier knows we're calling into bpf_xdp_redirect_map()
helper, so it can do a small insn rewrite transparent to the prog
itself in the sense that it fills R4 with a pointer to the own
bpf_prog. We have that pointer at verification time anyway and
R4 is allowed to be used as per calling convention we scratch
R0 to R5 anyway, so they become inaccessible and program cannot
read them prior to a write. Then, the helper would store the prog
pointer in the current CPUs struct redirect_info. Later in
xdp_do_*_redirect() we check whether the redirect_info's prog
pointer is the same as passed xdp_prog pointer, and if that's
the case then all good, since the prog holds a ref on the map
anyway, so it is always valid at that point in time and must
have a reference count of at least 1. If in the unlikely case
they are not equal, it means we got a stale pointer, so we clear
and bail out right there. Also do reset map and the owning prog
in bpf_xdp_redirect(), so that bpf_xdp_redirect_map() and
bpf_xdp_redirect() won't get mixed up, only the last call should
take precedence. A tc bpf_redirect() doesn't use map anywhere
yet, so no need to clear it there since never accessed in that
layer.
Note that in case the prog is released, and thus the map as
well we're still under RCU read critical section at that time
and have preemption disabled as well. Once we commit with the
__dev_map_insert_ctx() from xdp_do_redirect_map() and set the
map to ri->map_to_flush, we still wait for a xdp_do_flush_map()
to finish in devmap dismantle time once flush_needed bit is set,
so that is fine.
Fixes: 97f91a7cf04f ("bpf: add bpf_redirect_map helper routine")
Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/verifier.c | 16 ++++++++++++++++
net/core/filter.c | 21 +++++++++++++++++++--
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d690c7d..477b693 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4203,6 +4203,22 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
continue;
}
+ if (insn->imm == BPF_FUNC_redirect_map) {
+ u64 addr = (unsigned long)prog;
+ struct bpf_insn r4_ld[] = {
+ BPF_LD_IMM64(BPF_REG_4, addr),
+ *insn,
+ };
+ cnt = ARRAY_SIZE(r4_ld);
+
+ new_prog = bpf_patch_insn_data(env, i + delta, r4_ld, cnt);
+ if (!new_prog)
+ return -ENOMEM;
+
+ delta += cnt - 1;
+ env->prog = prog = new_prog;
+ insn = new_prog->insnsi + i + delta;
+ }
patch_call_imm:
fn = prog->aux->ops->get_func_proto(insn->imm);
/* all functions that have prototype and verifier allowed
diff --git a/net/core/filter.c b/net/core/filter.c
index 5912c73..0848df2 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1794,6 +1794,7 @@ struct redirect_info {
u32 flags;
struct bpf_map *map;
struct bpf_map *map_to_flush;
+ const struct bpf_prog *map_owner;
};
static DEFINE_PER_CPU(struct redirect_info, redirect_info);
@@ -1807,7 +1808,6 @@ struct redirect_info {
ri->ifindex = ifindex;
ri->flags = flags;
- ri->map = NULL;
return TC_ACT_REDIRECT;
}
@@ -2504,6 +2504,7 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
struct bpf_prog *xdp_prog)
{
struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+ const struct bpf_prog *map_owner = ri->map_owner;
struct bpf_map *map = ri->map;
u32 index = ri->ifindex;
struct net_device *fwd;
@@ -2511,6 +2512,15 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
ri->ifindex = 0;
ri->map = NULL;
+ ri->map_owner = NULL;
+
+ /* This is really only caused by a deliberately crappy
+ * BPF program, normally we would never hit that case,
+ * so no need to inform someone via tracepoints either,
+ * just bail out.
+ */
+ if (unlikely(map_owner != xdp_prog))
+ return -EINVAL;
fwd = __dev_map_lookup_elem(map, index);
if (!fwd) {
@@ -2607,6 +2617,8 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
ri->ifindex = ifindex;
ri->flags = flags;
+ ri->map = NULL;
+ ri->map_owner = NULL;
return XDP_REDIRECT;
}
@@ -2619,7 +2631,8 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
.arg2_type = ARG_ANYTHING,
};
-BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex, u64, flags)
+BPF_CALL_4(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex, u64, flags,
+ const struct bpf_prog *, map_owner)
{
struct redirect_info *ri = this_cpu_ptr(&redirect_info);
@@ -2629,10 +2642,14 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
ri->ifindex = ifindex;
ri->flags = flags;
ri->map = map;
+ ri->map_owner = map_owner;
return XDP_REDIRECT;
}
+/* Note, arg4 is hidden from users and populated by the verifier
+ * with the right pointer.
+ */
static const struct bpf_func_proto bpf_xdp_redirect_map_proto = {
.func = bpf_xdp_redirect_map,
.gpl_only = false,
--
1.9.3
^ permalink raw reply related
* RE: [Intel-wired-lan] [PATCH] e1000e: changed some expensive calls of udelay to usleep_range
From: Brown, Aaron F @ 2017-09-07 22:19 UTC (permalink / raw)
To: Pavel Machek, Matthew Tan, intel-wired-lan@lists.osuosl.org
Cc: michael.kardonik@nxp.com, Williams, Mitch A,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20170904162555.GB21849@amd>
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> Behalf Of Pavel Machek
> Sent: Monday, September 4, 2017 9:26 AM
> To: Matthew Tan <matthew.tan_1@nxp.com>
> Cc: michael.kardonik@nxp.com; Williams, Mitch A
> <mitch.a.williams@intel.com>; linux-kernel@vger.kernel.org;
> john.ronciak@intel.com; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org
> Subject: Re: [Intel-wired-lan] [PATCH] e1000e: changed some expensive calls
> of udelay to usleep_range
>
> Hi!
>
> > @@ -183,7 +183,7 @@ s32 e1000e_read_phy_reg_mdic(struct e1000_hw
> *hw, u32 offset, u16 *data)
> > * reading duplicate data in the next MDIC transaction.
> > */
> > if (hw->mac.type == e1000_pch2lan)
> > - udelay(100);
> > + usleep_range(90, 100);
> >
> > return 0;
> > }
>
> Can you explain why shortening the delay is acceptable here?
Maybe it's not.
This patch is causing speed / duplex tests to fail on several of my test systems. Specifically a Lenova laptop with an 82577 and a NUC with an i218 (though that does not mean it is limited to those or that it's not related to the individual link partner.)
In both cases they "blow up" when attempting to link at 10 Mb with a Call Trace on the console / log and a watchdog: BUG: soft lockup - CPU#X stuck appearing in the current login session. The simplest way to produce the crash is simply load the interface and attempt to bring it up with the link partner configured to 10 Mb half (force or autoneg) or forced to 10 Mb full (autoneg to 10 Mb full did not blow up.) The problem is also appearing at some, but not all 100 Mb speeds and duplexes. 100 Mb also crashes, apparently with the same pattern. With the link partner set to 100 Mb half Autoneg on, 100 Mb half forced and 100 Mb full forced the system crashes, 100 Full autoneg seems to work fine.
The call trace repeats every 30 seconds, is captured in the log and looks a lot like this:
----------------------------------------
Sep 7 14:17:10 u1459 kernel: watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [kworker/2:0:23]
Sep 7 14:17:10 u1459 kernel: Modules linked in: e1000e ptp pps_core ip6table_filter ip6_tables ebtable_nat ebtables nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack libcrc32c ipt_REJECT nf_reject_ipv4 xt_CHECKSUM iptable_mangle iptable_filter ip_tables bridge stp llc nfsd lockd grace nfs_acl auth_rpcgss sunrpc autofs4 ipv6 crc_ccitt dm_mirror dm_region_hash dm_log vhost_net vhost tap tun kvm_intel kvm irqbypass uinput joydev ax88179_178a usbnet mii iTCO_wdt iTCO_vendor_support sg i2c_i801 lpc_ich mfd_core xhci_pci snd_hda_codec_realtek xhci_hcd snd_hda_codec_hdmi snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd soundcore dm_mod(E) dax(E) ext4(E) jbd2(E) mbcache(E) sd_mod(E) ahci(E) libahci(E) i915(E) drm_kms_helper
(E) drm(E) fb_sys_fops(E)
Sep 7 14:17:10 u1459 kernel: sysimgblt(E) sysfillrect(E) syscopyarea(E) i2c_algo_bit(E) i2c_core(E) iosf_mbi(E) video(E)
Sep 7 14:17:10 u1459 kernel: CPU: 2 PID: 23 Comm: kworker/2:0 Tainted: G EL 4.13.0-rc6_next-queue_dev-queue_regress #38
Sep 7 14:17:10 u1459 kernel: Hardware name: /NUC5i5MYBE, BIOS MYBDWi5v.86A.0030.2016.0527.1657 05/27/2016
Sep 7 14:17:10 u1459 kernel: Workqueue: events linkwatch_event
Sep 7 14:17:10 u1459 kernel: task: ffff88024cf84680 task.stack: ffffc90000d44000
Sep 7 14:17:10 u1459 kernel: RIP: 0010:queued_spin_lock_slowpath+0x56/0x1d0
Sep 7 14:17:10 u1459 kernel: RSP: 0018:ffffc90000d478c8 EFLAGS: 00000202 ORIG_RAX: ffffffffffffff10
Sep 7 14:17:10 u1459 kernel: RAX: 0000000000000101 RBX: ffff880239fe8000 RCX: 0000000000000101
Sep 7 14:17:10 u1459 kernel: RDX: ffffc90000d47948 RSI: 0000000000000001 RDI: ffff880239feb1a0
Sep 7 14:17:10 u1459 kernel: RBP: ffffc90000d47968 R08: 0000000000000001 R09: ffff88024ce181a4
Sep 7 14:17:10 u1459 kernel: R10: 0000000000000000 R11: 0000000000000000 R12: ffff880239fe8840
Sep 7 14:17:10 u1459 kernel: R13: ffff88024ce180e4 R14: ffff88024ce180e4 R15: ffffc90000d47a48
Sep 7 14:17:10 u1459 kernel: FS: 0000000000000000(0000) GS:ffff880255d00000(0000) knlGS:0000000000000000
Sep 7 14:17:10 u1459 kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
Sep 7 14:17:10 u1459 kernel: CR2: 00007ffd4c3579b8 CR3: 0000000001c09000 CR4: 00000000003406e0
Sep 7 14:17:10 u1459 kernel: DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
Sep 7 14:17:10 u1459 kernel: DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Sep 7 14:17:10 u1459 kernel: Call Trace:
Sep 7 14:17:10 u1459 kernel: ? netlink_broadcast_filtered+0x15f/0x1a0
Sep 7 14:17:10 u1459 kernel: _raw_spin_lock+0x21/0x30
Sep 7 14:17:10 u1459 kernel: e1000e_get_stats64+0x2b/0x140 [e1000e]
Sep 7 14:17:10 u1459 kernel: dev_get_stats+0x3d/0xc0
Sep 7 14:17:10 u1459 kernel: ? __nla_reserve+0x53/0x70
Sep 7 14:17:10 u1459 kernel: rtnl_fill_stats+0x6b/0x150
Sep 7 14:17:10 u1459 kernel: rtnl_fill_ifinfo+0x542/0x920
Sep 7 14:17:10 u1459 kernel: ? if_nlmsg_size+0xae/0x180
Sep 7 14:17:10 u1459 kernel: rtmsg_ifinfo_build_skb+0x90/0xe0
Sep 7 14:17:10 u1459 kernel: rtmsg_ifinfo_event+0x35/0x60
Sep 7 14:17:10 u1459 kernel: rtmsg_ifinfo+0x13/0x20
Sep 7 14:17:10 u1459 kernel: netdev_state_change+0x46/0x50
Sep 7 14:17:10 u1459 kernel: linkwatch_do_dev+0x3c/0x60
Sep 7 14:17:10 u1459 kernel: __linkwatch_run_queue+0x13a/0x210
Sep 7 14:17:10 u1459 kernel: ? set_task_cpu+0xbb/0x1a0
Sep 7 14:17:10 u1459 kernel: ? lock_timer_base+0x8a/0xb0
Sep 7 14:17:10 u1459 kernel: linkwatch_event+0x25/0x30
Sep 7 14:17:10 u1459 kernel: process_one_work+0x193/0x410
Sep 7 14:17:10 u1459 kernel: ? try_to_del_timer_sync+0x53/0x70
Sep 7 14:17:10 u1459 kernel: ? del_timer_sync+0x3d/0x50
Sep 7 14:17:10 u1459 kernel: ? maybe_create_worker+0x8b/0x110
Sep 7 14:17:10 u1459 kernel: ? schedule+0x3a/0xa0
Sep 7 14:17:10 u1459 kernel: worker_thread+0x7c/0x420
Sep 7 14:17:10 u1459 kernel: ? default_wake_function+0x12/0x20
Sep 7 14:17:10 u1459 kernel: ? maybe_create_worker+0x110/0x110
Sep 7 14:17:10 u1459 kernel: ? schedule+0x3a/0xa0
Sep 7 14:17:10 u1459 kernel: ? maybe_create_worker+0x110/0x110
Sep 7 14:17:10 u1459 kernel: kthread+0x111/0x150
Sep 7 14:17:10 u1459 kernel: ? __kthread_init_worker+0x40/0x40
Sep 7 14:17:10 u1459 kernel: ret_from_fork+0x22/0x30
Sep 7 14:17:10 u1459 kernel: Code: ff ff 75 57 83 fe 01 89 ca 89 f0 41 0f 45 d0 f0 0f b1 17 39 f0 75 e3 83 fa 01 74 1c 8b 07 48 8d 55 e0 84 c0 89 45 e0 74 0a f3 90 <8b> 07 84 c0 89 02 75 f6 66 c7 07 01 00 c9 c3 8b 37 48 8d 45 a0
Sep 7 14:17:38 u1459 kernel: watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [kworker/2:0:23]
Sep 7 14:17:38 u1459 kernel: Modules linked in: e1000e ptp pps_core ip6table_filter ip6_tables ebtable_nat ebtables nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack libcrc32c ipt_REJECT nf_reject_ipv4 xt_CHECKSUM iptable_mangle iptable_filter ip_tables bridge stp llc nfsd lockd grace nfs_acl auth_rpcgss sunrpc autofs4 ipv6 crc_ccitt dm_mirror dm_region_hash dm_log vhost_net vhost tap tun kvm_intel kvm irqbypass uinput joydev ax88179_178a usbnet mii iTCO_wdt iTCO_vendor_support sg i2c_i801 lpc_ich mfd_core xhci_pci snd_hda_codec_realtek xhci_hcd snd_hda_codec_hdmi snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd soundcore dm_mod(E) dax(E) ext4(E) jbd2(E) mbcache(E) sd_mod(E) ahci(E) libahci(E) i915(E) drm_kms_helper
(E) drm(E) fb_sys_fops(E)
Sep 7 14:17:38 u1459 kernel: sysimgblt(E) sysfillrect(E) syscopyarea(E) i2c_algo_bit(E) i2c_core(E) iosf_mbi(E) video(E)
Sep 7 14:17:38 u1459 kernel: CPU: 2 PID: 23 Comm: kworker/2:0 Tainted: G EL 4.13.0-rc6_next-queue_dev-queue_regress #38
Sep 7 14:17:38 u1459 kernel: Hardware name: /NUC5i5MYBE, BIOS MYBDWi5v.86A.0030.2016.0527.1657 05/27/2016
Sep 7 14:17:38 u1459 kernel: Workqueue: events linkwatch_event
Sep 7 14:17:38 u1459 kernel: task: ffff88024cf84680 task.stack: ffffc90000d44000
Sep 7 14:17:38 u1459 kernel: RIP: 0010:queued_spin_lock_slowpath+0x56/0x1d0
Sep 7 14:17:38 u1459 kernel: RSP: 0018:ffffc90000d478c8 EFLAGS: 00000202 ORIG_RAX: ffffffffffffff10
Sep 7 14:17:38 u1459 kernel: RAX: 0000000000000101 RBX: ffff880239fe8000 RCX: 0000000000000101
Sep 7 14:17:38 u1459 kernel: RDX: ffffc90000d47948 RSI: 0000000000000001 RDI: ffff880239feb1a0
Sep 7 14:17:38 u1459 kernel: RBP: ffffc90000d47968 R08: 0000000000000001 R09: ffff88024ce181a4
Sep 7 14:17:38 u1459 kernel: R10: 0000000000000000 R11: 0000000000000000 R12: ffff880239fe8840
Sep 7 14:17:38 u1459 kernel: R13: ffff88024ce180e4 R14: ffff88024ce180e4 R15: ffffc90000d47a48
Sep 7 14:17:38 u1459 kernel: FS: 0000000000000000(0000) GS:ffff880255d00000(0000) knlGS:0000000000000000
Sep 7 14:17:38 u1459 kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
Sep 7 14:17:38 u1459 kernel: CR2: 00007ffd4c3579b8 CR3: 0000000001c09000 CR4: 00000000003406e0
Sep 7 14:17:38 u1459 kernel: DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
Sep 7 14:17:38 u1459 kernel: DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Sep 7 14:17:38 u1459 kernel: Call Trace:
Sep 7 14:17:38 u1459 kernel: ? netlink_broadcast_filtered+0x15f/0x1a0
Sep 7 14:17:38 u1459 kernel: _raw_spin_lock+0x21/0x30
Sep 7 14:17:38 u1459 kernel: e1000e_get_stats64+0x2b/0x140 [e1000e]
Sep 7 14:17:38 u1459 kernel: dev_get_stats+0x3d/0xc0
Sep 7 14:17:38 u1459 kernel: ? __nla_reserve+0x53/0x70
Sep 7 14:17:38 u1459 kernel: rtnl_fill_stats+0x6b/0x150
Sep 7 14:17:38 u1459 kernel: rtnl_fill_ifinfo+0x542/0x920
Sep 7 14:17:38 u1459 kernel: ? if_nlmsg_size+0xae/0x180
Sep 7 14:17:38 u1459 kernel: rtmsg_ifinfo_build_skb+0x90/0xe0
Sep 7 14:17:38 u1459 kernel: rtmsg_ifinfo_event+0x35/0x60
Sep 7 14:17:38 u1459 kernel: rtmsg_ifinfo+0x13/0x20
Sep 7 14:17:38 u1459 kernel: netdev_state_change+0x46/0x50
Sep 7 14:17:38 u1459 kernel: linkwatch_do_dev+0x3c/0x60
Sep 7 14:17:38 u1459 kernel: __linkwatch_run_queue+0x13a/0x210
Sep 7 14:17:38 u1459 kernel: ? set_task_cpu+0xbb/0x1a0
Sep 7 14:17:38 u1459 kernel: ? lock_timer_base+0x8a/0xb0
Sep 7 14:17:38 u1459 kernel: linkwatch_event+0x25/0x30
Sep 7 14:17:38 u1459 kernel: process_one_work+0x193/0x410
Sep 7 14:17:38 u1459 kernel: ? try_to_del_timer_sync+0x53/0x70
Sep 7 14:17:38 u1459 kernel: ? del_timer_sync+0x3d/0x50
Sep 7 14:17:38 u1459 kernel: ? maybe_create_worker+0x8b/0x110
Sep 7 14:17:38 u1459 kernel: ? schedule+0x3a/0xa0
Sep 7 14:17:38 u1459 kernel: worker_thread+0x7c/0x420
Sep 7 14:17:38 u1459 kernel: ? default_wake_function+0x12/0x20
Sep 7 14:17:38 u1459 kernel: ? maybe_create_worker+0x110/0x110
Sep 7 14:17:38 u1459 kernel: ? schedule+0x3a/0xa0
Sep 7 14:17:38 u1459 kernel: ? maybe_create_worker+0x110/0x110
Sep 7 14:17:38 u1459 kernel: kthread+0x111/0x150
Sep 7 14:17:38 u1459 kernel: ? __kthread_init_worker+0x40/0x40
Sep 7 14:17:38 u1459 kernel: ret_from_fork+0x22/0x30
Sep 7 14:17:38 u1459 kernel: Code: ff ff 75 57 83 fe 01 89 ca 89 f0 41 0f 45 d0 f0 0f b1 17 39 f0 75 e3 83 fa 01 74 1c 8b 07 48 8d 55 e0 84 c0 89 45 e0 74 0a f3 90 <8b> 07 84 c0 89 02 75 f6 66 c7 07 01 00 c9 c3 8b 37 48 8d 45 a0
----------------------------------------
Here is the contents of an ssh session to the box (using a usb ethernet port for a control network as the LOM goes dead.) :
----------------------
u1459:[0]/root> modprobe e1000e
u1459:[0]/root> nwk
Bringing up eth0... -> success
Bringing up eth1... -> success
Bringing up eth1 IPv6... -> success
u1459:[0]/root> lnks
Message from syslogd@u1459 at Sep 7 14:04:42 ...
kernel:watchdog: BUG: soft lockup - CPU#0 stuck for 23s! [ifconfig:3980]
Message from syslogd@u1459 at Sep 7 14:04:42 ...
kernel:watchdog: BUG: soft lockup - CPU#3 stuck for 22s! [ifconfig:3983]
Write failed: Broken pipe
[root@s252 ~]# ssh u1459
Warning: Permanently added 'u1459,190.0.14.59' (RSA) to the list of known hosts.
Last login: Thu Sep 7 14:12:06 2017
You are running LINUX 4.13.0-rc6_next-queue_dev-queue_regress.
This system was (post)installed: Fri Jan 27 16:38:47 PST 2017
It has been more than 7 days since you ran 'sgetall' on this system!
Your root drive is 11% used.
You have 4 CPUs on-line.
The current ethtool is 4.11
Your ethool is 4.8
To get the current version, run 'ethtool.inst'
u1459:[0]/root> dmesg -c > /dev/null
u1459:[0]/root> modprobe e1000e
u1459:[0]/root> dmesg -c
pps_core: LinuxPPS API ver. 1 registered
pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti <giometti@linux.it>
PTP clock support registered
e1000e: Intel(R) PRO/1000 Network Driver - 3.2.6-k
e1000e: Copyright(c) 1999 - 2015 Intel Corporation.
e1000e 0000:00:19.0: Interrupt Throttling Rate (ints/sec) set to dynamic conservative mode
e1000e 0000:00:19.0 0000:00:19.0 (uninitialized): registered PHC clock
e1000e 0000:00:19.0 eth1: (PCI Express:2.5GT/s:Width x1) f4:4d:30:62:c7:14
e1000e 0000:00:19.0 eth1: Intel(R) PRO/1000 Network Connection
e1000e 0000:00:19.0 eth1: MAC: 11, PHY: 12, PBA No: FFFFFF-0FF
u1459:[0]/root> ifconfig eth1 190.1.14.59
u1459:[0]/root>
Message from syslogd@u1459 at Sep 7 14:15:06 ...
kernel:watchdog: BUG: soft lockup - CPU#2 stuck for 23s! [kworker/2:0:23]
Message from syslogd@u1459 at Sep 7 14:15:34 ...
kernel:watchdog: BUG: soft lockup - CPU#2 stuck for 23s! [kworker/2:0:23]
Message from syslogd@u1459 at Sep 7 14:16:06 ...
kernel:watchdog: BUG: soft lockup - CPU#2 stuck for 23s! [kworker/2:0:23]
Message from syslogd@u1459 at Sep 7 14:16:34 ...
kernel:watchdog: BUG: soft lockup - CPU#2 stuck for 23s! [kworker/2:0:23]
Message from syslogd@u1459 at Sep 7 14:17:10 ...
kernel:watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [kworker/2:0:23]
Message from syslogd@u1459 at Sep 7 14:17:38 ...
kernel:watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [kworker/2:0:23]
^ permalink raw reply
* RE: [PATCH RFC 2/6] Create new file ksz9477.c from KSZ9477 code in ksz_common.c
From: Tristram.Ha @ 2017-09-07 22:20 UTC (permalink / raw)
To: andrew
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <20170907213242.GR11248@lunn.ch>
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, September 07, 2017 2:33 PM
> To: Tristram Ha - C24268
> Cc: muvarov@gmail.com; pavel@ucw.cz; nathan.leigh.conrad@gmail.com;
> vivien.didelot@savoirfairelinux.com; f.fainelli@gmail.com;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC 2/6] Create new file ksz9477.c from KSZ9477 code in
> ksz_common.c
>
> On Thu, Sep 07, 2017 at 09:09:04PM +0000, Tristram.Ha@microchip.com wrote:
> > From: Tristram Ha <Tristram.Ha@microchip.com>
> >
> > Create new ksz9477.c file from original ksz_common.c.
> >
> > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> > ---
> > diff --git a/drivers/net/dsa/microchip/ksz9477.c
> > b/drivers/net/dsa/microchip/ksz9477.c
> > new file mode 100644
> > index 0000000..bc722b4
> > --- /dev/null
> > +++ b/drivers/net/dsa/microchip/ksz9477.c
> > @@ -0,0 +1,1317 @@
> > +/*
> > + * Microchip switch driver main logic
> > + *
> > + * Copyright (C) 2017
> > + *
> > + * Permission to use, copy, modify, and/or distribute this software
> > +for any
> > + * purpose with or without fee is hereby granted, provided that the
> > +above
>
> Tristram
>
> It looks like something hand mangled this comment. "any" and "above"
> appear to be on a line on there own.
>
> > + * copyright notice and this permission notice appear in all copies.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
> > +WARRANTIES
> > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES
> OF
> > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE
> > +LIABLE FOR
> > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY
> > +DAMAGES
> > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER
> IN
> > +AN
> > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> ARISING
> > +OUT OF
> > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/export.h>
> > +#include <linux/gpio.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_data/microchip-ksz.h>
> > +#include <linux/phy.h>
> > +#include <linux/etherdevice.h>
> > +#include <linux/if_bridge.h>
> > +#include <net/dsa.h>
> > +#include <net/switchdev.h>
> > +
> > +#include "ksz_priv.h"
> > +#include "ksz_9477_reg.h"
> > +
> > +static const struct {
> > + int index;
> > + char string[ETH_GSTRING_LEN];
> > +} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
> > + { 0x00, "rx_hi" },
> > + { 0x01, "rx_undersize" },
> > + { 0x02, "rx_fragments" },
> > + { 0x03, "rx_oversize" },
> > + { 0x04, "rx_jabbers" },
> > + { 0x05, "rx_symbol_err" },
> > + { 0x06, "rx_crc_err" },
> > + { 0x07, "rx_align_err" },
> > + { 0x08, "rx_mac_ctrl" },
> > + { 0x09, "rx_pause" },
> > + { 0x0A, "rx_bcast" },
> > + { 0x0B, "rx_mcast" },
> > + { 0x0C, "rx_ucast" },
> > + { 0x0D, "rx_64_or_less" },
> > + { 0x0E, "rx_65_127" },
> > + { 0x0F, "rx_128_255" },
> > + { 0x10, "rx_256_511" },
> > + { 0x11, "rx_512_1023" },
> > + { 0x12, "rx_1024_1522" },
> > + { 0x13, "rx_1523_2000" },
> > + { 0x14, "rx_2001" },
> > + { 0x15, "tx_hi" },
> > + { 0x16, "tx_late_col" },
> > + { 0x17, "tx_pause" },
> > + { 0x18, "tx_bcast" },
> > + { 0x19, "tx_mcast" },
> > + { 0x1A, "tx_ucast" },
> > + { 0x1B, "tx_deferred" },
> > + { 0x1C, "tx_total_col" },
> > + { 0x1D, "tx_exc_col" },
> > + { 0x1E, "tx_single_col" },
> > + { 0x1F, "tx_mult_col" },
> > + { 0x80, "rx_total" },
> > + { 0x81, "tx_total" },
> > + { 0x82, "rx_discards" },
> > + { 0x83, "tx_discards" },
> > +};
> > +
> > +static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool
> > +set) {
>
> More mangling? Should set be on the end of the previous line?
>
> > +static void read_table(struct dsa_switch *ds, u32 *table) {
> > + struct ksz_device *dev = ds->priv;
> > +
> > + ksz_read32(dev, REG_SW_ALU_VAL_A, &table[0]);
> > + ksz_read32(dev, REG_SW_ALU_VAL_B, &table[1]);
> > + ksz_read32(dev, REG_SW_ALU_VAL_C, &table[2]);
> > + ksz_read32(dev, REG_SW_ALU_VAL_D, &table[3]); }
> > +
> > +static void write_table(struct dsa_switch *ds, u32 *table) {
> > + struct ksz_device *dev = ds->priv;
> > +
> > + ksz_write32(dev, REG_SW_ALU_VAL_A, table[0]);
> > + ksz_write32(dev, REG_SW_ALU_VAL_B, table[1]);
> > + ksz_write32(dev, REG_SW_ALU_VAL_C, table[2]);
> > + ksz_write32(dev, REG_SW_ALU_VAL_D, table[3]); }
>
> More mangling? } at the end of a line?
>
> I will stop reading now and wait for a v2 this is not corrupt.
>
> Andrew
Sorry about that. It seems my e-mail system wraps the line too soon.
^ permalink raw reply
* RE: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ switch drivers.
From: Tristram.Ha @ 2017-09-07 22:26 UTC (permalink / raw)
To: andrew
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <20170907212449.GQ11248@lunn.ch>
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, September 07, 2017 2:25 PM
> To: Tristram Ha - C24268
> Cc: muvarov@gmail.com; pavel@ucw.cz; nathan.leigh.conrad@gmail.com;
> vivien.didelot@savoirfairelinux.com; f.fainelli@gmail.com;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ
> switch drivers.
>
> On Thu, Sep 07, 2017 at 09:08:58PM +0000, Tristram.Ha@microchip.com wrote:
> > From: Tristram Ha <Tristram.Ha@microchip.com>
> >
> > Break ksz_common.c into 2 files so that the common code can be used by other
> KSZ switch drivers.
> >
> > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> > ---
> > diff --git a/drivers/net/dsa/microchip/Makefile
> > b/drivers/net/dsa/microchip/Makefile
> > index ed335e2..0961c30 100644
> > --- a/drivers/net/dsa/microchip/Makefile
> > +++ b/drivers/net/dsa/microchip/Makefile
> > @@ -1,2 +1,2 @@
> > -obj-$(CONFIG_MICROCHIP_KSZ) += ksz_common.o
> > +obj-$(CONFIG_MICROCHIP_KSZ) += ksz9477.o ksz_common.o
> > obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
>
> Hi Tristram
>
> I would of thought this would break the build. You don't add ksz9477.c until the
> next patch.
>
> Each patch needs to compile, otherwise you break git bisect.
>
> Andrew
Eventually the file will need to be broken in two, so you would like to see all 3 changes (Makefile, ksz_common.c, and ksz9477.c) in 1 patch file?
^ permalink raw reply
* RE: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile
From: Tristram.Ha @ 2017-09-07 22:29 UTC (permalink / raw)
To: andrew
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <20170907215551.GV11248@lunn.ch>
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, September 07, 2017 2:56 PM
> To: Tristram Ha - C24268
> Cc: muvarov@gmail.com; pavel@ucw.cz; nathan.leigh.conrad@gmail.com;
> vivien.didelot@savoirfairelinux.com; f.fainelli@gmail.com;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile
>
> On Thu, Sep 07, 2017 at 09:17:10PM +0000, Tristram.Ha@microchip.com wrote:
> > From: Tristram Ha <Tristram.Ha@microchip.com>
> >
> > Add KSZ8795 switch support with SPI access.
> >
> > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> > ---
> > diff --git a/drivers/net/dsa/microchip/Makefile
> b/drivers/net/dsa/microchip/Makefile
> > index 0961c30..0d8ba48 100644
> > --- a/drivers/net/dsa/microchip/Makefile
> > +++ b/drivers/net/dsa/microchip/Makefile
> > @@ -1,2 +1,4 @@
> > obj-$(CONFIG_MICROCHIP_KSZ) += ksz9477.o ksz_common.o
> > obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
> > +obj-$(CONFIG_MICROCHIP_KSZ8795) += ksz8795.o ksz_common.o
> > +obj-$(CONFIG_MICROCHIP_KSZ8795_SPI_DRIVER) += ksz8795_spi.o
>
> I've not tried it, but i think this breaks the build
>
> Andrew
So you would like to have all 5 patches in 1 patch file?
^ permalink raw reply
* Re: [PATCH RFC 3/5] Add KSZ8795 switch driver
From: Andrew Lunn @ 2017-09-07 22:36 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121A87@CHN-SV-EXMX02.mchp-main.com>
On Thu, Sep 07, 2017 at 09:17:16PM +0000, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Add KSZ8795 switch support with function code.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> ---
> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> new file mode 100644
> index 0000000..e4d4e6a
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -0,0 +1,2066 @@
> +/*
> + * Microchip KSZ8795 switch driver
> + *
> + * Copyright (C) 2017 Microchip Technology Inc.
> + * Tristram Ha <Tristram.Ha@microchip.com>
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/export.h>
> +#include <linux/gpio.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_data/microchip-ksz.h>
> +#include <linux/phy.h>
> +#include <linux/etherdevice.h>
> +#include <linux/if_bridge.h>
> +#include <net/dsa.h>
> +#include <net/switchdev.h>
> +
> +#include "ksz_priv.h"
> +#include "ksz8795.h"
> +
> +/**
> + * Some counters do not need to be read too often because they are less likely
> + * to increase much.
> + */
What does comment mean? Are you caching statistics, and updating
different values at different rates?
> +static const struct {
> + int interval;
> + char string[ETH_GSTRING_LEN];
> +} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
> + { 1, "rx_hi" },
> + { 4, "rx_undersize" },
> + { 4, "rx_fragments" },
> + { 4, "rx_oversize" },
> + { 4, "rx_jabbers" },
> + { 4, "rx_symbol_err" },
> + { 4, "rx_crc_err" },
> + { 4, "rx_align_err" },
> + { 4, "rx_mac_ctrl" },
> + { 1, "rx_pause" },
> + { 1, "rx_bcast" },
> + { 1, "rx_mcast" },
> + { 1, "rx_ucast" },
> + { 2, "rx_64_or_less" },
> + { 2, "rx_65_127" },
> + { 2, "rx_128_255" },
> + { 2, "rx_256_511" },
> + { 2, "rx_512_1023" },
> + { 2, "rx_1024_1522" },
> + { 2, "rx_1523_2000" },
> + { 2, "rx_2001" },
> + { 1, "tx_hi" },
> + { 4, "tx_late_col" },
> + { 1, "tx_pause" },
> + { 1, "tx_bcast" },
> + { 1, "tx_mcast" },
> + { 1, "tx_ucast" },
> + { 4, "tx_deferred" },
> + { 4, "tx_total_col" },
> + { 4, "tx_exc_col" },
> + { 4, "tx_single_col" },
> + { 4, "tx_mult_col" },
> + { 1, "rx_total" },
> + { 1, "tx_total" },
> + { 2, "rx_discards" },
> + { 2, "tx_discards" },
> +};
> +
> +static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set)
> +{
> + u8 data;
> +
> + ksz_read8(dev, addr, &data);
> + if (set)
> + data |= bits;
> + else
> + data &= ~bits;
> + ksz_write8(dev, addr, data);
> +}
Shouldn't this function be in the shared code? There is an exact copy
currently in ksz_common.c.
> +static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
> + bool set)
> +{
> + u32 addr;
> + u8 data;
> +
> + addr = PORT_CTRL_ADDR(port, offset);
> + ksz_read8(dev, addr, &data);
> +
> + if (set)
> + data |= bits;
> + else
> + data &= ~bits;
> +
> + ksz_write8(dev, addr, data);
> +}
And this also appears to be shared.
> +static void port_r_mib_cnt(struct ksz_device *dev, int port, u16 addr, u64 *cnt)
It would be good to use the ksz8795_ prefix for functions specific to
the KSZ8795.
> +{
> + u32 data;
> + u16 ctrl_addr;
> + u8 check;
> + int timeout;
> +
> + ctrl_addr = addr + SWITCH_COUNTER_NUM * port;
> +
> + ctrl_addr |= IND_ACC_TABLE(TABLE_MIB | TABLE_READ);
> + mutex_lock(&dev->stats_mutex);
> + ksz_write16(dev, REG_IND_CTRL_0, ctrl_addr);
> +
> + for (timeout = 1; timeout > 0; timeout--) {
A rather short timeount!
> + ksz_read8(dev, REG_IND_MIB_CHECK, &check);
> +
> + if (check & MIB_COUNTER_VALID) {
> + ksz_read32(dev, REG_IND_DATA_LO, &data);
> + if (check & MIB_COUNTER_OVERFLOW)
> + *cnt += MIB_COUNTER_VALUE + 1;
> + *cnt += data & MIB_COUNTER_VALUE;
> + break;
> + }
Should there be a dev_err() here about timing out?
> + }
> + mutex_unlock(&dev->stats_mutex);
> +}
> +
> +static int valid_dyn_entry(struct ksz_device *dev, u8 *data)
> +{
> + int timeout = 100;
> +
> + do {
> + ksz_read8(dev, REG_IND_DATA_CHECK, data);
> + timeout--;
> + } while ((*data & DYNAMIC_MAC_TABLE_NOT_READY) && timeout);
> +
> + /* Entry is not ready for accessing. */
> + if (*data & DYNAMIC_MAC_TABLE_NOT_READY) {
> + return 1;
Use standard error code. -ETIMEOUT
> + /* Entry is ready for accessing. */
> + } else {
> + ksz_read8(dev, REG_IND_DATA_8, data);
> +
> + /* There is no valid entry in the table. */
> + if (*data & DYNAMIC_MAC_TABLE_MAC_EMPTY)
> + return 2;
-EINVAL?
> + }
> + return 0;
> +}
It also looks like these return values get propagated further. So you
really should be using error code.
> +static int ksz_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
> + u8 *fid, u8 *src_port, u8 *timestamp,
> + u16 *entries)
> +{
> + u32 data_hi;
> + u32 data_lo;
> + u16 ctrl_addr;
> + int rc;
> + u8 data;
> +
> + ctrl_addr = IND_ACC_TABLE(TABLE_DYNAMIC_MAC | TABLE_READ) | addr;
> +
> + ksz_write16(dev, REG_IND_CTRL_0, ctrl_addr);
> +
> + rc = valid_dyn_entry(dev, &data);
> + if (rc == 1) {
> + if (addr == 0)
> + *entries = 0;
> + } else if (rc == 2) {
> + *entries = 0;
> + /* At least one valid entry in the table. */
> + } else {
> + u64 buf;
> +
> + dev->ops->get(dev, REG_IND_DATA_HI, &buf, sizeof(buf));
> + buf = be64_to_cpu(buf);
> + data_hi = (u32)(buf >> 32);
> + data_lo = (u32)buf;
> +
> + /* Check out how many valid entry in the table. */
> + *entries = (u16)(((((u16)
> + data & DYNAMIC_MAC_TABLE_ENTRIES_H) <<
> + DYNAMIC_MAC_ENTRIES_H_S) |
> + (((data_hi & DYNAMIC_MAC_TABLE_ENTRIES) >>
> + DYNAMIC_MAC_ENTRIES_S))) + 1);
When i see so many ((((( i think this needs splitting up to make it
readable.
> +
> + *fid = (u8)((data_hi & DYNAMIC_MAC_TABLE_FID) >>
> + DYNAMIC_MAC_FID_S);
> + *src_port = (u8)((data_hi & DYNAMIC_MAC_TABLE_SRC_PORT) >>
> + DYNAMIC_MAC_SRC_PORT_S);
> + *timestamp = (u8)((
> + data_hi & DYNAMIC_MAC_TABLE_TIMESTAMP) >>
> + DYNAMIC_MAC_TIMESTAMP_S);
Are all the casts needed? Please go through all the code and see about
removing all the unneeded casts.
> +
> + return rc;
> +}
> +
> +struct alu_struct {
> + u8 is_static:1;
> + u8 prio_age:3;
> + u8 is_override:1;
> + u8 is_use_fid:1;
> + u8 port_forward:5;
> + u8 fid:7;
> + u8 mac[ETH_ALEN];
> +};
> +
> +static int ksz_r_sta_mac_table(struct ksz_device *dev, u16 addr,
> + struct alu_struct *alu)
> +{
> + u64 data;
> + u32 data_hi;
> + u32 data_lo;
> +
> + ksz_r_table_64(dev, TABLE_STATIC_MAC, addr, &data);
> + data_hi = data >> 32;
> + data_lo = (u32)data;
> + if (data_hi & (STATIC_MAC_TABLE_VALID | STATIC_MAC_TABLE_OVERRIDE)) {
> + alu->mac[5] = (u8)data_lo;
> + alu->mac[4] = (u8)(data_lo >> 8);
> + alu->mac[3] = (u8)(data_lo >> 16);
> + alu->mac[2] = (u8)(data_lo >> 24);
> + alu->mac[1] = (u8)data_hi;
> + alu->mac[0] = (u8)(data_hi >> 8);
> + alu->port_forward =
> + (u8)((data_hi & STATIC_MAC_TABLE_FWD_PORTS) >>
> + STATIC_MAC_FWD_PORTS_S);
> + alu->is_override =
> + (data_hi & STATIC_MAC_TABLE_OVERRIDE) ? 1 : 0;
> + data_hi >>= 1;
> + alu->is_use_fid = (data_hi & STATIC_MAC_TABLE_USE_FID) ? 1 : 0;
> + alu->fid = (u8)((data_hi & STATIC_MAC_TABLE_FID) >>
> + STATIC_MAC_FID_S);
> + return 0;
> + }
> + return -1;
And what does -1 mean?
> +static void ksz_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
> +{
> + struct ksz_port *port;
> + u8 ctrl;
> + u8 restart;
> + u8 link;
> + u8 speed;
> + u8 force;
> + u8 p = phy;
> + u16 data = 0;
> + int processed = true;
> +
> + port = &dev->ports[p];
> + switch (reg) {
> + case PHY_REG_CTRL:
> + ksz_pread8(dev, p, P_LOCAL_CTRL, &ctrl);
> + ksz_pread8(dev, p, P_NEG_RESTART_CTRL, &restart);
> + ksz_pread8(dev, p, P_SPEED_STATUS, &speed);
> + ksz_pread8(dev, p, P_FORCE_CTRL, &force);
> + if (restart & PORT_PHY_LOOPBACK)
> + data |= PHY_LOOPBACK;
> + if (force & PORT_FORCE_100_MBIT)
> + data |= PHY_SPEED_100MBIT;
> + if (!(force & PORT_AUTO_NEG_DISABLE))
> + data |= PHY_AUTO_NEG_ENABLE;
> + if (restart & PORT_POWER_DOWN)
> + data |= PHY_POWER_DOWN;
Same questions i asked Pavel
Is there a way to access the PHY registers over SPI? The datasheet
suggests there is, but it does not say how, as far as i can tell.
Ideally, we want to use just a normal PHY driver.
> +static int ksz_sset_count(struct dsa_switch *ds)
> +{
> + return TOTAL_SWITCH_COUNTER_NUM;
> +}
> +
> +static void ksz_get_strings(struct dsa_switch *ds, int port, uint8_t *buf)
> +{
> + int i;
> +
> + for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
> + memcpy(buf + i * ETH_GSTRING_LEN, mib_names[i].string,
> + ETH_GSTRING_LEN);
> + }
> +}
> +
> +static void ksz_get_ethtool_stats(struct dsa_switch *ds, int port,
> + uint64_t *buf)
> +{
> + struct ksz_device *dev = ds->priv;
> + struct ksz_port_mib *mib;
> + struct ksz_port_mib_info *info;
> + int i;
> +
> + mib = &dev->ports[port].mib;
> +
> + spin_lock(&dev->mib_read_lock);
> + for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
> + info = &mib->info[i];
> + buf[i] = info->counter;
> + info->read_cnt += info->read_max;
> + }
> + spin_unlock(&dev->mib_read_lock);
> +
> + mib->ctrl = 1;
> + schedule_work(&dev->mib_read);
Humm. I want to take a closer look at this caching code...
> +}
> +
> +static u8 STP_MULTICAST_ADDR[] = {
> + 0x01, 0x80, 0xC2, 0x00, 0x00, 0x00
> +};
This could be const. And this is not python. Global variables don't
need to be all capitals.
Andrew
^ permalink raw reply
* Re: [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs
From: Alexei Starovoitov @ 2017-09-07 22:37 UTC (permalink / raw)
To: Daniel Borkmann, davem; +Cc: john.fastabend, brouer, andy, netdev
In-Reply-To: <01d69254802986ad3a8b18a8650c45df3df95def.1504821825.git.daniel@iogearbox.net>
On 9/7/17 3:14 PM, Daniel Borkmann wrote:
> Fixes: 97f91a7cf04f ("bpf: add bpf_redirect_map helper routine")
> Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> kernel/bpf/verifier.c | 16 ++++++++++++++++
> net/core/filter.c | 21 +++++++++++++++++++--
> 2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index d690c7d..477b693 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -4203,6 +4203,22 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
> continue;
> }
>
> + if (insn->imm == BPF_FUNC_redirect_map) {
> + u64 addr = (unsigned long)prog;
> + struct bpf_insn r4_ld[] = {
> + BPF_LD_IMM64(BPF_REG_4, addr),
> + *insn,
> + };
> + cnt = ARRAY_SIZE(r4_ld);
> +
> + new_prog = bpf_patch_insn_data(env, i + delta, r4_ld, cnt);
that's a neat trick.
I think we'll be seeing more of such pattern in the future.
Definitely less intrusive fix than asking drivers or net/core
to clear it.
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ switch drivers.
From: Andrew Lunn @ 2017-09-07 22:40 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121AFB@CHN-SV-EXMX02.mchp-main.com>
> > > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> > > ---
> > > diff --git a/drivers/net/dsa/microchip/Makefile
> > > b/drivers/net/dsa/microchip/Makefile
> > > index ed335e2..0961c30 100644
> > > --- a/drivers/net/dsa/microchip/Makefile
> > > +++ b/drivers/net/dsa/microchip/Makefile
> > > @@ -1,2 +1,2 @@
> > > -obj-$(CONFIG_MICROCHIP_KSZ) += ksz_common.o
> > > +obj-$(CONFIG_MICROCHIP_KSZ) += ksz9477.o ksz_common.o
> > > obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
> >
> > Hi Tristram
> >
> > I would of thought this would break the build. You don't add ksz9477.c until the
> > next patch.
> >
> > Each patch needs to compile, otherwise you break git bisect.
> >
> > Andrew
>
> Eventually the file will need to be broken in two, so you would like
> to see all 3 changes (Makefile, ksz_common.c, and ksz9477.c) in 1
> patch file?
You cannot break the build. Each patch must compile on its own.
Breaking changes up into smaller chunks is good. Makes it easier to
review. So think about how you can do it without breaking the build,
but have smaller changes. For example, move a group of functions at a
time?
Andrew
^ permalink raw reply
* Re: latest netdev net-next kernel panic
From: Paweł Staszewski @ 2017-09-07 22:38 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Linux Kernel Network Developers
In-Reply-To: <91ae50fc-300a-2b92-7de0-ae512f364280@itcare.pl>
What is weird - changed in bios and enabled p-states and no panic from
30 minutes
w
00:38:07 up 33 min, 2 users, load average: 0.10, 0.13, 0.17
USER TTY LOGIN@ IDLE JCPU PCPU WHAT
So the problem can be related to some cpu weird stuff with c/p states or
it is random.
W dniu 2017-09-08 o 00:08, Paweł Staszewski pisze:
> Also this panic occured about 3 minutes after traffic rise to 40G
>
> before host was working without trafic for about 30minutes - after
> traffic start to flow this bug start.
>
> Now i have this host without traffic and it is working for almost 1
> hour :)
>
>
>
> W dniu 2017-09-07 o 23:47, Paweł Staszewski pisze:
>> Hi Eric
>>
>>
>> today upgraded some host from 4.13.0-rc7+ to latest net-next from git
>>
>> and run some tests simulate clients traffic 40Gbit / 5Mpps mixed tcp
>> + udp
>>
>> There is also bgpd running that feeds FIB with full bgp feed - about
>> 700k prefixes (so routing tables is about 700k routes with about 300
>> nexthops)
>>
>>
>> There is also simulated about 1000 ip neigh hosts (arp) with about
>> 300 ip interfaces assigned on vlans
>>
>>
>>
>> With kernel 4.13.0-rc7+ (git from 05.09.2017) all tests are good
>> (besides that there is panic after about 10 hours - every time same
>> like in this link:
>>
>> https://bugzilla.kernel.org/attachment.cgi?id=258257
>>
>> But looks like acpi related or c/p states with new intel cpus... dono
>> now.
>>
>>
>>
>>
>> W dniu 2017-09-07 o 23:39, Eric Dumazet pisze:
>>> On Thu, 2017-09-07 at 23:30 +0200, Paweł Staszewski wrote:
>>>> I want to mention also here cause last trace for RIP was dst_dev_put
>>>>
>>>> https://bugzilla.kernel.org/attachment.cgi?id=258259
>>> Can you provide a bit more context ?
>>>
>>> Thanks !
>>>
>>>
>>>
>>
>>
>
>
^ permalink raw reply
* Re: [PATCH RFC 2/6] Create new file ksz9477.c from KSZ9477 code in ksz_common.c
From: Andrew Lunn @ 2017-09-07 22:41 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121AEB@CHN-SV-EXMX02.mchp-main.com>
> Sorry about that. It seems my e-mail system wraps the line too soon.
git send-email should solve your problems.
Andrew
^ permalink raw reply
* Re: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile
From: Andrew Lunn @ 2017-09-07 22:43 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121B0A@CHN-SV-EXMX02.mchp-main.com>
On Thu, Sep 07, 2017 at 10:29:34PM +0000, Tristram.Ha@microchip.com wrote:
> > -----Original Message-----
> > From: Andrew Lunn [mailto:andrew@lunn.ch]
> > Sent: Thursday, September 07, 2017 2:56 PM
> > To: Tristram Ha - C24268
> > Cc: muvarov@gmail.com; pavel@ucw.cz; nathan.leigh.conrad@gmail.com;
> > vivien.didelot@savoirfairelinux.com; f.fainelli@gmail.com;
> > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> > Subject: Re: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile
> >
> > On Thu, Sep 07, 2017 at 09:17:10PM +0000, Tristram.Ha@microchip.com wrote:
> > > From: Tristram Ha <Tristram.Ha@microchip.com>
> > >
> > > Add KSZ8795 switch support with SPI access.
> > >
> > > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> > > ---
> > > diff --git a/drivers/net/dsa/microchip/Makefile
> > b/drivers/net/dsa/microchip/Makefile
> > > index 0961c30..0d8ba48 100644
> > > --- a/drivers/net/dsa/microchip/Makefile
> > > +++ b/drivers/net/dsa/microchip/Makefile
> > > @@ -1,2 +1,4 @@
> > > obj-$(CONFIG_MICROCHIP_KSZ) += ksz9477.o ksz_common.o
> > > obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
> > > +obj-$(CONFIG_MICROCHIP_KSZ8795) += ksz8795.o ksz_common.o
> > > +obj-$(CONFIG_MICROCHIP_KSZ8795_SPI_DRIVER) += ksz8795_spi.o
> >
> > I've not tried it, but i think this breaks the build
> >
> > Andrew
>
> So you would like to have all 5 patches in 1 patch file?
Or maybe this one last? Would that stop the build from breaking?
Andrew
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox