* Re: bug: 'ethtool -m' reports spurious alarm & warning threshold values for QSFP28 transceivers
From: Andrew Lunn @ 2018-09-27 14:52 UTC (permalink / raw)
To: Eran Ben Elisha
Cc: Neil Horman, Chris Preimesberger, linville@tuxdriver.com,
netdev@vger.kernel.org
In-Reply-To: <091551a9-7a85-4112-bbe2-ce7c7d3cf964@mellanox.com>
> Both drivers read up to 256 bytes. 0-127 (from page 0). and 128-256 (from
> page 0). Driver is not capable of reading over 256 bytes currently.
Hi Erin
There should not be any need to read more than 256 bytes. For older
SFP devices, two addresses on the i2c bus are used, each with 256
bytes. For QSFP, one address is used, and you swap page by writing to
offset 127.
> looking on qsfp.c parser in ethtool.c (user space), I see an uninitialized
> bug issue that have caused bug #1 + #2.
> Applied it locally solved the issue (Not showing alarm data, which should be
> expected as driver do not fill it).
There appears to be a second bug somewhere. dumping the module info
using HEX returned 256 bytes. But the binary dump had more bytes.
Since you have the hardware, could you look into this?
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: change bridge maintainers
From: Roopa Prabhu @ 2018-09-27 14:43 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev, Stephen Hemminger
In-Reply-To: <20180927084701.3468-1-sthemmin@microsoft.com>
On Thu, Sep 27, 2018 at 1:47 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> I haven't been doing reviews only but not active development on bridge
> code for several years. Roopa and Nikolay have been doing most of
> the new features and have agreed to take over as new co-maintainers.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Thanks
^ permalink raw reply
* Re: [RFC PATCH iproute2-next V2] System specification exception API
From: Jiri Pirko @ 2018-09-27 14:34 UTC (permalink / raw)
To: Eran Ben Elisha
Cc: netdev, Jakub Kicinski, Jiri Pirko, Stephen Hemminger,
Andrew Lunn, Tobin C. Harding, Ariel Almog, Tal Alon
In-Reply-To: <8a064f7f-298e-9cbe-d58a-fdf3d79eef24@mellanox.com>
Thu, Sep 27, 2018 at 04:02:48PM CEST, eranbe@mellanox.com wrote:
>
>
>On 9/27/2018 3:47 PM, Jiri Pirko wrote:
>> Wed, Sep 26, 2018 at 01:52:58PM CEST, eranbe@mellanox.com wrote:
>> > The exception spec is targeted for Real Time Alerting, in order to know when
>> > something bad had happened to a PCI device
>> > - Provide alert debug information
>> > - Self healing
>> > - If problem needs vendor support, provide a way to gather all needed debugging
>> > information.
>> >
>> > The exception mechanism contains condition checkers which sense for malfunction. Upon a condition hit,
>> > actions such as logs and correction can be taken.
>> >
>> > The condition checkers are divided into the following groups
>> > - Hardware - a checker which is triggered by the device due to
>> > malfunction.
>> > - Software - a checker which is triggered by the software due to
>> > malfunction.
>>
>> What do you mean by a "software malfunction", a "FW malfunction"?
>> Also, I don't see this 2 groups in the man.
>
>Software malfunction can be a Transmit error (caused by bad send request).
Sorry, but I still don't undestand what "software malfuntion" are you
talking about. Could you be more specific please?
>FW/HW malfunction can be any catastrophic error report (the ones that should
>be exposed to driver).
>The comment here was to highlight that we can support different kinds of
>condition groups.
>If for a specific condition, we will need to highlight it is SW/HW, we can
>concatenate it to its name.
>
>Eran
>
>>
>>
>> > Both groups of condition checkers can be triggered due to error event or due to a periodic check.
>> >
>> > Actions are the way to handle those events. Action can be in one of the
>> > following groups:
>> > - Dump - SW trace, SW dump, HW trace, HW dump
>> > - Reset - Surgical correction (e.g. modify Q, flush Q, reset of device, etc)
>> > Actions can be performed by SW or HW.
>> >
>> > User is allowed to enable or disable condition checkers and its action mapping.
>> >
>> > This RFC man page patch describes the suggested API of devlink-exception in order
>> > to control conditions and actions.
>> >
>> > V2:
>> > * Renaming terms:
>> > health -> exception
>> > sensor -> condition
>> > * Remove reinit command and merge with action command.
>> > * Consmetics in grammer.
>> >
>> > Eran Ben Elisha (1):
>> > man: Add devlink exception man page
>> >
>> > man/man8/devlink-exception.8 | 158 +++++++++++++++++++++++++++++++++++++++++++
>> > 1 file changed, 158 insertions(+)
>> > create mode 100644 man/man8/devlink-exception.8
>> >
>> > --
>> > 1.8.3.1
>> >
^ permalink raw reply
* [PATCH net-next] net: sched: make function qdisc_free_cb() static
From: Wei Yongjun @ 2018-09-27 14:47 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, Vlad Buslov
Cc: Wei Yongjun, netdev, kernel-janitors
Fixes the following sparse warning:
net/sched/sch_generic.c:944:6: warning:
symbol 'qdisc_free_cb' was not declared. Should it be static?
Fixes: 3a7d0d07a386 ("net: sched: extend Qdisc with rcu")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
net/sched/sch_generic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 531fac1..3023929 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -941,7 +941,7 @@ void qdisc_free(struct Qdisc *qdisc)
kfree((char *) qdisc - qdisc->padded);
}
-void qdisc_free_cb(struct rcu_head *head)
+static void qdisc_free_cb(struct rcu_head *head)
{
struct Qdisc *q = container_of(head, struct Qdisc, rcu);
^ permalink raw reply related
* Re: [RFC PATCH iproute2-next V2] man: Add devlink exception man page
From: Jiri Pirko @ 2018-09-27 14:32 UTC (permalink / raw)
To: Eran Ben Elisha
Cc: netdev, Jakub Kicinski, Jiri Pirko, Stephen Hemminger,
Andrew Lunn, Tobin C. Harding, Ariel Almog, Tal Alon
In-Reply-To: <1537962779-8910-2-git-send-email-eranbe@mellanox.com>
Wed, Sep 26, 2018 at 01:52:59PM CEST, eranbe@mellanox.com wrote:
>Add devlink-exception man page. Devlink-exception tool will control device
>exception attributes, conditions, actions and logging.
>
>Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
>
>-------------------------------------------------------
>Copy paste man output to here for easier review process of the RFC.
>
>DEVLINK-EXCEPTION(8) Linux DEVLINK-EXCEPTION(8)
>
>NAME
> devlink-exception - devlink exception configuration
>
>SYNOPSIS
> devlink [ OPTIONS ] exception { COMMAND | help }
>
> OPTIONS := { -V[ersion] | -n[no-nice-names] }
>
> devlink exception show [ DEV ] [ condition NAME ] [ action NAME ]
>
> devlink exception condition set DEV name NAME [ action NAME { active | inactive } ]
>
> devlink exception action set DEV name NAME period PERIOD count COUNT fail { ignore | down }
>
> devlink exception help
>
>DESCRIPTION
> devlink-exception tool allows user to configure the way driver treats unexpected status. The tool allows configuration of the conditions that can trigger exception activity. Set for each condition the follow up opera‐
> tions, such as, reset and dump of info. In addition, set the exception activity termination action.
>
> devlink exception show - Display devlink exception conditions and actions attributes
> DEV Specifies the devlink device to show.
>
> condition NAME
> Specifies the devlink condition to show.
>
> action NAME
> Specifies the devlink action to show.
>
> devlink exception condition set - sets devlink exception condition attributes
> DEV Specifies the devlink device to set.
>
> name NAME
> Name of the condition to set.
>
> action NAME { active | inactive }
> Specify which actions to activate and which to deactivate once a condition was triggered. Actions can be dump, reset, etc.
>
> devlink exception action set - sets devlink action attributes.
> Once this command is launched, period and count measurement will be reset.
>
> DEV Specifies the devlink device to set.
>
> name NAME
> Specifies the devlink action to set.
>
> period PERIOD
> The period on which we limit the amount of performed actions, measured in seconds.
>
> count COUNT
> The maximum number of actions performed in a limited time frame.
>
> fail { ignore | down }
> Specify the behavior once count limit was reached.
>
> ignore - Skip triggering this action.
>
> down - Driver will remain in nonoperational state.
>
>EXAMPLES
> devlink exception show
> Shows the exception state of all devlink devices on the system.
>
> devlink exception show pci/0000:01:00.0
> Shows the exception state of specified devlink device.
>
> devlink exception condition set pci/0000:01:00.0 name TX_COMP_ERROR action reset off action dump on
> Sets TX_COMP_ERROR condition parameters for a specific device.
>
> devlink exception action set pci/0000:01:00.0 name reset period 3600 count 5 fail ignore
> Sets exception attributes for reset action. Period timer and counter are being reset.
Looks good to me. But still, I need the code so I can play with it, to
see the outputs etc.
Thanks!
>
>SEE ALSO
> devlink(8), devlink-port(8), devlink-sb(8), devlink-monitor(8), devlink-dev(8),
>
>AUTHOR
> Eran ben Elisha <eranbe@mellanox.com>
>
>iproute2 15 Aug 2018 DEVLINK-EXCEPTION(8)
>
>---
> man/man8/devlink-exception.8 | 158 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 158 insertions(+)
> create mode 100644 man/man8/devlink-exception.8
>
>diff --git a/man/man8/devlink-exception.8 b/man/man8/devlink-exception.8
>new file mode 100644
>index 000000000000..03f24b32cc98
>--- /dev/null
>+++ b/man/man8/devlink-exception.8
>@@ -0,0 +1,158 @@
>+.TH DEVLINK\-EXCEPTION 8 "15 Aug 2018" "iproute2" "Linux"
>+.SH NAME
>+devlink-exception \- devlink exception configuration
>+.SH SYNOPSIS
>+.sp
>+.ad l
>+.in +8
>+.ti -8
>+.B devlink
>+.RI "[ " OPTIONS " ]"
>+.BR exception
>+.RI " { " COMMAND " | "
>+.BR help " }"
>+.sp
>+
>+.ti -8
>+.IR OPTIONS " := { "
>+\fB\-V\fR[\fIersion\fR] |
>+\fB\-n\fR[\fIno-nice-names\fR] }
>+
>+.ti -8
>+.B devlink exception show
>+.RI "[ " DEV " ]"
>+.RI "[ "
>+.B condition
>+.IR NAME
>+.RI "]"
>+.RI "[ "
>+.B action
>+.IR NAME
>+.RI "]"
>+
>+.ti -8
>+.B devlink exception condition set
>+.IR DEV
>+.B name
>+.IR NAME
>+.RI "[ "
>+.BR action
>+.IR NAME
>+.R "{" active "|" inactive "}" ]
>+
>+.ti -8
>+.B devlink exception action set
>+.IR DEV
>+.B name
>+.IR NAME
>+.BR period
>+.IR PERIOD
>+.BR count
>+.IR COUNT
>+.BR fail " { "
>+.IR ignore
>+.BR "| "
>+.IR down
>+.R "} "
>+
>+.ti -8
>+.B devlink exception help
>+
>+.SH "DESCRIPTION"
>+.B devlink-exception
>+tool allows user to configure the way driver treats unexpected status. The tool allows configuration of the conditions that can trigger exception activity. Set for each condition the follow up operations, such as, reset and dump of info. In addition, set the exception activity termination action.
>+
>+.SS devlink exception show - Display devlink exception conditions and actions attributes
>+.TP
>+.BI "DEV"
>+Specifies the devlink device to show.
>+
>+.PP
>+.TP
>+.BI condition " NAME"
>+Specifies the devlink condition to show.
>+
>+.TP
>+.BI action " NAME"
>+Specifies the devlink action to show.
>+
>+.SS devlink exception condition set - sets devlink exception condition attributes
>+
>+.TP
>+.B "DEV"
>+Specifies the devlink device to set.
>+
>+.TP
>+.BI name " NAME"
>+Name of the condition to set.
>+
>+.TP
>+.BR action
>+.IR NAME
>+.R "{" active "|" inactive "} "
>+.in +4
>+Specify which actions to activate and which to deactivate once a condition was triggered. Actions can be dump, reset, etc.
>+
>+.SS devlink exception action set - sets devlink action attributes.
>+Once this command is launched, period and count measurement will be reset.
>+
>+.TP
>+.B "DEV"
>+Specifies the devlink device to set.
>+
>+.TP
>+.BI name " NAME"
>+Specifies the devlink action to set.
>+
>+.TP
>+.BI period " PERIOD"
>+The period on which we limit the amount of performed actions, measured in seconds.
>+
>+.TP
>+.BI count " COUNT"
>+The maximum number of actions performed in a limited time frame.
>+
>+.TP
>+.BR fail
>+.R "{" ignore "|" down "}"
>+.in +4
>+Specify the behavior once count limit was reached.
>+
>+.I ignore
>+- Skip triggering this action.
>+
>+.I down
>+- Driver will remain in nonoperational state.
>+
>+.SH "EXAMPLES"
>+.PP
>+devlink exception show
>+.RS 4
>+Shows the exception state of all devlink devices on the system.
>+.RE
>+.PP
>+devlink exception show pci/0000:01:00.0
>+.RS 4
>+Shows the exception state of specified devlink device.
>+.RE
>+.PP
>+devlink exception condition set pci/0000:01:00.0 name TX_COMP_ERROR action reset off action dump on
>+.RS 4
>+Sets TX_COMP_ERROR condition parameters for a specific device.
>+.RE
>+.PP
>+devlink exception action set pci/0000:01:00.0 name reset period 3600 count 5 fail ignore
>+.RS 4
>+Sets exception attributes for reset action. Period timer and counter are being reset.
>+.RE
>+
>+.SH SEE ALSO
>+.BR devlink (8),
>+.BR devlink-port (8),
>+.BR devlink-sb (8),
>+.BR devlink-monitor (8),
>+.BR devlink-dev (8),
>+.br
>+
>+.SH AUTHOR
>+Eran ben Elisha <eranbe@mellanox.com>
>--
>1.8.3.1
>
^ permalink raw reply
* Re: [PATCH v3] PCI: Reprogram bridge prefetch registers on resume
From: Bjorn Helgaas @ 2018-09-27 20:52 UTC (permalink / raw)
To: Daniel Drake
Cc: andy.shevchenko-VuQAYsv1563Yd54FQh9/CA,
linux-pm-u79uwXL29TY76Z2rM5mHXA, linux-pci-u79uwXL29TY76Z2rM5mHXA,
rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ,
keith.busch-ral2JQCrhuEAvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, rchang-eYqpPyKDWXRBDgjK7y7TUQ,
linux-6IF/jdPJHihWk0Htik3J/w, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
jonathan.derrick-ral2JQCrhuEAvxtiuMwx3w,
hkallweit1-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20180918213244.GE13616-1RhO1Y9PlrlHTL0Zs8A6p5iNqAH0jzoTYJqu5kTmcBRl57MIdRCFDg@public.gmane.org>
[+cc LKML]
On Tue, Sep 18, 2018 at 04:32:44PM -0500, Bjorn Helgaas wrote:
> On Thu, Sep 13, 2018 at 11:37:45AM +0800, Daniel Drake wrote:
> > On 38+ Intel-based Asus products, the nvidia GPU becomes unusable
> > after S3 suspend/resume. The affected products include multiple
> > generations of nvidia GPUs and Intel SoCs. After resume, nouveau logs
> > many errors such as:
> >
> > fifo: fault 00 [READ] at 0000005555555000 engine 00 [GR] client 04
> > [HUB/FE] reason 4a [] on channel -1 [007fa91000 unknown]
> > DRM: failed to idle channel 0 [DRM]
> >
> > Similarly, the nvidia proprietary driver also fails after resume
> > (black screen, 100% CPU usage in Xorg process). We shipped a sample
> > to Nvidia for diagnosis, and their response indicated that it's a
> > problem with the parent PCI bridge (on the Intel SoC), not the GPU.
> >
> > Runtime suspend/resume works fine, only S3 suspend is affected.
> >
> > We found a workaround: on resume, rewrite the Intel PCI bridge
> > 'Prefetchable Base Upper 32 Bits' register (PCI_PREF_BASE_UPPER32). In
> > the cases that I checked, this register has value 0 and we just have to
> > rewrite that value.
> >
> > Linux already saves and restores PCI config space during suspend/resume,
> > but this register was being skipped because upon resume, it already
> > has value 0 (the correct, pre-suspend value).
> >
> > Intel appear to have previously acknowledged this behaviour and the
> > requirement to rewrite this register.
> > https://bugzilla.kernel.org/show_bug.cgi?id=116851#c23
> >
> > Based on that, rewrite the prefetch register values even when that
> > appears unnecessary.
> >
> > We have confirmed this solution on all the affected models we have
> > in-hands (X542UQ, UX533FD, X530UN, V272UN).
> >
> > Additionally, this solves an issue where r8169 MSI-X interrupts were
> > broken after S3 suspend/resume on Asus X441UAR. This issue was recently
> > worked around in commit 7bb05b85bc2d ("r8169: don't use MSI-X on
> > RTL8106e"). It also fixes the same issue on RTL6186evl/8111evl on an
> > Aimfor-tech laptop that we had not yet patched. I suspect it will also
> > fix the issue that was worked around in commit 7c53a722459c ("r8169:
> > don't use MSI-X on RTL8168g").
> >
> > Thomas Martitz reports that this change also solves an issue where
> > the AMD Radeon Polaris 10 GPU on the HP Zbook 14u G5 is unresponsive
> > after S3 suspend/resume.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=201069
> > Signed-off-by: Daniel Drake <drake@endlessm.com>
>
> Applied with Rafael's and Peter's reviewed-by to pci/enumeration for v4.20.
> Thanks for the the huge investigative effort!
Since this looks low-risk and fixes several painful issues, I think
this merits a stable tag and being included in v4.19 (instead of
waiting for v4.20).
I moved it to for-linus for v4.19. Let me know if you object.
> > ---
> > drivers/pci/pci.c | 25 +++++++++++++++++--------
> > 1 file changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 29ff9619b5fa..5d58220b6997 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -1289,12 +1289,12 @@ int pci_save_state(struct pci_dev *dev)
> > EXPORT_SYMBOL(pci_save_state);
> >
> > static void pci_restore_config_dword(struct pci_dev *pdev, int offset,
> > - u32 saved_val, int retry)
> > + u32 saved_val, int retry, bool force)
> > {
> > u32 val;
> >
> > pci_read_config_dword(pdev, offset, &val);
> > - if (val == saved_val)
> > + if (!force && val == saved_val)
> > return;
> >
> > for (;;) {
> > @@ -1313,25 +1313,34 @@ static void pci_restore_config_dword(struct pci_dev *pdev, int offset,
> > }
> >
> > static void pci_restore_config_space_range(struct pci_dev *pdev,
> > - int start, int end, int retry)
> > + int start, int end, int retry,
> > + bool force)
> > {
> > int index;
> >
> > for (index = end; index >= start; index--)
> > pci_restore_config_dword(pdev, 4 * index,
> > pdev->saved_config_space[index],
> > - retry);
> > + retry, force);
> > }
> >
> > static void pci_restore_config_space(struct pci_dev *pdev)
> > {
> > if (pdev->hdr_type == PCI_HEADER_TYPE_NORMAL) {
> > - pci_restore_config_space_range(pdev, 10, 15, 0);
> > + pci_restore_config_space_range(pdev, 10, 15, 0, false);
> > /* Restore BARs before the command register. */
> > - pci_restore_config_space_range(pdev, 4, 9, 10);
> > - pci_restore_config_space_range(pdev, 0, 3, 0);
> > + pci_restore_config_space_range(pdev, 4, 9, 10, false);
> > + pci_restore_config_space_range(pdev, 0, 3, 0, false);
> > + } else if (pdev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
> > + pci_restore_config_space_range(pdev, 12, 15, 0, false);
> > + /* Force rewriting of prefetch registers to avoid
> > + * S3 resume issues on Intel PCI bridges that occur when
> > + * these registers are not explicitly written.
> > + */
> > + pci_restore_config_space_range(pdev, 9, 11, 0, true);
> > + pci_restore_config_space_range(pdev, 0, 8, 0, false);
> > } else {
> > - pci_restore_config_space_range(pdev, 0, 15, 0);
> > + pci_restore_config_space_range(pdev, 0, 15, 0, false);
> > }
> > }
> >
> > --
> > 2.17.1
> >
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
^ permalink raw reply
* Re: [PATCH] netfilter: check if the socket netns is correct.
From: Guenter Roeck @ 2018-09-27 20:46 UTC (permalink / raw)
To: Flavio Leitner
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
netfilter-devel, coreteam, netdev, linux-kernel
Hi Flavio,
On Wed, Jun 27, 2018 at 10:34:25AM -0300, Flavio Leitner wrote:
> Netfilter assumes that if the socket is present in the skb, then
> it can be used because that reference is cleaned up while the skb
> is crossing netns.
>
> We want to change that to preserve the socket reference in a future
> patch, so this is a preparation updating netfilter to check if the
> socket netns matches before use it.
>
> Signed-off-by: Flavio Leitner <fbl@redhat.com>
> Acked-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
...
> --- a/net/netfilter/xt_socket.c
> +++ b/net/netfilter/xt_socket.c
> @@ -56,8 +56,12 @@ socket_match(const struct sk_buff *skb, struct xt_action_param *par,
> struct sk_buff *pskb = (struct sk_buff *)skb;
> struct sock *sk = skb->sk;
>
> + if (!net_eq(xt_net(par), sock_net(sk)))
> + sk = NULL;
> +
I am having trouble with this code. With CONFIG_NET_NS enabled, it crashes
for me in read_pnet() because sk is NULL.
> if (!sk)
> sk = nf_sk_lookup_slow_v4(xt_net(par), skb, xt_in(par));
The old code seems to suggest that sk == NULL was possible.
I see the problem with the Chrome OS kernel rebased to v4.19-rc5, so I
can not guarantee that this really an upstream problem. The change seems
odd, though. Are you sure that it is not (or, rather, no longer) necessary
to check if sk == NULL before dereferencing it in sock_net() ?
> +
> if (sk) {
> bool wildcard;
> bool transparent = true;
> @@ -113,8 +117,12 @@ socket_mt6_v1_v2_v3(const struct sk_buff *skb, struct xt_action_param *par)
> struct sk_buff *pskb = (struct sk_buff *)skb;
> struct sock *sk = skb->sk;
>
> + if (!net_eq(xt_net(par), sock_net(sk)))
> + sk = NULL;
> +
Same here.
> if (!sk)
> sk = nf_sk_lookup_slow_v6(xt_net(par), skb, xt_in(par));
> +
> if (sk) {
> bool wildcard;
> bool transparent = true;
Thanks,
Guenter
^ permalink raw reply
* My Greetings
From: Mrs Marianne Jeanne @ 2018-09-27 14:21 UTC (permalink / raw)
Beloved,
I am writing this mail to you with heavy tears in my eyes and great
sorrow in my heart. As I informed you earlier, I am (Mrs.)Marianne
Jeanne,
suffering from long time Cancer of the breast. From all indications
my condition is really deteriorating and it's quite obvious
that I won't live more than 2 months according to my doctors.
I have some funds I inherited from my late loving husband Mr. Martin
Smith, the sum of ($8,5000.000) which he deposited in a Bank . I need
a very honest and God fearing person that can use these funds for
Charity work, helping the Less Privileges, and 20% of this money will
be for your time and expenses, while 80% goes to charities.
Please let me know if I can TRUST YOU ON THIS to carry out this favour
for me. I look forward to your prompt reply for more details.
Yours sincerely
Marianne Jeanne
^ permalink raw reply
* Re: [PATCH v2 07/22] soc/fsl/bman_portals: defer probe after bman's probe
From: Li Yang @ 2018-09-27 20:40 UTC (permalink / raw)
To: Laurentiu Tudor
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Netdev, lkml,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Roy Pledge, madalin.bucur, David Miller, Shawn Guo, robin.murphy,
bharat.bhushan
In-Reply-To: <20180926132247.10971-8-laurentiu.tudor@nxp.com>
On Wed, Sep 26, 2018 at 8:26 AM <laurentiu.tudor@nxp.com> wrote:
>
> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>
> A crash in bman portal probing could not be triggered (as is the case
> with qman portals) but it does make calls [1] into the bman driver so
> lets make sure the bman portal probing happens after bman's.
>
> [1] bman_p_irqsource_add() (in bman) called by:
> init_pcfg() called by:
> bman_portal_probe()
>
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
As this is part of a bug fix for v4.19, applied on soc/fsl for fix.
> ---
> drivers/soc/fsl/qbman/bman_portal.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/qbman/bman_portal.c b/drivers/soc/fsl/qbman/bman_portal.c
> index 2f71f7df3465..f9edd28894fd 100644
> --- a/drivers/soc/fsl/qbman/bman_portal.c
> +++ b/drivers/soc/fsl/qbman/bman_portal.c
> @@ -91,7 +91,15 @@ static int bman_portal_probe(struct platform_device *pdev)
> struct device_node *node = dev->of_node;
> struct bm_portal_config *pcfg;
> struct resource *addr_phys[2];
> - int irq, cpu;
> + int irq, cpu, err;
> +
> + err = bman_is_probed();
> + if (!err)
> + return -EPROBE_DEFER;
> + if (err < 0) {
> + dev_err(&pdev->dev, "failing probe due to bman probe error\n");
> + return -ENODEV;
> + }
>
> pcfg = devm_kmalloc(dev, sizeof(*pcfg), GFP_KERNEL);
> if (!pcfg)
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v2 06/22] soc/fsl/qman_portals: defer probe after qman's probe
From: Li Yang @ 2018-09-27 20:37 UTC (permalink / raw)
To: Laurentiu Tudor
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Netdev, lkml,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Roy Pledge, madalin.bucur, David Miller, Shawn Guo, robin.murphy,
bharat.bhushan
In-Reply-To: <20180926132247.10971-7-laurentiu.tudor@nxp.com>
On Wed, Sep 26, 2018 at 8:26 AM <laurentiu.tudor@nxp.com> wrote:
>
> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>
> Defer probe of qman portals after qman probing. This fixes the crash
> below, seen on NXP LS1043A SoCs:
>
> Unable to handle kernel NULL pointer dereference at virtual address
> 0000000000000004
> Mem abort info:
> ESR = 0x96000004
> Exception class = DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> Data abort info:
> ISV = 0, ISS = 0x00000004
> CM = 0, WnR = 0
> [0000000000000004] user address but active_mm is swapper
> Internal error: Oops: 96000004 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> 4.18.0-rc1-next-20180622-00200-g986f5c179185 #9
> Hardware name: LS1043A RDB Board (DT)
> pstate: 80000005 (Nzcv daif -PAN -UAO)
> pc : qman_set_sdest+0x74/0xa0
> lr : qman_portal_probe+0x22c/0x470
> sp : ffff00000803bbc0
> x29: ffff00000803bbc0 x28: 0000000000000000
> x27: ffff0000090c1b88 x26: ffff00000927cb68
> x25: ffff00000927c000 x24: ffff00000927cb60
> x23: 0000000000000000 x22: 0000000000000000
> x21: ffff0000090e9000 x20: ffff800073b5c810
> x19: ffff800027401298 x18: ffffffffffffffff
> x17: 0000000000000001 x16: 0000000000000000
> x15: ffff0000090e96c8 x14: ffff80002740138a
> x13: ffff0000090f2000 x12: 0000000000000030
> x11: ffff000008f25000 x10: 0000000000000000
> x9 : ffff80007bdfd2c0 x8 : 0000000000004000
> x7 : ffff80007393cc18 x6 : 0040000000000001
> x5 : 0000000000000000 x4 : ffffffffffffffff
> x3 : 0000000000000004 x2 : ffff00000927c900
> x1 : 0000000000000000 x0 : 0000000000000004
> Process swapper/0 (pid: 1, stack limit = 0x(____ptrval____))
> Call trace:
> qman_set_sdest+0x74/0xa0
> platform_drv_probe+0x50/0xa8
> driver_probe_device+0x214/0x2f8
> __driver_attach+0xd8/0xe0
> bus_for_each_dev+0x68/0xc8
> driver_attach+0x20/0x28
> bus_add_driver+0x108/0x228
> driver_register+0x60/0x110
> __platform_driver_register+0x40/0x48
> qman_portal_driver_init+0x20/0x84
> do_one_initcall+0x58/0x168
> kernel_init_freeable+0x184/0x22c
> kernel_init+0x10/0x108
> ret_from_fork+0x10/0x18
> Code: f9400443 11001000 927e4800 8b000063 (b9400063)
> ---[ end trace 4f6d50489ecfb930 ]---
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
As this is part of a bug fix for v4.19, applied on soc/fsl for fix.
> ---
> drivers/soc/fsl/qbman/qman_portal.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/soc/fsl/qbman/qman_portal.c b/drivers/soc/fsl/qbman/qman_portal.c
> index 6d9da3b1b5ad..eef93cab84f1 100644
> --- a/drivers/soc/fsl/qbman/qman_portal.c
> +++ b/drivers/soc/fsl/qbman/qman_portal.c
> @@ -229,6 +229,14 @@ static int qman_portal_probe(struct platform_device *pdev)
> int irq, cpu, err;
> u32 val;
>
> + err = qman_is_probed();
> + if (!err)
> + return -EPROBE_DEFER;
> + if (err < 0) {
> + dev_err(&pdev->dev, "failing probe due to qman probe error\n");
> + return -ENODEV;
> + }
> +
> pcfg = devm_kmalloc(dev, sizeof(*pcfg), GFP_KERNEL);
> if (!pcfg)
> return -ENOMEM;
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v2 05/22] soc/fsl/qbman: add APIs to retrieve the probing status
From: Li Yang @ 2018-09-27 20:37 UTC (permalink / raw)
To: Laurentiu Tudor
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Netdev, lkml,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Roy Pledge, madalin.bucur, David Miller, Shawn Guo, robin.murphy,
bharat.bhushan
In-Reply-To: <20180926132247.10971-6-laurentiu.tudor@nxp.com>
On Wed, Sep 26, 2018 at 8:26 AM <laurentiu.tudor@nxp.com> wrote:
>
> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>
> Add a couple of new APIs to check the probing status of qman and bman:
> 'int bman_is_probed()' and 'int qman_is_probed()'.
> They return the following values.
> * 1 if qman/bman were probed correctly
> * 0 if qman/bman were not yet probed
> * -1 if probing of qman/bman failed
> Drivers that use qman/bman driver services are required to use these
> APIs before calling any functions exported by qman or bman drivers
> or otherwise they will crash the kernel.
> The APIs will be used in the following couple of qbman portal patches
> and later in the series in the dpaa1 ethernet driver.
>
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
As this is part of a bug fix for v4.19, applied on soc/fsl for fix.
> ---
> drivers/soc/fsl/qbman/bman_ccsr.c | 11 +++++++++++
> drivers/soc/fsl/qbman/qman_ccsr.c | 11 +++++++++++
> include/soc/fsl/bman.h | 8 ++++++++
> include/soc/fsl/qman.h | 8 ++++++++
> 4 files changed, 38 insertions(+)
>
> diff --git a/drivers/soc/fsl/qbman/bman_ccsr.c b/drivers/soc/fsl/qbman/bman_ccsr.c
> index d180da003e4a..b209c79511bb 100644
> --- a/drivers/soc/fsl/qbman/bman_ccsr.c
> +++ b/drivers/soc/fsl/qbman/bman_ccsr.c
> @@ -121,6 +121,7 @@ static void bm_set_memory(u64 ba, u32 size)
> */
> static dma_addr_t fbpr_a;
> static size_t fbpr_sz;
> +static int __bman_probed;
>
> static int bman_fbpr(struct reserved_mem *rmem)
> {
> @@ -167,6 +168,12 @@ static irqreturn_t bman_isr(int irq, void *ptr)
> return IRQ_HANDLED;
> }
>
> +int bman_is_probed(void)
> +{
> + return __bman_probed;
> +}
> +EXPORT_SYMBOL_GPL(bman_is_probed);
> +
> static int fsl_bman_probe(struct platform_device *pdev)
> {
> int ret, err_irq;
> @@ -177,6 +184,8 @@ static int fsl_bman_probe(struct platform_device *pdev)
> u16 id, bm_pool_cnt;
> u8 major, minor;
>
> + __bman_probed = -1;
> +
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!res) {
> dev_err(dev, "Can't get %pOF property 'IORESOURCE_MEM'\n",
> @@ -266,6 +275,8 @@ static int fsl_bman_probe(struct platform_device *pdev)
> return ret;
> }
>
> + __bman_probed = 1;
> +
> return 0;
> };
>
> diff --git a/drivers/soc/fsl/qbman/qman_ccsr.c b/drivers/soc/fsl/qbman/qman_ccsr.c
> index 0cfe79f85a66..383a49dcce68 100644
> --- a/drivers/soc/fsl/qbman/qman_ccsr.c
> +++ b/drivers/soc/fsl/qbman/qman_ccsr.c
> @@ -274,6 +274,7 @@ static const struct qman_error_info_mdata error_mdata[] = {
> static u32 __iomem *qm_ccsr_start;
> /* A SDQCR mask comprising all the available/visible pool channels */
> static u32 qm_pools_sdqcr;
> +static int __qman_probed;
>
> static inline u32 qm_ccsr_in(u32 offset)
> {
> @@ -689,6 +690,12 @@ static int qman_resource_init(struct device *dev)
> return 0;
> }
>
> +int qman_is_probed(void)
> +{
> + return __qman_probed;
> +}
> +EXPORT_SYMBOL_GPL(qman_is_probed);
> +
> static int fsl_qman_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -699,6 +706,8 @@ static int fsl_qman_probe(struct platform_device *pdev)
> u16 id;
> u8 major, minor;
>
> + __qman_probed = -1;
> +
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!res) {
> dev_err(dev, "Can't get %pOF property 'IORESOURCE_MEM'\n",
> @@ -845,6 +854,8 @@ static int fsl_qman_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> + __qman_probed = 1;
> +
> return 0;
> }
>
> diff --git a/include/soc/fsl/bman.h b/include/soc/fsl/bman.h
> index eaaf56df4086..5b99cb2ea5ef 100644
> --- a/include/soc/fsl/bman.h
> +++ b/include/soc/fsl/bman.h
> @@ -126,4 +126,12 @@ int bman_release(struct bman_pool *pool, const struct bm_buffer *bufs, u8 num);
> */
> int bman_acquire(struct bman_pool *pool, struct bm_buffer *bufs, u8 num);
>
> +/**
> + * bman_is_probed - Check if bman is probed
> + *
> + * Returns 1 if the bman driver successfully probed, -1 if the bman driver
> + * failed to probe or 0 if the bman driver did not probed yet.
> + */
> +int bman_is_probed(void);
> +
> #endif /* __FSL_BMAN_H */
> diff --git a/include/soc/fsl/qman.h b/include/soc/fsl/qman.h
> index d4dfefdee6c1..597783b8a3a0 100644
> --- a/include/soc/fsl/qman.h
> +++ b/include/soc/fsl/qman.h
> @@ -1186,4 +1186,12 @@ int qman_alloc_cgrid_range(u32 *result, u32 count);
> */
> int qman_release_cgrid(u32 id);
>
> +/**
> + * qman_is_probed - Check if qman is probed
> + *
> + * Returns 1 if the qman driver successfully probed, -1 if the qman driver
> + * failed to probe or 0 if the qman driver did not probed yet.
> + */
> +int qman_is_probed(void);
> +
> #endif /* __FSL_QMAN_H */
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: test_bpf: add dummy dev->net for flow_dissector
From: Willem de Bruijn @ 2018-09-27 14:11 UTC (permalink / raw)
To: Eric Dumazet
Cc: songliubraving, Network Development, Daniel Borkmann, Kernel Team,
Willem de Bruijn, Petar Penkov
In-Reply-To: <906045d5-206d-09a4-9de9-97789adbe701@gmail.com>
On Thu, Sep 27, 2018 at 9:00 AM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 09/27/2018 12:15 AM, Song Liu wrote:
> > Latest changes in __skb_flow_dissect() assume skb->dev has valid nd_net.
> > However, this is not true for test_bpf. As a result, test_bpf.ko crashes
> > the system with the following stack trace:
> >
>
> > + dev_net_set(&dev, &net);
>
> This is yet another fake stuff that will break some day, so many other things would
> need to be setup properly in a real "struct net"
>
> What about using the current net_ns ?
>
> dev_net_set(&dev, current->nsproxy->net_ns);
>
> This might be &init_net in the context of test_bpf(), so maybe simply use &init_net
> to clearly state that test_bpf is not net ns fully ready.
Agreed.
Thanks for fixing this.
^ permalink raw reply
* Re: [PATCH iproute2] vxlan: show correct ttl inherit info
From: Hangbin Liu @ 2018-09-27 14:07 UTC (permalink / raw)
To: Phil Sutter, netdev, Stephen Hemminger, David Ahern
In-Reply-To: <20180927092745.GV14666@orbyte.nwl.cc>
On Thu, Sep 27, 2018 at 11:27:45AM +0200, Phil Sutter wrote:
> Hi Hangbin,
>
> On Thu, Sep 27, 2018 at 03:28:36PM +0800, Hangbin Liu wrote:
> > We should only show ttl inherit when IFLA_VXLAN_TTL_INHERIT supplied.
> > Otherwise show the ttl number, or auto when it is 0.
> >
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> > ---
> > ip/iplink_vxlan.c | 16 ++++++++++------
> > 1 file changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
> > index 831f39a..7fc0e2b 100644
> > --- a/ip/iplink_vxlan.c
> > +++ b/ip/iplink_vxlan.c
> > @@ -145,7 +145,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
> > NEXT_ARG();
> > check_duparg(&attrs, IFLA_VXLAN_TTL, "ttl", *argv);
> > if (strcmp(*argv, "inherit") == 0) {
> > - addattr_l(n, 1024, IFLA_VXLAN_TTL_INHERIT, NULL, 0);
> > + addattr(n, 1024, IFLA_VXLAN_TTL_INHERIT);
>
> So for VXLAN, the attribute is just added but with a zero value. Looking
> at respective kernel code, this seems fine. Now I wonder why for Geneve,
> you set the value to 1 and when displaying explicitly check whether the
> attribute is there *and* non-zero. OK, looks like Geneve driver always
> exports IFLA_GENEVE_TTL_INHERIT. Oddly, I can't find where VXLAN driver
> in kernel does export the IFLA_VXLAN_TTL_INHERIT attribute. Am I missing
> something?
Hi Phil,
The vxlan ttl inherit exportation is just fixed by
net commit 8fd78069874 ("vxlan: fill ttl inherit info") yesterday.
> Do you know why handling of the attributes in both drivers differ?
That's because I set IFLA_VXLAN_TTL_INHERIT type to NLA_FLAG. But when fix
the geneve issue, I forgot this and set IFLA_GENEVE_TTL_INHERIT type to
NLA_U8... :(
I noticed this when post the iproute2 patch. But I thought this should have
no much influence, so I just leave it as it is.
If you think we'd better have a same behaver. We can drop the geneve iproute2
patch and post a fix to net kernel tree.
Thanks
Hangbin
^ permalink raw reply
* Re: [PATCH net-next 0/7] rtnetlink: add RTM_GETADDR2
From: David Ahern @ 2018-09-27 20:24 UTC (permalink / raw)
To: Christian Brauner, jbenc, davem, stephen, netdev, linux-kernel
In-Reply-To: <20180927175857.3511-1-christian@brauner.io>
On 9/27/18 11:58 AM, Christian Brauner wrote:
> Various userspace programs (e.g. iproute2) have sent RTM_GETADDR
> requests with struct ifinfomsg. This is wrong and should have been
> struct ifaddrmsg all along as mandated by the manpages. However, dump
> requests so far didn't parse the netlink message that was sent and
> succeeded even when a wrong struct was passed along.
...
> The correct solution at this point seems to me to introduce a new
> RTM_GETADDR2 request. This way we can parse the message and fail hard if
> the struct is not struct ifaddrmsg and can safely extend it in the
> future. Userspace tools that rely on the buggy RTM_GETADDR API will
> still keep working without even having to see any log messages and new
> userspace tools that want to make user of new features can make use of
> the new RTM_GETADDR2 requests.
First, I think this is the wrong precedent when all we need is a single
bit flag that userspace can use to tell the kernel "I have a clue and I
am passing in the proper header for this dump request".
Second, you are not addressing the problems of the past by requiring the
proper header and checking values passed in it.
I have another idea. I'll send an RFC patch soon.
^ permalink raw reply
* Re: [RFC PATCH iproute2-next V2] System specification exception API
From: Eran Ben Elisha @ 2018-09-27 14:02 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, Jakub Kicinski, Jiri Pirko, Stephen Hemminger,
Andrew Lunn, Tobin C. Harding, Ariel Almog, Tal Alon
In-Reply-To: <20180927124736.GB2209@nanopsycho.orion>
On 9/27/2018 3:47 PM, Jiri Pirko wrote:
> Wed, Sep 26, 2018 at 01:52:58PM CEST, eranbe@mellanox.com wrote:
>> The exception spec is targeted for Real Time Alerting, in order to know when
>> something bad had happened to a PCI device
>> - Provide alert debug information
>> - Self healing
>> - If problem needs vendor support, provide a way to gather all needed debugging
>> information.
>>
>> The exception mechanism contains condition checkers which sense for malfunction. Upon a condition hit,
>> actions such as logs and correction can be taken.
>>
>> The condition checkers are divided into the following groups
>> - Hardware - a checker which is triggered by the device due to
>> malfunction.
>> - Software - a checker which is triggered by the software due to
>> malfunction.
>
> What do you mean by a "software malfunction", a "FW malfunction"?
> Also, I don't see this 2 groups in the man.
Software malfunction can be a Transmit error (caused by bad send request).
FW/HW malfunction can be any catastrophic error report (the ones that
should be exposed to driver).
The comment here was to highlight that we can support different kinds of
condition groups.
If for a specific condition, we will need to highlight it is SW/HW, we
can concatenate it to its name.
Eran
>
>
>> Both groups of condition checkers can be triggered due to error event or due to a periodic check.
>>
>> Actions are the way to handle those events. Action can be in one of the
>> following groups:
>> - Dump - SW trace, SW dump, HW trace, HW dump
>> - Reset - Surgical correction (e.g. modify Q, flush Q, reset of device, etc)
>> Actions can be performed by SW or HW.
>>
>> User is allowed to enable or disable condition checkers and its action mapping.
>>
>> This RFC man page patch describes the suggested API of devlink-exception in order
>> to control conditions and actions.
>>
>> V2:
>> * Renaming terms:
>> health -> exception
>> sensor -> condition
>> * Remove reinit command and merge with action command.
>> * Consmetics in grammer.
>>
>> Eran Ben Elisha (1):
>> man: Add devlink exception man page
>>
>> man/man8/devlink-exception.8 | 158 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 158 insertions(+)
>> create mode 100644 man/man8/devlink-exception.8
>>
>> --
>> 1.8.3.1
>>
^ permalink raw reply
* [PATCH v2 net-next 2/2] dt-bindings: net: add support for Microchip KSZ9131 Ethernet PHY
From: Yuiko Oshino @ 2018-09-27 20:16 UTC (permalink / raw)
To: davem, robh+dt, devicetree, f.fainelli, andrew
Cc: linux-kernel, mark.rutland, m.felsch, Markus.Niebel, netdev,
UNGLinuxDriver
Add support for Microchip Technology KSZ9131 10/100/1000 Ethernet PHY
Signed-off-by: Yuiko Oshino <yuiko.oshino@microchip.com>
---
.../devicetree/bindings/net/micrel-ksz90x1.txt | 29 +++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt b/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
index e22d8cf..d23d14a 100644
--- a/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
+++ b/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
@@ -1,4 +1,4 @@
-Micrel KSZ9021/KSZ9031 Gigabit Ethernet PHY
+Micrel/Microchip KSZ9021/KSZ9031/KSZ9131 Gigabit Ethernet PHY
Some boards require special tuning values, particularly when it comes
to clock delays. You can specify clock delay values in the PHY OF
@@ -64,6 +64,33 @@ KSZ9031:
Attention: The link partner must be configurable as slave otherwise
no link will be established.
+KSZ9131:
+
+ All skew control options are specified in picoseconds. The minimum
+ value is 0, and the maximum is property-dependent. The increment
+ step is 100ps. The default value is the neutral setting, so setting
+ rxc-skew-ps=<0> actually results in -700 picoseconds adjustment.
+
+ Optional properties:
+
+ Maximum value of 2400, default value 700:
+
+ - rxc-skew-ps : Skew control of RX clock pad
+ - txc-skew-ps : Skew control of TX clock pad
+
+ Maximum value of 1500, default value 700:
+
+ - rxdv-skew-ps : Skew control of RX CTL pad
+ - txen-skew-ps : Skew control of TX CTL pad
+ - rxd0-skew-ps : Skew control of RX data 0 pad
+ - rxd1-skew-ps : Skew control of RX data 1 pad
+ - rxd2-skew-ps : Skew control of RX data 2 pad
+ - rxd3-skew-ps : Skew control of RX data 3 pad
+ - txd0-skew-ps : Skew control of TX data 0 pad
+ - txd1-skew-ps : Skew control of TX data 1 pad
+ - txd2-skew-ps : Skew control of TX data 2 pad
+ - txd3-skew-ps : Skew control of TX data 3 pad
+
Examples:
mdio {
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next] virtio_net: ethtool tx napi configuration
From: Willem de Bruijn @ 2018-09-27 13:53 UTC (permalink / raw)
To: Jason Wang
Cc: Florian Fainelli, Network Development, David Miller, caleb.raitto,
Michael S. Tsirkin, Jon Olson (Google Drive), Willem de Bruijn
In-Reply-To: <841d0fc6-d9a2-84d0-f794-7a7b2019fa17@redhat.com>
On Thu, Sep 27, 2018 at 4:51 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
>
> On 2018年09月14日 12:46, Willem de Bruijn wrote:
> >> I'm not sure I get this. If we don't enable tx napi, we tend to delay TX
> >> interrupt if we found the ring is about to full to avoid interrupt
> >> storm, so we're probably ok in this case.
> > I'm only concerned about the transition state when converting from
> > napi to no-napi when the queue is stopped and tx interrupt disabled.
> >
> > With napi mode the interrupt is only disabled if napi is scheduled,
> > in which case it will eventually reenable the interrupt. But when
> > switching to no-napi mode in this state no progress will be made.
> >
> > But it seems this cannot happen. When converting to no-napi
> > mode, set_coalesce waits for napi to complete in napi_disable.
> > So the interrupt should always start enabled when transitioning
> > into no-napi mode.
>
> An update, I meet a hang in napi_disalbe(). But it's hard to be
> reproduced. I tend to choose a easy way like V1 that only allow the
> switching when device is down.
I agree.
> I will post the patch after a vacation. (or you can post if it was
> urgent for you).
If you have time to review and add your signed-off-by, I can post it.
It's a pretty small diff at this point.
But no rush, we can also wait until after your vacation.
I also need to look at a patch to toggle LRO using ethtool, btw.
^ permalink raw reply
* [PATCH ethtool] ethtool: Fix uninitialized variable use at qsfp dump
From: Eran Ben Elisha @ 2018-09-27 13:48 UTC (permalink / raw)
To: netdev, John W. Linville
Cc: Chris Preimesberger, Andrew Lunn, Neil Horman,
Vidya Sagar Ravipati, Eran Ben Elisha
Struct sff_diags can be used uninitialized at sff8636_show_dom, this
caused the tool to show unreported fields (supports_alarms) by the lower
level driver.
Fixes: a5e73bb05ee4 ("ethtool:QSFP Plus/QSFP28 Diagnostics Information Support")
Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
---
qsfp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qsfp.c b/qsfp.c
index 32e195d12dc0..d196aa1753de 100644
--- a/qsfp.c
+++ b/qsfp.c
@@ -671,7 +671,7 @@ static void sff8636_dom_parse(const __u8 *id, struct sff_diags *sd)
static void sff8636_show_dom(const __u8 *id, __u32 eeprom_len)
{
- struct sff_diags sd;
+ struct sff_diags sd = {0};
char *rx_power_string = NULL;
char power_string[MAX_DESC_SIZE];
int i;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2 08/22] soc/fsl/qbman_portals: add APIs to retrieve the probing status
From: Li Yang @ 2018-09-27 20:03 UTC (permalink / raw)
To: Laurentiu Tudor
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Netdev, lkml,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Roy Pledge, madalin.bucur, David Miller, Shawn Guo, robin.murphy,
bharat.bhushan
In-Reply-To: <20180926132247.10971-9-laurentiu.tudor@nxp.com>
On Wed, Sep 26, 2018 at 8:26 AM <laurentiu.tudor@nxp.com> wrote:
>
> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>
> Add a couple of new APIs to check the probing status of the required
> cpu bound qman and bman portals:
> 'int bman_portals_probed()' and 'int qman_portals_probed()'.
> They return the following values.
> * 1 if qman/bman portals were all probed correctly
> * 0 if qman/bman portals were not yet probed
> * -1 if probing of qman/bman portals failed
> Drivers that use qman/bman portal driver services are required to use
> these APIs before calling any functions exported by these drivers or
> otherwise they will crash the kernel.
> First user will be the dpaa1 ethernet driver, coming in a subsequent
> patch.
>
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> ---
> drivers/soc/fsl/qbman/bman_portal.c | 10 ++++++++++
> drivers/soc/fsl/qbman/qman_portal.c | 10 ++++++++++
> include/soc/fsl/bman.h | 8 ++++++++
> include/soc/fsl/qman.h | 9 +++++++++
> 4 files changed, 37 insertions(+)
>
> diff --git a/drivers/soc/fsl/qbman/bman_portal.c b/drivers/soc/fsl/qbman/bman_portal.c
> index f9edd28894fd..8048d35de8a2 100644
> --- a/drivers/soc/fsl/qbman/bman_portal.c
> +++ b/drivers/soc/fsl/qbman/bman_portal.c
> @@ -32,6 +32,7 @@
>
> static struct bman_portal *affine_bportals[NR_CPUS];
> static struct cpumask portal_cpus;
> +static int __bman_portals_probed;
> /* protect bman global registers and global data shared among portals */
> static DEFINE_SPINLOCK(bman_lock);
>
> @@ -85,6 +86,12 @@ static int bman_online_cpu(unsigned int cpu)
> return 0;
> }
>
> +int bman_portals_probed(void)
> +{
> + return __bman_portals_probed;
> +}
> +EXPORT_SYMBOL_GPL(bman_portals_probed);
> +
> static int bman_portal_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -148,6 +155,7 @@ static int bman_portal_probe(struct platform_device *pdev)
> spin_lock(&bman_lock);
> cpu = cpumask_next_zero(-1, &portal_cpus);
> if (cpu >= nr_cpu_ids) {
> + __bman_portals_probed = 1;
What if the last CPU is not used for portals? Is there a hard
requirement that all CPUs need to be used for portal? What happens if
the last CPU is offline?
> /* unassigned portal, skip init */
> spin_unlock(&bman_lock);
> return 0;
> @@ -173,6 +181,8 @@ static int bman_portal_probe(struct platform_device *pdev)
> err_ioremap2:
> memunmap(pcfg->addr_virt_ce);
> err_ioremap1:
> + __bman_portals_probed = 1;
> +
There are other error paths that not covered.
> return -ENXIO;
> }
>
> diff --git a/drivers/soc/fsl/qbman/qman_portal.c b/drivers/soc/fsl/qbman/qman_portal.c
> index eef93cab84f1..1b2fc981c269 100644
> --- a/drivers/soc/fsl/qbman/qman_portal.c
> +++ b/drivers/soc/fsl/qbman/qman_portal.c
> @@ -39,6 +39,7 @@ EXPORT_SYMBOL(qman_dma_portal);
> #define CONFIG_FSL_DPA_PIRQ_FAST 1
>
> static struct cpumask portal_cpus;
> +static int __qman_portals_probed;
> /* protect qman global registers and global data shared among portals */
> static DEFINE_SPINLOCK(qman_lock);
>
> @@ -219,6 +220,12 @@ static int qman_online_cpu(unsigned int cpu)
> return 0;
> }
>
> +int qman_portals_probed(void)
> +{
> + return __qman_portals_probed;
> +}
> +EXPORT_SYMBOL_GPL(qman_portals_probed);
> +
> static int qman_portal_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -306,6 +313,7 @@ static int qman_portal_probe(struct platform_device *pdev)
> spin_lock(&qman_lock);
> cpu = cpumask_next_zero(-1, &portal_cpus);
> if (cpu >= nr_cpu_ids) {
> + __qman_portals_probed = 1;
Ditto.
> /* unassigned portal, skip init */
> spin_unlock(&qman_lock);
> return 0;
> @@ -336,6 +344,8 @@ static int qman_portal_probe(struct platform_device *pdev)
> err_ioremap2:
> memunmap(pcfg->addr_virt_ce);
> err_ioremap1:
> + __qman_portals_probed = -1;
> +
Ditto.
> return -ENXIO;
> }
>
> diff --git a/include/soc/fsl/bman.h b/include/soc/fsl/bman.h
> index 5b99cb2ea5ef..173e4049d963 100644
> --- a/include/soc/fsl/bman.h
> +++ b/include/soc/fsl/bman.h
> @@ -133,5 +133,13 @@ int bman_acquire(struct bman_pool *pool, struct bm_buffer *bufs, u8 num);
> * failed to probe or 0 if the bman driver did not probed yet.
> */
> int bman_is_probed(void);
> +/**
> + * bman_portals_probed - Check if all cpu bound bman portals are probed
> + *
> + * Returns 1 if all the required cpu bound bman portals successfully probed,
> + * -1 if probe errors appeared or 0 if the bman portals did not yet finished
> + * probing.
> + */
> +int bman_portals_probed(void);
>
> #endif /* __FSL_BMAN_H */
> diff --git a/include/soc/fsl/qman.h b/include/soc/fsl/qman.h
> index 597783b8a3a0..7732e48081eb 100644
> --- a/include/soc/fsl/qman.h
> +++ b/include/soc/fsl/qman.h
> @@ -1194,4 +1194,13 @@ int qman_release_cgrid(u32 id);
> */
> int qman_is_probed(void);
>
> +/**
> + * qman_portals_probed - Check if all cpu bound qman portals are probed
> + *
> + * Returns 1 if all the required cpu bound qman portals successfully probed,
> + * -1 if probe errors appeared or 0 if the qman portals did not yet finished
> + * probing.
> + */
> +int qman_portals_probed(void);
> +
> #endif /* __FSL_QMAN_H */
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH net-next] selftests: forwarding: test for bridge sticky flag
From: Nikolay Aleksandrov @ 2018-09-27 13:35 UTC (permalink / raw)
To: netdev; +Cc: roopa, Nikolay Aleksandrov
In-Reply-To: <20180927133513.14175-1-nikolay@cumulusnetworks.com>
This test adds an fdb entry with the sticky flag and sends traffic from
a different port with the same mac as a source address expecting the entry
to not change ports if the flag is operating correctly.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
.../selftests/net/forwarding/bridge_sticky_fdb.sh | 69 ++++++++++++++++++++++
1 file changed, 69 insertions(+)
create mode 100755 tools/testing/selftests/net/forwarding/bridge_sticky_fdb.sh
diff --git a/tools/testing/selftests/net/forwarding/bridge_sticky_fdb.sh b/tools/testing/selftests/net/forwarding/bridge_sticky_fdb.sh
new file mode 100755
index 000000000000..1f8ef0eff862
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/bridge_sticky_fdb.sh
@@ -0,0 +1,69 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+ALL_TESTS="sticky"
+NUM_NETIFS=4
+TEST_MAC=de:ad:be:ef:13:37
+source lib.sh
+
+switch_create()
+{
+ ip link add dev br0 type bridge
+
+ ip link set dev $swp1 master br0
+ ip link set dev $swp2 master br0
+
+ ip link set dev br0 up
+ ip link set dev $h1 up
+ ip link set dev $swp1 up
+ ip link set dev $h2 up
+ ip link set dev $swp2 up
+}
+
+switch_destroy()
+{
+ ip link set dev $swp2 down
+ ip link set dev $h2 down
+ ip link set dev $swp1 down
+ ip link set dev $h1 down
+
+ ip link del dev br0
+}
+
+setup_prepare()
+{
+ h1=${NETIFS[p1]}
+ swp1=${NETIFS[p2]}
+ h2=${NETIFS[p3]}
+ swp2=${NETIFS[p4]}
+
+ switch_create
+}
+
+cleanup()
+{
+ pre_cleanup
+ switch_destroy
+}
+
+sticky()
+{
+ bridge fdb add $TEST_MAC dev $swp1 master static sticky
+ check_err $? "Could not add fdb entry"
+ bridge fdb del $TEST_MAC dev $swp1 vlan 1 master static sticky
+ $MZ $h2 -c 1 -a $TEST_MAC -t arp "request" -q
+ bridge -j fdb show br br0 brport $swp1\
+ | jq -e ".[] | select(.mac == \"$TEST_MAC\")" &> /dev/null
+ check_err $? "Did not find FDB record when should"
+
+ log_test "Sticky fdb entry"
+}
+
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tests_run
+
+exit $EXIT_STATUS
--
2.11.0
^ permalink raw reply related
* [PATCH iproute2 net-next] bridge: fdb: add support for sticky flag
From: Nikolay Aleksandrov @ 2018-09-27 13:35 UTC (permalink / raw)
To: netdev; +Cc: roopa, Nikolay Aleksandrov, David Ahern
Add support for the new sticky flag that can be set on fdbs and update the
man page.
CC: David Ahern <dsahern@gmail.com>
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
bridge/fdb.c | 9 +++++++--
man/man8/bridge.8 | 6 +++++-
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/bridge/fdb.c b/bridge/fdb.c
index 4dbc894ceab9..828fdab264cb 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -37,8 +37,8 @@ static void usage(void)
fprintf(stderr,
"Usage: bridge fdb { add | append | del | replace } ADDR dev DEV\n"
" [ self ] [ master ] [ use ] [ router ] [ extern_learn ]\n"
- " [ local | static | dynamic ] [ dst IPADDR ] [ vlan VID ]\n"
- " [ port PORT] [ vni VNI ] [ via DEV ]\n"
+ " [ sticky ] [ local | static | dynamic ] [ dst IPADDR ]\n"
+ " [ vlan VID ] [ port PORT] [ vni VNI ] [ via DEV ]\n"
" bridge fdb [ show [ br BRDEV ] [ brport DEV ] [ vlan VID ] [ state STATE ] ]\n");
exit(-1);
}
@@ -101,6 +101,9 @@ static void fdb_print_flags(FILE *fp, unsigned int flags)
if (flags & NTF_MASTER)
print_string(PRINT_ANY, NULL, "%s ", "master");
+ if (flags & NTF_STICKY)
+ print_string(PRINT_ANY, NULL, "%s ", "sticky");
+
close_json_array(PRINT_JSON, NULL);
}
@@ -414,6 +417,8 @@ static int fdb_modify(int cmd, int flags, int argc, char **argv)
req.ndm.ndm_flags |= NTF_USE;
} else if (matches(*argv, "extern_learn") == 0) {
req.ndm.ndm_flags |= NTF_EXT_LEARNED;
+ } else if (matches(*argv, "sticky") == 0) {
+ req.ndm.ndm_flags |= NTF_STICKY;
} else {
if (strcmp(*argv, "to") == 0)
NEXT_ARG();
diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index 53cd3d0a3d93..c0415bc646df 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -63,7 +63,7 @@ bridge \- show / manipulate bridge addresses and devices
.B dev
.IR DEV " { "
.BR local " | " static " | " dynamic " } [ "
-.BR self " ] [ " master " ] [ " router " ] [ " use " ] [ " extern_learn " ] [ "
+.BR self " ] [ " master " ] [ " router " ] [ " use " ] [ " extern_learn " ] [ " sticky " ] [ "
.B dst
.IR IPADDR " ] [ "
.B vni
@@ -448,6 +448,10 @@ indicate to the kernel that an entry was hardware or user-space
controller learnt dynamic entry. Kernel will not age such an entry.
.sp
+.B sticky
+- this entry will not change its port due to learning.
+.sp
+
.in -8
The next command line parameters apply only
when the specified device
--
2.11.0
^ permalink raw reply related
* [PATCH net-next] net: nixge: Address compiler warnings when building for i386
From: Moritz Fischer @ 2018-09-27 19:48 UTC (permalink / raw)
To: linux-kernel
Cc: netdev, keescook, davem, moritz.fischer, f.fainelli,
alex.williams, Moritz Fischer, Arnd Bergmann
Address compiler warning reported by kbuild autobuilders
when building for i386 as a result of dma_addr_t size on
different architectures.
warning: cast to pointer from integer of different size
[-Wint-to-pointer-cast]
Fixes: 7e8d5755be0e ("net: nixge: Add support for 64-bit platforms")
Signed-off-by: Moritz Fischer <mdf@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/ni/nixge.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
index 74cf52e3fb09..0611f2335b4a 100644
--- a/drivers/net/ethernet/ni/nixge.c
+++ b/drivers/net/ethernet/ni/nixge.c
@@ -127,8 +127,8 @@ struct nixge_hw_dma_bd {
#ifdef CONFIG_PHYS_ADDR_T_64BIT
#define nixge_hw_dma_bd_set_addr(bd, field, addr) \
do { \
- (bd)->field##_lo = lower_32_bits(((u64)addr)); \
- (bd)->field##_hi = upper_32_bits(((u64)addr)); \
+ (bd)->field##_lo = lower_32_bits((addr)); \
+ (bd)->field##_hi = upper_32_bits((addr)); \
} while (0)
#else
#define nixge_hw_dma_bd_set_addr(bd, field, addr) \
@@ -251,7 +251,7 @@ static void nixge_hw_dma_bd_release(struct net_device *ndev)
NIXGE_MAX_JUMBO_FRAME_SIZE,
DMA_FROM_DEVICE);
- skb = (struct sk_buff *)
+ skb = (struct sk_buff *)(uintptr_t)
nixge_hw_dma_bd_get_addr(&priv->rx_bd_v[i],
sw_id_offset);
dev_kfree_skb(skb);
@@ -323,7 +323,7 @@ static int nixge_hw_dma_bd_init(struct net_device *ndev)
if (!skb)
goto out;
- nixge_hw_dma_bd_set_offset(&priv->rx_bd_v[i], skb);
+ nixge_hw_dma_bd_set_offset(&priv->rx_bd_v[i], (uintptr_t)skb);
phys = dma_map_single(ndev->dev.parent, skb->data,
NIXGE_MAX_JUMBO_FRAME_SIZE,
DMA_FROM_DEVICE);
@@ -601,8 +601,8 @@ static int nixge_recv(struct net_device *ndev, int budget)
tail_p = priv->rx_bd_p + sizeof(*priv->rx_bd_v) *
priv->rx_bd_ci;
- skb = (struct sk_buff *)nixge_hw_dma_bd_get_addr(cur_p,
- sw_id_offset);
+ skb = (struct sk_buff *)(uintptr_t)
+ nixge_hw_dma_bd_get_addr(cur_p, sw_id_offset);
length = cur_p->status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK;
if (length > NIXGE_MAX_JUMBO_FRAME_SIZE)
@@ -643,7 +643,7 @@ static int nixge_recv(struct net_device *ndev, int budget)
nixge_hw_dma_bd_set_phys(cur_p, cur_phys);
cur_p->cntrl = NIXGE_MAX_JUMBO_FRAME_SIZE;
cur_p->status = 0;
- nixge_hw_dma_bd_set_offset(cur_p, new_skb);
+ nixge_hw_dma_bd_set_offset(cur_p, (uintptr_t)new_skb);
++priv->rx_bd_ci;
priv->rx_bd_ci %= RX_BD_NUM;
--
2.19.0
^ permalink raw reply related
* Re: [PATCH v2 net-next 2/2] dt-bindings: net: add support for Microchip KSZ9131 Ethernet PHY
From: Andrew Lunn @ 2018-09-27 19:45 UTC (permalink / raw)
To: Yuiko Oshino
Cc: davem, robh+dt, devicetree, f.fainelli, linux-kernel,
mark.rutland, m.felsch, Markus.Niebel, netdev, UNGLinuxDriver
In-Reply-To: <1538079415-6260-1-git-send-email-yuiko.oshino@microchip.com>
On Thu, Sep 27, 2018 at 04:16:55PM -0400, Yuiko Oshino wrote:
> Add support for Microchip Technology KSZ9131 10/100/1000 Ethernet PHY
>
> Signed-off-by: Yuiko Oshino <yuiko.oshino@microchip.com>
> ---
> .../devicetree/bindings/net/micrel-ksz90x1.txt | 29 +++++++++++++++++++++-
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt b/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
> index e22d8cf..d23d14a 100644
> --- a/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
> +++ b/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
> @@ -1,4 +1,4 @@
> -Micrel KSZ9021/KSZ9031 Gigabit Ethernet PHY
> +Micrel/Microchip KSZ9021/KSZ9031/KSZ9131 Gigabit Ethernet PHY
>
> Some boards require special tuning values, particularly when it comes
> to clock delays. You can specify clock delay values in the PHY OF
> @@ -64,6 +64,33 @@ KSZ9031:
> Attention: The link partner must be configurable as slave otherwise
> no link will be established.
>
> +KSZ9131:
> +
> + All skew control options are specified in picoseconds. The minimum
> + value is 0, and the maximum is property-dependent. The increment
> + step is 100ps. The default value is the neutral setting, so setting
> + rxc-skew-ps=<0> actually results in -700 picoseconds adjustment.
We also have:
KSZ9021:
All skew control options are specified in picoseconds. The minimum
value is 0, the maximum value is 3000, and it is incremented by 200ps
steps.
and
KSZ9031:
All skew control options are specified in picoseconds. The minimum
value is 0, and the maximum is property-dependent. The increment
step is 60ps. The default value is the neutral setting, so setting
rxc-skew-ps=<0> actually results in -900 picoseconds adjustment.
So this is the third meaning of 0.
How about making 0 mean 0. Have the range being -700 to 1800 and -700
to 800? KSZ9031 and KSZ9131 then use the same meaning of 0, with 0
actually meaning 0!
Andrew
^ permalink raw reply
* Re: bug: 'ethtool -m' reports spurious alarm & warning threshold values for QSFP28 transceivers
From: Eran Ben Elisha @ 2018-09-27 13:25 UTC (permalink / raw)
To: Neil Horman, Chris Preimesberger
Cc: linville@tuxdriver.com, netdev@vger.kernel.org
In-Reply-To: <20180926213436.GA4116@hmswarspite.think-freely.org>
> This is just a drive by guess, but I think this is a driver issue.
>
>
> Issue 1 seems like a red herring, cat doesn't modify output, nor does ethtool
> know if its output is going to a console or a pipe, its all the same. And given
> issue 2 (that the output of the thresholds, etc are spurriously changing and
> wrong), suggests that they are spurriously changing and wrong regardless of what
> cat does.
>
> That said, I think issue two is a problem with the mlx4 driver. Specifically
> that the driver is copying garbage data.
>
> The three ethtool functions at work here are:
> mlx4_en_get_module_info
> mlx4_en_get_module_eeprom
> mlx4_get_module_info
>
> When you run ethtool -m on this driver, the kernel calls mlx4_en_get_module_info
> to determine the length of the eeprom, and that value will be either 256 or 512
> bytes. Lets assume that the value is 256 for the sake of argument
>
> Next it calls mlx4_en_get_module_eeprom, passing in that size 256 to actually
> read the eeprom data, which in turn calls mlx4_get_module_info to fetch the data
> from hardware, again, passing in 256 as the size for the first call (theres a
> loop, but it will only get executed once in this scenario)
>
> mlx4_get_module_info then issues the appropriate mailbox commands to dump the
> eeprom. Here it starts to go sideways. The mailbox buffer allocated for the
> return data is of type mlx4_mad_ifc, which has some front matter information and
> a data buffer that is 192 bytes long!
>
> A little further down in the function, size gets restricted if the buffer
> crosses a page boundary, but given that the size is 256 on the first call here,
> and offset is zero on the first call, we're not crossing anything, so size
> remains unchanged.
>
> The output mailbox buffer outmad->data (a 192 byte array), then gets cast to a
> sturt mlx4_cable_info structure, which has its own internal data buffer that is
> only 48 bytes long.
Hi guys,
Thanks for digging into it.
Here are some observations I found:
1. Chris system has CX4 (which is served by mlx5 driver), all analysis
by Neil was done over mlx4 driver (which serves the older generation of
NICs. e.h CX3Pro).
2. In general, MAD commands are limited to 192 bytes of data.
3. CableInfo MAD command info is limited to 48 Bytes.
4. First check that mlx4_get_module_info is having:
if (size > MODULE_INFO_MAX_READ)
size = MODULE_INFO_MAX_READ;
So this is the info that were missing in the analysis. x <= 48 is
also returned by this function. No trash copy or overrun. It is
expected from the caller(also inside mlx4) to recall with new offset in
order to fetch more data.
5. I reviewed mlx5 driver, and it have reading mechanism (small diff:
via MCIA register and not via MAD)
Both drivers read up to 256 bytes. 0-127 (from page 0). and 128-256
(from page 0). Driver is not capable of reading over 256 bytes currently.
looking on qsfp.c parser in ethtool.c (user space), I see an
uninitialized bug issue that have caused bug #1 + #2.
Applied it locally solved the issue (Not showing alarm data, which
should be expected as driver do not fill it).
diff --git a/qsfp.c b/qsfp.c
index 32e195d12dc0..d196aa1753de 100644
--- a/qsfp.c
+++ b/qsfp.c
@@ -671,7 +671,7 @@ static void sff8636_dom_parse(const __u8 *id, struct
sff_diags *sd)
static void sff8636_show_dom(const __u8 *id, __u32 eeprom_len)
{
- struct sff_diags sd;
+ struct sff_diags sd = {0};
char *rx_power_string = NULL;
char power_string[MAX_DESC_SIZE];
int i;
I will soon post a fix for it.
Thanks,
Eran
Thanks,
Eran
>
> The memcpy in this functionthen copies cable_info->data to the buffer that gets
> returned to ethtool, but it copies size bytes (256), even though the source data
> buffer is only 48 bytes long. That 48 byte array is embedded in the larger 192
> byte structure, so there won't be a panic on the overrun, but theres no telling
> what garbage is in the buffer beyond those first 48 bytes. Even if the
> remaining 144 bytes have valid eeprom data, its less than the required 256
> bytes. The additional copy may cause a panic, but if the buffer commonly bumps
> up against other allocated memory, that will go unnoticed.
>
> after the memcpy, mlx4_get_module_info just returns the size of the passed in
> buffer (256), and so the calling function thinks its work is done, and lets the
> kernel send back the buffer with garbage data to ethtool.
>
> I think the mlx4 guys have some work to do here.
>
> My $0.02
> Neil
>
^ permalink raw reply related
* Re: bug: 'ethtool -m' reports spurious alarm & warning threshold values for QSFP28 transceivers
From: Neil Horman @ 2018-09-27 13:23 UTC (permalink / raw)
To: Andrew Lunn
Cc: Chris Preimesberger, linville@tuxdriver.com,
netdev@vger.kernel.org
In-Reply-To: <20180926215812.GD1251@lunn.ch>
On Wed, Sep 26, 2018 at 11:58:12PM +0200, Andrew Lunn wrote:
> > When you run ethtool -m on this driver, the kernel calls mlx4_en_get_module_info
> > to determine the length of the eeprom, and that value will be either 256 or 512
> > bytes.
>
> So it sounds like QSFP modules using 8636 are not supported. You would
> expect a size to be one of 256, 384, 512 or 640.
>
> > Next it calls mlx4_en_get_module_eeprom, passing in that size 256 to actually
> > read the eeprom data, which in turn calls mlx4_get_module_info to fetch the data
> > from hardware, again, passing in 256 as the size for the first call (theres a
> > loop, but it will only get executed once in this scenario)
> >
> > mlx4_get_module_info then issues the appropriate mailbox commands to dump the
> > eeprom. Here it starts to go sideways. The mailbox buffer allocated for the
> > return data is of type mlx4_mad_ifc, which has some front matter information and
> > a data buffer that is 192 bytes long!
>
> Which suggests all SFP dumps are broken as well, not just QSFP.
>
No, not at all. Each driver that implements a get_eeprom ethtool method, is
capable of doing multiple reads at various offsets, and filling up the user
buffer with real data. The bug here is that the mellanox data structures are
not sized properly vis a vis the amount of eeprom data that user space might
expect, or more specifically that the driver isn't smart enough to do several
small reads to fill up the full sized request buffer
Neil
> Oh dear.
>
> 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